Skip to content

Commit a3457de

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 a3457de

2 files changed

Lines changed: 108 additions & 21 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: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,12 +426,102 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) {
426426
}
427427
}
428428

429+
fn do_keysend_mpp_receive_timeout() {
430+
let chanmon_cfgs = create_chanmon_cfgs(4);
431+
let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
432+
let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
433+
let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
434+
435+
let node_a_id = nodes[0].node.get_our_node_id();
436+
let node_b_id = nodes[1].node.get_our_node_id();
437+
let node_c_id = nodes[2].node.get_our_node_id();
438+
let node_d_id = nodes[3].node.get_our_node_id();
439+
440+
let (chan_1_update, _, _, _) = create_announced_chan_between_nodes(&nodes, 0, 1);
441+
let (chan_2_update, _, _, _) = create_announced_chan_between_nodes(&nodes, 0, 2);
442+
let (chan_3_update, _, chan_3_id, _) = create_announced_chan_between_nodes(&nodes, 1, 3);
443+
let (chan_4_update, _, _, _) = create_announced_chan_between_nodes(&nodes, 2, 3);
444+
445+
let payment_params = PaymentParameters::for_keysend(node_d_id, TEST_FINAL_CLTV, true);
446+
let (mut route, hash, payment_preimage, payment_secret) =
447+
get_route_and_payment_hash!(nodes[0], nodes[3], payment_params, 100_000);
448+
let path = route.paths[0].clone();
449+
route.paths.push(path);
450+
route.paths[0].hops[0].pubkey = node_b_id;
451+
route.paths[0].hops[0].short_channel_id = chan_1_update.contents.short_channel_id;
452+
route.paths[0].hops[1].short_channel_id = chan_3_update.contents.short_channel_id;
453+
route.paths[1].hops[0].pubkey = node_c_id;
454+
route.paths[1].hops[0].short_channel_id = chan_2_update.contents.short_channel_id;
455+
route.paths[1].hops[1].short_channel_id = chan_4_update.contents.short_channel_id;
456+
route.route_params.as_mut().unwrap().final_value_msat *= 2;
457+
458+
// Initiate the MPP keysend and only relay the first half so the payment remains incomplete.
459+
let onion = RecipientOnionFields::secret_only(payment_secret, 200_000);
460+
let route_params = route.route_params.clone().unwrap();
461+
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
462+
nodes[0]
463+
.node
464+
.send_spontaneous_payment(
465+
Some(payment_preimage),
466+
onion,
467+
PaymentId(hash.0),
468+
route_params,
469+
Retry::Attempts(0),
470+
)
471+
.unwrap();
472+
check_added_monitors(&nodes[0], 2);
473+
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
474+
assert_eq!(events.len(), 2);
475+
476+
let node_1_msgs = remove_first_msg_event_to_node(&node_b_id, &mut events);
477+
let path = &[&nodes[1], &nodes[3]];
478+
pass_along_path(&nodes[0], path, 200_000, hash, Some(payment_secret), node_1_msgs, false, None);
479+
480+
for _ in 0..MPP_TIMEOUT_TICKS {
481+
nodes[3].node.timer_tick_occurred();
482+
}
483+
484+
// This should mirror invoice-backed MPP handling: the incomplete keysend MPP should time out
485+
// and fail backward instead of remaining claimable until CLTV expiry.
486+
let fail = HTLCHandlingFailureType::Receive { payment_hash: hash };
487+
expect_and_process_pending_htlcs_and_htlc_handling_failed(&nodes[3], &[fail]);
488+
489+
let htlc_fail_updates = get_htlc_update_msgs(&nodes[3], &node_b_id);
490+
assert_eq!(htlc_fail_updates.update_fail_htlcs.len(), 1);
491+
nodes[1].node.handle_update_fail_htlc(node_d_id, &htlc_fail_updates.update_fail_htlcs[0]);
492+
check_added_monitors(&nodes[3], 1);
493+
494+
let commitment = &htlc_fail_updates.commitment_signed;
495+
do_commitment_signed_dance(&nodes[1], &nodes[3], commitment, false, false);
496+
497+
let fail_type =
498+
HTLCHandlingFailureType::Forward { node_id: Some(node_d_id), channel_id: chan_3_id };
499+
expect_and_process_pending_htlcs_and_htlc_handling_failed(&nodes[1], &[fail_type]);
500+
501+
let htlc_fail_updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
502+
assert_eq!(htlc_fail_updates.update_fail_htlcs.len(), 1);
503+
nodes[0].node.handle_update_fail_htlc(node_b_id, &htlc_fail_updates.update_fail_htlcs[0]);
504+
check_added_monitors(&nodes[1], 1);
505+
let commitment = &htlc_fail_updates.commitment_signed;
506+
do_commitment_signed_dance(&nodes[0], &nodes[1], commitment, false, false);
507+
508+
let mut conditions = PaymentFailedConditions::new()
509+
.mpp_parts_remain()
510+
.expected_htlc_error_data(LocalHTLCFailureReason::MPPTimeout, &[][..]);
511+
expect_payment_failed_conditions(&nodes[0], hash, false, conditions);
512+
}
513+
429514
#[test]
430515
fn mpp_receive_timeout() {
431516
do_mpp_receive_timeout(true);
432517
do_mpp_receive_timeout(false);
433518
}
434519

520+
#[test]
521+
fn keysend_mpp_receive_timeout() {
522+
do_keysend_mpp_receive_timeout();
523+
}
524+
435525
#[test]
436526
fn test_keysend_payments() {
437527
do_test_keysend_payments(false);

0 commit comments

Comments
 (0)