From cc22910b330f55c9b9eed3aa457a4223ba8fbdb5 Mon Sep 17 00:00:00 2001 From: Nitesh Balusu Date: Sat, 17 Oct 2026 22:07:16 -0400 Subject: [PATCH 1/5] consensus: handle a txid repeated inside one confirm batch like Core Below BIP34 a coinbase can repeat once its first copy is spent. Core 31.1 accepts a chain where h1 creates X, h102 spends X:0, h103 creates X again and h203 spends the new X:0. rbitcoin could not connect it whenever those blocks shared a confirm batch, and IBD halted. - The load planner resolves a spend to the newest copy created before it, then to an outside copy, and only then to a later in-batch copy, which structural still rejects as not yet a coin. - Block assembly and structural validation free the per-batch spent marks of a txid when it is created again, so its new outputs can be spent. - BIP30 counts spends made earlier in the same batch when it checks a connected earlier copy. The repeat bookkeeping only runs below BIP34. Co-Authored-By: Claude Opus 5.5 --- changelog.d/repeated-txid-in-batch.md | 9 ++ crates/rbitcoin-query/src/archive.rs | 69 ++++++++-- crates/rbitcoin-query/src/block_pipeline.rs | 36 ++++- crates/rbitcoin-test/tests/consensus_rules.rs | 126 ++++++++++++++++++ docs/consensus-tests.md | 2 +- 5 files changed, 226 insertions(+), 16 deletions(-) create mode 100644 changelog.d/repeated-txid-in-batch.md diff --git a/changelog.d/repeated-txid-in-batch.md b/changelog.d/repeated-txid-in-batch.md new file mode 100644 index 000000000..db6ed8a89 --- /dev/null +++ b/changelog.d/repeated-txid-in-batch.md @@ -0,0 +1,9 @@ +Fixed + +- **A txid that repeats inside one confirm batch is handled like Core.** + Below BIP34 a coinbase can repeat once its first copy is spent. A spend now + uses the copy that exists at its height, the outputs of a recreated + transaction can be spent again, and BIP30 counts spends made earlier in the + same batch. Before, IBD could halt on such a chain that Core accepts. Only + reachable below BIP34 (for example regtest with + `-testactivationheight=bip34@N`). diff --git a/crates/rbitcoin-query/src/archive.rs b/crates/rbitcoin-query/src/archive.rs index 3935dbff5..e56fa70aa 100644 --- a/crates/rbitcoin-query/src/archive.rs +++ b/crates/rbitcoin-query/src/archive.rs @@ -570,27 +570,59 @@ struct PlanRow { tx_index: u32, } +/// The newest copy of `txid` this batch creates before `spender`. Below BIP34 +/// a txid can repeat once its first copy is spent; a spend uses the copy that +/// exists at its height. +fn batch_copy_before( + batch_map: &crate::TxidMap, + batch_repeats: &crate::TxidMap>, + txid: &[u8; 32], + spender: Fk, +) -> Option { + match batch_repeats.get(txid) { + Some(fks) => fks.iter().rev().find(|f| f.0 < spender.0).copied(), + None => batch_map.get(txid).copied().filter(|f| f.0 < spender.0), + } +} + +/// Parents to resolve outside this batch. A txid this batch only creates after +/// the spend is asked for when `outside` can have it (an earlier copy); otherwise +/// it stays a later in-batch create, which structural rejects as not yet a coin. fn collect_plan_need_external( work: &[PlanRow], batch_map: &crate::TxidMap, + batch_repeats: &crate::TxidMap>, carried_need: Option<&[[u8; 32]]>, + outside: impl Fn(&[u8; 32]) -> bool, ) -> Vec<[u8; 32]> { let mut need_external: crate::TxidSet = crate::TxidSet::with_hasher(Default::default()); if let Some(keys) = carried_need { + let mut created_here = false; for &prev in keys { - if prev == [0u8; 32] || batch_map.contains_key(&prev) { + if prev == [0u8; 32] { + continue; + } + if batch_map.contains_key(&prev) { + created_here = true; continue; } need_external.insert(prev); } - return need_external.into_iter().collect(); + // The wave asked about a txid this batch creates (below BIP34, or a + // spend before its create): only a per-input walk knows which copy. + if !created_here { + return need_external.into_iter().collect(); + } } for row in work { for inp in row.ins.iter() { - if inp.is_coinbase { + if inp.is_coinbase || inp.prev_txid == [0u8; 32] { + continue; + } + if batch_copy_before(batch_map, batch_repeats, &inp.prev_txid, row.tx_fk).is_some() { continue; } - if batch_map.contains_key(&inp.prev_txid) || inp.prev_txid == [0u8; 32] { + if batch_map.contains_key(&inp.prev_txid) && !outside(&inp.prev_txid) { continue; } need_external.insert(inp.prev_txid); @@ -1069,8 +1101,8 @@ impl Query { /// /// Does not build [`TxApply`]. `body_est` uses packed encoded lengths. /// Same txid in one block is Corrupt. Same txid across headers in the wave - /// is BIP30 (91842/91880): both rows are planned; `batch_map` keeps the - /// later fk. + /// is BIP30 (91842/91880, or regtest below BIP34): both rows are planned, + /// and a spend uses the newest copy created before it. pub fn archive_plan_batch_from_wire( &self, need: &[WirePlanNeed<'_>], @@ -1091,6 +1123,7 @@ impl Query { let t_assign = Instant::now(); let mut batch_map: crate::TxidMap = crate::TxidMap::default(); + let mut batch_repeats: crate::TxidMap> = crate::TxidMap::default(); let mut work: Vec = Vec::new(); let mut per_header_ranges: Vec<(Fk, Fk, u32)> = Vec::with_capacity(need.len()); @@ -1113,7 +1146,12 @@ impl Query { } let tx_fk = Fk(next_tx); next_tx += 1; - batch_map.insert(*txid, tx_fk); + if let Some(prev) = batch_map.insert(*txid, tx_fk) { + batch_repeats + .entry(*txid) + .or_insert_with(|| vec![prev]) + .push(tx_fk); + } let rec = tx_record_from_wire(tx, *txid); let ins: Vec = tx.input.iter().map(plan_in_from_txin).collect(); work.push(PlanRow { @@ -1130,6 +1168,7 @@ impl Query { let plan = self.finish_archive_plan( work, batch_map, + batch_repeats, per_header_ranges, n_headers, assign_ns, @@ -1145,6 +1184,7 @@ impl Query { &self, work: Vec, batch_map: crate::TxidMap, + batch_repeats: crate::TxidMap>, per_header_ranges: Vec<(Fk, Fk, u32)>, n_headers: u64, assign_ns: u64, @@ -1157,7 +1197,12 @@ impl Query { let index_tx = self.tx_index_enabled(); let t_collect = Instant::now(); - let need_vec = collect_plan_need_external(&work, &batch_map, carried_need); + // With a skeleton, a key the wave did not resolve is Corrupt, so ask + // only for what in-flight or the skeleton holds. + let need_vec = + collect_plan_need_external(&work, &batch_map, &batch_repeats, carried_need, |t| { + skeleton.is_none_or(|s| s.get(t).is_some()) || in_flight.get_create_fk(t).is_some() + }); let collect_ns = t_collect.elapsed().as_nanos() as u64; let ext = crate::stamp_external_parents( @@ -1209,12 +1254,18 @@ impl Query { }); continue; } - let create_fk = if let Some(&cfk) = batch_map.get(&inp.prev_txid) { + let create_fk = if let Some(cfk) = + batch_copy_before(&batch_map, &batch_repeats, &inp.prev_txid, tx_fk) + { batch_stamp = batch_stamp.saturating_add(1); cfk } else if let Some(&cfk) = resolved.get(&inp.prev_txid) { resolved_stamp = resolved_stamp.saturating_add(1); cfk + } else if let Some(&cfk) = batch_map.get(&inp.prev_txid) { + // Only a later copy here: not a coin yet; structural rejects it. + batch_stamp = batch_stamp.saturating_add(1); + cfk } else { return Err(StoreError::Corrupt( "archive: parent create_fk unresolved (contiguous batch required)", diff --git a/crates/rbitcoin-query/src/block_pipeline.rs b/crates/rbitcoin-query/src/block_pipeline.rs index 09f71f0ad..b941af941 100644 --- a/crates/rbitcoin-query/src/block_pipeline.rs +++ b/crates/rbitcoin-query/src/block_pipeline.rs @@ -193,6 +193,7 @@ pub fn assemble_block_prevouts( let t_loop = Instant::now(); let lock_time_cutoff = assemble_lock_time_cutoff(ctx, block, prev_mtp); + let txids_can_repeat = !ctx.params.bip34_active_at(ctx.height.0); for (ti, tx) in block.txdata.iter().enumerate() { let spend_fk = archived_tx_fks.map(|fks| fks[ti]); @@ -231,6 +232,9 @@ pub fn assemble_block_prevouts( if !create_fk.is_null() { pending_creates.insert(txid, create_fk); } + if txids_can_repeat { + release_repeated_outputs(pending_spent, txid, tx.output.len()); + } } let clk_prev = t_loop.elapsed().as_nanos() as u64; @@ -1172,7 +1176,7 @@ fn reject_bip30_unspent_overwrite( query: &Query, block: &Block, ctx: &rbitcoin_consensus::ValidationContext<'_>, - pending_spent: &rbitcoin_consensus::OutPointSet, + pending_spent: &mut rbitcoin_consensus::OutPointSet, scratch: &mut StructuralScratch, ) -> Result<(), ConfirmError> { if ctx.params.is_bip30_repeat(ctx.height.0, block.block_hash()) @@ -1185,6 +1189,7 @@ fn reject_bip30_unspent_overwrite( .iter() .map(|tx| tx.compute_txid().to_byte_array()) .collect(); + let txids_can_repeat = !ctx.params.bip34_active_at(ctx.height.0); // An earlier block of this batch is not connected yet, so the store // lookup below cannot see its creates. for txid in &create_txids { @@ -1200,7 +1205,7 @@ fn reject_bip30_unspent_overwrite( .store() .get_fk_by_txid_batch(&create_txids) .map_err(ConfirmError::from)?; - for (_txid, row) in hits { + for (txid, row) in hits { let Some((old_fk, _)) = row else { continue; }; @@ -1224,10 +1229,12 @@ fn reject_bip30_unspent_overwrite( if rbitcoin_consensus::policy::is_unspendable(&out.script) { continue; } - let spent = query - .store() - .has_confirmed_strong_spender_create(old_fk, v, None) - .map_err(ConfirmError::from)?; + // A spend earlier in this batch is not connected yet either. + let spent = pending_spent.contains(&(txid, v)) + || query + .store() + .has_confirmed_strong_spender_create(old_fk, v, None) + .map_err(ConfirmError::from)?; if !spent { unspent = true; break; @@ -1241,7 +1248,11 @@ fn reject_bip30_unspent_overwrite( )?; } } + if !txids_can_repeat { + return Ok(()); + } for (txid, tx) in create_txids.into_iter().zip(&block.txdata) { + release_repeated_outputs(pending_spent, txid, tx.output.len()); let lo = scratch.batch_vouts.len() as u32; scratch.batch_vouts.extend( (0u32..) @@ -1257,6 +1268,19 @@ fn reject_bip30_unspent_overwrite( Ok(()) } +/// Below BIP34 a txid can repeat once its first copy is spent. Its outputs are +/// coins again (Core overwrites them in the view), so this batch's earlier +/// spends of the old copy no longer block spending the new one. +fn release_repeated_outputs( + pending_spent: &mut rbitcoin_consensus::OutPointSet, + txid: [u8; 32], + n_out: usize, +) { + for v in 0..n_out as u32 { + pending_spent.remove(&(txid, v)); + } +} + fn bip34_ancestry_skips_bip30( query: &Query, ctx: &rbitcoin_consensus::ValidationContext<'_>, diff --git a/crates/rbitcoin-test/tests/consensus_rules.rs b/crates/rbitcoin-test/tests/consensus_rules.rs index 3d95f1ec9..f7cf288c4 100644 --- a/crates/rbitcoin-test/tests/consensus_rules.rs +++ b/crates/rbitcoin-test/tests/consensus_rules.rs @@ -753,6 +753,132 @@ fn bip30_duplicate_coinbase_in_one_batch() { assert_eq!(q.tip_height(), Some(Height(1))); } +/// Below BIP34 a coinbase can repeat once its first copy is spent. Core 31.1 +/// accepts `repeated_coinbase_chain(203, 48 BTC, true)`: h1 creates X, h102 +/// spends X:0, h103 creates X again, h203 spends the recreated X:0. +type Chain = Vec<(Height, bitcoin::Block)>; + +fn repeated_coinbase_chain( + second_spend_height: u32, + second_spend_sat: u64, + recreate: bool, +) -> Chain { + let op_true = bitcoin::ScriptBuf::from_bytes(vec![0x51]); + let g = regtest_genesis(); + let mut run: Chain = Vec::new(); + let mut x: Option = None; + for h in 1..=second_spend_height.max(103) { + let (prev, time) = run + .last() + .map(|(_, b)| (b.block_hash(), b.header.time + 600)) + .unwrap_or((g.block_hash(), g.header.time + 600)); + let spend = |sat: u64| { + let x = x.as_ref().expect("X before its spends"); + spend_anyone_can_spend(x.compute_txid(), 0, Amount::from_sat(sat)) + }; + let txs = match h { + 102 => vec![spend(49_0000_0000)], + h if h == second_spend_height => vec![spend(second_spend_sat)], + _ => vec![], + }; + let mut b = rbitcoin_consensus::mine_regtest_paying(prev, time, h, op_true.clone(), txs); + match (h, &x) { + (1, _) => x = Some(b.txdata[0].clone()), + (103, Some(first)) if recreate => { + b.txdata[0] = first.clone(); + b.header.merkle_root = b.compute_merkle_root().unwrap(); + rbitcoin_consensus::grind_regtest_pow(&mut b.header); + } + _ => {} + } + run.push((Height(h), b)); + } + run +} + +const BATCH_CUTS: [&[usize]; 9] = [ + &[101, 102, 103, 149, 202], + &[], + &[101], + &[102], + &[54], + &[102, 150], + &[103], + &[101, 103], + &[54, 103], +]; + +/// Connect `chain` split into confirm batches at `cuts`; the first error ends it. +fn connect_in_batches( + chain: &[(Height, bitcoin::Block)], + cuts: &[usize], +) -> Result, String> { + let (_td, q, mut params) = regtest_q(); + params.apply_test_activation_height("bip34", 1_000).unwrap(); + connect_genesis(&q, ¶ms); + let mut lo = 0usize; + for hi in cuts + .iter() + .copied() + .filter(|&c| c < chain.len()) + .chain([chain.len()]) + { + confirm_wire_run(&q, ¶ms, Milestone::NONE, &chain[lo..hi]) + .map_err(|e| format!("heights {}..={}: {e:?}", lo + 1, hi))?; + lo = hi; + } + Ok(q.tip_height()) +} + +/// The Core-valid chain above connects however IBD splits it into batches. +#[test] +fn repeated_coinbase_chain_connects_in_any_batch_split() { + let chain = repeated_coinbase_chain(203, 48_0000_0000, true); + for cuts in BATCH_CUTS { + assert_eq!( + connect_in_batches(&chain, cuts), + Ok(Some(Height(203))), + "cuts {cuts:?}" + ); + } +} + +/// What Core rejects around a repeated coinbase stays rejected in any split. +#[test] +fn repeated_coinbase_chain_rejects_what_core_rejects() { + let cases: [(&str, Chain, &[&str]); 3] = [ + ( + "h203 repeats h102's spend while its output is unspent", + repeated_coinbase_chain(203, 49_0000_0000, true), + &["BIP30"], + ), + ( + "h203 spends X:0 again without a new X", + repeated_coinbase_chain(203, 48_0000_0000, false), + &[ + "PrevoutSpent", + "double spend", + "MissingPrevout", + "missingorspent", + ], + ), + ( + "h150 spends the new X before it matures", + repeated_coinbase_chain(150, 48_0000_0000, true), + &["immature"], + ), + ]; + for (what, chain, want) in &cases { + for cuts in BATCH_CUTS { + let got = connect_in_batches(chain, cuts); + assert!( + matches!(&got, Err(e) if want.iter().any(|w| e.contains(w))), + "{what}, cuts {cuts:?}: got {got:?}, want one of {want:?}" + ); + } + } +} + // ─── 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 44c02b121..b42b3be1b 100644 --- a/docs/consensus-tests.md +++ b/docs/consensus-tests.md @@ -144,7 +144,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`, `bip30_duplicate_coinbase_in_one_batch` | +| 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`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects` | | 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 | From ca552a81c6feb00eb46a6f3d26de31d5212c5f76 Mon Sep 17 00:00:00 2001 From: Nitesh Balusu Date: Sat, 17 Oct 2026 22:44:01 -0400 Subject: [PATCH 2/5] consensus: track repeated-txid spends per copy above BIP34 too A pre-BIP34 coinbase that pushed a later height can repeat after BIP34. The assemble release now runs for every create, and the write stage keys its spent set by outpoint and the copy spent, so a spend of the new copy is not taken for a double spend of the old one. Adds a BIP34-era batch-split journey and a merged-write journey that drives the IBD append_contiguous path. Co-Authored-By: Claude Opus 5.5 --- changelog.d/repeated-txid-in-batch.md | 11 +- crates/rbitcoin-query/src/block_pipeline.rs | 58 +++--- .../rbitcoin-query/src/confirm_run/phases.rs | 2 +- .../src/confirm_run/write_idempotent_tests.rs | 10 +- .../src/structure_rule_tests.rs | 12 +- crates/rbitcoin-test/tests/consensus_rules.rs | 175 +++++++++++++++++- docs/consensus-tests.md | 2 +- 7 files changed, 222 insertions(+), 48 deletions(-) diff --git a/changelog.d/repeated-txid-in-batch.md b/changelog.d/repeated-txid-in-batch.md index db6ed8a89..654b8bf25 100644 --- a/changelog.d/repeated-txid-in-batch.md +++ b/changelog.d/repeated-txid-in-batch.md @@ -1,9 +1,8 @@ Fixed - **A txid that repeats inside one confirm batch is handled like Core.** - Below BIP34 a coinbase can repeat once its first copy is spent. A spend now - uses the copy that exists at its height, the outputs of a recreated - transaction can be spent again, and BIP30 counts spends made earlier in the - same batch. Before, IBD could halt on such a chain that Core accepts. Only - reachable below BIP34 (for example regtest with - `-testactivationheight=bip34@N`). + A coinbase can repeat once its first copy is spent: below BIP34, or after + BIP34 when an old coinbase already pushed the later height. A spend now uses + the copy that exists at its height, the outputs of a recreated transaction + can be spent again, and BIP30 counts spends made earlier in the same batch. + Before, IBD could halt on such a chain that Core accepts. diff --git a/crates/rbitcoin-query/src/block_pipeline.rs b/crates/rbitcoin-query/src/block_pipeline.rs index b941af941..d6aa2292d 100644 --- a/crates/rbitcoin-query/src/block_pipeline.rs +++ b/crates/rbitcoin-query/src/block_pipeline.rs @@ -193,8 +193,6 @@ pub fn assemble_block_prevouts( let t_loop = Instant::now(); let lock_time_cutoff = assemble_lock_time_cutoff(ctx, block, prev_mtp); - let txids_can_repeat = !ctx.params.bip34_active_at(ctx.height.0); - for (ti, tx) in block.txdata.iter().enumerate() { let spend_fk = archived_tx_fks.map(|fks| fks[ti]); let txid = create_txids[ti]; @@ -232,9 +230,7 @@ pub fn assemble_block_prevouts( if !create_fk.is_null() { pending_creates.insert(txid, create_fk); } - if txids_can_repeat { - release_repeated_outputs(pending_spent, txid, tx.output.len()); - } + release_repeated_outputs(pending_spent, txid, tx.output.len()); } let clk_prev = t_loop.elapsed().as_nanos() as u64; @@ -545,9 +541,9 @@ 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)>, + /// BIP30 inside one batch: each txid already created here, with its height, + /// its fk, and the span of `batch_vouts` holding its spendable outputs. + batch_creates: TxidMap<(u32, rbitcoin_primitives::Fk, u32, u32)>, batch_vouts: Vec, pub slots: AnnotateSlots, } @@ -621,7 +617,7 @@ pub fn structural_validate_spends( u32, )], fees: i64, - pending_spent: &mut rbitcoin_consensus::OutPointSet, + pending_spent: &mut BatchSpent, batch_parents: &crate::BatchParents, mtp_cache: &mut U32Map, run_create_height: &RunCreateHeight, @@ -635,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, pending_spent, scratch)?; + reject_bip30_unspent_overwrite(query, block, ctx, archived_tx_fks, 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); @@ -1012,7 +1008,7 @@ fn structural_mark_pending( rbitcoin_primitives::Fk, u32, )], - pending_spent: &mut rbitcoin_consensus::OutPointSet, + pending_spent: &mut BatchSpent, durable_spent: &DurableSpentSet, ) -> Result<(), ConfirmError> { for &(prev_txid, vout, _spend_fk, create_fk, _vin) in spends { @@ -1024,7 +1020,10 @@ fn structural_mark_pending( } else { false }; - rbitcoin_consensus::block::reject_prevout_spent(spent || !pending_spent.insert(key))?; + let again = pending_spent + .insert(key, create_fk) + .is_some_and(|prev| prev == create_fk); + rbitcoin_consensus::block::reject_prevout_spent(spent || again)?; } Ok(()) } @@ -1176,7 +1175,8 @@ fn reject_bip30_unspent_overwrite( query: &Query, block: &Block, ctx: &rbitcoin_consensus::ValidationContext<'_>, - pending_spent: &mut rbitcoin_consensus::OutPointSet, + create_fks: Option<&[rbitcoin_primitives::Fk]>, + pending_spent: &BatchSpent, scratch: &mut StructuralScratch, ) -> Result<(), ConfirmError> { if ctx.params.is_bip30_repeat(ctx.height.0, block.block_hash()) @@ -1193,12 +1193,12 @@ fn reject_bip30_unspent_overwrite( // 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 { + let Some(&(prior_height, prior_fk, 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))); + .any(|&v| pending_spent.get(&(*txid, v)) != Some(&prior_fk)); rbitcoin_consensus::block::reject_bip30_unspent_prior(ctx.height.0, prior_height, unspent)?; } let hits = query @@ -1230,7 +1230,7 @@ fn reject_bip30_unspent_overwrite( continue; } // A spend earlier in this batch is not connected yet either. - let spent = pending_spent.contains(&(txid, v)) + let spent = pending_spent.get(&(txid, v)) == Some(&old_fk) || query .store() .has_confirmed_strong_spender_create(old_fk, v, None) @@ -1251,8 +1251,10 @@ fn reject_bip30_unspent_overwrite( if !txids_can_repeat { return Ok(()); } - for (txid, tx) in create_txids.into_iter().zip(&block.txdata) { - release_repeated_outputs(pending_spent, txid, tx.output.len()); + for (i, (txid, tx)) in create_txids.into_iter().zip(&block.txdata).enumerate() { + let fk = create_fks + .and_then(|fks| fks.get(i).copied()) + .unwrap_or(rbitcoin_primitives::Fk::NULL); let lo = scratch.batch_vouts.len() as u32; scratch.batch_vouts.extend( (0u32..) @@ -1263,14 +1265,26 @@ fn reject_bip30_unspent_overwrite( .map(|(v, _)| v), ); let hi = scratch.batch_vouts.len() as u32; - scratch.batch_creates.insert(txid, (ctx.height.0, lo, hi)); + scratch + .batch_creates + .insert(txid, (ctx.height.0, fk, lo, hi)); } Ok(()) } -/// Below BIP34 a txid can repeat once its first copy is spent. Its outputs are -/// coins again (Core overwrites them in the view), so this batch's earlier -/// spends of the old copy no longer block spending the new one. +/// Outpoints spent so far in one write batch, each with the fk of the copy the +/// spend used. A txid can be created again once its first copy is spent (below +/// BIP34, or a pre-BIP34 coinbase whose scriptSig already pushes a later height), +/// so a spend of the new copy is not a second spend of the old one. +pub(crate) type BatchSpent = std::collections::HashMap< + ([u8; 32], u32), + rbitcoin_primitives::Fk, + BuildHasherDefault, +>; + +/// A txid can be created again once its first copy is spent (see [`BatchSpent`]). +/// Its outputs are coins again (Core overwrites them in the view), so this +/// load batch's earlier spends of the old copy no longer block the new one. fn release_repeated_outputs( pending_spent: &mut rbitcoin_consensus::OutPointSet, txid: [u8; 32], diff --git a/crates/rbitcoin-query/src/confirm_run/phases.rs b/crates/rbitcoin-query/src/confirm_run/phases.rs index 8c5ee2f82..8f865ef63 100644 --- a/crates/rbitcoin-query/src/confirm_run/phases.rs +++ b/crates/rbitcoin-query/src/confirm_run/phases.rs @@ -273,7 +273,7 @@ pub(super) fn assemble_run( #[derive(Default)] pub(super) struct StructuralReuse { pub scratch: crate::block_pipeline::StructuralScratch, - pub pending: crate::OutPointSet, + pub pending: crate::block_pipeline::BatchSpent, } /// Durable spentness + maturity + subsidy after scripts (height order). diff --git a/crates/rbitcoin-query/src/confirm_run/write_idempotent_tests.rs b/crates/rbitcoin-query/src/confirm_run/write_idempotent_tests.rs index d3dffdb26..780f1f13d 100644 --- a/crates/rbitcoin-query/src/confirm_run/write_idempotent_tests.rs +++ b/crates/rbitcoin-query/src/confirm_run/write_idempotent_tests.rs @@ -1339,7 +1339,7 @@ fn fill_same_batch_abs_from_append_loc_ram() { #[test] fn structural_same_batch_overlay_skips_meta_pread() { use crate::block_pipeline::{structural_validate_spends, RunCreateHeight}; - use crate::{BatchParents, FkMap, OutPointSet}; + use crate::{BatchParents, FkMap}; use bitcoin::absolute::LockTime; use bitcoin::block::{Header, Version}; use bitcoin::hashes::Hash; @@ -1459,7 +1459,7 @@ fn structural_same_batch_overlay_skips_meta_pread() { let run = RunCreateHeight::Map(map); let params = ChainParams::regtest(); let ctx = rbitcoin_consensus::block::ValidationContext::at(¶ms, Height(1), Milestone::NONE); - let mut pending = OutPointSet::default(); + let mut pending = crate::block_pipeline::BatchSpent::default(); let mut mtp = crate::U32Map::::default(); mtp.insert(0, 1_300_000_000); let mut scratch = crate::block_pipeline::StructuralScratch::default(); @@ -1515,7 +1515,7 @@ fn structural_same_batch_overlay_skips_meta_pread() { #[test] fn structural_scratch_second_block_does_not_replay_first_slots() { use crate::block_pipeline::{structural_validate_spends, RunCreateHeight, StructuralScratch}; - use crate::{BatchParents, OutPointSet}; + use crate::BatchParents; use bitcoin::absolute::LockTime; use bitcoin::block::{Header, Version}; use bitcoin::hashes::Hash; @@ -1609,7 +1609,7 @@ fn structural_scratch_second_block_does_not_replay_first_slots() { block.header.merkle_root = block.compute_merkle_root().unwrap(); let params = ChainParams::regtest(); let ctx = rbitcoin_consensus::block::ValidationContext::at(¶ms, Height(1), Milestone::NONE); - let mut pending = OutPointSet::default(); + let mut pending = crate::block_pipeline::BatchSpent::default(); let mut mtp = crate::U32Map::::default(); mtp.insert(0, 1_300_000_000); let mut scratch = StructuralScratch::default(); @@ -3321,7 +3321,7 @@ fn structural_run_clears_reused_scratch_between_batches() { .scratch .slots .push((999, Fk(1), 0, Fk(8), 0), (Fk::NULL, 0, 0)); - reuse.pending.insert(([0x41; 32], 0u32)); + reuse.pending.insert(([0x41; 32], 0u32), fks[0]); let run = |reuse: &mut StructuralReuse| { structural_run( diff --git a/crates/rbitcoin-query/src/structure_rule_tests.rs b/crates/rbitcoin-query/src/structure_rule_tests.rs index a00e67e51..94eadd8f5 100644 --- a/crates/rbitcoin-query/src/structure_rule_tests.rs +++ b/crates/rbitcoin-query/src/structure_rule_tests.rs @@ -362,7 +362,7 @@ fn padded_spend(data_len: usize) -> Transaction { /// 91842/91880 grandfather). Signet activates BIP34 at height 1, and Core's /// empty BIP34 hash still enforces BIP30 on every signet block. fn rejects_unspent_overwrite(q: &crate::Query, first: &Transaction) { - use crate::{BatchParents, OutPointSet, U32Map}; + use crate::{BatchParents, U32Map}; use rbitcoin_primitives::Fk; let dup = block_with(vec![first.clone()]); let signet = Box::leak(Box::new(ChainParams::signet())); @@ -385,7 +385,7 @@ fn rejects_unspent_overwrite(q: &crate::Query, first: &Transaction) { Some(&[Fk(2)]), &[], 0, - &mut OutPointSet::default(), + &mut crate::block_pipeline::BatchSpent::default(), &parents, &mut U32Map::default(), &heights, @@ -481,7 +481,7 @@ fn bip30_message_at_mainnet_above_bip34( q: &crate::Query, block: &Block, ) -> Result<(), ConfirmError> { - use crate::{BatchParents, OutPointSet, U32Map}; + use crate::{BatchParents, U32Map}; use rbitcoin_primitives::Fk; let main = Box::leak(Box::new(ChainParams::mainnet())); let ctx = ValidationContext::at(main, Height(main.btc.bip34_height + 1), Milestone::NONE); @@ -497,7 +497,7 @@ fn bip30_message_at_mainnet_above_bip34( Some(&[Fk(2)]), &[], 0, - &mut OutPointSet::default(), + &mut crate::block_pipeline::BatchSpent::default(), &parents, &mut U32Map::default(), &heights, @@ -2327,7 +2327,7 @@ fn bip30_and_immature_coinbase_reject_from_facts() { /// overwrite. A spendable unspent output still does. #[test] fn bip30_ignores_unspendable_outputs() { - use crate::{BatchParents, OutPointSet, U32Map}; + use crate::{BatchParents, U32Map}; let outputs = vec![ TxOut { value: Amount::from_sat(50_0000_0000), @@ -2363,7 +2363,7 @@ fn bip30_ignores_unspendable_outputs() { Some(&[rbitcoin_primitives::Fk(3)]), &[], 0, - &mut OutPointSet::default(), + &mut crate::block_pipeline::BatchSpent::default(), &parents, &mut mtp_cache, &RunCreateHeight::Spans(Vec::new()), diff --git a/crates/rbitcoin-test/tests/consensus_rules.rs b/crates/rbitcoin-test/tests/consensus_rules.rs index f7cf288c4..08213fb1a 100644 --- a/crates/rbitcoin-test/tests/consensus_rules.rs +++ b/crates/rbitcoin-test/tests/consensus_rules.rs @@ -812,9 +812,19 @@ const BATCH_CUTS: [&[usize]; 9] = [ fn connect_in_batches( chain: &[(Height, bitcoin::Block)], cuts: &[usize], +) -> Result, String> { + connect_in_batches_bip34_at(chain, cuts, 1_000) +} + +fn connect_in_batches_bip34_at( + chain: &[(Height, bitcoin::Block)], + cuts: &[usize], + bip34_height: u32, ) -> Result, String> { let (_td, q, mut params) = regtest_q(); - params.apply_test_activation_height("bip34", 1_000).unwrap(); + params + .apply_test_activation_height("bip34", bip34_height) + .unwrap(); connect_genesis(&q, ¶ms); let mut lo = 0usize; for hi in cuts @@ -830,6 +840,160 @@ fn connect_in_batches( Ok(q.tip_height()) } +/// After BIP34 a coinbase can still repeat an older one whose scriptSig +/// already starts with the later height (Core's 490,897 / 1,983,702 case). +/// With BIP34 at 100, Core 31.1 accepts: h1 creates X with scriptSig +/// push(150), h149 spends X:0, h150 creates X again, h250 spends the new X:0. +fn bip34_era_repeat_chain() -> Chain { + let op_true = bitcoin::ScriptBuf::from_bytes(vec![0x51]); + let x = + rbitcoin_consensus::regtest_coinbase(150, Amount::from_sat(25_0000_0000), op_true.clone()); + let g = regtest_genesis(); + let mut run: Chain = Vec::new(); + for h in 1..=250u32 { + let (prev, time) = run + .last() + .map(|(_, b)| (b.block_hash(), b.header.time + 600)) + .unwrap_or((g.block_hash(), g.header.time + 600)); + let txs = match h { + 149 => vec![spend_anyone_can_spend( + x.compute_txid(), + 0, + Amount::from_sat(24_0000_0000), + )], + 250 => vec![spend_anyone_can_spend( + x.compute_txid(), + 0, + Amount::from_sat(23_0000_0000), + )], + _ => vec![], + }; + let mut b = rbitcoin_consensus::mine_regtest_paying(prev, time, h, op_true.clone(), txs); + if h == 1 || h == 150 { + b.txdata[0] = x.clone(); + b.header.merkle_root = b.compute_merkle_root().unwrap(); + rbitcoin_consensus::grind_regtest_pow(&mut b.header); + } + run.push((Height(h), b)); + } + run +} + +#[test] +fn bip34_era_repeat_connects_in_any_batch_split() { + let chain = bip34_era_repeat_chain(); + for cuts in [ + &[][..], + &[148], + &[149], + &[148, 150], + &[100, 200], + &[148, 149, 150, 249], + ] { + assert_eq!( + connect_in_batches_bip34_at(&chain, cuts, 100), + Ok(Some(Height(250))), + "cuts {cuts:?}" + ); + } +} + +/// IBD's write path: load each slice ahead of the write (in-flight parents, +/// reserved fks), run scripts, merge the slices into one write batch, write once. +fn connect_merged( + chain: &[(Height, bitcoin::Block)], + load_cuts: &[usize], + bip34_height: u32, +) -> Result, String> { + use rbitcoin_query::{ + confirm_scripts_phase, confirm_wire_load_phase_pipelined, confirm_write_phase, InFlight, + ScriptPreverified, WireLoadPipeline, + }; + let (_td, q, mut params) = regtest_q(); + params + .apply_test_activation_height("bip34", bip34_height) + .unwrap(); + connect_genesis(&q, ¶ms); + q.enter_direct_index_mode().unwrap(); + let none = ScriptPreverified::new(); + let mut inflight = InFlight::new(); + let mut next_tx_start = q.tx_body_count().saturating_add(1).max(1); + let mut merged: Option = None; + let mut lo = 0usize; + for hi in load_cuts + .iter() + .copied() + .filter(|&c| c < chain.len()) + .chain([chain.len()]) + { + let slice = &chain[lo..hi]; + let pipe = WireLoadPipeline { + path_lo: slice[0].0 .0, + parent_hash: lo + .checked_sub(1) + .map(|i| chain[i].1.block_hash().to_byte_array()), + next_tx_start, + in_flight: &inflight, + skeleton: None, + carried_need: Vec::new(), + carried_header_fks: Vec::new(), + carried_header_hashes: Vec::new(), + }; + let mat = confirm_wire_load_phase_pipelined( + &q, + ¶ms, + Milestone::NONE, + slice, + &none, + Some(&pipe), + ) + .map_err(|e| format!("load {}..={hi}: {e:?}", lo + 1))?; + let plan = mat.batch.archive_plan.as_ref().expect("wire plan"); + inflight.note_pins( + plan.planned_fks.iter().copied().zip(plan.batch_pin.iter()), + Some(slice[slice.len() - 1].0 .0), + ); + if let Some(last) = plan.planned_fks.last().and_then(|f| f.get()) { + next_tx_start = last + 1; + } + let ok = confirm_scripts_phase(mat.batch) + .map_err(|e| format!("scripts {}..={hi}: {e:?}", lo + 1))?; + merged = Some(match merged.take() { + None => ok.batch, + Some(mut m) => { + if m.append_contiguous(ok.batch).is_err() { + return Err(format!("merge at {}", lo + 1)); + } + m + } + }); + lo = hi; + } + confirm_write_phase(&q, ¶ms, Milestone::NONE, merged.expect("one slice")) + .map_err(|e| format!("write: {e:?}"))?; + Ok(q.tip_height()) +} + +#[test] +fn repeated_coinbase_connects_when_ibd_merges_load_batches() { + let pre = repeated_coinbase_chain(203, 48_0000_0000, true); + for cuts in [&[102][..], &[101, 103], &[54, 103, 150]] { + assert_eq!( + connect_merged(&pre, cuts, 1_000), + Ok(Some(Height(203))), + "pre-BIP34, load cuts {cuts:?}" + ); + } + let era = bip34_era_repeat_chain(); + for cuts in [&[149][..], &[100, 149, 200], &[148, 150]] { + assert_eq!( + connect_merged(&era, cuts, 100), + Ok(Some(Height(250))), + "BIP34 era, load cuts {cuts:?}" + ); + } +} + /// The Core-valid chain above connects however IBD splits it into batches. #[test] fn repeated_coinbase_chain_connects_in_any_batch_split() { @@ -855,12 +1019,9 @@ fn repeated_coinbase_chain_rejects_what_core_rejects() { ( "h203 spends X:0 again without a new X", repeated_coinbase_chain(203, 48_0000_0000, false), - &[ - "PrevoutSpent", - "double spend", - "MissingPrevout", - "missingorspent", - ], + // Only spent errors: MissingPrevout would also match a spend bound + // to a later copy, which is the bug this guards against. + &["PrevoutSpent", "double spend"], ), ( "h150 spends the new X before it matures", diff --git a/docs/consensus-tests.md b/docs/consensus-tests.md index b42b3be1b..ceb4eb5dd 100644 --- a/docs/consensus-tests.md +++ b/docs/consensus-tests.md @@ -144,7 +144,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`, `bip30_duplicate_coinbase_in_one_batch`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects` | +| 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`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects`, `bip34_era_repeat_connects_in_any_batch_split`, `repeated_coinbase_connects_when_ibd_merges_load_batches` | | 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 | From 17cf96a1e3f962d506e4de19c1fcb35daa195409 Mon Sep 17 00:00:00 2001 From: Nitesh Balusu Date: Sat, 17 Oct 2026 22:50:44 -0400 Subject: [PATCH 3/5] consensus: record every create for same-batch BIP30, not just below BIP34 cc22910b stopped recording creates for the in-batch BIP30 check above BIP34. A spend tx made after BIP34 can still repeat byte for byte once a pre-BIP34 coinbase that pushed a later height is mined again. Core rejects the repeat while the first copy's output is unspent; in one batch we accepted it. Master recorded every create; this restores that. Co-Authored-By: Claude Opus 5.5 --- crates/rbitcoin-query/src/block_pipeline.rs | 4 --- crates/rbitcoin-test/tests/consensus_rules.rs | 29 ++++++++++++++++--- docs/consensus-tests.md | 2 +- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/crates/rbitcoin-query/src/block_pipeline.rs b/crates/rbitcoin-query/src/block_pipeline.rs index d6aa2292d..e1c2d8340 100644 --- a/crates/rbitcoin-query/src/block_pipeline.rs +++ b/crates/rbitcoin-query/src/block_pipeline.rs @@ -1189,7 +1189,6 @@ fn reject_bip30_unspent_overwrite( .iter() .map(|tx| tx.compute_txid().to_byte_array()) .collect(); - let txids_can_repeat = !ctx.params.bip34_active_at(ctx.height.0); // An earlier block of this batch is not connected yet, so the store // lookup below cannot see its creates. for txid in &create_txids { @@ -1248,9 +1247,6 @@ fn reject_bip30_unspent_overwrite( )?; } } - if !txids_can_repeat { - return Ok(()); - } for (i, (txid, tx)) in create_txids.into_iter().zip(&block.txdata).enumerate() { let fk = create_fks .and_then(|fks| fks.get(i).copied()) diff --git a/crates/rbitcoin-test/tests/consensus_rules.rs b/crates/rbitcoin-test/tests/consensus_rules.rs index 08213fb1a..0621ddfc1 100644 --- a/crates/rbitcoin-test/tests/consensus_rules.rs +++ b/crates/rbitcoin-test/tests/consensus_rules.rs @@ -844,7 +844,7 @@ fn connect_in_batches_bip34_at( /// already starts with the later height (Core's 490,897 / 1,983,702 case). /// With BIP34 at 100, Core 31.1 accepts: h1 creates X with scriptSig /// push(150), h149 spends X:0, h150 creates X again, h250 spends the new X:0. -fn bip34_era_repeat_chain() -> Chain { +fn bip34_era_repeat_chain(second_spend_sat: u64) -> Chain { let op_true = bitcoin::ScriptBuf::from_bytes(vec![0x51]); let x = rbitcoin_consensus::regtest_coinbase(150, Amount::from_sat(25_0000_0000), op_true.clone()); @@ -864,7 +864,7 @@ fn bip34_era_repeat_chain() -> Chain { 250 => vec![spend_anyone_can_spend( x.compute_txid(), 0, - Amount::from_sat(23_0000_0000), + Amount::from_sat(second_spend_sat), )], _ => vec![], }; @@ -881,7 +881,7 @@ fn bip34_era_repeat_chain() -> Chain { #[test] fn bip34_era_repeat_connects_in_any_batch_split() { - let chain = bip34_era_repeat_chain(); + let chain = bip34_era_repeat_chain(23_0000_0000); for cuts in [ &[][..], &[148], @@ -898,6 +898,27 @@ fn bip34_era_repeat_connects_in_any_batch_split() { } } +/// Core 31.1 rejects h250 with `bad-txns-BIP30` when it repeats h149's spend +/// byte for byte: h149's output is still unspent. +#[test] +fn bip34_era_repeated_spend_rejects_bip30_in_any_batch_split() { + let chain = bip34_era_repeat_chain(24_0000_0000); + for cuts in [&[][..], &[148], &[149], &[149, 150], &[100, 200]] { + let got = connect_in_batches_bip34_at(&chain, cuts, 100); + assert!( + matches!(&got, Err(e) if e.contains("BIP30")), + "cuts {cuts:?}: got {got:?}" + ); + } + for cuts in [&[149][..], &[100, 149, 200]] { + let got = connect_merged(&chain, cuts, 100); + assert!( + matches!(&got, Err(e) if e.contains("BIP30")), + "merged, cuts {cuts:?}: got {got:?}" + ); + } +} + /// IBD's write path: load each slice ahead of the write (in-flight parents, /// reserved fks), run scripts, merge the slices into one write batch, write once. fn connect_merged( @@ -984,7 +1005,7 @@ fn repeated_coinbase_connects_when_ibd_merges_load_batches() { "pre-BIP34, load cuts {cuts:?}" ); } - let era = bip34_era_repeat_chain(); + let era = bip34_era_repeat_chain(23_0000_0000); for cuts in [&[149][..], &[100, 149, 200], &[148, 150]] { assert_eq!( connect_merged(&era, cuts, 100), diff --git a/docs/consensus-tests.md b/docs/consensus-tests.md index ceb4eb5dd..1b6a8ba9e 100644 --- a/docs/consensus-tests.md +++ b/docs/consensus-tests.md @@ -144,7 +144,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`, `bip30_duplicate_coinbase_in_one_batch`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects`, `bip34_era_repeat_connects_in_any_batch_split`, `repeated_coinbase_connects_when_ibd_merges_load_batches` | +| 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`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects`, `bip34_era_repeat_connects_in_any_batch_split`, `bip34_era_repeated_spend_rejects_bip30_in_any_batch_split`, `repeated_coinbase_connects_when_ibd_merges_load_batches` | | 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 | From 4d328799d60d53f1ed3a1c264ff9eab3704e4ac1 Mon Sep 17 00:00:00 2001 From: Nitesh Balusu Date: Sat, 17 Oct 2026 23:27:51 -0400 Subject: [PATCH 4/5] consensus: identify a spent copy by its height in the write batch ca552a81 keyed write-batch spends by create fk. A same-block spend has no create fk (NULL), so a later spend of that coin in a merged write was not a double spend, and BIP30 saw a copy spent in its own block as unspent. The connected-copy BIP30 check also missed a spend that a later copy's spend replaced. A same-block spend now records its spender's fk. Two spends hit the same copy when their fks match or have the same height in the batch (one block holds one copy of a txid). In-batch BIP30 compares that height with the earlier copy's; the connected-copy check counts any spend in the batch. Co-Authored-By: Claude Opus 5.5 --- crates/rbitcoin-query/src/block_pipeline.rs | 77 ++++++--- crates/rbitcoin-test/tests/consensus_rules.rs | 155 ++++++++++++++++-- docs/consensus-tests.md | 2 +- 3 files changed, 191 insertions(+), 43 deletions(-) diff --git a/crates/rbitcoin-query/src/block_pipeline.rs b/crates/rbitcoin-query/src/block_pipeline.rs index e1c2d8340..4107c6896 100644 --- a/crates/rbitcoin-query/src/block_pipeline.rs +++ b/crates/rbitcoin-query/src/block_pipeline.rs @@ -193,6 +193,7 @@ pub fn assemble_block_prevouts( let t_loop = Instant::now(); let lock_time_cutoff = assemble_lock_time_cutoff(ctx, block, prev_mtp); + for (ti, tx) in block.txdata.iter().enumerate() { let spend_fk = archived_tx_fks.map(|fks| fks[ti]); let txid = create_txids[ti]; @@ -541,9 +542,9 @@ 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, - /// its fk, and the span of `batch_vouts` holding its spendable outputs. - batch_creates: TxidMap<(u32, rbitcoin_primitives::Fk, u32, u32)>, + /// 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, } @@ -631,7 +632,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, archived_tx_fks, pending_spent, scratch)?; + reject_bip30_unspent_overwrite(query, block, ctx, pending_spent, run_create_height, 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); @@ -646,7 +647,12 @@ pub fn structural_validate_spends( let spent_abs_ns = (t_abs.elapsed().as_nanos() as u64).saturating_sub(spent_strong_ns); let spent_cold_ns = multi_list_ns; let t_pending = Instant::now(); - structural_mark_pending(spends, pending_spent, &scratch.durable_spent)?; + structural_mark_pending( + spends, + pending_spent, + run_create_height, + &scratch.durable_spent, + )?; let spent_pending_ns = t_pending.elapsed().as_nanos() as u64; let spent_ns = t_spent.elapsed().as_nanos() as u64; let t_create = Instant::now(); @@ -1009,9 +1015,10 @@ fn structural_mark_pending( u32, )], pending_spent: &mut BatchSpent, + run_create_height: &RunCreateHeight, durable_spent: &DurableSpentSet, ) -> Result<(), ConfirmError> { - for &(prev_txid, vout, _spend_fk, create_fk, _vin) in spends { + for &(prev_txid, vout, spend_fk, create_fk, _vin) in spends { let key = (prev_txid, vout); let spent = if create_fk.is_null() { false @@ -1020,9 +1027,16 @@ fn structural_mark_pending( } else { false }; + // A same-block create has no fk here. Its spender's fk has the same + // height in this batch, which is what identifies the copy. + let copy = if create_fk.is_null() { + spend_fk + } else { + create_fk + }; let again = pending_spent - .insert(key, create_fk) - .is_some_and(|prev| prev == create_fk); + .insert(key, copy) + .is_some_and(|prev| same_copy(run_create_height, prev, copy)); rbitcoin_consensus::block::reject_prevout_spent(spent || again)?; } Ok(()) @@ -1175,8 +1189,8 @@ fn reject_bip30_unspent_overwrite( query: &Query, block: &Block, ctx: &rbitcoin_consensus::ValidationContext<'_>, - create_fks: Option<&[rbitcoin_primitives::Fk]>, pending_spent: &BatchSpent, + run_create_height: &RunCreateHeight, scratch: &mut StructuralScratch, ) -> Result<(), ConfirmError> { if ctx.params.is_bip30_repeat(ctx.height.0, block.block_hash()) @@ -1192,12 +1206,17 @@ fn reject_bip30_unspent_overwrite( // 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, prior_fk, lo, hi)) = scratch.batch_creates.get(txid) else { + 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.get(&(*txid, v)) != Some(&prior_fk)); + .any(|&v| { + pending_spent + .get(&(*txid, v)) + .and_then(|©| run_create_height.get(copy)) + != Some(prior_height) + }); rbitcoin_consensus::block::reject_bip30_unspent_prior(ctx.height.0, prior_height, unspent)?; } let hits = query @@ -1228,8 +1247,9 @@ fn reject_bip30_unspent_overwrite( if rbitcoin_consensus::policy::is_unspendable(&out.script) { continue; } - // A spend earlier in this batch is not connected yet either. - let spent = pending_spent.get(&(txid, v)) == Some(&old_fk) + // A spend earlier in this batch is not connected yet either. It + // spent this copy, or a later one that needed this copy spent. + let spent = pending_spent.contains_key(&(txid, v)) || query .store() .has_confirmed_strong_spender_create(old_fk, v, None) @@ -1247,10 +1267,7 @@ fn reject_bip30_unspent_overwrite( )?; } } - for (i, (txid, tx)) in create_txids.into_iter().zip(&block.txdata).enumerate() { - let fk = create_fks - .and_then(|fks| fks.get(i).copied()) - .unwrap_or(rbitcoin_primitives::Fk::NULL); + for (txid, tx) in create_txids.into_iter().zip(&block.txdata) { let lo = scratch.batch_vouts.len() as u32; scratch.batch_vouts.extend( (0u32..) @@ -1261,23 +1278,35 @@ fn reject_bip30_unspent_overwrite( .map(|(v, _)| v), ); let hi = scratch.batch_vouts.len() as u32; - scratch - .batch_creates - .insert(txid, (ctx.height.0, fk, lo, hi)); + scratch.batch_creates.insert(txid, (ctx.height.0, lo, hi)); } Ok(()) } -/// Outpoints spent so far in one write batch, each with the fk of the copy the -/// spend used. A txid can be created again once its first copy is spent (below -/// BIP34, or a pre-BIP34 coinbase whose scriptSig already pushes a later height), -/// so a spend of the new copy is not a second spend of the old one. +/// Outpoints spent so far in one write batch, each with the copy the spend used: +/// its create fk, or for a same-block create the spender's fk. A txid can be +/// created again once its first copy is spent (below BIP34, or a pre-BIP34 +/// coinbase whose scriptSig already pushes a later height), so a spend of the +/// new copy is not a second spend of the old one. pub(crate) type BatchSpent = std::collections::HashMap< ([u8; 32], u32), rbitcoin_primitives::Fk, BuildHasherDefault, >; +/// One block holds one copy of a txid, so two copies created in this batch are +/// the same copy when they have the same height. +fn same_copy( + run_create_height: &RunCreateHeight, + a: rbitcoin_primitives::Fk, + b: rbitcoin_primitives::Fk, +) -> bool { + a == b + || run_create_height + .get(a) + .is_some_and(|h| run_create_height.get(b) == Some(h)) +} + /// A txid can be created again once its first copy is spent (see [`BatchSpent`]). /// Its outputs are coins again (Core overwrites them in the view), so this /// load batch's earlier spends of the old copy no longer block the new one. diff --git a/crates/rbitcoin-test/tests/consensus_rules.rs b/crates/rbitcoin-test/tests/consensus_rules.rs index 0621ddfc1..1f82fcf97 100644 --- a/crates/rbitcoin-test/tests/consensus_rules.rs +++ b/crates/rbitcoin-test/tests/consensus_rules.rs @@ -846,30 +846,32 @@ fn connect_in_batches_bip34_at( /// push(150), h149 spends X:0, h150 creates X again, h250 spends the new X:0. fn bip34_era_repeat_chain(second_spend_sat: u64) -> Chain { let op_true = bitcoin::ScriptBuf::from_bytes(vec![0x51]); - let x = - rbitcoin_consensus::regtest_coinbase(150, Amount::from_sat(25_0000_0000), op_true.clone()); + let x = rbitcoin_consensus::regtest_coinbase(150, Amount::from_sat(25_0000_0000), op_true); + let spend = |sat: u64| spend_anyone_can_spend(x.compute_txid(), 0, Amount::from_sat(sat)); + repeat_chain(250, &x, &[1, 150], |h| match h { + 149 => vec![spend(24_0000_0000)], + 250 => vec![spend(second_spend_sat)], + _ => vec![], + }) +} + +/// Chain to `tip` with coinbase `x` at each `x_at` height and `txs(h)` in block h. +fn repeat_chain( + tip: u32, + x: &bitcoin::Transaction, + x_at: &[u32], + txs: impl Fn(u32) -> Vec, +) -> Chain { + let op_true = bitcoin::ScriptBuf::from_bytes(vec![0x51]); let g = regtest_genesis(); let mut run: Chain = Vec::new(); - for h in 1..=250u32 { + for h in 1..=tip { let (prev, time) = run .last() .map(|(_, b)| (b.block_hash(), b.header.time + 600)) .unwrap_or((g.block_hash(), g.header.time + 600)); - let txs = match h { - 149 => vec![spend_anyone_can_spend( - x.compute_txid(), - 0, - Amount::from_sat(24_0000_0000), - )], - 250 => vec![spend_anyone_can_spend( - x.compute_txid(), - 0, - Amount::from_sat(second_spend_sat), - )], - _ => vec![], - }; - let mut b = rbitcoin_consensus::mine_regtest_paying(prev, time, h, op_true.clone(), txs); - if h == 1 || h == 150 { + let mut b = rbitcoin_consensus::mine_regtest_paying(prev, time, h, op_true.clone(), txs(h)); + if x_at.contains(&h) { b.txdata[0] = x.clone(); b.header.merkle_root = b.compute_merkle_root().unwrap(); rbitcoin_consensus::grind_regtest_pow(&mut b.header); @@ -879,6 +881,123 @@ fn bip34_era_repeat_chain(second_spend_sat: u64) -> Chain { run } +/// Pre-BIP34 (BIP34 at 1000) coinbase X paying 25 BTC, and `spend(prev, sat)`. +fn pre_bip34_x() -> bitcoin::Transaction { + let op_true = bitcoin::ScriptBuf::from_bytes(vec![0x51]); + rbitcoin_consensus::regtest_coinbase(1, Amount::from_sat(25_0000_0000), op_true) +} + +fn spend_of(prev: &bitcoin::Transaction, sat: u64) -> bitcoin::Transaction { + spend_anyone_can_spend(prev.compute_txid(), 0, Amount::from_sat(sat)) +} + +/// h102 creates P and spends P:0 in the same block; h103 spends P:0 again. +/// Core 31.1 rejects h103 (`bad-txns-inputs-missingorspent`), including when +/// IBD merges the two load batches into one write. +#[test] +fn respend_of_a_same_block_spend_rejects_in_any_split() { + let x = pre_bip34_x(); + let p = spend_of(&x, 24_0000_0000); + let chain = repeat_chain(103, &x, &[1], |h| match h { + 102 => vec![p.clone(), spend_of(&p, 23_0000_0000)], + 103 => vec![spend_of(&p, 22_0000_0000)], + _ => vec![], + }); + let spent = |got: &Result, String>| matches!(got, Err(e) if ["PrevoutSpent", "double spend"].iter().any(|w| e.contains(w))); + for cuts in [&[][..], &[102], &[54]] { + let got = connect_in_batches(&chain, cuts); + assert!(spent(&got), "cuts {cuts:?}: got {got:?}"); + } + for cuts in [&[102][..], &[54, 102]] { + let got = connect_merged(&chain, cuts, 1000); + assert!(spent(&got), "merged, cuts {cuts:?}: got {got:?}"); + } +} + +/// h102 spends X:0 by T and T:0 by U in the same block, h103 mines X again, +/// h204 repeats T, h205 spends the new T:0. Core 31.1 accepts to 205: the +/// first T was spent inside its own block. +#[test] +fn repeated_tx_spent_in_its_own_block_connects_in_any_split() { + let x = pre_bip34_x(); + let t = spend_of(&x, 24_0000_0000); + let chain = repeat_chain(205, &x, &[1, 103], |h| match h { + 102 => vec![t.clone(), spend_of(&t, 23_0000_0000)], + 204 => vec![t.clone()], + 205 => vec![spend_of(&t, 22_0000_0000)], + _ => vec![], + }); + for cuts in [&[][..], &[101], &[102], &[103], &[150], &[203]] { + assert_eq!( + connect_in_batches(&chain, cuts), + Ok(Some(Height(205))), + "cuts {cuts:?}" + ); + } + for cuts in [&[102][..], &[101, 103], &[150]] { + assert_eq!( + connect_merged(&chain, cuts, 1000), + Ok(Some(Height(205))), + "merged, cuts {cuts:?}" + ); + } +} + +/// X three times: h1, h103, h204, each copy spent before the next (h102, +/// h203) and the last at h304. Core 31.1 accepts to 304. With h1 connected +/// and the rest in one batch, the BIP30 check of h204 must count h102's spend +/// of the connected copy. +#[test] +fn third_copy_of_a_coinbase_connects_in_any_split() { + let x = pre_bip34_x(); + let chain = repeat_chain(304, &x, &[1, 103, 204], |h| match h { + 102 => vec![spend_of(&x, 24_0000_0000)], + 203 => vec![spend_of(&x, 23_0000_0000)], + 304 => vec![spend_of(&x, 22_0000_0000)], + _ => vec![], + }); + for cuts in [&[][..], &[101], &[101, 204], &[102], &[203]] { + assert_eq!( + connect_in_batches(&chain, cuts), + Ok(Some(Height(304))), + "cuts {cuts:?}" + ); + } + for cuts in [&[101][..], &[150, 250]] { + assert_eq!( + connect_merged(&chain, cuts, 1000), + Ok(Some(Height(304))), + "merged, cuts {cuts:?}" + ); + } +} + +/// Same, but h204 mines X while h103's copy is still unspent. Core 31.1: +/// h204 `bad-txns-BIP30`. h102's spend of the connected copy does not count +/// as a spend of h103's copy. +#[test] +fn third_copy_over_an_unspent_copy_rejects_bip30_in_any_split() { + let x = pre_bip34_x(); + let chain = repeat_chain(204, &x, &[1, 103, 204], |h| match h { + 102 => vec![spend_of(&x, 24_0000_0000)], + _ => vec![], + }); + for cuts in [&[][..], &[101], &[102], &[103]] { + let got = connect_in_batches(&chain, cuts); + assert!( + matches!(&got, Err(e) if e.contains("BIP30")), + "cuts {cuts:?}: got {got:?}" + ); + } + for cuts in [&[101][..], &[150]] { + let got = connect_merged(&chain, cuts, 1000); + assert!( + matches!(&got, Err(e) if e.contains("BIP30")), + "merged, cuts {cuts:?}: got {got:?}" + ); + } +} + #[test] fn bip34_era_repeat_connects_in_any_batch_split() { let chain = bip34_era_repeat_chain(23_0000_0000); diff --git a/docs/consensus-tests.md b/docs/consensus-tests.md index 1b6a8ba9e..c684a82cd 100644 --- a/docs/consensus-tests.md +++ b/docs/consensus-tests.md @@ -144,7 +144,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`, `bip30_duplicate_coinbase_in_one_batch`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects`, `bip34_era_repeat_connects_in_any_batch_split`, `bip34_era_repeated_spend_rejects_bip30_in_any_batch_split`, `repeated_coinbase_connects_when_ibd_merges_load_batches` | +| 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`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects`, `bip34_era_repeat_connects_in_any_batch_split`, `bip34_era_repeated_spend_rejects_bip30_in_any_batch_split`, `repeated_coinbase_connects_when_ibd_merges_load_batches`, `respend_of_a_same_block_spend_rejects_in_any_split`, `repeated_tx_spent_in_its_own_block_connects_in_any_split`, `third_copy_of_a_coinbase_connects_in_any_split`, `third_copy_over_an_unspent_copy_rejects_bip30_in_any_split` | | 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 | From e98f3885a628b43f1af945ea3f9649f247e7448a Mon Sep 17 00:00:00 2001 From: "rearden-grok[bot]" <317016512+rearden-grok[bot]@users.noreply.github.com> Date: Sat, 17 Oct 2026 20:17:10 -0700 Subject: [PATCH 5/5] consensus: bind a repeated txid the same way on an archived retry A structural failure archives the bodies and leaves the tip unmoved. The retry has no fresh plan, so the stamp used to keep one fk per txid and bind the later copy. It now uses the same order as the planner: the newest copy created before the spend, then a connected copy, then a later in-batch copy. --- changelog.d/repeated-txid-in-batch.md | 1 + crates/rbitcoin-query/src/archive.rs | 59 ++++-- .../rbitcoin-query/src/confirm_run/lookup.rs | 198 +++++++++++++----- crates/rbitcoin-query/src/confirm_run/pin.rs | 28 ++- crates/rbitcoin-test/tests/consensus_rules.rs | 19 ++ docs/consensus-tests.md | 4 +- 6 files changed, 238 insertions(+), 71 deletions(-) diff --git a/changelog.d/repeated-txid-in-batch.md b/changelog.d/repeated-txid-in-batch.md index 654b8bf25..5de70f339 100644 --- a/changelog.d/repeated-txid-in-batch.md +++ b/changelog.d/repeated-txid-in-batch.md @@ -5,4 +5,5 @@ Fixed BIP34 when an old coinbase already pushed the later height. A spend now uses the copy that exists at its height, the outputs of a recreated transaction can be spent again, and BIP30 counts spends made earlier in the same batch. + A retry after those bodies were already archived uses the same order. Before, IBD could halt on such a chain that Core accepts. diff --git a/crates/rbitcoin-query/src/archive.rs b/crates/rbitcoin-query/src/archive.rs index e56fa70aa..197aaf7df 100644 --- a/crates/rbitcoin-query/src/archive.rs +++ b/crates/rbitcoin-query/src/archive.rs @@ -570,18 +570,19 @@ struct PlanRow { tx_index: u32, } -/// The newest copy of `txid` this batch creates before `spender`. Below BIP34 -/// a txid can repeat once its first copy is spent; a spend uses the copy that -/// exists at its height. -fn batch_copy_before( - batch_map: &crate::TxidMap, - batch_repeats: &crate::TxidMap>, - txid: &[u8; 32], +/// Newest in-batch create of this txid strictly before `spender`. +/// +/// `repeats` holds every fk when the txid occurs more than once. Otherwise +/// `latest` is the only copy. A copy created later in the wave is not a coin yet. +/// The fresh plan and the archived stamp share this. +pub(crate) fn batch_copy_before( + latest: Option, + repeats: Option<&[Fk]>, spender: Fk, ) -> Option { - match batch_repeats.get(txid) { + match repeats { Some(fks) => fks.iter().rev().find(|f| f.0 < spender.0).copied(), - None => batch_map.get(txid).copied().filter(|f| f.0 < spender.0), + None => latest.filter(|f| f.0 < spender.0), } } @@ -619,7 +620,13 @@ fn collect_plan_need_external( if inp.is_coinbase || inp.prev_txid == [0u8; 32] { continue; } - if batch_copy_before(batch_map, batch_repeats, &inp.prev_txid, row.tx_fk).is_some() { + if batch_copy_before( + batch_map.get(&inp.prev_txid).copied(), + batch_repeats.get(&inp.prev_txid).map(Vec::as_slice), + row.tx_fk, + ) + .is_some() + { continue; } if batch_map.contains_key(&inp.prev_txid) && !outside(&inp.prev_txid) { @@ -1254,9 +1261,11 @@ impl Query { }); continue; } - let create_fk = if let Some(cfk) = - batch_copy_before(&batch_map, &batch_repeats, &inp.prev_txid, tx_fk) - { + let create_fk = if let Some(cfk) = batch_copy_before( + batch_map.get(&inp.prev_txid).copied(), + batch_repeats.get(&inp.prev_txid).map(Vec::as_slice), + tx_fk, + ) { batch_stamp = batch_stamp.saturating_add(1); cfk } else if let Some(&cfk) = resolved.get(&inp.prev_txid) { @@ -2780,6 +2789,30 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// A spend sitting between two copies of one txid must bind the copy that + /// already exists, not the copy this wave creates later. + #[test] + fn plan_batch_spend_between_repeated_txid_binds_the_earlier_copy() { + let (dir, q) = temp_query("bip30-between"); + let txid = coinbase_apply(1).tx.txid; + let child = child_spend(txid, 0x99); + let need = vec![ + (Fk(1), vec![coinbase_apply(1)]), + (Fk(2), vec![child]), + (Fk(3), vec![coinbase_apply(1)]), + ]; + let plan = plan_applies(&q, &need, 1, &crate::InFlight::new(), None) + .expect("spend between two copies of one txid"); + let spend = plan.edges.get(&2).expect("child"); + assert_eq!( + spend[0].create_fk, + Fk(1), + "spend between two copies binds the earlier one, got {:?}", + spend[0].create_fk + ); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn plan_batch_from_wire_duplicate_txid_in_one_block_is_corrupt() { let (dir, q) = temp_query("dup-txid-in-block"); diff --git a/crates/rbitcoin-query/src/confirm_run/lookup.rs b/crates/rbitcoin-query/src/confirm_run/lookup.rs index 11705f3d6..60641af3d 100644 --- a/crates/rbitcoin-query/src/confirm_run/lookup.rs +++ b/crates/rbitcoin-query/src/confirm_run/lookup.rs @@ -166,6 +166,129 @@ pub fn confirm_wire_lookup_stamp( }) } +/// One fk per txid (`latest`), plus every fk when a txid is created again. +pub(super) struct BatchCopies { + pub latest: crate::TxidMap, + pub repeats: crate::TxidMap>, +} + +/// Record each meta's creates in order. A second fk for one txid starts `repeats`. +pub(super) fn batch_copies_from_metas(metas: &[BodyMeta]) -> BatchCopies { + let mut latest: crate::TxidMap = crate::TxidMap::default(); + let mut repeats: crate::TxidMap> = crate::TxidMap::default(); + for m in metas { + for (tid, fk) in m.txids.iter().zip(m.tx_fks.iter()) { + let Some(id) = fk.get() else { + continue; + }; + let Some(prev) = latest.insert(*tid, id) else { + continue; + }; + repeats.entry(*tid).or_insert_with(|| vec![prev]).push(id); + } + } + BatchCopies { latest, repeats } +} + +/// Carried keys this skeleton does not already hold as an in-batch create. +/// Returns whether any carried key is also created in this batch. +fn carried_keys_outside_batch( + latest: &crate::TxidMap, + carried_need: Option<&[[u8; 32]]>, + need_external: &mut crate::TxidMap, +) -> bool { + let mut created_here = false; + for &prev in carried_need.unwrap_or(&[]) { + if prev == [0u8; 32] { + continue; + } + if latest.contains_key(&prev) { + created_here = true; + continue; + } + need_external.insert(prev, true); + } + created_here +} + +fn spend_needs_external_parent( + copies: &BatchCopies, + prev: &[u8; 32], + spender: rbitcoin_primitives::Fk, + outside: bool, +) -> bool { + let repeats: Option> = copies + .repeats + .get(prev) + .map(|ids| ids.iter().copied().map(rbitcoin_primitives::Fk).collect()); + if crate::archive::batch_copy_before( + copies + .latest + .get(prev) + .copied() + .map(rbitcoin_primitives::Fk), + repeats.as_deref(), + spender, + ) + .is_some() + { + return false; + } + if copies.latest.contains_key(prev) && !outside { + return false; + } + true +} + +/// Wire spends with no earlier in-batch copy. `true` means the input spends it. +fn note_spends_without_earlier_copy( + metas: &[BodyMeta], + wire_blocks: &[Arc], + copies: &BatchCopies, + outside: impl Fn(&[u8; 32]) -> bool, + need_external: &mut crate::TxidMap, +) { + for (m, block) in metas.iter().zip(wire_blocks.iter()) { + for (ti, tx) in block.txdata.iter().enumerate() { + let Some(sfk) = m.tx_fks.get(ti).and_then(|f| f.get()) else { + continue; + }; + let spender = rbitcoin_primitives::Fk(sfk); + for inp in &tx.input { + if inp.previous_output.is_null() { + continue; + } + let prev = inp.previous_output.txid.to_byte_array(); + if prev == [0u8; 32] + || !spend_needs_external_parent(copies, &prev, spender, outside(&prev)) + { + continue; + } + need_external.insert(prev, true); + } + } + } +} + +fn stamp_batch_idents(stamp: &mut ParentPinStamp, copies: &BatchCopies) { + for (tid, id) in &copies.latest { + stamp + .idents + .entry(*id) + .or_insert_with(|| crate::ParentIdent::new(*tid)) + .txid = *tid; + } + for (tid, ids) in &copies.repeats { + for id in ids { + stamp + .idents + .entry(*id) + .or_insert_with(|| crate::ParentIdent::new(*tid)) + .txid = *tid; + } + } +} + /// plan=None rehydrate: stamp external parent create_fk + body_range + txid /// via the shared query helper so load never probes those tables. /// @@ -180,48 +303,26 @@ pub(super) fn stamp_parent_pin_archived( skeleton: Option<&crate::BatchParentIds>, carried_need: Option<&[[u8; 32]]>, ) -> Result { - let mut same_batch: crate::TxidMap = crate::TxidMap::default(); - for m in metas { - for (tid, fk) in m.txids.iter().zip(m.tx_fks.iter()) { - if let Some(id) = fk.get() { - same_batch.insert(*tid, id); - } - } - } + let copies = batch_copies_from_metas(metas); + let outside = |prev: &[u8; 32]| { + skeleton.is_none_or(|s| s.get(prev).is_some()) + || in_flight.is_some_and(|ifo| ifo.get_create_fk(prev).is_some()) + }; // Value: a wire input spends this txid (BIP30 probes are `false`). + // Empty carried_need must not walk wire inputs. A carried key that this + // batch also creates needs the walk: the spend may be before that create. let mut need_external: crate::TxidMap = crate::TxidMap::default(); - if skeleton.is_some() { - for &prev in carried_need.unwrap_or(&[]) { - if same_batch.contains_key(&prev) { - continue; - } - if prev != [0u8; 32] { - need_external.insert(prev, true); - } - } - } else { - for block in wire_blocks { - for tx in &block.txdata { - for inp in &tx.input { - if inp.previous_output.is_null() { - continue; - } - let prev = inp.previous_output.txid.to_byte_array(); - if same_batch.contains_key(&prev) { - continue; - } - if prev != [0u8; 32] { - need_external.insert(prev, true); - } - } - } - } + let walk_inputs = skeleton.is_none() + || carried_keys_outside_batch(&copies.latest, carried_need, &mut need_external); + if walk_inputs { + note_spends_without_earlier_copy(metas, wire_blocks, &copies, outside, &mut need_external); } for m in metas { - if !params.bip34_active_at(m.height.0) { - for p in m.pres.iter() { - need_external.entry(p.txid).or_insert(false); - } + if params.bip34_active_at(m.height.0) { + continue; + } + for p in m.pres.iter() { + need_external.entry(p.txid).or_insert(false); } } let empty = crate::InFlight::new(); @@ -242,26 +343,19 @@ pub(super) fn stamp_parent_pin_archived( rbitcoin_consensus::block::reject_unresolved_spend(true)?; } let mut stamp = ParentPinStamp { - resolved: crate::TxidMap::with_capacity_and_hasher( - ext.resolved.len().saturating_add(same_batch.len()), - Default::default(), - ), + // External parents only. Same-batch copies live in `idents`; edges + // pick the copy that exists at the spend. + resolved: crate::TxidMap::with_capacity_and_hasher(ext.resolved.len(), Default::default()), idents: ext.idents, parent_vouts: U64Map::default(), }; for (tid, fk) in ext.resolved { - if let Some(id) = fk.get() { - stamp.resolved.insert(tid, id); - } - } - for (tid, id) in same_batch { + let Some(id) = fk.get() else { + continue; + }; stamp.resolved.insert(tid, id); - stamp - .idents - .entry(id) - .or_insert_with(|| crate::ParentIdent::new(tid)) - .txid = tid; } + stamp_batch_idents(&mut stamp, &copies); // Identities are stamped from wire prev_txid at insert time — never soft-fill // from txid.body here (that would be a dual path after lookup promised identity). if skeleton.is_none() { diff --git a/crates/rbitcoin-query/src/confirm_run/pin.rs b/crates/rbitcoin-query/src/confirm_run/pin.rs index 1f491d3f8..3ba08ab7f 100644 --- a/crates/rbitcoin-query/src/confirm_run/pin.rs +++ b/crates/rbitcoin-query/src/confirm_run/pin.rs @@ -60,6 +60,7 @@ fn spend_edges_from_stamp( metas: &[BodyMeta], wire_blocks: &[Arc], ) -> (crate::SpendEdges, U64Map>) { + let copies = super::lookup::batch_copies_from_metas(metas); let mut spend_edges = crate::SpendEdges::default(); let mut parent_vouts: U64Map> = U64Map::default(); for (m, block) in metas.iter().zip(wire_blocks.iter()) { @@ -67,6 +68,7 @@ fn spend_edges_from_stamp( let Some(sfk) = m.tx_fks.get(ti).and_then(|f| f.get()) else { continue; }; + let spender = rbitcoin_primitives::Fk(sfk); let mut edges = Vec::with_capacity(tx.input.len()); for (vin, inp) in tx.input.iter().enumerate() { let vin = vin as u32; @@ -74,7 +76,7 @@ fn spend_edges_from_stamp( edges.push(crate::SpendEdge { prev_txid: [0u8; 32], vout: u32::MAX, - spend_fk: rbitcoin_primitives::Fk(sfk), + spend_fk: spender, create_fk: rbitcoin_primitives::Fk::NULL, vin, }); @@ -82,11 +84,29 @@ fn spend_edges_from_stamp( } let prev_txid = inp.previous_output.txid.to_byte_array(); let vout = inp.previous_output.vout; - if let Some(&pid) = parent_pin.resolved.get(&prev_txid) { + let repeats: Option> = copies + .repeats + .get(&prev_txid) + .map(|ids| ids.iter().copied().map(rbitcoin_primitives::Fk).collect()); + // Newest copy before this spend, else a connected copy, else + // the later in-batch copy (structural rejects that as not a coin). + let pid = crate::archive::batch_copy_before( + copies + .latest + .get(&prev_txid) + .copied() + .map(rbitcoin_primitives::Fk), + repeats.as_deref(), + spender, + ) + .map(|fk| fk.0) + .or_else(|| parent_pin.resolved.get(&prev_txid).copied()) + .or_else(|| copies.latest.get(&prev_txid).copied()); + if let Some(pid) = pid { edges.push(crate::SpendEdge { prev_txid, vout, - spend_fk: rbitcoin_primitives::Fk(sfk), + spend_fk: spender, create_fk: rbitcoin_primitives::Fk(pid), vin, }); @@ -96,7 +116,7 @@ fn spend_edges_from_stamp( edges.push(crate::SpendEdge { prev_txid, vout, - spend_fk: rbitcoin_primitives::Fk(sfk), + spend_fk: spender, create_fk: rbitcoin_primitives::Fk::NULL, vin, }); diff --git a/crates/rbitcoin-test/tests/consensus_rules.rs b/crates/rbitcoin-test/tests/consensus_rules.rs index 1f82fcf97..e4f56ed76 100644 --- a/crates/rbitcoin-test/tests/consensus_rules.rs +++ b/crates/rbitcoin-test/tests/consensus_rules.rs @@ -1180,6 +1180,25 @@ fn repeated_coinbase_chain_rejects_what_core_rejects() { } } +/// A failed batch archives every body before Class C. Retrying the valid +/// prefix must bind through the archived stamp, not the fresh plan. +#[test] +fn repeated_coinbase_archived_retry_connects_the_valid_prefix() { + let chain = repeated_coinbase_chain(150, 48_0000_0000, true); + let (_td, q, mut params) = regtest_q(); + params.apply_test_activation_height("bip34", 1_000).unwrap(); + connect_genesis(&q, ¶ms); + let err = confirm_wire_run(&q, ¶ms, Milestone::NONE, &chain); + assert!( + matches!(err, Err(ConfirmError::Rules(ConsensusError::BadTx(s))) if s.contains("immature")), + "immature spend of the recreated coinbase: {err:?}" + ); + assert_eq!(q.tip_height(), Some(Height::GENESIS)); + confirm_wire_run(&q, ¶ms, Milestone::NONE, &chain[..103]) + .expect("archived retry of h1..=h103"); + assert_eq!(q.tip_height(), Some(Height(103))); +} + // ─── 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 c684a82cd..d3d62c4dd 100644 --- a/docs/consensus-tests.md +++ b/docs/consensus-tests.md @@ -144,11 +144,11 @@ 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`, `bip30_duplicate_coinbase_in_one_batch`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects`, `bip34_era_repeat_connects_in_any_batch_split`, `bip34_era_repeated_spend_rejects_bip30_in_any_batch_split`, `repeated_coinbase_connects_when_ibd_merges_load_batches`, `respend_of_a_same_block_spend_rejects_in_any_split`, `repeated_tx_spent_in_its_own_block_connects_in_any_split`, `third_copy_of_a_coinbase_connects_in_any_split`, `third_copy_over_an_unspent_copy_rejects_bip30_in_any_split` | +| 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`, `repeated_coinbase_chain_connects_in_any_batch_split`, `repeated_coinbase_chain_rejects_what_core_rejects`, `bip34_era_repeat_connects_in_any_batch_split`, `bip34_era_repeated_spend_rejects_bip30_in_any_batch_split`, `repeated_coinbase_connects_when_ibd_merges_load_batches`, `respend_of_a_same_block_spend_rejects_in_any_split`, `repeated_tx_spent_in_its_own_block_connects_in_any_split`, `third_copy_of_a_coinbase_connects_in_any_split`, `third_copy_over_an_unspent_copy_rejects_bip30_in_any_split`, `repeated_coinbase_archived_retry_connects_the_valid_prefix` | | 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 | -| C42 | An input binds only to an output from an earlier block or an earlier tx in its own block, also inside one multi-block confirm batch (Core connects one block at a time) | `MissingPrevout` (`bad-txns-inputs-missingorspent`) | `consensus_rules::same_batch_spend_of_a_later_block_is_missing`; IBD isolation `batched_spend_of_a_later_block_is_rejected_alone` ([087](./external_findings/087-forward-spend-in-batch.md)); same block: `header_and_spending_boundaries` ([005](./external_findings/005-non-topological-block-accepted.md)) | +| C42 | An input binds only to an output from an earlier block or an earlier tx in its own block, also inside one multi-block confirm batch (Core connects one block at a time). A repeated txid binds the newest copy created before the spend, including when the bodies are already archived | `MissingPrevout` (`bad-txns-inputs-missingorspent`) | `consensus_rules::same_batch_spend_of_a_later_block_is_missing`; `plan_batch_spend_between_repeated_txid_binds_the_earlier_copy`; `repeated_coinbase_archived_retry_connects_the_valid_prefix`; IBD isolation `batched_spend_of_a_later_block_is_rejected_alone` ([087](./external_findings/087-forward-spend-in-batch.md)); same block: `header_and_spending_boundaries` ([005](./external_findings/005-non-topological-block-accepted.md)) | | C43 | An output whose script starts with `OP_RETURN`, or is longer than 10,000 bytes, is not a coin. A 10,000-byte script that does not start with `OP_RETURN` is a coin. The archive still stores the output | `MissingPrevout` / `bad-txns-inputs-missingorspent`, with scripts on and under a height-only milestone | `spending_an_unspendable_output_is_missing_prevout`; `resolve_prevout_rejects_an_unspendable_script`; `proposal_spend_of_an_unspendable_output_is_missing` | | C44 | Anchored script skip requires this block's hash and the anchor hash on the header path. A failing script above the anchor, or under a different hash, still runs | confirm accepts only when both hashes match | `anchored_milestone_skips_a_failing_p2pkh_script` (empty P2PKH scriptSig) |