Skip to content

Trampoline forwarding #3976

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

a-mpch
Copy link
Contributor

@a-mpch a-mpch commented Jul 30, 2025

Supersedes #3711
Rebase and fixes to a sooner main branch.

Some changes from the previous branch:

  • Changes how we handle the unblinded receiving without hardcoded values
  • Takes into account refactor changes of errors names and attributable errors.
  • Moves logic to outbound_payment.rs and actually trampolines retry if some error appears and we take care of not sending payment events.
  • Fixes tests for unblinded and blinded scenarios

What's missing:

  • Keep trampoline routing disabled (accept suggestions)

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jul 30, 2025

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!


#[cfg(not(any(test, feature = "_test_utils")))]
let retry_strategy = Retry::Attempts(3);
#[cfg(any(test, feature = "_test_utils"))]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really a fan on this one, I'd accept any idea

@a-mpch
Copy link
Contributor Author

a-mpch commented Jul 30, 2025

drafting while rebasing again and fixing minor issues of CI

@a-mpch a-mpch marked this pull request as draft July 30, 2025 23:16
a-mpch and others added 3 commits July 31, 2025 10:26
Ensure that the Trampolin onion's amount and CLTV values does not exceed
the limitations imposed by the outer onion.

Co-authored-by: Arik Sosman <[email protected]>
To process errors returned from downstream nodes when forwarding between
Trampoline nodes, we need to store information beyond what's available
in `HTLCPreviousHopData`, suc as the used hops as the newly generated
outer onion's session_priv.

To that end, we add a new variant to `HTLCSource` in this commit, which
futre-proofs mapping an outbound forward to an incoming MPP.

Co-authored-by: Arik Sosman <[email protected]>
The previously existing `HTLCDestination` do not map nicely to the
failure vent of a Trampoline forward, so we introduce a new variant to
fill the gap.

Co-authored-by: Arik Sosman <[email protected]>
{
// We use Unkonwn Failure code instead of TemporaryTrampolineFailure because errors uses u16 bolt04 codes.
let payment_failed_conditions = PaymentFailedConditions::new()
.expected_htlc_error_data(LocalHTLCFailureReason::UnknownFailureCode { code: 16409 }, &[0; 0]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really sure on testing like this or just add the TemporaryTrampolineFailure

@a-mpch a-mpch force-pushed the 2025-07-trampoline-forwarding branch 2 times, most recently from 4a72640 to ef09262 Compare August 1, 2025 14:44
Copy link

codecov bot commented Aug 1, 2025

Codecov Report

❌ Patch coverage is 85.20710% with 225 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.91%. Comparing base (664511b) to head (3d2fe8d).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/channelmanager.rs 83.05% 80 Missing and 10 partials ⚠️
lightning/src/ln/outbound_payment.rs 71.56% 56 Missing and 4 partials ⚠️
lightning/src/ln/onion_payment.rs 52.43% 38 Missing and 1 partial ⚠️
lightning/src/ln/onion_utils.rs 67.46% 26 Missing and 1 partial ⚠️
lightning/src/ln/blinded_payment_tests.rs 98.85% 5 Missing and 2 partials ⚠️
lightning/src/chain/channelmonitor.rs 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #3976     +/-   ##
=========================================
  Coverage   88.91%   88.91%             
=========================================
  Files         174      174             
  Lines      124232   125526   +1294     
  Branches   124232   125526   +1294     
=========================================
+ Hits       110455   111610   +1155     
- Misses      11301    11419    +118     
- Partials     2476     2497     +21     
Flag Coverage Δ
fuzzing 22.43% <12.77%> (-0.18%) ⬇️
tests 88.74% <85.20%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

In this commit, we expand our forwarding logic to do ad-hoc pathfinding
to subsequent Trampoline nodes, covering both blinded and unblinded scenarios.
We set a constant for retrying payments in Trampoline nodes up to 3 failures.

We further modify our error propagation logic to handle Trampoline forward
HTLC failures.

Co-authored-by: Arik Sosman <[email protected]>
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.

2 participants