ln: remove incoming trampoline secret from HTLCSource
What the developer wrote
Authored by Carla Kirk-Cohen
ln: remove incoming trampoline secret from HTLCSource
We don't need to track a single trampoline secret in our HTLCSource
because this is already tracked in each of our previous hops contained
in the source. This field was unnecessarily added under the belief that
each inner trampoline onion we receive for inbound MPP trampoline would
have the same session key.
It can be removed with breaking changes to persistence because we
currently refuse to decode trampoline forwards, and will not read
HTLCSource::Trampoline to prevent downgrades.
What changed, and why it matters
This commit removes a redundant 32-byte shared secret field from an internal data structure used when forwarding trampoline payments in the Lightning Dev Kit. The developers realized the secret was already stored inside each previous hop's data, so keeping a single copy at the top level was unnecessary. The change also updates serialization so older saved state cannot be cleanly loaded, but the code currently refuses to decode trampoline forwards anyway, so that downgrade risk is intentional and noted.
Treat as a cleanup/refactoring commit with a noted but accepted persistence-compatibility break. Review that any future trampoline-forward enablement correctly handles the absence of the top-level secret and relies on per-hop data. No immediate security patch is required based on the diff alone.
Security signals we found
Removes redundant secret field from in-memory/persisted state
Changes TLV serialization layout for HTLCSource::TrampolineForward
Breaking persistence change acknowledged by commit author
Currently no trampoline forward decoding means persisted variant is effectively unreachable
No direct memory-safety, cryptographic, or authorization flaw visible in diff
Evidence from the diff
The patch deletes incoming_trampoline_shared_secret: [u8; 32] from HTLCSource::TrampolineForward in lightning/src/ln/channelmanager.rs. It updates Hash, Writeable, and failure-handling code to use trampoline_shared_secret stored per HTLCPreviousHopData instead. Serialization TLV tag 3 is repurposed from the removed secret to outbound_payment, which is a breaking persistence change. The commit message states this is acceptable because trampoline forwards are currently refused to decode and downgrades are prevented.
Changed components
lightning/src/ln/channelmanager.rsHTLCSource::TrampolineForwardTrampoline payment forwarding failure pathHTLCSource serialization (Writeable)Inspect captured patch +10 / −22
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 939e004..b822b9a 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -873,7 +873,6 @@ mod fuzzy_channelmanager {
/// We might be forwarding an incoming payment that was received over MPP, and therefore
/// need to store the vector of corresponding `HTLCPreviousHopData` values.
previous_hop_data: Vec<HTLCPreviousHopData>,
- incoming_trampoline_shared_secret: [u8; 32],
/// Track outbound payment details once the payment has been dispatched, will be `None`
/// when waiting for incoming MPP to accumulate.
outbound_payment: Option<TrampolineDispatch>,
@@ -978,14 +977,9 @@ impl Hash for HTLCSource {
first_hop_htlc_msat.hash(hasher);
bolt12_invoice.hash(hasher);
},
- HTLCSource::TrampolineForward {
- previous_hop_data,
- incoming_trampoline_shared_secret,
- outbound_payment,
- } => {
+ HTLCSource::TrampolineForward { previous_hop_data, outbound_payment } => {
2u8.hash(hasher);
previous_hop_data.hash(hasher);
- incoming_trampoline_shared_secret.hash(hasher);
if let Some(payment) = outbound_payment {
payment.payment_id.hash(hasher);
payment.path.hash(hasher);
@@ -9402,11 +9396,7 @@ impl<
None,
));
},
- HTLCSource::TrampolineForward {
- previous_hop_data,
- incoming_trampoline_shared_secret,
- ..
- } => {
+ HTLCSource::TrampolineForward { previous_hop_data, .. } => {
let decoded_onion_failure =
onion_error.decode_onion_failure(&self.secp_ctx, &self.logger, &source);
log_trace!(
@@ -9418,8 +9408,6 @@ impl<
"unknown channel".to_string()
},
);
- let incoming_trampoline_shared_secret = Some(*incoming_trampoline_shared_secret);
-
// TODO: when we receive a failure from a single outgoing trampoline HTLC, we don't
// necessarily want to fail all of our incoming HTLCs back yet. We may have other
// outgoing HTLCs that need to resolve first. This will be tracked in our
@@ -9431,6 +9419,7 @@ impl<
incoming_packet_shared_secret,
blinded_failure,
channel_id,
+ trampoline_shared_secret,
..
} = current_hop_data;
log_trace!(
@@ -9442,13 +9431,17 @@ impl<
LocalHTLCFailureReason::TemporaryTrampolineFailure,
Vec::new(),
);
+ debug_assert!(
+ trampoline_shared_secret.is_some(),
+ "trampoline hop should have secret"
+ );
push_forward_htlcs_failure(
*prev_outbound_scid_alias,
get_htlc_forward_failure(
blinded_failure,
&onion_error,
incoming_packet_shared_secret,
- &incoming_trampoline_shared_secret,
+ &trampoline_shared_secret,
&None,
*htlc_id,
),
@@ -18153,16 +18146,11 @@ impl Writeable for HTLCSource {
1u8.write(writer)?;
field.write(writer)?;
},
- HTLCSource::TrampolineForward {
- ref previous_hop_data,
- incoming_trampoline_shared_secret,
- ref outbound_payment,
- } => {
+ HTLCSource::TrampolineForward { ref previous_hop_data, ref outbound_payment } => {
2u8.write(writer)?;
write_tlv_fields!(writer, {
(1, *previous_hop_data, required_vec),
- (3, incoming_trampoline_shared_secret, required),
- (5, outbound_payment, option),
+ (3, outbound_payment, option),
});
},
}
Why this scored 23/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.