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.

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

Route blinding: support forwarding as the intro node #2540

Merged

Conversation

valentinewallace
Copy link
Contributor

@valentinewallace valentinewallace commented Aug 29, 2023

Adds support for being the intro node to a blinded path. As such, we won't advertise route blinding support in our feature bits yet.

The next PR (#2688) adds support for receiving to a multi-hop blinded path, to be followed by complete blinded forwarding support. #2688 also adds testing for the success case of forwarding-as-intro.

Helps address #1970.

Based on #2514, #2413

@codecov-commenter
Copy link

codecov-commenter commented Aug 29, 2023

Codecov Report

Attention: 56 lines in your changes are missing coverage. Please review.

Comparison is base (f07f4b9) 88.57% compared to head (6af786a) 88.63%.

Files Patch % Lines
lightning/src/ln/onion_utils.rs 37.03% 6 Missing and 11 partials ⚠️
lightning/src/ln/onion_payment.rs 76.92% 7 Missing and 5 partials ⚠️
lightning/src/ln/channel.rs 91.72% 3 Missing and 8 partials ⚠️
lightning/src/ln/channelmanager.rs 80.00% 9 Missing and 2 partials ⚠️
lightning/src/ln/blinded_payment_tests.rs 98.93% 3 Missing ⚠️
lightning/src/ln/msgs.rs 87.50% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2540      +/-   ##
==========================================
+ Coverage   88.57%   88.63%   +0.05%     
==========================================
  Files         115      115              
  Lines       89479    90023     +544     
  Branches    89479    90023     +544     
==========================================
+ Hits        79258    79788     +530     
+ Misses       7858     7840      -18     
- Partials     2363     2395      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Quick glance, nothing wild

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
lightning/src/ln/channel.rs Outdated Show resolved Hide resolved
lightning/src/ln/channel.rs Outdated Show resolved Hide resolved
@TheBlueMatt
Copy link
Collaborator

Also, what should/are we doing about blinded tails on probe calls? Presumably we should just drop the blinded tail or tweak the parameter to not included blinded tails?

@valentinewallace valentinewallace changed the title Error handling for blinded payments Route blinding: forwarding and error handling Sep 8, 2023
@valentinewallace valentinewallace modified the milestones: 0.0.117, 0.0.118 Sep 11, 2023
@jkczyz jkczyz self-requested a review September 22, 2023 17:33
@TheBlueMatt TheBlueMatt modified the milestones: 0.0.118, 0.0.119 Oct 12, 2023
@jkczyz jkczyz mentioned this pull request Oct 23, 2023
@valentinewallace valentinewallace changed the title Route blinding: forwarding and error handling Route blinding: support forwarding as the intro node Oct 26, 2023
@valentinewallace valentinewallace force-pushed the 2023-08-blinded-errors branch 3 times, most recently from 29b1d33 to 9c171a7 Compare October 31, 2023 17:30
@valentinewallace
Copy link
Contributor Author

Expanded test coverage a bit and broke out a few more commits. Think this is ready for review.

@valentinewallace valentinewallace marked this pull request as ready for review October 31, 2023 17:32
Copy link
Contributor

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

Still working through some of the commits

lightning/src/ln/channel.rs Outdated Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
@valentinewallace
Copy link
Contributor Author

valentinewallace commented Nov 29, 2023

Noted the follow-ups here: #2540 (review) in #1970.

Also, squashed.

A blinding point is provided in update_add_htlc messages if we are relaying or
receiving a payment within a blinded path, to decrypt the onion routing packet
and the recipient-provided encrypted payload within. Will be used in upcoming
commits.
Will be used in the next commit to parse onion errors from blinded paths in
tests only.
So we can make sure they're encoded properly.
A blinding point is provided in update_add_htlc messages if we are relaying or
receiving a payment within a blinded path, to decrypt the onion routing packet
and the recipient-provided encrypted payload within. Will be used in upcoming
commits.
We need to store the inbound blinding point in PendingHTLCRouting in order to
calculate the outbound blinding point.

The new BlindedForward struct will be augmented when we add support for
forwarding as a non-intro node.
Useful so we know to fail blinded intro node HTLCs back with an
invalid_onion_blinding error per BOLT 4.

Another variant will be added to the new Blinded enum when we support
receiving/forwarding as a non-intro node.
Useful so we know to fail back blinded HTLCs where we are the intro node with
the invalid_onion_blinding error per BOLT 4.

We don't set this field for blinded received HTLCs because we don't support
receiving to multi-hop blinded paths yet, and there's no point in setting it
for HTLCs received to 1-hop blinded paths because per the spec they should fail
back using an unblinded error code.
Used in the next commit to set the update_add blinding point on HTLC forward.
Used by the next hop to decode their blinded onion payload.
Previously, we only parsed blinded receive payloads.
@TheBlueMatt
Copy link
Collaborator

Needs rebase.

@@ -119,6 +119,8 @@ pub enum PendingHTLCRouting {
/// The SCID from the onion that we should forward to. This could be a real SCID or a fake one
/// generated using `get_fake_scid` from the scid_utils::fake_scid module.
short_channel_id: u64, // This should be NonZero<u64> eventually when we bump MSRV
/// Set if this HTLC is being forwarded within a blinded path.
blinded: Option<BlindedForward>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, this is public now, we should include a bit more details about why this is here and what its used for. Happy to just do that in #2762.

Copy link
Contributor Author

@valentinewallace valentinewallace Nov 30, 2023

Choose a reason for hiding this comment

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

Good point, doing it in #2762 sgtm. FWIW, there are more detailed docs on the BlindedForward struct itself.

@valentinewallace valentinewallace merged commit 74bc9e2 into lightningdevkit:main Dec 1, 2023
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants