net: decode each parent once per block proposal check

changelog.d/proposal-parent-once.md

@@ -0,0 +1,5 @@
+Changed
+
+- **A block proposal decodes each confirmed parent once.** `getblocktemplate`
+  proposal mode resolves a fan-out parent's fk once and decodes its packed
+  outputs once per check, not once per spending input.

crates/rbitcoin-net/src/chain.rs

@@ -20,6 +20,7 @@ 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;
@@ -3177,6 +3178,9 @@ fn proposal_connect(
     let cb_txid = block.txdata[0].compute_txid();
     let mut created: HashMap<OutPoint, TxOut> = HashMap::new();
     let mut spent: HashSet<OutPoint> = 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<Txid, (Fk, Vec<TxOut>)> = HashMap::new();
     let mut fees = 0u64;
     for (i, tx) in block.txdata.iter().enumerate() {
         if !rbitcoin_consensus::is_final_tx(tx, height, mtp.max(block.header.time)) {
@@ -3207,7 +3211,7 @@ fn proposal_connect(
             }
             let txout = if let Some(o) = created.get(&op) {
                 o.clone()
-            } else if let Some((fk, o)) = chain_txout(query, &op) {
+            } else if let Some((fk, o)) = chain_txout(query, &mut parents, &op) {
                 if coinbase_spend_is_immature(query, fk, height, maturity)? {
                     return Err("bad-txns-premature-spend-of-coinbase".into());
                 }
@@ -3236,28 +3240,41 @@ fn proposal_connect(
     Ok(fees)
 }
 
-fn chain_txout(query: &Query, op: &OutPoint) -> Option<(Fk, TxOut)> {
+/// `op`'s unspent confirmed output. `parents` holds each parent's create fk
+/// and decoded outputs for this one proposal check.
+fn chain_txout(
+    query: &Query,
+    parents: &mut HashMap<Txid, (Fk, Vec<TxOut>)>,
+    op: &OutPoint,
+) -> Option<(Fk, TxOut)> {
     let tid = op.txid.to_byte_array();
     if query.is_outpoint_spent(&tid, op.vout).ok()? {
         return None;
     }
-    let (fk, rec) = query.get_tx_by_txid(&tid).ok().flatten()?;
-    let out = query
-        .tx_output_at_fk(fk, op.vout)
-        .ok()
-        .or_else(|| query.tx_output(&rec, op.vout).ok())?;
-    let value = if out.value < 0 {
-        Amount::ZERO
-    } else {
-        Amount::from_sat(out.value as u64)
+    let slot = match parents.entry(op.txid) {
+        Entry::Occupied(e) => e.into_mut(),
+        Entry::Vacant(v) => v.insert(chain_tx_outputs(query, &tid)?),
     };
-    Some((
-        fk,
-        TxOut {
-            value,
+    let out = slot.1.get(op.vout as usize).cloned()?;
+    Some((slot.0, out))
+}
+
+/// 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<TxOut>)> {
+    let fk = query.tx_fk_by_txid(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))
 }
 
 /// Core `CheckTxInputs`: the creating tx is the coinbase at its height and
@@ -6345,6 +6362,65 @@ mod tests {
         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,

crates/rbitcoin-store/src/store.rs

@@ -292,6 +292,11 @@ 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<Vec<u64>>,
     #[cfg(debug_assertions)]
@@ -395,6 +400,8 @@ 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,
@@ -460,6 +467,8 @@ 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,
@@ -729,10 +738,19 @@ 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`].
     pub fn get_tx(&self, fk: Fk) -> Result<TxRecord, StoreError> {
+        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();
@@ -801,9 +819,17 @@ impl Store {
         &self,
         fk: Fk,
     ) -> Result<(TxRecord, Vec<OutputRecord>), 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<Vec<Option<[u8; 32]>>, StoreError> {
         #[cfg(debug_assertions)]
@@ -1069,6 +1095,8 @@ impl Store {
         mode: TxidResolveMode,
     ) -> Result<Option<Fk>, 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);
         }
@@ -1130,7 +1158,11 @@ impl Store {
     ///
     /// **`TipThenAny`:** RPC / reconstruct (connected if present, else newest).
     pub fn get_fk_by_txid(&self, txid: &[u8; 32]) -> Result<Option<Fk>, StoreError> {
-        self.resolve_txid(txid, TxidResolveMode::TipThenAny)
+        Ok(self
+            .get_fk_by_txid_batch_mode(std::slice::from_ref(txid), TxidResolveMode::TipThenAny)?
+            .into_iter()
+            .next()
+            .and_then(|(_, hit)| hit.map(|(fk, _)| fk)))
     }
 
     /// Confirm / consensus: connected instance only.
@@ -2550,9 +2582,28 @@ 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 (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(),
+            1,
+            "one packed outs decode"
+        );
+        assert_eq!(
+            s.sample_reset_tx_gets(),
+            0,
+            "an outs decode is not a get_tx"
+        );
+        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"
+        );
         let full = s.get_tx_full(create_fk).unwrap();
         assert_eq!(full.2.len(), 2);
         let (m2, prevs) = s.get_tx_meta_and_prevouts(create_fk).unwrap();
@@ -2561,6 +2612,16 @@ mod tests {
         assert_eq!(s.get_tx_full(create_fk).unwrap().0.txid, [10u8; 32]);
         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(),
+            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"
+        );
         let mut span_hashes = Vec::new();
         s.for_each_create_script_hashes_in_fk_span(create_fk.0, create_fk.0, |_fk, sh| {
             span_hashes.push(sh);
@@ -2575,7 +2636,13 @@ 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(),
+            0,
+            "fk resolve does not decode the body"
+        );
         assert_eq!(s.get_tx_by_txid(&[10u8; 32]).unwrap().unwrap().0, create_fk);
 
         // Second tx spends create vout 0.
@@ -3768,6 +3835,7 @@ mod tests {
             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
@@ -3784,6 +3852,7 @@ mod tests {
             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)
             .unwrap();