Skip to content

Commit 38e3070

Browse files
committed
Wipe empty entries from actions_blocking_raa_monitor_updates
In a very specific case, forgetting to do so can lead to a debug assertion failure when we see a double-claim of an HTLC (see the included test). Found by @joostjager's work on growing the chanmon_consistency fuzzer.
1 parent 77a0e45 commit 38e3070

2 files changed

Lines changed: 74 additions & 4 deletions

File tree

lightning/src/ln/channelmanager.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15251,10 +15251,12 @@ impl<
1525115251
let peer_state = &mut *peer_state_lck;
1525215252
if let Some(blocker) = completed_blocker.take() {
1525315253
// Only do this on the first iteration of the loop.
15254-
if let Some(blockers) = peer_state.actions_blocking_raa_monitor_updates
15255-
.get_mut(&channel_id)
15256-
{
15257-
blockers.retain(|iter| iter != &blocker);
15254+
let entry = peer_state.actions_blocking_raa_monitor_updates.entry(channel_id);
15255+
if let btree_map::Entry::Occupied(mut entry) = entry {
15256+
entry.get_mut().retain(|iter| iter != &blocker);
15257+
if entry.get().is_empty() {
15258+
entry.remove();
15259+
}
1525815260
}
1525915261
}
1526015262

lightning/src/ln/functional_tests.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10168,3 +10168,71 @@ pub fn test_dust_exposure_holding_cell_assertion() {
1016810168
// Now that everything has settled, make sure the channels still work with a simple claim.
1016910169
claim_payment(&nodes[2], &[&nodes[1]], payment_preimage_cb);
1017010170
}
10171+
10172+
#[test]
10173+
fn test_dup_htlc_claim_onchain_and_offchain() {
10174+
// Tests what happens if we receive a claim first offchain, then see a counterparty broadcast
10175+
// their commitment transaction and re-claim the same HTLC on-chain. This was never broken, but
10176+
// the very specific ordering in this test did hit a debug assertion failure.
10177+
let chanmon_cfgs = create_chanmon_cfgs(3);
10178+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
10179+
let legacy_cfg = test_legacy_channel_config();
10180+
let node_chanmgrs = create_node_chanmgrs(
10181+
3,
10182+
&node_cfgs,
10183+
&[Some(legacy_cfg.clone()), Some(legacy_cfg.clone()), Some(legacy_cfg)],
10184+
);
10185+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
10186+
10187+
let node_b_id = nodes[1].node.get_our_node_id();
10188+
let node_c_id = nodes[2].node.get_our_node_id();
10189+
10190+
create_announced_chan_between_nodes(&nodes, 0, 1);
10191+
let chan_bc = create_announced_chan_between_nodes(&nodes, 1, 2);
10192+
10193+
// Route payment A -> B -> C.
10194+
let (payment_preimage, payment_hash, _, _) =
10195+
route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
10196+
10197+
// C claims the payment.
10198+
nodes[2].node.claim_funds(payment_preimage);
10199+
expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);
10200+
check_added_monitors(&nodes[2], 1);
10201+
10202+
// Deliver only C's update_fulfill_htlc to B (NOT the commitment_signed). B learns
10203+
// the preimage and claims from A (adding an RAA blocker on B-C via
10204+
// internal_update_fulfill_htlc, then removing it when the A-B monitor update completes
10205+
// and the EmitEventOptionAndFreeOtherChannel action runs).
10206+
let cs_updates = get_htlc_update_msgs(&nodes[2], &node_b_id);
10207+
nodes[1]
10208+
.node
10209+
.handle_update_fulfill_htlc(node_c_id, cs_updates.update_fulfill_htlcs[0].clone());
10210+
check_added_monitors(&nodes[1], 1);
10211+
10212+
// Ignore B's attempts to claim the HTLC from A.
10213+
nodes[1].node.get_and_clear_pending_msg_events();
10214+
10215+
// Get C's commitment transactions. C's commitment includes the HTLC and C has
10216+
// an HTLC-success transaction (claiming with preimage). Mine both on B.
10217+
let cs_txn = get_local_commitment_txn!(nodes[2], chan_bc.2);
10218+
assert!(cs_txn.len() >= 2, "Expected commitment + HTLC-success tx, got {}", cs_txn.len());
10219+
10220+
// Mine C's commitment on B. B sees the counterparty commitment on-chain.
10221+
mine_transaction(&nodes[1], &cs_txn[0]);
10222+
check_closed_broadcast(&nodes[1], 1, true);
10223+
check_added_monitors(&nodes[1], 1);
10224+
let events = nodes[1].node.get_and_clear_pending_events();
10225+
assert!(
10226+
events.iter().any(|e| matches!(e, Event::ChannelClosed { .. })),
10227+
"Expected ChannelClosed event"
10228+
);
10229+
10230+
// Mine C's HTLC-success transaction. B's monitor sees the preimage being used on-chain
10231+
// and generates an HTLCEvent with the preimage.
10232+
mine_transaction(&nodes[1], &cs_txn[1]);
10233+
10234+
// Advance past ANTI_REORG_DELAY so the on-chain HTLC resolution matures. This triggers
10235+
// the monitor to generate an HTLCEvent with the preimage via process_pending_monitor_events,
10236+
// which calls claim_funds_internal a second time.
10237+
connect_blocks(&nodes[1], ANTI_REORG_DELAY);
10238+
}

0 commit comments

Comments
 (0)