Skip to content

Commit 8cab7d0

Browse files
Merge pull request #4537 from TheBlueMatt/2026-04-blocking-actions-missed-clear
Wipe empty entries from `actions_blocking_raa_monitor_updates`
2 parents 4ac8a76 + f14b4b2 commit 8cab7d0

2 files changed

Lines changed: 79 additions & 8 deletions

File tree

lightning/src/ln/channelmanager.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9799,12 +9799,12 @@ impl<
97999799
{
98009800
if let Some(peer_state_mtx) = per_peer_state.get(&node_id) {
98019801
let mut peer_state = peer_state_mtx.lock().unwrap();
9802-
if let Some(blockers) = peer_state
9802+
let entry = peer_state
98039803
.actions_blocking_raa_monitor_updates
9804-
.get_mut(&channel_id)
9805-
{
9804+
.entry(channel_id);
9805+
if let btree_map::Entry::Occupied(mut entry) = entry {
98069806
let mut found_blocker = false;
9807-
blockers.retain(|iter| {
9807+
entry.get_mut().retain(|iter| {
98089808
// Note that we could actually be blocked, in
98099809
// which case we need to only remove the one
98109810
// blocker which was added duplicatively.
@@ -9814,6 +9814,9 @@ impl<
98149814
}
98159815
*iter != blocker || !first_blocker
98169816
});
9817+
if entry.get().is_empty() {
9818+
entry.remove();
9819+
}
98179820
debug_assert!(found_blocker);
98189821
}
98199822
} else {
@@ -15172,10 +15175,12 @@ impl<
1517215175
let peer_state = &mut *peer_state_lck;
1517315176
if let Some(blocker) = completed_blocker.take() {
1517415177
// Only do this on the first iteration of the loop.
15175-
if let Some(blockers) = peer_state.actions_blocking_raa_monitor_updates
15176-
.get_mut(&channel_id)
15177-
{
15178-
blockers.retain(|iter| iter != &blocker);
15178+
let entry = peer_state.actions_blocking_raa_monitor_updates.entry(channel_id);
15179+
if let btree_map::Entry::Occupied(mut entry) = entry {
15180+
entry.get_mut().retain(|iter| iter != &blocker);
15181+
if entry.get().is_empty() {
15182+
entry.remove();
15183+
}
1517915184
}
1518015185
}
1518115186

lightning/src/ln/functional_tests.rs

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

0 commit comments

Comments
 (0)