Full Trampoline Support#4414
Draft
carlaKC wants to merge 47 commits into
Draft
Conversation
|
👋 Hi! I see this is a draft PR. |
This was referenced Feb 12, 2026
Closed
carlaKC
force-pushed
the
2299-end-to-end
branch
2 times, most recently
from
0bfc08a to
8002678
Compare
carlaKC
force-pushed
the
2299-end-to-end
branch
from
8002678 to
1cf6cd1
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4414 +/- ##
==========================================
- Coverage 87.16% 86.33% -0.84%
==========================================
Files 161 157 -4
Lines 109251 109624 +373
Branches 109251 109624 +373
==========================================
- Hits 95230 94640 -590
- Misses 11547 12351 +804
- Partials 2474 2633 +159
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:
|
carlaKC
force-pushed
the
2299-end-to-end
branch
4 times, most recently
from
6e01fea to
28ca55f
Compare
carlaKC
force-pushed
the
2299-end-to-end
branch
3 times, most recently
from
3a8eb0f to
c4c9290
Compare
carlaKC
force-pushed
the
2299-end-to-end
branch
10 times, most recently
from
88e6907 to
3da50ac
Compare
carlaKC
force-pushed
the
2299-end-to-end
branch
5 times, most recently
from
3bb2c0c to
2a44a80
Compare
Review comment from valentinewallace on lightning/src/ln/blinded_payment_tests.rs:2861: > Generally preferred to use the public API in tests. I clauded this > patch but haven't vetted it much, let me know your thoughts: > valentinewallace@79a0771 Adopt the patch's core idea: * Extend `create_trampoline_forward_blinded_tail` to also return the underlying `BlindedPaymentPath` so callers can register it in `PaymentParameters`. Update the existing single-path caller to use `.0` on the new tuple. * Build `PaymentParameters::blinded(vec![path_bob, path_barry])` from the two paths and dispatch the MPP via `send_payment_with_route` instead of `test_add_new_pending_payment` + `test_send_payment_along_path`. The registered blinded paths are required, not decorative: `insert_previously_failed_blinded_path` (outbound_payment.rs:2508) is invoked on the trampoline rejection / timeout failures this test exercises, and `debug_assert!`s that the failing tail is registered in payment_params. Also bump Carol's outer-hop `cltv_expiry_delta` from `trampoline_cltv + excess_final_cltv` (112) to `carol_relay.cltv_ expiry_delta + trampoline_cltv + excess_final_cltv` (184). Required by `FixedRouter` (router.rs:765-773), which validates that the last outer- hop's `cltv_expiry_delta` equals `sum(trampoline_hops[*].cltv_expiry_ delta)`. That sum is `blinded_path.payinfo.cltv_expiry_delta + excess_final_cltv_delta`, and the payinfo aggregates the intermediate trampoline relay's `cltv_expiry_delta` (= `carol_relay.cltv_expiry_ delta` from `fwd_tail`) plus `min_final_cltv_expiry_delta` (= `trampoline_cltv`). The `test_send_payment_along_path` helpers bypassed this validation, so 112 worked there; the public API does not. Reverting to 112 panics with "Path had a total trampoline CLTV of 184, which is not equal to the total last-hop CLTV delta of 112". Comment the derivation at the call site so the overloaded "relay" naming doesn't trip up future readers. Return `last_hop_cltv_delta` from `send_trampoline_mpp_payment` so the on-chain timeout branch can compute Carol's incoming HTLC CLTV from the same value the route is built with, rather than re-stating the magic constant 184 in two places.
Use even persistence value because we can't downgrade with a trampoline payment in flight, we'll fail to claim the appropriate incoming HTLCs. We track previous_hop_data in `TrampolineForwardInfo` so that we have it on hand in our `OutboundPayment::Retryable` to build `HTLCSource` for our retries.
When we are forwarding as a trampoline within a blinded path, we need to be able to set a blinding point in the outer onion so that the next blinded trampoline can use it to decrypt its inner onion. This is only used for relaying nodes in the blinded path, because the introduction node's inner onion is encrypted using its node_id (unblinded) pubkey so it can retrieve the path key from inside its trampoline onion. Relaying nodes node_id is unknown to the original sender, so their inner onion is encrypted with their blinded identity. Relaying trampoline nodes therefore have to include the path key in the outer payload so that the inner onion can be decrypted, which in turn contains their blinded data for forwarding. This isn't used for the case where we're the sending node, because all we have to do is include the blinding point for the introduction node. For relaying nodes, we just put their encrypted data inside of their trampoline payload, relying on nodes in the blinded path to pass the blinding point along.
- [ ] Check whether we can get away with checking path.hops[0] directly (outbound_payment should always be present?)
ln: return appropriate error for trampoline failures
The blinding point that we pass in is supposed to be the "update add" blinding point equivalent, which in blinded trampoline relay is the one that we get in the outer onion.
We failed here to prevent downgrade to versions of LDK that didn't have full trampoline support. Now that we're done, we can allow reads.
To enable trampoline forwarding fully, remove the forced error introduced to prevent forwarding trampoline payments when we weren't ready.
Don't always blindly replace with a manually built test onion when we run trampoline tests (only for unblinded / failure cases where we need to mess with the onion). The we update our replacement onion logic to correctly match our internal behavior which adds one block to the current height when dispatching payments.
- [ ] Right now, we assume that the presence of a trampoline means
that we're in a blinded route. This fails when we test an
unblinded case (which we do to get coverage for forwarding).
We likely need to decouple trampoline and blinded tail to allow
this to work properly.
- [ ] TODO: should we always double wrap?
When we add handling for trampoline payments, we're going to need the full HTLCSource (with multiple prev_htlcs) to replay settles/claims. Here we update our existing logic to support tracking by source.
For trampoline, we have multiple outgoing HTLCs for our single source.
Taking the bluntest approach of storing all information for trampoline forwards as a first stab, can possibly reduce data later.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains all the changes to support trampoline forwarding in LDK.
It depends on #4304
and #4402 (and thus also #4373)Outstanding:
forwarding_amt_msatinPaymentForwardedfor trampolineHTLCHandlingFailureType::TrampolineForwardpayment_forwarded(if required?) / better expression ofskimmed_feenext_node_idwe receive across trampoline parts is consistentThis obviously needs to be broken up into parts, and could certainly use some cleaning up (specifically around replays), but opening it up early to provide some context for the decisions make in #4304.