diff --git a/zebra/zebra-consensus/src/transaction.rs b/zebra/zebra-consensus/src/transaction.rs index d3fb26f9..d77179b3 100644 --- a/zebra/zebra-consensus/src/transaction.rs +++ b/zebra/zebra-consensus/src/transaction.rs @@ -44,6 +44,7 @@ use zebra_state as zs; use crate::{error::TransactionError, primitives, script, BoxError}; pub mod check; +mod script_cache; #[cfg(test)] mod tests; @@ -61,6 +62,14 @@ mod tests; /// chain in the correct order.) const UTXO_LOOKUP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(6 * 60); +/// The maximum number of in-flight state UTXO lookups per transaction. +/// +/// Bounds the concurrency of `block_spent_utxos` so one transaction with +/// thousands of transparent inputs cannot flood the state service's buffer, +/// while still overlapping the per-lookup state latency instead of paying it +/// as a serial chain of awaited round trips. +const MAX_CONCURRENT_UTXO_LOOKUPS: usize = 64; + /// A timeout applied to output lookup requests sent to the mempool. This is shorter than the /// timeout for the state UTXO lookups because a block is likely to be mined every 75 seconds /// after Blossom is active, changing the best chain tip and requiring re-verification of transactions @@ -296,6 +305,17 @@ where auth_digest, }), }; + + // This id keys the script verification cache, so it must name the transaction + // actually carried by this request. The auth digest half is recomputed from + // `tx` above; the hash half is trusted because the block verifier derives + // `transaction_hash` from the same transaction it sends here, and binds it to + // the block through the merkle root check. See `script_cache::ScriptCacheKey`. + debug_assert_eq!( + req.transaction_hash, + tx.hash(), + "BlockRequest::transaction_hash must be the hash of BlockRequest::transaction", + ); let height = req.height; let time = req.time; let known_utxos = req.known_utxos.clone(); @@ -347,9 +367,9 @@ where // Load spent UTXOs from the block context and state. // The UTXOs are required for almost all the async checks. // - // This phase is one awaited state round trip per transparent input, so its share of - // the total tells operators whether transaction verification is bound by state - // lookups or by the cryptographic checks timed below. + // This phase overlaps up to `MAX_CONCURRENT_UTXO_LOOKUPS` state lookups, so its + // share of the total tells operators whether transaction verification is bound by + // state lookups or by the cryptographic checks timed below. let utxo_fetch_start = Instant::now(); let spent_utxos_result = Self::block_spent_utxos(tx.clone(), known_utxos, state.clone()).await; @@ -371,6 +391,7 @@ where // Select version-specific async verification pipeline let async_checks = dispatch_version_verification( tx.as_ref(), + tx_id, nu, script_verifier, cached_ffi_transaction.clone() @@ -394,6 +415,20 @@ where tracing::trace!(?tx_id, "finished async checks"); + // Remember the verified transparent scripts, so re-verifying this + // transaction (proposal to submitblock, a reorg, or a resubmit) can skip + // them. Success-only; see the `script_cache` module docs. Coinbase + // transactions have no `PrevOut` inputs to remember, and shielded-only + // transactions have no transparent inputs at all, so remembering them + // would only occupy a cache slot whose hit saves nothing. + if !tx.is_coinbase() && !tx.inputs().is_empty() { + script_cache::verified_scripts().insert(script_cache::ScriptCacheKey::new( + tx_id, + nu, + cached_ffi_transaction.all_previous_outputs(), + )); + } + let miner_fee = if tx.is_coinbase() { None } else { @@ -448,33 +483,63 @@ where // Pre-allocate with None so we can fill each slot by input index, preserving input order. let mut spent_outputs: Vec> = vec![None; inputs.len()]; - for (input_idx, input) in inputs.iter().enumerate() { - if let transparent::Input::PrevOut { outpoint, .. } = input { - tracing::trace!("awaiting outpoint lookup"); - - let utxo = if let Some(output) = known_utxos.get(outpoint) { - tracing::trace!("UTXO in known_utxos, discarding query"); - output.utxo.clone() - } else { - let response = state - .clone() - .oneshot(zebra_state::Request::AwaitUtxo(*outpoint)) - .await - .map_err(|boxed_error| match boxed_error.downcast::() { - Ok(_) => TransactionError::TransparentInputNotFound, - Err(boxed_error) => TransactionError::from(boxed_error), - })?; - - if let zebra_state::Response::Utxo(utxo) = response { + // Look up the spent UTXOs concurrently. With hundreds or thousands of + // transparent inputs, one awaited state round trip per input turns block + // verification into a serial latency chain (the `utxo_fetch` phase of the + // `zebra.consensus.transaction.duration_seconds` metric); overlapping the + // lookups pays that latency once, not once per input. Lookup results carry + // their input index, so completion order cannot affect `spent_outputs` order. + // + // The futures are collected eagerly so each owns its captures; a lazy + // iterator would hold `&state` inside the stream and require `ZS: Sync`. + let lookups: Vec<_> = inputs + .iter() + .enumerate() + .filter_map(|(input_idx, input)| { + let transparent::Input::PrevOut { outpoint, .. } = input else { + return None; + }; + let outpoint = *outpoint; + let known_utxo = known_utxos + .get(&outpoint) + .map(|ordered| ordered.utxo.clone()); + let state = state.clone(); + + Some(async move { + let utxo = if let Some(utxo) = known_utxo { + tracing::trace!("UTXO in known_utxos, discarding query"); utxo } else { - unreachable!("AwaitUtxo always responds with Utxo") - } - }; - tracing::trace!(?utxo, "got UTXO"); - spent_outputs[input_idx] = Some(utxo.output.clone()); - spent_utxos.insert(*outpoint, utxo); - } + tracing::trace!("awaiting outpoint lookup"); + + let response = state + .oneshot(zebra_state::Request::AwaitUtxo(outpoint)) + .await + .map_err(|boxed_error| match boxed_error.downcast::() { + Ok(_) => TransactionError::TransparentInputNotFound, + Err(boxed_error) => TransactionError::from(boxed_error), + })?; + + if let zebra_state::Response::Utxo(utxo) = response { + utxo + } else { + unreachable!("AwaitUtxo always responds with Utxo") + } + }; + + Ok::<_, TransactionError>((input_idx, outpoint, utxo)) + }) + }) + .collect(); + + let mut lookups = + futures::stream::iter(lookups).buffer_unordered(MAX_CONCURRENT_UTXO_LOOKUPS); + + while let Some(lookup) = lookups.next().await { + let (input_idx, outpoint, utxo) = lookup?; + tracing::trace!(?utxo, "got UTXO"); + spent_outputs[input_idx] = Some(utxo.output.clone()); + spent_utxos.insert(outpoint, utxo); } let spent_outputs: Vec = spent_outputs.into_iter().flatten().collect(); @@ -608,6 +673,7 @@ where // Select version-specific async verification pipeline let mut async_checks = dispatch_version_verification( tx.as_ref(), + tx_id, nu, script_verifier, cached_ffi_transaction.clone() @@ -646,13 +712,30 @@ where tracing::trace!(?tx_id, "finished async checks"); + // Remember the verified transparent scripts, so the block that mines this + // transaction can skip re-running them. Success-only; see the + // `script_cache` module docs. Shielded-only transactions have no scripts + // worth remembering. + // + // Conservatively skipped when the transaction spends unmined mempool + // outputs. `mempool_spent_utxos()` does pair those outputs by input index + // today, but the pairing has a history of alignment bugs (zebra#10346), + // the spent data comes from unmined transactions rather than the chain, + // and the reuse value is small: such transactions are simply re-verified + // in full when they arrive in a block. + if spent_mempool_outpoints.is_empty() && !tx.inputs().is_empty() { + script_cache::verified_scripts().insert(script_cache::ScriptCacheKey::new( + tx_id, + nu, + cached_ffi_transaction.all_previous_outputs(), + )); + } + let sigops = tx.sigops().map_err(zebra_script::Error::from)?; - // TODO: `spent_outputs` may not align with `tx.inputs()` when a transaction - // spends both chain and mempool UTXOs (mempool outputs are appended last by - // `mempool_spent_utxos()`), causing policy checks to pair the wrong input with - // the wrong spent output. - // https://github.com/ZcashFoundation/zebra/issues/10346 + // `spent_outputs` is aligned with `tx.inputs()`: `mempool_spent_utxos()` + // fills a slot per input index in both its chain and mempool passes. + // zebra#10346 tracked a misalignment in an earlier version of that pairing. let spent_outputs = cached_ffi_transaction.all_previous_outputs().clone(); let transaction = VerifiedUnminedTx::new( @@ -981,6 +1064,7 @@ fn check_maturity_height( /// are not supported by any network upgrade Zebra verifies. fn dispatch_version_verification( tx: &Transaction, + tx_id: UnminedTxId, nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, @@ -992,16 +1076,17 @@ fn dispatch_version_verification( } Transaction::V4 { joinsplit_data, .. } => verify_v4_transaction( tx, + tx_id, nu, script_verifier, cached_ffi_transaction, joinsplit_data, ), Transaction::V5 { .. } => { - verify_v5_transaction(tx, nu, script_verifier, cached_ffi_transaction) + verify_v5_transaction(tx, tx_id, nu, script_verifier, cached_ffi_transaction) } Transaction::V6 { .. } => { - verify_v6_transaction(tx, nu, script_verifier, cached_ffi_transaction) + verify_v6_transaction(tx, tx_id, nu, script_verifier, cached_ffi_transaction) } } } @@ -1025,6 +1110,7 @@ fn dispatch_version_verification( #[allow(clippy::unwrap_in_result)] fn verify_v4_transaction( tx: &Transaction, + tx_id: UnminedTxId, nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, @@ -1038,11 +1124,15 @@ fn verify_v4_transaction( .sighasher() .sighash(HashType::ALL, None); - Ok( - verify_transparent_inputs_and_outputs(tx, script_verifier, cached_ffi_transaction)? - .and(verify_sprout_shielded_data(joinsplit_data, &sighash)?) - .and(verify_sapling_bundle(sapling_bundle, &sighash)), - ) + Ok(verify_transparent_inputs_and_outputs( + tx, + tx_id, + nu, + script_verifier, + cached_ffi_transaction, + )? + .and(verify_sprout_shielded_data(joinsplit_data, &sighash)?) + .and(verify_sapling_bundle(sapling_bundle, &sighash))) } /// Verifies if a V4 `transaction` is supported by `network_upgrade`. @@ -1110,6 +1200,7 @@ fn verify_v4_transaction_network_upgrade( #[allow(clippy::unwrap_in_result)] fn verify_v5_transaction( tx: &Transaction, + tx_id: UnminedTxId, nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, @@ -1123,11 +1214,15 @@ fn verify_v5_transaction( .sighasher() .sighash(HashType::ALL, None); - Ok( - verify_transparent_inputs_and_outputs(tx, script_verifier, cached_ffi_transaction)? - .and(verify_sapling_bundle(sapling_bundle, &sighash)) - .and(verify_orchard_bundle(orchard_bundle, &sighash, nu)), - ) + Ok(verify_transparent_inputs_and_outputs( + tx, + tx_id, + nu, + script_verifier, + cached_ffi_transaction, + )? + .and(verify_sapling_bundle(sapling_bundle, &sighash)) + .and(verify_orchard_bundle(orchard_bundle, &sighash, nu))) } /// Verifies if a V5 `transaction` is supported by `network_upgrade`. @@ -1179,6 +1274,7 @@ fn verify_v5_transaction_network_upgrade( /// NU6.3 key, not the v5 fixed key. fn verify_v6_transaction( tx: &Transaction, + tx_id: UnminedTxId, nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, @@ -1195,12 +1291,16 @@ fn verify_v6_transaction( // The Ironwood bundle reuses the Orchard Action proof system and the NU6.3 circuit key, so // it is verified the same way as the v6 Orchard bundle (against the NU6.3 key). - Ok( - verify_transparent_inputs_and_outputs(tx, script_verifier, cached_ffi_transaction)? - .and(verify_sapling_bundle(sapling_bundle, &sighash)) - .and(verify_orchard_v6_bundle(orchard_bundle, &sighash)) - .and(verify_orchard_v6_bundle(ironwood_bundle, &sighash)), - ) + Ok(verify_transparent_inputs_and_outputs( + tx, + tx_id, + nu, + script_verifier, + cached_ffi_transaction, + )? + .and(verify_sapling_bundle(sapling_bundle, &sighash)) + .and(verify_orchard_v6_bundle(orchard_bundle, &sighash)) + .and(verify_orchard_v6_bundle(ironwood_bundle, &sighash))) } /// Verifies that a V6 `transaction` is supported by `network_upgrade`. @@ -1240,6 +1340,8 @@ fn verify_v6_transaction_network_upgrade( /// Returns the asynchronous script verification checks for transparent inputs in `tx`. fn verify_transparent_inputs_and_outputs( tx: &Transaction, + tx_id: UnminedTxId, + nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, ) -> Result { @@ -1247,6 +1349,23 @@ fn verify_transparent_inputs_and_outputs( // The script verifier only verifies PrevOut inputs and their corresponding UTXOs. // Coinbase transactions don't have any PrevOut inputs. Ok(AsyncChecks::new()) + } else if tx.inputs().is_empty() { + // A shielded-only transaction has no transparent input scripts to verify, + // so there is nothing to check and nothing worth consulting the cache for. + Ok(AsyncChecks::new()) + } else if script_cache::verified_scripts().contains(&script_cache::ScriptCacheKey::new( + tx_id, + nu, + cached_ffi_transaction.all_previous_outputs(), + )) { + // This transaction's input scripts already verified against these spent + // outputs under this network upgrade (at mempool admission, in a template + // proposal, or in an earlier block request), and the key determines + // everything script verification reads, so the per-input script checks are + // skipped rather than re-run. Every other check on this transaction still + // runs at the requesting block's height. See the `script_cache` module docs + // for the derivation. + Ok(AsyncChecks::new()) } else { // feed all of the inputs to the script verifier let inputs = tx.inputs(); diff --git a/zebra/zebra-consensus/src/transaction/script_cache.rs b/zebra/zebra-consensus/src/transaction/script_cache.rs new file mode 100644 index 00000000..85116fbf --- /dev/null +++ b/zebra/zebra-consensus/src/transaction/script_cache.rs @@ -0,0 +1,455 @@ +//! Memoization of successful transparent script verification, per transaction. +//! +//! Zebra verifies every transaction's transparent input scripts at least twice +//! on the common path: once at mempool admission, and again when the +//! transaction arrives in a block (a `getblocktemplate` proposal check pays a +//! third time). zcashd skips the repeat through its script execution and +//! signature caches, so a zcashd miner revalidates a block built from its own +//! mempool in well under a second, while a Zebra miner pays for every +//! signature again. This module removes that asymmetry for the script half of +//! the work. +//! +//! # What is remembered +//! +//! One entry means: "every transparent input script of the transaction named +//! by this key verified successfully under this network upgrade". A hit lets +//! the transaction verifier skip re-running the script interpreter and its +//! signature checks for that transaction. Nothing else is skipped on a hit: +//! structure checks, lock time, expiry height, fees, sigop limits, shielded +//! proof verification, and the state service's spentness and double-spend +//! checks all still run for every block the transaction appears in. +//! +//! # Why the key is complete +//! +//! A hit replaces a verification, so the key must determine every input that +//! verification reads. Transparent script verification of one transaction +//! reads the transaction bytes (scriptSigs, outpoints, and for v5+ sighashes +//! the whole effecting data), the spent outputs' scriptPubKeys and values, +//! and the consensus branch semantics active at the verification height. +//! +//! * The transaction bytes are committed by [`UnminedTxId`]: +//! - a legacy (v1-v4) id is the double-SHA256 of the whole serialized +//! transaction, which contains the scriptSigs directly; +//! - a witnessed (v5+) id pairs the ZIP-244 txid, which commits to all +//! effecting data, with the ZIP-244 authorizing-data digest, which +//! commits to the scriptSigs. The txid alone would not be enough: it +//! deliberately excludes authorizing data, and answering a same-txid twin +//! from the cached verification of a differently-signed transaction is +//! exactly CVE-2026-34377 (GHSA-3vmh-33xr-9cqh). +//! * The spent outputs (each input's scriptPubKey and value) are committed +//! directly, as a digest over their serialization in input order. On a real +//! chain this is already implied: each input names an outpoint (creating +//! txid, output index), outpoints are effecting data, and a txid commits to +//! its transaction's outputs, so one outpoint denotes exactly one +//! scriptPubKey and value everywhere. The digest makes that property +//! structural instead of argued: even a caller that presents divergent data +//! for an outpoint (mocks and tests can; production cannot) gets a distinct +//! key, never a reused verdict. Whether the output exists and is unspent on +//! this chain at this height stays the state service's contextual check, +//! which never consults this cache. +//! * The branch semantics are the [`NetworkUpgrade`] in the key: it is the +//! same value the verifier hands to `zebra_script`, and it selects the +//! sighash algorithm and interpreter flags. Distinct upgrades never share +//! an entry, so an upgrade boundary re-verifies instead of reusing. +//! +//! # Why a stale or missing entry is always safe +//! +//! Only successful verifications are inserted, so the cache can turn repeated +//! work into a hit but can never turn a failure into an acceptance. Evicting +//! or removing an entry only costs a re-verification. + +use std::{ + collections::{HashSet, VecDeque}, + sync::Mutex, +}; + +use once_cell::sync::Lazy; + +use zebra_chain::{ + parameters::NetworkUpgrade, + serialization::{sha256d, ZcashSerialize}, + transaction::UnminedTxId, + transparent, +}; + +/// The maximum number of remembered transactions. +/// +/// Sized to hold several blocks of history plus a full mempool's worth of +/// churn. Each entry is stored twice (lookup set and eviction queue) at under +/// 100 bytes per copy, so a full cache costs a few MiB. +const SCRIPT_CACHE_CAPACITY: usize = 30_000; + +/// The process-wide transparent script verification cache. +/// +/// Process-wide for the same reason the signature batch verifiers in +/// [`crate::primitives`] are: the proposition it stores does not depend on +/// which service verified it (see the module docs), and the mempool and block +/// verifiers must share it for mempool-to-block reuse to happen. +static VERIFIED_SCRIPTS: Lazy = + Lazy::new(|| VerifiedScripts::new(SCRIPT_CACHE_CAPACITY)); + +/// Returns the process-wide transparent script verification cache. +pub(super) fn verified_scripts() -> &'static VerifiedScripts { + &VERIFIED_SCRIPTS +} + +/// A key naming one proposition: "every transparent input script of this +/// transaction verifies against these spent outputs under this network +/// upgrade". +/// +/// # Correctness +/// +/// The caller constructing a key promises that `tx_id` is derived from the +/// transaction whose scripts are verified (the hash half from the bytes the +/// block or mempool actually carried, and, for v5+, the authorizing-data +/// digest recomputed from that same transaction), and that `spent_outputs` +/// are the outputs the script interpreter actually reads, in input order. An +/// id that does not determine the transaction's authorizing data would let a +/// differently-signed twin be answered from this transaction's verification +/// (CVE-2026-34377); see the module docs for the full derivation. +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub(super) struct ScriptCacheKey { + /// The unmined id of the verified transaction. + tx_id: UnminedTxId, + /// The network upgrade whose branch id and interpreter semantics the + /// scripts verified under. + nu: NetworkUpgrade, + /// A double-SHA256 digest of the spent outputs' serializations, in input + /// order. Output serialization is self-delimiting (value, then a + /// length-prefixed script), so the concatenation is unambiguous. + spent_outputs_digest: [u8; 32], +} + +impl ScriptCacheKey { + /// Builds the key for `tx_id`'s script verification against + /// `spent_outputs` under `nu`. + pub(super) fn new( + tx_id: UnminedTxId, + nu: NetworkUpgrade, + spent_outputs: &[transparent::Output], + ) -> Self { + let mut writer = sha256d::Writer::default(); + for output in spent_outputs { + output.zcash_serialize(&mut writer).expect( + "output serialization only fails if the lock script length exceeds MAX_PROTOCOL_MESSAGE_LEN (2 MiB); \ + spent outputs come from blocks bounded by MAX_BLOCK_BYTES (2,000,000), so serialization is infallible here", + ); + } + + Self { + tx_id, + nu, + spent_outputs_digest: writer.finish(), + } + } +} + +/// The set of remembered script verifications, bounded by FIFO eviction. +/// +/// # Correctness +/// +/// `lookup` and `eviction_order` always hold the same keys: only +/// [`VerifiedScripts::insert`] and the test-only [`VerifiedScripts::remove`] +/// mutate them, and each mutates both, so no caller can leave them holding +/// different keys. +/// +/// Eviction is FIFO rather than LRU: the working set is the mempool, which +/// turns over roughly in arrival order, and FIFO keeps the read path free of +/// bookkeeping ([`VerifiedScripts::contains`] never mutates). Evicting an +/// entry only costs a re-verification, never correctness. +pub(super) struct VerifiedScripts { + inner: Mutex, + /// Test-only: hits per key, so a test can observe that a specific + /// verification was answered from the cache rather than re-run. + /// + /// Grows with every distinct key hit in the test process and is never + /// reset (counts survive [`VerifiedScripts::remove`]); both are fine for a + /// test binary's lifetime and keep hit history observable. + #[cfg(test)] + hits_by_key: Mutex>, +} + +/// The collections behind [`VerifiedScripts`]' mutex. +struct VerifiedScriptsInner { + /// Answers [`VerifiedScripts::contains`]. + lookup: HashSet, + /// Chooses which key to drop when the cache is full. + eviction_order: VecDeque, + /// The maximum number of keys kept. + capacity: usize, +} + +impl VerifiedScripts { + /// Creates an empty cache holding at most `capacity` keys. + /// + /// A capacity of zero degenerates to a capacity of one: the eviction loop + /// empties the queue and the new key is still pushed. + fn new(capacity: usize) -> Self { + Self { + inner: Mutex::new(VerifiedScriptsInner { + lookup: HashSet::with_capacity(capacity), + eviction_order: VecDeque::with_capacity(capacity), + capacity, + }), + #[cfg(test)] + hits_by_key: Mutex::new(std::collections::HashMap::new()), + } + } + + /// Returns whether `key` was previously recorded by [`VerifiedScripts::insert`]. + /// + /// The hit/miss metric is reported after the lock is released: the metrics + /// macros allocate their label sets, and this lock is taken by every + /// non-coinbase transaction the node verifies. + pub(super) fn contains(&self, key: &ScriptCacheKey) -> bool { + let hit = self + .inner + .lock() + .expect("the script cache lock only guards infallible collection operations, so no panic can poison it") + .lookup + .contains(key); + + #[cfg(test)] + if hit { + *self + .hits_by_key + .lock() + .expect("the hit counter lock only guards infallible map operations, so no panic can poison it") + .entry(*key) + .or_default() += 1; + } + + metrics::counter!( + "zebra.consensus.transaction.script_cache.lookups", + "outcome" => if hit { "hit" } else { "miss" }, + ) + .increment(1); + + hit + } + + /// Records that `key`'s proposition was verified. + /// + /// The caller promises the named transaction's scripts actually verified + /// under the named upgrade; only successes may be recorded (module docs). + pub(super) fn insert(&self, key: ScriptCacheKey) { + let (inserted, evicted, size) = { + let mut inner = self + .inner + .lock() + .expect("the script cache lock only guards infallible collection operations, so no panic can poison it"); + + // Concurrent verifications of one transaction can both miss and + // both insert; the second insert must not push a duplicate into + // the eviction queue, or the two collections would drift apart. + if !inner.lookup.insert(key) { + (false, 0, inner.lookup.len()) + } else { + // Evict before pushing: pushing first would grow the queue + // past `capacity`, doubling its allocation for the rest of + // the process. + let mut evicted: u64 = 0; + while inner.eviction_order.len() >= inner.capacity { + // `break` rather than unwrap, so a zero capacity cannot + // spin or panic. + let Some(oldest) = inner.eviction_order.pop_front() else { + break; + }; + inner.lookup.remove(&oldest); + evicted += 1; + } + inner.eviction_order.push_back(key); + + (true, evicted, inner.lookup.len()) + } + }; + + // Metrics after the lock is released; see `contains`. + if inserted { + metrics::counter!("zebra.consensus.transaction.script_cache.inserts").increment(1); + if evicted > 0 { + metrics::counter!("zebra.consensus.transaction.script_cache.evictions") + .increment(evicted); + } + metrics::gauge!("zebra.consensus.transaction.script_cache.entries").set(size as f64); + } + } + + /// Test-only: how many times `key` was answered from the cache. + #[cfg(test)] + pub(super) fn hits_for(&self, key: &ScriptCacheKey) -> u64 { + self.hits_by_key + .lock() + .expect("the hit counter lock only guards infallible map operations, so no panic can poison it") + .get(key) + .copied() + .unwrap_or_default() + } + + /// Forgets one remembered verification. + /// + /// Always safe: forgetting a key costs a re-verification and can never + /// accept scripts that were not verified. Test-only and key-scoped, so a + /// test can force re-verification of its own transaction without racing + /// other tests' entries in the process-global cache. + #[cfg(test)] + pub(super) fn remove(&self, key: &ScriptCacheKey) { + let mut inner = self + .inner + .lock() + .expect("the script cache lock only guards infallible collection operations, so no panic can poison it"); + inner.lookup.remove(key); + inner.eviction_order.retain(|queued| queued != key); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use zebra_chain::transaction::Hash; + + /// A distinct legacy-id key for tests. + fn key(n: u8, nu: NetworkUpgrade) -> ScriptCacheKey { + ScriptCacheKey::new(UnminedTxId::Legacy(Hash([n; 32])), nu, &[]) + } + + #[test] + fn distinct_spent_outputs_are_distinct_entries() { + let cache = VerifiedScripts::new(4); + let tx_id = UnminedTxId::Legacy(Hash([1; 32])); + + let output = transparent::Output { + value: zebra_chain::amount::Amount::try_from(1).expect("valid amount"), + lock_script: transparent::Script::new(&[0x51]), + }; + let other_output = transparent::Output { + value: zebra_chain::amount::Amount::try_from(2).expect("valid amount"), + lock_script: transparent::Script::new(&[0x51]), + }; + + cache.insert(ScriptCacheKey::new( + tx_id, + NetworkUpgrade::Nu5, + std::slice::from_ref(&output), + )); + + assert!(cache.contains(&ScriptCacheKey::new( + tx_id, + NetworkUpgrade::Nu5, + std::slice::from_ref(&output), + ))); + assert!( + !cache.contains(&ScriptCacheKey::new( + tx_id, + NetworkUpgrade::Nu5, + std::slice::from_ref(&other_output), + )), + "a verification against one spent-output dataset must not answer another" + ); + + // A script-only difference must also produce a distinct key, so the + // digest cannot degenerate to covering values alone. + let other_script_output = transparent::Output { + value: output.value, + lock_script: transparent::Script::new(&[0x52]), + }; + assert!( + !cache.contains(&ScriptCacheKey::new( + tx_id, + NetworkUpgrade::Nu5, + std::slice::from_ref(&other_script_output), + )), + "a spent-output set differing only in scriptPubKey must not answer another" + ); + } + + #[test] + fn insert_then_contains() { + let cache = VerifiedScripts::new(4); + + assert!(!cache.contains(&key(1, NetworkUpgrade::Nu5))); + cache.insert(key(1, NetworkUpgrade::Nu5)); + assert!(cache.contains(&key(1, NetworkUpgrade::Nu5))); + } + + #[test] + fn distinct_network_upgrades_are_distinct_entries() { + let cache = VerifiedScripts::new(4); + + cache.insert(key(1, NetworkUpgrade::Nu5)); + + assert!( + !cache.contains(&key(1, NetworkUpgrade::Nu6)), + "a verification under one network upgrade must not answer another" + ); + } + + #[test] + fn eviction_is_fifo_and_bounded() { + let cache = VerifiedScripts::new(2); + + cache.insert(key(1, NetworkUpgrade::Nu5)); + cache.insert(key(2, NetworkUpgrade::Nu5)); + cache.insert(key(3, NetworkUpgrade::Nu5)); + + assert!( + !cache.contains(&key(1, NetworkUpgrade::Nu5)), + "the oldest entry must be evicted first" + ); + assert!(cache.contains(&key(2, NetworkUpgrade::Nu5))); + assert!(cache.contains(&key(3, NetworkUpgrade::Nu5))); + + let inner = cache.inner.lock().expect("not poisoned"); + assert_eq!(inner.lookup.len(), 2, "the lookup set must stay bounded"); + assert_eq!( + inner.eviction_order.len(), + 2, + "the eviction queue must stay bounded" + ); + } + + #[test] + fn duplicate_insert_does_not_grow_the_eviction_queue() { + let cache = VerifiedScripts::new(2); + + cache.insert(key(1, NetworkUpgrade::Nu5)); + cache.insert(key(1, NetworkUpgrade::Nu5)); + cache.insert(key(2, NetworkUpgrade::Nu5)); + + // If the duplicate had entered the queue, key 1 would occupy two slots + // and this insert would evict it while `lookup` still answered hits. + cache.insert(key(3, NetworkUpgrade::Nu5)); + + assert!(!cache.contains(&key(1, NetworkUpgrade::Nu5))); + assert!(cache.contains(&key(2, NetworkUpgrade::Nu5))); + assert!(cache.contains(&key(3, NetworkUpgrade::Nu5))); + + let inner = cache.inner.lock().expect("not poisoned"); + assert_eq!( + inner.lookup.len(), + inner.eviction_order.len(), + "the two collections must never drift apart" + ); + } + + #[test] + fn remove_forgets_one_key_and_keeps_the_collections_in_lockstep() { + let cache = VerifiedScripts::new(4); + + cache.insert(key(1, NetworkUpgrade::Nu5)); + cache.insert(key(2, NetworkUpgrade::Nu5)); + cache.remove(&key(1, NetworkUpgrade::Nu5)); + + assert!(!cache.contains(&key(1, NetworkUpgrade::Nu5))); + assert!(cache.contains(&key(2, NetworkUpgrade::Nu5))); + + let inner = cache.inner.lock().expect("not poisoned"); + assert_eq!( + inner.lookup.len(), + inner.eviction_order.len(), + "the two collections must never drift apart" + ); + assert_eq!(inner.eviction_order.len(), 1); + } +} diff --git a/zebra/zebra-consensus/src/transaction/tests.rs b/zebra/zebra-consensus/src/transaction/tests.rs index e4529446..c9c8dac5 100644 --- a/zebra/zebra-consensus/src/transaction/tests.rs +++ b/zebra/zebra-consensus/src/transaction/tests.rs @@ -31,7 +31,7 @@ use zebra_chain::{ insert_fake_orchard_shielded_data, test_transactions, transactions_from_blocks, v5_transactions, }, - zip317, Hash, HashType, JoinSplitData, LockTime, Transaction, + zip317, Hash, HashType, JoinSplitData, LockTime, Transaction, UnminedTx, }, transparent::{self, CoinbaseSpendRestriction}, }; @@ -40,7 +40,7 @@ use zebra_node_services::mempool; use zebra_state::ValidateContextError; use zebra_test::mock_service::MockService; -use crate::{error::TransactionError, transaction::POLL_MEMPOOL_DELAY}; +use crate::{error::TransactionError, transaction::POLL_MEMPOOL_DELAY, BoxError}; use super::{check, BlockRequest, BlockTxVerifier, MempoolRequest, MempoolTxVerifier}; @@ -1109,10 +1109,13 @@ async fn block_verification_does_not_use_mempool_verified_state() { }; // The mempool has already verified this transaction, and it is now submitted twice as a block - // request. Each request runs full block verification independently, which for this transaction - // means fetching the spent UTXO from the state service, so two AwaitUtxo responses are queued - // below. Reuse of the mempool's result — or of the first block request's result — is prevented - // structurally: BlockTxVerifier holds no mempool handle and caches nothing across requests. + // request. Each request fetches the spent UTXO from the state service fresh (spent-UTXO + // lookups are never cached, and BlockTxVerifier holds no mempool handle), so two AwaitUtxo + // responses are queued below. The second request's script checks may be answered by the + // transparent script cache; every state-dependent check still reruns per request. + // + // The mempool attempt itself recorded nothing in the script cache: this transaction spends a + // mempool output, which the mempool-site insert guard excludes. let utxo_clone = utxo.clone(); tokio::spawn(async move { state @@ -3661,6 +3664,791 @@ fn mock_transparent_transfer( (input, output, known_utxos) } +// Transparent script verification cache tests. +// +// The cache is process-global, so these tests must not observe each other's +// entries. Their transaction ids are NOT unique across tests (the fund amount +// only varies the spent UTXO, never the transaction bytes, and several tests +// build byte-identical transactions); isolation comes from the key's +// spent-outputs digest, which differs whenever the UTXO data differs. Tests +// that need a fully unique key vary the fund amount for exactly that reason. + +/// The transparent script cache must key on the witnessed transaction id, not the txid. +/// +/// A v5 txid deliberately excludes authorizing data (scriptSigs), so a valid +/// transaction and a corrupted twin can share a txid while differing in the +/// ZIP-244 authorizing-data digest. A txid-keyed cache would answer the twin +/// from the valid transaction's verification and accept an invalid block: +/// CVE-2026-34377 (GHSA-3vmh-33xr-9cqh), driven here through the production +/// block verifier. +#[tokio::test] +async fn v5_script_cache_rejects_an_authorizing_data_twin() { + let network = Network::new_default_testnet(); + let network_upgrade = NetworkUpgrade::Nu5; + + let block_height = (network_upgrade + .activation_height(&network) + .expect("NU5 activation height is specified") + + 10) + .expect("transaction block height is too large"); + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10_101).expect("invalid value"), + ); + + let transaction = Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::Height(block::Height(0)), + expiry_height: (block_height + 1).expect("expiry height is too large"), + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade, + }; + + // The twin corrupts only authorizing data: its scriptSig no longer satisfies + // the P2SH lock script, while all effecting data stays identical. + let mut twin = transaction.clone(); + let Transaction::V5 { inputs, .. } = &mut twin else { + unreachable!("twin is V5"); + }; + let transparent::Input::PrevOut { unlock_script, .. } = &mut inputs[0] else { + panic!("test input is a PrevOut"); + }; + *unlock_script = transparent::Script::new(&[0]); + + assert_eq!( + transaction.hash(), + twin.hash(), + "the twin must share the valid transaction's txid" + ); + assert_ne!( + transaction.auth_digest(), + twin.auth_digest(), + "the twin must differ in its authorizing-data digest" + ); + + // Verify the valid transaction, which records its scripts in the cache. + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = BlockTxVerifier::new(&network, state_service); + + let result = verifier + .oneshot(BlockRequest { + transaction_hash: transaction.hash(), + transaction: Arc::new(transaction), + known_utxos: Arc::new(known_utxos.clone()), + height: block_height, + time: DateTime::::MAX_UTC, + }) + .await; + assert!( + result.is_ok(), + "the valid transaction must verify: {result:?}" + ); + + // The twin's witnessed id differs, so it must be re-verified and rejected, + // never answered from the valid transaction's cache entry. + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = BlockTxVerifier::new(&network, state_service); + + let result = verifier + .oneshot(BlockRequest { + transaction_hash: twin.hash(), + transaction: Arc::new(twin), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: DateTime::::MAX_UTC, + }) + .await; + assert!( + result.is_err(), + "the authorizing-data twin must fail script verification, not hit the cache" + ); +} + +/// A failed script verification must never be recorded: re-verifying the same +/// failing transaction must fail again, not be answered from the cache. +#[tokio::test] +async fn failed_script_verification_is_not_recorded() { + let network = Network::new_default_testnet(); + let network_upgrade = NetworkUpgrade::Nu5; + + let block_height = (network_upgrade + .activation_height(&network) + .expect("NU5 activation height is specified") + + 10) + .expect("transaction block height is too large"); + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + false, + 0, + Amount::try_from(10_102).expect("invalid value"), + ); + + let transaction = Arc::new(Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::Height(block::Height(0)), + expiry_height: (block_height + 1).expect("expiry height is too large"), + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade, + }); + let known_utxos = Arc::new(known_utxos); + + for attempt in 1..=2 { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = BlockTxVerifier::new(&network, state_service); + + let result = verifier + .oneshot(BlockRequest { + transaction_hash: transaction.hash(), + transaction: transaction.clone(), + known_utxos: known_utxos.clone(), + height: block_height, + time: DateTime::::MAX_UTC, + }) + .await; + assert!( + result.is_err(), + "attempt {attempt}: the failing script must be rejected every time" + ); + } +} + +/// A v4 legacy id hashes the whole serialized transaction, so a scriptSig +/// change produces a different id: an authorizing-data twin cannot share a v4 +/// cache entry by construction. +#[tokio::test] +async fn v4_script_cache_key_binds_the_scriptsigs() { + let network = Network::Mainnet; + + let block_height = (NetworkUpgrade::Canopy + .activation_height(&network) + .expect("Canopy activation height is specified") + + 10) + .expect("transaction block height is too large"); + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10_103).expect("invalid value"), + ); + + let transaction = Transaction::V4 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::Height(block::Height(0)), + expiry_height: (block_height + 1).expect("expiry height is too large"), + joinsplit_data: None, + sapling_shielded_data: None, + }; + + let mut twin = transaction.clone(); + let Transaction::V4 { inputs, .. } = &mut twin else { + unreachable!("twin is V4"); + }; + let transparent::Input::PrevOut { unlock_script, .. } = &mut inputs[0] else { + panic!("test input is a PrevOut"); + }; + *unlock_script = transparent::Script::new(&[0]); + + // Unlike v5, the v4 txid commits to the scriptSigs directly. + assert_ne!( + transaction.hash(), + twin.hash(), + "a v4 scriptSig change must change the legacy transaction id" + ); + + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = BlockTxVerifier::new(&network, state_service); + + let result = verifier + .oneshot(BlockRequest { + transaction_hash: transaction.hash(), + transaction: Arc::new(transaction), + known_utxos: Arc::new(known_utxos.clone()), + height: block_height, + time: DateTime::::MAX_UTC, + }) + .await; + assert!( + result.is_ok(), + "the valid transaction must verify: {result:?}" + ); + + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = BlockTxVerifier::new(&network, state_service); + + let result = verifier + .oneshot(BlockRequest { + transaction_hash: twin.hash(), + transaction: Arc::new(twin), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: DateTime::::MAX_UTC, + }) + .await; + assert!( + result.is_err(), + "the v4 twin has a different id, so it must be re-verified and rejected" + ); +} + +/// A repeat verification is answered from the cache, and removing the entry +/// forces a real re-verification. +/// +/// Observed through the cache's test-only per-key hit counter, which is keyed +/// exactly as production keys are (witnessed id, network upgrade, and the +/// spent-outputs digest), so the count below can only come from this test's +/// transaction. +#[tokio::test] +async fn cached_verification_skips_script_checks_until_removed() { + let network = Network::new_default_testnet(); + let network_upgrade = NetworkUpgrade::Nu5; + + let block_height = (network_upgrade + .activation_height(&network) + .expect("NU5 activation height is specified") + + 10) + .expect("transaction block height is too large"); + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10_104).expect("invalid value"), + ); + + // The spent output, in input order, exactly as the block verifier fetches it. + let spent_outputs: Vec = known_utxos + .values() + .map(|ordered| ordered.utxo.output.clone()) + .collect(); + + let transaction = Arc::new(Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::Height(block::Height(0)), + expiry_height: (block_height + 1).expect("expiry height is too large"), + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade, + }); + + let cache_key = super::script_cache::ScriptCacheKey::new( + transaction.unmined_id(), + network_upgrade, + &spent_outputs, + ); + let known_utxos = Arc::new(known_utxos); + + let verify = || async { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = BlockTxVerifier::new(&network, state_service); + verifier + .oneshot(BlockRequest { + transaction_hash: transaction.hash(), + transaction: transaction.clone(), + known_utxos: known_utxos.clone(), + height: block_height, + time: DateTime::::MAX_UTC, + }) + .await + }; + + // A real verification populates the cache without hitting it. + let result = verify().await; + assert!( + result.is_ok(), + "the valid transaction must verify: {result:?}" + ); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&cache_key), + 0, + "the first verification must run for real" + ); + + // The repeat is answered from the cache. + let result = verify().await; + assert!(result.is_ok(), "the repeat must succeed: {result:?}"); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&cache_key), + 1, + "the repeat verification must be answered from the cache" + ); + + // Removing the entry forces a real re-verification: same successful result, + // no new hit. The removal is key-scoped, so concurrently running tests + // never lose their own entries. + super::script_cache::verified_scripts().remove(&cache_key); + + let result = verify().await; + assert!( + result.is_ok(), + "the re-verification after removal must succeed: {result:?}" + ); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&cache_key), + 1, + "after removal, verification must run for real instead of hitting" + ); +} + +/// Exercises the wild-path shape end to end, deterministically, in bounded time. +/// +/// A pool-consolidation-shaped transaction (1001 standard P2SH inputs) is +/// admitted through the production mempool verifier, then verified through the +/// production block verifier with every spent UTXO served by the state service, +/// the way the same transaction flows through a real node. +/// +/// Determinism and bounds: +/// * No sleeps, no randomness, no network, no disk: the state service is an +/// in-process mock, and all serialization, hashing, and script work is the +/// production code. +/// * The block-path state mock releases `AwaitUtxo` responses only when the +/// expected batch of concurrent lookups is pending, so a regression back to +/// serial per-input lookups deadlocks and fails the timeout instead of +/// passing slowly. +/// * The whole test must finish within 10 seconds, enforced by a timeout. +#[tokio::test] +async fn wild_path_fat_p2sh_mempool_admission_then_block_verification() { + const FAT_TX_INPUTS: usize = 1001; + + tokio::time::timeout(std::time::Duration::from_secs(10), async { + let network = Network::new_default_testnet(); + let network_upgrade = NetworkUpgrade::Nu5; + + let block_height = (network_upgrade + .activation_height(&network) + .expect("NU5 activation height is specified") + + 10) + .expect("transaction block height is too large"); + let fund_height = (block_height - 1).expect("fund height is too small"); + + // The same standard P2SH pattern as `mock_transparent_transfer`: an + // OP_TRUE redeem script spend, standard for the mempool gate and valid + // for the interpreter, with no signatures so the test stays fast. + const OP_TRUE: u8 = 0x51; + let unlock_script = transparent::Script::new(&[0x01, OP_TRUE]); + let mut p2sh_lock_bytes = vec![0xa9, 0x14]; + p2sh_lock_bytes.extend_from_slice(&[ + 0xda, 0x17, 0x45, 0xe9, 0xb5, 0x49, 0xbd, 0x0b, 0xfa, 0x1a, 0x56, 0x99, 0x71, 0xc7, + 0x7e, 0xba, 0x30, 0xcd, 0x5a, 0x4b, + ]); + p2sh_lock_bytes.push(0x87); + let lock_script = transparent::Script::new(&p2sh_lock_bytes); + + let source_hash = Hash([7u8; 32]); + let spent_output = transparent::Output { + value: Amount::try_from(10_000).expect("valid amount"), + lock_script, + }; + + let inputs: Vec = (0..FAT_TX_INPUTS) + .map(|index| transparent::Input::PrevOut { + outpoint: transparent::OutPoint { + hash: source_hash, + // Bounded by FAT_TX_INPUTS, so the cast cannot truncate. + index: index as u32, + }, + unlock_script: unlock_script.clone(), + sequence: 0, + }) + .collect(); + + // One consolidated output; the large remainder is the miner fee, which + // comfortably clears the ZIP-317 conventional fee for this size. + let output = transparent::Output { + value: Amount::try_from(5_000).expect("valid amount"), + lock_script: transparent::Script::new(&[0]), + }; + + let transaction = Transaction::V5 { + inputs, + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height: (block_height + 1).expect("expiry height is too large"), + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade, + }; + let unmined_transaction: UnminedTx = transaction.clone().into(); + let transaction = Arc::new(transaction); + + // Phase 1: mempool admission, with every spent UTXO answered from the + // mocked best chain. + let mempool_state = { + let spent_output = spent_output.clone(); + service_fn(move |request: zebra_state::Request| { + let spent_output = spent_output.clone(); + async move { + match request { + zebra_state::Request::UnspentBestChainUtxo(_) => { + Ok::<_, BoxError>(zebra_state::Response::UnspentBestChainUtxo(Some( + transparent::Utxo::new(spent_output, fund_height, false), + ))) + } + zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_) => { + Ok(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors) + } + other => unreachable!("unexpected mempool state request: {other:?}"), + } + } + }) + }; + + let mempool_verifier = MempoolTxVerifier::new_for_tests(&network, mempool_state); + let mempool_response = mempool_verifier + .oneshot(MempoolRequest { + transaction: unmined_transaction, + height: block_height, + }) + .await + .expect("mempool admission of the consolidation transaction succeeds"); + assert!( + mempool_response.spent_mempool_outpoints.is_empty(), + "all spends come from the mocked best chain" + ); + + // Phase 2: the mined block arrives. Every spent UTXO is served through + // `AwaitUtxo`, behind a batch barrier: responses are released only when + // the expected number of lookups is pending at once, so serial lookups + // deadlock (failing the timeout) instead of passing slowly. + let pending: Arc>>> = + Arc::new(std::sync::Mutex::new(Vec::new())); + let released = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + + let block_state = { + let spent_output = spent_output.clone(); + let pending = pending.clone(); + let released = released.clone(); + service_fn(move |request: zebra_state::Request| { + let spent_output = spent_output.clone(); + let pending = pending.clone(); + let released = released.clone(); + async move { + match request { + zebra_state::Request::AwaitUtxo(_) => { + let (sender, receiver) = tokio::sync::oneshot::channel(); + { + let mut pending = pending.lock().expect("not poisoned"); + pending.push(sender); + + let released_so_far = + released.load(std::sync::atomic::Ordering::SeqCst); + let batch = super::MAX_CONCURRENT_UTXO_LOOKUPS + .min(FAT_TX_INPUTS - released_so_far); + if pending.len() == batch { + released.fetch_add(batch, std::sync::atomic::Ordering::SeqCst); + for waiter in pending.drain(..) { + let _ = waiter.send(()); + } + } + } + receiver + .await + .expect("the barrier always releases a full batch"); + + Ok::<_, BoxError>(zebra_state::Response::Utxo(transparent::Utxo::new( + spent_output, + fund_height, + false, + ))) + } + other => unreachable!("unexpected block state request: {other:?}"), + } + } + }) + }; + + let block_verifier = BlockTxVerifier::new(&network, block_state); + let block_response = block_verifier + .oneshot(BlockRequest { + transaction_hash: transaction.hash(), + transaction: transaction.clone(), + known_utxos: Arc::new(HashMap::new()), + height: block_height, + time: DateTime::::MAX_UTC, + }) + .await + .expect("block verification of the mempool-admitted transaction succeeds"); + + assert_eq!( + released.load(std::sync::atomic::Ordering::SeqCst), + FAT_TX_INPUTS, + "every spent UTXO must be fetched from the state exactly once" + ); + + let expected_fee = (10_000 * FAT_TX_INPUTS as i64 - 5_000) + .try_into() + .expect("valid fee"); + assert_eq!( + block_response.miner_fee, + Some(expected_fee), + "the block verifier must compute the consolidation fee from the fetched UTXOs" + ); + + // The block verification must have been answered from the mempool + // admission's cache entry: this is the mempool-to-block reuse the cache + // exists for. The key is unique to this test's transaction, and entry + // removal is key-scoped, so this count is deterministic under parallel + // test execution. + let spent_outputs = vec![spent_output; FAT_TX_INPUTS]; + let cache_key = super::script_cache::ScriptCacheKey::new( + transaction.unmined_id(), + network_upgrade, + &spent_outputs, + ); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&cache_key), + 1, + "block verification must reuse the mempool admission's script verification" + ); + }) + .await + .expect("the wild path must complete within 10 seconds"); +} + +/// A mempool script failure is never recorded, so it cannot poison block +/// verification of the same transaction. +/// +/// This drives the failure through the mempool call site specifically: the +/// transaction passes every earlier mempool gate (standardness, fees, lock +/// time) and fails only inside the async script checks, so an +/// insert-before-success mutation at the mempool site would record it. The +/// block verification afterwards uses the exact key the mempool attempt would +/// have poisoned; it must re-verify and reject. +#[tokio::test] +async fn mempool_script_failure_cannot_poison_block_verification() { + let network = Network::new_default_testnet(); + let network_upgrade = NetworkUpgrade::Nu5; + + let block_height = (network_upgrade + .activation_height(&network) + .expect("NU5 activation height is specified") + + 10) + .expect("transaction block height is too large"); + let fund_height = (block_height - 1).expect("fake source fund block height is too small"); + + // A standard P2SH spend whose pushed redeem script does not hash to the + // lock script's commitment: push-only and low-sigop, so it passes the + // mempool standardness gate, and fails only in the script interpreter. + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(2_000_105).expect("invalid value"), + ); + let transparent::Input::PrevOut { + outpoint, sequence, .. + } = input + else { + panic!("mock input is a PrevOut"); + }; + let input = transparent::Input::PrevOut { + outpoint, + // A push of OP_2: standard, zero sigops, wrong redeem script hash. + unlock_script: transparent::Script::new(&[0x01, 0x52]), + sequence, + }; + + let transaction = Arc::new(Transaction::V5 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height: (block_height + 1).expect("expiry height is too large"), + sapling_shielded_data: None, + orchard_shielded_data: None, + network_upgrade, + }); + + let spent_outputs: Vec = known_utxos + .values() + .map(|ordered| ordered.utxo.output.clone()) + .collect(); + let cache_key = super::script_cache::ScriptCacheKey::new( + transaction.unmined_id(), + network_upgrade, + &spent_outputs, + ); + + // Mempool admission must fail in the script checks and record nothing. + let spent_output = spent_outputs[0].clone(); + let mempool_state = service_fn(move |request: zebra_state::Request| { + let spent_output = spent_output.clone(); + async move { + match request { + zebra_state::Request::UnspentBestChainUtxo(_) => { + Ok::<_, BoxError>(zebra_state::Response::UnspentBestChainUtxo(Some( + transparent::Utxo::new(spent_output, fund_height, false), + ))) + } + zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_) => { + Ok(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors) + } + other => unreachable!("unexpected mempool state request: {other:?}"), + } + } + }); + let mempool_verifier = MempoolTxVerifier::new_for_tests(&network, mempool_state); + let result = mempool_verifier + .oneshot(MempoolRequest { + transaction: UnminedTx::from(transaction.as_ref().clone()), + height: block_height, + }) + .await; + assert!( + result.is_err(), + "the wrong redeem script must fail mempool script verification" + ); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&cache_key), + 0, + "the failed mempool verification must not be answered from the cache" + ); + + // Block verification of the same transaction, against the same spent + // output data, must re-verify and reject rather than hit a poisoned entry. + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let block_verifier = BlockTxVerifier::new(&network, state_service); + let result = block_verifier + .oneshot(BlockRequest { + transaction_hash: transaction.hash(), + transaction: transaction.clone(), + known_utxos: Arc::new(known_utxos), + height: block_height, + time: DateTime::::MAX_UTC, + }) + .await; + assert!( + result.is_err(), + "block verification must re-run and reject the failing script, never hit a cache entry seeded by a failed mempool attempt" + ); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&cache_key), + 0, + "no phase of a failing transaction's verification may be answered from the cache" + ); +} + +/// Cache entries are scoped to the network upgrade through the production +/// call sites, not just the key type. +/// +/// A v4 transaction carries no branch id, so the same bytes verify at a +/// Canopy height and at an NU5 height. The NU5 verification must not be +/// answered by the Canopy entry; each upgrade earns its own. +#[tokio::test] +async fn script_cache_entries_are_scoped_to_the_network_upgrade() { + let network = Network::Mainnet; + + let canopy_height = (NetworkUpgrade::Canopy + .activation_height(&network) + .expect("Canopy activation height is specified") + + 10) + .expect("transaction block height is too large"); + let nu5_height = (NetworkUpgrade::Nu5 + .activation_height(&network) + .expect("NU5 activation height is specified") + + 10) + .expect("transaction block height is too large"); + let fund_height = (canopy_height - 1).expect("fake source fund block height is too small"); + + let (input, output, known_utxos) = mock_transparent_transfer( + fund_height, + true, + 0, + Amount::try_from(10_106).expect("invalid value"), + ); + + // No lock time and an expiry beyond both heights, so the same transaction + // is valid at Canopy and at NU5. + let transaction = Arc::new(Transaction::V4 { + inputs: vec![input], + outputs: vec![output], + lock_time: LockTime::unlocked(), + expiry_height: (nu5_height + 1).expect("expiry height is too large"), + joinsplit_data: None, + sapling_shielded_data: None, + }); + + let spent_outputs: Vec = known_utxos + .values() + .map(|ordered| ordered.utxo.output.clone()) + .collect(); + let key_for = + |nu| super::script_cache::ScriptCacheKey::new(transaction.unmined_id(), nu, &spent_outputs); + let known_utxos = Arc::new(known_utxos); + + let verify_at = |height| { + let transaction = transaction.clone(); + let known_utxos = known_utxos.clone(); + let network = network.clone(); + async move { + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let verifier = BlockTxVerifier::new(&network, state_service); + verifier + .oneshot(BlockRequest { + transaction_hash: transaction.hash(), + transaction: transaction.clone(), + known_utxos, + height, + time: DateTime::::MAX_UTC, + }) + .await + } + }; + + let result = verify_at(canopy_height).await; + assert!( + result.is_ok(), + "the transaction must verify at the Canopy height: {result:?}" + ); + + // The NU5 verification runs for real: the Canopy entry must not answer it. + let result = verify_at(nu5_height).await; + assert!( + result.is_ok(), + "the transaction must verify at the NU5 height: {result:?}" + ); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&key_for(NetworkUpgrade::Canopy)), + 0, + "an entry earned under Canopy must not answer an NU5 verification" + ); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&key_for(NetworkUpgrade::Nu5)), + 0, + "the first NU5 verification must run for real" + ); + + // Sanity: repeats under the upgrade that earned the entry do hit. + let result = verify_at(canopy_height).await; + assert!(result.is_ok(), "the Canopy repeat must succeed: {result:?}"); + assert_eq!( + super::script_cache::verified_scripts().hits_for(&key_for(NetworkUpgrade::Canopy)), + 1, + "a repeat under the same upgrade must be answered from the cache" + ); +} + /// Create a mock coinbase input with a transparent output. /// /// Create a [`transparent::Input::Coinbase`] at `coinbase_height`.