[bug]: non-deterministic behavior when circular cross-channel type payments are attempted

[bitcoin-coder-bob]

Background

In a 2 node set up, there exists between the nodes 1 sat channel and one taproot asset channel.
Node A generates an asset invoice. Node A also then pays this same invoice using sats.
Node A's local balance on the sat channel should decresae, and Node A's asset balance on the asset channel should increase. What I have witnessed is ~20% chance that this desired result happens. The other 80% of the time Node A's sat channel local balance decreases, and the balances on the asset channel remain unchanged, effectively meaning node A has paid sats and received nothing in return. In both scenarios, the payment and invoice show successful states, and show that intented route was used.

Node A has adequate liquidity on their sat channel to make the payment in sats, and their asset channel has adequate liquidity to receive on the asset channel. Fee policy is not preventing the payment.

Your environment

Running in simnet locally
lnd v0.18.4-beta
lightning-terminal v0.14.0-alpha
taproot-assets v0.5.0
btcd v0.24.1
I do have trace level logging on for lnd and taproot-assets

Steps to reproduce

First evaluate the local/remote balances of both the sat and asset channel.
Then the invoice can be generated with tapchannelrpc.TaprootAssetChannelsClient.AddInoice.
The payment is then made with routerrpc.RouterClient.SendPaymentV2. The payment request looks like:

routerrpc.SendPaymentRequest{
			PaymentRequest:   paymentRequest.InvoiceResult.PaymentRequest,
			TimeoutSeconds:   60,
			AllowSelfPayment: true,
			FeeLimitMsat:  20000,
			RouteHints: []*lnrpc.RouteHint{
				{
					HopHints: []*lnrpc.HopHint{
						{
							NodeId:                    nodeB,
							ChanId:                    satChannelId,
							FeeBaseMsat:               0,
							FeeProportionalMillionths: 0,
							CltvExpiryDelta:           144,
						},
						{
							NodeId:                    nodeA,
							ChanId:                    assetChannelId,
							FeeBaseMsat:               0,
							FeeProportionalMillionths: 0,
							CltvExpiryDelta:           144,
						},
					},
				},
			},
		},

With trace level logging on for lnd and taproot assets, the logs become very verbose but a couple lines stand out to me:

In the case where the desired result happens (asset channel balances update) I see:
queueing keystone of ADD open circuit: (Chan ID=523:1:0, HTLC ID=1)->(Chan ID=513:1:0, HTLC ID=1)
and Closed completed SETTLE circuit for 3b437af9ae09c1551a3dea1c88687424fba8b19ba968f2576501d11ed0bf0577: (523:1:0, 1) <-> (513:1:0, 1) (where 3b437af9ae09c1551a3dea1c88687424fba8b19ba968f2576501d11ed0bf0577 happens to be the payment hash I can see in the invoice)

In the bad case where the asset channel balances do not update I see: queueing keystone of ADD open circuit: (Chan ID=523:1:0, HTLC ID=1)->(Chan ID=523:1:0, HTLC ID=0) and Closed completed SETTLE circuit for 3b437af9ae09c1551a3dea1c88687424fba8b19ba968f2576501d11ed0bf0577: (523:1:0, 1) <-> (523:1:0, 0)

This leads me to believe that the route being used is using the same channel for both hops. In both of these cases the 523 channel is the sats channel. I have never seen it use both hops being the asset channel (the 513 channel).

In the failure case I also see: [DBG] CRTR: Attempt 2 for payment 3b437af9ae09c1551a3dea1c88687424fba8b19ba968f2576501d11ed0bf0577 successfully sent to switch, route: 575044581392384 (99801090 mSAT) -> 17661713758531595845 (99799991 mSAT), cltv 759 which when I evaluate those channel ids in the route tells me that is is a correct sats -> asset route. Given that this is attempt 2 perhaps there is some sort of conflict with another attempt or htlc?

Happy to provide more logs if needed.

It feels like the route I provided in the route hints is merely a suggestion, and not being used. I am unsure if SendToRouteV2 would respect it.

Expected behavior

From Node A's perspective the sat channel's local balance should be decremented. On Node A's asset channel their sat local balance should increase as well as the amount of asset units reflected in the custom data. The payment should also settle (as it has).

Actual behavior

After the payment is made, checking the status shows Payment_SUCCEEDED. Inspecting the htlcs in this payment and the underlying route for the 1 lnrpc.HTLCAttempt revals the two hop's channel ids map to the same two channels supplied in the payment request's route hints. I also noted that the second hop's channel id appears to be in the scid format (and I checked it does map to the asset channel), however in the payment request I used a channel id format supplied from lncli listchannels.

