diff --git a/changelog.d/proposal-parent-followup.md b/changelog.d/proposal-parent-followup.md new file mode 100644 index 000000000..322a5009e --- /dev/null +++ b/changelog.d/proposal-parent-followup.md @@ -0,0 +1,14 @@ +Changed + +- **Packed body decodes are counted at the decode site.** The counter the + mempool and block-proposal pins read sits on the tx table's outs decoder, so + every per-fk decode is seen; the confirm write stage's full decoder is not + counted. +- **Txid and spender resolution no longer decode the body.** Resolving a txid + to its row, probing whether an output is spent, and recording a spend read + the `txid.body` identity only; the packed decode runs only for a reader that + needs the record. +- **A block proposal keeps only the parent outputs it spends.** The check + resolves each confirmed parent on the connected chain, decodes it once, and + holds only the spent outputs until it returns; a row that exists only in a + reorged-out block is not an input. diff --git a/changelog.d/proposal-spentness-connected.md b/changelog.d/proposal-spentness-connected.md new file mode 100644 index 000000000..2c66bbfd2 --- /dev/null +++ b/changelog.d/proposal-spentness-connected.md @@ -0,0 +1,6 @@ +Fixed + +- **Spentness is probed on the connected create.** `gettxout`, mempool accept, + the proposal check, and the Electrum and Esplora spent checks read the + spender slots of the row on the best chain; a newer never-connected row for + the same txid no longer hides a confirmed spend. diff --git a/crates/rbitcoin-net/src/chain.rs b/crates/rbitcoin-net/src/chain.rs index 8ae2f147d..c66d53b17 100644 --- a/crates/rbitcoin-net/src/chain.rs +++ b/crates/rbitcoin-net/src/chain.rs @@ -20,7 +20,6 @@ use rbitcoin_consensus::{ use rbitcoin_log::{debug, info}; use rbitcoin_primitives::{Fk, Height}; use rbitcoin_query::Query; -use std::collections::hash_map::Entry; use std::collections::{HashMap, HashSet}; use std::sync::atomic::{AtomicBool, AtomicI32, AtomicU32, AtomicU64, Ordering}; use std::sync::Arc; @@ -1775,9 +1774,9 @@ impl ChainHub { ) } - /// Core `TestBlockValidity` for a `getblocktemplate` proposal or an SV2 - /// job: no PoW, no UTXO write. `Ok` is the block fees in sat; `Err` is - /// Core's reject string. + /// Core `TestBlockValidity` for a `getblocktemplate` proposal: no PoW, no + /// UTXO write. `Ok` is the block fees in sat; `Err` is Core's reject + /// string. pub fn check_block_proposal(&self, block: &Block) -> Result { check_block_proposal_with( &self.query, @@ -3261,6 +3260,12 @@ pub fn check_block_proposal_with( /// Resolves every spend against the block and the confirmed chain. `Ok` is /// the fee total and the prevouts already resolved for each non-coinbase tx. /// Caller has already passed [`rbitcoin_consensus::validate_block_structure`]. +/// +/// RAM: `created` holds this block's outputs and [`confirmed_parent_outputs`] +/// only the parent outputs this block spends, O(block outputs + block +/// inputs), dropped at return; the returned prevouts are those spent outputs +/// in block order. CPU: each distinct parent's packed body is decoded once; +/// spentness is still probed per input, never served from that map. fn proposal_connect( query: &Query, block: &Block, @@ -3274,15 +3279,14 @@ fn proposal_connect( if !block.txdata[0].is_coinbase() { return Err("bad-cb-missing".into()); } - let cb_txid = block.txdata[0].compute_txid(); + let txids: Vec = block.txdata.iter().map(|tx| tx.compute_txid()).collect(); + let parents = confirmed_parent_outputs(query, block, &txids); + let cb_txid = txids[0]; let mut created: HashMap = HashMap::new(); let mut spent: HashSet = HashSet::new(); - // RAM: decoded outputs of this block's distinct confirmed parents. - // Dropped when the check returns. The fk resolve does not decode the body. - let mut parents: HashMap)> = HashMap::new(); let mut fees = 0u64; let mut prevouts = Vec::with_capacity(block.txdata.len().saturating_sub(1)); - for (i, tx) in block.txdata.iter().enumerate() { + for (i, (tx, &txid)) in block.txdata.iter().zip(&txids).enumerate() { if !rbitcoin_consensus::is_final_tx(tx, height, mtp.max(block.header.time)) { return Err("bad-txns-nonfinal".into()); } @@ -3290,7 +3294,7 @@ fn proposal_connect( for (vout, o) in tx.output.iter().enumerate() { created.insert( OutPoint { - txid: cb_txid, + txid, vout: vout as u32, }, o.clone(), @@ -3312,11 +3316,11 @@ fn proposal_connect( } let txout = if let Some(o) = created.get(&op) { o.clone() - } else if let Some((fk, o)) = chain_txout(query, &mut parents, &op) { - if coinbase_spend_is_immature(query, fk, height, maturity)? { + } else if let Some((fk, o)) = chain_txout(query, &parents, &op) { + if coinbase_spend_is_immature(query, *fk, height, maturity)? { return Err("bad-txns-premature-spend-of-coinbase".into()); } - o + o.clone() } else { return Err("bad-txns-inputs-missingorspent".into()); }; @@ -3338,11 +3342,10 @@ fn proposal_connect( .checked_add(in_val - out_val) .ok_or("bad-txns-fee-outofrange")?; prevouts.push(tx_prevouts); - let tid = tx.compute_txid(); for (vout, o) in tx.output.iter().enumerate() { created.insert( OutPoint { - txid: tid, + txid, vout: vout as u32, }, o.clone(), @@ -3352,40 +3355,67 @@ fn proposal_connect( Ok((fees, prevouts)) } -/// `op`'s unspent confirmed output. `parents` holds each parent's create fk -/// and decoded outputs for this one proposal check. -fn chain_txout( +/// `op`'s unspent confirmed output: a per-input spentness probe, then the +/// pre-decoded entry in `parents`. +fn chain_txout<'a>( query: &Query, - parents: &mut HashMap)>, + parents: &'a HashMap, op: &OutPoint, -) -> Option<(Fk, TxOut)> { - let tid = op.txid.to_byte_array(); - if query.is_outpoint_spent(&tid, op.vout).ok()? { +) -> Option<&'a (Fk, TxOut)> { + if query + .is_outpoint_spent(&op.txid.to_byte_array(), op.vout) + .ok()? + { return None; } - let slot = match parents.entry(op.txid) { - Entry::Occupied(e) => e.into_mut(), - Entry::Vacant(v) => v.insert(chain_tx_outputs(query, &tid)?), - }; - let out = slot.1.get(op.vout as usize).cloned()?; - Some((slot.0, out)) + parents.get(op) +} + +/// The confirmed-parent outputs `block` spends: one tip-only fk resolve and +/// one packed body decode per distinct parent, keeping only the spent vouts. +/// Spends of `txids` (created in the block) are left to the connect loop; a +/// parent that does not resolve contributes nothing, so its spends reject in +/// block order. +fn confirmed_parent_outputs( + query: &Query, + block: &Block, + txids: &[Txid], +) -> HashMap { + let in_block: HashSet<&Txid> = txids.iter().collect(); + let mut vouts_by_parent: HashMap> = HashMap::new(); + for inp in block.txdata.iter().skip(1).flat_map(|tx| &tx.input) { + let op = inp.previous_output; + if !in_block.contains(&op.txid) { + vouts_by_parent.entry(op.txid).or_default().push(op.vout); + } + } + let mut outs = HashMap::new(); + for (txid, vouts) in vouts_by_parent { + let Some((fk, all)) = chain_tx_outputs(query, &txid.to_byte_array()) else { + continue; + }; + for vout in vouts { + if let Some(out) = all.get(vout as usize) { + let txout = TxOut { + value: Amount::from_sat(u64::try_from(out.value).unwrap_or(0)), + script_pubkey: ScriptBuf::from_bytes(out.script.clone()), + }; + outs.insert(OutPoint { txid, vout }, (fk, txout)); + } + } + } + outs } -/// Create fk, from an identity resolve, and every output of a confirmed tx. -fn chain_tx_outputs(query: &Query, txid: &[u8; 32]) -> Option<(Fk, Vec)> { - let fk = query.tx_fk_by_txid(txid).ok().flatten()?; +/// Every output of a tx on the connected chain: one head resolve, one packed +/// outs decode. A row that exists only in a reorged-out block is `None`, as +/// in Core. +fn chain_tx_outputs( + query: &Query, + txid: &[u8; 32], +) -> Option<(Fk, Vec)> { + let fk = query.tx_fk_by_txid_tip(txid).ok().flatten()?; let (_, outs) = query.store().get_tx_meta_and_outputs(fk).ok()?; - let outs = outs - .into_iter() - .map(|out| TxOut { - value: if out.value < 0 { - Amount::ZERO - } else { - Amount::from_sat(out.value as u64) - }, - script_pubkey: ScriptBuf::from_bytes(out.script), - }) - .collect(); Some((fk, outs)) } @@ -6326,27 +6356,197 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } - /// The block proposal check shared by `getblocktemplate` proposal mode - /// and the SV2 template provider: a child of the tip passes, a block on - /// any other parent is `inconclusive-not-best-prevblk`. + /// One regtest node at height 101 checking block proposals, the Core + /// `TestBlockValidity` path behind `getblocktemplate` proposal mode. + /// Regtest coinbase maturity is 100, so a mature spend needs a 101-block + /// chain and no smaller N reaches one; that one boot is what runs this + /// test past the two-second budget, and it serves every beat. The + /// tip-child, coinbase-amount, immature-coinbase, and mature-spend beats + /// do not observe each other; they ride this hub rather than a shared + /// fixture because they only read the 101-block tip the mutating beats + /// then build on (the parent mined at 102, its child confirmed at 103, + /// that block invalidated), so a second setup would add store copies and + /// test names without a new observation. + /// A tip child passes; a block on any other + /// parent is `inconclusive-not-best-prevblk`. Core prices the coinbase + /// in ConnectBlock, after CheckBlock: a coinbase over subsidy + fees is + /// `bad-cb-amount` once the structure checks pass, and a structure + /// reject on the same block still wins. An immature coinbase is not a + /// fee source: spending this block's coinbase, or one still inside the + /// maturity window, is `bad-txns-premature-spend-of-coinbase` before + /// fees are summed, even when that spend would cover an overpaying + /// coinbase. `Ok` carries the block fees, so a template provider can + /// price the coinbase at exactly subsidy + fees. A fan-out parent is + /// decoded once per check, not once per child + /// (`TxTable::sample_reset_body_decodes`); output `i` carries a distinct + /// value so the fee total also pins vout indexing. Spentness is probed + /// per input: with vout 0 confirmed spent, a proposal whose first sight + /// of the parent is the unspent vout 1 still rejects a fresh spend of + /// vout 0 (a fresh tx, so Core's answer is the same reject and not + /// `bad-txns-BIP30`). + /// Parents resolve on the connected chain only: after the block that + /// confirmed `child(0)` is invalidated, a spend of its output is + /// `bad-txns-inputs-missingorspent` and the stale row is not decoded. #[test] - fn check_block_proposal_accepts_tip_child_and_rejects_other_parent() { + fn check_block_proposal_prices_fees_and_rejects_on_one_chain() { let (dir, hub) = tmp_hub(); - hub.ensure_genesis().unwrap(); + let op_true = ScriptBuf::from_bytes(vec![0x51]); + hub.generate_to_script(101, op_true.clone(), vec![]) + .unwrap(); + let block = hub - .assemble_block_to_script(ScriptBuf::from_bytes(vec![0x51]), vec![]) + .assemble_block_to_script(op_true.clone(), vec![]) .unwrap(); assert_eq!( hub.check_block_proposal(&block), Ok(0), "no fees in a coinbase-only block" ); - let mut off_tip = block; + let mut off_tip = block.clone(); off_tip.header.prev_blockhash = BlockHash::from_byte_array([0xab; 32]); assert_eq!( hub.check_block_proposal(&off_tip).unwrap_err(), "inconclusive-not-best-prevblk" ); + + let mut fat_cb = block; + let cb = &mut fat_cb.txdata[0].output[0].value; + *cb = Amount::from_sat(cb.to_sat() + 1); + let root_for_fat_cb = fat_cb.compute_merkle_root().unwrap(); + assert_eq!( + hub.check_block_proposal(&fat_cb).unwrap_err(), + "bad-txnmrklroot", + "structure checks run before the coinbase is priced" + ); + fat_cb.header.merkle_root = root_for_fat_cb; + assert_eq!( + hub.check_block_proposal(&fat_cb).unwrap_err(), + "bad-cb-amount" + ); + + let cb_txid = fat_cb.txdata[0].compute_txid(); + fat_cb.txdata.push(spend_out(cb_txid, 0)); + assert_eq!( + hub.check_block_proposal(&fat_cb).unwrap_err(), + "bad-txnmrklroot", + "structure checks run before the immature spend" + ); + fat_cb.header.merkle_root = fat_cb.compute_merkle_root().unwrap(); + assert_eq!( + hub.check_block_proposal(&fat_cb).unwrap_err(), + "bad-txns-premature-spend-of-coinbase", + "a same-block coinbase spend must not inflate fees" + ); + let tip_cb = hub + .query + .reconstruct_block_at_height(Height(101)) + .unwrap() + .txdata[0] + .clone(); + let tip_cb_value = tip_cb.output[0].value.to_sat(); + let immature = spend_out(tip_cb.compute_txid(), tip_cb_value - 1_000); + let block = hub + .assemble_block_to_script(op_true.clone(), vec![immature]) + .unwrap(); + assert_eq!( + hub.check_block_proposal(&block).unwrap_err(), + "bad-txns-premature-spend-of-coinbase" + ); + let over = spend_out(tip_cb.compute_txid(), tip_cb_value + 1); + let block = hub + .assemble_block_to_script(op_true.clone(), vec![over]) + .unwrap(); + assert_eq!( + hub.check_block_proposal(&block).unwrap_err(), + "bad-txns-premature-spend-of-coinbase", + "maturity is reported before bad-txns-in-belowout" + ); + + let spend = mature_spend_tx(&hub, 1); + let mut block = hub + .assemble_block_to_script(op_true.clone(), vec![spend]) + .unwrap(); + assert_eq!(hub.check_block_proposal(&block), Ok(10_000)); + let subsidy = rbitcoin_consensus::block_subsidy(102, &hub.params) as u64; + block.txdata[0].output[0].value = Amount::from_sat(subsidy + 10_000); + block.header.merkle_root = block.compute_merkle_root().unwrap(); + assert_eq!(hub.check_block_proposal(&block), Ok(10_000)); + + let payout = 98_000_000u64; + let parent_value = |vout: u32| payout + 1_000 * u64::from(vout + 1); + let mut parent = mature_spend_tx(&hub, 1); + parent.output = (0..50) + .map(|vout| TxOut { + value: Amount::from_sat(parent_value(vout)), + script_pubkey: op_true.clone(), + }) + .collect(); + let parent_txid = parent.compute_txid(); + hub.generate_to_script(1, op_true.clone(), vec![parent]) + .unwrap(); + let child_paying = |vout: u32, value: u64| Transaction { + version: TxVersion::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint { + txid: parent_txid, + vout, + }, + script_sig: ScriptBuf::new(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(value), + script_pubkey: op_true.clone(), + }], + }; + let child = |vout: u32| child_paying(vout, payout); + let block = hub + .assemble_block_to_script(op_true.clone(), (0..50).map(child).collect()) + .unwrap(); + let fees: u64 = (0..50).map(|vout| parent_value(vout) - payout).sum(); + let store = hub.query.store(); + let _ = store.txs.sample_reset_body_decodes(); + assert_eq!(hub.check_block_proposal(&block), Ok(fees)); + assert_eq!( + store.txs.sample_reset_body_decodes(), + 1, + "one packed body decode per distinct parent; the spentness probes and the fk resolve decode nothing" + ); + + hub.generate_to_script(1, op_true.clone(), vec![child(0)]) + .unwrap(); + let respend = child_paying(0, payout - 1_000); + let block = hub + .assemble_block_to_script(op_true.clone(), vec![child(1), respend]) + .unwrap(); + let _ = store.txs.sample_reset_body_decodes(); + assert_eq!( + hub.check_block_proposal(&block).unwrap_err(), + "bad-txns-inputs-missingorspent", + "spentness is probed per input; the parent seen first through unspent vout 1 does not vouch for vout 0" + ); + assert_eq!(store.txs.sample_reset_body_decodes(), 1); + + let stale = hub.tip_hash().unwrap(); + hub.invalidate_block(stale).unwrap(); + assert_eq!(hub.tip_height(), Some(102)); + let grandchild = spend_out(child(0).compute_txid(), payout - 1_000); + let block = hub + .assemble_block_to_script(op_true, vec![grandchild]) + .unwrap(); + let _ = store.txs.sample_reset_body_decodes(); + assert_eq!( + hub.check_block_proposal(&block).unwrap_err(), + "bad-txns-inputs-missingorspent", + "an output that exists only in a reorged-out block is not spendable" + ); + assert_eq!( + store.txs.sample_reset_body_decodes(), + 0, + "a row with no fence height is not decoded" + ); let _ = std::fs::remove_dir_all(dir); } @@ -6428,189 +6628,6 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } - /// Core prices the coinbase in ConnectBlock, after CheckBlock: a coinbase - /// over subsidy + fees is `bad-cb-amount` once the structure checks pass, - /// and a structure reject on the same block still wins. - #[test] - fn check_block_proposal_rejects_bad_cb_amount_after_structure() { - let (dir, hub) = tmp_hub(); - hub.ensure_genesis().unwrap(); - let mut block = hub - .assemble_block_to_script(ScriptBuf::from_bytes(vec![0x51]), vec![]) - .unwrap(); - let cb = &mut block.txdata[0].output[0].value; - *cb = Amount::from_sat(cb.to_sat() + 1); - let root_for_fat_cb = block.compute_merkle_root().unwrap(); - assert_eq!( - hub.check_block_proposal(&block).unwrap_err(), - "bad-txnmrklroot", - "structure checks run before the coinbase is priced" - ); - block.header.merkle_root = root_for_fat_cb; - assert_eq!( - hub.check_block_proposal(&block).unwrap_err(), - "bad-cb-amount" - ); - let _ = std::fs::remove_dir_all(dir); - } - - /// `Ok` carries the block fees, so a template provider can price the - /// coinbase: a coinbase at exactly subsidy + fees passes. - #[test] - fn check_block_proposal_returns_the_block_fees() { - let (dir, hub) = tmp_hub(); - let op_true = ScriptBuf::from_bytes(vec![0x51]); - hub.generate_to_script(101, op_true.clone(), vec![]) - .unwrap(); - let spend = mature_spend_tx(&hub, 1); - let mut block = hub.assemble_block_to_script(op_true, vec![spend]).unwrap(); - assert_eq!(hub.check_block_proposal(&block), Ok(10_000)); - let subsidy = rbitcoin_consensus::block_subsidy(102, &hub.params) as u64; - block.txdata[0].output[0].value = Amount::from_sat(subsidy + 10_000); - block.header.merkle_root = block.compute_merkle_root().unwrap(); - assert_eq!(hub.check_block_proposal(&block), Ok(10_000)); - let _ = std::fs::remove_dir_all(dir); - } - - /// An immature coinbase is not a fee source. Spending this block's - /// coinbase, or one still inside the maturity window, is Core's - /// `bad-txns-premature-spend-of-coinbase` even when that spend would - /// otherwise cover an overpaying coinbase. A bad merkle root still wins. - #[test] - fn check_block_proposal_rejects_immature_coinbase_before_fees() { - let (dir, hub) = tmp_hub(); - hub.ensure_genesis().unwrap(); - let op_true = ScriptBuf::from_bytes(vec![0x51]); - let mut block = hub - .assemble_block_to_script(op_true.clone(), vec![]) - .unwrap(); - let subsidy = block.txdata[0].output[0].value.to_sat(); - block.txdata[0].output[0].value = Amount::from_sat(subsidy + 1); - let cb_txid = block.txdata[0].compute_txid(); - block.txdata.push(spend_out(cb_txid, 0)); - assert_eq!( - hub.check_block_proposal(&block).unwrap_err(), - "bad-txnmrklroot", - "structure checks run before the immature spend" - ); - block.header.merkle_root = block.compute_merkle_root().unwrap(); - assert_eq!( - hub.check_block_proposal(&block).unwrap_err(), - "bad-txns-premature-spend-of-coinbase", - "a same-block coinbase spend must not inflate fees" - ); - - hub.generate_to_script(1, op_true.clone(), vec![]).unwrap(); - let prev = hub - .query - .reconstruct_block_at_height(Height(1)) - .unwrap() - .txdata[0] - .clone(); - let prev_value = prev.output[0].value.to_sat(); - let immature = spend_out(prev.compute_txid(), prev_value - 1_000); - let block = hub - .assemble_block_to_script(op_true.clone(), vec![immature]) - .unwrap(); - assert_eq!( - hub.check_block_proposal(&block).unwrap_err(), - "bad-txns-premature-spend-of-coinbase" - ); - let over = spend_out(prev.compute_txid(), prev_value + 1); - let block = hub.assemble_block_to_script(op_true, vec![over]).unwrap(); - assert_eq!( - hub.check_block_proposal(&block).unwrap_err(), - "bad-txns-premature-spend-of-coinbase", - "maturity is reported before bad-txns-in-belowout" - ); - let _ = std::fs::remove_dir_all(dir); - } - - /// A body that is still in the tx index after its block leaves the best - /// chain has no fence height. That is not a mature input. - #[test] - fn check_block_proposal_rejects_unconnected_prevout() { - let (dir, hub) = tmp_hub(); - let op_true = ScriptBuf::from_bytes(vec![0x51]); - hub.generate_to_script(1, op_true, vec![]).unwrap(); - let prev = hub - .query - .reconstruct_block_at_height(Height(1)) - .unwrap() - .txdata[0] - .clone(); - let value = prev.output[0].value.to_sat(); - hub.rewind_to_height(0).unwrap(); - let spend = spend_out(prev.compute_txid(), value - 1_000); - let block = hub - .assemble_block_to_script(ScriptBuf::from_bytes(vec![0x52]), vec![spend]) - .unwrap(); - assert_eq!( - hub.check_block_proposal(&block).unwrap_err(), - "bad-txns-inputs-missingorspent" - ); - let _ = std::fs::remove_dir_all(dir); - } - - /// A fan-out parent is decoded once per proposal check, not once per - /// child. Output `i` carries a distinct value so the fee total also - /// pins vout indexing. - #[test] - fn check_block_proposal_decodes_each_parent_once() { - let (dir, hub) = tmp_hub(); - let op_true = ScriptBuf::from_bytes(vec![0x51]); - hub.generate_to_script(101, op_true.clone(), vec![]) - .unwrap(); - let payout = 98_000_000u64; - let parent_value = |vout: u32| payout + 1_000 * u64::from(vout + 1); - let mut parent = mature_spend_tx(&hub, 1); - parent.output = (0..50) - .map(|vout| TxOut { - value: Amount::from_sat(parent_value(vout)), - script_pubkey: op_true.clone(), - }) - .collect(); - let parent_txid = parent.compute_txid(); - hub.generate_to_script(1, op_true.clone(), vec![parent]) - .unwrap(); - let children = (0..50) - .map(|vout| Transaction { - version: TxVersion::TWO, - lock_time: LockTime::ZERO, - input: vec![TxIn { - previous_output: OutPoint { - txid: parent_txid, - vout, - }, - script_sig: ScriptBuf::new(), - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Witness::new(), - }], - output: vec![TxOut { - value: Amount::from_sat(payout), - script_pubkey: op_true.clone(), - }], - }) - .collect(); - let block = hub.assemble_block_to_script(op_true, children).unwrap(); - let fees: u64 = (0..50).map(|vout| parent_value(vout) - payout).sum(); - let store = hub.query.store(); - let _ = store.sample_reset_tx_outs_decodes(); - let _ = store.sample_reset_tx_gets(); - assert_eq!(hub.check_block_proposal(&block), Ok(fees)); - assert_eq!( - store.sample_reset_tx_outs_decodes(), - 1, - "one packed outs decode per distinct parent" - ); - assert_eq!( - store.sample_reset_tx_gets(), - 0, - "the parent fk resolve does not decode the body" - ); - let _ = std::fs::remove_dir_all(dir); - } - fn spend_out(txid: bitcoin::Txid, value: u64) -> Transaction { Transaction { version: TxVersion::TWO, diff --git a/crates/rbitcoin-query/src/lib.rs b/crates/rbitcoin-query/src/lib.rs index 77ecc5e1d..e096c1c0d 100644 --- a/crates/rbitcoin-query/src/lib.rs +++ b/crates/rbitcoin-query/src/lib.rs @@ -1214,18 +1214,19 @@ impl Query { } /// Resolve txid → fk via durable `tx.head` (when the index is enabled). + /// Head probe plus `txid.body` verify; no packed body decode. + /// + /// `TipThenAny`: RPC / reconstruct may want a never-connected archive + /// row. A consensus-style check (confirm, spentness, proposal) wants + /// [`Self::tx_fk_by_txid_tip`]. /// /// ConfirmParentCache is keyed by create fk only (no process-local txid map). /// IBD thin edges carry stamped create_fk; cold/soft paths use durable head. fn lookup_tx_fk(&self, txid: &[u8; 32]) -> Result, QueryError> { - if self.tx_index_enabled() { - // body_txid verify only — avoid full packed decode on probe misses. - // TipThenAny: RPC / reconstruct may want a never-connected archive row. - if let Some(fk) = self.store.get_fk_by_txid(txid)? { - return Ok(Some(fk)); - } + if !self.tx_index_enabled() { + return Ok(None); } - Ok(None) + self.store.get_fk_by_txid(txid) } /// Public resolve by txid (durable head when index enabled). @@ -1233,7 +1234,8 @@ impl Query { self.lookup_tx_fk(txid) } - /// Confirm / spentness: connected instance only (height fence Some). + /// Confirm / spentness / block proposal: connected instance only (height + /// fence Some). pub fn tx_fk_by_txid_tip(&self, txid: &[u8; 32]) -> Result, QueryError> { if self.tx_index_enabled() { return self.store.get_fk_by_txid_tip(txid); diff --git a/crates/rbitcoin-store/src/store.rs b/crates/rbitcoin-store/src/store.rs index d6599f485..e825e8a9d 100644 --- a/crates/rbitcoin-store/src/store.rs +++ b/crates/rbitcoin-store/src/store.rs @@ -292,11 +292,6 @@ pub struct Store { /// Even while confirmed spentness is stable. Odd while a confirm annotate /// or a disconnect is publishing a change. utxo_view: std::sync::atomic::AtomicU64, - /// Packed `txout` decodes through [`Self::get_tx_meta_and_outputs`]. - tx_outs_decodes: std::sync::atomic::AtomicU64, - /// Whole-body decodes that keep only the meta: [`Self::get_tx`], and - /// each matching row in [`Self::resolve_txid`]. - tx_gets: std::sync::atomic::AtomicU64, #[cfg(debug_assertions)] tx_full_log: std::sync::Mutex>, #[cfg(debug_assertions)] @@ -400,8 +395,6 @@ impl Store { spend_annotate_from: std::sync::atomic::AtomicU64::new(0), spend_reorg_gen: std::sync::atomic::AtomicU64::new(0), utxo_view: std::sync::atomic::AtomicU64::new(0), - tx_outs_decodes: std::sync::atomic::AtomicU64::new(0), - tx_gets: std::sync::atomic::AtomicU64::new(0), spend_marker: std::sync::Mutex::new(()), path, cold_path, @@ -467,8 +460,6 @@ impl Store { spend_annotate_from: std::sync::atomic::AtomicU64::new(0), spend_reorg_gen: std::sync::atomic::AtomicU64::new(0), utxo_view: std::sync::atomic::AtomicU64::new(0), - tx_outs_decodes: std::sync::atomic::AtomicU64::new(0), - tx_gets: std::sync::atomic::AtomicU64::new(0), spend_marker: std::sync::Mutex::new(()), path, cold_path, @@ -738,19 +729,12 @@ impl Store { Ok(()) } - /// Meta by fk. Decodes the whole packed body and drops the ins and outs. - /// A parent-prevout reader wants [`Self::get_tx_meta_and_outputs`]. + /// Meta by fk: the same packed decode as [`Self::get_tx_meta_and_outputs`] + /// with the outs dropped, so a reader that needs both takes the latter once. pub fn get_tx(&self, fk: Fk) -> Result { - self.tx_gets - .fetch_add(1, std::sync::atomic::Ordering::Relaxed); self.txs.get(fk) } - /// Sample-and-reset meta-only whole-body decodes (instance stats). - pub fn sample_reset_tx_gets(&self) -> u64 { - self.tx_gets.swap(0, std::sync::atomic::Ordering::Relaxed) - } - pub fn reset_tx_full_gets(&self) { #[cfg(debug_assertions)] self.tx_full_log.lock().unwrap().clear(); @@ -819,17 +803,9 @@ impl Store { &self, fk: Fk, ) -> Result<(TxRecord, Vec), StoreError> { - self.tx_outs_decodes - .fetch_add(1, std::sync::atomic::Ordering::Relaxed); self.txs.get_meta_and_outputs(fk) } - /// Sample-and-reset packed outs decodes (instance stats). - pub fn sample_reset_tx_outs_decodes(&self) -> u64 { - self.tx_outs_decodes - .swap(0, std::sync::atomic::Ordering::Relaxed) - } - /// Page-grouped `txid.body` identity for scattered create fks. pub fn txids_get_many(&self, fks: &[Fk]) -> Result>, StoreError> { #[cfg(debug_assertions)] @@ -1062,8 +1038,7 @@ impl Store { fk } else { self.txs - .get_by_txid(out_txid)? - .map(|(fk, _)| fk) + .probe_body_match_fk(out_txid)? .ok_or(StoreError::NotFound)? }; self.put_spend_create(create_fk, out_index, spending_tx_fk, spending_vin)?; @@ -1094,13 +1069,10 @@ impl Store { txid: &[u8; 32], mode: TxidResolveMode, ) -> Result, StoreError> { - let all = self.txs.get_all_by_txid(txid)?; - self.tx_gets - .fetch_add(all.len() as u64, std::sync::atomic::Ordering::Relaxed); - if all.is_empty() { + let fks = self.txs.fks_by_txid(txid)?; + if fks.is_empty() { return Ok(None); } - let fks: Vec = all.iter().map(|(fk, _)| *fk).collect(); let heights = self.tx_height_get_batch(&fks)?; for (fk, h) in fks.iter().zip(heights.iter()) { if h.is_some() { @@ -1109,7 +1081,7 @@ impl Store { } match mode { TxidResolveMode::TipOnly => Ok(None), - TxidResolveMode::TipThenAny => Ok(Some(all[0].0)), + TxidResolveMode::TipThenAny => Ok(Some(fks[0])), } } @@ -1546,6 +1518,9 @@ impl Store { } /// True if any annotated spender for this outpoint is confirmed-strong. + /// The create is the connected row for `out_txid`: a strong spender + /// always hangs off it, so a newer never-connected row for the same txid + /// cannot hide a confirmed spend. pub fn has_confirmed_strong_spender( &self, out_txid: &[u8; 32], @@ -1562,7 +1537,7 @@ impl Store { out_index: u32, tip: Option, ) -> Result { - let Some((create_fk, _)) = self.txs.get_by_txid(out_txid)? else { + let Some(create_fk) = self.get_fk_by_txid_tip(out_txid)? else { return Ok(false); }; let mut found = false; @@ -1608,13 +1583,14 @@ impl Store { Ok(out) } - /// All annotated spenders (including non-strong / reorg history). + /// All annotated spenders on the connected create (including non-strong / + /// reorg history). A txid with no connected row has none. pub fn spenders_raw( &self, out_txid: &[u8; 32], out_index: u32, ) -> Result, StoreError> { - let Some((create_fk, _)) = self.txs.get_by_txid(out_txid)? else { + let Some(create_fk) = self.get_fk_by_txid_tip(out_txid)? else { return Ok(Vec::new()); }; let mut out = Vec::new(); @@ -2582,27 +2558,20 @@ mod tests { ); let fks = s.put_tx_full_batch_indexed(&[create], true).unwrap(); let create_fk = fks[0]; - let _ = s.sample_reset_tx_outs_decodes(); - let _ = s.sample_reset_tx_gets(); + let _ = s.txs.sample_reset_body_decodes(); let (meta, outs) = s.get_tx_meta_and_outputs(create_fk).unwrap(); assert_eq!(meta.txid, [10u8; 32]); assert_eq!(outs.len(), 2); assert_eq!( - s.sample_reset_tx_outs_decodes(), + s.txs.sample_reset_body_decodes(), 1, - "one packed outs decode" - ); - assert_eq!( - s.sample_reset_tx_gets(), - 0, - "an outs decode is not a get_tx" + "one packed body decode" ); assert_eq!(s.get_tx(create_fk).unwrap().txid, [10u8; 32]); - assert_eq!(s.sample_reset_tx_gets(), 1, "one meta-only body decode"); assert_eq!( - s.sample_reset_tx_outs_decodes(), - 0, - "a get_tx is not an outs decode" + s.txs.sample_reset_body_decodes(), + 1, + "get_tx is the same packed decode with the outs dropped" ); let full = s.get_tx_full(create_fk).unwrap(); assert_eq!(full.2.len(), 2); @@ -2613,14 +2582,9 @@ mod tests { assert_eq!(s.get_tx_meta_and_prevouts(create_fk).unwrap().1.len(), 1); assert_eq!(s.get_tx_meta_and_outputs(create_fk).unwrap().1.len(), 2); assert_eq!( - s.sample_reset_tx_outs_decodes(), + s.txs.sample_reset_body_decodes(), 1, - "full and prevout reads are not outs decodes; the sample resets" - ); - assert_eq!( - s.sample_reset_tx_gets(), - 0, - "full and prevout reads are not get_tx" + "one outs read decodes; full and prevout reads are not outs decodes" ); let mut span_hashes = Vec::new(); s.for_each_create_script_hashes_in_fk_span(create_fk.0, create_fk.0, |_fk, sh| { @@ -2636,14 +2600,18 @@ mod tests { span_hashes, expect, "fk-span script hashes must match per-fk decode" ); - let _ = s.sample_reset_tx_gets(); assert_eq!(s.get_fk_by_txid(&[10u8; 32]).unwrap(), Some(create_fk)); assert_eq!( - s.sample_reset_tx_gets(), + s.txs.sample_reset_body_decodes(), 0, - "fk resolve does not decode the body" + "fk resolve verifies txid.body only" ); assert_eq!(s.get_tx_by_txid(&[10u8; 32]).unwrap().unwrap().0, create_fk); + assert_eq!( + s.txs.sample_reset_body_decodes(), + 1, + "one decode for the returned record" + ); // Second tx spends create vout 0. let spend = ( @@ -2717,7 +2685,13 @@ mod tests { vec![OutputRecord::unspent(1, vec![0x51])], ); let spend3_fk = s.put_tx_full_batch_indexed(&[spend3], true).unwrap()[0]; + let _ = s.txs.sample_reset_body_decodes(); s.put_spend(&[10u8; 32], 0, spend3_fk, 0).unwrap(); + assert_eq!( + s.txs.sample_reset_body_decodes(), + 0, + "put_spend resolves the create by head probe only" + ); s.put_spend_batch(&[([10u8; 32], 1, spend_fk, 0)]).unwrap(); s.put_spend_create(create_fk, 1, spend2_fk, 0).unwrap(); let (soff, slen) = s.tx_spent_range(create_fk).unwrap(); @@ -2759,7 +2733,13 @@ mod tests { assert!(s .has_confirmed_strong_spender_create(create_fk, 0, Some((soff, slen))) .unwrap()); + let _ = s.txs.sample_reset_body_decodes(); assert!(s.has_confirmed_strong_spender(&[10u8; 32], 0).unwrap()); + assert_eq!( + s.txs.sample_reset_body_decodes(), + 0, + "the spentness probe resolves the create by head probe only" + ); let unspent = s .unspent_create_vouts(create_fk, &[0, 1], Some((soff, slen))) .unwrap(); @@ -2770,6 +2750,11 @@ mod tests { let strong_sp = s.spenders(&[10u8; 32], 0).unwrap(); assert_eq!(strong_sp.len(), 1); assert_eq!(strong_sp[0].spending_tx_fk, spend_fk); + assert_eq!( + s.txs.sample_reset_body_decodes(), + 0, + "spender walks resolve the create by head probe only" + ); // Batch helpers let ranges = s.tx_body_range_batch(&[create_fk, spend_fk]).unwrap(); @@ -3809,6 +3794,15 @@ mod tests { assert!(none.is_empty()); } + /// Two Class A rows for one txid, the older connected and the newer not + /// (a competing block at the same height, archived and then reorged + /// away): the tip resolvers pick the connected row and the any-row + /// resolve the newest. The last beat stamps a connected spender on the + /// connected row's vout 0 at height 1; the newer row has no spenders, so + /// the spentness probe and the spender walk must read the connected row. + /// A store unit, not a journey: the result is pure (which row each reader + /// picks), and reaching two rows for one txid through a session needs a + /// competing stale block, a fixture cost with no extra observation. #[test] fn resolve_txid_prefers_connected_over_newer_unconnected() { let dir = tmp(); @@ -3861,6 +3855,46 @@ mod tests { .get_fk_by_txid_batch_mode(&[txid], TxidResolveMode::TipThenAny) .unwrap(); assert_eq!(batch_any[0].1.map(|(f, _)| f), Some(old)); + + let spender = s + .put_tx_full_batch_indexed( + &[( + TxRecord { + txid: [0xBAu8; 32], + version: 1, + locktime: 0, + input_start_fk: Fk::NULL, + input_count: 1, + output_start_fk: Fk::NULL, + output_count: 1, + }, + vec![InputRecord { + prev_txid: txid, + create_fk: old, + prev_index: 0, + sequence: u32::MAX, + script_sig: vec![], + witness: vec![], + }], + vec![OutputRecord::unspent(1, vec![0x51])], + )], + true, + ) + .unwrap()[0]; + s.header_txs.put_range(Fk(2), spender, 1).unwrap(); + s.confirmed.set(Height(1), Fk(2)).unwrap(); + s.strong_tx.set_strong(spender, Fk(2)).unwrap(); + s.rebuild_height_fence().unwrap(); + s.put_spend_create(old, 0, spender, 0).unwrap(); + assert!( + s.has_confirmed_strong_spender(&txid, 0).unwrap(), + "spentness is probed on the connected create, not the newest row" + ); + assert_eq!( + s.spenders(&txid, 0).unwrap()[0].spending_tx_fk, + spender, + "spenders walk the connected create" + ); let _ = std::fs::remove_dir_all(&dir); } diff --git a/crates/rbitcoin-store/src/tx_table/mod.rs b/crates/rbitcoin-store/src/tx_table/mod.rs index bbfa5576b..c762bbd1f 100644 --- a/crates/rbitcoin-store/src/tx_table/mod.rs +++ b/crates/rbitcoin-store/src/tx_table/mod.rs @@ -585,6 +585,13 @@ pub struct TxTable { rebuild_seal_bits: u32, rebuild_workers: usize, prune_seqsigwit_mode: std::sync::atomic::AtomicBool, + /// Per-fk packed `txout` decodes for an outs reader + /// ([`Self::get_meta_and_outputs`], and [`Self::get`] through it), the + /// decode the mempool and block-proposal pins count. [`Self::get_full`] is + /// the confirm write stage's decoder and is not counted; span and pin + /// machines decode raw spans and are not counted. One Relaxed add per + /// decode. + body_decodes: std::sync::atomic::AtomicU64, } /// Structural-meta backend from env hierarchy. @@ -901,6 +908,7 @@ impl TxTable { input: crate::input::Input::create(seqsigwit_dir)?, secret, pending_head: pending_head::PendingHeadInserts::new(), + body_decodes: std::sync::atomic::AtomicU64::new(0), rebuild_seal_bits: seal_bits, rebuild_workers: workers, prune_seqsigwit_mode: std::sync::atomic::AtomicBool::new(false), @@ -1132,6 +1140,7 @@ impl TxTable { input, secret, pending_head: pending_head::PendingHeadInserts::new(), + body_decodes: std::sync::atomic::AtomicU64::new(0), rebuild_seal_bits: seal_bits, rebuild_workers: workers, prune_seqsigwit_mode: std::sync::atomic::AtomicBool::new(prune_seqsigwit_mode), @@ -1518,21 +1527,10 @@ impl TxTable { self.body.reserve_append(body_bytes, n_records) } + /// Meta by fk: the same packed decode as [`Self::get_meta_and_outputs`] + /// with the outs dropped (`txout` never carries ins). pub fn get(&self, fk: Fk) -> Result { - let pair = self - .create_loc_range_batch(&[fk])? - .into_iter() - .next() - .flatten() - .ok_or(StoreError::NotFound)?; - let raw = self - .body - .with_bytes_at(pair.txout.0, pair.txout.1, |b| Ok(b.to_vec()))?; - let (mut tx, _, _, _) = - decode_packed_tx_with_spender_rels_secret(&raw, pair.n_out, Some(&self.secret))?; - tx.txid = self.txids.get(fk)?; - self.overlay_stamped_n_in(fk, &mut tx)?; - Ok(tx) + self.get_meta_and_outputs(fk).map(|(tx, _)| tx) } /// Read create identity from **`txid.body`** (schema 13+). @@ -2219,6 +2217,8 @@ impl TxTable { let raw = self .body .with_bytes_at(pair.txout.0, pair.txout.1, |b| Ok(b.to_vec()))?; + self.body_decodes + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); let (mut tx, outs, _) = decode_packed_tx_outs_with_spender_rels_secret(&raw, pair.n_out, Some(&self.secret))?; tx.txid = self.txids.get(fk)?; @@ -2226,6 +2226,12 @@ impl TxTable { Ok((tx, outs)) } + /// Sample-and-reset per-fk packed body decodes (instance stats). + pub fn sample_reset_body_decodes(&self) -> u64 { + self.body_decodes + .swap(0, std::sync::atomic::Ordering::Relaxed) + } + /// Walk create_fks `first..=last` from a coalesced `txout.body` span (one idx /// walk, sequential body pread), yielding `script_hash` per out. /// @@ -2634,20 +2640,18 @@ impl TxTable { /// All Class A fks whose body txid equals `txid` (BIP30: more than one). /// /// Order is **newest-first** (deepest probe match first), matching - /// [`Self::probe_body_match_fk`]. - pub fn get_all_by_txid(&self, txid: &[u8; 32]) -> Result, StoreError> { - let mut out: Vec<(Fk, TxRecord)> = Vec::new(); + /// [`Self::probe_body_match_fk`]. Verifies `txid.body` only; no packed + /// decode. + pub(crate) fn fks_by_txid(&self, txid: &[u8; 32]) -> Result, StoreError> { + let mut out: Vec = Vec::new(); let mixed = self.secret.mix_txid(txid); // probe_candidates already open-first then sealed newest→oldest, deep-first within. let cands = self.head.probe_candidates(&mixed)?; for fk in cands { - if out.iter().any(|(have, _)| have.0 == fk.0) { - continue; - } - if self.body_txid(fk)? != *txid { + if out.contains(&fk) || self.body_txid(fk)? != *txid { continue; } - out.push((fk, self.get(fk)?)); + out.push(fk); } Ok(out) } diff --git a/crates/rbitcoin-store/src/tx_table/tests.rs b/crates/rbitcoin-store/src/tx_table/tests.rs index cdb51916e..bcadb81d5 100644 --- a/crates/rbitcoin-store/src/tx_table/tests.rs +++ b/crates/rbitcoin-store/src/tx_table/tests.rs @@ -3497,10 +3497,10 @@ fn bip30_duplicate_txid_seal_succeeds_and_resolves() { // Newest BIP30 create wins (deeper probe). let hit = t.probe_body_match_fk(&shared).unwrap(); assert_eq!(hit, Some(fks[1]), "newest same-txid create"); - let all = t.get_all_by_txid(&shared).unwrap(); + let all = t.fks_by_txid(&shared).unwrap(); assert_eq!(all.len(), 2, "both BIP30 creates body-verify"); - assert_eq!(all[0].0, fks[1]); - assert_eq!(all[1].0, fks[0]); + assert_eq!(all[0], fks[1]); + assert_eq!(all[1], fks[0]); let _ = std::fs::remove_dir_all(&dir); } @@ -4044,10 +4044,10 @@ fn rebuild_head_direct_mphf_bip30_newest_first() { let t = open_tiny_rebuild(&dir, 6, 2); let mut shared = [0u8; 32]; shared[0..8].copy_from_slice(&1u64.to_le_bytes()); - let all = t.get_all_by_txid(&shared).unwrap(); + let all = t.fks_by_txid(&shared).unwrap(); assert_eq!(all.len(), 2, "both BIP30 creates"); - assert_eq!(all[0].0, Fk(2), "newest first {all:?}"); - assert_eq!(all[1].0, Fk(1)); + assert_eq!(all[0], Fk(2), "newest first {all:?}"); + assert_eq!(all[1], Fk(1)); let _ = std::fs::remove_dir_all(&dir); } }