Skip to content

Commit fd8846b

Browse files
committed
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
1 parent ed02087 commit fd8846b

2 files changed

Lines changed: 60 additions & 28 deletions

File tree

lightning/src/ln/channelmanager.rs

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8878,27 +8878,24 @@ impl<
88788878
debug_assert!(false);
88798879
return false;
88808880
}
8881-
if let OnionPayload::Invoice { .. } = payment.htlcs[0].onion_payload {
8882-
// Check if we've received all the parts we need for an MPP (the value of the parts adds to total_msat).
8883-
// In this case we're not going to handle any timeouts of the parts here.
8884-
// This condition determining whether the MPP is complete here must match
8885-
// exactly the condition used in `process_pending_htlc_forwards`.
8886-
let total_intended_recvd_value =
8887-
payment.htlcs.iter().map(|h| h.sender_intended_value).sum();
8888-
let total_mpp_value = payment.onion_fields.total_mpp_amount_msat;
8889-
if total_mpp_value <= total_intended_recvd_value {
8890-
return true;
8891-
} else if payment.htlcs.iter_mut().any(|htlc| {
8892-
htlc.timer_ticks += 1;
8893-
return htlc.timer_ticks >= MPP_TIMEOUT_TICKS;
8894-
}) {
8895-
let htlcs = payment
8896-
.htlcs
8897-
.drain(..)
8898-
.map(|htlc: ClaimableHTLC| (htlc.prev_hop, *payment_hash));
8899-
timed_out_mpp_htlcs.extend(htlcs);
8900-
return false;
8901-
}
8881+
// Check if we've received all the parts we need for an MPP.
8882+
// This condition determining whether the MPP is complete here must match
8883+
// exactly the condition used in `process_pending_htlc_forwards`.
8884+
let total_intended_recvd_value =
8885+
payment.htlcs.iter().map(|h| h.sender_intended_value).sum();
8886+
let total_mpp_value = payment.onion_fields.total_mpp_amount_msat;
8887+
if total_mpp_value <= total_intended_recvd_value {
8888+
return true;
8889+
} else if payment.htlcs.iter_mut().any(|htlc| {
8890+
htlc.timer_ticks += 1;
8891+
return htlc.timer_ticks >= MPP_TIMEOUT_TICKS;
8892+
}) {
8893+
let htlcs = payment
8894+
.htlcs
8895+
.drain(..)
8896+
.map(|htlc: ClaimableHTLC| (htlc.prev_hop, *payment_hash));
8897+
timed_out_mpp_htlcs.extend(htlcs);
8898+
return false;
89028899
}
89038900
true
89048901
},

lightning/src/ln/payment_tests.rs

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ fn mpp_retry_overpay() {
335335
expect_payment_sent!(&nodes[0], payment_preimage, Some(expected_total_fee_msat));
336336
}
337337

338-
fn do_mpp_receive_timeout(send_partial_mpp: bool) {
338+
fn do_mpp_receive_timeout(send_partial_mpp: bool, keysend: bool) {
339339
let chanmon_cfgs = create_chanmon_cfgs(4);
340340
let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
341341
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) {
351351
let (chan_3_update, _, chan_3_id, _) = create_announced_chan_between_nodes(&nodes, 1, 3);
352352
let (chan_4_update, _, _, _) = create_announced_chan_between_nodes(&nodes, 2, 3);
353353

354-
let (mut route, hash, payment_preimage, payment_secret) =
355-
get_route_and_payment_hash!(nodes[0], nodes[3], 100_000);
354+
let (mut route, hash, payment_preimage, payment_secret) = if keysend {
355+
let payment_params = PaymentParameters::for_keysend(node_d_id, TEST_FINAL_CLTV, true);
356+
get_route_and_payment_hash!(nodes[0], nodes[3], payment_params, 100_000)
357+
} else {
358+
get_route_and_payment_hash!(nodes[0], nodes[3], 100_000)
359+
};
356360
let path = route.paths[0].clone();
357361
route.paths.push(path);
358362
route.paths[0].hops[0].pubkey = node_b_id;
@@ -365,7 +369,22 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
365369

366370
// Initiate the MPP payment.
367371
let onion = RecipientOnionFields::secret_only(payment_secret, 200_000);
368-
nodes[0].node.send_payment_with_route(route, hash, onion, PaymentId(hash.0)).unwrap();
372+
if keysend {
373+
let route_params = route.route_params.clone().unwrap();
374+
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
375+
nodes[0]
376+
.node
377+
.send_spontaneous_payment(
378+
Some(payment_preimage),
379+
onion,
380+
PaymentId(hash.0),
381+
route_params,
382+
Retry::Attempts(0),
383+
)
384+
.unwrap();
385+
} else {
386+
nodes[0].node.send_payment_with_route(route, hash, onion, PaymentId(hash.0)).unwrap();
387+
}
369388
check_added_monitors(&nodes[0], 2); // one monitor per path
370389
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
371390
assert_eq!(events.len(), 2);
@@ -414,7 +433,17 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
414433
let node_2_msgs = remove_first_msg_event_to_node(&node_c_id, &mut events);
415434
let path = &[&nodes[2], &nodes[3]];
416435
let payment_secret = Some(payment_secret);
417-
pass_along_path(&nodes[0], path, 200_000, hash, payment_secret, node_2_msgs, true, None);
436+
let expected_preimage = if keysend { Some(payment_preimage) } else { None };
437+
pass_along_path(
438+
&nodes[0],
439+
path,
440+
200_000,
441+
hash,
442+
payment_secret,
443+
node_2_msgs,
444+
true,
445+
expected_preimage,
446+
);
418447

419448
// Even after MPP_TIMEOUT_TICKS we should not timeout the MPP if we have all the parts
420449
for _ in 0..MPP_TIMEOUT_TICKS {
@@ -428,8 +457,14 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
428457

429458
#[test]
430459
fn mpp_receive_timeout() {
431-
do_mpp_receive_timeout(true);
432-
do_mpp_receive_timeout(false);
460+
do_mpp_receive_timeout(true, false);
461+
do_mpp_receive_timeout(false, false);
462+
}
463+
464+
#[test]
465+
fn keysend_mpp_receive_timeout() {
466+
do_mpp_receive_timeout(true, true);
467+
do_mpp_receive_timeout(false, true);
433468
}
434469

435470
#[test]

0 commit comments

Comments
 (0)