Release held htlcs on release_held_htlc
What the developer wrote
Authored by Valentine Wallace
Release held htlcs on release_held_htlc
As part of supporting sending payments as an often-offline sender, the sender's
always-online channel counterparty needs to hold onto the sender's HTLC until
they receive a release_held_htlc onion message from the often-offline
recipient.
Here we implement forwarding these held HTLCs upon receipt of the release
message from the recipient.
What changed, and why it matters
This commit adds logic to actually forward HTLCs (payment promises) that were being held when a 'release_held_htlc' onion message arrives. Previously the handler only supported one kind of release (outbound static-invoice payments); now it also releases intercepted/forwards held by an always-online counterparty on behalf of an often-offline sender. It is a feature completion/fix for the async/offline-sender payment flow, not a clearly advertised security bug fix.
Review as part of async-payments feature audit. Verify that ReleaseHeldHtlc onion messages are authenticated and replay-protected, and that removing an intercept_id cannot be abused to release an HTLC prematurely or twice. No immediate patch deployment required solely on the basis of this diff.
Security signals we found
Functional completion of held-HTLC release path
Adds state transition from held to forwarded for intercepted HTLCs
No explicit security framing or CVE references in commit
No input validation changes beyond existing context matching
Potential concern: release message could be replayed or forged if onion message authentication is weak, but diff does not show new auth checks
Evidence from the diff
The patch extends ChannelManager::handle_release_held_htlc to handle AsyncPaymentsContext::ReleaseHeldHtlc { intercept_id }. It removes the HTLC from pending_intercepted_htlcs, clears the hold_htlc flag on PendingHTLCRouting::Forward, and forwards it via forward_htlcs. The change completes the held-HTLC release path for often-offline senders. There is no explicit security framing in the commit; it reads as a functional completion of an async-payments feature. The use of remove() and debug_assert! suggest normal invariants rather than a vulnerability remediation.
Changed components
lightning/src/ln/channelmanager.rsChannelManager::handle_release_held_htlcAsyncPaymentsContext::ReleaseHeldHtlc handlingpending_intercepted_htlcs mapPendingHTLCRouting::Forward hold_htlc fieldInspect captured patch +57 / −11
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 5a2cadb..e5b501f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -14781,18 +14781,64 @@ where
}
fn handle_release_held_htlc(&self, _message: ReleaseHeldHtlc, context: AsyncPaymentsContext) {
- let payment_id = match context {
- AsyncPaymentsContext::OutboundPayment { payment_id } => payment_id,
- _ => return,
- };
+ match context {
+ AsyncPaymentsContext::OutboundPayment { payment_id } => {
+ if let Err(e) = self.send_payment_for_static_invoice(payment_id) {
+ log_trace!(
+ self.logger,
+ "Failed to release held HTLC with payment id {}: {:?}",
+ payment_id,
+ e
+ );
+ }
+ },
+ AsyncPaymentsContext::ReleaseHeldHtlc { intercept_id } => {
+ let mut htlc = {
+ let mut pending_intercept_htlcs =
+ self.pending_intercepted_htlcs.lock().unwrap();
+ match pending_intercept_htlcs.remove(&intercept_id) {
+ Some(htlc) => htlc,
+ None => {
+ log_trace!(
+ self.logger,
+ "Failed to release HTLC with intercept_id {}: HTLC not found",
+ intercept_id
+ );
+ return;
+ },
+ }
+ };
+ match htlc.forward_info.routing {
+ PendingHTLCRouting::Forward { ref mut hold_htlc, .. } => {
+ debug_assert!(hold_htlc.is_some());
+ *hold_htlc = None;
+ },
+ _ => {
+ debug_assert!(false, "HTLC intercepts can only be forwards");
+ return;
+ },
+ }
- if let Err(e) = self.send_payment_for_static_invoice(payment_id) {
- log_trace!(
- self.logger,
- "Failed to release held HTLC with payment id {}: {:?}",
- payment_id,
- e
- );
+ let logger = WithContext::from(
+ &self.logger,
+ Some(htlc.prev_counterparty_node_id),
+ Some(htlc.prev_channel_id),
+ Some(htlc.forward_info.payment_hash),
+ );
+ log_trace!(logger, "Releasing held htlc with intercept_id {}", intercept_id);
+
+ let mut per_source_pending_forward = [(
+ htlc.prev_short_channel_id,
+ htlc.prev_counterparty_node_id,
+ htlc.prev_funding_outpoint,
+ htlc.prev_channel_id,
+ htlc.prev_user_channel_id,
+ vec![(htlc.forward_info, htlc.prev_htlc_id)],
+ )];
+ self.forward_htlcs(&mut per_source_pending_forward);
+ PersistenceNotifierGuard::notify_on_drop(self);
+ },
+ _ => return,
}
}
Why this scored 41/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.