-
Notifications
You must be signed in to change notification settings - Fork 384
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
Support constructing blinded path onion keys #2411
Support constructing blinded path onion keys #2411
Conversation
15ba6d4
to
769e222
Compare
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #2411 +/- ##
==========================================
+ Coverage 90.35% 91.02% +0.67%
==========================================
Files 106 106
Lines 56242 62890 +6648
Branches 56242 62890 +6648
==========================================
+ Hits 50816 57246 +6430
- Misses 5426 5644 +218
☔ View full report in Codecov by Sentry. |
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.
Looks pretty good!
769e222
to
87a64bc
Compare
CI failure is #2385. |
d230f57
to
b694acd
Compare
Rebased to fix CI. |
lightning/src/ln/onion_utils.rs
Outdated
let route_hop = match route_hop_opt { | ||
Some(hop) => hop, | ||
None => { | ||
res = Some((None, None, true)); |
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.
For all these lines where we set res = Some((None, None, true));
when the error is from a blinded hop, do we set them to be retryable just because we don't have a way of knowing otherwise?
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.
Yeah, if we return payment_retryable = false
here then we mark the payment as abandoned in outbound_payment
, which would be bad since it would prevent further payment retries. A payment is only not retryable if the recipient explicitly rejects it, which we have no way of knowing with blinded path errors.
Get rid of a bunch of indentation and be more idiomatic.
And fix an its vs it's grammar
b694acd
to
4af6d98
Compare
4af6d98
to
80a949d
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.
Wondering was there any reason the "Add missing test coverage for bogus err packet with valid hmac" and "Struct-ify decoded onion failures" commits got dropped?
80a949d
to
d858987
Compare
I was planning to split them into a different PR with another onion failure decode refactor, but ended up tabling that refactor for now. So, I re-added them here, sorry for the confusion lol. |
We don't bother actually parsing errors from within a blinded path, since all errors should be wiped by the introduction node by the time it gets back to us (the sender).
Useful for generating a next hop blinding point when forwarding a blinded payment.
To avoid several long hard-to-read tuple return values.
d858987
to
7b31712
Compare
Lays some groundwork for route blinding support, helping address #1970.
Prior to this PR we would only construct onion keys for unblinded path hops, and also only parse onion failures from unblinded path hops. This is tested in #2413.