From 07b6bbe83369b64b177b683246742c6082cf1587 Mon Sep 17 00:00:00 2001 From: Gary Krause Date: Thu, 8 Oct 2026 18:20:45 -0400 Subject: [PATCH 1/5] store: count packed body decodes at the decode site The two proposal-check counters sat on `Store` beside `utxo_view`, the word every mempool accept loads, and only saw `Store::get_tx`, `Store::get_tx_meta_and_outputs`, and the `resolve_txid` candidates counted by hand at that call site. `TxTable::get` reached from the spentness probe and `put_spend` was never counted, so the proposal pin `tx_gets == 0` held while the check decoded one body per input. Move the counter into `TxTable`, one Relaxed add where the packed `txout` bytes are decoded for an outs reader (`get_meta_and_outputs`), off `Store` and the `utxo_view` word. It lives in `TxTable` rather than on a `Store` wrapper because the spentness probe, `put_spend`, and the spender walks decode inside `TxTable` (`get_by_txid`), where a `Store` word cannot see them. `get_full` is the confirm write stage's decoder and stays uncounted. The confirm path reaches the counted decoder only through `Store::get_tx_meta_and_outputs` (the scripthash collect cold branch and tip disconnect) and `Store::get_tx`, the wrappers every other reader uses; on master those wrappers did this same one Relaxed add on `Store::tx_outs_decodes` and `tx_gets`, so IBD does the word it already did and gains none. That cold decode is already in the `ibd: perf` inventory as `sh_collect_cold`, so no timer is added. The other callers of `TxTable::get`, `get_by_txid` and the unstamped-row arm of `get_meta_and_prevouts`, serve tests, scripthash history, and `getblockstats`, not an IBD stage. `TxTable::get` becomes the outs decoder with the outs dropped: the whole-body decoder it used returned an empty ins vector for `txout`, so the record is the same and `Store::get_tx`'s doc no longer implies a cheaper decode. The store and proposal pins now read the real counts: `put_spend`, the spentness probe, each spender walk, and every proposal input decode the create body once. Co-Authored-By: Claude Fable 5.1 --- changelog.d/proposal-parent-followup.md | 6 ++ crates/rbitcoin-net/src/chain.rs | 14 ++-- crates/rbitcoin-store/src/store.rs | 85 +++++++++-------------- crates/rbitcoin-store/src/tx_table/mod.rs | 34 +++++---- 4 files changed, 64 insertions(+), 75 deletions(-) create mode 100644 changelog.d/proposal-parent-followup.md diff --git a/changelog.d/proposal-parent-followup.md b/changelog.d/proposal-parent-followup.md new file mode 100644 index 000000000..fc2742fe8 --- /dev/null +++ b/changelog.d/proposal-parent-followup.md @@ -0,0 +1,6 @@ +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. diff --git a/crates/rbitcoin-net/src/chain.rs b/crates/rbitcoin-net/src/chain.rs index 8ae2f147d..5923d7f17 100644 --- a/crates/rbitcoin-net/src/chain.rs +++ b/crates/rbitcoin-net/src/chain.rs @@ -6595,18 +6595,12 @@ mod tests { 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(); + let _ = store.txs.sample_reset_body_decodes(); 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" + store.txs.sample_reset_body_decodes(), + 51, + "one packed body decode per distinct parent, plus one per input from the spentness probe" ); let _ = std::fs::remove_dir_all(dir); } diff --git a/crates/rbitcoin-store/src/store.rs b/crates/rbitcoin-store/src/store.rs index d6599f485..f84ffc70b 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)] @@ -1095,8 +1071,6 @@ impl Store { 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() { return Ok(None); } @@ -2582,27 +2556,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 +2580,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 +2598,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 +2683,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(), + 1, + "put_spend decodes the create body to find its fk" + ); 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 +2731,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(), + 1, + "the spentness probe decodes the create body to find its fk" + ); let unspent = s .unspent_create_vouts(create_fk, &[0, 1], Some((soff, slen))) .unwrap(); @@ -2770,6 +2748,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(), + 2, + "each spender walk decodes the create body to find its fk" + ); // Batch helpers let ranges = s.tx_body_range_batch(&[create_fk, spend_fk]).unwrap(); diff --git a/crates/rbitcoin-store/src/tx_table/mod.rs b/crates/rbitcoin-store/src/tx_table/mod.rs index bbfa5576b..1440ce43c 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. /// From 0d04a109fc39f1d0112787af02652314cd431e85 Mon Sep 17 00:00:00 2001 From: Gary Krause Date: Thu, 8 Oct 2026 18:23:01 -0400 Subject: [PATCH 2/5] store: resolve txids and spender creates without decoding the body `resolve_txid` went through `get_all_by_txid`, which decoded the packed body of every head candidate and then kept only the fks. `put_spend`, `has_confirmed_strong_spender_at`, and `spenders_raw` went through `get_by_txid` and discarded the record. Each `is_outpoint_spent` from mempool accept, gettxout, Electrum, and the block proposal check was one full body decode per input, and `get_fk_by_txid_tip` one per BIP30 candidate. `fks_by_txid` keeps the probe order and the `txid.body` verify and returns only the fks; it is in-crate, so it is `pub(crate)`. The three spend paths take `probe_body_match_fk` directly. The store pins flip to zero and the proposal pin reads one decode per distinct parent. Co-Authored-By: Claude Fable 5.1 --- changelog.d/proposal-parent-followup.md | 4 ++++ crates/rbitcoin-net/src/chain.rs | 4 ++-- crates/rbitcoin-store/src/store.rs | 26 ++++++++++----------- crates/rbitcoin-store/src/tx_table/mod.rs | 14 +++++------ crates/rbitcoin-store/src/tx_table/tests.rs | 12 +++++----- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/changelog.d/proposal-parent-followup.md b/changelog.d/proposal-parent-followup.md index fc2742fe8..75f3f5d47 100644 --- a/changelog.d/proposal-parent-followup.md +++ b/changelog.d/proposal-parent-followup.md @@ -4,3 +4,7 @@ Changed 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. diff --git a/crates/rbitcoin-net/src/chain.rs b/crates/rbitcoin-net/src/chain.rs index 5923d7f17..66750a1ba 100644 --- a/crates/rbitcoin-net/src/chain.rs +++ b/crates/rbitcoin-net/src/chain.rs @@ -6599,8 +6599,8 @@ mod tests { assert_eq!(hub.check_block_proposal(&block), Ok(fees)); assert_eq!( store.txs.sample_reset_body_decodes(), - 51, - "one packed body decode per distinct parent, plus one per input from the spentness probe" + 1, + "one packed body decode per distinct parent; the spentness probes and the fk resolve decode nothing" ); let _ = std::fs::remove_dir_all(dir); } diff --git a/crates/rbitcoin-store/src/store.rs b/crates/rbitcoin-store/src/store.rs index f84ffc70b..9041e82d4 100644 --- a/crates/rbitcoin-store/src/store.rs +++ b/crates/rbitcoin-store/src/store.rs @@ -1038,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)?; @@ -1070,11 +1069,10 @@ impl Store { txid: &[u8; 32], mode: TxidResolveMode, ) -> Result, StoreError> { - let all = self.txs.get_all_by_txid(txid)?; - 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() { @@ -1083,7 +1081,7 @@ impl Store { } match mode { TxidResolveMode::TipOnly => Ok(None), - TxidResolveMode::TipThenAny => Ok(Some(all[0].0)), + TxidResolveMode::TipThenAny => Ok(Some(fks[0])), } } @@ -1536,7 +1534,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.txs.probe_body_match_fk(out_txid)? else { return Ok(false); }; let mut found = false; @@ -1588,7 +1586,7 @@ impl Store { 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.txs.probe_body_match_fk(out_txid)? else { return Ok(Vec::new()); }; let mut out = Vec::new(); @@ -2687,8 +2685,8 @@ mod tests { s.put_spend(&[10u8; 32], 0, spend3_fk, 0).unwrap(); assert_eq!( s.txs.sample_reset_body_decodes(), - 1, - "put_spend decodes the create body to find its fk" + 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(); @@ -2735,8 +2733,8 @@ mod tests { assert!(s.has_confirmed_strong_spender(&[10u8; 32], 0).unwrap()); assert_eq!( s.txs.sample_reset_body_decodes(), - 1, - "the spentness probe decodes the create body to find its fk" + 0, + "the spentness probe resolves the create by head probe only" ); let unspent = s .unspent_create_vouts(create_fk, &[0, 1], Some((soff, slen))) @@ -2750,8 +2748,8 @@ mod tests { assert_eq!(strong_sp[0].spending_tx_fk, spend_fk); assert_eq!( s.txs.sample_reset_body_decodes(), - 2, - "each spender walk decodes the create body to find its fk" + 0, + "spender walks resolve the create by head probe only" ); // Batch helpers diff --git a/crates/rbitcoin-store/src/tx_table/mod.rs b/crates/rbitcoin-store/src/tx_table/mod.rs index 1440ce43c..c762bbd1f 100644 --- a/crates/rbitcoin-store/src/tx_table/mod.rs +++ b/crates/rbitcoin-store/src/tx_table/mod.rs @@ -2640,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); } } From 0684f180fcab20035461a9264b3b033c9fe0574f Mon Sep 17 00:00:00 2001 From: Gary Krause Date: Thu, 8 Oct 2026 18:37:01 -0400 Subject: [PATCH 3/5] net: keep only the outputs a block proposal spends from its parents The parent memo kept every decoded output of each distinct parent until the check returned, so a block spending one vout each of many fan-out parents held all of their outputs in RAM. It also resolved the parent with `TipThenAny`, which returns a row that exists only in a reorged-out block; that row was decoded before `coinbase_spend_is_immature` rejected it on the missing fence height. A pre-pass groups the block's spent vouts per parent (in-block creates excluded), resolves each parent on the connected chain only, decodes it once, and keeps just the spent vouts as `TxOut`s (value and script, which the script and sigop walk needs), O(block inputs). The connect loop still probes spentness per input and keeps its reject order; a parent that does not resolve contributes nothing, so its spends reject in block order as before. The RAM/CPU trade is on `proposal_connect`'s rustdoc rather than a comment at the map. The unconnected-prevout pin now also reads zero decodes. Co-Authored-By: Claude Fable 5.1 --- changelog.d/proposal-parent-followup.md | 4 + crates/rbitcoin-net/src/chain.rs | 115 ++++++++++++++++-------- crates/rbitcoin-query/src/lib.rs | 18 ++-- 3 files changed, 90 insertions(+), 47 deletions(-) diff --git a/changelog.d/proposal-parent-followup.md b/changelog.d/proposal-parent-followup.md index 75f3f5d47..322a5009e 100644 --- a/changelog.d/proposal-parent-followup.md +++ b/changelog.d/proposal-parent-followup.md @@ -8,3 +8,7 @@ Changed 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/crates/rbitcoin-net/src/chain.rs b/crates/rbitcoin-net/src/chain.rs index 66750a1ba..39ee7a3d9 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; @@ -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)) } @@ -6545,10 +6575,17 @@ mod tests { let block = hub .assemble_block_to_script(ScriptBuf::from_bytes(vec![0x52]), vec![spend]) .unwrap(); + let store = hub.query.store(); + let _ = store.txs.sample_reset_body_decodes(); assert_eq!( hub.check_block_proposal(&block).unwrap_err(), "bad-txns-inputs-missingorspent" ); + 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); } 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); From 6d2a9c6809f059b88f635b095b8f9dcf2b5da9fe Mon Sep 17 00:00:00 2001 From: Gary Krause Date: Thu, 8 Oct 2026 18:39:20 -0400 Subject: [PATCH 4/5] test: fold the block proposal checks into one chain Six `check_block_proposal_*` tests booted six hubs, two of them to height 101 (12.7 s together), to tell one story about one node: a tip child passes, a fat coinbase is `bad-cb-amount` after the structure checks, an immature coinbase is rejected before fees, a mature spend is priced, a fan-out parent decodes once, and a reorged-out row is not an input. One journey on one 101-block hub now carries every beat (7.8 s); the mutating beats observe the earlier ones, so it stays one test rather than a shared fixture. The unconnected-prevout test was a twin of the invalidate beat and is folded into it. The fold also adds the pin the memo was missing: a parent first seen through its unspent vout 1 does not vouch for its confirmed-spent vout 0, so spentness stays a per-input probe. The rustdoc names the 101-block chain that regtest maturity forces, and that one boot, as the reason the test runs past the two-second budget, says why the read-only beats ride the same hub, and no longer claims that the SV2 template provider runs this check; its only caller is `getblocktemplate` proposal mode. Co-Authored-By: Claude Fable 5.1 --- crates/rbitcoin-net/src/chain.rs | 374 +++++++++++++++---------------- 1 file changed, 180 insertions(+), 194 deletions(-) diff --git a/crates/rbitcoin-net/src/chain.rs b/crates/rbitcoin-net/src/chain.rs index 39ee7a3d9..c66d53b17 100644 --- a/crates/rbitcoin-net/src/chain.rs +++ b/crates/rbitcoin-net/src/chain.rs @@ -1774,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, @@ -6356,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); } @@ -6458,190 +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(); - let store = hub.query.store(); - let _ = store.txs.sample_reset_body_decodes(); - assert_eq!( - hub.check_block_proposal(&block).unwrap_err(), - "bad-txns-inputs-missingorspent" - ); - 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); - } - - /// 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.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" - ); - let _ = std::fs::remove_dir_all(dir); - } - fn spend_out(txid: bitcoin::Txid, value: u64) -> Transaction { Transaction { version: TxVersion::TWO, From 8655524f94ad41fac9f6bd1c3c2745c2aa4535d0 Mon Sep 17 00:00:00 2001 From: Gary Krause Date: Thu, 8 Oct 2026 20:46:31 -0400 Subject: [PATCH 5/5] store: probe spentness on the connected create only `has_confirmed_strong_spender_at` and `spenders_raw` resolved the create with `probe_body_match_fk`, the newest body match with the fence ignored, while the value side of the proposal check and of mempool accept resolves the same txid on the connected chain. With two Class A rows for one txid where the older row is connected and carries the spend stamp and the newer is not (a competing block at the same height, archived and then reorged away), `is_outpoint_spent` probed the newer row's empty slots and answered false, so a confirmed-spent output priced as spendable in `getblocktemplate` proposal mode, mempool accept, `gettxout`, and the Electrum and Esplora spent checks. Resolve the create with `get_fk_by_txid_tip` in both readers: head probe, `txid.body` verify, one fence read, no body decode. A strong spender always hangs off the connected create, so `spenders` and `spenders_at` lose nothing; a txid with no connected row has no best-chain spenders. Co-Authored-By: Claude Fable 5.1 --- changelog.d/proposal-spentness-connected.md | 6 +++ crates/rbitcoin-store/src/store.rs | 59 +++++++++++++++++++-- 2 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 changelog.d/proposal-spentness-connected.md 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-store/src/store.rs b/crates/rbitcoin-store/src/store.rs index 9041e82d4..e825e8a9d 100644 --- a/crates/rbitcoin-store/src/store.rs +++ b/crates/rbitcoin-store/src/store.rs @@ -1518,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], @@ -1534,7 +1537,7 @@ impl Store { out_index: u32, tip: Option, ) -> Result { - let Some(create_fk) = self.txs.probe_body_match_fk(out_txid)? else { + let Some(create_fk) = self.get_fk_by_txid_tip(out_txid)? else { return Ok(false); }; let mut found = false; @@ -1580,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.probe_body_match_fk(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(); @@ -3790,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(); @@ -3842,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); }