From 4a8f9bd01ded242f0facf15f5b5e9ce6df4c72e5 Mon Sep 17 00:00:00 2001 From: Benjamen Keroack Date: Wed, 7 Oct 2026 14:29:17 -0600 Subject: [PATCH 1/3] Count sigops exactly as Bitcoin Core does satd counted sigops with rust-bitcoin 0.32's `Script::count_sigops` and `Transaction::total_sigop_cost`. In accurate mode those keep the last OP_1..OP_16 across later OP_CHECKSIG and OP_CHECKMULTISIG opcodes, where Core's `GetSigOpCount` reads the opcode immediately before each multisig opcode (src/script/script.cpp). An OP_CHECKMULTISIG that followed another signature opcode took the earlier number instead of 20, so P2SH redeem scripts and P2WSH witness scripts could count lower than Core counts them (`OP_1 OP_CHECKSIG OP_CHECKMULTISIG`: 2 instead of 21). This is rust-bitcoin issue #6368; no released rust-bitcoin version has the fix. Add `validation::sigops`, a port of Core's `GetSigOpCount(fAccurate)`, `GetSigOpCount(scriptSig)`, `CountWitnessSigOps`, `GetLegacySigOpCount`, `GetP2SHSigOpCount` and `GetTransactionSigOpCost`, with Core's script parsing, and Core's sigopcount_tests, script_p2sh_tests and transaction_tests vectors. Use it for the block limit in `connect_block` and `check_block`, the cost each mempool entry records (which the block template budgets by and getblocktemplate reports), the policy engine's `tx.sigops_cost`, and the Stratum coinbase bound. The submitpackage fee-bypass path recorded a sigop cost of 0 and no prevout metadata for the parents it admits, so the template counted them as free. It now records what `accept_transaction` records, through one shared helper. Co-Authored-By: Claude Opus 5.5 (1M context) --- node/src/chain/connect.rs | 67 ++-- node/src/chain/connect_sigops_tests.rs | 175 +++++++++ node/src/mempool/policy_engine.rs | 18 +- node/src/mempool/pool.rs | 143 +++++--- node/src/mempool/pool_sigops_tests.rs | 183 ++++++++++ node/src/stratum/template.rs | 6 +- node/src/stratum/v2/jd.rs | 19 +- node/src/validation/block.rs | 13 +- node/src/validation/mod.rs | 1 + node/src/validation/sigops.rs | 326 +++++++++++++++++ node/src/validation/sigops_tests.rs | 486 +++++++++++++++++++++++++ satd/tests/regtest.rs | 9 +- 12 files changed, 1319 insertions(+), 127 deletions(-) create mode 100644 node/src/chain/connect_sigops_tests.rs create mode 100644 node/src/mempool/pool_sigops_tests.rs create mode 100644 node/src/validation/sigops.rs create mode 100644 node/src/validation/sigops_tests.rs diff --git a/node/src/chain/connect.rs b/node/src/chain/connect.rs index 66694481e..4d0f2ae2e 100644 --- a/node/src/chain/connect.rs +++ b/node/src/chain/connect.rs @@ -302,31 +302,21 @@ fn bip30_exempt(network: Network, height: u32, hash: &BlockHash) -> bool { == "00000000000743f190a18c5577a3c2d2a1f610ae9601ac046a38084ccb7cd721") } -/// Block-level signature-operation cost for one transaction, height-gated to -/// match Bitcoin Core's `GetTransactionSigOpCost`. Below P2SH activation only -/// legacy sigops count (scaled by the witness factor); at/after it the full -/// BIP 141 witness-aware cost applies. Witness sigops are naturally zero -/// before any witness data exists, so the single `>= P2SH` gate is exact -/// across the P2SH→segwit range. `spent` maps each input's prevout to the -/// output it spends (empty for coinbase, which Core counts as legacy-only). -fn transaction_sigop_cost( - tx: &Transaction, - network: Network, - height: u32, - spent: &HashMap, -) -> u64 { - if height >= crate::validation::script::activation_heights(network).p2sh { - tx.total_sigop_cost(|op| spent.get(op).cloned()) as u64 +/// Block-level signature-operation cost for one transaction: Bitcoin Core's +/// `GetTransactionSigOpCost` (see [`crate::validation::sigops`]). At and +/// after P2SH activation it counts with `SCRIPT_VERIFY_P2SH | +/// SCRIPT_VERIFY_WITNESS`, the flags Core's `GetBlockScriptFlags` gives +/// every block but its exceptions; below it, legacy sigops only (issue #724 +/// tracks that height gate). `prevouts` holds the output each input spends, +/// in input order; a coinbase's are not read. +fn transaction_sigop_cost(tx: &Transaction, network: Network, height: u32, prevouts: &[TxOut]) -> u64 { + use crate::validation::sigops::{self, SigOpFlags}; + let flags = if height >= crate::validation::script::activation_heights(network).p2sh { + SigOpFlags::P2shWitness } else { - let mut legacy: usize = 0; - for input in &tx.input { - legacy = legacy.saturating_add(input.script_sig.count_sigops_legacy()); - } - for output in &tx.output { - legacy = legacy.saturating_add(output.script_pubkey.count_sigops_legacy()); - } - (legacy.saturating_mul(4)) as u64 - } + SigOpFlags::Legacy + }; + sigops::transaction_sigop_cost(tx, prevouts, flags) } /// Parameters for block connection. Groups the many inputs needed by connect_block @@ -490,9 +480,6 @@ fn connect_block_inner(params: &ConnectParams) -> Result = HashMap::new(); // BIP 30: enforce the no-overwrite-of-an-unspent-txid rule unless this // block is past the buried BIP 34 height or is a grandfathered exception. let enforce_bip30 = @@ -864,15 +851,10 @@ fn connect_block_inner(params: &ConnectParams) -> Result MAX_BLOCK_SIGOPS_COST { return Err(ConnectError::BadBlockSigops); } @@ -892,9 +874,8 @@ fn connect_block_inner(params: &ConnectParams) -> Result MAX_BLOCK_SIGOPS_COST { return Err(ConnectError::BadBlockSigops); } @@ -1165,6 +1146,10 @@ fn connect_block_inner(params: &ConnectParams) -> Result = HashMap::new(); // 10 OP_CHECKMULTISIG * 20 legacy sigops * 4 (witness scale) = 800. let mainnet_p2sh = crate::validation::script::activation_heights(Network::Bitcoin).p2sh; - assert_eq!(transaction_sigop_cost(&cb, Network::Bitcoin, 0, &empty), 800); - assert_eq!(transaction_sigop_cost(&cb, Network::Bitcoin, mainnet_p2sh, &empty), 800); + assert_eq!(transaction_sigop_cost(&cb, Network::Bitcoin, 0, &[]), 800); + assert_eq!(transaction_sigop_cost(&cb, Network::Bitcoin, mainnet_p2sh, &[]), 800); // Regtest/signet: P2SH active from genesis, so the witness-aware // path is taken even at height 0 (same cost here — no P2SH data). - assert_eq!(transaction_sigop_cost(&cb, Network::Regtest, 0, &empty), 800); + assert_eq!(transaction_sigop_cost(&cb, Network::Regtest, 0, &[]), 800); } // ── helpers for connect_block tests ─────────────────────────────── diff --git a/node/src/chain/connect_sigops_tests.rs b/node/src/chain/connect_sigops_tests.rs new file mode 100644 index 000000000..bfac58b35 --- /dev/null +++ b/node/src/chain/connect_sigops_tests.rs @@ -0,0 +1,175 @@ +//! `connect_block`'s block sigop limit, counted as Bitcoin Core counts it. +//! +//! The witness scripts here put `OP_CHECKMULTISIG` after another signature +//! opcode. Core's `GetSigOpCount(fAccurate=true)` (src/script/script.cpp) +//! reads the opcode immediately before each `OP_CHECKMULTISIG`, so only the +//! first one, after `OP_1`, counts 1 and every later one counts 20. + +use super::*; +use crate::storage::db::InMemoryStore; +use crate::validation::script::NoopVerifier; +use bitcoin::block::Header; +use bitcoin::hashes::Hash; +use bitcoin::opcodes::all::{OP_CHECKMULTISIG, OP_CHECKSIG, OP_ENDIF, OP_IF, OP_PUSHNUM_1}; +use bitcoin::opcodes::OP_0; +use bitcoin::pow::CompactTarget; +use bitcoin::transaction::Version; +use bitcoin::{Amount, ScriptBuf, Sequence, TxIn, Witness}; + +/// `OP_0 OP_IF OP_1 OP_CHECKMULTISIG×k OP_ENDIF OP_1`: the branch never runs +/// and the script leaves `[1]`. Core counts `1 + 20·(k-1)`. +fn stale_op_n_script(k: usize) -> ScriptBuf { + let mut s = vec![OP_0.to_u8(), OP_IF.to_u8(), OP_PUSHNUM_1.to_u8()]; + s.extend(std::iter::repeat_n(OP_CHECKMULTISIG.to_u8(), k)); + s.extend([OP_ENDIF.to_u8(), OP_PUSHNUM_1.to_u8()]); + ScriptBuf::from_bytes(s) +} + +/// `OP_CHECKSIG×n OP_1`. Core counts `n`. +fn checksig_script(n: usize) -> ScriptBuf { + let mut s = vec![OP_CHECKSIG.to_u8(); n]; + s.push(OP_PUSHNUM_1.to_u8()); + ScriptBuf::from_bytes(s) +} + +const COIN_VALUE: u64 = 100_000; + +/// A regtest block at height 1 whose second transaction spends one P2WSH +/// coin per witness script, each revealing its script in the witness. +/// Returns the store holding those coins and the block. +fn block_spending_p2wsh(witness_scripts: &[ScriptBuf]) -> (InMemoryStore, Block) { + let store = InMemoryStore::new(); + let mut batch = StoreBatch::default(); + batch.chain_tx_puts.push((BlockHash::all_zeros(), 0)); + + let mut inputs = Vec::new(); + for (i, ws) in witness_scripts.iter().enumerate() { + let outpoint = OutPoint { + txid: bitcoin::Txid::from_byte_array([0x70 ^ (i as u8); 32]), + vout: i as u32, + }; + batch.coin_puts.push(( + outpoint, + Coin { + amount: COIN_VALUE, + script_pubkey: ScriptBuf::new_p2wsh(&ws.wscript_hash()), + height: 0, + coinbase: false, + txseq: node_index::TXSEQ_UNKNOWN, + }, + )); + inputs.push(TxIn { + previous_output: outpoint, + script_sig: ScriptBuf::new(), + sequence: Sequence::MAX, + witness: Witness::from_slice(&[ws.as_bytes()]), + }); + } + store.write_batch(batch).unwrap(); + + let height = 1; + let coinbase = Transaction { + version: Version(2), + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![TxIn { + previous_output: OutPoint::null(), + script_sig: bitcoin::script::Builder::new() + .push_int(height as i64) + .push_opcode(OP_0) + .into_script(), + sequence: Sequence::MAX, + witness: Witness::new(), + }], + output: vec![TxOut { + value: Amount::from_sat(block_subsidy(Network::Regtest, height)), + script_pubkey: ScriptBuf::new(), + }], + }; + let spend = Transaction { + version: Version(2), + lock_time: bitcoin::absolute::LockTime::ZERO, + input: inputs, + output: vec![TxOut { + value: Amount::from_sat(COIN_VALUE * witness_scripts.len() as u64), + script_pubkey: ScriptBuf::new(), + }], + }; + let mut block = Block { + header: Header { + version: bitcoin::block::Version::from_consensus(0x2000_0000), + prev_blockhash: BlockHash::all_zeros(), + merkle_root: bitcoin::TxMerkleNode::all_zeros(), + time: 1_700_000_000, + bits: CompactTarget::from_consensus(0x207f_ffff), + nonce: 0, + }, + txdata: vec![coinbase, spend], + }; + block.header.merkle_root = block.compute_merkle_root().unwrap(); + (store, block) +} + +fn connect_at_height_1(store: &InMemoryStore, block: &Block) -> Result { + connect_block(&ConnectParams { + replay_plan: None, + store, + block, + height: 1, + parent_chainwork: &[0u8; 32], + flat_pos: FlatFilePos { file_number: 0, data_pos: 0 }, + script_verifier: &NoopVerifier, + median_time_past: 0, + network: Network::Regtest, + pre_verified_txs: None, + num_threads: 1, + precomputed_txids: None, + address_index: &Default::default(), + sp_index: &Default::default(), + #[cfg(feature = "block-filter-index")] + filter_index: &Default::default(), + phase_tracker: None, + interrupt: None, + }) +} + +/// Twenty inputs at 3,941 each (78,820) plus one more to reach the limit +/// exactly: a block at 80,000 connects and one at 80,001 is +/// `bad-blk-sigops`, as `MAX_BLOCK_SIGOPS_COST` is applied in Core's +/// `ConnectBlock` (`nSigOpsCost > MAX_BLOCK_SIGOPS_COST`). +/// +/// Witness sigops are not scaled, so each input adds its witness script's +/// accurate count. A counter that keeps an earlier `OP_1` across later +/// `OP_CHECKMULTISIG`s sees 198 per heavy input and about 5,140 in all, +/// and lets the 80,001 block through. +#[test] +fn a_block_one_sigop_over_the_limit_by_cores_count_is_refused() { + let heavy = stale_op_n_script(198); + let mut scripts = vec![heavy; 20]; + scripts.push(checksig_script(80_000 - 20 * 3_941)); + let (store, at_limit) = block_spending_p2wsh(&scripts); + connect_at_height_1(&store, &at_limit).expect("a block at exactly 80,000 sigop cost connects"); + + *scripts.last_mut().unwrap() = checksig_script(80_000 - 20 * 3_941 + 1); + let (store, over) = block_spending_p2wsh(&scripts); + assert!( + matches!(connect_at_height_1(&store, &over), Err(ConnectError::BadBlockSigops)), + "a block at 80,001 sigop cost is bad-blk-sigops" + ); +} + +/// The block-level wrapper applies Core's `GetTransactionSigOpCost` to the +/// resolved prevouts: one P2WSH input whose script is `OP_1 OP_CHECKSIG +/// OP_CHECKMULTISIG` costs 21, not 2. +#[test] +fn the_block_sigop_cost_of_a_p2wsh_spend_follows_cores_count() { + let ws = ScriptBuf::from_bytes(vec![0x51, 0xac, 0xae]); + let (_store, block) = block_spending_p2wsh(std::slice::from_ref(&ws)); + let prevouts = vec![TxOut { + value: Amount::from_sat(COIN_VALUE), + script_pubkey: ScriptBuf::new_p2wsh(&ws.wscript_hash()), + }]; + assert_eq!(transaction_sigop_cost(&block.txdata[1], Network::Regtest, 1, &prevouts), 21); + // Below the P2SH gate only legacy sigops count (issue #724 tracks that + // gate), and this transaction has none. + assert_eq!(transaction_sigop_cost(&block.txdata[1], Network::Bitcoin, 1, &prevouts), 0); +} diff --git a/node/src/mempool/policy_engine.rs b/node/src/mempool/policy_engine.rs index dcb9fc841..090d5c5f2 100644 --- a/node/src/mempool/policy_engine.rs +++ b/node/src/mempool/policy_engine.rs @@ -16,12 +16,11 @@ //! buffers for the 32-byte txids; everything is constructed and consumed inside //! one stack frame, so no heap-resident view outlives the evaluation. -use std::collections::HashMap; use std::path::Path; use bitcoin::blockdata::script::Instruction; use bitcoin::hashes::{Hash, sha256}; -use bitcoin::{Network as BNetwork, OutPoint, Script, Transaction, TxIn, TxOut, Txid}; +use bitcoin::{Network as BNetwork, Script, Transaction, TxIn, TxOut, Txid}; use satd_policy::{ CompiledRuleset, Ctx, InputView, Network, OutputView, ScriptType, Source, TxView, Verdict, @@ -326,14 +325,13 @@ fn with_view( .collect(); // sigop cost (`tx.sigops_cost`), computed against the resolved prevouts - // exactly as the entry accounting does at insert time. - let prev_map: HashMap = tx - .input - .iter() - .zip(prev_outputs.iter()) - .map(|(i, o)| (i.previous_output, o.clone())) - .collect(); - let sigops_cost = tx.total_sigop_cost(|op| prev_map.get(op).cloned()) as i128; + // exactly as the entry accounting does at insert time: Core's + // `GetTransactionSigOpCost` with the standard flags. + let sigops_cost = crate::validation::sigops::transaction_sigop_cost( + tx, + prev_outputs, + crate::validation::sigops::SigOpFlags::P2shWitness, + ) as i128; let mut inputs: Vec = Vec::with_capacity(tx.input.len()); for (idx, tin) in tx.input.iter().enumerate() { diff --git a/node/src/mempool/pool.rs b/node/src/mempool/pool.rs index 02ba62fb7..84fc1741a 100644 --- a/node/src/mempool/pool.rs +++ b/node/src/mempool/pool.rs @@ -1317,6 +1317,17 @@ pub trait PackageMember { fn txid(&self) -> Txid; } +/// What an admission records from the outputs its transaction spends. +/// Both admission paths build it with [`Mempool::spend_record`], so an +/// entry carries the same sigop cost and prevout metadata however it got in. +struct SpendRecord { + sigop_cost: u64, + prev_scripthashes: Vec, + prev_amounts: Vec, + prev_scripts: Vec, + sp_tweak: Option, +} + /// So the checks can be exercised against a plain transaction list. impl PackageMember for Transaction { fn tx(&self) -> &Transaction { @@ -1413,6 +1424,64 @@ impl Mempool { self.inner.read().entries.get(txid).and_then(|e| e.sp_tweak.clone()) } + /// The [`SpendRecord`] for `tx`, whose inputs spend `prev_outputs` (input + /// order). + /// + /// The sigop cost is Bitcoin Core's `GetTransactionSigOpCost` with the + /// standard script flags, which include P2SH and WITNESS (Core's + /// `PreChecks`, src/validation.cpp): the number the block template budgets + /// by and getblocktemplate reports. + /// + /// `prev_outputs` is dropped after admission, so this is the one chance + /// to keep prevout data for mempool spend-side matching (exact script + + /// prefix bucket) without re-resolving prevouts in the (decoupled) + /// matcher. The scripthash is always kept; value and full script are gated + /// by `streamprevoutmeta` (see `PrevoutMetaLevel`) so operators pay only + /// for the matcher capabilities they want. Gate-cold the scripts are empty, + /// so the event payload is byte-identical to a node without silent + /// payments. + /// + /// Silent-payment tweak: while an SP scan-key watch (D7, Tier 2) OR a + /// `mempool_tweaks` firehose subscriber (Tier 1.5) is live, compute the + /// BIP 352 public tweak `T = input_hash · A` once here — the resolved + /// prevout `scriptPubKey`s (including in-mempool parents) are in hand and + /// cannot be reconstructed off the hot path. The ~73-byte result is cached + /// on the entry so the unconfirmed matcher never recomputes it and the + /// firehose bridge can read it by txid. Gate-cold this is `None` with no + /// EC work, so a node without silent payments is unchanged. + fn spend_record( + &self, + prevout_meta: PrevoutMetaLevel, + tx: &Transaction, + prev_outputs: &[TxOut], + ) -> SpendRecord { + let sigop_cost = crate::validation::sigops::transaction_sigop_cost( + tx, + prev_outputs, + crate::validation::sigops::SigOpFlags::P2shWitness, + ); + let prev_scripthashes: Vec = + prev_outputs.iter().map(|o| scripthash_of(&o.script_pubkey)).collect(); + let prev_amounts: Vec = if prevout_meta.retains_amount() { + prev_outputs.iter().map(|o| o.value.to_sat()).collect() + } else { + Vec::new() + }; + let prev_scripts: Vec = if prevout_meta.retains_script() { + prev_outputs.iter().map(|o| o.script_pubkey.clone()).collect() + } else { + Vec::new() + }; + let sp_tweak: Option = if self.sp_gate_hot() || self.mempool_tweaks_hot() { + let spks: Vec = + prev_outputs.iter().map(|o| o.script_pubkey.clone()).collect(); + compute_tweak(tx, &spks) + } else { + None + }; + SpendRecord { sigop_cost, prev_scripthashes, prev_amounts, prev_scripts, sp_tweak } + } + /// Whether the operator authorized emitting full prevout `scriptPubKey`s to /// streaming watchers (`streamprevoutmeta = full`). This is the *policy* /// signal, distinct from whether scripts happen to be *retained* at @@ -2996,56 +3065,9 @@ impl Mempool { inner.spends.insert(input.previous_output, txid); } - let prev_outputs_map: HashMap = tx - .input - .iter() - .zip(prev_outputs.iter()) - .map(|(i, o)| (i.previous_output, o.clone())) - .collect(); - let sigop_cost = tx.total_sigop_cost(|op| prev_outputs_map.get(op).cloned()) as u64; - - // Retain the spent prevout metadata (input order) for mempool - // spend-side matching (exact script + prefix bucket). `prev_outputs` is - // fully resolved above and dropped after this; capturing it now is the - // one chance to keep the data without re-resolving prevouts in the - // (decoupled) matcher. The scripthash is always kept; value and full - // script are gated by `streamprevoutmeta` (see `PrevoutMetaLevel`) so - // operators pay only for the matcher capabilities they want. - let prev_scripthashes: Vec = prev_outputs - .iter() - .map(|o| scripthash_of(&o.script_pubkey)) - .collect(); - let prev_amounts: Vec = if cfg.prevout_meta.retains_amount() { - prev_outputs.iter().map(|o| o.value.to_sat()).collect() - } else { - Vec::new() - }; - // Retain full prevout scripts only when the operator asked for them - // (`streamprevoutmeta=full`); gate-cold this is empty, so the event - // payload is byte-identical to a node without silent payments. - let prev_scripts: Vec = if cfg.prevout_meta.retains_script() { - prev_outputs - .iter() - .map(|o| o.script_pubkey.clone()) - .collect() - } else { - Vec::new() - }; - // Silent-payment tweak: while an SP scan-key watch (D7, Tier 2) OR a - // `mempool_tweaks` firehose subscriber (Tier 1.5) is live, compute the - // BIP 352 public tweak `T = input_hash · A` once here — the resolved - // prevout `scriptPubKey`s (including in-mempool parents) are in hand and - // cannot be reconstructed off the hot path. The ~73-byte result is cached - // on the entry so the unconfirmed matcher never recomputes it and the - // firehose bridge can read it by txid. Gate-cold this is `None` with no - // EC work, so a node without silent payments is unchanged. - let sp_tweak: Option = if self.sp_gate_hot() || self.mempool_tweaks_hot() { - let spks: Vec = - prev_outputs.iter().map(|o| o.script_pubkey.clone()).collect(); - compute_tweak(&tx, &spks) - } else { - None - }; + // Sigop cost and prevout metadata, the same for every admission path. + let SpendRecord { sigop_cost, prev_scripthashes, prev_amounts, prev_scripts, sp_tweak } = + self.spend_record(cfg.prevout_meta, &tx, &prev_outputs); let entry_weight_u64 = weight as u64; let vsize_u64 = policy::weight_to_vsize(entry_weight_u64); @@ -5945,6 +5967,7 @@ impl Mempool { let weight = tx.weight().to_wu() as usize; let tx_size = bitcoin::consensus::serialize(&tx).len(); + let prevout_meta = self.config.read().prevout_meta; // Take write lock let mut inner = self.inner.write(); @@ -6032,6 +6055,12 @@ impl Mempool { let fee_rate = policy::fee_rate_sat_per_kvb(fee, weight as u64); + // The same sigop cost and prevout metadata `accept_transaction` + // records. A zero cost here let the block template count this entry + // as free against the block sigop limit. + let SpendRecord { sigop_cost, prev_scripthashes, prev_amounts, prev_scripts, sp_tweak } = + self.spend_record(prevout_meta, &tx, &prev_outputs); + // Insert into mempool let scope = QuarantineScope::acting(); inner.spends.extend( @@ -6056,11 +6085,11 @@ impl Mempool { .unwrap_or_default() .as_secs(), fee_delta: pending_delta, - sigop_cost: 0, - prev_scripthashes: Vec::new(), - prev_amounts: Vec::new(), - prev_scripts: Vec::new(), - sp_tweak: None, + sigop_cost, + prev_scripthashes, + prev_amounts, + prev_scripts, + sp_tweak, scope, source: TxSource::Rpc, quarantine_rule: None, @@ -6586,6 +6615,10 @@ impl Mempool { } } +#[cfg(test)] +#[path = "pool_sigops_tests.rs"] +mod sigops_tests; + #[cfg(test)] mod tests { diff --git a/node/src/mempool/pool_sigops_tests.rs b/node/src/mempool/pool_sigops_tests.rs new file mode 100644 index 000000000..749ac3f12 --- /dev/null +++ b/node/src/mempool/pool_sigops_tests.rs @@ -0,0 +1,183 @@ +//! Mempool sigop cost, counted as Bitcoin Core counts it, and the block +//! template budget that reads it. +//! +//! The P2WSH inputs here reveal `OP_0 OP_IF OP_1 OP_CHECKMULTISIG×198 +//! OP_ENDIF OP_1`. Core's `GetSigOpCount(fAccurate=true)` +//! (src/script/script.cpp) reads the opcode immediately before each +//! `OP_CHECKMULTISIG`: the first follows `OP_1` and counts 1, the other 197 +//! follow `OP_CHECKMULTISIG` and count 20 each, 3,941 in all. Core's mempool +//! stores that cost on the entry (`GetTransactionSigOpCost` with the +//! standard flags, src/validation.cpp `PreChecks`), and its block assembler +//! budgets by it. + +use super::*; +use crate::mining::template::tests::make_funded_template_env_with; +use crate::storage::coinview::Coin; +use crate::validation::script::NoopVerifier; +use bitcoin::hashes::Hash; +use bitcoin::opcodes::all::{OP_CHECKMULTISIG, OP_CHECKSIG, OP_ENDIF, OP_IF, OP_PUSHNUM_1}; +use bitcoin::opcodes::OP_0; +use bitcoin::{Amount, ScriptBuf, Sequence, TxIn, Witness}; + +/// Core's count for one input revealing [`heavy_script`]. +const HEAVY: u64 = 1 + 197 * 20; +const COIN_VALUE: u64 = 100_000; + +fn heavy_script() -> ScriptBuf { + let mut s = vec![OP_0.to_u8(), OP_IF.to_u8(), OP_PUSHNUM_1.to_u8()]; + s.extend(std::iter::repeat_n(OP_CHECKMULTISIG.to_u8(), 198)); + s.extend([OP_ENDIF.to_u8(), OP_PUSHNUM_1.to_u8()]); + ScriptBuf::from_bytes(s) +} + +/// `OP_CHECKSIG OP_1`: one sigop by any count. +fn light_script() -> ScriptBuf { + ScriptBuf::from_bytes(vec![OP_CHECKSIG.to_u8(), OP_PUSHNUM_1.to_u8()]) +} + +fn prev(tag: u8) -> OutPoint { + OutPoint { txid: Txid::from_byte_array([tag; 32]), vout: 0 } +} + +/// A confirmed P2WSH coin committing to `witness_script`. +fn p2wsh_coin(witness_script: &ScriptBuf) -> Coin { + Coin { + amount: COIN_VALUE, + script_pubkey: ScriptBuf::new_p2wsh(&witness_script.wscript_hash()), + height: 0, + coinbase: false, + txseq: node_index::TXSEQ_UNKNOWN, + } +} + +/// Spend each of `prevs` by revealing `witness_script`, paying all but `fee` +/// to one P2WPKH output. +fn spend_p2wsh(prevs: &[OutPoint], witness_script: &ScriptBuf, fee: u64, out_tag: u8) -> Transaction { + let mut spk = vec![0x00, 0x14]; + spk.extend_from_slice(&[out_tag; 20]); + Transaction { + version: bitcoin::transaction::Version(2), + lock_time: bitcoin::absolute::LockTime::ZERO, + input: prevs + .iter() + .map(|p| TxIn { + previous_output: *p, + script_sig: ScriptBuf::new(), + sequence: Sequence::MAX, + witness: Witness::from_slice(&[witness_script.as_bytes()]), + }) + .collect(), + output: vec![TxOut { + value: Amount::from_sat(COIN_VALUE * prevs.len() as u64 - fee), + script_pubkey: ScriptBuf::from_bytes(spk), + }], + } +} + +/// Admission records Core's count: four heavy inputs cost 15,764, which +/// is under Core's standard per-transaction 16,000. +#[test] +fn an_entry_carries_cores_sigop_cost() { + let ws = heavy_script(); + let prevs: Vec = (1..=4).map(prev).collect(); + let coins: Vec<(OutPoint, Coin)> = prevs.iter().map(|p| (*p, p2wsh_coin(&ws))).collect(); + let (cs, mp, dir) = make_funded_template_env_with(&coins, Box::new(NoopVerifier)); + + let txid = mp + .accept_transaction(spend_p2wsh(&prevs, &ws, 1_000, 0x21), &cs, &NoopVerifier, TxSource::Rpc, false) + .expect("admitted"); + assert_eq!(mp.get(&txid).unwrap().sigop_cost, 4 * HEAVY); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// The policy engine's `tx.sigops_cost` is the same number: a rule that +/// holds transactions above 3,000 holds the heavy spend (3,941) and not the +/// light one (1). +#[test] +fn the_policy_view_sees_cores_sigop_cost() { + let (heavy, light) = (heavy_script(), light_script()); + let coins = vec![(prev(1), p2wsh_coin(&heavy)), (prev(2), p2wsh_coin(&light))]; + let (cs, mp, dir) = make_funded_template_env_with(&coins, Box::new(NoopVerifier)); + let rs = satd_policy::parse_ruleset("version 1\nquarantine sigops when tx.sigops_cost > 3000") + .expect("ruleset compiles"); + mp.set_policy(std::sync::Arc::new(rs)); + + let held = mp + .accept_transaction(spend_p2wsh(&[prev(1)], &heavy, 1_000, 0x21), &cs, &NoopVerifier, TxSource::P2p, false) + .expect("admitted into quarantine"); + let free = mp + .accept_transaction(spend_p2wsh(&[prev(2)], &light, 1_000, 0x22), &cs, &NoopVerifier, TxSource::P2p, false) + .expect("admitted"); + let inner = mp.inner.read(); + assert!(inner.entries.get(&held).unwrap().scope.is_quarantined(), "3,941 is over 3,000"); + assert!(inner.entries.get(&free).unwrap().scope.is_acting()); + drop(inner); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// A `submitpackage` ephemeral-dust parent enters through the fee-bypass +/// path. It records the same sigop cost and prevout metadata as any other +/// admission; it recorded zero and nothing, so the template under-counted +/// it. +#[test] +fn a_fee_bypass_admission_records_its_sigop_cost_and_prevouts() { + let ws = heavy_script(); + let coins = vec![(prev(1), p2wsh_coin(&ws))]; + let (cs, mp, dir) = make_funded_template_env_with(&coins, Box::new(NoopVerifier)); + + let txid = mp + .accept_transaction_bypass_fee(spend_p2wsh(&[prev(1)], &ws, 0, 0x21), &cs, &NoopVerifier) + .expect("admitted"); + let entry = mp.get(&txid).unwrap(); + assert_eq!(entry.sigop_cost, HEAVY); + let spk = p2wsh_coin(&ws).script_pubkey; + assert_eq!(entry.prev_scripthashes, vec![scripthash_of(&spk)]); + // The default `streamprevoutmeta` level keeps amounts, not scripts. + assert_eq!(entry.prev_amounts, vec![COIN_VALUE]); + assert!(entry.prev_scripts.is_empty()); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// Six transactions of four heavy inputs each, 15,764 apiece by Core's +/// count. The template reserves 400 for the coinbase and refuses a +/// transaction that would bring the total to 80,000 (Core's +/// `TestChunkBlockLimits`, src/node/miner.cpp), so five fit (79,220) and +/// the sixth does not. getblocktemplate reports each one's cost, and the +/// block built from the template passes the structural validity check. +#[test] +fn a_template_keeps_to_the_sigop_limit_by_cores_count() { + let ws = heavy_script(); + let prevs: Vec = (1..=24).map(prev).collect(); + let coins: Vec<(OutPoint, Coin)> = prevs.iter().map(|p| (*p, p2wsh_coin(&ws))).collect(); + let (cs, mp, dir) = make_funded_template_env_with(&coins, Box::new(NoopVerifier)); + for (i, four) in prevs.chunks(4).enumerate() { + // Higher fee first, so the one left out is the last. + let fee = 10_000 - 1_000 * i as u64; + mp.accept_transaction(spend_p2wsh(four, &ws, fee, 0x30 + i as u8), &cs, &NoopVerifier, TxSource::Rpc, false) + .expect("admitted"); + } + + let template = crate::mining::template::create_template(&cs, &mp); + assert_eq!(template.transactions.len(), 5, "five of six fit under 80,000"); + for t in &template.transactions { + assert_eq!(t.sigop_cost, 4 * HEAVY); + } + + let gbt = crate::rpc::mining::get_block_template(&cs, &mp).unwrap(); + let reported: Vec = gbt["transactions"] + .as_array() + .unwrap() + .iter() + .map(|t| t["sigops"].as_u64().unwrap()) + .collect(); + assert_eq!(reported, vec![4 * HEAVY; 5]); + + let block = crate::mining::miner::build_block_to_script(&cs, &mp, ScriptBuf::new(), None) + .expect("the template's block passes the structural check"); + assert_eq!(block.txdata.len(), 6); + + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/node/src/stratum/template.rs b/node/src/stratum/template.rs index ffb38ef9b..1f836fdde 100644 --- a/node/src/stratum/template.rs +++ b/node/src/stratum/template.rs @@ -393,7 +393,11 @@ pub(crate) mod tests { let weight = coinbase.weight().to_wu() as usize; assert!(weight <= COINBASE_WEIGHT_RESERVE, "{payout}: {weight} WU"); // A coinbase's sigop cost, as `connect_block` counts it. - let sigops = coinbase.total_sigop_cost(|_| None) as u64; + let sigops = crate::validation::sigops::transaction_sigop_cost( + &coinbase, + &[], + crate::validation::sigops::SigOpFlags::P2shWitness, + ); assert!(sigops <= COINBASE_SIGOPS_RESERVE, "{payout}: {sigops} sigop cost"); if payout.is_p2pkh() { assert_eq!(sigops, 4, "the P2PKH payout's OP_CHECKSIG counts, and nothing else"); diff --git a/node/src/stratum/v2/jd.rs b/node/src/stratum/v2/jd.rs index 3a741cff4..2da08e75a 100644 --- a/node/src/stratum/v2/jd.rs +++ b/node/src/stratum/v2/jd.rs @@ -599,14 +599,15 @@ fn commits_to_height(script: &[u8], height: u32) -> bool { /// opcodes whose count cannot be known in advance, and the scriptSig is /// bounded at 20 per byte, the most any opcode counts (`OP_CHECKMULTISIG`). fn coinbase_sigop_cost_bound(coinbase: &Transaction, hole: std::ops::Range) -> u64 { - let outputs: usize = coinbase.output.iter().map(|o| o.script_pubkey.count_sigops_legacy()).sum(); + use crate::validation::sigops::{MAX_PUBKEYS_PER_MULTISIG, WITNESS_SCALE_FACTOR, script_sigop_count}; + let outputs: u64 = coinbase.output.iter().map(|o| script_sigop_count(&o.script_pubkey, false)).sum(); let script = &coinbase.input[0].script_sig; let script_sigops = if hole.is_empty() || hole_inside_one_push(script, &hole) { - script.count_sigops_legacy() + script_sigop_count(script, false) } else { - 20 * script.len() + MAX_PUBKEYS_PER_MULTISIG * script.len() as u64 }; - 4 * (outputs + script_sigops) as u64 + WITNESS_SCALE_FACTOR * (outputs + script_sigops) } /// Whether a single push's data covers all of `hole`. @@ -697,7 +698,8 @@ mod tests { by_wtxid: txs .iter() .map(|t| { - let sigops = t.total_sigop_cost(|_| None) as u64; + // Legacy only: these spend made-up outpoints. + let sigops = 4 * crate::validation::sigops::legacy_sigop_count(t); (t.compute_wtxid(), (t.clone(), FEE, t.weight().to_wu(), sigops)) }) .collect(), @@ -1010,14 +1012,17 @@ mod tests { assert_eq!(coinbase_sigop_cost_bound(&coinbase(pushed.clone()), 4..12), 80, "the output's 20, exactly"); // Filled with OP_CHECKMULTISIG bytes it still counts only the output. let filled = [&pushed[..4], &[0xae; 8]].concat(); - assert_eq!(coinbase(filled).total_sigop_cost(|_| None), 80); + let cost = |tx: &Transaction| { + crate::validation::sigops::transaction_sigop_cost(tx, &[], crate::validation::sigops::SigOpFlags::P2shWitness) + }; + assert_eq!(cost(&coinbase(filled)), 80); // The same hole with no push around it: 12 bytes of scriptSig at 20. let raw = [&[0x02, 0x41, 0x01][..], &[0; 8], &[0x51]].concat(); let bound = coinbase_sigop_cost_bound(&coinbase(raw.clone()), 3..11); assert_eq!(bound, 80 + 4 * 20 * 12); let worst = [&raw[..3], &[0xae; 8], &raw[11..]].concat(); - assert!(coinbase(worst).total_sigop_cost(|_| None) as u64 <= bound, "the bound holds for the worst filling"); + assert!(cost(&coinbase(worst)) <= bound, "the bound holds for the worst filling"); } #[test] diff --git a/node/src/validation/block.rs b/node/src/validation/block.rs index dcb637630..a0a1c9926 100644 --- a/node/src/validation/block.rs +++ b/node/src/validation/block.rs @@ -136,17 +136,12 @@ pub fn check_block( // p2sh sigops", but it is a hard ceiling a block cannot talk its way out // of, and it fires ahead of `bad-txns-inputs-missingorspent`. // `connect_block` still applies the full accurate count. - let mut legacy_sigops: usize = 0; + let mut legacy_sigops: u64 = 0; for tx in &block.txdata { - for input in &tx.input { - legacy_sigops = legacy_sigops.saturating_add(input.script_sig.count_sigops_legacy()); - } - for output in &tx.output { - legacy_sigops = - legacy_sigops.saturating_add(output.script_pubkey.count_sigops_legacy()); - } + legacy_sigops = + legacy_sigops.saturating_add(crate::validation::sigops::legacy_sigop_count(tx)); } - if legacy_sigops.saturating_mul(WITNESS_SCALE_FACTOR) > MAX_BLOCK_SIGOPS_COST { + if legacy_sigops.saturating_mul(WITNESS_SCALE_FACTOR as u64) > MAX_BLOCK_SIGOPS_COST as u64 { return Err(ValidationError::BadBlockSigops); } diff --git a/node/src/validation/mod.rs b/node/src/validation/mod.rs index 9206271b6..e768971ce 100644 --- a/node/src/validation/mod.rs +++ b/node/src/validation/mod.rs @@ -2,6 +2,7 @@ pub mod block; pub mod pow; pub mod script; pub mod signet; +pub mod sigops; pub mod tx; pub mod versionbits; diff --git a/node/src/validation/sigops.rs b/node/src/validation/sigops.rs new file mode 100644 index 000000000..9376251a4 --- /dev/null +++ b/node/src/validation/sigops.rs @@ -0,0 +1,326 @@ +//! Signature-operation counting, ported from Bitcoin Core. +//! +//! Every sigop number satd acts on comes from here: the block limit in +//! `connect_block` and `check_block`, the cost the mempool records on each +//! entry (which the block template budgets by and getblocktemplate reports), +//! the policy engine's `tx.sigops_cost`, and the Stratum coinbase bound. +//! +//! | here | Bitcoin Core | +//! |---|---| +//! | [`script_sigop_count`] | `CScript::GetSigOpCount(bool fAccurate)`, src/script/script.cpp | +//! | [`p2sh_sigop_count`] | `CScript::GetSigOpCount(const CScript& scriptSig)`, src/script/script.cpp | +//! | [`witness_sigop_count`] | `CountWitnessSigOps`, src/script/interpreter.cpp | +//! | [`legacy_sigop_count`] | `GetLegacySigOpCount`, src/consensus/tx_verify.cpp | +//! | [`transaction_p2sh_sigop_count`] | `GetP2SHSigOpCount`, src/consensus/tx_verify.cpp | +//! | [`transaction_sigop_cost`] | `GetTransactionSigOpCost`, src/consensus/tx_verify.cpp | +//! +//! rust-bitcoin 0.32's `Script::count_sigops` is not used. In accurate mode +//! it keeps the last `OP_1`..`OP_16` it saw until a push or a +//! non-signature opcode replaces it, so an `OP_CHECKMULTISIG` that follows +//! `OP_CHECKSIG` or another `OP_CHECKMULTISIG` takes that earlier number. +//! Core sets `lastOpcode` after every opcode, and such an `OP_CHECKMULTISIG` +//! counts 20: `OP_1 OP_CHECKSIG OP_CHECKMULTISIG` is 21 in Core and 2 in +//! rust-bitcoin 0.32 (rust-bitcoin issue #6368). The parsing here is Core's +//! `GetScriptOp` too, so a truncated push ends a count exactly where Core's +//! does. + +use bitcoin::opcodes::all::{ + OP_CHECKMULTISIG as CHECKMULTISIG, OP_CHECKMULTISIGVERIFY as CHECKMULTISIGVERIFY, + OP_CHECKSIG as CHECKSIG, OP_CHECKSIGVERIFY as CHECKSIGVERIFY, OP_EQUAL as EQUAL, + OP_HASH160 as HASH160, OP_INVALIDOPCODE as INVALIDOPCODE, OP_PUSHDATA1 as PUSHDATA1, + OP_PUSHDATA2 as PUSHDATA2, OP_PUSHDATA4 as PUSHDATA4, OP_PUSHNUM_1 as PUSHNUM_1, + OP_PUSHNUM_16 as PUSHNUM_16, +}; +use bitcoin::{Script, Transaction, TxOut, Witness}; + +/// Core's `MAX_PUBKEYS_PER_MULTISIG` (src/script/script.h): what a +/// multisig opcode counts when the count is not accurate, or when the opcode +/// before it is not `OP_1`..`OP_16`. +pub const MAX_PUBKEYS_PER_MULTISIG: u64 = 20; + +/// Core's `WITNESS_SCALE_FACTOR` (src/consensus/consensus.h): legacy and +/// P2SH sigops cost this much each, witness sigops one. +pub const WITNESS_SCALE_FACTOR: u64 = 4; + +/// Core's `WITNESS_V0_KEYHASH_SIZE` and `WITNESS_V0_SCRIPTHASH_SIZE` +/// (src/script/interpreter.h). +const WITNESS_V0_KEYHASH_SIZE: usize = 20; +const WITNESS_V0_SCRIPTHASH_SIZE: usize = 32; + +const OP_0: u8 = 0x00; +const OP_PUSHDATA1: u8 = PUSHDATA1.to_u8(); +const OP_PUSHDATA2: u8 = PUSHDATA2.to_u8(); +const OP_PUSHDATA4: u8 = PUSHDATA4.to_u8(); +const OP_1: u8 = PUSHNUM_1.to_u8(); +const OP_16: u8 = PUSHNUM_16.to_u8(); +const OP_EQUAL: u8 = EQUAL.to_u8(); +const OP_HASH160: u8 = HASH160.to_u8(); +const OP_CHECKSIG: u8 = CHECKSIG.to_u8(); +const OP_CHECKSIGVERIFY: u8 = CHECKSIGVERIFY.to_u8(); +const OP_CHECKMULTISIG: u8 = CHECKMULTISIG.to_u8(); +const OP_CHECKMULTISIGVERIFY: u8 = CHECKMULTISIGVERIFY.to_u8(); +const OP_INVALIDOPCODE: u8 = INVALIDOPCODE.to_u8(); + +/// The script-verify flags Core's `GetTransactionSigOpCost` reads. +/// +/// `SCRIPT_VERIFY_P2SH` adds the sigops of P2SH redeem scripts and +/// `SCRIPT_VERIFY_WITNESS` those of witness programs. Core's +/// `CountWitnessSigOps` asserts P2SH whenever WITNESS is set, so these are +/// the only three combinations. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SigOpFlags { + /// Neither flag: legacy sigops only. + Legacy, + /// `SCRIPT_VERIFY_P2SH` alone. + P2sh, + /// `SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS`. Core's mempool counts + /// with these (both are in its standard script flags), and Core's + /// `GetBlockScriptFlags` sets both on every block but two exceptions. + P2shWitness, +} + +impl SigOpFlags { + fn p2sh(self) -> bool { + !matches!(self, SigOpFlags::Legacy) + } + + fn witness(self) -> bool { + matches!(self, SigOpFlags::P2shWitness) + } +} + +/// Core's `GetScriptOp`: the opcode at `*pc` and the data it pushes (empty +/// for an opcode that pushes nothing), with `*pc` moved past both. `None` +/// where Core returns false: past the end of the script, or a push whose +/// length bytes or data run past it. +fn get_op<'a>(script: &'a [u8], pc: &mut usize) -> Option<(u8, &'a [u8])> { + let opcode = *script.get(*pc)?; + *pc += 1; + let size = match opcode { + OP_PUSHDATA1 => { + let n = *script.get(*pc)?; + *pc += 1; + usize::from(n) + } + OP_PUSHDATA2 => { + let b = script.get(*pc..*pc + 2)?; + *pc += 2; + usize::from(u16::from_le_bytes([b[0], b[1]])) + } + OP_PUSHDATA4 => { + let b = script.get(*pc..*pc + 4)?; + *pc += 4; + u32::from_le_bytes([b[0], b[1], b[2], b[3]]) as usize + } + n if n < OP_PUSHDATA1 => usize::from(n), + _ => return Some((opcode, &[])), + }; + let data = script.get(*pc..pc.checked_add(size)?)?; + *pc += size; + Some((opcode, data)) +} + +/// Core's `CScript::DecodeOP_N`, for `OP_0` and `OP_1`..`OP_16`. +fn decode_op_n(opcode: u8) -> u8 { + debug_assert!(opcode == OP_0 || (OP_1..=OP_16).contains(&opcode)); + if opcode == OP_0 { 0 } else { opcode - (OP_1 - 1) } +} + +/// Core's `CScript::IsPayToScriptHash`. +fn is_pay_to_script_hash(script: &[u8]) -> bool { + script.len() == 23 && script[0] == OP_HASH160 && script[1] == 0x14 && script[22] == OP_EQUAL +} + +/// Core's `CScript::IsPushOnly`: every opcode parses and is `OP_16` or +/// below (which takes in `OP_1NEGATE` and `OP_RESERVED`). +fn is_push_only(script: &[u8]) -> bool { + let mut pc = 0; + while pc < script.len() { + match get_op(script, &mut pc) { + Some((opcode, _)) if opcode <= OP_16 => {} + _ => return false, + } + } + true +} + +/// Core's `CScript::IsWitnessProgram`: a version opcode (`OP_0`, +/// `OP_1`..`OP_16`) and one direct push of 2 to 40 bytes that ends the +/// script. Returns the version and the program. +fn witness_program(script: &[u8]) -> Option<(u8, &[u8])> { + if script.len() < 4 || script.len() > 42 { + return None; + } + if script[0] != OP_0 && !(OP_1..=OP_16).contains(&script[0]) { + return None; + } + if usize::from(script[1]) + 2 == script.len() { + return Some((decode_op_n(script[0]), &script[2..])); + } + None +} + +/// Core's `CScript::GetSigOpCount(bool fAccurate)`. +/// +/// `OP_CHECKSIG` and `OP_CHECKSIGVERIFY` count 1. `OP_CHECKMULTISIG` and +/// `OP_CHECKMULTISIGVERIFY` count the number pushed by the opcode right +/// before them when `accurate` and that opcode is `OP_1`..`OP_16`, and 20 +/// otherwise. Counting stops at the first opcode that does not parse. +pub fn script_sigop_count(script: &Script, accurate: bool) -> u64 { + let script = script.as_bytes(); + let mut n = 0; + let mut pc = 0; + let mut last_opcode = OP_INVALIDOPCODE; + while pc < script.len() { + let Some((opcode, _)) = get_op(script, &mut pc) else { + break; + }; + if opcode == OP_CHECKSIG || opcode == OP_CHECKSIGVERIFY { + n += 1; + } else if opcode == OP_CHECKMULTISIG || opcode == OP_CHECKMULTISIGVERIFY { + if accurate && (OP_1..=OP_16).contains(&last_opcode) { + n += u64::from(decode_op_n(last_opcode)); + } else { + n += MAX_PUBKEYS_PER_MULTISIG; + } + } + last_opcode = opcode; + } + n +} + +/// Core's `CScript::GetSigOpCount(const CScript& scriptSig)`: the sigops of +/// the redeem script `script_sig` reveals for the P2SH `script_pubkey`. +/// +/// The redeem script is what the last opcode of `script_sig` pushes, counted +/// accurately. A `script_sig` with an opcode above `OP_16`, or one that does +/// not parse, counts 0, and so does one that ends in a number opcode, which +/// pushes no data. A `script_pubkey` that is not P2SH is counted accurately +/// itself. +pub fn p2sh_sigop_count(script_pubkey: &Script, script_sig: &Script) -> u64 { + if !is_pay_to_script_hash(script_pubkey.as_bytes()) { + return script_sigop_count(script_pubkey, true); + } + let sig = script_sig.as_bytes(); + let mut pc = 0; + let mut data: &[u8] = &[]; + while pc < sig.len() { + let Some((opcode, pushed)) = get_op(sig, &mut pc) else { + return 0; + }; + if opcode > OP_16 { + return 0; + } + data = pushed; + } + script_sigop_count(Script::from_bytes(data), true) +} + +/// Core's `WitnessSigOps`: a version 0 key-hash program costs 1, a version 0 +/// script-hash program the accurate count of the witness script (the last +/// witness item), and anything else 0. +fn witness_program_sigops(version: u8, program: &[u8], witness: &Witness) -> u64 { + if version == 0 { + if program.len() == WITNESS_V0_KEYHASH_SIZE { + return 1; + } + if program.len() == WITNESS_V0_SCRIPTHASH_SIZE + && let Some(witness_script) = witness.last() + { + return script_sigop_count(Script::from_bytes(witness_script), true); + } + } + 0 +} + +/// Core's `CountWitnessSigOps`: the witness sigops of one input that spends +/// `script_pubkey`, either a witness program itself or P2SH wrapping one in a +/// push-only `script_sig`. 0 unless `flags` has `SCRIPT_VERIFY_WITNESS`. +pub fn witness_sigop_count( + script_sig: &Script, + script_pubkey: &Script, + witness: &Witness, + flags: SigOpFlags, +) -> u64 { + if !flags.witness() { + return 0; + } + if let Some((version, program)) = witness_program(script_pubkey.as_bytes()) { + return witness_program_sigops(version, program, witness); + } + let sig = script_sig.as_bytes(); + if is_pay_to_script_hash(script_pubkey.as_bytes()) && is_push_only(sig) { + let mut pc = 0; + let mut data: &[u8] = &[]; + while pc < sig.len() { + // `is_push_only` parsed every opcode already. + let Some((_, pushed)) = get_op(sig, &mut pc) else { + break; + }; + data = pushed; + } + if let Some((version, program)) = witness_program(data) { + return witness_program_sigops(version, program, witness); + } + } + 0 +} + +/// Core's `GetLegacySigOpCount`: the inaccurate count of every scriptSig and +/// every output script of `tx`. +pub fn legacy_sigop_count(tx: &Transaction) -> u64 { + let inputs: u64 = tx.input.iter().map(|i| script_sigop_count(&i.script_sig, false)).sum(); + let outputs: u64 = tx.output.iter().map(|o| script_sigop_count(&o.script_pubkey, false)).sum(); + inputs + outputs +} + +/// Panics unless `prevouts` holds one output per input, as Core's +/// `assert(!coin.IsSpent())` does for a coin that is not there. +fn assert_prevouts(tx: &Transaction, prevouts: &[TxOut]) { + assert_eq!( + prevouts.len(), + tx.input.len(), + "sigop counting needs the output each input spends, in input order" + ); +} + +/// Core's `GetP2SHSigOpCount`: the redeem-script sigops of every input of +/// `tx` that spends a P2SH output. `prevouts` holds the output each input +/// spends, in input order. A coinbase counts 0 and its `prevouts` are not +/// read. +pub fn transaction_p2sh_sigop_count(tx: &Transaction, prevouts: &[TxOut]) -> u64 { + if tx.is_coinbase() { + return 0; + } + assert_prevouts(tx, prevouts); + tx.input + .iter() + .zip(prevouts) + .filter(|(_, prevout)| is_pay_to_script_hash(prevout.script_pubkey.as_bytes())) + .map(|(input, prevout)| p2sh_sigop_count(&prevout.script_pubkey, &input.script_sig)) + .sum() +} + +/// Core's `GetTransactionSigOpCost`: legacy sigops × 4, plus P2SH sigops × 4 +/// under `SCRIPT_VERIFY_P2SH`, plus witness sigops under +/// `SCRIPT_VERIFY_WITNESS`. A coinbase counts its legacy sigops alone and its +/// `prevouts` are not read; any other transaction needs the output each +/// input spends, in input order. +pub fn transaction_sigop_cost(tx: &Transaction, prevouts: &[TxOut], flags: SigOpFlags) -> u64 { + let mut cost = legacy_sigop_count(tx) * WITNESS_SCALE_FACTOR; + if tx.is_coinbase() { + return cost; + } + assert_prevouts(tx, prevouts); + if flags.p2sh() { + cost += transaction_p2sh_sigop_count(tx, prevouts) * WITNESS_SCALE_FACTOR; + } + for (input, prevout) in tx.input.iter().zip(prevouts) { + cost += witness_sigop_count(&input.script_sig, &prevout.script_pubkey, &input.witness, flags); + } + cost +} + +#[cfg(test)] +#[path = "sigops_tests.rs"] +mod tests; diff --git a/node/src/validation/sigops_tests.rs b/node/src/validation/sigops_tests.rs new file mode 100644 index 000000000..1710c06d0 --- /dev/null +++ b/node/src/validation/sigops_tests.rs @@ -0,0 +1,486 @@ +//! Bitcoin Core's sigop-counting test vectors, and the cases where the +//! opcode before a multisig opcode decides its count. +//! +//! The ports keep Core's test and variable names. Keys and signatures are +//! placeholders of the right length: no count reads their bytes. + +use super::*; +use bitcoin::hashes::Hash; +use bitcoin::{Amount, OutPoint, ScriptBuf, Sequence, TxIn, Txid}; + +const OP_2: u8 = 0x52; +const OP_3: u8 = 0x53; +const OP_11: u8 = 0x5b; +const OP_15: u8 = 0x5f; +const OP_IF: u8 = 0x63; +const OP_ENDIF: u8 = 0x68; +const OP_DROP: u8 = 0x75; +const OP_2DUP: u8 = 0x6e; +const OP_NOT: u8 = 0x91; +const OP_NOP: u8 = 0x61; +const OP_RESERVED: u8 = 0x50; +const OP_1NEGATE: u8 = 0x4f; + +/// A script built the way Core's `CScript` `operator<<` builds one. +#[derive(Clone, Default)] +struct S(Vec); + +impl S { + fn new() -> S { + S::default() + } + + /// `<< opcode`. + fn op(mut self, opcode: u8) -> S { + self.0.push(opcode); + self + } + + /// `<< std::vector`: a direct push below 76 bytes, then + /// `OP_PUSHDATA1`/`2`/`4`. Never an `OP_N`, even for one small byte. + fn push(mut self, data: &[u8]) -> S { + let n = data.len(); + if n < usize::from(OP_PUSHDATA1) { + self.0.push(n as u8); + } else if n <= 0xff { + self.0.extend([OP_PUSHDATA1, n as u8]); + } else if n <= 0xffff { + self.0.push(OP_PUSHDATA2); + self.0.extend((n as u16).to_le_bytes()); + } else { + self.0.push(OP_PUSHDATA4); + self.0.extend((n as u32).to_le_bytes()); + } + self.0.extend_from_slice(data); + self + } + + fn script(&self) -> ScriptBuf { + ScriptBuf::from_bytes(self.0.clone()) + } +} + +fn script(bytes: &[u8]) -> ScriptBuf { + ScriptBuf::from_bytes(bytes.to_vec()) +} + +fn accurate(bytes: &[u8]) -> u64 { + script_sigop_count(&script(bytes), true) +} + +fn inaccurate(bytes: &[u8]) -> u64 { + script_sigop_count(&script(bytes), false) +} + +/// A compressed public key's worth of bytes. +fn pubkey(tag: u8) -> [u8; 33] { + let mut k = [tag; 33]; + k[0] = 0x02; + k +} + +/// A DER signature's worth of bytes. +const SIG: [u8; 72] = [0x30; 72]; + +/// Core's `GetScriptForDestination(ScriptHash(s))`. +fn p2sh_of(s: &ScriptBuf) -> ScriptBuf { + ScriptBuf::new_p2sh(&s.script_hash()) +} + +/// Core's `GetScriptForMultisig(n, keys)`. +fn multisig(n: u8, keys: &[[u8; 33]]) -> S { + let mut s = S::new().op(OP_1 + n - 1); + for k in keys { + s = s.push(k); + } + s.op(OP_1 + keys.len() as u8 - 1).op(OP_CHECKMULTISIG) +} + +fn txid(tag: u8) -> Txid { + Txid::from_byte_array([tag; 32]) +} + +fn input(prevout: OutPoint, script_sig: ScriptBuf, witness: Witness) -> TxIn { + TxIn { previous_output: prevout, script_sig, sequence: Sequence::MAX, witness } +} + +fn tx(input: Vec, output: Vec) -> Transaction { + Transaction { + version: bitcoin::transaction::Version(1), + lock_time: bitcoin::absolute::LockTime::ZERO, + input, + output, + } +} + +fn out(script_pubkey: ScriptBuf) -> TxOut { + TxOut { value: Amount::from_sat(1), script_pubkey } +} + +/// Core: src/test/sigopcount_tests.cpp, `GetSigOpCount`. +#[test] +fn core_get_sig_op_count() { + let s1 = S::new(); + assert_eq!(script_sigop_count(&s1.script(), false), 0); + assert_eq!(script_sigop_count(&s1.script(), true), 0); + + let dummy = [0u8; 20]; + let s1 = s1.op(OP_1).push(&dummy).push(&dummy).op(OP_2).op(OP_CHECKMULTISIG); + assert_eq!(script_sigop_count(&s1.script(), true), 2); + let s1 = s1.op(OP_IF).op(OP_CHECKSIG).op(OP_ENDIF); + assert_eq!(script_sigop_count(&s1.script(), true), 3); + assert_eq!(script_sigop_count(&s1.script(), false), 21); + + let p2sh = p2sh_of(&s1.script()); + let script_sig = S::new().op(OP_0).push(&s1.0).script(); + assert_eq!(p2sh_sigop_count(&p2sh, &script_sig), 3); + + let keys = [pubkey(1), pubkey(2), pubkey(3)]; + let s2 = multisig(1, &keys); + assert_eq!(script_sigop_count(&s2.script(), true), 3); + assert_eq!(script_sigop_count(&s2.script(), false), 20); + + let p2sh = p2sh_of(&s2.script()); + assert_eq!(script_sigop_count(&p2sh, true), 0); + assert_eq!(script_sigop_count(&p2sh, false), 0); + let script_sig2 = S::new().op(OP_1).push(&dummy).push(&dummy).push(&s2.0).script(); + assert_eq!(p2sh_sigop_count(&p2sh, &script_sig2), 3); +} + +/// Core reads the opcode immediately before a multisig opcode +/// (`lastOpcode`, set after every opcode). Only `OP_1`..`OP_16` there gives +/// an accurate count below 20. +#[test] +fn a_multisig_count_reads_the_opcode_right_before_it() { + // OP_1 OP_CHECKSIG OP_CHECKMULTISIG: 1 + 20. rust-bitcoin 0.32 says 2. + assert_eq!(accurate(&[0x51, 0xac, 0xae]), 21); + assert_eq!(inaccurate(&[0x51, 0xac, 0xae]), 21); + // OP_1 OP_CHECKMULTISIG OP_CHECKMULTISIG: 1 + 20. rust-bitcoin 0.32 says 2. + assert_eq!(accurate(&[0x51, 0xae, 0xae]), 21); + assert_eq!(inaccurate(&[0x51, 0xae, 0xae]), 40); + // The VERIFY forms are the same opcodes to the count. + assert_eq!(accurate(&[0x51, 0xad, 0xaf]), 21); + assert_eq!(accurate(&[0x51, 0xaf, 0xaf]), 21); + assert_eq!(accurate(&[0x52, 0xae]), 2); + assert_eq!(accurate(&[0x60, 0xaf]), 16); + // OP_1NEGATE and OP_0 are not OP_1..OP_16. + assert_eq!(accurate(&[OP_1NEGATE, 0xae]), 20); + assert_eq!(accurate(&[OP_0, 0xae]), 20); + // Neither is a push of the same byte, nor a NOP in between. + assert_eq!(accurate(&[0x01, 0x01, 0xae]), 20); + assert_eq!(accurate(&[0x51, OP_NOP, 0xae]), 20); + assert_eq!(accurate(&[0x51, 0x01, 0x01, 0xae]), 20); + + // 198 multisigs after one OP_1, in a branch that never runs: 1 + 197 × 20. + let mut heavy = vec![OP_0, OP_IF, 0x51]; + heavy.extend([0xae; 198]); + heavy.extend([OP_ENDIF, 0x51]); + assert_eq!(accurate(&heavy), 3_941); +} + +/// Core's `GetScriptOp` fails on a push that runs past the end, and the +/// count keeps what it had (`if (!GetOp(pc, opcode)) break;`). +#[test] +fn a_count_stops_where_the_script_stops_parsing() { + assert_eq!(accurate(&[0xac, OP_PUSHDATA1]), 1); + assert_eq!(accurate(&[0xac, 0x03, 0x00, 0xac]), 1); + assert_eq!(accurate(&[0xac, OP_PUSHDATA2, 0x01]), 1); + assert_eq!(accurate(&[0xac, OP_PUSHDATA4, 0xff, 0xff, 0xff, 0xff, 0xac]), 1); + // A push that fits, including a zero-length PUSHDATA, is skipped whole. + assert_eq!(accurate(&[OP_PUSHDATA1, 0x00, 0xac]), 1); + assert_eq!(accurate(&[OP_PUSHDATA2, 0x01, 0x00, 0xae, 0xac]), 1); + assert_eq!(accurate(&[OP_PUSHDATA4, 0x02, 0, 0, 0, 0xae, 0xae, 0xac]), 1); + assert_eq!(accurate(&[0x02, 0xac, 0xac]), 0); +} + +/// Core's `GetSigOpCount(scriptSig)` counts the data of the scriptSig's last +/// opcode, and only when every opcode parses and is `OP_16` or below. +#[test] +fn p2sh_counts_the_last_push_of_a_push_only_script_sig() { + let redeem = script(&[0x51, 0xac, 0xae]); + let p2sh = p2sh_of(&redeem); + assert_eq!(p2sh_sigop_count(&p2sh, &S::new().push(redeem.as_bytes()).script()), 21); + // OP_RESERVED and OP_1NEGATE are at or below OP_16. + let sig = S::new().op(OP_RESERVED).op(OP_1NEGATE).push(redeem.as_bytes()).script(); + assert_eq!(p2sh_sigop_count(&p2sh, &sig), 21); + // Any push form. + let mut long_redeem = redeem.as_bytes().to_vec(); + long_redeem.extend([OP_NOP; 300]); + assert_eq!(p2sh_sigop_count(&p2sh, &S::new().push(&long_redeem).script()), 21); + // A NOP anywhere: 0. + let sig = S::new().op(OP_NOP).push(redeem.as_bytes()).script(); + assert_eq!(p2sh_sigop_count(&p2sh, &sig), 0); + // Ending on a number opcode pushes no data: 0. + let sig = S::new().push(redeem.as_bytes()).op(OP_1).script(); + assert_eq!(p2sh_sigop_count(&p2sh, &sig), 0); + // Nothing, or a scriptSig that does not parse: 0. + assert_eq!(p2sh_sigop_count(&p2sh, &ScriptBuf::new()), 0); + let mut truncated = S::new().push(redeem.as_bytes()).0; + truncated.push(0x05); + assert_eq!(p2sh_sigop_count(&p2sh, &script(&truncated)), 0); + // A push that claims more bytes than follow is not a redeem script, even + // when the bytes that do follow would count: 0, not 21. + assert_eq!(p2sh_sigop_count(&p2sh, &script(&[0x05, 0x51, 0xac, 0xae])), 0); + // A scriptPubKey that is not P2SH is counted itself, accurately. + assert_eq!(p2sh_sigop_count(&redeem, &ScriptBuf::new()), 21); +} + +/// Core: src/test/script_p2sh_tests.cpp, `AreInputsStandard`, the +/// `GetP2SHSigOpCount` checks. +#[test] +fn core_are_inputs_standard_p2sh_sig_op_counts() { + let key: Vec<[u8; 33]> = (1..=6).map(pubkey).collect(); + let pay1 = S::new().push(&key[0]).op(OP_CHECKSIG); + let pay1of3 = multisig(1, &key[0..3]); + let one_and_two = S::new() + .op(OP_1) + .push(&key[0]) + .push(&key[1]) + .push(&key[2]) + .op(OP_3) + .op(OP_CHECKMULTISIGVERIFY) + .op(OP_2) + .push(&key[3]) + .push(&key[4]) + .push(&key[5]) + .op(OP_3) + .op(OP_CHECKMULTISIG); + let mut fifteen_sigops = S::new().op(OP_1); + for i in 0..15 { + fifteen_sigops = fifteen_sigops.push(&key[i % 3]); + } + let fifteen_sigops = fifteen_sigops.op(OP_15).op(OP_CHECKMULTISIG); + let sixteen_sigops = S::new().op(OP_16).op(OP_CHECKMULTISIG); + let twenty_sigops = S::new().op(OP_CHECKMULTISIG); + + let tx_from: Vec = vec![ + out(p2sh_of(&pay1.script())), + out(pay1.script()), + out(pay1of3.script()), + out(p2sh_of(&one_and_two.script())), + out(p2sh_of(&fifteen_sigops.script())), + out(p2sh_of(&sixteen_sigops.script())), + out(p2sh_of(&twenty_sigops.script())), + ]; + let from = txid(0xf0); + let spend = |vout: u32, script_sig: ScriptBuf| input(OutPoint { txid: from, vout }, script_sig, Witness::new()); + + // What SignSignature produces for vin[0..3], then Core's dummy + // scriptSigs for vin[3] and vin[4]. + let tx_to = tx( + vec![ + spend(0, S::new().push(&SIG).push(&pay1.0).script()), + spend(1, S::new().push(&SIG).script()), + spend(2, S::new().op(OP_0).push(&SIG).script()), + spend(3, S::new().op(OP_11).op(OP_11).push(&one_and_two.0).script()), + spend(4, S::new().push(&fifteen_sigops.0).script()), + ], + vec![out(ScriptBuf::new())], + ); + // 22 P2SH sigops for all inputs (1 for vin[0], 6 for vin[3], 15 for vin[4]). + assert_eq!(transaction_p2sh_sigop_count(&tx_to, &tx_from[0..5]), 22); + + let coinbase_tx = tx(vec![input(OutPoint::null(), ScriptBuf::new(), Witness::new())], vec![]); + assert!(coinbase_tx.is_coinbase()); + assert_eq!(transaction_p2sh_sigop_count(&coinbase_tx, &[]), 0); + + let tx_to_non_std1 = tx(vec![spend(5, S::new().push(&sixteen_sigops.0).script())], vec![]); + assert_eq!(transaction_p2sh_sigop_count(&tx_to_non_std1, &tx_from[5..6]), 16); + + let tx_to_non_std2 = tx(vec![spend(6, S::new().push(&twenty_sigops.0).script())], vec![]); + assert_eq!(transaction_p2sh_sigop_count(&tx_to_non_std2, &tx_from[6..7]), 20); +} + +/// Core: src/test/transaction_tests.cpp, `max_standard_legacy_sigops`, the +/// `GetP2SHSigOpCount` checks. +#[test] +fn core_max_standard_legacy_sigops_p2sh_counts() { + let mut redeem = S::new().push(&[]).push(&pubkey(7)); + for _ in 0..14 { + redeem = redeem.op(OP_2DUP).op(OP_CHECKSIG).op(OP_DROP); + } + let redeem = redeem.op(OP_CHECKSIG).op(OP_NOT); + let p2sh = p2sh_of(&redeem.script()); + + for (inputs, expected) in [(166u32, 2_490), (167, 2_505)] { + let tx_max_sigops = tx( + (0..inputs) + .map(|i| input(OutPoint { txid: txid(0xf1), vout: i }, S::new().push(&redeem.0).script(), Witness::new())) + .collect(), + vec![], + ); + let prevouts = vec![out(p2sh.clone()); inputs as usize]; + assert_eq!(transaction_p2sh_sigop_count(&tx_max_sigops, &prevouts), expected); + } +} + +/// Core: src/test/sigopcount_tests.cpp, `GetTxSigOpCost`. +#[test] +fn core_get_tx_sig_op_cost() { + let flags = SigOpFlags::P2shWitness; + let pk = pubkey(9); + let creation = |script_pubkey: ScriptBuf| { + tx(vec![input(OutPoint::null(), ScriptBuf::new(), Witness::new())], vec![out(script_pubkey)]) + }; + let spending = |creation_tx: &Transaction, script_sig: ScriptBuf, witness: Witness| { + tx( + vec![input(OutPoint { txid: creation_tx.compute_txid(), vout: 0 }, script_sig, witness)], + vec![out(ScriptBuf::new())], + ) + }; + let two_empty = || Witness::from_slice(&[&[][..], &[][..]]); + let multisig_1_of_2 = S::new().op(OP_1).push(&pk).push(&pk).op(OP_2).op(OP_CHECKMULTISIGVERIFY); + + // Multisig script (legacy counting). + { + let script_pubkey = multisig_1_of_2.script(); + let script_sig = S::new().op(OP_0).op(OP_0).script(); + let creation_tx = creation(script_pubkey); + let spending_tx = spending(&creation_tx, script_sig, Witness::new()); + let prevouts = creation_tx.output.clone(); + // Legacy counting only includes signature operations in scriptSigs + // and scriptPubKeys of a transaction. + assert_eq!(transaction_sigop_cost(&spending_tx, &prevouts, flags), 0); + // creationTx contains two signature operations in its scriptPubKey, + // but legacy counting is not accurate. + assert_eq!( + transaction_sigop_cost(&creation_tx, &[], flags), + MAX_PUBKEYS_PER_MULTISIG * WITNESS_SCALE_FACTOR + ); + } + + // Multisig nested in P2SH. + { + let redeem_script = multisig_1_of_2.clone(); + let creation_tx = creation(p2sh_of(&redeem_script.script())); + let script_sig = S::new().op(OP_0).op(OP_0).push(&redeem_script.0).script(); + let spending_tx = spending(&creation_tx, script_sig, Witness::new()); + let prevouts = creation_tx.output.clone(); + assert_eq!(transaction_sigop_cost(&spending_tx, &prevouts, flags), 2 * WITNESS_SCALE_FACTOR); + // P2SH sigops are not counted without SCRIPT_VERIFY_P2SH. + assert_eq!(transaction_sigop_cost(&spending_tx, &prevouts, SigOpFlags::Legacy), 0); + } + + // P2WPKH witness program. + { + let mut script_pubkey = S::new().op(OP_0).push(&[0x33; 20]).0; + let creation_tx = creation(script(&script_pubkey)); + let spending_tx = spending(&creation_tx, ScriptBuf::new(), two_empty()); + let prevouts = creation_tx.output.clone(); + assert_eq!(transaction_sigop_cost(&spending_tx, &prevouts, flags), 1); + // No signature operations if we don't verify the witness. + assert_eq!(transaction_sigop_cost(&spending_tx, &prevouts, SigOpFlags::P2sh), 0); + + // The sig op cost for witness version != 0 is zero. + assert_eq!(script_pubkey[0], 0x00); + script_pubkey[0] = 0x51; + let creation_tx = creation(script(&script_pubkey)); + let spending_tx = spending(&creation_tx, ScriptBuf::new(), two_empty()); + assert_eq!(transaction_sigop_cost(&spending_tx, &creation_tx.output, flags), 0); + script_pubkey[0] = 0x00; + let creation_tx = creation(script(&script_pubkey)); + let mut spending_tx = spending(&creation_tx, ScriptBuf::new(), two_empty()); + + // The witness of a coinbase transaction is not taken into account. + spending_tx.input[0].previous_output = OutPoint::null(); + assert_eq!(transaction_sigop_cost(&spending_tx, &creation_tx.output, flags), 0); + } + + // P2WPKH nested in P2SH. + { + let program = S::new().op(OP_0).push(&[0x33; 20]); + let creation_tx = creation(p2sh_of(&program.script())); + let script_sig = S::new().push(&program.0).script(); + let spending_tx = spending(&creation_tx, script_sig, two_empty()); + assert_eq!(transaction_sigop_cost(&spending_tx, &creation_tx.output, flags), 1); + } + + // P2WSH witness program. + { + let witness_script = multisig_1_of_2.script(); + let creation_tx = creation(ScriptBuf::new_p2wsh(&witness_script.wscript_hash())); + let witness = Witness::from_slice(&[&[][..], &[][..], witness_script.as_bytes()]); + let spending_tx = spending(&creation_tx, ScriptBuf::new(), witness); + assert_eq!(transaction_sigop_cost(&spending_tx, &creation_tx.output, flags), 2); + assert_eq!(transaction_sigop_cost(&spending_tx, &creation_tx.output, SigOpFlags::P2sh), 0); + } + + // P2WSH nested in P2SH. + { + let witness_script = multisig_1_of_2.script(); + let redeem_script = ScriptBuf::new_p2wsh(&witness_script.wscript_hash()); + let creation_tx = creation(p2sh_of(&redeem_script)); + let script_sig = S::new().push(redeem_script.as_bytes()).script(); + let witness = Witness::from_slice(&[&[][..], &[][..], witness_script.as_bytes()]); + let spending_tx = spending(&creation_tx, script_sig, witness); + assert_eq!(transaction_sigop_cost(&spending_tx, &creation_tx.output, flags), 2); + } +} + +/// Witness sigops follow Core's `IsWitnessProgram` and `WitnessSigOps`, and +/// the witness script is counted accurately. +#[test] +fn witness_sigops_follow_cores_program_rules() { + let flags = SigOpFlags::P2shWitness; + let ws = script(&[0x51, 0xac, 0xae]); + let p2wsh = ScriptBuf::new_p2wsh(&ws.wscript_hash()); + let with_ws = Witness::from_slice(&[ws.as_bytes()]); + // The witness script is the last item, counted accurately: 21. + assert_eq!(witness_sigop_count(&ScriptBuf::new(), &p2wsh, &with_ws, flags), 21); + // An empty witness has no script: 0. + assert_eq!(witness_sigop_count(&ScriptBuf::new(), &p2wsh, &Witness::new(), flags), 0); + // P2SH-wrapped, but the scriptSig is not push-only: 0. + let wrapped = p2sh_of(&p2wsh); + let sig = S::new().op(OP_NOP).push(p2wsh.as_bytes()).script(); + assert_eq!(witness_sigop_count(&sig, &wrapped, &with_ws, flags), 0); + let sig = S::new().push(p2wsh.as_bytes()).script(); + assert_eq!(witness_sigop_count(&sig, &wrapped, &with_ws, flags), 21); + // A scriptSig whose push runs one byte past the end is not push-only. + let mut short = sig.as_bytes().to_vec(); + short[0] += 1; + assert_eq!(witness_sigop_count(&script(&short), &wrapped, &with_ws, flags), 0); + // A version 0 program of another length, a 41-byte program, and a + // push that does not end the script are not counted. + let v0_40 = S::new().op(OP_0).push(&[0x44; 40]).script(); + assert_eq!(witness_sigop_count(&ScriptBuf::new(), &v0_40, &with_ws, flags), 0); + let v0_41 = S::new().op(OP_0).push(&[0x44; 41]).script(); + assert_eq!(witness_program(v0_41.as_bytes()), None); + let mut trailing = S::new().op(OP_0).push(&[0x44; 20]).0; + trailing.push(OP_NOP); + assert_eq!(witness_sigop_count(&ScriptBuf::new(), &script(&trailing), &with_ws, flags), 0); + // OP_1NEGATE is not a version opcode. + let negate = S::new().op(OP_1NEGATE).push(&[0x44; 20]).script(); + assert_eq!(witness_program(negate.as_bytes()), None); + // Version 16 is a program, of no sigops. + let v16 = S::new().op(OP_16).push(&[0x44; 32]).script(); + assert_eq!(witness_program(v16.as_bytes()).map(|(v, p)| (v, p.len())), Some((16, 32))); + assert_eq!(witness_sigop_count(&ScriptBuf::new(), &v16, &with_ws, flags), 0); +} + +/// Four P2WSH inputs of 3,941 each cost 15,764, unscaled. +#[test] +fn four_heavy_p2wsh_inputs_cost_their_accurate_count() { + let mut heavy = vec![OP_0, OP_IF, 0x51]; + heavy.extend([0xae; 198]); + heavy.extend([OP_ENDIF, 0x51]); + let ws = script(&heavy); + let prevouts = vec![out(ScriptBuf::new_p2wsh(&ws.wscript_hash())); 4]; + let spend = tx( + (0..4) + .map(|i| input(OutPoint { txid: txid(0xf2), vout: i }, ScriptBuf::new(), Witness::from_slice(&[ws.as_bytes()]))) + .collect(), + vec![out(ScriptBuf::new())], + ); + assert_eq!(transaction_sigop_cost(&spend, &prevouts, SigOpFlags::P2shWitness), 4 * 3_941); + // Without SCRIPT_VERIFY_WITNESS none of it counts. + assert_eq!(transaction_sigop_cost(&spend, &prevouts, SigOpFlags::P2sh), 0); +} + +/// Like Core's `assert(!coin.IsSpent())`, a missing prevout is a bug in the +/// caller, not a count of zero. +#[test] +#[should_panic(expected = "sigop counting needs the output each input spends")] +fn a_missing_prevout_is_refused() { + let spend = tx(vec![input(OutPoint { txid: txid(0xf3), vout: 0 }, ScriptBuf::new(), Witness::new())], vec![]); + transaction_sigop_cost(&spend, &[], SigOpFlags::P2shWitness); +} diff --git a/satd/tests/regtest.rs b/satd/tests/regtest.rs index c6c7f42f5..149003607 100644 --- a/satd/tests/regtest.rs +++ b/satd/tests/regtest.rs @@ -2168,9 +2168,12 @@ fn fill_mempool_past_the_block_sigop_limit(node: &TestNode) -> (usize, u64) { /// counts it. Every input in the fixture above spends a P2WPKH coinbase. fn template_tx_sigop_cost(tx: &bitcoin::Transaction) -> u64 { let p2wpkh = DeterministicWallet::from_secret([0x5c; 32]).address.script_pubkey(); - tx.total_sigop_cost(|_| { - Some(bitcoin::TxOut { value: bitcoin::Amount::from_sat(50 * 100_000_000), script_pubkey: p2wpkh.clone() }) - }) as u64 + let prevout = bitcoin::TxOut { value: bitcoin::Amount::from_sat(50 * 100_000_000), script_pubkey: p2wpkh }; + node::validation::sigops::transaction_sigop_cost( + tx, + &vec![prevout; tx.input.len()], + node::validation::sigops::SigOpFlags::P2shWitness, + ) } /// A template must stay under the block sigop limit. `create_template` From e77b0806d1c10cad41a68097998e5c657a1d1316 Mon Sep 17 00:00:00 2001 From: Benjamen Keroack Date: Wed, 7 Oct 2026 21:28:57 -0600 Subject: [PATCH 2/3] mempool: verify scripts before evicting, and make eviction linear A transaction arriving at a full pool evicted entries to make room for itself before its scripts were checked. Core checks the scripts (PolicyScriptChecks, ConsensusScriptChecks) before it adds the transaction and trims the pool (src/validation.cpp, AcceptSingleTransactionInternal), so a transaction whose scripts fail changes nothing. satd now does the same: the cheap fee-rate refusal stays ahead of the scripts, and eviction comes after them. Eviction itself rescanned the whole pool for every evicted entry and every descendant, checking each candidate against a Vec of what was already chosen. It now pops a heap built in one pass, finds descendants through the spends index, and tracks membership in a HashSet. Around the same path: - Eviction never takes the incoming transaction's own ancestors, which left the child in the pool spending an output nothing provided. The fee-rate bar is the cheapest entry it may actually evict. - Admission trims the pool down to its limit, as Core's TrimToSize does, instead of freeing only its own size. A lowered -maxmempool trims the pool at once on reload; before, every admission was refused until the pool drained. - A refusal after eviction reports the entries it evicted. - The specific script error behind a libbitcoinconsensus rejection is found by running the native engine over the failing input only, after the pool's write lock is released. - The orphanage's FIFO queue holds each live orphan once, keyed by an arrival sequence as Core's orphanage is, instead of keeping the txid of every orphan it ever held. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/manual/src/configuration.md | 2 +- node/src/mempool/orphanage.rs | 60 +-- node/src/mempool/orphanage_order_tests.rs | 114 ++++++ node/src/mempool/pool.rs | 194 +++++++--- node/src/mempool/pool_fullpool_tests.rs | 410 +++++++++++++++++++++ node/src/validation/script.rs | 114 +++++- node/src/validation/script_naming_tests.rs | 111 ++++++ 7 files changed, 915 insertions(+), 90 deletions(-) create mode 100644 node/src/mempool/orphanage_order_tests.rs create mode 100644 node/src/mempool/pool_fullpool_tests.rs create mode 100644 node/src/validation/script_naming_tests.rs diff --git a/docs/manual/src/configuration.md b/docs/manual/src/configuration.md index 4739a0cc7..549c37d43 100644 --- a/docs/manual/src/configuration.md +++ b/docs/manual/src/configuration.md @@ -143,7 +143,7 @@ changed. Their values are redacted in the log, never printed. | `rpcextendederrors`, `rpcdefaultunits` | Switches the RPC error-payload shape and the default amount unit. | | `maxconnections`, `maxinboundperip` | New limits govern subsequent connections. Existing peers above a lowered cap are not dropped. | | `bantime` | New ban duration applies to bans created after the change. | -| `minrelaytxfee`, `maxmempool`, `dustrelayfee`, `datacarrier`, `datacarriersize`, `mempoolfullrbf`, `limitclustercount`, `limitancestorcount`, `limitdescendantcount`, `mempoolexpiry`, `permitbaremultisig` | Mempool and relay policy is swapped atomically and governs subsequent transaction admissions. Already-admitted entries are not re-evaluated. | +| `minrelaytxfee`, `maxmempool`, `dustrelayfee`, `datacarrier`, `datacarriersize`, `mempoolfullrbf`, `limitclustercount`, `limitancestorcount`, `limitdescendantcount`, `mempoolexpiry`, `permitbaremultisig` | Mempool and relay policy is swapped atomically and governs subsequent transaction admissions. Already-admitted entries are not re-evaluated, except that a lower `maxmempool` evicts the lowest-feerate entries at once, down to the new size. | | `connect`, `addnode`, `seednode` | Newly added peers are registered as at startup and dialed immediately; existing connections are untouched. A new `addnode` entry is an added node, listed by `getaddednodeinfo` and removable with `addnode remove`, and a new `addnode` or `connect` host name that does not resolve yet is kept and looked up again on every reconnect tick. Removing an entry does not disconnect that peer (use `disconnectnode`) or remove the added node (use `addnode remove`), matching Core's `-addnode`. The exclusivity of `-connect` (connect only to these peers, with automatic outbound and DNS seeding suppressed) is a startup-time decision and is not re-evaluated on reload. Adding `-connect` live dials the new peer but does not put a running node into connect-only mode; restart for that. | | `peerblockfilters` | Turns `NODE_COMPACT_FILTERS` advertisement on or off for new handshakes, still gated on a complete `blockfilterindex`. | | `addrindexsubscriptions` | New address-index subscription cap, applied to subsequent subscriptions. Lowering it does not evict existing subscribers. | diff --git a/node/src/mempool/orphanage.rs b/node/src/mempool/orphanage.rs index 54ef75594..4999455a9 100644 --- a/node/src/mempool/orphanage.rs +++ b/node/src/mempool/orphanage.rs @@ -16,7 +16,7 @@ use bitcoin::{Transaction, Txid}; use parking_lot::Mutex; -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::time::{Duration, Instant}; use crate::mempool::policy::MAX_STANDARD_TX_WEIGHT; @@ -81,6 +81,8 @@ pub struct OrphanEntry { pub added_at: Instant, pub missing_parents: HashSet, pub bytes: usize, + /// Its key in the orphanage's arrival-order queue. + seq: u64, } struct OrphanageInner { @@ -88,10 +90,13 @@ struct OrphanageInner { by_parent: HashMap>, by_peer: HashMap>, total_bytes: usize, - // Insertion-order queue for FIFO eviction. Entries may refer to - // txids that have already been removed (via explicit remove or - // per-peer eviction); eviction skips stale entries at the front. - order: VecDeque, + /// The live orphans in arrival order, for FIFO eviction, keyed by an + /// arrival sequence number as Core's orphanage keys its announcements + /// (`m_entry_sequence`, src/node/txorphanage.cpp). Every removal takes + /// its orphan's key out, so this holds exactly the orphans in `by_txid`. + order: BTreeMap, + /// The next arrival's key in `order`. + next_seq: u64, } pub struct TxOrphanage { @@ -107,7 +112,8 @@ impl TxOrphanage { by_parent: HashMap::new(), by_peer: HashMap::new(), total_bytes: 0, - order: VecDeque::new(), + order: BTreeMap::new(), + next_seq: 0, }), config, } @@ -178,23 +184,26 @@ impl TxOrphanage { // Global cap: evict oldest (FIFO) until we have room. while inner.by_txid.len() >= self.config.max_count { - if let Some(victim) = Self::pop_oldest(&mut inner) { + if let Some(victim) = Self::oldest(&inner) { Self::remove_locked(&mut inner, &victim); } else { break; } } + let seq = inner.next_seq; + inner.next_seq += 1; let entry = OrphanEntry { tx, from_peer, added_at, missing_parents: missing_parents.clone(), bytes, + seq, }; inner.by_txid.insert(txid, entry); inner.total_bytes += bytes; - inner.order.push_back(txid); + inner.order.insert(seq, txid); for parent in &missing_parents { inner.by_parent.entry(*parent).or_default().insert(txid); } @@ -238,8 +247,7 @@ impl TxOrphanage { inner.by_peer.remove(&entry.from_peer); } } - // We don't eagerly strip `order` — stale entries are skipped at - // eviction time. This keeps remove() O(1). + inner.order.remove(&entry.seq); Some(entry) } @@ -276,29 +284,27 @@ impl TxOrphanage { self.inner.lock().by_txid.contains_key(txid) } - fn pop_oldest(inner: &mut OrphanageInner) -> Option { - while let Some(txid) = inner.order.pop_front() { - if inner.by_txid.contains_key(&txid) { - return Some(txid); - } - // Stale order entry (already removed via some other path). - } - None + fn oldest(inner: &OrphanageInner) -> Option { + inner.order.first_key_value().map(|(_, txid)| *txid) } + /// The peer's oldest orphan: a scan of that peer's own orphans, at most + /// `config.max_per_peer` of them. fn oldest_for_peer(inner: &OrphanageInner, peer: PeerId) -> Option { - let peer_set = inner.by_peer.get(&peer)?; - // Linear scan of order queue, picking the first that still - // belongs to this peer. Bounded by config.max_count. - for txid in &inner.order { - if peer_set.contains(txid) { - return Some(*txid); - } - } - None + inner + .by_peer + .get(&peer)? + .iter() + .filter_map(|txid| inner.by_txid.get(txid).map(|e| (e.seq, *txid))) + .min() + .map(|(_, txid)| txid) } } +#[cfg(test)] +#[path = "orphanage_order_tests.rs"] +mod order_tests; + #[cfg(test)] mod tests { use super::*; diff --git a/node/src/mempool/orphanage_order_tests.rs b/node/src/mempool/orphanage_order_tests.rs new file mode 100644 index 000000000..bbf919e07 --- /dev/null +++ b/node/src/mempool/orphanage_order_tests.rs @@ -0,0 +1,114 @@ +//! The orphanage's eviction queue holds each live orphan exactly once. +//! +//! It kept the txid of every orphan it had ever held: a removal or an +//! expiry left the txid behind, and only a full pool popped stale ones off +//! the front. A peer at its per-peer quota scans the queue on every add, so +//! the scan grew with every orphan that peer had sent. + +use super::*; +use bitcoin::hashes::Hash; + +fn parent(i: u32) -> Txid { + let mut bytes = [0u8; 32]; + bytes[..4].copy_from_slice(&i.to_le_bytes()); + Txid::from_byte_array(bytes) +} + +/// An orphan spending output 0 of [`parent`]`(i)`. +fn orphan(i: u32) -> (Transaction, HashSet) { + let tx = Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![bitcoin::TxIn { + previous_output: bitcoin::OutPoint { txid: parent(i), vout: 0 }, + script_sig: bitcoin::ScriptBuf::new(), + sequence: bitcoin::Sequence::MAX, + witness: bitcoin::Witness::new(), + }], + output: vec![bitcoin::TxOut { + value: bitcoin::Amount::from_sat(1_000), + script_pubkey: bitcoin::ScriptBuf::new(), + }], + }; + (tx, HashSet::from([parent(i)])) +} + +fn add(pool: &TxOrphanage, i: u32, peer: PeerId) -> Txid { + let (tx, missing) = orphan(i); + let txid = tx.compute_txid(); + assert_eq!(pool.add(tx, peer, missing).unwrap(), AddOutcome::Added); + txid +} + +fn queued(pool: &TxOrphanage) -> usize { + pool.inner.lock().order.len() +} + +/// One peer streaming orphans past its quota of 50: each add evicts that +/// peer's oldest, and the queue stays the size of the pool. +#[test] +fn a_peer_at_its_quota_does_not_grow_the_queue() { + let pool = TxOrphanage::with_defaults(); + for i in 0..10_000 { + add(&pool, i, 1); + } + assert_eq!(pool.len(), DEFAULT_MAX_ORPHAN_PER_PEER); + assert_eq!(queued(&pool), pool.len(), "the queue kept evicted orphans"); +} + +/// Removal and expiry take the orphan out of the queue too. +#[test] +fn removed_and_expired_orphans_leave_the_queue() { + let pool = TxOrphanage::with_defaults(); + let ids: Vec = (0..20).map(|i| add(&pool, i, i as PeerId)).collect(); + for txid in &ids[..10] { + pool.remove(txid).expect("present"); + } + assert_eq!(queued(&pool), 10, "removed orphans stayed queued"); + pool.expire(Instant::now() + DEFAULT_ORPHAN_EXPIRY); + assert!(pool.is_empty()); + assert_eq!(queued(&pool), 0, "expired orphans stayed queued"); +} + +/// An orphan that is removed and sent again queues as the newest, once. +/// With its first place still queued, the full pool evicted it in place of +/// the orphan that really was the oldest. +#[test] +fn a_returning_orphan_queues_as_the_newest() { + let pool = TxOrphanage::new(OrphanageConfig { + max_count: 2, + max_per_peer: 10, + expiry: DEFAULT_ORPHAN_EXPIRY, + }); + let a = add(&pool, 1, 1); + let b = add(&pool, 2, 1); + pool.remove(&a).expect("present"); + assert_eq!(add(&pool, 1, 1), a); + assert_eq!(queued(&pool), 2); + + let c = add(&pool, 3, 1); + assert!(!pool.contains(&b), "the oldest orphan should make the room"); + assert!(pool.contains(&a), "the orphan that came back was evicted as if it were the oldest"); + assert!(pool.contains(&c)); +} + +/// The per-peer quota still evicts that peer's oldest orphan, and no one +/// else's. +#[test] +fn the_quota_evicts_the_peers_own_oldest() { + let pool = TxOrphanage::new(OrphanageConfig { + max_count: 100, + max_per_peer: 3, + expiry: DEFAULT_ORPHAN_EXPIRY, + }); + let first = add(&pool, 1, 1); + let other = add(&pool, 2, 2); + let second = add(&pool, 3, 1); + let third = add(&pool, 4, 1); + let fourth = add(&pool, 5, 1); + assert!(!pool.contains(&first)); + for txid in [other, second, third, fourth] { + assert!(pool.contains(&txid)); + } + assert_eq!(queued(&pool), 4); +} diff --git a/node/src/mempool/pool.rs b/node/src/mempool/pool.rs index 84fc1741a..61c69fd09 100644 --- a/node/src/mempool/pool.rs +++ b/node/src/mempool/pool.rs @@ -1630,9 +1630,30 @@ impl Mempool { /// Swap in a new mempool/relay policy live (SIGHUP config reload). Takes /// effect on the next `accept_transaction` call; already-admitted entries - /// are not re-evaluated. + /// are not re-evaluated, except that a lower size limit trims the pool to + /// it at once. pub fn reload_policy(&self, new: MempoolConfig) { + let (max_size_bytes, quarantine_max_bytes, incremental_relay_fee) = + (new.max_size_bytes, new.quarantine_max_bytes, new.incremental_relay_fee); *self.config.write() = new; + + // Core never leaves the pool over `-maxmempool`: every admission ends + // in `LimitMempoolSize` → `TrimToSize` (src/validation.cpp). Left over a + // lowered limit, the pool refused every admission until it drained. + let evicted = { + let mut inner = self.inner.write(); + let over = inner.acting_bytes().saturating_sub(max_size_bytes); + let evicted = Self::evict_lowest_fee_entries(&mut inner, over, false, incremental_relay_fee); + // A held entry never emitted an `Enter`, so its eviction emits no + // `LeaveEvicted` (§10), as in `accept_transaction`. + let over = inner.quarantine_bytes.saturating_sub(quarantine_max_bytes); + Self::evict_lowest_fee_entries(&mut inner, over, true, incremental_relay_fee); + self.sync_unbroadcast_len(&inner); + evicted + }; + for txid in &evicted { + self.emit(MempoolEvent::LeaveEvicted { txid: *txid, reason: EvictReason::FullPool }); + } } /// Load (or replace) the transaction-filtering ruleset from a file. Returns @@ -2989,21 +3010,54 @@ impl Mempool { } else { EvictReason::FullPool }; - let mut evicted_full_pool: Vec = Vec::new(); - if class_bytes + tx_size > class_budget { - // Only evict if the new tx outbids the cheapest entry *in its own class*. - let min_class_fee_rate = inner + let needs_room = class_bytes + tx_size > class_budget; + if needs_room { + // Only evict if the new tx outbids the cheapest entry *in its own + // class* that it may evict. Its own in-pool ancestors stay (see the + // eviction below), so they are not that bar. + let Some(min_class_fee_rate) = inner .entries - .values() - .filter(|e| e.scope.is_quarantined() == quarantined) - .map(|e| e.fee_rate) + .iter() + .filter(|(t, e)| e.scope.is_quarantined() == quarantined && !ancestors.contains(*t)) + .map(|(_, e)| e.fee_rate) .min() - .unwrap_or(0); + else { + return Err(MempoolError::MempoolFull); + }; if fee_rate <= min_class_fee_rate { return Err(MempoolError::MempoolFull); } - // Evict enough lowest-fee-rate entries *of this class* to make room. - evicted_full_pool = Self::evict_lowest_fee_entries(&mut inner, tx_size, quarantined, cfg.incremental_relay_fee); + } + + // Script verification (all inputs at once for taproot), before + // anything leaves the pool to make room. Core runs PolicyScriptChecks + // and ConsensusScriptChecks before it adds the transaction and trims + // the pool (src/validation.cpp, AcceptSingleTransactionInternal), so a + // transaction whose scripts fail changes nothing. + // Use tip_height + 1 since the tx will be mined in the next block + if let Err(e) = script_verifier.verify_transaction_unnamed(&tx, &prev_outputs, tip_height + 1) { + // Naming the script error can mean running the second engine over + // the input. Nothing has changed in the pool, so release it first. + drop(inner); + let e = script_verifier.name_failure(&tx, &prev_outputs, tip_height + 1, e); + return Err(MempoolError::Script(e.reason().to_string(), e.core_debug_string(&tx))); + } + + let mut evicted_full_pool: Vec = Vec::new(); + if needs_room { + // Evict lowest-fee-rate entries *of this class* until the pool, + // with this transaction in it, is back within its limit: Core's + // `TrimToSize` (src/txmempool.cpp). Freeing only this transaction's + // size left a pool that was already over (a lowered `-maxmempool`) + // over, and refused everything. Never this transaction's own + // ancestors. + evicted_full_pool = Self::evict_lowest_fee_entries_except( + &mut inner, + class_bytes + tx_size - class_budget, + quarantined, + cfg.incremental_relay_fee, + &ancestors, + ); self.sync_unbroadcast_len(&inner); // If still not enough room after eviction, reject. let class_bytes_after = if quarantined { @@ -3012,18 +3066,17 @@ impl Mempool { inner.acting_bytes() }; if class_bytes_after + tx_size > class_budget { + drop(inner); + // The evicted entries are gone whether or not this one gets in. + if !quarantined { + for evicted_txid in &evicted_full_pool { + self.emit(MempoolEvent::LeaveEvicted { txid: *evicted_txid, reason: evict_reason }); + } + } return Err(MempoolError::MempoolFull); } } - // Script verification (all inputs at once for taproot) - // Use tip_height + 1 since the tx will be mined in the next block - script_verifier - .verify_transaction(&tx, &prev_outputs, tip_height + 1) - .map_err(|e| { - MempoolError::Script(e.reason().to_string(), e.core_debug_string(&tx)) - })?; - // RBF: remove conflicted transactions before inserting replacement. // Collect replaced txids so we can emit LeaveReplaced events after the // write lock is dropped. Only *acting* conflicts go on the standard @@ -6046,12 +6099,13 @@ impl Mempool { // Skip fee rate check — this is the bypass. - // Script verification - script_verifier - .verify_transaction(&tx, &prev_outputs, tip_height + 1) - .map_err(|e| { - MempoolError::Script(e.reason().to_string(), e.core_debug_string(&tx)) - })?; + // Script verification; the failure is named once the pool is released, + // as in `accept_transaction`. + if let Err(e) = script_verifier.verify_transaction_unnamed(&tx, &prev_outputs, tip_height + 1) { + drop(inner); + let e = script_verifier.name_failure(&tx, &prev_outputs, tip_height + 1, e); + return Err(MempoolError::Script(e.reason().to_string(), e.core_debug_string(&tx))); + } let fee_rate = policy::fee_rate_sat_per_kvb(fee, weight as u64); @@ -6115,7 +6169,36 @@ impl Mempool { want_quarantined: bool, incremental_relay_fee: u64, ) -> Vec { - // Sort *this class's* entries by fee rate ascending; the other class is + Self::evict_lowest_fee_entries_except( + inner, + bytes_needed, + want_quarantined, + incremental_relay_fee, + &HashSet::new(), + ) + } + + /// [`evict_lowest_fee_entries`](Self::evict_lowest_fee_entries), never + /// choosing an entry in `keep`. Admission passes the incoming + /// transaction's in-pool ancestors: evicting one would leave that + /// transaction spending an output nothing provides. An ancestor set is + /// closed under "parent of", so no kept entry descends from an evicted one + /// either. + /// + /// The cost follows what is evicted, not the size of the pool: one pass + /// builds a heap of the class, each eviction pops it, and descendants come + /// from the `spends` index. + fn evict_lowest_fee_entries_except( + inner: &mut MempoolInner, + bytes_needed: usize, + want_quarantined: bool, + incremental_relay_fee: u64, + keep: &HashSet, + ) -> Vec { + if bytes_needed == 0 { + return Vec::new(); + } + // *This class's* entries, lowest fee rate first; the other class is // never an eviction candidate (its own budget governs it). // // On the *modified* fee rate. `entry.fee_rate` is the rate as of @@ -6124,55 +6207,49 @@ impl Mempool { // delta did not exist — which defeats the point of the RPC. Core // evicts on the modified feerate (`CTxMemPool::TrimToSize` sorts by // descendant-modified feerate). - let mut by_fee_rate: Vec<(Txid, u64)> = inner + let mut by_fee_rate: std::collections::BinaryHeap> = inner .entries .iter() - .filter(|(_, entry)| entry.scope.is_quarantined() == want_quarantined) + .filter(|(txid, entry)| { + entry.scope.is_quarantined() == want_quarantined && !keep.contains(*txid) + }) .map(|(txid, entry)| { - ( - *txid, + std::cmp::Reverse(( policy::fee_rate_sat_per_kvb( modified_fee(entry.fee, entry.fee_delta), entry.weight as u64, ), - ) + *txid, + )) }) .collect(); - by_fee_rate.sort_by_key(|(_, rate)| *rate); let mut freed = 0usize; let mut to_remove: Vec = Vec::new(); + // What `to_remove` holds, for membership tests. + let mut removing: HashSet = HashSet::new(); + let mut children: Vec = Vec::new(); - for (txid, _) in &by_fee_rate { - if freed >= bytes_needed { + while freed < bytes_needed { + let Some(std::cmp::Reverse((_, txid))) = by_fee_rate.pop() else { break; - } - if to_remove.contains(txid) { + }; + if !removing.insert(txid) { continue; } let package_start = to_remove.len(); - to_remove.push(*txid); - if let Some(entry) = inner.entries.get(txid) { + to_remove.push(txid); + if let Some(entry) = inner.entries.get(&txid) { freed += bitcoin::consensus::serialize(&entry.tx).len(); } // Also collect descendants of the evicted entry - let mut desc_queue = vec![*txid]; + let mut desc_queue = vec![txid]; while let Some(current) = desc_queue.pop() { - let current_txid_for_search = current; - let children: Vec = inner - .entries - .iter() - .filter(|(child_txid, child_entry)| { - !to_remove.contains(child_txid) - && child_entry - .tx - .input - .iter() - .any(|i| i.previous_output.txid == current_txid_for_search) - }) - .map(|(child_txid, _)| *child_txid) - .collect(); - for child in children { + Self::collect_children(inner, ¤t, &mut children); + for child in children.drain(..) { + if !removing.insert(child) { + continue; + } if let Some(child_entry) = inner.entries.get(&child) { // A cross-class descendant (e.g. a quarantined child of an // evicted acting parent) must still be removed for graph @@ -6619,6 +6696,10 @@ impl Mempool { #[path = "pool_sigops_tests.rs"] mod sigops_tests; +#[cfg(test)] +#[path = "pool_fullpool_tests.rs"] +mod fullpool_tests; + #[cfg(test)] mod tests { @@ -8786,6 +8867,11 @@ mod tests { let txid = tx.compute_txid(); let tx_size = bitcoin::consensus::serialize(&tx).len(); let mut inner = mp.inner.write(); + // Index the spends as every admission does: eviction finds a + // descendant through them. + for i in &tx.input { + inner.spends.insert(i.previous_output, txid); + } inner.entries.insert( txid, MempoolEntry { diff --git a/node/src/mempool/pool_fullpool_tests.rs b/node/src/mempool/pool_fullpool_tests.rs new file mode 100644 index 000000000..054d11a82 --- /dev/null +++ b/node/src/mempool/pool_fullpool_tests.rs @@ -0,0 +1,410 @@ +//! Admission into a full pool, checked against Bitcoin Core's +//! `MemPoolAccept` (src/validation.cpp, `AcceptSingleTransactionInternal`): +//! the scripts are checked (`PolicyScriptChecks`, `ConsensusScriptChecks`) +//! before anything leaves the pool, and the pool is then trimmed to its +//! limit (`LimitMempoolSize` → `CTxMemPool::TrimToSize`). +//! +//! satd frees room before it inserts rather than trimming after, so it also +//! has to keep the incoming transaction's own ancestors: Core's trim takes a +//! parent together with the child that pays for it, never the parent alone. + +use super::*; +use crate::mining::template::tests::make_funded_template_env_with; +use crate::storage::coinview::Coin; +use crate::validation::script::{NoopVerifier, ScriptError}; +use bitcoin::hashes::Hash; +use bitcoin::{Amount, Sequence, TxIn, Witness}; + +const COIN_VALUE: u64 = 100_000; + +fn prev(tag: u8) -> OutPoint { + OutPoint { txid: Txid::from_byte_array([tag; 32]), vout: 0 } +} + +fn p2wpkh(tag: u8) -> ScriptBuf { + let mut spk = vec![0x00, 0x14]; + spk.extend_from_slice(&[tag; 20]); + ScriptBuf::from_bytes(spk) +} + +fn coin() -> Coin { + Coin { + amount: COIN_VALUE, + script_pubkey: p2wpkh(0x11), + height: 1, + coinbase: false, + txseq: node_index::TXSEQ_UNKNOWN, + } +} + +/// Spend `prevs` (worth `in_value` together), paying `fee` and splitting the +/// rest over `outs` P2WPKH outputs. One input and one output is 82 bytes; +/// each further output adds 31. +fn spend(prevs: &[OutPoint], in_value: u64, fee: u64, outs: u64, tag: u8) -> Transaction { + let each = (in_value - fee) / outs; + let mut output: Vec = (0..outs) + .map(|_| TxOut { value: Amount::from_sat(each), script_pubkey: p2wpkh(tag) }) + .collect(); + output[0].value = Amount::from_sat(in_value - fee - each * (outs - 1)); + Transaction { + version: bitcoin::transaction::Version(2), + lock_time: bitcoin::absolute::LockTime::ZERO, + input: prevs + .iter() + .map(|p| TxIn { + previous_output: *p, + script_sig: ScriptBuf::new(), + sequence: Sequence::MAX, + witness: Witness::new(), + }) + .collect(), + output, + } +} + +/// A regtest chain holding one coin per tag, and an empty pool with no relay +/// floor. +fn env(tags: &[u8]) -> (ChainState, Mempool, std::path::PathBuf) { + let coins: Vec<(OutPoint, Coin)> = tags.iter().map(|t| (prev(*t), coin())).collect(); + let (cs, mp, dir) = make_funded_template_env_with(&coins, Box::new(NoopVerifier)); + *mp.config.write() = + MempoolConfig { max_size_bytes: 1_000_000, min_fee_rate: 0, ..Default::default() }; + (cs, mp, dir) +} + +/// Set the size limit without going through `reload_policy`. +fn set_max(mp: &Mempool, max: usize) { + mp.config.write().max_size_bytes = max; +} + +fn admit(mp: &Mempool, cs: &ChainState, tx: Transaction) -> Txid { + mp.accept_transaction(tx, cs, &NoopVerifier, TxSource::P2p, false).expect("admitted") +} + +fn pool(mp: &Mempool) -> HashSet { + mp.inner.read().entries.keys().copied().collect() +} + +fn events(mp: &Mempool) -> broadcast::Receiver { + let (tx, rx) = broadcast::channel(1024); + mp.set_event_sender(tx); + rx +} + +/// The `LeaveEvicted { FullPool }` events sent so far. +fn evicted(rx: &mut broadcast::Receiver) -> Vec { + let mut out = Vec::new(); + while let Ok(ev) = rx.try_recv() { + if let MempoolEvent::LeaveEvicted { txid, reason: EvictReason::FullPool } = ev { + out.push(txid); + } + } + out +} + +/// Fails every transaction's scripts, as libconsensus does a forged signature. +struct Refuses; + +impl ScriptVerifier for Refuses { + fn verify_transaction( + &self, + _tx: &Transaction, + _prev_outputs: &[TxOut], + _height: u32, + ) -> Result<(), ScriptError> { + Err(ScriptError::VerifyFailed { input: 0, reason: "Invalid Schnorr signature".into() }) + } +} + +/// A transaction that outbids the pool but fails its scripts takes nothing +/// out of it: no entry, no rise in the minimum fee, no `LeaveEvicted`. The +/// same transaction with valid scripts then takes the one entry it needs. +#[test] +fn a_script_failure_on_a_full_pool_evicts_nothing() { + let (cs, mp, dir) = env(&[1, 2, 3, 4]); + for t in 1..=3 { + admit(&mp, &cs, spend(&[prev(t)], COIN_VALUE, 100, 1, t)); + } + let before = pool(&mp); + set_max(&mp, mp.acting_bytes()); + let floor = mp.info().mempool_min_fee; + let mut rx = events(&mp); + + let rich = spend(&[prev(4)], COIN_VALUE, 5_000, 1, 4); + let err = mp + .accept_transaction(rich.clone(), &cs, &Refuses, TxSource::P2p, false) + .expect_err("its scripts fail"); + assert!(matches!(err, MempoolError::Script(..)), "{err:?}"); + assert_eq!(pool(&mp), before, "a transaction whose scripts fail evicted entries"); + assert_eq!(mp.info().mempool_min_fee, floor, "a transaction whose scripts fail raised the floor"); + assert!(evicted(&mut rx).is_empty()); + + let txid = mp + .accept_transaction(rich, &cs, &NoopVerifier, TxSource::P2p, false) + .expect("admitted"); + let gone = evicted(&mut rx); + assert_eq!(gone.len(), 1, "one 82-byte entry makes room for one 82-byte transaction"); + assert!(before.contains(&gone[0])); + assert!(pool(&mp).contains(&txid)); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// A child whose parent is the cheapest entry keeps its parent: the room +/// comes from the cheapest entry that is not one of its ancestors. Evicting +/// the parent left the child in the pool with an input nothing provides. +#[test] +fn a_child_keeps_its_parent_in_a_full_pool() { + let (cs, mp, dir) = env(&[1, 2, 3]); + let parent = admit(&mp, &cs, spend(&[prev(1)], COIN_VALUE, 100, 1, 1)); + let second = admit(&mp, &cs, spend(&[prev(2)], COIN_VALUE, 200, 1, 2)); + let third = admit(&mp, &cs, spend(&[prev(3)], COIN_VALUE, 300, 1, 3)); + set_max(&mp, mp.acting_bytes()); + + let child = admit( + &mp, + &cs, + spend(&[OutPoint { txid: parent, vout: 0 }], COIN_VALUE - 100, 10_000, 1, 4), + ); + let now = pool(&mp); + assert!(now.contains(&parent), "the child's own parent was evicted to make room for it"); + assert!(!now.contains(&second), "the cheapest entry that is not an ancestor makes the room"); + assert!(now.contains(&third)); + assert!(now.contains(&child)); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// A child that outbids only its own parent has nothing it may evict, so it +/// is refused and the pool is unchanged. (Core trims the parent and the child +/// together as the worst-paying chunk and answers "mempool full".) +#[test] +fn a_child_that_outbids_only_its_parent_is_refused() { + let (cs, mp, dir) = env(&[1, 2, 3]); + let parent = admit(&mp, &cs, spend(&[prev(1)], COIN_VALUE, 100, 1, 1)); + admit(&mp, &cs, spend(&[prev(2)], COIN_VALUE, 2_000, 1, 2)); + admit(&mp, &cs, spend(&[prev(3)], COIN_VALUE, 2_000, 1, 3)); + set_max(&mp, mp.acting_bytes()); + let before = pool(&mp); + + let child = spend(&[OutPoint { txid: parent, vout: 0 }], COIN_VALUE - 100, 1_000, 1, 4); + let err = mp + .accept_transaction(child, &cs, &NoopVerifier, TxSource::P2p, false) + .expect_err("it pays less than every entry it could evict"); + assert!(matches!(err, MempoolError::MempoolFull), "{err:?}"); + assert_eq!(pool(&mp), before); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// When the room still runs short after eviction, the transaction is refused, +/// and what was evicted on the way is reported: those entries are gone. +#[test] +fn a_full_pool_refusal_reports_its_evictions() { + let (cs, mp, dir) = env(&[1, 2]); + let parent = admit(&mp, &cs, spend(&[prev(1)], COIN_VALUE, 200, 1, 1)); + let cheap = admit(&mp, &cs, spend(&[prev(2)], COIN_VALUE, 100, 1, 2)); + set_max(&mp, mp.acting_bytes()); + let mut rx = events(&mp); + + // 144 bytes, and only the 82-byte `cheap` can go. + let child = spend(&[OutPoint { txid: parent, vout: 0 }], COIN_VALUE - 200, 10_000, 3, 3); + let err = mp + .accept_transaction(child, &cs, &NoopVerifier, TxSource::P2p, false) + .expect_err("evicting everything but its parent does not make enough room"); + assert!(matches!(err, MempoolError::MempoolFull), "{err:?}"); + assert_eq!(pool(&mp), HashSet::from([parent])); + assert_eq!(evicted(&mut rx), vec![cheap], "an eviction was never reported"); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// Lowering `-maxmempool` on a live node trims the pool to the new limit at +/// once, lowest feerate first, and reports each eviction. Left over the limit, +/// every later admission freed one transaction's worth and was refused. +#[test] +fn lowering_maxmempool_trims_the_pool_at_once() { + let (cs, mp, dir) = env(&[1, 2, 3, 4, 5]); + let ids: Vec = (1..=4u8) + .map(|t| admit(&mp, &cs, spend(&[prev(t)], COIN_VALUE, 100 * t as u64, 1, t))) + .collect(); + let two = mp.acting_bytes() / 2; + let mut rx = events(&mp); + + let cfg = MempoolConfig { max_size_bytes: two, ..mp.config.read().clone() }; + mp.reload_policy(cfg); + assert!(mp.acting_bytes() <= two, "{} bytes over a {two}-byte limit", mp.acting_bytes()); + assert_eq!(pool(&mp), HashSet::from([ids[2], ids[3]])); + let mut gone = evicted(&mut rx); + gone.sort(); + let mut lowest = vec![ids[0], ids[1]]; + lowest.sort(); + assert_eq!(gone, lowest); + + let txid = admit(&mp, &cs, spend(&[prev(5)], COIN_VALUE, 10_000, 1, 5)); + assert!(pool(&mp).contains(&txid)); + assert!(mp.acting_bytes() <= two); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// An admission into a pool that is over its limit trims it to the limit, +/// as Core's `TrimToSize` does, instead of freeing only its own size and +/// refusing because the pool is still over. +#[test] +fn an_admission_trims_an_over_limit_pool() { + let (cs, mp, dir) = env(&[1, 2, 3, 4, 5]); + let ids: Vec = (1..=4u8) + .map(|t| admit(&mp, &cs, spend(&[prev(t)], COIN_VALUE, 100 * t as u64, 1, t))) + .collect(); + let two = mp.acting_bytes() / 2; + set_max(&mp, two); + + let txid = mp + .accept_transaction(spend(&[prev(5)], COIN_VALUE, 10_000, 1, 5), &cs, &NoopVerifier, TxSource::P2p, false) + .expect("the pool is trimmed to its limit and the transaction admitted"); + assert_eq!(pool(&mp), HashSet::from([ids[3], txid])); + assert!(mp.acting_bytes() <= two); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// Evicting a parent takes every descendant once: a child spending two of +/// its outputs, a child spending the third, and a grandchild. The spend +/// index and the byte count follow. +#[test] +fn evicting_a_parent_takes_every_descendant() { + let (cs, mp, dir) = env(&[1, 2]); + let parent = admit(&mp, &cs, spend(&[prev(1)], COIN_VALUE, 100, 3, 1)); + let out = |txid, vout| OutPoint { txid, vout }; + let per = (COIN_VALUE - 100) / 3; + let two_outs = admit(&mp, &cs, spend(&[out(parent, 1), out(parent, 2)], 2 * per, 5_000, 1, 2)); + let one_out = admit(&mp, &cs, spend(&[out(parent, 0)], COIN_VALUE - 100 - 2 * per, 5_000, 1, 3)); + let grandchild = admit(&mp, &cs, spend(&[out(two_outs, 0)], 2 * per - 5_000, 5_000, 1, 4)); + let other = admit(&mp, &cs, spend(&[prev(2)], COIN_VALUE, 1_000, 1, 5)); + let other_size = bitcoin::consensus::serialize(&mp.get(&other).unwrap().tx).len(); + + let mut inner = mp.inner.write(); + let mut gone = Mempool::evict_lowest_fee_entries(&mut inner, 1, false, 0); + assert_eq!(inner.entries.keys().copied().collect::>(), HashSet::from([other])); + assert_eq!(inner.spends.len(), 1); + assert_eq!(inner.acting_bytes(), other_size); + drop(inner); + gone.sort(); + let mut family = vec![parent, two_outs, one_out, grandchild]; + family.sort(); + assert_eq!(gone, family, "each descendant is evicted, and only once"); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// The cost of eviction follows what it evicts, not the pool size times the +/// evictions. 600 evictions from a pool of 20,000 scanned the whole pool once +/// per eviction, each scan checking a growing list of what was already +/// chosen: billions of comparisons under the pool's write lock. +#[test] +fn eviction_cost_follows_what_it_evicts() { + const N: u32 = 20_000; + const EVICT: usize = 600; + let mp = Mempool::with_config(MempoolConfig { + max_size_bytes: usize::MAX, + min_fee_rate: 0, + ..Default::default() + }); + let mut inner = mp.inner.write(); + let mut by_rate: Vec = Vec::new(); + let mut size = 0; + for i in 0..N { + let mut bytes = [0u8; 32]; + bytes[..4].copy_from_slice(&i.to_le_bytes()); + let tx = spend(&[OutPoint { txid: Txid::from_byte_array(bytes), vout: 0 }], COIN_VALUE, 1_000, 1, 7); + let txid = tx.compute_txid(); + size = bitcoin::consensus::serialize(&tx).len(); + inner.spends.insert(tx.input[0].previous_output, txid); + // A distinct feerate per entry, highest first. + let fee = 1_000 + (N - i) as u64; + inner.entries.insert( + txid, + MempoolEntry { + tx, + fee, + weight: 4 * size, + fee_rate: policy::fee_rate_sat_per_kvb(fee, 4 * size as u64), + time: 0, + fee_delta: 0, + sigop_cost: 0, + prev_scripthashes: Vec::new(), + prev_amounts: Vec::new(), + prev_scripts: Vec::new(), + sp_tweak: None, + source: TxSource::P2p, + scope: QuarantineScope::acting(), + quarantine_rule: None, + }, + ); + inner.account_insert(QuarantineScope::acting(), size); + by_rate.push(txid); + } + + let start = std::time::Instant::now(); + let gone = Mempool::evict_lowest_fee_entries(&mut inner, EVICT * size, false, 0); + let took = start.elapsed(); + drop(inner); + + let mut expected: Vec = by_rate[by_rate.len() - EVICT..].to_vec(); + expected.sort(); + let mut got = gone.clone(); + got.sort(); + assert_eq!(got, expected, "the {EVICT} lowest feerates go"); + assert!(took < std::time::Duration::from_secs(2), "evicting {EVICT} of {N} took {took:?}"); +} + +/// Fails every transaction with libbitcoinconsensus's coarse reason, and +/// records whether the pool was free when asked to name the failure. +struct NamesWhenAsked<'a> { + mp: &'a Mempool, + pool_free: Mutex>, +} + +impl ScriptVerifier for NamesWhenAsked<'_> { + fn verify_transaction( + &self, + _tx: &Transaction, + _prev_outputs: &[TxOut], + _height: u32, + ) -> Result<(), ScriptError> { + Err(ScriptError::VerifyFailed { input: 0, reason: "ERR_SCRIPT".into() }) + } + + fn name_failure( + &self, + _tx: &Transaction, + _prev_outputs: &[TxOut], + _height: u32, + err: ScriptError, + ) -> ScriptError { + *self.pool_free.lock() = Some(self.mp.inner.try_write().is_some()); + ScriptError::VerifyFailed { input: err.input(), reason: "Invalid Schnorr signature".into() } + } +} + +/// The rejection carries the specific script error, and finding it (which +/// can mean running the second script engine over the input) happens after +/// the pool is released, so block connection and template building do not +/// wait on it. +#[test] +fn a_script_failure_is_named_after_the_pool_is_released() { + let (cs, mp, dir) = env(&[1]); + let verifier = NamesWhenAsked { mp: &mp, pool_free: Mutex::new(None) }; + let err = mp + .accept_transaction(spend(&[prev(1)], COIN_VALUE, 1_000, 1, 1), &cs, &verifier, TxSource::P2p, false) + .expect_err("its scripts fail"); + match err { + MempoolError::Script(reason, _) => assert_eq!(reason, "Invalid Schnorr signature"), + other => panic!("{other:?}"), + } + assert_eq!(*verifier.pool_free.lock(), Some(true), "the failure was named under the pool's write lock"); + assert!(pool(&mp).is_empty()); + + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/node/src/validation/script.rs b/node/src/validation/script.rs index 8f03bf38c..f8892c0f0 100644 --- a/node/src/validation/script.rs +++ b/node/src/validation/script.rs @@ -74,6 +74,34 @@ pub trait ScriptVerifier: Send + Sync { height: u32, ) -> Result<(), ScriptError>; + /// [`verify_transaction`](Self::verify_transaction), except that a + /// failure may come back with a coarse reason. Pass it to + /// [`name_failure`](Self::name_failure) for the specific one. Finding + /// that name can mean running a second script engine over the input, so + /// a caller holding a lock can verify with this, release the lock, and + /// name the failure afterwards. + fn verify_transaction_unnamed( + &self, + tx: &Transaction, + prev_outputs: &[TxOut], + height: u32, + ) -> Result<(), ScriptError> { + self.verify_transaction(tx, prev_outputs, height) + } + + /// Give a failure from + /// [`verify_transaction_unnamed`](Self::verify_transaction_unnamed) its + /// specific reason. The verdict never changes; only the reason can. + fn name_failure( + &self, + _tx: &Transaction, + _prev_outputs: &[TxOut], + _height: u32, + err: ScriptError, + ) -> ScriptError { + err + } + /// If this verifier runs in shadow mode, return the shadow engine. /// Currently unused — ShadowVerifier handles async dispatch internally. fn shadow_verifier(&self) -> Option<&dyn ScriptVerifier> { @@ -309,8 +337,12 @@ impl ConsensusVerifier { /// revisited: libconsensus has already rejected this input and that stands. /// This runs only on the failure path, and only to write the message. /// +/// Only the input that failed is run. libconsensus has already passed every +/// input before it, so running those again would repeat that work for +/// nothing. +/// /// If the two engines disagree about this input — the native one finding it -/// valid — there is no name to borrow, and the coarse error is reported as +/// valid — there is no name to borrow, and `None` leaves the coarse error as /// it was. That disagreement is a parity bug worth knowing about, so it is /// logged rather than papered over. fn name_script_failure( @@ -318,17 +350,29 @@ fn name_script_failure( prev_outputs: &[TxOut], flags: u32, input: usize, - coarse: bitcoinconsensus::Error, -) -> String { - match consensus::verify_transaction(tx, prev_outputs, flags) { - Err((idx, consensus::Error::ErrScript(e))) if idx == input => e.as_str().to_string(), - _ => { +) -> Option { + let (txin, prev) = (tx.input.get(input)?, prev_outputs.get(input)?); + // The BIP 341 signature hash commits to every spent output; nothing + // else reads them (as in `consensus::verify_transaction`). + let checker_prevs: &[TxOut] = + if flags & consensus::VERIFY_TAPROOT != 0 { prev_outputs } else { &[] }; + let checker = consensus::sighash::TxSignatureChecker::new(tx, input, prev.value, checker_prevs); + let witness: Vec> = txin.witness.iter().map(|w| w.to_vec()).collect(); + match consensus::verify::verify_script( + txin.script_sig.as_bytes(), + prev.script_pubkey.as_bytes(), + &witness, + flags, + &checker, + ) { + Err(e) => Some(e.as_str().to_string()), + Ok(()) => { tracing::debug!( input, "the native engine does not agree that this input fails; \ reporting the coarse error" ); - format!("{coarse:?}") + None } } } @@ -339,6 +383,17 @@ impl ScriptVerifier for ConsensusVerifier { tx: &Transaction, prev_outputs: &[TxOut], height: u32, + ) -> Result<(), ScriptError> { + self.verify_transaction_unnamed(tx, prev_outputs, height) + .map_err(|e| self.name_failure(tx, prev_outputs, height, e)) + } + + /// libconsensus alone. A failure carries its coarse `ERR_SCRIPT`. + fn verify_transaction_unnamed( + &self, + tx: &Transaction, + prev_outputs: &[TxOut], + height: u32, ) -> Result<(), ScriptError> { let tx_bytes = bitcoin::consensus::serialize(tx); @@ -374,12 +429,27 @@ impl ScriptVerifier for ConsensusVerifier { ) .map_err(|e| ScriptError::VerifyFailed { input: input_index, - reason: name_script_failure(tx, prev_outputs, flags, input_index, e), + reason: format!("{e:?}"), })?; } Ok(()) } + + fn name_failure( + &self, + tx: &Transaction, + prev_outputs: &[TxOut], + height: u32, + err: ScriptError, + ) -> ScriptError { + let input = err.input(); + let flags = script_verify_flags(self.network, height); + match name_script_failure(tx, prev_outputs, flags, input) { + Some(reason) => ScriptError::VerifyFailed { input, reason }, + None => err, + } + } } /// No-op verifier for tests that don't need script checking. @@ -647,6 +717,30 @@ impl ScriptVerifier for ShadowVerifier { result } + /// The primary engine's unnamed verify, with the same shadow dispatch. + fn verify_transaction_unnamed( + &self, + tx: &Transaction, + prev_outputs: &[TxOut], + height: u32, + ) -> Result<(), ScriptError> { + let result = self.primary.verify_transaction_unnamed(tx, prev_outputs, height); + if result.is_ok() { + self.dispatch_shadow(tx, prev_outputs, height); + } + result + } + + fn name_failure( + &self, + tx: &Transaction, + prev_outputs: &[TxOut], + height: u32, + err: ScriptError, + ) -> ScriptError { + self.primary.name_failure(tx, prev_outputs, height, err) + } + fn dispatch_shadow(&self, tx: &Transaction, prev_outputs: &[TxOut], height: u32) { self.enqueue(ShadowWork { tx_bytes: bitcoin::consensus::serialize(tx), @@ -679,6 +773,10 @@ impl Drop for ShadowVerifier { } } +#[cfg(test)] +#[path = "script_naming_tests.rs"] +mod naming_tests; + #[cfg(test)] mod tests { use super::*; diff --git a/node/src/validation/script_naming_tests.rs b/node/src/validation/script_naming_tests.rs new file mode 100644 index 000000000..5e044da3f --- /dev/null +++ b/node/src/validation/script_naming_tests.rs @@ -0,0 +1,111 @@ +//! Naming a libbitcoinconsensus rejection. +//! +//! libbitcoinconsensus reports a coarse `ERR_SCRIPT`; Bitcoin Core, calling +//! `VerifyScript` directly, names the script error (`ScriptErrorString`, +//! src/script/script_error.cpp). satd asks its own engine for the name. That +//! takes only the input that failed, and the caller decides when to do it. + +use super::*; +use bitcoin::hashes::Hash; +use bitcoin::opcodes::all::{OP_PUSHNUM_1, OP_RETURN}; +use bitcoin::opcodes::OP_0; +use bitcoin::{Amount, OutPoint, ScriptBuf, Sequence, TxIn, Witness}; + +const HEIGHT: u32 = 1_000; +const EVAL_FALSE: &str = "Script evaluated without error but finished with a false/empty top stack element"; +const OP_RETURN_HIT: &str = "OP_RETURN was encountered"; + +/// A P2WSH input per witness script, each revealing its script. +fn p2wsh_spend(scripts: &[ScriptBuf]) -> (Transaction, Vec) { + let prevs: Vec = scripts + .iter() + .map(|ws| TxOut { + value: Amount::from_sat(10_000), + script_pubkey: ScriptBuf::new_p2wsh(&ws.wscript_hash()), + }) + .collect(); + let tx = Transaction { + version: bitcoin::transaction::Version(2), + lock_time: bitcoin::absolute::LockTime::ZERO, + input: scripts + .iter() + .enumerate() + .map(|(i, ws)| TxIn { + previous_output: OutPoint { txid: bitcoin::Txid::from_byte_array([i as u8 + 1; 32]), vout: 0 }, + script_sig: ScriptBuf::new(), + sequence: Sequence::MAX, + witness: Witness::from_slice(&[ws.as_bytes()]), + }) + .collect(), + output: vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: ScriptBuf::new_op_return([]) }], + }; + (tx, prevs) +} + +fn script(op: u8) -> ScriptBuf { + ScriptBuf::from_bytes(vec![op]) +} + +/// Only the input that failed is run to name it. Input 0 here fails too, +/// but naming input 1's failure must not depend on it: libbitcoinconsensus +/// already passed every input before the one it rejected, and running them +/// again only repeats that work. +#[test] +fn naming_runs_only_the_failing_input() { + let (tx, prevs) = p2wsh_spend(&[script(OP_0.to_u8()), script(OP_RETURN.to_u8())]); + let cpp = ConsensusVerifier::new(Network::Regtest); + let coarse = ScriptError::VerifyFailed { input: 1, reason: "ERR_SCRIPT".into() }; + let named = cpp.name_failure(&tx, &prevs, HEIGHT, coarse); + assert_eq!(named.input(), 1); + assert_eq!(named.reason(), OP_RETURN_HIT); +} + +/// `verify_transaction` names the failure as Core does, on the input that +/// failed. +#[test] +fn verify_transaction_names_the_failure() { + let (tx, prevs) = p2wsh_spend(&[script(OP_PUSHNUM_1.to_u8()), script(OP_RETURN.to_u8())]); + let err = ConsensusVerifier::new(Network::Regtest) + .verify_transaction(&tx, &prevs, HEIGHT) + .expect_err("input 1 fails"); + assert_eq!((err.input(), err.reason()), (1, OP_RETURN_HIT)); + + let (tx, prevs) = p2wsh_spend(&[script(OP_0.to_u8())]); + let err = ConsensusVerifier::new(Network::Regtest) + .verify_transaction(&tx, &prevs, HEIGHT) + .expect_err("input 0 fails"); + assert_eq!((err.input(), err.reason()), (0, EVAL_FALSE)); + + let (tx, prevs) = p2wsh_spend(&[script(OP_PUSHNUM_1.to_u8())]); + ConsensusVerifier::new(Network::Regtest) + .verify_transaction(&tx, &prevs, HEIGHT) + .expect("a true script passes"); +} + +/// The unnamed verify gives the same verdict and input with the coarse +/// reason, and leaves the name to `name_failure`. The shadow verifier passes +/// both through to its primary engine. +#[test] +fn the_unnamed_verify_leaves_the_name_for_later() { + let (tx, prevs) = p2wsh_spend(&[script(OP_PUSHNUM_1.to_u8()), script(OP_RETURN.to_u8())]); + let shadow = ShadowVerifier::new( + Box::new(ConsensusVerifier::new(Network::Regtest)), + Box::new(RustVerifier::new(Network::Regtest)), + "cpp", + "rust", + 16, + 1, + ); + let verifiers: [(&str, &dyn ScriptVerifier); 2] = + [("cpp", &ConsensusVerifier::new(Network::Regtest)), ("rust-shadow", &shadow)]; + for (name, v) in verifiers { + let err = v.verify_transaction_unnamed(&tx, &prevs, HEIGHT).expect_err("input 1 fails"); + assert_eq!(err.input(), 1, "{name}"); + assert_eq!(err.reason(), "ERR_SCRIPT", "{name}: named under the caller's lock"); + let named = v.name_failure(&tx, &prevs, HEIGHT, err); + assert_eq!((named.input(), named.reason()), (1, OP_RETURN_HIT), "{name}"); + + v.verify_transaction_unnamed(&p2wsh_spend(&[script(OP_PUSHNUM_1.to_u8())]).0, &prevs[..1], HEIGHT) + .expect("a true script passes"); + } +} From e1bc433ce6643f4218807b9a867c8bdf27d4c9b2 Mon Sep 17 00:00:00 2001 From: Benjamen Keroack Date: Thu, 8 Oct 2026 11:12:20 -0600 Subject: [PATCH 3/3] mempool: report only acting entries evicted; test helpers index spends Eviction removes a held descendant of an evicted acting entry with it. That entry never emitted an Enter, so a LeaveEvicted for it is a Leave for a txid the standard stream never showed. The eviction now returns each entry's scope, and admission (success and refusal) and the -maxmempool reload trim emit LeaveEvicted only for entries that were acting. The test insert helpers that take a caller's transaction now index its spends, as every admission does, since eviction finds descendants through that index. Co-Authored-By: Claude Opus 5.5 (1M context) --- node/src/mempool/pool.rs | 82 +++++++++++++++---------- node/src/mempool/pool_fullpool_tests.rs | 63 +++++++++++++++++++ 2 files changed, 113 insertions(+), 32 deletions(-) diff --git a/node/src/mempool/pool.rs b/node/src/mempool/pool.rs index 61c69fd09..d418aec52 100644 --- a/node/src/mempool/pool.rs +++ b/node/src/mempool/pool.rs @@ -1560,6 +1560,17 @@ impl Mempool { } } + /// `LeaveEvicted` for each evicted entry that was acting. A held entry + /// never emitted an `Enter`, so a `Leave` for it would be a phantom that + /// leaks a withheld txid (§10). + fn emit_evicted(&self, evicted: &[(Txid, QuarantineScope)], reason: EvictReason) { + for (txid, scope) in evicted { + if scope.is_acting() { + self.emit(MempoolEvent::LeaveEvicted { txid: *txid, reason }); + } + } + } + /// Emit a quarantine-class event on the separate channel (§10). Best-effort, /// no-op when no sender is wired. fn emit_quarantine(&self, event: QuarantineEvent) { @@ -1640,20 +1651,21 @@ impl Mempool { // Core never leaves the pool over `-maxmempool`: every admission ends // in `LimitMempoolSize` → `TrimToSize` (src/validation.cpp). Left over a // lowered limit, the pool refused every admission until it drained. - let evicted = { + let none = HashSet::new(); + let (acting, held) = { let mut inner = self.inner.write(); let over = inner.acting_bytes().saturating_sub(max_size_bytes); - let evicted = Self::evict_lowest_fee_entries(&mut inner, over, false, incremental_relay_fee); - // A held entry never emitted an `Enter`, so its eviction emits no - // `LeaveEvicted` (§10), as in `accept_transaction`. + let acting = + Self::evict_lowest_fee_entries_except(&mut inner, over, false, incremental_relay_fee, &none); let over = inner.quarantine_bytes.saturating_sub(quarantine_max_bytes); - Self::evict_lowest_fee_entries(&mut inner, over, true, incremental_relay_fee); + let held = + Self::evict_lowest_fee_entries_except(&mut inner, over, true, incremental_relay_fee, &none); self.sync_unbroadcast_len(&inner); - evicted + (acting, held) }; - for txid in &evicted { - self.emit(MempoolEvent::LeaveEvicted { txid: *txid, reason: EvictReason::FullPool }); - } + // Reported as admission reports them: only what was acting. + self.emit_evicted(&acting, EvictReason::FullPool); + self.emit_evicted(&held, EvictReason::Policy); } /// Load (or replace) the transaction-filtering ruleset from a file. Returns @@ -3043,7 +3055,7 @@ impl Mempool { return Err(MempoolError::Script(e.reason().to_string(), e.core_debug_string(&tx))); } - let mut evicted_full_pool: Vec = Vec::new(); + let mut evicted_full_pool: Vec<(Txid, QuarantineScope)> = Vec::new(); if needs_room { // Evict lowest-fee-rate entries *of this class* until the pool, // with this transaction in it, is back within its limit: Core's @@ -3068,11 +3080,7 @@ impl Mempool { if class_bytes_after + tx_size > class_budget { drop(inner); // The evicted entries are gone whether or not this one gets in. - if !quarantined { - for evicted_txid in &evicted_full_pool { - self.emit(MempoolEvent::LeaveEvicted { txid: *evicted_txid, reason: evict_reason }); - } - } + self.emit_evicted(&evicted_full_pool, evict_reason); return Err(MempoolError::MempoolFull); } } @@ -3157,19 +3165,10 @@ impl Mempool { // are best-effort but keeping the lock duration tight is the rule. drop(inner); - // `evicted_full_pool` is always drawn from the incoming tx's own class - // (`evict_lowest_fee_entries` filters to `quarantined`). Acting-class - // evictions go on the standard stream as before; quarantine-class - // evictions never emitted an `Enter`, so a `LeaveEvicted` for them would - // be a phantom Leave leaking a withheld txid (§10) — suppress it. - if !quarantined { - for evicted_txid in &evicted_full_pool { - self.emit(MempoolEvent::LeaveEvicted { - txid: *evicted_txid, - reason: evict_reason, - }); - } - } + // Acting-class evictions go on the standard stream as before; + // quarantine-class ones never emitted an `Enter`, so a `LeaveEvicted` + // for them would be a phantom Leave leaking a withheld txid (§10). + self.emit_evicted(&evicted_full_pool, evict_reason); for conflict_txid in &replaced { self.emit(MempoolEvent::LeaveReplaced { txid: *conflict_txid, @@ -6176,6 +6175,9 @@ impl Mempool { incremental_relay_fee, &HashSet::new(), ) + .into_iter() + .map(|(txid, _)| txid) + .collect() } /// [`evict_lowest_fee_entries`](Self::evict_lowest_fee_entries), never @@ -6188,13 +6190,17 @@ impl Mempool { /// The cost follows what is evicted, not the size of the pool: one pass /// builds a heap of the class, each eviction pops it, and descendants come /// from the `spends` index. + /// + /// Each evicted txid comes with the scope it had: a held descendant of an + /// evicted acting entry goes with it, but never emitted an `Enter`, so it + /// gets no `LeaveEvicted` (§10). fn evict_lowest_fee_entries_except( inner: &mut MempoolInner, bytes_needed: usize, want_quarantined: bool, incremental_relay_fee: u64, keep: &HashSet, - ) -> Vec { + ) -> Vec<(Txid, QuarantineScope)> { if bytes_needed == 0 { return Vec::new(); } @@ -6281,6 +6287,7 @@ impl Mempool { } } + let mut evicted = Vec::with_capacity(to_remove.len()); for txid in &to_remove { if let Some(entry) = inner.entries.remove(txid) { let tx_size = bitcoin::consensus::serialize(&entry.tx).len(); @@ -6290,13 +6297,14 @@ impl Mempool { } inner.unbroadcast.remove(txid); tracing::debug!(%txid, fee_rate = entry.fee_rate, "Evicted low-fee tx from mempool"); + evicted.push((*txid, entry.scope)); } } - if !to_remove.is_empty() { - tracing::info!(evicted = to_remove.len(), "Mempool eviction complete"); + if !evicted.is_empty() { + tracing::info!(evicted = evicted.len(), "Mempool eviction complete"); } - to_remove + evicted } /// Keep the lock-free `unbroadcast_len` mirror coherent with the map. @@ -6491,6 +6499,11 @@ impl Mempool { #[cfg(test)] pub(crate) fn insert_entry_for_test(&self, txid: Txid, tx: Transaction, fee_rate: u64) { let mut inner = self.inner.write(); + // Every admission indexes the entry's spends, and eviction finds + // descendants through them. + for input in &tx.input { + inner.spends.insert(input.previous_output, txid); + } inner.entries.insert( txid, MempoolEntry { @@ -6668,6 +6681,11 @@ impl Mempool { let txid = tx.compute_txid(); let tx_size = bitcoin::consensus::serialize(&tx).len(); let mut inner = self.inner.write(); + // Every admission indexes the entry's spends, and eviction finds + // descendants through them. + for input in &tx.input { + inner.spends.insert(input.previous_output, txid); + } inner.entries.insert( txid, MempoolEntry { diff --git a/node/src/mempool/pool_fullpool_tests.rs b/node/src/mempool/pool_fullpool_tests.rs index 054d11a82..b90f767bd 100644 --- a/node/src/mempool/pool_fullpool_tests.rs +++ b/node/src/mempool/pool_fullpool_tests.rs @@ -408,3 +408,66 @@ fn a_script_failure_is_named_after_the_pool_is_released() { let _ = std::fs::remove_dir_all(&dir); } + +const HELD: QuarantineScope = QuarantineScope { relay: true, template: true }; + +/// An acting parent with a held child, and a better-paying acting entry. +/// The held child never emitted an `Enter`, so it must not be reported as +/// leaving the standard stream when it goes with its parent. +fn parent_with_held_child(tags: &[u8]) -> (ChainState, Mempool, std::path::PathBuf, Txid, Txid) { + let (cs, mp, dir) = env(tags); + let parent = admit(&mp, &cs, spend(&[prev(tags[0])], COIN_VALUE, 100, 1, tags[0])); + admit(&mp, &cs, spend(&[prev(tags[1])], COIN_VALUE, 2_000, 1, tags[1])); + let child = mp.insert_tx_scoped_for_test( + spend(&[OutPoint { txid: parent, vout: 0 }], COIN_VALUE - 100, 1_000, 1, 0x40), + HELD, + ); + // Unexpired, so the expiry sweep after an admission leaves it alone. + mp.inner.write().entries.get_mut(&child).unwrap().time = crate::time::now_secs(); + (cs, mp, dir, parent, child) +} + +/// Admission evicts the acting parent, and the held child with it, and +/// reports only the parent. +#[test] +fn a_held_descendant_evicted_on_admission_is_not_reported() { + let (cs, mp, dir, parent, child) = parent_with_held_child(&[1, 2, 3]); + set_max(&mp, mp.acting_bytes()); + let mut rx = events(&mp); + + admit(&mp, &cs, spend(&[prev(3)], COIN_VALUE, 10_000, 1, 3)); + let now = pool(&mp); + assert!(!now.contains(&parent) && !now.contains(&child)); + assert_eq!(evicted(&mut rx), vec![parent], "a held entry was reported leaving the standard stream"); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// The same on a live `-maxmempool` decrease. +#[test] +fn a_held_descendant_trimmed_on_reload_is_not_reported() { + let (_cs, mp, dir, parent, child) = parent_with_held_child(&[1, 2]); + let mut rx = events(&mp); + + let cfg = MempoolConfig { max_size_bytes: mp.acting_bytes() - 1, ..mp.config.read().clone() }; + mp.reload_policy(cfg); + let now = pool(&mp); + assert!(!now.contains(&parent) && !now.contains(&child)); + assert_eq!(evicted(&mut rx), vec![parent], "a held entry was reported leaving the standard stream"); + + let _ = std::fs::remove_dir_all(&dir); +} + +/// The test insert helpers index spends as every admission does, so a pool +/// built with them has the shape eviction expects. +#[test] +fn test_inserted_entries_index_their_spends() { + let mp = Mempool::new(1_000_000, 0); + let parent = mp.insert_tx_scoped_for_test(spend(&[prev(1)], COIN_VALUE, 100, 1, 1), QuarantineScope::acting()); + let child = spend(&[OutPoint { txid: parent, vout: 0 }], COIN_VALUE - 100, 100, 1, 2); + let child_txid = child.compute_txid(); + mp.insert_entry_for_test(child_txid, child, 1); + let inner = mp.inner.read(); + assert_eq!(inner.spends.get(&prev(1)), Some(&parent)); + assert_eq!(inner.spends.get(&OutPoint { txid: parent, vout: 0 }), Some(&child_txid)); +}