ln+events: allow multiple prev_channel_id in HTLCHandlingFailed
What the developer wrote
Authored by Carla Kirk-Cohen
ln+events: allow multiple prev_channel_id in HTLCHandlingFailed
In preparation for trampoline failures, allow multiple previous channel
ids. We'll only emit a single HTLCHandlingFailed for all of our failed
back HTLCs, so we want to be able to express all of them in one event.
What changed, and why it matters
This commit is a straightforward internal API and data-format change in the Lightning Dev Kit. It changes one field in an event from holding a single channel identifier to holding a list of channel identifiers, so that future trampoline-routing failures can report all related channels at once. It also adds backward-compatible serialization logic so older versions can still read the new event format. There is no indication this fixes an active security bug; it is preparatory refactoring.
No immediate security action required. Treat as normal refactoring/forward-compatibility work. Reviewers may want to confirm that the new TLV field index (3) does not collide with any other pending event changes and that the downgrade note is accurate.
Security signals we found
Field type change from scalar to vector in a public event enum
Backward-compatible TLV serialization/deserialization added for new vector field
Downgrade documentation note about loss of information for multipart trampoline forwards
No security-relevant keywords (fix, vulnerability, CVE, exploit, bug, etc.) in commit message or diff
Evidence from the diff
The commit renames prev_channel_id: ChannelId to prev_channel_ids: Vec<ChannelId> inside Event::HTLCHandlingFailed. Call sites in channelmanager.rs now wrap the single channel id in a one-element vector, and a serialization helper for Vec<ChannelId> is added. The TLV serialization writes the first id into the legacy required field 0 and writes the full vector into a new required field 3. Deserialization uses a default-value fallback that populates prev_channel_ids from the legacy field when reading older data. A downgrade note is added to the docs warning that pending trampoline forwards with multipart payments will only report the first HTLC after downgrading from 0.3.
Changed components
lightning/src/events/mod.rslightning/src/ln/channelmanager.rslightning/src/ln/monitor_tests.rslightning/src/util/ser.rsInspect captured patch +30 / −11
diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 01bbd5d..adf4ca0 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -1665,12 +1665,17 @@ pub enum Event {
/// Indicates that the HTLC was accepted, but could not be processed when or after attempting to
/// forward it.
///
+ /// Note that downgrading from 0.3 with pending trampoline forwards that have incoming multipart
+ /// payments will produce an event that only provides information about the first htlc that was
+ /// received/dispatched.
+ ///
/// # Failure Behavior and Persistence
/// This event will eventually be replayed after failures-to-handle (i.e., the event handler
/// returning `Err(ReplayEvent ())`) and will be persisted across restarts.
HTLCHandlingFailed {
- /// The channel over which the HTLC was received.
- prev_channel_id: ChannelId,
+ /// The channel(s) over which the HTLC(s) was received. May contain multiple entries for
+ /// trampoline forwards.
+ prev_channel_ids: Vec<ChannelId>,
/// The type of HTLC handling that failed.
failure_type: HTLCHandlingFailureType,
/// The reason that the HTLC failed.
@@ -2223,15 +2228,24 @@ impl Writeable for Event {
})
},
&Event::HTLCHandlingFailed {
- ref prev_channel_id,
+ ref prev_channel_ids,
ref failure_type,
ref failure_reason,
} => {
25u8.write(writer)?;
+ // Legacy field is written for backwards compatibility. We don't want to fail writes
+ // so we write garbage data if we don't have the data we expect.
+ debug_assert!(
+ !prev_channel_ids.is_empty(),
+ "at least one prev_channel_id required for HTLCHandlingFailed"
+ );
+ let zero_id = ChannelId::new_zero();
+ let legacy_chan_id = prev_channel_ids.first().unwrap_or(&zero_id);
write_tlv_fields!(writer, {
- (0, prev_channel_id, required),
+ (0, legacy_chan_id, required),
(1, failure_reason, option),
(2, failure_type, required),
+ (3, *prev_channel_ids, required),
})
},
&Event::BumpTransaction(ref event) => {
@@ -2806,13 +2820,17 @@ impl MaybeReadable for Event {
},
25u8 => {
let mut f = || {
- let mut prev_channel_id = ChannelId::new_zero();
+ let mut prev_channel_id_legacy = ChannelId::new_zero();
let mut failure_reason = None;
let mut failure_type_opt = UpgradableRequired(None);
+ let mut prev_channel_ids = vec![];
read_tlv_fields!(reader, {
- (0, prev_channel_id, required),
+ (0, prev_channel_id_legacy, required),
(1, failure_reason, option),
(2, failure_type_opt, upgradable_required),
+ (3, prev_channel_ids, (default_value, vec![
+ prev_channel_id_legacy,
+ ])),
});
// If a legacy HTLCHandlingFailureType::UnknownNextHop was written, upgrade
@@ -2827,7 +2845,7 @@ impl MaybeReadable for Event {
failure_reason = Some(LocalHTLCFailureReason::UnknownNextPeer.into());
}
Ok(Some(Event::HTLCHandlingFailed {
- prev_channel_id,
+ prev_channel_ids,
failure_type: _init_tlv_based_struct_field!(
failure_type_opt,
upgradable_required
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index acf1872..2520d68 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -7423,7 +7423,7 @@ impl<
.push(failure);
self.pending_events.lock().unwrap().push_back((
events::Event::HTLCHandlingFailed {
- prev_channel_id: incoming_channel_id,
+ prev_channel_ids: vec![incoming_channel_id],
failure_type,
failure_reason: Some(failure_reason),
},
@@ -9018,7 +9018,7 @@ impl<
let mut pending_events = self.pending_events.lock().unwrap();
pending_events.push_back((
events::Event::HTLCHandlingFailed {
- prev_channel_id: *channel_id,
+ prev_channel_ids: vec![*channel_id],
failure_type,
failure_reason: Some(onion_error.into()),
},
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index 18a9768..2368776 100644
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -3780,8 +3780,8 @@ fn do_test_lost_timeout_monitor_events(confirm_tx: CommitmentType, dust_htlcs: b
Event::PaymentFailed { payment_hash, .. } => {
assert_eq!(payment_hash, Some(hash_b));
},
- Event::HTLCHandlingFailed { prev_channel_id, .. } => {
- assert_eq!(prev_channel_id, chan_a);
+ Event::HTLCHandlingFailed { prev_channel_ids, .. } => {
+ assert_eq!(prev_channel_ids[0], chan_a);
},
_ => panic!("Wrong event {ev:?}"),
}
diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs
index 45ca98b..b226332 100644
--- a/lightning/src/util/ser.rs
+++ b/lightning/src/util/ser.rs
@@ -1110,6 +1110,7 @@ impl_for_vec_with_element_length_prefix!(crate::ln::msgs::UpdateAddHTLC);
impl_writeable_for_vec_with_element_length_prefix!(&crate::ln::msgs::UpdateAddHTLC);
impl_for_vec!(u32);
impl_for_vec!(crate::events::HTLCLocator);
+impl_for_vec!(crate::ln::types::ChannelId);
impl Writeable for Vec<Witness> {
#[inline]
Why this scored 18/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.