Inspecting the invoice (using lnrpc.LookupInvoice) after the successful payment reveals a state of lnrpc.Invoice_SETTLED and for the correct sat amount.

The local balance (from Node A's persepctive) was properly decremented. Both local/remote balances (for both the sat amount and asset amount) on the asset channel remain unchanged.

I tossed in a custom log line around here to see if isAssetInvoice is true and in every case it is true.

Something pretty interesting is that I have had a 100% success rate if I do this same flow but using the CLI:

// make invoice
litcli -n simnet --tlscertpath ./lit2/lnd/tls.cert --rpcserver 127.0.0.1:45197 --macaroonpath ./lit2/tapd/data/simnet/admin.macaroon ln addinvoice --asset_id 558d6ee705a1fd37806e3e44d5df85727c5433c5d793a7a46010f3461c79448b  --asset_amount 100000


// pay invoice
lncli -n simnet --tlscertpath ./lit2/lnd/tls.cert --macaroonpath ./lit2/lnd/data/chain/bitcoin/simnet/admin.macaroon --rpcserver 127.0.0.1:45197 payinvoice --allow_self_payment --outgoing_chan_id 564049465114624 --pay_req  <the-payreq-from-above-command>

In these CLI command I do not specify the entire route, just the outgoing channel id, but other than that it is pretty similar to how I have created the invoice and paid it in the code using the rpc calls. Unclear why the CLI would always succeed and not doing this flow via the rpc.

[guggero]

I'm not 100% sure I fully understand the scenario yet. But two things look weird to me from the start:

1. What are satChannelId and assetChannelId in the payment request you showed? Is either of those the SCID alias that was specifically created during the RFQ negotiation? Or are those the "real" channel IDs?
2. If there are only 2 nodes, why are both node A and node B represented as a hop hint? Who is the sender and who is the recipient? The sender never needs to be present as a hop hint.

[bitcoin-coder-bob]

@guggero In the payment request the satChannelId is 523:1:0 (also represented as 575044581392384) and assetChannelId is 513:1:0 (also represented in scid form as 17661713758531595845. So the sat channel id is the "real" channel id, while the asset channel scid is the on of the scid alias in the Aliases field from the response of lnrpc.ListAliases. Since posting, I have removed the hop hints field from SendPayment and instead included them the same way when generating the invoice with tapchannelrpc.AddInvoice via the RouteHints field inside the InvoiceRequest field:

	paymentRequest, err := tapChannelClient.AddInvoice(context.TODO(), &tapchannelrpc.AddInvoiceRequest{
		AssetId:     assetHex,
		AssetAmount: uint64(bobAssetAmount),
		PeerPubkey: marketMakerHex,
		InvoiceRequest: &lnrpc.Invoice{
			Expiry: 300,
			RouteHints: []*lnrpc.RouteHint{
				{
					HopHints: []*lnrpc.HopHint{

						{
							NodeId: nodeB,
							ChanId: assetChannelScidAlias,
							FeeBaseMsat:               1000,
							FeeProportionalMillionths: 1,
							CltvExpiryDelta:           80,
						},
					},
				},
			},
		},
	})

so when I do SendPayment presumably it will use these route hints from the generated asset invoice:

paymentClient, err := rc.SendPaymentV2(
	context.TODO(), &routerrpc.SendPaymentRequest{
		PaymentRequest:    paymentRequest.InvoiceResult.PaymentRequest,
		TimeoutSeconds:    60,
		AllowSelfPayment:  true,
		OutgoingChanIds:   eligibleChannels,
		FeeLimitMsat: 200000,
		MaxParts:     16,

	},
)

Note I also cut out using a hop hint of node A, I just have node B in here now. A is both the invoice creator and the one who is paying the invoice, using B as the intermediary hop.

With all these changes expressed I am still witnessing the same behaviors described in original issue. Perhaps I am using the wrong scid for the ChanId field in the hop hints in the invoice? Which scid should I be using to reference the asset channel, given the scid changes over time?

Are the route hints provided in the invoice actually used when making the payment, or are they merely a suggestion? It feels like when the payment is made it considers the route hint, but does not necessarily use the route even if the route has adequate liquidity to service the payment.

[Roasbeef]

Are the route hints provided in the invoice actually used when making the payment, or are they merely a suggestion? It feels like when the payment is made it considers the route hint, but does not necessarily use the route even if the route has adequate liquidity to service the payment.

You don't need to manually populate hop hints at all. If you have an invoice, then hop hints are included in the invoice.

Can you try your scenario just using the vanilla invoice as is?

When you make an asset invoice, the hop hints are set up such that we only make one feasible path to the destination. Bitcoin invoices operate as normal.

It sounds like you're trying to pay an invoice back to yourself. When you have many channels between yourself and the other node, lnd will pick at runtime which channel it will go over. Unless you restrict both the outgoing and incoming channels.

[bitcoin-coder-bob]

@Roasbeef

You don't need to manually populate hop hints at all. If you have an invoice, then hop hints are included in the invoice.
To be clear you are saying I need not populate hop hints in SendPaymentRequest if the invoice had included them, correct? (if so, this is what I have in my setup)

Can you try your scenario just using the vanilla invoice as is?
I have removed RouteHints from the invoice. I still am witnessing the behavior where despite the payment saying it used the asset channel, it does not appear to be doing so in the bad behavior case. Here is that taproot asset invoice code I use:

	paymentRequest, err := tapChannelClient.AddInvoice(context.TODO(), &tapchannelrpc.AddInvoiceRequest{
		AssetId:     assetHex,
		AssetAmount: uint64(someAmount),
		PeerPubkey: nodeB,  // should I even include this?
		InvoiceRequest: &lnrpc.Invoice{
			Expiry: 300,
                        // do I need other fields here? I have removed route hints here
		},
	})

It sounds like you're trying to pay an invoice back to yourself. When you have many channels between yourself and the other node, lnd will pick at runtime which channel it will go over.

Correct. In my setup here I only have 1 sat channel and 1 asset channel between the 2 nodes. Both nodes do not have any other channels. However I would like going forward to be able to have more channels for each node (both with each other and with other peers), and to be able to specify specifically which channels to route the payment through.

Unless you restrict both the outgoing and incoming channels.
I see I can use OutgoingChanIds in SendPaymentRequest (which I am) but am unclear how to restrict the incoming channels.

I used this for the payment, where PaymentRequest comes from the above paymentRequest I made during the taproot invoice creation in the above code snippet:

paymentClient, err := rc.SendPaymentV2(
	context.TODO(), &routerrpc.SendPaymentRequest{
		PaymentRequest:    paymentRequest.InvoiceResult.PaymentRequest,
		TimeoutSeconds:    60,
		AllowSelfPayment:  true,
		OutgoingChanIds:   eligibleChannels, // I set this to only be 1 sat channel
		DestCustomRecords: make(map[uint64][]byte),
		FeeLimitMsat: 200000,
		MaxParts:     16,
	},
)

[bitcoin-coder-bob]

Added a few more details here: #1275 (comment)

In addition: looking into the payment from SendPaymentV2, looking into the htlcs field (my payment has 1 htlc) there are cases of 2 hops. The first hop is the channel id of a sats channel, and the second hop contains this SCID of an asset channel. The invoice shows as SETTLED and the payment SUCCEEDED. However I see no change in local_balance (sats) for the asset channel nor any change in the local_balance inside the CustomChannelData for the asset channel. Presumably then no asset was moved during this payment, despite the hops indicating the asset channel was used.

I also notice that if I remove RouteHints from the InvoiceRequest, it has no effect on the probability of manifesting this issue (payment succeeds, invoice settles, asset does not move across channel)

[guggero]

Do you have an automated (code-based) test setup that can reproduce this? If yes, would you mind sharing the code so we can see exactly what calls/values/params you're using?

[bitcoin-coder-bob]

Do you have an automated (code-based) test setup that can reproduce this? If yes, would you mind sharing the code so we can see exactly what calls/values/params you're using?

I have a playground set up in a private repo that I could share. It is not something I could port over publicly. I could provide access and instructions on how to run this setup. @guggero

[guggero]

Let me take a look at the logs you sent in #1275 first. If that doesn't give me enough hints on what's going on, I'll come back to your offer. Thanks!

Assuming the two problems (self-payment only working sometimes and balances not updating) both happen on the same code/test?

[bitcoin-coder-bob]

@guggero thank you for looking into this. To be clear, every payment is marked as SUCCEEDED and every invoice as SETTLED in this self-pay flow. The only difference is the happy case is when asset balance actually updates, and sad case is when asset balance does not update. So if I do 5 self payments I may get 2 happy cases and 3 sad cases. So my "test" may just be 1 payment, or it could be 5, but the random variance still applies to any number of payments set up in the test.