Model RBF splice tx replacement in chanmon_consistency
What changed, and why it matters
This commit fixes a bug in a fuzzing test harness, not in the production Lightning Dev Kit code. The test was incorrectly treating splice transactions as confirmed immediately, which could cause simulated force-closes when multiple RBF (fee-bump) replacements existed. The fix adds a fake mempool so the test models real Bitcoin behavior: only one transaction spending the same funding output can be confirmed. End users running real Lightning nodes are not directly affected.
No production action required. Reviewers should verify the fuzz harness now exercises both RBF replacement confirmation and rejection paths correctly, and that production splice/RBF handling in the non-fuzz codebase is separately tested.
Security signals we found
Fixes a test-harness modeling bug that produced false-positive force-closes during fuzzing of splice/RBF flows
Adds double-spend detection for confirmed outpoints in confirm_tx and confirm_pending_txs
No change to consensus, P2P, or channel-state machine logic in production crates
Evidence from the diff
The change is confined to fuzz/src/chanmon_consistency.rs, a fuzzing target. ChainState now holds a pending_txs mempool. SplicePending event handling adds the splice tx to the pool instead of confirming it directly. During chain-sync fuzz operations, confirm_pending_txs sorts pending transactions by txid and confirms non-conflicting ones in a block, skipping double-spends. This prevents the harness from confirming both an original splice tx and an RBF replacement, which previously caused inconsistent channel state and force-closes in the fuzz model.
Changed components
fuzz/src/chanmon_consistency.rsChainState test helperSplicePending event handler in fuzz targetRBF splice transaction fuzz modelingInspect captured patch +99 / −12
diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
index b205120..9602fc9 100644
--- a/fuzz/src/chanmon_consistency.rs
+++ b/fuzz/src/chanmon_consistency.rs
@@ -186,24 +186,42 @@ impl BroadcasterInterface for TestBroadcaster {
struct ChainState {
blocks: Vec<(Header, Vec<Transaction>)>,
confirmed_txids: HashSet<Txid>,
+ /// Unconfirmed transactions (e.g., splice txs). Conflicting RBF candidates may coexist;
+ /// `confirm_pending_txs` determines which one confirms.
+ pending_txs: Vec<(Txid, Transaction)>,
}
impl ChainState {
fn new() -> Self {
let genesis_hash = genesis_block(Network::Bitcoin).block_hash();
let genesis_header = create_dummy_header(genesis_hash, 42);
- Self { blocks: vec![(genesis_header, Vec::new())], confirmed_txids: HashSet::new() }
+ Self {
+ blocks: vec![(genesis_header, Vec::new())],
+ confirmed_txids: HashSet::new(),
+ pending_txs: Vec::new(),
+ }
}
fn tip_height(&self) -> u32 {
(self.blocks.len() - 1) as u32
}
+ fn is_outpoint_spent(&self, outpoint: &bitcoin::OutPoint) -> bool {
+ self.blocks.iter().any(|(_, txs)| {
+ txs.iter().any(|tx| {
+ tx.input.iter().any(|input| input.previous_output == *outpoint)
+ })
+ })
+ }
+
fn confirm_tx(&mut self, tx: Transaction) -> bool {
let txid = tx.compute_txid();
if self.confirmed_txids.contains(&txid) {
return false;
}
+ if tx.input.iter().any(|input| self.is_outpoint_spent(&input.previous_output)) {
+ return false;
+ }
self.confirmed_txids.insert(txid);
let prev_hash = self.blocks.last().unwrap().0.block_hash();
@@ -218,6 +236,53 @@ impl ChainState {
true
}
+ /// Add a transaction to the pending pool (mempool). Multiple conflicting transactions (RBF
+ /// candidates) may coexist; `confirm_pending_txs` selects which one to confirm.
+ fn add_pending_tx(&mut self, tx: Transaction) {
+ self.pending_txs.push((tx.compute_txid(), tx));
+ }
+
+ /// Confirm pending transactions in a single block, selecting deterministically among
+ /// conflicting RBF candidates. Sorting by txid ensures the winner is determined by fuzz input
+ /// content. Transactions that double-spend an already-confirmed outpoint are skipped.
+ fn confirm_pending_txs(&mut self) {
+ let mut txs = std::mem::take(&mut self.pending_txs);
+ txs.sort_by_key(|(txid, _)| *txid);
+
+ let mut confirmed = Vec::new();
+ let mut spent_outpoints = Vec::new();
+ for (txid, tx) in txs {
+ if self.confirmed_txids.contains(&txid) {
+ continue;
+ }
+ if tx.input.iter().any(|input| {
+ self.is_outpoint_spent(&input.previous_output)
+ || spent_outpoints.contains(&input.previous_output)
+ }) {
+ continue;
+ }
+ self.confirmed_txids.insert(txid);
+ for input in &tx.input {
+ spent_outpoints.push(input.previous_output);
+ }
+ confirmed.push(tx);
+ }
+
+ if confirmed.is_empty() {
+ return;
+ }
+
+ let prev_hash = self.blocks.last().unwrap().0.block_hash();
+ let header = create_dummy_header(prev_hash, 42);
+ self.blocks.push((header, confirmed));
+
+ for _ in 0..5 {
+ let prev_hash = self.blocks.last().unwrap().0.block_hash();
+ let header = create_dummy_header(prev_hash, 42);
+ self.blocks.push((header, Vec::new()));
+ }
+ }
+
fn block_at(&self, height: u32) -> &(Header, Vec<Transaction>) {
&self.blocks[height as usize]
}
@@ -862,11 +927,15 @@ fn send_mpp_hop_payment(
fn assert_action_timeout_awaiting_response(action: &msgs::ErrorAction) {
// Since sending/receiving messages may be delayed, `timer_tick_occurred` may cause a node to
// disconnect their counterparty if they're expecting a timely response.
- assert!(matches!(
+ assert!(
+ matches!(
+ action,
+ msgs::ErrorAction::DisconnectPeerWithWarning { msg }
+ if msg.data.contains("Disconnecting due to timeout awaiting response")
+ ),
+ "Expected timeout disconnect, got: {:?}",
action,
- msgs::ErrorAction::DisconnectPeerWithWarning { msg }
- if msg.data.contains("Disconnecting due to timeout awaiting response")
- ));
+ );
}
enum ChanType {
@@ -2033,7 +2102,7 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
assert!(txs.len() >= 1);
let splice_tx = txs.remove(0);
assert_eq!(new_funding_txo.txid, splice_tx.compute_txid());
- chain_state.confirm_tx(splice_tx);
+ chain_state.add_pending_tx(splice_tx);
},
events::Event::SpliceFailed { .. } => {},
events::Event::DiscardFunding {
@@ -2506,13 +2575,31 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
},
// Sync node by 1 block to cover confirmation of a transaction.
- 0xa8 => sync_with_chain_state(&mut chain_state, &nodes[0], &mut node_height_a, Some(1)),
- 0xa9 => sync_with_chain_state(&mut chain_state, &nodes[1], &mut node_height_b, Some(1)),
- 0xaa => sync_with_chain_state(&mut chain_state, &nodes[2], &mut node_height_c, Some(1)),
+ 0xa8 => {
+ chain_state.confirm_pending_txs();
+ sync_with_chain_state(&mut chain_state, &nodes[0], &mut node_height_a, Some(1));
+ },
+ 0xa9 => {
+ chain_state.confirm_pending_txs();
+ sync_with_chain_state(&mut chain_state, &nodes[1], &mut node_height_b, Some(1));
+ },
+ 0xaa => {
+ chain_state.confirm_pending_txs();
+ sync_with_chain_state(&mut chain_state, &nodes[2], &mut node_height_c, Some(1));
+ },
// Sync node to chain tip to cover confirmation of a transaction post-reorg-risk.
- 0xab => sync_with_chain_state(&mut chain_state, &nodes[0], &mut node_height_a, None),
- 0xac => sync_with_chain_state(&mut chain_state, &nodes[1], &mut node_height_b, None),
- 0xad => sync_with_chain_state(&mut chain_state, &nodes[2], &mut node_height_c, None),
+ 0xab => {
+ chain_state.confirm_pending_txs();
+ sync_with_chain_state(&mut chain_state, &nodes[0], &mut node_height_a, None);
+ },
+ 0xac => {
+ chain_state.confirm_pending_txs();
+ sync_with_chain_state(&mut chain_state, &nodes[1], &mut node_height_b, None);
+ },
+ 0xad => {
+ chain_state.confirm_pending_txs();
+ sync_with_chain_state(&mut chain_state, &nodes[2], &mut node_height_c, None);
+ },
0xb0 | 0xb1 | 0xb2 => {
// Restart node A, picking among the in-flight `ChannelMonitor`s to use based on
Why this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.