lightning / bolts

BOLT: Basis of Lightning Technology (Lightning Network Specifications)
2.1k stars 494 forks source link

Discussion: some confusion re blinded path `max_cltv` #1174

Closed ellemouton closed 2 months ago

ellemouton commented 3 months ago

Going through the example outlines in the route blinding proposal doc And I'm just a bit confused about the values used for CLTVs.

To recap the example a bit:

The rest of the example, where Erin pays via the route, because Erin chooses a final CLTV that is significantly below Alice's given maximum.

But let's say that Erin wants to play on the edge and instead chooses a final CLTV value of 1200. Erin would then add the total blinded route delta of 300 to get the CLTV value to use on C's incoming link: 1500. Ok so let's say C get's this incoming CLTV of 1300, then C will check it's encoded max_cltv from Alice, see that it is 1500 so the incoming value is ok. C then subtracts the 144 delta and sends B a CLTV of 1356. This is also ok given C's encoded max_cltv so he subtracts 144 and sends Alice a CLTV of 1212. But this then violates Alice's initial chosen max_cltv value of 1200.

My first thought was perhaps the max_cltv value for B should not have included A's min_final_cltv_expiry but all this would mean is that C's max_cltv will end up being 1488 which is then lower than the max_cltv+total_blinded_path_cltv value of 1500. Meaning we can have the same issue just earlier in the path.

This makes me wonder if either: 1) the recipient A should not be too strict about the max_cltv or should always account for the fact that it might be min_final_cltv_expiry less than they originally intended. 2) Or: we still need to communicate min_final_cltv_expiry to the recipient and hence not include it in the total blinded cltv calc

EDIT: apologies if this is the wrong spot for a question like this 🙏 lemme know where a better spot would be if so

ellemouton commented 3 months ago

I also think that perhaps some clarification is needed in BOLT 4 to make the answer to the above more clear as currently it is explicit about how the total_fee_base_msat and total_fee_proportional_millionths is calculated but not explicit about the total CLTV (if the CLTV should in fact include the min_final_cltv_delta as is done in the proposal doc, then we should add that to bolt 4.

One other place I think could use some clarity is around if we still expect the sender to add the extra blocks for the "shadow route" or if we can now just leave that up to the receiver since they now have the ability to easily create shadow hops. This was mentioned in this thread in the offers proposal but I think this suggestion should possibly be added to the current spec as it stands today.

Once there is some clarity here, im happy to open a PR 😊

t-bast commented 3 months ago

I agree this is currently a bit unclear, because there is currently no way in the spec to communicate a blinded route to a potential payer: that is introduced by the Bolt 12 PR, which is why I previously said that it was the responsibility of that PR (or any other PR that uses blinded paths) to detail how max_cltv should be computed. But maybe it's clearer to do it right now, directly in Bolt 4, and let protocols that use blinded paths simply refer to this default construction. If you feel like giving this a shot, I'll gladly review that PR!

But this then violates Alice's initial chosen max_cltv value of 1200.

You are completely correct, and that's a mistake in the example! And when implementing it in eclair, we actually got it right. The mistake is that in the encrypted_data for herself, Alice takes into account the min_final_cltv_expiry and sets max_cltv_expiry to 1212. Everything else remains the same, which works correctly:

This needs to be fixed in proposal doc, probably in the same PR that details how this should be computed!