ln: handle DecodedOnionFailure for local trampoline failures
What the developer wrote
Authored by Carla Kirk-Cohen
ln: handle DecodedOnionFailure for local trampoline failures
While proper error handling will be added in a followup, we add the
bare minimum required here for testing.
Note that we intentionally keep the behavior of not setting
`payment_failed_permanently` for local failures because we can possibly
retry it because we're the sender as a trampoline forwarder.
For example, a local ChannelClosed error is considered to be permanent,
but we can still retry along another channel.
What changed, and why it matters
This commit fixes a code path in the Lightning Dev Kit where a specific kind of local payment failure (called a 'TrampolineForward' failure) was not being handled. Previously, the code only expected failures from regular outbound routes and would panic with 'unreachable!' if a trampoline forward failure occurred. The change adds handling for that case, returning a decoded failure with no specific short channel ID. It is described by the author as a minimal fix for testing, with fuller error handling planned later.
Treat as a routine bug-fix commit rather than an urgent security patch. Reviewers should verify that the new TrampolineForward arm correctly propagates failure information without introducing incorrect permanent-failure classification, and monitor the promised follow-up for complete error handling.
Security signals we found
Removal of unreachable!() panic path for TrampolineForward HTLC source
Addition of local trampoline failure decoding in onion error handling
Intentional non-setting of payment_failed_permanently for local failures to allow retry
debug_assert used for runtime invariant (outbound_payment is None)
Commit message describes this as minimal/bare-bones fix pending follow-up proper error handling
Evidence from the diff
In lightning/src/ln/onion_utils.rs, the HTLCFailReason::decode_onion_failure method previously matched HTLCFailReasonRepr::Reason only against HTLCSource::OutboundRoute, and used unreachable!() for any other source. The patch introduces a helper closure decoded_onion_failure and adds a match arm for HTLCSource::TrampolineForward, with a debug_assert that outbound_payment is None. It intentionally leaves network_update as None and payment_failed_permanently as false for local trampoline failures, noting that retry may still be possible. The change is framed as the bare minimum needed for testing, with follow-up work planned.
Changed components
lightning/src/ln/onion_utils.rsHTLCFailReason::decode_onion_failureTrampoline payment forwarding logicInspect captured patch +28 / −17
diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs
index 4be803f..040139b 100644
--- a/lightning/src/ln/onion_utils.rs
+++ b/lightning/src/ln/onion_utils.rs
@@ -2141,33 +2141,44 @@ impl HTLCFailReason {
pub(super) fn decode_onion_failure<T: secp256k1::Signing, L: Logger>(
&self, secp_ctx: &Secp256k1<T>, logger: &L, htlc_source: &HTLCSource,
) -> DecodedOnionFailure {
+ let decoded_onion_failure = |short_channel_id: Option<u64>,
+ _failure_reason: LocalHTLCFailureReason,
+ _data: &[u8]| {
+ DecodedOnionFailure {
+ network_update: None,
+ payment_failed_permanently: false,
+ short_channel_id,
+ failed_within_blinded_path: false,
+ hold_times: Vec::new(),
+ #[cfg(any(test, feature = "_test_utils"))]
+ onion_error_code: Some(_failure_reason),
+ #[cfg(any(test, feature = "_test_utils"))]
+ onion_error_data: Some(_data.to_vec()),
+ #[cfg(test)]
+ attribution_failed_channel: None,
+ }
+ };
match self.0 {
HTLCFailReasonRepr::LightningError { ref err, .. } => {
process_onion_failure(secp_ctx, logger, &htlc_source, err.clone())
},
- #[allow(unused)]
HTLCFailReasonRepr::Reason { ref data, ref failure_reason } => {
// we get a fail_malformed_htlc from the first hop
// TODO: We'd like to generate a NetworkUpdate for temporary
// failures here, but that would be insufficient as find_route
// generally ignores its view of our own channels as we provide them via
// ChannelDetails.
- if let &HTLCSource::OutboundRoute { ref path, .. } = htlc_source {
- DecodedOnionFailure {
- network_update: None,
- payment_failed_permanently: false,
- short_channel_id: Some(path.hops[0].short_channel_id),
- failed_within_blinded_path: false,
- hold_times: Vec::new(),
- #[cfg(any(test, feature = "_test_utils"))]
- onion_error_code: Some(*failure_reason),
- #[cfg(any(test, feature = "_test_utils"))]
- onion_error_data: Some(data.clone()),
- #[cfg(test)]
- attribution_failed_channel: None,
- }
- } else {
- unreachable!();
+ match htlc_source {
+ &HTLCSource::OutboundRoute { ref path, .. } => decoded_onion_failure(
+ Some(path.hops[0].short_channel_id),
+ *failure_reason,
+ data,
+ ),
+ &HTLCSource::TrampolineForward { ref outbound_payment, .. } => {
+ debug_assert!(outbound_payment.is_none());
+ decoded_onion_failure(None, *failure_reason, data)
+ },
+ _ => unreachable!(),
}
},
}
Why this scored 26/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.