tests: xpay issue and fixes#8972
Conversation
|
This also happens with This is a direct pay from l1 to l2 with an "any" offer aswell and a fetched invoice for 10000msat. Notice how both |
|
Weird. Some line above in the same test there is a BOLT11 test payment to a direct peer with no weird fees. Could it be the blinded path adding this fee? |
|
Very possible, look at the path from the other error i sometimes get above: |
|
I am able to reproduce the flaky test locally I am looking into it. It looks like some memory un-initialized in the bolt12 decoded structure. |
|
OK, so l2 gives a blinded path to l1, which starts with l1. l1 should realize that and not charge itself fees, but doesn't. Diagnosing now. |
|
Ah, this is cool! So, xpay specifies layers like so:
This means we don't set zero fees on the blinded path, since we apply layers in order (as documented). More interestingly, if you "fix" this, the payment fails: pyln.client.lightning.RpcError: RPC call failed: method: xpay, payload: {'invstring': 'lni1qqgpydqs7v2tvcagxe3hjfdmpmtp6q3qqc3xu3s3rg94nj40zfsy866mhu5vxne6tcej5878k2mneuvgjy83pmsrsx2ttuetmu92txqjepkyaaad9u55zp86qf734n5mg6dmd7yv7das9gyzx6nru5rnk63r2rytcajmhh6pj3hjxjzgw8zpx528wr4zulj8qgpry78qzse0fsxqyqt2cny0rk62ec88w8k2tvvn2hlu0s0gcf83mqgqxdv3zcmg9964yw30648fqaea7q2pr9u9h5zuc4jkqny9hzp6rn84tgvavct76pjlz7pulcx6gfecut7kyncsynjqf8eza33dhgzcfvcq2hhn4z38vmwamep7r2kv3ttq7fu5wzysqqe24akse2k5hndly934he0mvz6tyty904ujayfznh8yt05xk6rf706qagd4y7kfw8pl56wze3trz80685zpvggr8pzcqtf9kns8fn8a0nvtxwdyrhr4h7vh3gp5sqzyfdgag2c80xd9qgqxyfhyvyg6pdvu4tcjvpp7kkal9rp57wj7xv4pl3ajku70rzy3pafqyfcs2sq9sggrjrya2te7jnnnhrqjrpxjrzk76028y9fheacmxp4q9ps46kc00wq2plgql5pcr9947v4a7z49nqfvsmzw77kj722pqnaqylg6e6d5dxaklzx0x7cz49fmkr3eq50zhln30j7h5p49ajp09annwk66kt6dj0mjgml4lresyqem4qcvxj7dakxxynvcz8sf0w0pvqn0zsadum84u9qqdad7ke9ylsqyy5g8stg6yw6ljl9rgaj0meu4nq8nm2eak6dm4gnp03ykes2jq0fhcjw3ts237072hqstrsr0mhkttrr5nm3pnr5tvejxr92hwpcmamj5lksr83ytznav5pwp6dfym5jqg475rq4rvx374t7yncgstan8zukhhzrsqvh95sal4n8vg9gfeur0cjf8wcsh63lmsrjdflp3fc5nnnv6xpttnf0jawdrp6zzmzcj6wdpg8xchvz8yrazrsqqqqqpqqqqqzsqpvqqqqqqqqqqqqqqqqqqqwjcqqqqqq9yq35uk8g64qszza9777w4ua0pgtc6zjkgeqa4gvshfuf8k82434d05lr2kdx05592qgn3ptsrqgqqpvppqvuytqpdyk6wqaxvl47d3vee5swuwklej79qxjqqg394r4ptqaue4uzqj4p9mp3xku9a7dw4gwej0ah75et3mfwgn585qhx5knp7005ty6wp9wedpw43lell4svkhckxqd6hpvs48k2myjq2cp624226g5yaj4s'}, error: {'code': 209, 'message': "Failed after 1 attempts. Unexpected error (invalid_onion_blinding) from final node: disabling the invoice's blinded path (0x0x0/1) for this payment. Then routing failed: We could not find a usable set of paths. The destination has disabled 1 of 1 channels, leaving capacity only 0msat of 1515000msat."} This is because, to avoid probing, the (encrypted) inside of the blinded path insists that the fee be paid, for important obfuscation reasons. In summary, this is weird behavior, but we shouldn't fix it. If we override this check for local payments, it would allow any user of the node (who may not be privileged with the ability to use the node keys to decrypt the onion) to probe the next hop in this case. |
|
Oh, and the reason this changed? We use blinded paths for the case where our node doesn't advertize addresses: since l2's addresses are all localhost, they don't get advertized. So it uses an incoming node, and that's l1. This can be fixed by using |
This is, fascinatingly, a different bug! I've got a reproduce for exactly this, and as you suggested, I fixed decode to print it. I'll get to the bottom of this too... |
|
@daywalker90, @rustyrussell: false alarm this is not an issue it was just missing liquidity and expected behavior (see #9009). |
f6ac3ae to
933010c
Compare
|
Your diagnosis is correct, but it should always choose l1 (which has incoming capacity) not l3 (which does not). It is a corner case, though. The final commit here fixes this. |
933010c to
bb7b31b
Compare
…n a direct route Changelog-None
Weird we didn't print this before! Reported-by: https://github.com/Lagrang3 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It depends on gossip_store order, so it's not reliable. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We simply store it in the db, and return it with `listoffers` for now. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When an invoice request comes in, this will let us know that we need to limit ourselves to the blinded paths, as they have been forced by the user. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ths. Now we know whether blinded paths in the offer were added explicitly (by the fronting_nodes option to the `offer command, or via the `payment-fronting-node` config option) we know whether we need to use them when minting an invoice. Previously when I introduced fronting nodes in this release, we *always* reused them. But that's not quite right: the blinded path for the offer can use any peer, but for the invoice we need to use a peer with sufficient capacity to pay us. For some offers we won't even know the amount at the time the offer is made. So now we only force reuse of the offer's paths when they were set by the user in the first place: otherwise, we can be more flexible in path selection when minting the invoice. This is consistent with what we did before introducing fronting. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This lets us reliably eliminate peers which can't give us anything when seeking blinded paths for invoices. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
|
Please review. I decided to fix this properly, especially since it's a regression. The change which caused this regression is that (once we added fronting), I made invoices always use a subset of the offers' blinded paths. But this unhealthily reduces our ability to adapt when we produce the invoice: the offer only needs a blinded path which can be used to get messages to us, but the invoice needs a channel with sufficient funds. The fix is to note when we've forced creation of blinded paths in an offer, and only then restrict our paths on the invoice. |
bb7b31b to
fd11943
Compare
I noticed this in one of my plugin's tests.
This is also quite flaky on my local tests with this result instead:
pyln.client.lightning.RpcError: RPC call failed: method: xpay, payload: {'invstring': '<removed>'}, error: {'code': 205, 'message': 'Failed: We could not find a usable set of paths. The shortest path is 103x1x0->103x2x0->0x0x0, but 0x0x0/1 exceeds htlc_maximum_msat ~0msat'}