From 9532d8c4547730c2c67a55c7b5f5188571e30c06 Mon Sep 17 00:00:00 2001 From: Gary Krause Date: Thu, 8 Oct 2026 18:51:57 -0400 Subject: [PATCH 1/3] mempool: decode a confirmed parent once per coin lookup `chain_prevout` read the parent three times per `get_coin`: two packed outs decodes (`get_tx_by_txid` for the meta, `tx_output_at_fk` for the one output) and a `get_tx_full` read of input 0 through `tx_input_at_fk` to tell a coinbase apart (skipped on a seqsigwit cache hit), with a `block_tx_fks` fallback when the input record was pruned, and then read the fence a second time for the create height the resolve had just consulted. The block proposal check had just grown the same resolve-then-decode pair and the same per-input fence re-read. `Query::connected_tx_outputs` now owns that algorithm for both: a tip-only fk resolve that verifies `txid.body` only and returns the create's fence height from the same read (`Store::get_fk_by_txid_tip_height`), then one packed outs decode. `Query::is_coinbase_create` answers the coinbase question from the Class C coinbase table, the rule `coinbase_spend_is_immature` already used, so the input read and its pruned fallback go away along with the `block_tx_fks` meter that only counted that fallback. A height with no Class C coinbase entry answers "not a coinbase", the lenient posture `coinbase_spend_is_immature` already had; it is unchanged here. The proposal's parent memo carries the height, so `coinbase_spend_is_immature` no longer re-reads the fence per input and its no-height arm is gone: a parent is in the memo only if it resolved on the connected chain. `Store::resolve_txid` loses its last production caller and is deleted; its pins move to `get_fk_by_txid` and `get_fk_by_txid_tip`. The get_coin shape test pins one body decode per lookup on a private pad, spends vout 1 of a two-output parent so the fee pins the vout index, and spends the tip coinbase for `ImmatureCoinbase` so the Class C coinbase answer is pinned. Co-Authored-By: Claude Fable 5.1 --- changelog.d/mempool-prevout-once.md | 7 +++ crates/rbitcoin-net/src/chain.rs | 63 +++++++++------------- crates/rbitcoin-net/src/tx_relay.rs | 82 +++++++++++++++-------------- crates/rbitcoin-query/src/lib.rs | 25 +++++++++ crates/rbitcoin-store/src/store.rs | 69 ++++++++---------------- docs/concurrency.md | 2 +- 6 files changed, 121 insertions(+), 127 deletions(-) create mode 100644 changelog.d/mempool-prevout-once.md diff --git a/changelog.d/mempool-prevout-once.md b/changelog.d/mempool-prevout-once.md new file mode 100644 index 0000000000..abc4bba5eb --- /dev/null +++ b/changelog.d/mempool-prevout-once.md @@ -0,0 +1,7 @@ +Changed + +- **Mempool accept decodes a confirmed parent once per coin lookup.** Resolving + a spent coin reads the parent's packed outputs once; the fk resolve verifies + `txid.body` only and coinbase-ness comes from the block's first tx, not a + second decode of the parent's inputs. The `getblocktemplate` proposal check + shares that lookup. diff --git a/crates/rbitcoin-net/src/chain.rs b/crates/rbitcoin-net/src/chain.rs index c66d53b175..1ee95a733f 100644 --- a/crates/rbitcoin-net/src/chain.rs +++ b/crates/rbitcoin-net/src/chain.rs @@ -3262,9 +3262,10 @@ pub fn check_block_proposal_with( /// 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; +/// only the parent outputs this block spends with their parent's fk and +/// height, 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 and its fence height read once; /// spentness is still probed per input, never served from that map. fn proposal_connect( query: &Query, @@ -3316,8 +3317,8 @@ fn proposal_connect( } let txout = if let Some(o) = created.get(&op) { o.clone() - } else if let Some((fk, o)) = chain_txout(query, &parents, &op) { - if coinbase_spend_is_immature(query, *fk, height, maturity)? { + } else if let Some((fk, created_h, o)) = chain_txout(query, &parents, &op) { + if coinbase_spend_is_immature(query, *fk, *created_h, height, maturity)? { return Err("bad-txns-premature-spend-of-coinbase".into()); } o.clone() @@ -3359,9 +3360,9 @@ fn proposal_connect( /// pre-decoded entry in `parents`. fn chain_txout<'a>( query: &Query, - parents: &'a HashMap, + parents: &'a HashMap, op: &OutPoint, -) -> Option<&'a (Fk, TxOut)> { +) -> Option<&'a (Fk, u32, TxOut)> { if query .is_outpoint_spent(&op.txid.to_byte_array(), op.vout) .ok()? @@ -3372,15 +3373,17 @@ fn chain_txout<'a>( } /// 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. +/// one packed body decode per distinct parent +/// ([`Query::connected_tx_outputs`]), keeping only the spent vouts with the +/// parent's fk and create height. A row that exists only in a reorged-out +/// block resolves to nothing, as in Core. 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 { +) -> 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) { @@ -3391,7 +3394,11 @@ fn confirmed_parent_outputs( } 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 { + let Some((fk, created_h, all)) = query + .connected_tx_outputs(&txid.to_byte_array()) + .ok() + .flatten() + else { continue; }; for vout in vouts { @@ -3400,48 +3407,28 @@ fn confirmed_parent_outputs( 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.insert(OutPoint { txid, vout }, (fk, created_h, txout)); } } } outs } -/// 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()?; - Some((fk, outs)) -} - /// Core `CheckTxInputs`: the creating tx is the coinbase at its height and /// `spend_height` is still inside the maturity window. fn coinbase_spend_is_immature( query: &Query, create_fk: Fk, + created_h: u32, spend_height: u32, maturity: u32, ) -> Result { - let created_h = match query.store().tx_height_get(create_fk) { - Ok(Some(h)) => h, - // Not on the best-chain fence (reorged-out or never connected). - // `tx_height_get` does not return `NotFound`. - Ok(None) => return Err("bad-txns-inputs-missingorspent".into()), - Err(e) => return Err(e.to_string()), - }; if spend_height >= created_h.saturating_add(maturity) { return Ok(false); } - let coinbases = query - .store() - .coinbase_fk_at_heights(&[created_h]) - .map_err(|e| e.to_string())?; - Ok(coinbases.get(&created_h).copied() == Some(create_fk)) + query + .is_coinbase_create(create_fk, created_h) + .map_err(|e| e.to_string()) } pub fn accept_block_header_nodos_log(hash: impl std::fmt::Display) -> String { diff --git a/crates/rbitcoin-net/src/tx_relay.rs b/crates/rbitcoin-net/src/tx_relay.rs index 4416609640..71daa1bd53 100644 --- a/crates/rbitcoin-net/src/tx_relay.rs +++ b/crates/rbitcoin-net/src/tx_relay.rs @@ -216,7 +216,6 @@ pub struct QueryUtxoProvider<'a> { pub query: &'a Query, need_create_mtp: AtomicBool, meter_get_coin: Option<&'a AtomicU64>, - meter_block_tx_fks: Option<&'a AtomicU64>, meter_create_mtp: Option<&'a AtomicU64>, } @@ -226,7 +225,6 @@ impl<'a> QueryUtxoProvider<'a> { query, need_create_mtp: AtomicBool::new(false), meter_get_coin: None, - meter_block_tx_fks: None, meter_create_mtp: None, } } @@ -254,10 +252,8 @@ impl UtxoProvider for QueryUtxoProvider<'_> { m.fetch_add(1, Ordering::Relaxed); } let tid = op.txid.to_byte_array(); - let Some((fk, rec)) = self.query.get_tx_by_txid(&tid).ok().flatten() else { - return ChainPrevout::Unknown; - }; - let Some(create_height) = self.query.store().tx_height_get(fk).ok().flatten() else { + let Some((fk, create_height, outs)) = self.query.connected_tx_outputs(&tid).ok().flatten() + else { return ChainPrevout::Unknown; }; let Some(tip) = self.query.tip_height().map(|h| h.0) else { @@ -271,12 +267,7 @@ impl UtxoProvider for QueryUtxoProvider<'_> { Ok(false) => {} Err(_) => return ChainPrevout::KnownUnavailable, } - let Some(out) = self - .query - .tx_output_at_fk(fk, op.vout) - .ok() - .or_else(|| self.query.tx_output(&rec, op.vout).ok()) - else { + let Some(out) = outs.into_iter().nth(op.vout as usize) else { return ChainPrevout::KnownUnavailable; }; let value = if out.value < 0 { @@ -284,21 +275,10 @@ impl UtxoProvider for QueryUtxoProvider<'_> { } else { Amount::from_sat(out.value as u64) }; - let is_coinbase = match self.query.tx_input_at_fk(fk, &rec, 0) { - Ok(i) => i.is_coinbase() || i.prev_index == u32::MAX, - Err(_) => { - if let Some(m) = self.meter_block_tx_fks { - m.fetch_add(1, Ordering::Relaxed); - } - create_height > 0 - && self - .query - .block_tx_fks(Height(create_height)) - .ok() - .and_then(|fks| fks.first().copied()) - == Some(fk) - } - }; + let is_coinbase = self + .query + .is_coinbase_create(fk, create_height) + .unwrap_or(false); let create_mtp = if create_height == 0 || !self.need_create_mtp.load(Ordering::Relaxed) { 0 } else { @@ -465,8 +445,6 @@ pub struct MempoolPerfSample { pub tip_mtp: u64, /// `QueryUtxoProvider::get_coin` calls on the hub provider. pub get_coin: u64, - /// `block_tx_fks` from `get_coin` (missing input-0 record only). - pub get_coin_block_tx_fks: u64, /// Create-block MTP from `get_coin` (BIP68 time-lock spends only). pub get_coin_create_mtp: u64, } @@ -605,7 +583,6 @@ pub struct MempoolHub { meter_expire_full_scans: AtomicU64, meter_tip_mtp: AtomicU64, meter_get_coin: AtomicU64, - meter_get_coin_block_tx_fks: AtomicU64, meter_get_coin_create_mtp: AtomicU64, /// Live mempool txs by Electrum scripthash (updated on accept/remove). sh_index: Mutex, @@ -813,7 +790,6 @@ impl MempoolHub { meter_expire_full_scans: AtomicU64::new(0), meter_tip_mtp: AtomicU64::new(0), meter_get_coin: AtomicU64::new(0), - meter_get_coin_block_tx_fks: AtomicU64::new(0), meter_get_coin_create_mtp: AtomicU64::new(0), sh_index: Mutex::new(MempoolShIndex::new()), unbroadcast: Mutex::new(unbroadcast), @@ -976,7 +952,6 @@ impl MempoolHub { fn utxo_provider(&self) -> QueryUtxoProvider<'_> { let mut p = QueryUtxoProvider::new(self.query.as_ref()); p.meter_get_coin = Some(&self.meter_get_coin); - p.meter_block_tx_fks = Some(&self.meter_get_coin_block_tx_fks); p.meter_create_mtp = Some(&self.meter_get_coin_create_mtp); p } @@ -1130,7 +1105,6 @@ impl MempoolHub { expire_full_scans: self.meter_expire_full_scans.swap(0, Ordering::Relaxed), tip_mtp: self.meter_tip_mtp.swap(0, Ordering::Relaxed), get_coin: self.meter_get_coin.swap(0, Ordering::Relaxed), - get_coin_block_tx_fks: self.meter_get_coin_block_tx_fks.swap(0, Ordering::Relaxed), get_coin_create_mtp: self.meter_get_coin_create_mtp.swap(0, Ordering::Relaxed), } } @@ -5055,10 +5029,14 @@ mod tests { let _ = std::fs::remove_dir_all(&mp); } - /// Non-coinbase, no BIP68 time-lock: no `block_tx_fks` and no create MTP. + /// Non-coinbase, no BIP68 time-lock: one packed body decode per coin + /// lookup and no create MTP. The parent has two outputs with distinct + /// values and the child spends vout 1, so the fee pins the vout index. + /// A spend of the tip coinbase is `ImmatureCoinbase`: coinbase-ness comes + /// from the Class C coinbase table, not the parent's inputs. /// A satisfied time-lock spend must survive `evict_after_reorg`. #[test] - fn get_coin_skips_block_tx_fks_and_mtp_without_time_lock() { + fn get_coin_decodes_once_and_skips_mtp_without_time_lock() { let (store, owned_q, owned_cbs) = copy_maturity_pad(3); let q = &owned_q; let cbs = owned_cbs.as_slice(); @@ -5073,7 +5051,18 @@ mod tests { let tip = bitcoin::BlockHash::from_byte_array(rec.hash); let next = tip_h + 1; let spk = ScriptBuf::from_bytes(vec![0x51]); - let confirmed = spend_true(cbs[0], 1_000, spk.clone()); + let mut confirmed = spend_true(cbs[0], 1_000, spk.clone()); + let total = confirmed.output[0].value.to_sat(); + confirmed.output = vec![ + TxOut { + value: Amount::from_sat(5_000), + script_pubkey: spk.clone(), + }, + TxOut { + value: Amount::from_sat(total - 5_000), + script_pubkey: spk.clone(), + }, + ]; let b = rbitcoin_consensus::mine_regtest_paying( tip, rec.timestamp + 600, @@ -5093,18 +5082,24 @@ mod tests { input: vec![TxIn { previous_output: OutPoint { txid: confirmed.compute_txid(), - vout: 0, + vout: 1, }, script_sig: ScriptBuf::new(), sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, witness: Witness::new(), }], output: vec![TxOut { - value: Amount::from_sat(confirmed.output[0].value.to_sat() - 1_000), + value: Amount::from_sat(confirmed.output[1].value.to_sat() - 1_000), script_pubkey: spk.clone(), }], }; + let _ = q.store().txs.sample_reset_body_decodes(); hub.accept_tx(&child).expect("non-coinbase chain spend"); + assert_eq!( + hub.get_live_meta(&child.compute_txid()).map(|(fee, _)| fee), + Some(1_000), + "the coin is vout 1's value; vout 0 holds 5_000 sat" + ); let s = hub.sample_reset_perf(); assert_eq!( s.get_coin, 2, @@ -5112,13 +5107,20 @@ mod tests { s.get_coin ); assert_eq!( - s.get_coin_block_tx_fks, 0, - "non-coinbase get_coin must not call block_tx_fks" + q.store().txs.sample_reset_body_decodes(), + s.get_coin, + "one packed body decode per get_coin; the fk resolve verifies txid.body only" ); assert_eq!( s.get_coin_create_mtp, 0, "no BIP68 time-lock must not compute create MTP" ); + let immature = spend_true(b.txdata[0].compute_txid(), 1_000, spk.clone()); + let err = hub.accept_tx(&immature).unwrap_err(); + assert!( + matches!(err, AcceptError::ImmatureCoinbase), + "the tip coinbase is inside the maturity window, got {err}" + ); assert!( !hub.scripthash_mempool(&script_hash(spk.as_bytes())) .is_empty(), diff --git a/crates/rbitcoin-query/src/lib.rs b/crates/rbitcoin-query/src/lib.rs index e096c1c0d1..48c7f8a108 100644 --- a/crates/rbitcoin-query/src/lib.rs +++ b/crates/rbitcoin-query/src/lib.rs @@ -2065,6 +2065,31 @@ impl Query { outs.get(vout as usize).cloned().ok_or(StoreError::NotFound) } + /// A parent on the connected chain for a prevout reader: tip-only fk + /// resolve (`txid.body` verify, no decode) that also yields the create + /// height, then one packed outs decode for every output. `None` when + /// `txid` has no row on the best chain or the tx index is off. Mempool + /// accept and the block proposal check share this. + pub fn connected_tx_outputs( + &self, + txid: &[u8; 32], + ) -> Result)>, QueryError> { + if !self.tx_index_enabled() { + return Ok(None); + } + let Some((fk, height)) = self.store.get_fk_by_txid_tip_height(txid)? else { + return Ok(None); + }; + let (_, outs) = self.store.get_tx_meta_and_outputs(fk)?; + Ok(Some((fk, height, outs))) + } + + /// Core's coinbase test for a create on the best chain: `fk` is the first + /// tx of the block at `height`. Class C dense tables only; no body read. + pub fn is_coinbase_create(&self, fk: Fk, height: u32) -> Result { + Ok(self.store.coinbase_fk_at_heights(&[height])?.get(&height) == Some(&fk)) + } + /// Strong (best-chain confirmed) spenders only. pub fn spenders( &self, diff --git a/crates/rbitcoin-store/src/store.rs b/crates/rbitcoin-store/src/store.rs index e825e8a9d4..6046217297 100644 --- a/crates/rbitcoin-store/src/store.rs +++ b/crates/rbitcoin-store/src/store.rs @@ -1063,28 +1063,6 @@ impl Store { Ok(self.fence().get_batch(fks)) } - /// Among Class A rows for `txid`, prefer a connected fk (fence height Some). - pub fn resolve_txid( - &self, - txid: &[u8; 32], - mode: TxidResolveMode, - ) -> Result, StoreError> { - let fks = self.txs.fks_by_txid(txid)?; - if fks.is_empty() { - return Ok(None); - } - let heights = self.tx_height_get_batch(&fks)?; - for (fk, h) in fks.iter().zip(heights.iter()) { - if h.is_some() { - return Ok(Some(*fk)); - } - } - match mode { - TxidResolveMode::TipOnly => Ok(None), - TxidResolveMode::TipThenAny => Ok(Some(fks[0])), - } - } - /// Coinbase Class A fk for each confirmed height (or `None` if tip/header missing). /// /// Uses only Class C dense tables (`confirmed` + `header_txs_first`) — **no** @@ -1139,7 +1117,22 @@ impl Store { /// Confirm / consensus: connected instance only. pub fn get_fk_by_txid_tip(&self, txid: &[u8; 32]) -> Result, StoreError> { - self.resolve_txid(txid, TxidResolveMode::TipOnly) + Ok(self.get_fk_by_txid_tip_height(txid)?.map(|(fk, _)| fk)) + } + + /// The connected instance and its fence height: verifies each head + /// candidate's body txid and does one batched fence read, no body decode. + /// A prevout reader that needs the create height takes this once. + pub fn get_fk_by_txid_tip_height( + &self, + txid: &[u8; 32], + ) -> Result, StoreError> { + let fks = self.txs.fks_by_txid(txid)?; + let heights = self.tx_height_get_batch(&fks)?; + Ok(fks + .into_iter() + .zip(heights) + .find_map(|(fk, h)| Some((fk, h?)))) } /// Batch head resolve for plan stamp: txid → (fk, body_range). @@ -3825,27 +3818,16 @@ mod tests { .put_tx_full_batch_indexed(&[(rec(2), vec![], out)], true) .unwrap()[0]; assert_ne!(old, new); - assert_eq!( - s.resolve_txid(&txid, TxidResolveMode::TipThenAny).unwrap(), - Some(new) - ); assert_eq!(s.get_fk_by_txid(&txid).unwrap(), Some(new)); - assert_eq!( - s.resolve_txid(&txid, TxidResolveMode::TipOnly).unwrap(), - None - ); + assert_eq!(s.get_fk_by_txid_tip(&txid).unwrap(), None); s.header_txs.put_range(Fk(1), old, 1).unwrap(); s.confirmed.set(Height(0), Fk(1)).unwrap(); s.rebuild_height_fence().unwrap(); assert_eq!( - s.resolve_txid(&txid, TxidResolveMode::TipOnly).unwrap(), + s.get_fk_by_txid_tip(&txid).unwrap(), Some(old), "connected older row must win" ); - assert_eq!( - s.resolve_txid(&txid, TxidResolveMode::TipThenAny).unwrap(), - Some(old) - ); assert_eq!(s.get_fk_by_txid(&txid).unwrap(), Some(old)); let batch_tip = s .get_fk_by_txid_batch_mode(&[txid], TxidResolveMode::TipOnly) @@ -4002,10 +3984,7 @@ mod tests { ); // Neither connected yet: newest unconnected (hot) for TipThenAny. - assert_eq!( - s.resolve_txid(&txid, TxidResolveMode::TipThenAny).unwrap(), - Some(new) - ); + assert_eq!(s.get_fk_by_txid(&txid).unwrap(), Some(new)); assert_eq!( s.get_fk_by_txid_batch_mode(&[txid], TxidResolveMode::TipThenAny) .unwrap()[0] @@ -4025,14 +4004,8 @@ mod tests { s.confirmed.set(Height(0), Fk(1)).unwrap(); s.rebuild_height_fence().unwrap(); - assert_eq!( - s.resolve_txid(&txid, TxidResolveMode::TipOnly).unwrap(), - Some(old) - ); - assert_eq!( - s.resolve_txid(&txid, TxidResolveMode::TipThenAny).unwrap(), - Some(old) - ); + assert_eq!(s.get_fk_by_txid_tip(&txid).unwrap(), Some(old)); + assert_eq!(s.get_fk_by_txid(&txid).unwrap(), Some(old)); let batch_tip = s .get_fk_by_txid_batch_mode(&[txid], TxidResolveMode::TipOnly) .unwrap(); diff --git a/docs/concurrency.md b/docs/concurrency.md index 7b465d4595..292a3750be 100644 --- a/docs/concurrency.md +++ b/docs/concurrency.md @@ -89,7 +89,7 @@ spend annotations. | `tx.head` segment seal | Roll opens the next OA immediately; BDZ+fuse8 runs on a sidecar. Lookup probes every unsealed OA until publish. Write joins the sidecar only on the *next* roll, `flush`, or `Drop` (not on the rolling insert). | | `header.head` overflow | Insert past 7/8 rolls `header.head.gN` (new empty file). Occupied rewrite is open-only: undersized single gen writes `header.head.grow` then rename. | | `ChainHub::confirmed` | `RwLock` for O(1) `has_block` (IBD assign path) | -| `MempoolHub::inner` | One `RwLock`. **Write** is graph mutation + durable slot append only (microseconds at ~12k live). No Query / script verify / compact reconstruct under write. The commit recheck of a confirmed coin runs before the write lock unless a confirm or disconnect holds the UTXO view. Prepare may hold a **read** across Query. Blocking `read`/`write` is `assert_not_reactor`. Session INV/getdata/compact use `try_read`; miss = skip that item (re-getdata / notfound / reconstruct without those txs). Compact fill siphashes live txid/wtxid and clones **matching** bodies only — never `list_live()`. Handshake `FeeFilter` reads the published fee-floor **atomic**, not `inner`. Age-INV log and min live `accept_at` are **not** `inner` (same class as `accept_at`). Accept caches `{tip_header_fk, height, mtp}` until the header fk changes. `get_coin` computes create MTP only for BIP68 **time** locks and calls `block_tx_fks` only when input-0 is missing. `index_txid` hashes the admitted tx + prepare prevouts (no second Query). Do not CoW a 12k-txid presence set on every accept. Lock order: `inner` → `sh_index` / `unbroadcast` / `relay_*`. | +| `MempoolHub::inner` | One `RwLock`. **Write** is graph mutation + durable slot append only (microseconds at ~12k live). No Query / script verify / compact reconstruct under write. The commit recheck of a confirmed coin runs before the write lock unless a confirm or disconnect holds the UTXO view. Prepare may hold a **read** across Query. Blocking `read`/`write` is `assert_not_reactor`. Session INV/getdata/compact use `try_read`; miss = skip that item (re-getdata / notfound / reconstruct without those txs). Compact fill siphashes live txid/wtxid and clones **matching** bodies only — never `list_live()`. Handshake `FeeFilter` reads the published fee-floor **atomic**, not `inner`. Age-INV log and min live `accept_at` are **not** `inner` (same class as `accept_at`). Accept caches `{tip_header_fk, height, mtp}` until the header fk changes. `get_coin` resolves the parent on the connected chain, decodes its packed outputs once, reads coinbase-ness from the Class C coinbase table, and computes create MTP only for BIP68 **time** locks. `index_txid` hashes the admitted tx + prepare prevouts (no second Query). Do not CoW a 12k-txid presence set on every accept. Lock order: `inner` → `sh_index` / `unbroadcast` / `relay_*`. | There is **no** global “pause queries during confirm write.” Tip-as-commit + `is_confirmed_strong` define query visibility ([`crash-recovery.md`](./crash-recovery.md)). From 12f10eb94afd6d59852eba8758b84c5ff679fe0c Mon Sep 17 00:00:00 2001 From: "rearden-grok[bot]" <317016512+rearden-grok[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:02:33 -0700 Subject: [PATCH 2/3] net: reject a proposal spend whose create left the fence The parent memo caches the create height. A disconnect before the input loop leaves spentness false while the memo still holds the output, so a cached height already past maturity priced a coin that is no longer on the best chain. Re-read the fence and treat a missing or moved height as bad-txns-inputs-missingorspent. Co-authored-by: Cursor --- changelog.d/mempool-prevout-once.md | 4 ++- crates/rbitcoin-net/src/chain.rs | 46 +++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/changelog.d/mempool-prevout-once.md b/changelog.d/mempool-prevout-once.md index abc4bba5eb..06dd89dc66 100644 --- a/changelog.d/mempool-prevout-once.md +++ b/changelog.d/mempool-prevout-once.md @@ -4,4 +4,6 @@ Changed a spent coin reads the parent's packed outputs once; the fk resolve verifies `txid.body` only and coinbase-ness comes from the block's first tx, not a second decode of the parent's inputs. The `getblocktemplate` proposal check - shares that lookup. + shares that lookup and re-reads the create's fence height before spending + the cached output, so a disconnect during the check cannot price a coin + that has left the best chain. diff --git a/crates/rbitcoin-net/src/chain.rs b/crates/rbitcoin-net/src/chain.rs index 1ee95a733f..2d46b00457 100644 --- a/crates/rbitcoin-net/src/chain.rs +++ b/crates/rbitcoin-net/src/chain.rs @@ -3265,8 +3265,10 @@ pub fn check_block_proposal_with( /// only the parent outputs this block spends with their parent's fk and /// height, 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 and its fence height read once; -/// spentness is still probed per input, never served from that map. +/// parent's packed body is decoded once. The pre-pass reads the fence once +/// to cache the create height; each confirmed spend reads that fence again, +/// and a missing or moved height is `bad-txns-inputs-missingorspent`. +/// Spentness is still probed per input, never served from that map. fn proposal_connect( query: &Query, block: &Block, @@ -3416,6 +3418,11 @@ fn confirmed_parent_outputs( /// Core `CheckTxInputs`: the creating tx is the coinbase at its height and /// `spend_height` is still inside the maturity window. +/// +/// `created_h` is the height the parent memo cached. The fence is read again +/// here: a disconnect after that cache leaves no height, and that spend is +/// `bad-txns-inputs-missingorspent` even when the cached height is already +/// past maturity. fn coinbase_spend_is_immature( query: &Query, create_fk: Fk, @@ -3423,6 +3430,11 @@ fn coinbase_spend_is_immature( spend_height: u32, maturity: u32, ) -> Result { + match query.store().tx_height_get(create_fk) { + Ok(Some(h)) if h == created_h => {} + Ok(_) => return Err("bad-txns-inputs-missingorspent".into()), + Err(e) => return Err(e.to_string()), + } if spend_height >= created_h.saturating_add(maturity) { return Ok(false); } @@ -6343,6 +6355,36 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + /// A cached create height is not spendable once the fence drops that fk. + /// The maturity short-circuit must not run on a height the best chain no + /// longer has: a disconnect between the parent memo and this check is + /// `bad-txns-inputs-missingorspent`. + #[test] + fn coinbase_spend_rejects_cached_height_after_disconnect() { + let (dir, hub) = tmp_hub(); + let op_true = ScriptBuf::from_bytes(vec![0x51]); + hub.generate_to_script(1, op_true, vec![]).unwrap(); + let fk = hub.query.block_tx_fks(Height(1)).unwrap()[0]; + let created_h = hub.query.store().tx_height_get(fk).unwrap().unwrap(); + assert_eq!(created_h, 1); + hub.invalidate_block(hub.tip_hash().unwrap()).unwrap(); + assert_eq!( + hub.query.store().tx_height_get(fk).unwrap(), + None, + "invalidate drops the create off the fence" + ); + let err = coinbase_spend_is_immature( + hub.query.as_ref(), + fk, + created_h, + created_h.saturating_add(100), + 100, + ) + .expect_err("a disconnected create is not a mature input"); + assert_eq!(err, "bad-txns-inputs-missingorspent"); + let _ = std::fs::remove_dir_all(dir); + } + /// 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 From 6d164cb19e90fdd6cbc7de002a74e7c262eaef2c Mon Sep 17 00:00:00 2001 From: "rearden-grok[bot]" <317016512+rearden-grok[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:06:44 -0700 Subject: [PATCH 3/3] net: fail closed when the mempool coinbase read errors unwrap_or(false) treated a Class C failure as "not a coinbase", so an immature coinbase spend could be admitted. Ok(false) stays the lenient answer for a height with no coinbase row; Err leaves the coin unavailable, the same way a spentness read error already does. Co-authored-by: Cursor --- changelog.d/mempool-prevout-once.md | 3 +- crates/rbitcoin-net/src/tx_relay.rs | 74 +++++++++++++++++++++++++++-- docs/concurrency.md | 2 +- 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/changelog.d/mempool-prevout-once.md b/changelog.d/mempool-prevout-once.md index 06dd89dc66..810bcc6431 100644 --- a/changelog.d/mempool-prevout-once.md +++ b/changelog.d/mempool-prevout-once.md @@ -6,4 +6,5 @@ Changed second decode of the parent's inputs. The `getblocktemplate` proposal check shares that lookup and re-reads the create's fence height before spending the cached output, so a disconnect during the check cannot price a coin - that has left the best chain. + that has left the best chain. A failed coinbase-table read leaves the + mempool coin unavailable instead of treating it as a non-coinbase. diff --git a/crates/rbitcoin-net/src/tx_relay.rs b/crates/rbitcoin-net/src/tx_relay.rs index 71daa1bd53..eebe5226e2 100644 --- a/crates/rbitcoin-net/src/tx_relay.rs +++ b/crates/rbitcoin-net/src/tx_relay.rs @@ -217,6 +217,10 @@ pub struct QueryUtxoProvider<'a> { need_create_mtp: AtomicBool, meter_get_coin: Option<&'a AtomicU64>, meter_create_mtp: Option<&'a AtomicU64>, + /// Test seam for the Class C coinbase read. `0` uses the store. + /// `1` is `Ok(false)`, `2` is `Ok(true)`, anything else is a read error. + #[cfg(test)] + coinbase_read: std::sync::atomic::AtomicU8, } impl<'a> QueryUtxoProvider<'a> { @@ -226,8 +230,29 @@ impl<'a> QueryUtxoProvider<'a> { need_create_mtp: AtomicBool::new(false), meter_get_coin: None, meter_create_mtp: None, + #[cfg(test)] + coinbase_read: std::sync::atomic::AtomicU8::new(0), } } + + /// Class C coinbase bit for `fk` at `create_height`. + /// + /// `Ok(false)` is a height with no coinbase row. `Err` is a failed read; + /// the caller leaves the coin unavailable. + fn coinbase_flag( + &self, + fk: Fk, + create_height: u32, + ) -> Result { + #[cfg(test)] + match self.coinbase_read.load(Ordering::Relaxed) { + 0 => {} + 1 => return Ok(false), + 2 => return Ok(true), + _ => return Err(rbitcoin_store::StoreError::Corrupt("coinbase table")), + } + self.query.is_coinbase_create(fk, create_height) + } } impl UtxoProvider for QueryUtxoProvider<'_> { @@ -275,10 +300,10 @@ impl UtxoProvider for QueryUtxoProvider<'_> { } else { Amount::from_sat(out.value as u64) }; - let is_coinbase = self - .query - .is_coinbase_create(fk, create_height) - .unwrap_or(false); + let is_coinbase = match self.coinbase_flag(fk, create_height) { + Ok(flag) => flag, + Err(_) => return ChainPrevout::KnownUnavailable, + }; let create_mtp = if create_height == 0 || !self.need_create_mtp.load(Ordering::Relaxed) { 0 } else { @@ -5029,6 +5054,47 @@ mod tests { let _ = std::fs::remove_dir_all(&mp); } + /// A Class C coinbase read that errors is not "not a coinbase". The + /// genesis coinbase is unspent; forcing that read to fail must make the + /// prevout unavailable, while `Ok(false)` still returns the coin. + #[test] + fn coinbase_table_error_makes_the_prevout_unavailable() { + use rbitcoin_consensus::{accept_and_connect_block, ChainParams, Milestone}; + use rbitcoin_mempool::{ChainPrevout, UtxoProvider}; + use rbitcoin_primitives::Height; + + let dir = tmp(); + let q = Query::open_or_create_tiny(&dir).unwrap(); + let params = ChainParams::regtest(); + let genesis = bitcoin::blockdata::constants::genesis_block(bitcoin::Network::Regtest); + accept_and_connect_block(&q, ¶ms, Height::GENESIS, &genesis, Milestone::NONE).unwrap(); + let op = OutPoint { + txid: genesis.txdata[0].compute_txid(), + vout: 0, + }; + let provider = QueryUtxoProvider::new(&q); + provider.coinbase_read.store(3, Ordering::Relaxed); + assert!( + matches!(provider.chain_prevout(&op), ChainPrevout::KnownUnavailable), + "a failed Class C coinbase read must not admit the coin" + ); + provider.coinbase_read.store(1, Ordering::Relaxed); + match provider.chain_prevout(&op) { + ChainPrevout::Unspent(coin) => { + assert!(!coin.is_coinbase, "Ok(false) stays not a coinbase"); + } + ChainPrevout::KnownUnavailable => panic!("Ok(false) must still return the coin"), + ChainPrevout::Unknown => panic!("genesis coinbase is a known create"), + } + provider.coinbase_read.store(0, Ordering::Relaxed); + match provider.chain_prevout(&op) { + ChainPrevout::Unspent(coin) => assert!(coin.is_coinbase, "genesis is the coinbase"), + ChainPrevout::KnownUnavailable => panic!("genesis coinbase is unspent"), + ChainPrevout::Unknown => panic!("genesis coinbase is a known create"), + } + let _ = std::fs::remove_dir_all(dir); + } + /// Non-coinbase, no BIP68 time-lock: one packed body decode per coin /// lookup and no create MTP. The parent has two outputs with distinct /// values and the child spends vout 1, so the fee pins the vout index. diff --git a/docs/concurrency.md b/docs/concurrency.md index 292a3750be..dcbe841d68 100644 --- a/docs/concurrency.md +++ b/docs/concurrency.md @@ -89,7 +89,7 @@ spend annotations. | `tx.head` segment seal | Roll opens the next OA immediately; BDZ+fuse8 runs on a sidecar. Lookup probes every unsealed OA until publish. Write joins the sidecar only on the *next* roll, `flush`, or `Drop` (not on the rolling insert). | | `header.head` overflow | Insert past 7/8 rolls `header.head.gN` (new empty file). Occupied rewrite is open-only: undersized single gen writes `header.head.grow` then rename. | | `ChainHub::confirmed` | `RwLock` for O(1) `has_block` (IBD assign path) | -| `MempoolHub::inner` | One `RwLock`. **Write** is graph mutation + durable slot append only (microseconds at ~12k live). No Query / script verify / compact reconstruct under write. The commit recheck of a confirmed coin runs before the write lock unless a confirm or disconnect holds the UTXO view. Prepare may hold a **read** across Query. Blocking `read`/`write` is `assert_not_reactor`. Session INV/getdata/compact use `try_read`; miss = skip that item (re-getdata / notfound / reconstruct without those txs). Compact fill siphashes live txid/wtxid and clones **matching** bodies only — never `list_live()`. Handshake `FeeFilter` reads the published fee-floor **atomic**, not `inner`. Age-INV log and min live `accept_at` are **not** `inner` (same class as `accept_at`). Accept caches `{tip_header_fk, height, mtp}` until the header fk changes. `get_coin` resolves the parent on the connected chain, decodes its packed outputs once, reads coinbase-ness from the Class C coinbase table, and computes create MTP only for BIP68 **time** locks. `index_txid` hashes the admitted tx + prepare prevouts (no second Query). Do not CoW a 12k-txid presence set on every accept. Lock order: `inner` → `sh_index` / `unbroadcast` / `relay_*`. | +| `MempoolHub::inner` | One `RwLock`. **Write** is graph mutation + durable slot append only (microseconds at ~12k live). No Query / script verify / compact reconstruct under write. The commit recheck of a confirmed coin runs before the write lock unless a confirm or disconnect holds the UTXO view. Prepare may hold a **read** across Query. Blocking `read`/`write` is `assert_not_reactor`. Session INV/getdata/compact use `try_read`; miss = skip that item (re-getdata / notfound / reconstruct without those txs). Compact fill siphashes live txid/wtxid and clones **matching** bodies only — never `list_live()`. Handshake `FeeFilter` reads the published fee-floor **atomic**, not `inner`. Age-INV log and min live `accept_at` are **not** `inner` (same class as `accept_at`). Accept caches `{tip_header_fk, height, mtp}` until the header fk changes. `get_coin` resolves the parent on the connected chain, decodes its packed outputs once, reads coinbase-ness from the Class C coinbase table (a failed read leaves the coin unavailable), and computes create MTP only for BIP68 **time** locks. `index_txid` hashes the admitted tx + prepare prevouts (no second Query). Do not CoW a 12k-txid presence set on every accept. Lock order: `inner` → `sh_index` / `unbroadcast` / `relay_*`. | There is **no** global “pause queries during confirm write.” Tip-as-commit + `is_confirmed_strong` define query visibility ([`crash-recovery.md`](./crash-recovery.md)).