Skip to content

Commit c972bf2

Browse files
jkczyzclaude
andcommitted
Model RBF splice tx replacement in chanmon_consistency
The SplicePending event handler was immediately confirming splice transactions, which caused force-closes when RBF splice replacements were also confirmed for the same channel. Since both transactions spend the same funding UTXO, only one can exist on a real chain. Model this properly by adding a mempool-like pending pool to ChainState. Splice transactions are added to the pending pool instead of being confirmed immediately. Conflicting RBF candidates coexist in the pool until chain-sync time, when one is selected deterministically (by txid sort order) and the rest are rejected as double-spends. If a conflicting transaction was already confirmed, new candidates are dropped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5dfe303 commit c972bf2

File tree

1 file changed

+75
-12
lines changed

1 file changed

+75
-12
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -186,24 +186,42 @@ impl BroadcasterInterface for TestBroadcaster {
186186
struct ChainState {
187187
blocks: Vec<(Header, Vec<Transaction>)>,
188188
confirmed_txids: HashSet<Txid>,
189+
/// Unconfirmed transactions (e.g., splice txs). Conflicting RBF candidates may coexist;
190+
/// `confirm_pending_txs` determines which one confirms.
191+
pending_txs: Vec<Transaction>,
189192
}
190193

191194
impl ChainState {
192195
fn new() -> Self {
193196
let genesis_hash = genesis_block(Network::Bitcoin).block_hash();
194197
let genesis_header = create_dummy_header(genesis_hash, 42);
195-
Self { blocks: vec![(genesis_header, Vec::new())], confirmed_txids: HashSet::new() }
198+
Self {
199+
blocks: vec![(genesis_header, Vec::new())],
200+
confirmed_txids: HashSet::new(),
201+
pending_txs: Vec::new(),
202+
}
196203
}
197204

198205
fn tip_height(&self) -> u32 {
199206
(self.blocks.len() - 1) as u32
200207
}
201208

209+
fn is_outpoint_spent(&self, outpoint: &bitcoin::OutPoint) -> bool {
210+
self.blocks.iter().any(|(_, txs)| {
211+
txs.iter().any(|tx| {
212+
tx.input.iter().any(|input| input.previous_output == *outpoint)
213+
})
214+
})
215+
}
216+
202217
fn confirm_tx(&mut self, tx: Transaction) -> bool {
203218
let txid = tx.compute_txid();
204219
if self.confirmed_txids.contains(&txid) {
205220
return false;
206221
}
222+
if tx.input.iter().any(|input| self.is_outpoint_spent(&input.previous_output)) {
223+
return false;
224+
}
207225
self.confirmed_txids.insert(txid);
208226

209227
let prev_hash = self.blocks.last().unwrap().0.block_hash();
@@ -218,6 +236,29 @@ impl ChainState {
218236
true
219237
}
220238

239+
/// Add a transaction to the pending pool (mempool). Multiple conflicting transactions (RBF
240+
/// candidates) may coexist; `confirm_pending_txs` selects which one to confirm. If the
241+
/// conflicting transaction was already confirmed, the new transaction is dropped since a
242+
/// confirmed transaction cannot be replaced on chain.
243+
fn add_pending_tx(&mut self, tx: Transaction) {
244+
if tx.input.iter().any(|i| self.is_outpoint_spent(&i.previous_output)) {
245+
return;
246+
}
247+
self.pending_txs.push(tx);
248+
}
249+
250+
/// Confirm pending transactions, selecting deterministically among conflicting RBF candidates.
251+
/// Sorting by txid before confirming means the winner depends on the fuzz input (which
252+
/// determines tx content and thus txid), while `confirm_tx` rejects double-spends so only one
253+
/// conflicting tx confirms.
254+
fn confirm_pending_txs(&mut self) {
255+
let mut txs = std::mem::take(&mut self.pending_txs);
256+
txs.sort_by_key(|tx| tx.compute_txid());
257+
for tx in txs {
258+
self.confirm_tx(tx);
259+
}
260+
}
261+
221262
fn block_at(&self, height: u32) -> &(Header, Vec<Transaction>) {
222263
&self.blocks[height as usize]
223264
}
@@ -856,11 +897,15 @@ fn send_mpp_hop_payment(
856897
fn assert_action_timeout_awaiting_response(action: &msgs::ErrorAction) {
857898
// Since sending/receiving messages may be delayed, `timer_tick_occurred` may cause a node to
858899
// disconnect their counterparty if they're expecting a timely response.
859-
assert!(matches!(
900+
assert!(
901+
matches!(
902+
action,
903+
msgs::ErrorAction::DisconnectPeerWithWarning { msg }
904+
if msg.data.contains("Disconnecting due to timeout awaiting response")
905+
),
906+
"Expected timeout disconnect, got: {:?}",
860907
action,
861-
msgs::ErrorAction::DisconnectPeerWithWarning { msg }
862-
if msg.data.contains("Disconnecting due to timeout awaiting response")
863-
));
908+
);
864909
}
865910

866911
enum ChanType {
@@ -2025,7 +2070,7 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
20252070
assert!(txs.len() >= 1);
20262071
let splice_tx = txs.remove(0);
20272072
assert_eq!(new_funding_txo.txid, splice_tx.compute_txid());
2028-
chain_state.confirm_tx(splice_tx);
2073+
chain_state.add_pending_tx(splice_tx);
20292074
},
20302075
events::Event::SpliceFailed { .. } => {},
20312076
events::Event::DiscardFunding {
@@ -2478,13 +2523,31 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
24782523
},
24792524

24802525
// Sync node by 1 block to cover confirmation of a transaction.
2481-
0xa8 => sync_with_chain_state(&mut chain_state, &nodes[0], &mut node_height_a, Some(1)),
2482-
0xa9 => sync_with_chain_state(&mut chain_state, &nodes[1], &mut node_height_b, Some(1)),
2483-
0xaa => sync_with_chain_state(&mut chain_state, &nodes[2], &mut node_height_c, Some(1)),
2526+
0xa8 => {
2527+
chain_state.confirm_pending_txs();
2528+
sync_with_chain_state(&mut chain_state, &nodes[0], &mut node_height_a, Some(1));
2529+
},
2530+
0xa9 => {
2531+
chain_state.confirm_pending_txs();
2532+
sync_with_chain_state(&mut chain_state, &nodes[1], &mut node_height_b, Some(1));
2533+
},
2534+
0xaa => {
2535+
chain_state.confirm_pending_txs();
2536+
sync_with_chain_state(&mut chain_state, &nodes[2], &mut node_height_c, Some(1));
2537+
},
24842538
// Sync node to chain tip to cover confirmation of a transaction post-reorg-risk.
2485-
0xab => sync_with_chain_state(&mut chain_state, &nodes[0], &mut node_height_a, None),
2486-
0xac => sync_with_chain_state(&mut chain_state, &nodes[1], &mut node_height_b, None),
2487-
0xad => sync_with_chain_state(&mut chain_state, &nodes[2], &mut node_height_c, None),
2539+
0xab => {
2540+
chain_state.confirm_pending_txs();
2541+
sync_with_chain_state(&mut chain_state, &nodes[0], &mut node_height_a, None);
2542+
},
2543+
0xac => {
2544+
chain_state.confirm_pending_txs();
2545+
sync_with_chain_state(&mut chain_state, &nodes[1], &mut node_height_b, None);
2546+
},
2547+
0xad => {
2548+
chain_state.confirm_pending_txs();
2549+
sync_with_chain_state(&mut chain_state, &nodes[2], &mut node_height_c, None);
2550+
},
24882551

24892552
0xb0 | 0xb1 | 0xb2 => {
24902553
// Restart node A, picking among the in-flight `ChannelMonitor`s to use based on

0 commit comments

Comments
 (0)