Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bolt4+proposals: fix max_cltv_expiry calculation #1176

Merged
merged 3 commits into from
Jul 15, 2024
Merged

bolt4+proposals: fix max_cltv_expiry calculation #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f48c60f
BOLT4: include `min_final_cltv_expiry_delta` in `max_cltv_expiry` calc
ellemouton Jun 26, 2024
e49890c
BOLT4: include calculation for `total_cltv_delta` of a blinded path
ellemouton Jun 26, 2024
849092d
proposals: fix `max_cltv_expiry` value for final hop in example
ellemouton Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions 04-onion-routing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,18 @@ The creator of `encrypted_recipient_data` (usually, the recipient of payment):
- MUST create `encrypted_data_tlv` for each node in the blinded route (including itself).
- MUST include `encrypted_data_tlv.short_channel_id` and `encrypted_data_tlv.payment_relay` for each non-final node.
- MUST NOT include `encrypted_data_tlv.next_node_id`.
- MUST set `encrypted_data_tlv.payment_constraints` for each non-final node:
- MUST set `encrypted_data_tlv.payment_constraints` for each non-final node and MAY set it for the final node:
- `max_cltv_expiry` to the largest block height at which the route is allowed to be used, starting
from the final node and adding `encrypted_data_tlv.payment_relay.cltv_expiry_delta` at each hop.
from the final node's chosen `max_cltv_expiry` height at which the route should expire, adding
the final node's `min_final_cltv_expiry_delta` and then adding
`encrypted_data_tlv.payment_relay.cltv_expiry_delta` at each hop.
- `htlc_minimum_msat` to the largest minimum HTLC value the nodes will allow.
- If it sets `encrypted_data_tlv.allowed_features`:
- MUST set it to an empty array.
- MUST compute the total fees and cltv delta of the route as follows and communicate them to the sender:
- MUST compute the total fees and CLTV delta of the route as follows and communicate them to the sender:
- `total_fee_base_msat(n+1) = (fee_base_msat(n+1) * 1000000 + total_fee_base_msat(n) * (1000000 + fee_proportional_millionths(n+1)) + 1000000 - 1) / 1000000`
- `total_fee_proportional_millionths(n+1) = ((total_fee_proportional_millionths(n) + fee_proportional_millionths(n+1)) * 1000000 + total_fee_proportional_millionths(n) * fee_proportional_millionths(n+1) + 1000000 - 1) / 1000000`
- `total_cltv_delta = cltv_delta(0) + cltv_delta(1) + ... + cltv_delta(n) + min_final_cltv_expiry_delta`
- MUST create the `encrypted_recipient_data` from the `encrypted_data_tlv` as required in [Route Blinding](#route-blinding).

The writer of the TLV `payload`:
Expand Down
6 changes: 4 additions & 2 deletions proposals/route-blinding.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor

@valentinewallace valentinewallace Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@valentinewallace valentinewallace Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

also wants a `min_final_cltv_expiry_delta` of 12 though and so in the encrypted payload to herself,
she sets `max_cltv_expiry = 1212` and adds `cltv_expiry_delta` for each hop after that. Alice then
transmits the following information to the sender (most likely via an invoice):

* Blinded route: `[N(carol), B(bob), B(alice)]`
Expand All @@ -276,7 +278,7 @@ transmits the following information to the sender (most likely via an invoice):
* Encrypted data for blinded nodes:
* `encrypted_payload(alice)`:
* `path_id`: `payment_preimage`
* `max_cltv_expiry`: 1200
* `max_cltv_expiry`: 1212
* `encrypted_payload(bob)`:
* `outgoing_channel_id`: `scid_bob_alice`
* `fee_base_msat`: 100
Expand Down