Apply MPP receive timeout to keysend payments
What the developer wrote
Authored by Elias Rohrer
Apply MPP receive timeout to keysend payments
Incomplete keysend MPPs skipped the receive timeout path,
allowing partial payments to hold HTLC slots until CLTV
expiry instead of failing after `MPP_TIMEOUT_TICKS`.
Apply the existing `total_mpp_amount_msat` completeness
check to all MPP receives and add a regression test
covering the keysend case.
The timeout logic was originally added only for
invoice-backed MPPs in 2022, and that invoice-only
guard remained when receive-side MPP keysend support
landed in 2023, leaving this gap latent until now.
Co-Authored-By: HAL 9000
What changed, and why it matters
This commit fixes a bug in the Lightning Dev Kit where multi-part keysend payments (a way to send Bitcoin over the Lightning Network without an invoice) did not use the normal timeout safety check. Normally, if only some parts of a multi-part payment arrive, the node waits a short time and then fails the partial payments to free up resources. For keysend payments, this timeout was skipped, so partial payments could sit around for hours or days, tying up funds and network slots until the payment's final deadline. The fix applies the same completeness and timeout check to all multi-part receives, including keysend, and adds a test to prevent the bug from returning.
Review and merge the patch, then backport to affected release branches that include receive-side keysend MPP support. Monitor for any nodes that may have experienced stuck HTLCs from partial keysend MPPs prior to the fix.
Security signals we found
Resource exhaustion via held HTLC slots from incomplete MPP receives
Denial-of-service-like effect from prolonged locking of channel liquidity
Logic gap introduced when keysend receive support was added without extending existing timeout guard
Regression test added for keysend MPP receive timeout behavior
Evidence from the diff
In channelmanager.rs, the MPP receive timeout logic was guarded by if let OnionPayload::Invoice { .. }, so only invoice-backed MPPs were checked for completeness and timed out after MPP_TIMEOUT_TICKS. Keysend/spontaneous MPPs used a different OnionPayload variant and bypassed this path entirely. The patch removes the invoice-only guard and applies the total_mpp_amount_msat <= total_intended_recvd_value completeness check and the timer tick/timeout logic to all claimable MPP payments. A regression test in payment_tests.rs parameterizes the existing MPP receive timeout test to also exercise keysend MPPs.
Changed components
lightning/src/ln/channelmanager.rslightning/src/ln/payment_tests.rsInspect captured patch +60 / −28
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 7f6d653..3bd90db 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -8878,27 +8878,24 @@ impl<
debug_assert!(false);
return false;
}
- if let OnionPayload::Invoice { .. } = payment.htlcs[0].onion_payload {
- // Check if we've received all the parts we need for an MPP (the value of the parts adds to total_msat).
- // In this case we're not going to handle any timeouts of the parts here.
- // This condition determining whether the MPP is complete here must match
- // exactly the condition used in `process_pending_htlc_forwards`.
- let total_intended_recvd_value =
- payment.htlcs.iter().map(|h| h.sender_intended_value).sum();
- let total_mpp_value = payment.onion_fields.total_mpp_amount_msat;
- if total_mpp_value <= total_intended_recvd_value {
- return true;
- } else if payment.htlcs.iter_mut().any(|htlc| {
- htlc.timer_ticks += 1;
- return htlc.timer_ticks >= MPP_TIMEOUT_TICKS;
- }) {
- let htlcs = payment
- .htlcs
- .drain(..)
- .map(|htlc: ClaimableHTLC| (htlc.prev_hop, *payment_hash));
- timed_out_mpp_htlcs.extend(htlcs);
- return false;
- }
+ // Check if we've received all the parts we need for an MPP.
+ // This condition determining whether the MPP is complete here must match
+ // exactly the condition used in `process_pending_htlc_forwards`.
+ let total_intended_recvd_value =
+ payment.htlcs.iter().map(|h| h.sender_intended_value).sum();
+ let total_mpp_value = payment.onion_fields.total_mpp_amount_msat;
+ if total_mpp_value <= total_intended_recvd_value {
+ return true;
+ } else if payment.htlcs.iter_mut().any(|htlc| {
+ htlc.timer_ticks += 1;
+ return htlc.timer_ticks >= MPP_TIMEOUT_TICKS;
+ }) {
+ let htlcs = payment
+ .htlcs
+ .drain(..)
+ .map(|htlc: ClaimableHTLC| (htlc.prev_hop, *payment_hash));
+ timed_out_mpp_htlcs.extend(htlcs);
+ return false;
}
true
},
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 807d1a1..5b4f5f9 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -335,7 +335,7 @@ fn mpp_retry_overpay() {
expect_payment_sent!(&nodes[0], payment_preimage, Some(expected_total_fee_msat));
}
-fn do_mpp_receive_timeout(send_partial_mpp: bool) {
+fn do_mpp_receive_timeout(send_partial_mpp: bool, keysend: bool) {
let chanmon_cfgs = create_chanmon_cfgs(4);
let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
@@ -351,8 +351,12 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
let (chan_3_update, _, chan_3_id, _) = create_announced_chan_between_nodes(&nodes, 1, 3);
let (chan_4_update, _, _, _) = create_announced_chan_between_nodes(&nodes, 2, 3);
- let (mut route, hash, payment_preimage, payment_secret) =
- get_route_and_payment_hash!(nodes[0], nodes[3], 100_000);
+ let (mut route, hash, payment_preimage, payment_secret) = if keysend {
+ let payment_params = PaymentParameters::for_keysend(node_d_id, TEST_FINAL_CLTV, true);
+ get_route_and_payment_hash!(nodes[0], nodes[3], payment_params, 100_000)
+ } else {
+ get_route_and_payment_hash!(nodes[0], nodes[3], 100_000)
+ };
let path = route.paths[0].clone();
route.paths.push(path);
route.paths[0].hops[0].pubkey = node_b_id;
@@ -365,7 +369,22 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
// Initiate the MPP payment.
let onion = RecipientOnionFields::secret_only(payment_secret, 200_000);
- nodes[0].node.send_payment_with_route(route, hash, onion, PaymentId(hash.0)).unwrap();
+ if keysend {
+ let route_params = route.route_params.clone().unwrap();
+ nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
+ nodes[0]
+ .node
+ .send_spontaneous_payment(
+ Some(payment_preimage),
+ onion,
+ PaymentId(hash.0),
+ route_params,
+ Retry::Attempts(0),
+ )
+ .unwrap();
+ } else {
+ nodes[0].node.send_payment_with_route(route, hash, onion, PaymentId(hash.0)).unwrap();
+ }
check_added_monitors(&nodes[0], 2); // one monitor per path
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 2);
@@ -414,7 +433,17 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
let node_2_msgs = remove_first_msg_event_to_node(&node_c_id, &mut events);
let path = &[&nodes[2], &nodes[3]];
let payment_secret = Some(payment_secret);
- pass_along_path(&nodes[0], path, 200_000, hash, payment_secret, node_2_msgs, true, None);
+ let expected_preimage = if keysend { Some(payment_preimage) } else { None };
+ pass_along_path(
+ &nodes[0],
+ path,
+ 200_000,
+ hash,
+ payment_secret,
+ node_2_msgs,
+ true,
+ expected_preimage,
+ );
// Even after MPP_TIMEOUT_TICKS we should not timeout the MPP if we have all the parts
for _ in 0..MPP_TIMEOUT_TICKS {
@@ -428,8 +457,14 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
#[test]
fn mpp_receive_timeout() {
- do_mpp_receive_timeout(true);
- do_mpp_receive_timeout(false);
+ do_mpp_receive_timeout(true, false);
+ do_mpp_receive_timeout(false, false);
+}
+
+#[test]
+fn keysend_mpp_receive_timeout() {
+ do_mpp_receive_timeout(true, true);
+ do_mpp_receive_timeout(false, true);
}
#[test]
Why this scored 60/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.