Skip to content

Commit 9514637

Browse files
authored
Merge pull request #4106 from valentinewallace/2025-08-async-sender-fix-race
Fix race condition causing async payment failure
2 parents e929517 + ade1f34 commit 9514637

File tree

4 files changed

+181
-58
lines changed

4 files changed

+181
-58
lines changed

lightning/src/blinded_path/message.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,20 @@ pub enum AsyncPaymentsContext {
587587
/// An identifier for the HTLC that should be released by us as the sender's always-online
588588
/// channel counterparty to the often-offline recipient.
589589
intercept_id: InterceptId,
590+
/// The short channel id alias corresponding to the to-be-released inbound HTLC, to help locate
591+
/// the HTLC internally if the [`ReleaseHeldHtlc`] races our node decoding the held HTLC's
592+
/// onion.
593+
///
594+
/// We use the outbound scid alias because it is stable even if the channel splices, unlike
595+
/// regular short channel ids.
596+
///
597+
/// [`ReleaseHeldHtlc`]: crate::onion_message::async_payments::ReleaseHeldHtlc
598+
prev_outbound_scid_alias: u64,
599+
/// The id of the to-be-released HTLC, to help locate the HTLC internally if the
600+
/// [`ReleaseHeldHtlc`] races our node decoding the held HTLC's onion.
601+
///
602+
/// [`ReleaseHeldHtlc`]: crate::onion_message::async_payments::ReleaseHeldHtlc
603+
htlc_id: u64,
590604
},
591605
}
592606

@@ -645,6 +659,8 @@ impl_writeable_tlv_based_enum!(AsyncPaymentsContext,
645659
},
646660
(6, ReleaseHeldHtlc) => {
647661
(0, intercept_id, required),
662+
(2, prev_outbound_scid_alias, required),
663+
(4, htlc_id, required),
648664
},
649665
);
650666

lightning/src/ln/async_payments_tests.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3364,3 +3364,70 @@ fn fail_held_htlcs_when_cfg_unset() {
33643364
PaymentFailureReason::RetriesExhausted,
33653365
);
33663366
}
3367+
3368+
#[test]
3369+
fn release_htlc_races_htlc_onion_decode() {
3370+
// Test that an async sender's LSP will release held HTLCs even if they receive the
3371+
// release_held_htlc message before they have a chance to process the held HTLC's onion. This was
3372+
// previously broken.
3373+
let chanmon_cfgs = create_chanmon_cfgs(4);
3374+
let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
3375+
3376+
let (sender_cfg, recipient_cfg) = (often_offline_node_cfg(), often_offline_node_cfg());
3377+
let mut sender_lsp_cfg = test_default_channel_config();
3378+
sender_lsp_cfg.enable_htlc_hold = true;
3379+
let mut invoice_server_cfg = test_default_channel_config();
3380+
invoice_server_cfg.accept_forwards_to_priv_channels = true;
3381+
3382+
let node_chanmgrs = create_node_chanmgrs(
3383+
4,
3384+
&node_cfgs,
3385+
&[Some(sender_cfg), Some(sender_lsp_cfg), Some(invoice_server_cfg), Some(recipient_cfg)],
3386+
);
3387+
let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
3388+
create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
3389+
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0);
3390+
create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0);
3391+
unify_blockheight_across_nodes(&nodes);
3392+
let sender = &nodes[0];
3393+
let sender_lsp = &nodes[1];
3394+
let invoice_server = &nodes[2];
3395+
let recipient = &nodes[3];
3396+
3397+
let amt_msat = 5000;
3398+
let (static_invoice, peer_id, static_invoice_om) =
3399+
build_async_offer_and_init_payment(amt_msat, &nodes);
3400+
let payment_hash =
3401+
lock_in_htlc_for_static_invoice(&static_invoice_om, peer_id, sender, sender_lsp);
3402+
3403+
// The LSP has not transitioned the HTLC to the intercepts map internally because
3404+
// process_pending_htlc_forwards has not been called.
3405+
let (peer_id, held_htlc_om) =
3406+
extract_held_htlc_available_oms(sender, &[sender_lsp, invoice_server, recipient])
3407+
.pop()
3408+
.unwrap();
3409+
recipient.onion_messenger.handle_onion_message(peer_id, &held_htlc_om);
3410+
3411+
// Extract the release_htlc_om and ensure the sender's LSP will release the HTLC on the next call
3412+
// to process_pending_htlc_forwards, even though the HTLC was not yet officially intercepted when
3413+
// the release message arrived.
3414+
let (peer_id, release_htlc_om) =
3415+
extract_release_htlc_oms(recipient, &[sender, sender_lsp, invoice_server]).pop().unwrap();
3416+
sender_lsp.onion_messenger.handle_onion_message(peer_id, &release_htlc_om);
3417+
3418+
sender_lsp.node.process_pending_htlc_forwards();
3419+
let mut events = sender_lsp.node.get_and_clear_pending_msg_events();
3420+
assert_eq!(events.len(), 1);
3421+
let ev = remove_first_msg_event_to_node(&invoice_server.node.get_our_node_id(), &mut events);
3422+
check_added_monitors!(sender_lsp, 1);
3423+
3424+
let path: &[&Node] = &[invoice_server, recipient];
3425+
let args = PassAlongPathArgs::new(sender_lsp, path, amt_msat, payment_hash, ev);
3426+
let claimable_ev = do_pass_along_path(args).unwrap();
3427+
3428+
let route: &[&[&Node]] = &[&[sender_lsp, invoice_server, recipient]];
3429+
let keysend_preimage = extract_payment_preimage(&claimable_ev);
3430+
let (res, _) =
3431+
claim_payment_along_route(ClaimAlongRouteArgs::new(sender, route, keysend_preimage));
3432+
assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));
3433+
}

0 commit comments

Comments
 (0)