bolt4+proposals: fix max_cltv_expiry calculation
#1176
Conversation
Include `min_final_cltv_expiry_delta` in the `max_cltv_expiry` calculation. Also add a note that indicates that this field may be set for the final node too. This is useful for the final node as then it does not need to persist the path expiry separately and can rely on just checking the `payment_relay` field when the payment arrives.
Include an explicit formula to use for determining the total CLTV delta of a blinded path so that it is clear that it should include the recipient's `min_final_cltv_expiry_delta`.
More info [here](lightning#1174 (comment)) outlining why the example needed to be updated.
There was a problem hiding this comment.
Thanks for writing this up! It looks good to me, @TheBlueMatt @jkczyz does this match what is implemented by LDK?
| @@ -261,7 +261,9 @@ total `cltv_expiry_delta`. This yields the following values: | |||
| * `route_cltv_expiry_delta`: 300 | |||
|
|
|||
| Let's assume the current block height is 1000. Alice wants the route to be used in the next 200 | |||
| blocks, so she sets `max_cltv_expiry = 1200` and adds `cltv_expiry_delta` for each hop. Alice then | |||
| blocks, meaning that the `max_cltv_expiry` she will communicate to the payer will be 1200. She | |||
There was a problem hiding this comment.
What is meant by "the max_cltv_expiry she will communicate to the payer"? Does that mean through the invoice's payinfo.cltv_expiry_delta? If so, why is that a delta while max_cltv_expiry is absolute? 🤔
AFAICT, LDK doesn't include min_final_cltv_expiry_delta in the encrypted payload calculation, but does include it in what Alice communicates to the payer (i.e., in payinfo.cltv_expiry_delta), which seems like the opposite of what is stated here.
@valentinewallace Could confirm / provide some insight here? You have a better understanding of this than I do. Background: #1174 (comment)
There was a problem hiding this comment.
^ That's all accurate from my understanding.
What is meant by "the max_cltv_expiry she will communicate to the payer"?
I tried to clarify this in the past via #1069, i.e the max_cltv_expiry should be conveyed via the invoice absolute expiry. But LDK doesn't do this lol, we just choose a very buffered absolute final CLTV expiry (2 weeks + latency grace period of 3 blocks). I think Elle's clarifications are still helpful as instructions for how to set a more minimal absolute expiry.
There was a problem hiding this comment.
Is my understanding right, though, that LDK doesn't include min_final_cltv_expiry_delta in the encrypted data calculation? That seems at odds with the change in the first commit.
There was a problem hiding this comment.
I believe we don't need to explicitly include min_final_cltv_expiry_delta in our encrypted data calculation because we add so much buffer as mentioned above. Because senders only see the invoice expiry (2 hours), the final expiry that they set shouldn't come close to violating our min final delta which is relative to the max_cltv_expiry (2 weeks). Does that make sense to you?
max_ctlv_expirythat is added to thepayment_constraintsfield forhops in a blinded path.
blinded path cltv delta so that it is clear that this should include the
min_final_cltv_expiry_delta.fixed so that the correct
max_cltv_expiryis included in the finalhop's payload.
Fixes #1174
The one other thing from #1174 that I didnt cover here was re the shadow route suggestions: for blinded paths. See comment here for more details. If it is agreed that it no longer makes sense for the sender to be responsible for adding the shadow route buffer, then maybe we should add that to the spec