-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[2/3]: Support Forwarding of Blinded Payments #8160
[2/3]: Support Forwarding of Blinded Payments #8160
Conversation
b0f5ca8
to
2096b96
Compare
Will this be the last in the series to support forwarding? |
One more (WIP) to add and test error handling - it's quite delicate so I want to separate it out. |
2096b96
to
23df342
Compare
Important Auto Review SkippedAuto reviews are limited to the following labels: llm-review. Please add one of these labels to enable auto reviews. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
23df342
to
0683a2e
Compare
1892855
to
5fe3bb5
Compare
eb4b8d8
to
8ddafed
Compare
8ddafed
to
a5c3bb4
Compare
Rebase! Also re-confirmed interop testing here. |
Needs a rebase! |
a5c3bb4
to
9c2efac
Compare
htlcswitch/hop/iterator.go
Outdated
// route forwarding. Payloads will simply be rejected as invalid if | ||
// they contain blinding points, as our node does not want to forward | ||
// them. | ||
DisableBlindedFwd bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about if we can lift this up to a higher layer. So we never even try to decrypt/decode something when we don't support it.
I think not yet in the scope of these PRs, but we should also have path finding logic to not attempt to include a node in our blinded path if they don't support it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eg: we don't even need to call MakeBlindingKit
(just for it to reject the payload) if we don't support the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but we should also have path finding logic to not attempt to include a node in our blinded path if they don't support it.
Def, I imagine that would be for blinded path creation? Filter by feature bit and then make a path seems to be the norm.
Co-authored-by: Calvin Zachman <calvin.zachman@protonmail.com>
9c2efac
to
59d817f
Compare
Add an option to disable route blinding, failing back any HTLC with a blinding point set when we haven't got the feature enabled. Note that this commit only handles the case where we're chosen as the relaying node (where the blinding point is in update_add_htlc), we'll add handling for the introduction node case once we get to handling of blinded payloads).
This commit introduces a blinding kits which abstracts over the operations required to decrypt, deserialize and reconstruct forwarding data from an encrypted blob of data included for nodes in blinded routes.
When we have a HTLC that is part of a blinded route, we need to include the next ephemeral blinding point in UpdateAddHtlc for the next hop. The way that we handle the addition of this key is the same for introduction nodes and relaying nodes within the route.
To separate blinded route parsing from payload parsing, we need to return the parsed types map so that we can properly validate blinded data payloads against what we saw in the onion.
If we received a payload with a encrypted data point set, our forwarding information should be set from the information in our encrypted blob. This behavior is the same for introduction and relying nodes in a blinded route.
Reject any HTLCs that use us as an introduction point in a blinded route if we have disabled route blinding. We have to do this after we've processed the payload, because we only know we're an introduction point once we've processed the payload itself.
Note: the itest is broken up into multiple commits to make it more readable, they can be squashed post-review.
We don't support receiving blinded in this PR - just intercept and settle instead. The HTLC's arrival on the interceptor indicates that it was successfully forwarded on a blinded hop.
This commit turns off route blinding for the daemon while we're waiting on full handling for blinded errors. The feature remains on for itests so that tests covering blinding can run as usual.
59d817f
to
2188dd9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🥕
Thanks for considering those late-ish review comments, we're super close, let's keep this momentum going!
This PR is the second in a series to add the ability for LND to forward payments to blinded routes.
Reviewer Notes:
Replaces #7376
Fixes #7298