From 41d0f66e57e7c16073fe7e840d2cbeb7e73aeb4c Mon Sep 17 00:00:00 2001 From: Nitesh Balusu Date: Sat, 17 Oct 2026 19:13:16 -0400 Subject: [PATCH] consensus: catch a BIP30 duplicate inside one confirm batch The BIP30 check looked up earlier creates of a txid in the store, which only sees connected blocks. An earlier block of the same write batch is not connected yet, so a duplicate coinbase one block later in the same batch was accepted. Core rejects it. Structural validation now remembers each checked block's txids and spendable outputs for the rest of the batch, and a later duplicate with an output not yet spent in the batch is rejected through the same rule. The mainnet repeat exceptions and BIP34 skips still run first. Only reachable below BIP34, for example regtest with -testactivationheight=bip34@N. Co-Authored-By: Claude Opus 5.5 --- changelog.d/bip30-same-batch.md | 6 ++++ crates/rbitcoin-query/src/block_pipeline.rs | 36 ++++++++++++++++++- crates/rbitcoin-test/tests/consensus_rules.rs | 36 +++++++++++++++++++ docs/consensus-tests.md | 2 +- 4 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 changelog.d/bip30-same-batch.md diff --git a/changelog.d/bip30-same-batch.md b/changelog.d/bip30-same-batch.md new file mode 100644 index 000000000..f61632268 --- /dev/null +++ b/changelog.d/bip30-same-batch.md @@ -0,0 +1,6 @@ +Fixed + +- **BIP30 sees a duplicate created earlier in the same confirm batch.** + A batch's earlier blocks are not connected yet, so the store lookup missed + their creates. Only reachable below BIP34 (for example regtest with + `-testactivationheight=bip34@N`). diff --git a/crates/rbitcoin-query/src/block_pipeline.rs b/crates/rbitcoin-query/src/block_pipeline.rs index 0458e0f6c..09f71f0ad 100644 --- a/crates/rbitcoin-query/src/block_pipeline.rs +++ b/crates/rbitcoin-query/src/block_pipeline.rs @@ -541,6 +541,10 @@ pub(crate) struct StructuralScratch { durable_spent: DurableSpentSet, height_list: Vec, create_height_by_fk: FkMap, + /// BIP30 inside one batch: each txid already created here, with its height + /// and the span of `batch_vouts` holding its spendable outputs. + batch_creates: TxidMap<(u32, u32, u32)>, + batch_vouts: Vec, pub slots: AnnotateSlots, } @@ -560,6 +564,8 @@ impl Default for StructuralScratch { durable_spent: DurableSpentSet::with_hasher(Default::default()), height_list: Vec::new(), create_height_by_fk: FkMap::default(), + batch_creates: TxidMap::default(), + batch_vouts: Vec::new(), slots: AnnotateSlots::default(), } } @@ -571,6 +577,8 @@ impl StructuralScratch { pub(crate) fn begin_batch(&mut self) { self.slots.abs_edges.clear(); self.slots.known.clear(); + self.batch_creates.clear(); + self.batch_vouts.clear(); } fn begin_block(&mut self) { @@ -623,7 +631,7 @@ pub fn structural_validate_spends( let maturity = ctx.params.coinbase_maturity(); // BIP30's txid batch is inside `spent_ns` (signet runs it on every block). let t_spent = Instant::now(); - reject_bip30_unspent_overwrite(query, block, ctx)?; + reject_bip30_unspent_overwrite(query, block, ctx, pending_spent, scratch)?; let t_abs = Instant::now(); structural_abs_heights(query, run_create_height, scratch, abs_jobs)?; let tip = query.tip_height().map(|h| h.0); @@ -1164,6 +1172,8 @@ fn reject_bip30_unspent_overwrite( query: &Query, block: &Block, ctx: &rbitcoin_consensus::ValidationContext<'_>, + pending_spent: &rbitcoin_consensus::OutPointSet, + scratch: &mut StructuralScratch, ) -> Result<(), ConfirmError> { if ctx.params.is_bip30_repeat(ctx.height.0, block.block_hash()) || bip34_ancestry_skips_bip30(query, ctx) @@ -1175,6 +1185,17 @@ fn reject_bip30_unspent_overwrite( .iter() .map(|tx| tx.compute_txid().to_byte_array()) .collect(); + // An earlier block of this batch is not connected yet, so the store + // lookup below cannot see its creates. + for txid in &create_txids { + let Some(&(prior_height, lo, hi)) = scratch.batch_creates.get(txid) else { + continue; + }; + let unspent = scratch.batch_vouts[lo as usize..hi as usize] + .iter() + .any(|&v| !pending_spent.contains(&(*txid, v))); + rbitcoin_consensus::block::reject_bip30_unspent_prior(ctx.height.0, prior_height, unspent)?; + } let hits = query .store() .get_fk_by_txid_batch(&create_txids) @@ -1220,6 +1241,19 @@ fn reject_bip30_unspent_overwrite( )?; } } + for (txid, tx) in create_txids.into_iter().zip(&block.txdata) { + let lo = scratch.batch_vouts.len() as u32; + scratch.batch_vouts.extend( + (0u32..) + .zip(&tx.output) + .filter(|(_, out)| { + !rbitcoin_consensus::policy::is_unspendable(out.script_pubkey.as_bytes()) + }) + .map(|(v, _)| v), + ); + let hi = scratch.batch_vouts.len() as u32; + scratch.batch_creates.insert(txid, (ctx.height.0, lo, hi)); + } Ok(()) } diff --git a/crates/rbitcoin-test/tests/consensus_rules.rs b/crates/rbitcoin-test/tests/consensus_rules.rs index 02aa46d84..3d95f1ec9 100644 --- a/crates/rbitcoin-test/tests/consensus_rules.rs +++ b/crates/rbitcoin-test/tests/consensus_rules.rs @@ -717,6 +717,42 @@ fn header_and_spending_boundaries() { pin_subsidy_interval_two_overlay(); } +/// Below BIP34 two coinbases can share a txid. Core rejects the second while +/// the first is unspent, including when both blocks arrive in one run. +#[test] +fn bip30_duplicate_coinbase_in_one_batch() { + let (_td, q, mut params) = regtest_q(); + params.apply_test_activation_height("bip34", 1_000).unwrap(); + connect_genesis(&q, ¶ms); + let g = regtest_genesis(); + let b1 = mine_regtest_block(g.block_hash(), g.header.time + 600, 1, vec![]); + let mut b2 = mine_regtest_block(b1.block_hash(), b1.header.time + 600, 2, vec![]); + b2.txdata[0] = b1.txdata[0].clone(); + b2.header.merkle_root = b2.compute_merkle_root().unwrap(); + rbitcoin_consensus::grind_regtest_pow(&mut b2.header); + let bip30 = |r: &Result<_, ConfirmError>| matches!(r, Err(ConfirmError::Rules(ConsensusError::BadTx(s))) if s.contains("BIP30")); + + let one_batch = confirm_wire_run( + &q, + ¶ms, + Milestone::NONE, + &[(Height(1), b1.clone()), (Height(2), b2.clone())], + ); + assert!( + bip30(&one_batch), + "duplicate coinbase one block later in the same batch: {one_batch:?}" + ); + assert_eq!(q.tip_height(), Some(Height::GENESIS)); + + confirm_wire_run(&q, ¶ms, Milestone::NONE, &[(Height(1), b1)]).unwrap(); + let next_batch = confirm_wire_run(&q, ¶ms, Milestone::NONE, &[(Height(2), b2)]); + assert!( + bip30(&next_batch), + "duplicate coinbase in the next batch: {next_batch:?}" + ); + assert_eq!(q.tip_height(), Some(Height(1))); +} + // ─── Script flags (Core `GetBlockScriptFlags`) ────────────────────────────── /// Core sets WITNESS on every block, not from the segwit height. Below the diff --git a/docs/consensus-tests.md b/docs/consensus-tests.md index bfda6eae2..b521159ba 100644 --- a/docs/consensus-tests.md +++ b/docs/consensus-tests.md @@ -141,7 +141,7 @@ version floors and exact +2h). | C34 | Base CHECKMULTISIG FindAndDelete of an empty sig removes every OP_0 opcode before the other sigs hash scriptCode | accept / `SIG_DER` | `legacy_multisig_empty_sig_deletes_op_0_from_script_code` | | C35 | P2SH scriptPubKey `HASH160 <20> EQUAL` runs on the scriptSig stack: 1000 items fail `MAX_STACK_SIZE` at the `<20>` push, with or without BIP16 | `stack size` | `p2sh_script_pubkey_push_counts_against_max_stack_size` ([086](./external_findings/086-p2sh-spk-stack-size.md)) | | C36 | A tx is final at the connect height | `locktime=100` accepts at height 101; `locktime==height` rejects | `header_and_spending_boundaries` | -| C37 | BIP30: a txid is not created again while an unspent output of that txid still exists. Exceptions 91842 and 91880 | every connecting block; `is_bip30_repeat_matches_core` | `buried_rules_and_a_lying_header_path` | +| C37 | BIP30: a txid is not created again while an unspent output of that txid still exists. Exceptions 91842 and 91880 | every connecting block; `is_bip30_repeat_matches_core` | `buried_rules_and_a_lying_header_path`, `bip30_duplicate_coinbase_in_one_batch` | | C38 | An input's prevout exists and is still unspent | height-1 coinbase spend accepts | missing txid → `MissingPrevout`; child-before-parent; a second spend of the same outpoint; two spends in one block. `header_and_spending_boundaries` | | C40 | A tx's output sum ≤ its input sum | `in==out` accepts | `in+1` rejects. `header_and_spending_boundaries` | | C41 | BIP68 relative finality. The coinbase is not an input: version ≥ 2 and a relative lock that would fail on a non-coinbase still connect | `nSequence=10` at height 101 accepts; time-type after MTP | `nSequence=200` at 101 rejects; `bip68_skips_a_coinbase_whose_relative_lock_would_fail`; `finality_tests` 109/110 |