fix: SCID preference for outbound payments on spliced channel #4376
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
TheBlueMatt
requested review from
TheBlueMatt
and removed request for
wpaulino
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Please fix rustfmt.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4376 +/- ##
==========================================
+ Coverage 86.02% 86.05% +0.02%
==========================================
Files 156 156
Lines 103100 103384 +284
Branches 103100 103384 +284
==========================================
+ Hits 88693 88965 +272
- Misses 11896 11903 +7
- Partials 2511 2516 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@TheBlueMatt, please, are all the checks needed to pass before my work gets reviewed?. Just asking, because while the checks are happening, maybe a new commit is added to main that causes a conflict, and before you know it, my check here is failing. |
okekefrancis112
force-pushed
the
prefer_outbound_scid
branch
from
30bf388 to
e00baa1
Compare
|
✅ Added second reviewer: @joostjager |
okekefrancis112
force-pushed
the
prefer_outbound_scid
branch
2 times, most recently
from
f6dd57d to
e00baa1
Compare
|
All checks pass now. @TheBlueMatt and @joostjager |
|
|
||
| // intermediate node failure | ||
| let short_channel_id = channels[1].0.contents.short_channel_id; | ||
| let short_channel_id = route.paths[0].hops[1].short_channel_id; |
There was a problem hiding this comment.
I think there is some independent refactoring/correctness improvement here that could go in a separate commit?
The commit that makes the actual change can then also contain the test changes/additions that it requires.
There was a problem hiding this comment.
Alright, I will work on it and get back to you. @joostjager
There was a problem hiding this comment.
Hi @joostjager, I have made the fix and pushed. Can you help me review?
okekefrancis112
force-pushed
the
prefer_outbound_scid
branch
from
e00baa1 to
a153dbe
Compare
|
You don't need to tag people to ask for review so urgently. There's a lot going on on the project, you can hit the review request button and people will get to it when they have a chance. |
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
basically lgtm, one nit otherwise great, thanks!
okekefrancis112
force-pushed
the
prefer_outbound_scid
branch
from
a153dbe to
c3ea75e
Compare
4 participants
Summary
Spliced channels can cause real SCIDs to change unexpectedly, which breaks the
assumption that they are more stable than SCID aliases. This PR corrects the
outbound payment SCID selection logic to reflect this reality.
Details
ChannelDetails::get_outbound_payment_scidnow prefers SCID aliases.This aligns outbound routing with current splicing behavior.
Tests that depended on the old assumption were updated accordingly.
Closes: #4249