Public commit record
68/100 · Adequate
What the developer wrote
Authored by benthecarman
Add amounts to HTLC locators
Include per-HTLC amounts in `PaymentForwarded` locators so callers can
account for each channel independently when a forward uses multiple
incoming or outgoing HTLCs.
AI-assisted-by: OpenAI Codex
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit adds the HTLC amount (in milli-satoshis) to the data structures LDK uses to locate forwarded payments. It is a feature/enhancement change: callers of the PaymentForwarded event can now see per-HTLC amounts when a forward involves multiple incoming or outgoing HTLCs. There is no security vulnerability in the diff; it only enriches event metadata and updates serialization, tests, and test helpers accordingly.
Recommended action
No security action required. Treat as a normal feature commit. Reviewers may want to confirm that the legacy deserialization fallback for prev_htlcs amount_msat (using outbound_amount_forwarded_msat + total fee when total_fee_earned_msat is known) is acceptable for downstream accounting, but this is a data-quality consideration, not a security issue.
Security signals we found
01
No memory-safety issues introduced (Rust, optional u64 field)
02
No new panics, unwraps, or arithmetic on untrusted values beyond existing fee math
03
Serialization uses existing TLV framework with optional fields, preserving backward compatibility
04
No authentication, authorization, or cryptographic changes
05
No external input parsing beyond existing event deserialization
Evidence from the diff
The change extends HTLCLocator and HTLCPreviousHopData with an optional amount_msat field, wires the amount through channel HTLC handling and channel manager claim paths, and adds TLV serialization (field 7 for HTLCLocator, field 15 for HTLCPreviousHopData). Legacy PaymentForwarded event deserialization is updated to populate amount_msat where the old scalar fields allow it, and tests assert the new field is present and consistent with total_fee_earned_msat/outbound_amount_forwarded_msat.
Changed components
lightning/src/events/mod.rs (HTLCLocator, Event serialization/deserialization)lightning/src/ln/channel.rs (HTLCPreviousHopData construction)lightning/src/ln/channelmanager.rs (HTLCPreviousHopData, PaymentForwarded event emission)lightning/src/ln/functional_test_utils.rs (test helper assertions)lightning/src/ln/functional_tests.rs (legacy on-chain success test assertions)lightning/src/ln/trampoline_forward_tests.rs (test fixture)Inspect captured patch +99 / −10
diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index ad493a1..6bbcf4f 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -866,6 +866,9 @@ pub struct HTLCLocator {
/// The channel that the HTLC was sent or received on.
pub channel_id: ChannelId,
+ /// The amount, in milli-satoshis, of the HTLC that was sent or received, if known.
+ pub amount_msat: Option<u64>,
+
/// The `user_channel_id` for `channel_id`.
///
/// This will be `None` if the payment was settled via an on-chain transaction. It will also
@@ -883,6 +886,7 @@ impl_ser_tlv_based!(HTLCLocator, {
(1, channel_id, required),
(3, user_channel_id, option),
(5, node_id, option),
+ (7, amount_msat, option),
});
/// An Event which you should probably take some action in response to.
@@ -2215,6 +2219,7 @@ impl Writeable for Event {
);
let empty_locator = HTLCLocator {
channel_id: ChannelId::new_zero(),
+ amount_msat: None,
user_channel_id: None,
node_id: None,
};
@@ -2782,11 +2787,14 @@ impl MaybeReadable for Event {
// with pending forwards to 0.1 for any version 0.0.123 or earlier.
(17, prev_htlcs, (default_value, vec![HTLCLocator{
channel_id: prev_channel_id_legacy.ok_or(DecodeError::InvalidValue)?,
+ amount_msat: total_fee_earned_msat
+ .map(|fee| outbound_amount_forwarded_msat + fee),
user_channel_id: prev_user_channel_id_legacy,
node_id: prev_node_id_legacy,
}])),
(19, next_htlcs, (default_value, vec![HTLCLocator{
channel_id: next_channel_id_legacy.ok_or(DecodeError::InvalidValue)?,
+ amount_msat: Some(outbound_amount_forwarded_msat),
user_channel_id: next_user_channel_id_legacy,
node_id: next_node_id_legacy,
}])),
@@ -3228,6 +3236,48 @@ impl MaybeReadable for Event {
}
}
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn legacy_payment_forwarded_preserves_unknown_inbound_htlc_amount() {
+ let prev_channel_id = ChannelId::from_bytes([1; 32]);
+ let next_channel_id = ChannelId::from_bytes([2; 32]);
+ let mut encoded_legacy_event = vec![
+ 7, // Event::PaymentForwarded
+ 81, // TLV stream length
+ 1, 32, // prev_channel_id
+ ];
+ encoded_legacy_event.extend_from_slice(&[1; 32]);
+ encoded_legacy_event.extend_from_slice(&[2, 1, 0]); // claim_from_onchain_tx
+ encoded_legacy_event.extend_from_slice(&[3, 32]); // next_channel_id
+ encoded_legacy_event.extend_from_slice(&[2; 32]);
+ // outbound_amount_forwarded_msat
+ encoded_legacy_event.extend_from_slice(&[5, 8, 0, 0, 0, 0, 0, 45, 198, 192]);
+
+ match Event::read(&mut &encoded_legacy_event[..]).unwrap().unwrap() {
+ Event::PaymentForwarded {
+ prev_htlcs,
+ next_htlcs,
+ total_fee_earned_msat,
+ outbound_amount_forwarded_msat,
+ ..
+ } => {
+ assert_eq!(total_fee_earned_msat, None);
+ assert_eq!(outbound_amount_forwarded_msat, 3_000_000);
+ assert_eq!(prev_htlcs.len(), 1);
+ assert_eq!(prev_htlcs[0].channel_id, prev_channel_id);
+ assert_eq!(prev_htlcs[0].amount_msat, None);
+ assert_eq!(next_htlcs.len(), 1);
+ assert_eq!(next_htlcs[0].channel_id, next_channel_id);
+ assert_eq!(next_htlcs[0].amount_msat, Some(3_000_000));
+ },
+ _ => panic!("expected PaymentForwarded event"),
+ }
+ }
+}
+
/// A trait indicating an object may generate events.
///
/// Events are processed by passing an [`EventHandler`] to [`process_pending_events`].
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index fb5a7de..6e20b6c 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -8175,6 +8175,7 @@ where
let prev_hop_data = HTLCPreviousHopData {
prev_outbound_scid_alias,
user_channel_id: Some(user_channel_id),
+ amount_msat: Some(htlc.amount_msat),
htlc_id: htlc.htlc_id,
incoming_packet_shared_secret: *incoming_packet_shared_secret,
phantom_shared_secret: *phantom_shared_secret,
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 93dfd1c..27765f9 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -481,6 +481,7 @@ impl PendingAddHTLCInfo {
HTLCPreviousHopData {
prev_outbound_scid_alias: self.prev_outbound_scid_alias,
user_channel_id: Some(self.prev_user_channel_id),
+ amount_msat: self.forward_info.incoming_amt_msat,
outpoint: self.prev_funding_outpoint,
channel_id: self.prev_channel_id,
counterparty_node_id: Some(self.prev_counterparty_node_id),
@@ -944,6 +945,7 @@ mod fuzzy_channelmanager {
pub struct HTLCPreviousHopData {
pub prev_outbound_scid_alias: u64,
pub user_channel_id: Option<u128>,
+ pub amount_msat: Option<u64>,
pub htlc_id: u64,
pub incoming_packet_shared_secret: [u8; 32],
pub phantom_shared_secret: Option<[u8; 32]>,
@@ -960,12 +962,13 @@ mod fuzzy_channelmanager {
pub cltv_expiry: Option<u32>,
}
- impl From<&HTLCPreviousHopData> for events::HTLCLocator {
- fn from(value: &HTLCPreviousHopData) -> Self {
+ impl HTLCPreviousHopData {
+ pub(super) fn htlc_locator(&self, amount_msat: Option<u64>) -> events::HTLCLocator {
events::HTLCLocator {
- channel_id: value.channel_id,
- user_channel_id: value.user_channel_id,
- node_id: value.counterparty_node_id,
+ channel_id: self.channel_id,
+ amount_msat,
+ user_channel_id: self.user_channel_id,
+ node_id: self.counterparty_node_id,
}
}
}
@@ -8860,6 +8863,7 @@ impl<
let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
prev_outbound_scid_alias: prev_hop.prev_outbound_scid_alias,
user_channel_id: prev_hop.user_channel_id,
+ amount_msat: Some(value),
counterparty_node_id: prev_hop.counterparty_node_id,
channel_id: prev_channel_id,
outpoint: prev_funding_outpoint,
@@ -10522,7 +10526,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
},
HTLCSource::PreviousHopData(hop_data) => {
- let prev_htlcs = vec![events::HTLCLocator::from(&hop_data)];
+ let event_prev_hop_data = hop_data.clone();
self.claim_funds_from_htlc_forward_hop(
payment_preimage,
|htlc_claim_value_msat: Option<u64>| -> Option<events::Event> {
@@ -10536,11 +10540,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
skimmed_fee_msat <= total_fee_earned_msat,
"skimmed_fee_msat must always be included in total_fee_earned_msat"
);
+ let prev_htlc_amount_msat =
+ event_prev_hop_data.amount_msat.or(htlc_claim_value_msat);
Some(events::Event::PaymentForwarded {
- prev_htlcs,
+ prev_htlcs: vec![
+ event_prev_hop_data.htlc_locator(prev_htlc_amount_msat)
+ ],
next_htlcs: vec![events::HTLCLocator {
channel_id: next_channel_id,
+ amount_msat: Some(forwarded_htlc_value_msat),
user_channel_id: next_user_channel_id,
node_id: Some(next_channel_counterparty_node_id),
}],
@@ -10561,20 +10570,29 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
},
HTLCSource::TrampolineForward { previous_hop_data, .. } => {
// Only emit a single event for trampoline claims.
- let prev_htlcs: Vec<events::HTLCLocator> =
- previous_hop_data.iter().map(Into::into).collect();
+ let mut event_prev_htlcs = Some(
+ previous_hop_data.iter().map(|hop| hop.htlc_locator(hop.amount_msat)).collect(),
+ );
for (i, current_previous_hop_data) in previous_hop_data.into_iter().enumerate() {
self.claim_funds_from_htlc_forward_hop(
payment_preimage,
|_: Option<u64>| -> Option<events::Event> {
if i == 0 {
+ let Some(prev_htlcs) = event_prev_htlcs.take() else {
+ debug_assert!(
+ false,
+ "trampoline forward event already emitted"
+ );
+ return None;
+ };
Some(events::Event::PaymentForwarded {
- prev_htlcs: prev_htlcs.clone(),
+ prev_htlcs,
// TODO: When trampoline payments are tracked in our
// pending_outbound_payments, we'll be able to provide all the
// outgoing htlcs for this forward.
next_htlcs: vec![events::HTLCLocator {
channel_id: next_channel_id,
+ amount_msat: Some(forwarded_htlc_value_msat),
user_channel_id: next_user_channel_id,
node_id: Some(next_channel_counterparty_node_id),
}],
@@ -18343,6 +18361,7 @@ impl_ser_tlv_based!(HTLCPreviousHopData, {
(9, channel_id, (default_value, ChannelId::v1_from_funding_outpoint(outpoint.0.unwrap()))),
(11, counterparty_node_id, option),
(13, trampoline_shared_secret, option),
+ (15, amount_msat, option),
});
fn write_claimable_htlc<W: Writer>(
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index acb6782..3601675 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -3097,6 +3097,7 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM = CM>>(
total_fee_earned_msat,
skimmed_fee_msat,
claim_from_onchain_tx,
+ outbound_amount_forwarded_msat,
..
} => {
assert_eq!(prev_htlcs.len(), 1);
@@ -3115,6 +3116,19 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM = CM>>(
// Check that the (knowingly) withheld amount is always less or equal to the expected
// overpaid amount.
assert!(skimmed_fee_msat == expected_extra_fees_msat);
+ match expected_fee {
+ Some(_) => {
+ let actual_fee = total_fee_earned_msat.unwrap();
+ assert_eq!(next_htlcs[0].amount_msat, Some(outbound_amount_forwarded_msat));
+ assert_eq!(
+ prev_htlcs[0].amount_msat,
+ Some(next_htlcs[0].amount_msat.unwrap() + actual_fee)
+ );
+ },
+ None => {
+ assert_eq!(total_fee_earned_msat, None);
+ },
+ }
if !upstream_force_closed {
let prev_node_id = prev_htlcs[0].node_id.unwrap();
let prev_channel_id = prev_htlcs[0].channel_id;
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index b84a486..2e21974 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -1506,8 +1506,10 @@ pub fn test_htlc_on_chain_success() {
} => {
assert_eq!(total_fee_earned_msat, Some(1000));
assert_eq!(prev_htlcs[0].channel_id, chan_id);
+ assert_eq!(prev_htlcs[0].amount_msat, Some(3001000));
assert_eq!(claim_from_onchain_tx, true);
assert_eq!(next_htlcs[0].channel_id, chan_2.2);
+ assert_eq!(next_htlcs[0].amount_msat, Some(3000000));
assert_eq!(outbound_amount_forwarded_msat, 3000000);
},
_ => panic!(),
@@ -1523,8 +1525,10 @@ pub fn test_htlc_on_chain_success() {
} => {
assert_eq!(total_fee_earned_msat, Some(1000));
assert_eq!(prev_htlcs[0].channel_id, chan_id);
+ assert_eq!(prev_htlcs[0].amount_msat, Some(3001000));
assert_eq!(claim_from_onchain_tx, true);
assert_eq!(next_htlcs[0].channel_id, chan_2.2);
+ assert_eq!(next_htlcs[0].amount_msat, Some(3000000));
assert_eq!(outbound_amount_forwarded_msat, 3000000);
},
_ => panic!(),
diff --git a/lightning/src/ln/trampoline_forward_tests.rs b/lightning/src/ln/trampoline_forward_tests.rs
index 00f5074..c2c4f69 100644
--- a/lightning/src/ln/trampoline_forward_tests.rs
+++ b/lightning/src/ln/trampoline_forward_tests.rs
@@ -27,6 +27,7 @@ fn test_prev_hop_data(htlc_id: u64) -> HTLCPreviousHopData {
HTLCPreviousHopData {
prev_outbound_scid_alias: 0,
user_channel_id: None,
+ amount_msat: None,
htlc_id,
incoming_packet_shared_secret: [0; 32],
phantom_shared_secret: None,
Risk score
Our methodology →Why this scored 19/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.