Fix async release before HTLC decode#4548
Fix async release before HTLC decode#4548valentinewallace wants to merge 1 commit intolightningdevkit:mainfrom
Conversation
Handle `ReleaseHeldHtlc` messages that arrive before the sender-side LSP has even queued the held HTLC for onion decoding. Unlike lightningdevkit#4106, which covers releases arriving after the HTLC is in `decode_update_add_htlcs` but before it reaches `pending_intercepted_htlcs`, this preserves releases that arrive one step earlier and would otherwise be dropped as HTLC not found. Co-Authored-By: HAL 9000 Co-Authored-By: Elias Rohrer <dev@tnull.de>
|
👋 Thanks for assigning @tnull as a reviewer! |
| } else { | ||
| htlc_forwards.push(pending_add); | ||
| } |
There was a problem hiding this comment.
When released_in_transit is true, the HTLC is pushed directly to htlc_forwards, bypassing the intercept_forward check at line 7535. In contrast, the existing release-from-pending_intercepted_htlcs path (lines 17316-17356) does check whether the forward should be intercepted (forward_needs_intercept_to_known_chan) before forwarding.
This means a held HTLC released early (before entering the decode pipeline) will skip user-level interception, while one released later (from pending_intercepted_htlcs) will be checked for interception. Should the else branch here also consider intercept_forward?
| } else { | |
| htlc_forwards.push(pending_add); | |
| } | |
| } else if intercept_forward { | |
| let intercept_id = intercept_id(); | |
| let mut pending_intercepts = | |
| self.pending_intercepted_htlcs.lock().unwrap(); | |
| match pending_intercepts.entry(intercept_id) { | |
| hash_map::Entry::Vacant(entry) => { | |
| if let Ok(intercept_ev) = | |
| create_htlc_intercepted_event(intercept_id, &pending_add) | |
| { | |
| log_debug!( | |
| logger, | |
| "Intercepting released held HTLC with ID {intercept_id}" | |
| ); | |
| let ev_entry = (intercept_ev, None); | |
| self.pending_events | |
| .lock() | |
| .unwrap() | |
| .push_back(ev_entry); | |
| entry.insert(pending_add); | |
| } | |
| }, | |
| hash_map::Entry::Occupied(_) => { | |
| debug_assert!(false, "Should never have two HTLCs with the same channel id and htlc id"); | |
| }, | |
| } | |
| } else { | |
| htlc_forwards.push(pending_add); | |
| } |
Review SummaryAfter a thorough review of all files and hunks in this PR, I found one issue: Inline comments posted:
Overall assessment: The core logic for the new |
Handle
ReleaseHeldHtlcmessages that arrive before the sender-side LSP has even queued the held HTLC for onion decoding. Unlike #4106, which covers releases arriving after the HTLC is indecode_update_add_htlcsbut before it reachespending_intercepted_htlcs, this preserves releases that arrive one step earlier and would otherwise be dropped as HTLC not found.Supersedes #4542