prefactor: Allow multiple htlcs in/out in forwarding events for trampoline #4304
+656
−317
Conversation
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
In the commits that follow, we want to be able to free the other channel without emitting an event so that we can emit a single event for trampoline payments with multiple incoming HTLCs. We still want to go through the full claim flow for each incoming HTLC (and persist the EmitEventAndFreeOtherChannel event to be picked up on restart), but do not want multiple events for the same trampoline forward. Changing from upgradable_required to upgradable_option is forwards compatible - old versions of the software will always have written this field, newer versions don't require it to be there but will be able to read it as-is. This change is not backwards compatible, because older versions of the software will expect the field to be present but newer versions may not write it. An alternative would be to add a new event type, but that would need to have an even TLV (because the event must be understood and processed on restart to claim the incoming HTLC), so that option isn't backwards compatible either.
In preparation for trampoline failures, allow multiple previous channel ids. We'll only omit a single HTLCHandlingFailed for all of our failed back HTLCs, so we want to be able to express all of them in one event.
This commit adds a SendHTLCId for trampoline forwards, identified by their session_priv. As with an OutboundRoute, we can expect our HTLC to be uniquely identified by a randomly generated session_priv. TrampolineForward could also be identified by the set of all previous outbound scid/htlc id pairs that represent its incoming HTLC(s). We choose the 32 byte session_priv to fix the size of this identifier rather than 16 byte scid/id pairs that will grow with the number of incoming htlcs.
Co-authored-by: Arik Sosman <[email protected]> Co-authored-by: Maurice Poirrier <[email protected]>
Will need to share this code when we add trampoline forwarding. This commit exactly moves the logic as-is, in preparation for the next commit that will update to suit trampoline. Co-authored-by: Arik Sosman <[email protected]> Co-authored-by: Maurice Poirrier <[email protected]>
When we introduce trampoline forwards, we're going to want to provide two external pieces of information to create events: - When to emit an event: we only want to emit one trampoline event, even when we have multiple incoming htlcs. We need to make multiple calls to claim_funds_from_htlc_forward_hop to claim each individual htlc, which are not aware of each other, so we rely on the caller's closure to decide when to emit Some or None. - Forwarding fees: we will not be able to calculate the total fee for a trampoline forward when an individual outgoing htlcs is fulfilled, because there may be other outgoing htlcs that are not accounted for (we only get the htlc_claim_value_msat for the single htlc that was just fulfilled). In future, we'll be able to provide the total fee from the channelmanager's top level view.
Implement payment claiming for `HTLCSource::TrampolineForward` by iterating through previous hop data and claiming funds for each HTLC. Co-authored-by: Arik Sosman <[email protected]> Co-authored-by: Maurice Poirrier <[email protected]>
We'll want this extracted when we need to handle trampoline and regular forwards. Co-authored-by: Arik Sosman <[email protected]> Co-authored-by: Maurice Poirrier <[email protected]>
Implement failure propagation for `HTLCSource::TrampolineForward` by iterating through previous hop data and failing each HTLC with `TemporaryTrampolineFailure`. Note that testing should be implemented when trampoline forward is completed. Co-authored-by: Arik Sosman <[email protected]> Co-authored-by: Maurice Poirrier <[email protected]>
Move recovery logic for `HTLCSource::PreviousHopData` into `channel_monitor_recovery_internal` to prepare for trampoline forward reuse. Co-authored-by: Arik Sosman <[email protected]> Co-authored-by: Maurice Poirrier <[email protected]>
Implement channel monitor recovery for trampoline forwards iterating over all hop data and updating pending forwards.
|
👋 Thanks for assigning @valentinewallace as a reviewer! |
carlaKC
changed the title
carlaKC
commented
Jan 8, 2026
| node_id: prev_node_id_legacy, | ||
| }])), | ||
| (19, next_htlcs, (default_value, vec
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4304 +/- ##
==========================================
- Coverage 86.60% 86.51% -0.09%
==========================================
Files 158 158
Lines 102010 102381 +371
Branches 102010 102381 +371
==========================================
+ Hits 88342 88578 +236
- Misses 11258 11386 +128
- Partials 2410 2417 +7
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:
|
valentinewallace
requested review from
valentinewallace
and removed request for
joostjager
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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 is a prefactor to support trampoline in LDK. The last commit in #3976 contains the remainder of the trampoline logic, which may be useful to understanding the context for some of these refactors. These changes move us from a one HTLC in/ one HTLC out world to one where we allow multiple incoming and multiple outgoing HTLCs, to allow MPP trampoline:
In this world, we only want to emit one
PaymentForwarded/HTLCHandlingFailedevent per trampoline forward. We will receive and process aupdate_fail/fulfillfrom each of our downstream peers (D+E), and need to understand whether we want to emit an event:pending_outbound_payments. If we only fail back once the last outgoing HTLC is cleared, we'll only emit one event.🧹 A few of these commits can definitely be squashed - I went with splitting move/rename commits up and using a few
todo!s that are later filled in to help keep review more readable. I don't mind this, but happy to squash where people see fit!Also could use some tests - will add once approach has an ACK.