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

Blinded Paths: clarify outgoing_cltv_value for final hop #1097

Merged
merged 2 commits into from
Jul 31, 2023
Merged

Blinded Paths: clarify outgoing_cltv_value for final hop #1097

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dec7fa
Fix undeclared reference in onion-routing
TheBlueMatt Apr 21, 2023
dd0af11
Correct final_cltv handling in blinded paths
TheBlueMatt Apr 21, 2023
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
6 changes: 5 additions & 1 deletion 04-onion-routing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,18 @@ The creator of `encrypted_recipient_data` (usually, the recipient of payment):
- `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`
- MUST create the `encrypted_recipient_data` from the `encrypted_data_tlv` as required in [Route Blinding](#route-blinding).

The writer of `tlv_payload`:
The writer of the TLV `payload`:

- For every node inside a blinded route:
- MUST include the `encrypted_recipient_data` provided by the recipient
- For the first node in the blinded route:
- MUST include the `blinding_point` provided by the recipient in `current_blinding_point`
- If it is the final node:
- MUST include `amt_to_forward`, `outgoing_cltv_value` and `total_amount_msat`.
- The value set for `outgoing_cltv_value`:
- MUST use the current block height as a baseline value.
Copy link
Contributor

@ProofOfKeags ProofOfKeags Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If I understand this correctly, the outgoing_cltv_value is a replacement integrity check on the final onion packet to make sure that nodes along the route haven't laid claim to larger shares of the original CLTV budget than they were alotted.

Looking here it states that the final node:

MUST return an error if incoming cltv_expiry < current_block_height + min_final_cltv_expiry_delta.

It seems to me that if I read your change as written then the final HTLC can be exactly equal to the current block height. This means that without the random offset (specified as SHOULD), then there is zero "headroom" to resolve the HTLC.

Maybe this edge case is allowable, but it does seem to me that we should EITHER:

  1. relax the baseline from MUST to SHOULD
  2. constrict the offset recommendation from SHOULD to MUST
  3. add a mandatory offset > 0 to the current block height as the baseline

Leaving this as is is also acceptable but it would mean that valid implementations of the spec can run into this weird edge case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems to me that if I read your change as written then the final HTLC can be exactly equal to the current block height. This means that without the random offset (specified as SHOULD), then there is zero "headroom" to resolve the HTLC.

This is not an issue, for two different reasons:

  1. The same issue exists with non-blinded payments: this is why implementations currently already add a small random offset to the current block height to avoid this issue
  2. Even if the sender doesn't add a random offset, the recipient will add one in the blinded path's total cltv_expiry_delta, to make sure it covers its min_final_cltv_expiry_delta (to allow the recipient enough time before fulfilling this HTLC without risk of the downstream node using the HTLC-timeout path)

- if a [random offset](07-routing-gossip.md#recommendations-for-routing) was added to improve privacy:
- SHOULD add the offset to the baseline value.
- MUST NOT include any other tlv field.
- For every node outside of a blinded route:
- MUST include `amt_to_forward` and `outgoing_cltv_value`.
Expand Down