Set PaymentSent::fee_paid_msat in abandoned case
What changed, and why it matters
This commit fixes a small bookkeeping bug in the Lightning Dev Kit. When a user abandoned an in-flight payment but the payment still succeeded later, the reported fee field was incorrectly left blank (None), even though the documentation promised it would always be filled in for newer versions. The patch preserves the fee amount when a payment is abandoned so it can still be reported correctly if an HTLC later succeeds. There is no direct security vulnerability here—no funds are stolen, lost, or incorrectly routed—but it removes a contradiction between documented behavior and actual behavior that downstream software might rely on.
No immediate security action required. Treat as a normal bugfix release. Downstream consumers that parse PaymentSent events can remove workarounds for the None fee case after upgrading to the fixed versions (0.3, 0.2.3, 0.1.10).
Security signals we found
Incorrect/incomplete API contract between documented behavior and returned event data
Potential for downstream accounting or fee-reporting logic to misbehave if it assumes fee_paid_msat is always Some for modern versions
Regression test added for the fixed behavior
Evidence from the diff
The change adds a pending_fee_msat field to PendingOutboundPayment::Abandoned, populated from Retryable before abandonment, and includes it in serialization. When an abandoned payment’s HTLC is later claimed, get_pending_fee_msat now returns the preserved fee so Event::PaymentSent::fee_paid_msat is Some instead of None. The docs are updated to note the exception only for older versions, and a regression test (abandoned_payment_fulfilled_preserves_fee_paid_msat) is added.
Changed components
lightning/src/ln/outbound_payment.rslightning/src/events/mod.rslightning/src/ln/payment_tests.rslightning/src/ln/functional_tests.rsInspect captured patch +40 / −2
diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index b094718..4853c83 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -1201,7 +1201,8 @@ pub enum Event {
/// If the recipient or an intermediate node misbehaves and gives us free money, this may
/// overstate the amount paid, though this is unlikely.
///
- /// This is only `None` for payments initiated on LDK versions prior to 0.0.103.
+ /// This is only `None` for payments abandoned but ultimately claimed when using LDK versions
+ /// prior to 0.3, 0.2.3, or 0.1.10.
///
/// [`Route::get_total_fees`]: crate::routing::router::Route::get_total_fees
fee_paid_msat: Option<u64>,
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 37dd518..52e2f2e 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -8579,7 +8579,7 @@ pub fn test_inconsistent_mpp_params() {
pass_along_path(&nodes[0], path_b, real_amt, hash, Some(payment_secret), event, true, None);
do_claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], &[path_a, path_b], preimage));
- expect_payment_sent(&nodes[0], preimage, Some(None), true, true);
+ expect_payment_sent(&nodes[0], preimage, Some(Some(2000)), true, true);
}
#[xtest(feature = "_externalize_tests")]
diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs
index 67fea50..04e8003 100644
--- a/lightning/src/ln/outbound_payment.rs
+++ b/lightning/src/ln/outbound_payment.rs
@@ -164,6 +164,9 @@ pub(crate) enum PendingOutboundPayment {
/// The total payment amount across all paths, used to be able to issue `PaymentSent` if
/// an HTLC still happens to succeed after we marked the payment as abandoned.
total_msat: Option<u64>,
+ /// Preserved from `Retryable` so we can still report `fee_paid_msat` if an HTLC succeeds after
+ /// the payment was abandoned. Added in 0.3.
+ pending_fee_msat: Option<u64>,
},
}
@@ -252,6 +255,7 @@ impl PendingOutboundPayment {
fn get_pending_fee_msat(&self) -> Option<u64> {
match self {
PendingOutboundPayment::Retryable { pending_fee_msat, .. } => pending_fee_msat.clone(),
+ PendingOutboundPayment::Abandoned { pending_fee_msat, .. } => pending_fee_msat.clone(),
_ => None,
}
}
@@ -308,6 +312,7 @@ impl PendingOutboundPayment {
_ => new_hash_set(),
};
let total_msat = self.total_msat();
+ let pending_fee_msat = self.get_pending_fee_msat();
match self {
Self::Retryable { payment_hash, .. } |
Self::InvoiceReceived { payment_hash, .. } |
@@ -318,6 +323,7 @@ impl PendingOutboundPayment {
payment_hash: *payment_hash,
reason: Some(reason),
total_msat,
+ pending_fee_msat,
};
},
_ => {}
@@ -2778,6 +2784,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
(1, reason, upgradable_option),
(2, payment_hash, required),
(3, total_msat, option),
+ (5, pending_fee_msat, option),
},
(5, AwaitingInvoice) => {
(0, expiration, required),
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 33c7df9..90656b3 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -2241,6 +2241,36 @@ fn abandoned_send_payment_idempotent() {
claim_payment(&nodes[0], &[&nodes[1]], second_payment_preimage);
}
+#[test]
+fn abandoned_payment_fulfilled_preserves_fee_paid_msat() {
+ // Previously, if we abandoned a payment with HTLCs in-flight and the payment eventually
+ // succeeded, we would set the `Event::PaymentSent::fee_paid_msat` to None, even though we had
+ // docs guaranteeing that it would always be Some after 0.0.103.
+ let chanmon_cfgs = create_chanmon_cfgs(3);
+ let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+ let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+ create_announced_chan_between_nodes(&nodes, 0, 1);
+ create_announced_chan_between_nodes(&nodes, 1, 2);
+
+ let amt_msat = 10_000_000;
+ let (route, payment_hash, payment_preimage, payment_secret) =
+ get_route_and_payment_hash!(&nodes[0], nodes[2], amt_msat);
+ let payment_id = PaymentId(payment_hash.0);
+ let onion = RecipientOnionFields::secret_only(payment_secret, amt_msat);
+ nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
+ check_added_monitors(&nodes[0], 1);
+
+ let path: &[&Node] = &[&nodes[1], &nodes[2]];
+ pass_along_route(&nodes[0], &[path], amt_msat, payment_hash, payment_secret);
+
+ nodes[0].node.abandon_payment(payment_id);
+ assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+
+ claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], &[path], payment_preimage));
+}
+
#[derive(PartialEq)]
enum InterceptTest {
Forward,
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.