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 |