← Watch feed
ln: add failure_type helper to HTLCSource for HTLCHandlingFailureType
Public commit record
85/100 · Strong
What the developer wrote
Authored by Carla Kirk-Cohen
ln: add failure_type helper to HTLCSource for HTLCHandlingFailureType
To create the right handling type based on source, add a helper. This
is mainly useful for PreviousHopData/TrampolineForward. This helper
maps an OutboundRoute to a HTLCHandlingFailureType::Forward. This value
isn't actually used once we reach `forward_htlc_backwards_internal`,
because we don't emit `HTLCHandlingFailed` events for our own payments.
This issue is pre-existing, and could be addressed with an API change
to the failure function, which is left out of scope of this work.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This commit is a small internal cleanup in a Lightning Network library. It adds a helper method so the code can pick the correct failure classification (regular forward vs. trampoline forward) based on where an HTLC came from, instead of always assuming a regular forward. The commit message explicitly says the wrong value for the node’s own outbound payments was already not used, so this is mostly preparatory/refactoring work rather than a fix for an active security bug.
Recommended action
Treat as a low-risk refactoring/correctness improvement. Reviewers should verify that all call sites now pass the source-derived failure type and that `fail_htlc_backwards_internal` handles `TrampolineForward` correctly downstream. No urgent security response is indicated by the commit itself.
Security signals we found
01
Adds type-safe helper to avoid misclassifying trampoline forwards as regular forwards
02
Replaces hardcoded Forward failure type with source-derived failure type
03
Commit message states the incorrect value for OutboundRoute was pre-existing and unused
04
Adds defensive assertions/comments about trampoline payment handling
05
No explicit vulnerability, CVE, or security advisory referenced in commit
Evidence from the diff
The patch introduces HTLCSource::failure_type(...) -> HTLCHandlingFailureType, mapping PreviousHopData/OutboundRoute to HTLCHandlingFailureType::Forward and TrampolineForward to HTLCHandlingFailureType::TrampolineForward. It replaces several inline Forward { ... } constructions with calls to this helper. The commit notes that for OutboundRoute the returned value is not actually consumed by fail_htlc_backwards_internal because no HTLCHandlingFailed event is emitted for the node’s own payments, calling that a pre-existing issue out of scope. Two debug_assert!s and comments are added clarifying that trampoline payments are not processed as forwards in the affected code paths.
Changed components
lightning/src/ln/channelmanager.rsHTLCSource enum and failure classification logicfail_htlc_backwards_internal callersTrampoline forward handling pathsInspect captured patch +62 / −37
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 053f8fe..69cf85a 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -839,6 +839,26 @@ mod fuzzy_channelmanager {
},
}
+ impl HTLCSource {
+ pub fn failure_type(
+ &self, counterparty_node: PublicKey, channel_id: ChannelId,
+ ) -> HTLCHandlingFailureType {
+ match self {
+ // We won't actually emit an event with HTLCHandlingFailure if our source is an
+ // OutboundRoute, but `fail_htlc_backwards_internal` requires that we provide it.
+ HTLCSource::PreviousHopData(_) | HTLCSource::OutboundRoute { .. } => {
+ HTLCHandlingFailureType::Forward {
+ node_id: Some(counterparty_node),
+ channel_id,
+ }
+ },
+ HTLCSource::TrampolineForward { .. } => {
+ HTLCHandlingFailureType::TrampolineForward {}
+ },
+ }
+ }
+ }
+
/// Tracks the inbound corresponding to an outbound HTLC
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
pub struct HTLCPreviousHopData {
@@ -4043,12 +4063,9 @@ impl<
for htlc_source in failed_htlcs.drain(..) {
let failure_reason = LocalHTLCFailureReason::ChannelClosed;
let reason = HTLCFailReason::from_failure_code(failure_reason);
- let receiver = HTLCHandlingFailureType::Forward {
- node_id: Some(*counterparty_node_id),
- channel_id: *chan_id,
- };
let (source, hash) = htlc_source;
- self.fail_htlc_backwards_internal(&source, &hash, &reason, receiver, None);
+ let failure_type = source.failure_type(*counterparty_node_id, *chan_id);
+ self.fail_htlc_backwards_internal(&source, &hash, &reason, failure_type, None);
}
let _ = self.handle_error(shutdown_result, *counterparty_node_id);
@@ -4210,11 +4227,8 @@ impl<
let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
let failure_reason = LocalHTLCFailureReason::ChannelClosed;
let reason = HTLCFailReason::from_failure_code(failure_reason);
- let receiver = HTLCHandlingFailureType::Forward {
- node_id: Some(counterparty_node_id),
- channel_id,
- };
- self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver, None);
+ let failure_type = source.failure_type(counterparty_node_id, channel_id);
+ self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, failure_type, None);
}
if let Some((_, funding_txo, _channel_id, monitor_update)) = shutdown_res.monitor_update {
debug_assert!(false, "This should have been handled in `convert_channel_err`");
@@ -7657,6 +7671,8 @@ impl<
};
failed_forwards.push((
+ // This can't be a trampoline payment because we don't process them
+ // as forwards (we're the last/"receiving" onion node).
HTLCSource::PreviousHopData(prev_hop),
payment_hash,
HTLCFailReason::reason(reason, err_data),
@@ -7767,6 +7783,10 @@ impl<
continue;
}
} else {
+ debug_assert!(
+ false,
+ "We only expect to handle regular forwards in forwarding_channel_not_found"
+ );
let msg =
format!("Unknown short channel id {} for forward HTLC", short_chan_id);
failure_handler(
@@ -8935,11 +8955,14 @@ impl<
for (htlc_src, payment_hash) in htlcs_to_fail.drain(..) {
let reason = HTLCFailReason::reason(failure_reason, onion_failure_data.clone());
- let receiver = HTLCHandlingFailureType::Forward {
- node_id: Some(counterparty_node_id.clone()),
- channel_id,
- };
- self.fail_htlc_backwards_internal(&htlc_src, &payment_hash, &reason, receiver, None);
+ let failure_type = htlc_src.failure_type(*counterparty_node_id, channel_id);
+ self.fail_htlc_backwards_internal(
+ &htlc_src,
+ &payment_hash,
+ &reason,
+ failure_type,
+ None,
+ );
}
}
@@ -9910,11 +9933,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
self.finalize_claims(finalized_claimed_htlcs);
for failure in failed_htlcs {
- let receiver = HTLCHandlingFailureType::Forward {
- node_id: Some(counterparty_node_id),
- channel_id,
- };
- self.fail_htlc_backwards_internal(&failure.0, &failure.1, &failure.2, receiver, None);
+ let failure_type = failure.0.failure_type(counterparty_node_id, channel_id);
+ self.fail_htlc_backwards_internal(
+ &failure.0,
+ &failure.1,
+ &failure.2,
+ failure_type,
+ None,
+ );
}
self.prune_persisted_inbound_htlc_onions(
channel_id,
@@ -12062,13 +12088,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
}
for htlc_source in dropped_htlcs.drain(..) {
- let receiver = HTLCHandlingFailureType::Forward {
- node_id: Some(counterparty_node_id.clone()),
- channel_id: msg.channel_id,
- };
- let reason = HTLCFailReason::from_failure_code(LocalHTLCFailureReason::ChannelClosed);
let (source, hash) = htlc_source;
- self.fail_htlc_backwards_internal(&source, &hash, &reason, receiver, None);
+ let failure_type = source.failure_type(*counterparty_node_id, msg.channel_id);
+ let reason = HTLCFailReason::from_failure_code(LocalHTLCFailureReason::ChannelClosed);
+ self.fail_htlc_backwards_internal(&source, &hash, &reason, failure_type, None);
}
Ok(())
@@ -13111,10 +13134,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
} else {
log_trace!(logger, "Failing HTLC from our monitor");
let failure_reason = LocalHTLCFailureReason::OnChainTimeout;
- let receiver = HTLCHandlingFailureType::Forward {
- node_id: Some(counterparty_node_id),
- channel_id,
- };
+ let failure_type =
+ htlc_update.source.failure_type(counterparty_node_id, channel_id);
let reason = HTLCFailReason::from_failure_code(failure_reason);
let completion_update = Some(PaymentCompleteUpdate {
counterparty_node_id,
@@ -13126,7 +13147,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
&htlc_update.source,
&htlc_update.payment_hash,
&reason,
- receiver,
+ failure_type,
completion_update,
);
}
@@ -15579,8 +15600,8 @@ impl<
for (source, payment_hash) in timed_out_pending_htlcs.drain(..) {
let reason = LocalHTLCFailureReason::CLTVExpiryTooSoon;
let data = self.get_htlc_inbound_temp_fail_data(reason);
- timed_out_htlcs.push((source, payment_hash, HTLCFailReason::reason(reason, data),
- HTLCHandlingFailureType::Forward { node_id: Some(funded_channel.context.get_counterparty_node_id()), channel_id: *channel_id }));
+ let failure_type = source.failure_type(funded_channel.context.get_counterparty_node_id(), *channel_id);
+ timed_out_htlcs.push((source, payment_hash, HTLCFailReason::reason(reason, data), failure_type));
}
let logger = WithChannelContext::from(&self.logger, &funded_channel.context, None);
match funding_confirmed_opt {
@@ -20080,11 +20101,15 @@ impl<
for htlc_source in failed_htlcs {
let (source, hash, counterparty_id, channel_id, failure_reason, ev_action) =
htlc_source;
- let receiver =
- HTLCHandlingFailureType::Forward { node_id: Some(counterparty_id), channel_id };
+ let failure_type = source.failure_type(counterparty_id, channel_id);
let reason = HTLCFailReason::from_failure_code(failure_reason);
- channel_manager
- .fail_htlc_backwards_internal(&source, &hash, &reason, receiver, ev_action);
+ channel_manager.fail_htlc_backwards_internal(
+ &source,
+ &hash,
+ &reason,
+ failure_type,
+ ev_action,
+ );
}
for ((_, hash), htlcs) in already_forwarded_htlcs.into_iter() {
for (htlc, _) in htlcs {
Risk score
Our methodology →Why this scored 20/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.