From 31432a078df8193f499d5f3d7ed28cb40b737d16 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Mon, 31 Aug 2026 12:46:47 -0500 Subject: [PATCH] perf(consensus): cache shielded bundle verification Every shielded proof and signature is verified twice: once when the transaction arrives over mempool gossip, and again when it arrives inside a block. Zebra used to avoid the second pass by skipping whole-transaction verification for block transactions already accepted into the mempool, and removed that in #10494 as a security fix without replacing it. The reason that bypass kept breaking is structural: it cached `valid(tx, height, block time, spent outputs)` under a key that did not determine that proposition. Expiry, lock time, the consensus branch id, the Orchard soft-fork gates and the proof-size rule all move with height, and the network upgrade even selects which Orchard circuit verifying key applies, so a stale verdict could answer a different question than the one being asked. Cache the bundle verification itself instead. `verify(bundle, sighash, vk)` is a pure function, so a hit is bit-identical to the computation it replaces. On a hit the transaction verifier still runs end to end at the block's height, with the block's time and the block's spent outputs; only one `oneshot` inside `verify_sapling_bundle` or `queue_orchard_bundle` is short-circuited. That makes key completeness the whole of the safety argument. An entry is keyed by the transaction's unmined ID, the sighash it was verified against, and the shielded pool the bundle sits in: * the ID determines the bundle. A witnessed ID's ZIP 244 authorizing-data digest commits to the proofs and signatures, which the txid alone does not - that was CVE-2026-34377. A v4 transaction's legacy ID is the hash of the whole serialization, which carries the same authorizing data; * the sighash is named separately because it is not always a function of the transaction alone: a v5 or v6 sighash also commits to the amounts and scripts of the spent transparent outputs, and a v4 shielded sighash commits to the block's consensus branch id, which a v4 ID does not; * the pool separates the Orchard and Ironwood bundles of one v6 transaction, which share an ID and a sighash; * the verifying key is committed to structurally: each Orchard circuit version already has its own verifier, so it now also has its own cache, and an entry can only be read back under the key it was written against. Sapling has one key pair for all of history. Only `Ok` results are recorded. A batch error is not per-item evidence, since `Fallback` resolves batch failures by re-verifying each item singly, and an error out of the service need not be a verdict at all - it can report that the batch worker shut down. Recording that as "invalid" would make the node reject a valid block. For the same reason `Cached::poll_ready` does not delegate to the inner service: callers poll before they call, so delegating would surface a dead batch worker's error for an item whose result the cache already holds. The miss path acquires inner readiness inside `call`, which also stops hits from holding `Batch`'s semaphore permits. Items built without a witnessed transaction ID are verified every time. Each cache holds 20,000 keys - several blocks of history plus a full mempool - and reports hits, misses, inserts, evictions and size under `zebra.consensus.cache.*`, labelled by the same `verifier` names as `zebra.consensus.batch.duration_seconds`. Tests: cache-key completeness for both verifiers over real mainnet bundles; the cache's own behaviour (hit, miss, eviction, no reuse across items, no memory of failures, cancellation, readiness) against a stub verifier; a real Sapling bundle rejected when replayed under another branch id; and two end-to-end tests through the transaction verifiers showing that a mempool verification is reused by the block that mines the transaction, that the block one height past expiry is still rejected, and that an authorizing-data twin with an identical txid is fully re-verified and rejected. --- ...bra-consensus-Changed-20260902-120000.yaml | 4 + zebra-consensus/src/primitives.rs | 1 + zebra-consensus/src/primitives/cache.rs | 483 +++++++++++++ zebra-consensus/src/primitives/cache/tests.rs | 140 ++++ zebra-consensus/src/primitives/halo2.rs | 112 ++- zebra-consensus/src/primitives/halo2/tests.rs | 645 +++++++++++++++++- zebra-consensus/src/primitives/sapling.rs | 96 ++- .../src/primitives/sapling/tests.rs | 354 ++++++++++ zebra-consensus/src/transaction.rs | 95 ++- zebra-consensus/src/transaction/tests.rs | 384 +++++++++++ 10 files changed, 2261 insertions(+), 53 deletions(-) create mode 100644 .changes/unreleased/zebra-consensus-Changed-20260902-120000.yaml create mode 100644 zebra-consensus/src/primitives/cache.rs create mode 100644 zebra-consensus/src/primitives/cache/tests.rs create mode 100644 zebra-consensus/src/primitives/sapling/tests.rs diff --git a/.changes/unreleased/zebra-consensus-Changed-20260902-120000.yaml b/.changes/unreleased/zebra-consensus-Changed-20260902-120000.yaml new file mode 100644 index 00000000000..74f64383445 --- /dev/null +++ b/.changes/unreleased/zebra-consensus-Changed-20260902-120000.yaml @@ -0,0 +1,4 @@ +project: zebra-consensus +kind: Changed +body: 'Orchard and Sapling bundle verification results are cached, so a proof or signature verified when its transaction entered the mempool is not verified again when the block that mines it arrives. `halo2::VerifierService` now names a caching wrapper around the batch-and-fallback stack, and each cache reports hits, misses, inserts, evictions and size under `zebra.consensus.cache.*` ([#11380](https://github.com/ZcashFoundation/zebra/pull/11380)).' +time: 2026-09-02T12:00:00.000000000Z diff --git a/zebra-consensus/src/primitives.rs b/zebra-consensus/src/primitives.rs index 822349e18d1..7e48ea801e3 100644 --- a/zebra-consensus/src/primitives.rs +++ b/zebra-consensus/src/primitives.rs @@ -4,6 +4,7 @@ use tokio::sync::oneshot::error::RecvError; use crate::BoxError; +mod cache; pub mod ed25519; pub mod groth16; pub mod halo2; diff --git a/zebra-consensus/src/primitives/cache.rs b/zebra-consensus/src/primitives/cache.rs new file mode 100644 index 00000000000..57f337b026d --- /dev/null +++ b/zebra-consensus/src/primitives/cache.rs @@ -0,0 +1,483 @@ +//! A bounded cache of shielded bundle verifications that have already succeeded. +//! +//! Zebra verifies a transaction's shielded proofs and signatures when the transaction arrives +//! over mempool gossip, and again when it arrives in a block. This service skips the second +//! verification. The Halo2 Orchard and Ironwood verifiers ([`super::halo2`]) and the Sapling +//! verifier ([`super::sapling`]) share it, and key their entries the same way. +//! +//! # Why this is not the mempool bypass +//! +//! Zebra once skipped whole-transaction verification for transactions already in the mempool, and +//! removed it as a security fix (PR #10494). Transaction validity depends on height, block time +//! and spent outputs, and that cache's key named none of them. +//! +//! This cache remembers successful bundle verification by transaction ID, sighash and shielded +//! pool. A hit still runs the whole transaction verifier against the block's height, time and +//! spent outputs, and skips only the proof and signature checks. Each Orchard circuit era has its +//! own cache, which binds an entry to its verifying key (see [`super::halo2::orchard_v5_verifier_for`]); +//! Sapling has one verifying key pair for all of history, so one cache covers it. +//! +//! Only `Ok` results are cached. A batch error is not per-item evidence, because +//! [`Fallback`](tower_fallback::Fallback) re-verifies failures singly, and it may not be a verdict +//! at all — a shut-down batch worker reports the same way. Caching it would reject a valid block. + +use std::{ + collections::{HashSet, VecDeque}, + future, + sync::{Arc, Mutex}, + task::{Context, Poll}, +}; + +use futures::{future::BoxFuture, FutureExt}; +use tower::{Service, ServiceExt}; +use zebra_chain::transaction::UnminedTxId; + +use crate::BoxError; + +#[cfg(test)] +mod tests; + +/// The number of verified-bundle keys retained per cache. +/// +/// Sized to hold several blocks of history plus a full mempool, so that a transaction gossiped +/// well before the block that mines it is still remembered. +/// +/// Each key is a 98-byte transaction ID, sighash and pool tag, held twice — once in the lookup +/// set and once in the eviction queue — so a full cache costs about 5 MiB, and about 20 MiB +/// across the three Orchard circuit eras and Sapling. Only the eras a node actually verifies pay +/// it, because each cache is built with its verifier on first use. +pub(super) const CACHE_CAPACITY: usize = 20_000; + +/// The label naming which verifier's cache a metric belongs to. +/// +/// One cache instance per Orchard circuit era and one for Sapling all report under the same +/// metric names, so every series carries this label. Its values are `halo2_pre_nu6_2`, +/// `halo2_nu6_2`, `halo2_nu6_3_onward` and `groth16_sapling`. Only Sapling's matches the +/// `verifier` label of its `zebra.consensus.batch.duration_seconds` series as well; the Halo2 +/// batch metric reports all three eras as one `halo2` series. +const VERIFIER_LABEL: &str = "verifier"; + +/// Counts verifications answered from a cache. +const CACHE_HIT: &str = "zebra.consensus.cache.hit"; + +/// Counts verifications that reached a cache's inner service. +const CACHE_MISS: &str = "zebra.consensus.cache.miss"; + +/// Counts keys recorded as verified. +const CACHE_INSERT: &str = "zebra.consensus.cache.insert"; + +/// Counts keys dropped to stay within a cache's capacity. +const CACHE_EVICT: &str = "zebra.consensus.cache.evict"; + +/// Reports how many keys a cache currently remembers. +const CACHE_SIZE: &str = "zebra.consensus.cache.size"; + +/// The shielded bundle slot a cache entry was verified for. +/// +/// One v6 transaction has an Orchard bundle, an Ironwood bundle and a Sapling bundle, all under +/// one transaction ID and one sighash, so the key names which one it stands for. The Orchard and +/// Ironwood caches at NU6.3 onward are the same cache, so this tag is what keeps their entries +/// apart; Sapling has its own cache and its tag is defence in depth. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub(super) enum ShieldedPool { + /// The Sapling value pool. + Sapling, + + /// The Orchard value pool. + Orchard, + + /// The Ironwood value pool. + Ironwood, +} + +impl From for ShieldedPool { + fn from(pool: orchard::ValuePool) -> Self { + match pool { + orchard::ValuePool::Orchard => Self::Orchard, + orchard::ValuePool::Ironwood => Self::Ironwood, + } + } +} + +/// A transaction, sighash and shielded pool whose bundle has verified. +/// +/// # Correctness +/// +/// A hit replaces a verification, so the key must determine every input that verification reads: +/// the bundle and the sighash. +/// +/// The transaction ID determines the bundle, in both of the forms it takes: +/// +/// * [`UnminedTxId::Witnessed`] carries a [`WtxId`](zebra_chain::transaction::WtxId), whose +/// txid commits to the transaction's effecting data and whose ZIP 244 authorizing-data digest +/// commits to its proofs and signatures. The txid alone would not: it excludes authorizing +/// data, which is what CVE-2026-34377 exploited. +/// * [`UnminedTxId::Legacy`] is a v1-v4 transaction ID, the hash of the whole serialized +/// transaction, so it commits to the Sapling proofs and signatures directly. V4 transactions +/// have no witnessed ID, and this is why they do not need one here. +/// +/// The sighash is named separately because it is not always a function of the transaction alone. +/// A v5 or v6 sighash also commits to the amounts and `scriptPubKey`s of the spent transparent +/// outputs, which the verification context supplies. A v4 shielded sighash does not — it is +/// computed with no input index, so ZIP 143 and ZIP 243 leave the spent output out — but it does +/// commit to the block's consensus branch id, which a v4 transaction ID does not carry, and which +/// selects the verification a bundle is checked against. +/// +/// The verifying key is absent on purpose. Each Orchard circuit era has its own cache, so an +/// entry is only ever read back under the key it was written against, and Sapling has one key +/// pair for all of history. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub(super) struct CacheKey { + /// The ID of the transaction the bundle was parsed from. + tx_id: UnminedTxId, + + /// The signature digest used to verify the bundle's signatures. + sighash: [u8; 32], + + /// The bundle slot verified for this transaction. + pool: ShieldedPool, +} + +impl CacheKey { + /// Returns the key for `pool`'s bundle in the transaction identified by `tx_id`, verified + /// against `sighash`. + pub(super) fn new(tx_id: UnminedTxId, sighash: [u8; 32], pool: ShieldedPool) -> Self { + Self { + tx_id, + sighash, + pool, + } + } +} + +/// An item whose successful verification can be remembered. +/// +/// Items without a key are verified normally and never cached, which is how a caller that has no +/// transaction identity to offer stays correct. +/// +/// # Correctness +/// +/// An implementer promises that the returned key determines every input the item's verification +/// reads: the bundle, and the sighash it is checked against. [`Cached`] answers any later item +/// with an equal key from the remembered `Ok` without verifying it, so a key that two different +/// bundles can share accepts a bundle this node never checked. An implementer that cannot offer +/// a complete key returns `None`, and the item is verified every time. +pub(super) trait CachedItem { + /// Returns the key this item's successful verification is remembered under, or `None` if the + /// item must always be verified. + /// + /// The key is a pure function of the item: two calls on one item return the same key, and two + /// items with equal keys have identical verification inputs. [`CacheKey`] derives why a + /// transaction ID, a sighash and a shielded pool are enough. + fn cache_key(&self) -> Option; +} + +/// What one [`VerifiedBundles::insert`] did, so the caller can report it after releasing the +/// lock. +/// +/// Metrics are emitted outside the critical section: a labelled `metrics` macro allocates its +/// label set on every call, and this lock is taken by every shielded verification the node runs. +#[derive(Clone, Copy, Debug)] +struct InsertOutcome { + /// Whether this key was new, rather than a concurrent duplicate. + inserted: bool, + + /// How many keys were evicted to make room for it. + evicted: usize, + + /// How many keys the cache holds now. + size: usize, +} + +/// A bounded set of keys for bundles that have already verified successfully. +/// +/// Eviction is first-in-first-out rather than least-recently-used: the working set is the +/// mempool, which turns over in arrival order anyway, and FIFO needs no bookkeeping on the read +/// path. Evicting an entry only costs a re-verification, never correctness. +/// +/// # Correctness +/// +/// `keys` and `insertion_order` hold the same keys. `keys` answers [`Self::contains`], and +/// `insertion_order` chooses which key to drop. Only [`Self::insert`] and `Self::clear` change +/// them, and each changes both, so no caller can leave them holding different keys. +#[derive(Debug)] +struct VerifiedBundles { + /// The keys currently remembered. + keys: HashSet, + + /// The same keys in insertion order, so the oldest can be evicted. + insertion_order: VecDeque, + + /// The maximum number of keys to retain. + capacity: usize, +} + +impl VerifiedBundles { + /// Creates an empty cache that retains at most `capacity` keys. + fn new(capacity: usize) -> Self { + Self { + keys: HashSet::with_capacity(capacity), + insertion_order: VecDeque::with_capacity(capacity), + capacity, + } + } + + /// Returns `true` if `key` has already verified. + fn contains(&self, key: &CacheKey) -> bool { + self.keys.contains(key) + } + + /// Records that `key` has verified, evicting the oldest keys to stay within the capacity. + fn insert(&mut self, key: CacheKey) -> InsertOutcome { + // Concurrent verifications of the same item both miss and both insert. The second is a + // no-op, and must not push a duplicate into the eviction queue. + if !self.keys.insert(key) { + return InsertOutcome { + inserted: false, + evicted: 0, + size: self.keys.len(), + }; + } + + // Evict before pushing, so the queue never has to grow past the capacity it was built + // with. Pushing first would take it to `capacity + 1` and double its allocation for the + // rest of the process. + let mut evicted = 0; + while self.insertion_order.len() >= self.capacity { + // The queue is non-empty whenever its length reaches a capacity of one or more, which + // is what every caller passes. Breaking rather than unwrapping keeps a capacity of + // zero from looping forever. + let Some(oldest) = self.insertion_order.pop_front() else { + break; + }; + self.keys.remove(&oldest); + evicted += 1; + } + + self.insertion_order.push_back(key); + + InsertOutcome { + inserted: true, + evicted, + size: self.keys.len(), + } + } + + /// Forgets every key. + /// + /// Test-only: it lets a test start from a cold cache. Forgetting a key only costs a + /// re-verification, so it is always safe. + #[cfg(test)] + fn clear(&mut self) { + self.keys.clear(); + self.insertion_order.clear(); + } +} + +impl InsertOutcome { + /// Reports this insert under `verifier_name`. + /// + /// [`Cached::call`] calls this after it releases the cache lock. + fn report(self, verifier_name: &'static str) { + if !self.inserted { + return; + } + + metrics::counter!(CACHE_INSERT, VERIFIER_LABEL => verifier_name).increment(1); + + if self.evicted > 0 { + // Cast is safe: at most `capacity` keys are evicted by one insert. + metrics::counter!(CACHE_EVICT, VERIFIER_LABEL => verifier_name) + .increment(self.evicted as u64); + } + + // Cast is safe: the length is bounded by `capacity`, far below f64's exact integer range. + metrics::gauge!(CACHE_SIZE, VERIFIER_LABEL => verifier_name).set(self.size as f64); + } +} + +/// A service that skips inner verification for items whose bundle has already verified. +/// +/// This wraps one verifier's batch-and-fallback stack. The cache is shared between clones, so +/// every handle to a global verifier sees the same set of verified bundles. +/// +/// This type is public only because it appears in existing public verifier signatures. The +/// private `cache` module is not re-exported, and its constructor and accessors are private. +pub struct Cached { + /// The verification service to consult on a miss. + inner: S, + + /// The keys of items that have already verified under this cache's verifying key. + verified: Arc>, + + /// The value this cache reports in the `verifier` label of its metrics. + verifier_name: &'static str, + + /// The keys of the items that reached the inner service, in call order. + /// + /// Test-only. See [`Self::inner_calls_for`]. + #[cfg(test)] + inner_calls: Arc>>, +} + +impl Clone for Cached { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + verified: self.verified.clone(), + verifier_name: self.verifier_name, + #[cfg(test)] + inner_calls: self.inner_calls.clone(), + } + } +} + +impl Cached { + /// Wraps `inner` in a cache that retains at most `capacity` verified-bundle keys and reports + /// its metrics under the `verifier` label `verifier_name`. + pub(super) fn new(inner: S, capacity: usize, verifier_name: &'static str) -> Self { + Self { + inner, + verified: Arc::new(Mutex::new(VerifiedBundles::new(capacity))), + verifier_name, + #[cfg(test)] + inner_calls: Arc::new(Mutex::new(Vec::new())), + } + } + + /// Returns the wrapped verification service. + /// + /// Test-only: it lets a test read back the inner service it installed with + /// [`Self::with_inner`]. + #[cfg(test)] + pub(super) fn inner(&self) -> &S { + &self.inner + } + + /// Returns how many times an item equivalent to `item` has reached the inner service. + /// + /// Test-only. It counts one item rather than all calls because the global verifiers are shared + /// by every test in the process, so a plain call counter would also see verifications a test + /// did not make. Counting one transaction's own item isolates a test from the rest. + /// + /// Readiness failures are not counted: an item that never reached the inner service was never + /// verified by it. + #[cfg(test)] + pub(super) fn inner_calls_for(&self, item: &I) -> usize { + let Some(key) = item.cache_key() else { + return 0; + }; + + self.inner_calls + .lock() + .expect("inner call record mutex should not be poisoned") + .iter() + .filter(|called| **called == key) + .count() + } + + /// Returns a cache sharing this one's remembered keys, but consulting `inner` on a miss. + /// + /// Test-only. It exists so a test can warm the cache through a healthy service and then swap + /// in a broken one, which is how the hit path is exercised in isolation from the inner + /// service. + #[cfg(test)] + pub(super) fn with_inner(&self, inner: T) -> Cached { + Cached { + inner, + verified: self.verified.clone(), + verifier_name: self.verifier_name, + inner_calls: self.inner_calls.clone(), + } + } +} + +impl Service for Cached +where + // `Send + 'static` because a miss moves the item into the boxed future that awaits inner + // readiness — see `poll_ready`. + I: CachedItem + Send + 'static, + S: Service + Clone + Send + 'static, + S::Future: Send + 'static, +{ + type Response = (); + type Error = BoxError; + type Future = BoxFuture<'static, Result<(), BoxError>>; + + /// Always ready. + /// + /// This does not delegate to the inner service, because the item is not known yet, so neither + /// is whether the inner service will be used at all. Delegating would reserve inner capacity + /// for every request, including the hits that never spend it: + /// + /// * [`Batch::poll_ready`](tower_batch_control::Batch) holds a semaphore permit until + /// `Batch::call` consumes it. A hit never calls, so it holds that permit until the handle + /// drops, denying capacity to a genuine miss. + /// * `Batch::poll_ready` also errors once its worker exits. Callers poll before they call, + /// so that error would surface for an item this cache already holds, reporting a verified + /// proof as a verification failure. + /// + /// [`Self::call`] awaits readiness on the miss path instead. The semaphore still bounds + /// concurrent batch requests; only the timing of the wait changes. + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, item: I) -> Self::Future { + // Copied once here, outside `Fallback`, which clones every request eagerly. + let key = item.cache_key(); + + if let Some(key) = key { + if self + .verified + .lock() + .expect("verified bundle cache mutex should not be poisoned") + .contains(&key) + { + metrics::counter!(CACHE_HIT, VERIFIER_LABEL => self.verifier_name).increment(1); + return future::ready(Ok(())).boxed(); + } + } + + metrics::counter!(CACHE_MISS, VERIFIER_LABEL => self.verifier_name).increment(1); + + let verified = self.verified.clone(); + let verifier_name = self.verifier_name; + let mut inner = self.inner.clone(); + + #[cfg(test)] + let inner_calls = self.inner_calls.clone(); + + async move { + // Readiness is acquired here rather than in `poll_ready` so that only misses reserve + // inner capacity. See `poll_ready`. + let result = match inner.ready().await { + Ok(inner) => { + #[cfg(test)] + if let Some(key) = key { + inner_calls + .lock() + .expect("inner call record mutex should not be poisoned") + .push(key); + } + + inner.call(item).await + } + Err(error) => Err(error), + }; + + // Only successes are recorded: see the module docs. + if let (Ok(()), Some(key)) = (&result, key) { + let outcome = verified + .lock() + .expect("verified bundle cache mutex should not be poisoned") + .insert(key); + + outcome.report(verifier_name); + } + + result + } + .boxed() + } +} diff --git a/zebra-consensus/src/primitives/cache/tests.rs b/zebra-consensus/src/primitives/cache/tests.rs new file mode 100644 index 00000000000..59c29d7827d --- /dev/null +++ b/zebra-consensus/src/primitives/cache/tests.rs @@ -0,0 +1,140 @@ +//! Tests for the shared verification cache key and the bounded store behind it. +//! +//! The verifiers pin their own key construction over real bundles. These cover the parts that are +//! the same for all of them: which of the key's three components separate two entries, and the +//! store's capacity and eviction. + +use zebra_chain::{ + serialization::BytesInDisplayOrder, + transaction::{AuthDigest, Hash, UnminedTxId, WtxId}, +}; + +use super::{CacheKey, ShieldedPool, VerifiedBundles}; + +/// Returns a transaction ID with no witness, as a v1-v4 transaction has. +fn legacy_tx_id(tag: u8) -> UnminedTxId { + UnminedTxId::Legacy(Hash::from_bytes_in_display_order(&[tag; 32])) +} + +/// Returns a witnessed transaction ID, as a v5 or v6 transaction has. +fn witnessed_tx_id(txid_tag: u8, auth_digest_tag: u8) -> UnminedTxId { + UnminedTxId::Witnessed(WtxId { + id: Hash::from_bytes_in_display_order(&[txid_tag; 32]), + auth_digest: AuthDigest::from_bytes_in_display_order(&[auth_digest_tag; 32]), + }) +} + +/// The pool separates the bundles a v6 transaction carries under one ID and one sighash. +/// +/// The Orchard and Ironwood caches are one cache from NU6.3 onward, so this is what keeps their +/// entries apart there. +#[test] +fn keys_for_the_same_transaction_differ_by_pool() { + let tx_id = witnessed_tx_id(1, 2); + let sighash = [3; 32]; + + let keys = [ + CacheKey::new(tx_id, sighash, ShieldedPool::Sapling), + CacheKey::new(tx_id, sighash, ShieldedPool::Orchard), + CacheKey::new(tx_id, sighash, ShieldedPool::Ironwood), + ]; + + let unique: std::collections::HashSet<_> = keys.iter().collect(); + assert_eq!( + unique.len(), + keys.len(), + "one transaction's three shielded bundles must not share a cache key" + ); +} + +/// The authorizing-data digest separates two witnessed IDs that share a txid. +/// +/// Under ZIP 244 a v5 transaction's txid excludes its proofs and signatures, so a key that named +/// only the txid would answer both of these with one verification. That is CVE-2026-34377. +#[test] +fn witnessed_keys_differ_by_authorizing_data() { + let sighash = [3; 32]; + + assert_ne!( + CacheKey::new(witnessed_tx_id(1, 2), sighash, ShieldedPool::Orchard), + CacheKey::new(witnessed_tx_id(1, 4), sighash, ShieldedPool::Orchard), + "the same txid with different authorizing data must not share a cache key" + ); +} + +/// The sighash separates two verifications of one transaction's bundle. +/// +/// The sighash is not a function of the transaction alone: the amounts and scripts of the spent +/// transparent outputs enter it, and those come from the verification context. +#[test] +fn keys_differ_by_sighash() { + let tx_id = witnessed_tx_id(1, 2); + + assert_ne!( + CacheKey::new(tx_id, [3; 32], ShieldedPool::Sapling), + CacheKey::new(tx_id, [4; 32], ShieldedPool::Sapling), + "the sighash is an input to verification, so it must be an input to the key" + ); +} + +/// A legacy ID and a witnessed ID never collide, whatever they contain. +#[test] +fn legacy_and_witnessed_keys_differ() { + let sighash = [3; 32]; + + assert_ne!( + CacheKey::new(legacy_tx_id(1), sighash, ShieldedPool::Sapling), + CacheKey::new(witnessed_tx_id(1, 1), sighash, ShieldedPool::Sapling), + "a v4 transaction ID must not collide with a witnessed one" + ); +} + +/// Every Orchard value pool has a distinct tag. +#[test] +fn orchard_value_pools_map_to_distinct_tags() { + assert_eq!( + ShieldedPool::from(orchard::ValuePool::Orchard), + ShieldedPool::Orchard + ); + assert_eq!( + ShieldedPool::from(orchard::ValuePool::Ironwood), + ShieldedPool::Ironwood + ); +} + +/// The lookup set and the eviction queue always hold the same keys. +/// +/// They are two representations of one fact. A path that updated one without the other would +/// either drop a key that `contains` still answers — remembering a bundle for the rest of the +/// process — or grow the queue past the capacity it was built with. +#[test] +fn the_lookup_set_and_the_eviction_queue_hold_the_same_keys() { + let mut verified = VerifiedBundles::new(2); + let keys = [ + CacheKey::new(legacy_tx_id(1), [0; 32], ShieldedPool::Sapling), + CacheKey::new(legacy_tx_id(2), [0; 32], ShieldedPool::Sapling), + CacheKey::new(legacy_tx_id(3), [0; 32], ShieldedPool::Sapling), + ]; + + for key in keys { + verified.insert(key); + assert_eq!( + verified.keys.len(), + verified.insertion_order.len(), + "the lookup set and the eviction queue must hold the same keys" + ); + assert!(verified.keys.len() <= 2, "the capacity bounds the cache"); + } + assert!( + !verified.contains(&keys[0]), + "the oldest key must be evicted" + ); + + let repeated = verified.insert(keys[2]); + assert!(!repeated.inserted, "a concurrent duplicate is not recorded"); + assert_eq!(repeated.evicted, 0, "a duplicate must not evict anything"); + assert_eq!(verified.keys.len(), verified.insertion_order.len()); + + verified.clear(); + assert!(verified.keys.is_empty() && verified.insertion_order.is_empty()); +} diff --git a/zebra-consensus/src/primitives/halo2.rs b/zebra-consensus/src/primitives/halo2.rs index 8b5f12986b9..01e0b7cc00e 100644 --- a/zebra-consensus/src/primitives/halo2.rs +++ b/zebra-consensus/src/primitives/halo2.rs @@ -17,7 +17,10 @@ use orchard::{ }; use rand::thread_rng; use zcash_protocol::value::ZatBalance; -use zebra_chain::{parameters::NetworkUpgrade, transaction::SigHash}; +use zebra_chain::{ + parameters::NetworkUpgrade, + transaction::{SigHash, UnminedTxId, WtxId}, +}; use crate::{error::TransactionError, BoxError}; use thiserror::Error; @@ -26,7 +29,10 @@ use tower::Service; use tower_batch_control::{Batch, BatchControl, RequestWeight}; use tower_fallback::Fallback; -use super::spawn_fifo; +use super::{ + cache::{CacheKey, Cached, CachedItem, ShieldedPool, CACHE_CAPACITY}, + spawn_fifo, +}; #[cfg(test)] mod tests; @@ -110,9 +116,11 @@ lazy_static::lazy_static! { /// A Halo2 verification item, used as the request type of the service. /// -/// An [`Item`] is key-agnostic: it carries only the bundle and sighash. The circuit era's verifying -/// key is supplied by whichever [`Verifier`] processes the item, so an item is always validated -/// against exactly one key and eras are never mixed within a batch. +/// An [`Item`] is key-agnostic: the circuit era's verifying key is supplied by whichever +/// [`Verifier`] processes the item, so an item is always validated against exactly one key and +/// eras are never mixed within a batch. Items built by the transaction verifier also carry a +/// cache key derived from their transaction's [`WtxId`], sighash, and bundle pool, so a bundle +/// verified from the mempool is not verified again when the block that mines it arrives. #[derive(Clone, Debug)] pub struct Item { // `Arc`-wrapped so cloning an `Item` — which `tower-fallback` does eagerly for every request — @@ -120,6 +128,7 @@ pub struct Item { // needs `&Bundle`. bundle: Arc>, sighash: SigHash, + cache_key: Option, } impl RequestWeight for Item { @@ -130,6 +139,10 @@ impl RequestWeight for Item { impl Item { /// Creates a new [`Item`] from a bundle and sighash. + /// + /// Items constructed without their transaction's [`WtxId`] are verified normally but are not + /// cached. The transaction verifier supplies the witnessed transaction ID through a + /// crate-private constructor so its items can reuse successful results. pub fn new( bundle: orchard::bundle::Bundle, sighash: SigHash, @@ -137,6 +150,29 @@ impl Item { Self { bundle: Arc::new(bundle), sighash, + cache_key: None, + } + } + + /// Creates a cacheable item using its already-computed witnessed transaction ID. + /// + /// `wtx_id` must identify the transaction containing `bundle`. The transaction verifier + /// passes the ID it derived from its own request, whose caller must preserve this invariant. + pub(crate) fn new_with_wtx_id( + bundle: orchard::bundle::Bundle, + sighash: SigHash, + wtx_id: WtxId, + ) -> Self { + let pool = ShieldedPool::from(bundle.bundle_version().value_pool()); + + Self { + bundle: Arc::new(bundle), + sighash, + cache_key: Some(CacheKey::new( + UnminedTxId::Witnessed(wtx_id), + sighash.0, + pool, + )), } } @@ -156,12 +192,35 @@ impl Item { } } +impl CachedItem for Item { + /// Returns this item's cache key, if it was constructed with a witnessed transaction ID. + /// + /// [`WtxId`] commits to the transaction's effecting and authorizing data. The sighash + /// additionally commits to the amounts and scripts of spent transparent outputs, which are + /// supplied by the verification context and are not part of the `WtxId`. The pool selects one + /// of the two Orchard-shaped bundles a v6 transaction can carry. The verifying key is absent + /// on purpose: each Orchard circuit era has its own cache, so an entry is only read back + /// under the key it was written against (see [`orchard_v5_verifier_for`]). + /// + /// The txid alone is insufficient because it excludes authorizing data under ZIP 244 + /// (CVE-2026-34377). The pool is also required because both bundles in a v6 transaction share + /// the same [`WtxId`]. + fn cache_key(&self) -> Option { + self.cache_key + } +} + trait QueueBatchVerify { fn queue(&mut self, item: Item) -> Result<(), orchard::bundle::BatchError>; } impl QueueBatchVerify for BatchValidator<'_> { - fn queue(&mut self, Item { bundle, sighash }: Item) -> Result<(), orchard::bundle::BatchError> { + fn queue( + &mut self, + Item { + bundle, sighash, .. + }: Item, + ) -> Result<(), orchard::bundle::BatchError> { self.add_bundle(bundle.as_ref(), sighash.0) } } @@ -221,13 +280,17 @@ impl Service for OrchardFallback { } } +/// The batching-and-fallback stack for one Orchard circuit version, before caching. +type BatchFallbackService = Fallback, OrchardFallback>; + /// The concrete type of a global Halo2 verification service. /// /// Each Orchard circuit version gets its own instance — see [`VERIFIER_PRE_NU6_2`], -/// [`VERIFIER_NU6_2`], and [`VERIFIER_NU6_3_ONWARD`] — so that batches, fallbacks, and verifying -/// keys are fully separated per circuit version. The Orchard verifier routing functions -/// ([`orchard_v5_verifier_for`] / [`orchard_v6_verifier`]) return a borrow of the matching one. -pub type VerifierService = Fallback, OrchardFallback>; +/// [`VERIFIER_NU6_2`], and [`VERIFIER_NU6_3_ONWARD`] — so that batches, fallbacks, verifying +/// keys, and caches are fully separated per circuit version. The Orchard verifier routing +/// functions ([`orchard_v5_verifier_for`] / [`orchard_v6_verifier`]) return a borrow of the +/// matching one. +pub type VerifierService = Cached; /// Builds a global Halo2 verifier that validates every item against `vk`. /// @@ -236,7 +299,19 @@ pub type VerifierService = Fallback, OrchardFallback>; /// passed here, so an item built by this verifier is always checked against exactly one era's key. /// Callers select the correct era's key by which `VERIFYING_KEY_*` they pass (see the two statics /// below); there is no runtime key resolution. -fn batch_verifier(vk: &'static ItemVerifyingKey) -> VerifierService { +/// +/// The stack is wrapped in a [`Cached`] so that a proof verified when its transaction was +/// gossiped into the mempool does not have to be verified again when the block that mines it +/// arrives. Because each circuit version builds its own verifier here, each also gets its own +/// cache, which is what binds a remembered result to the `vk` it was produced under. +/// `verifier_name` is the era's `verifier` metrics label, so each era's cache reports its own hit +/// rate. +fn batch_verifier(vk: &'static ItemVerifyingKey, verifier_name: &'static str) -> VerifierService { + Cached::new(batch_fallback_verifier(vk), CACHE_CAPACITY, verifier_name) +} + +/// Builds the uncached batching-and-fallback stack for `vk`. +fn batch_fallback_verifier(vk: &'static ItemVerifyingKey) -> BatchFallbackService { Fallback::new( Batch::new( Verifier::new(vk), @@ -257,7 +332,7 @@ fn batch_verifier(vk: &'static ItemVerifyingKey) -> VerifierService { /// Note that making a `Service` call requires mutable access to the service, so you should call /// `.clone()` on the global handle to create a local, mutable handle. pub static VERIFIER_PRE_NU6_2: Lazy = - Lazy::new(|| batch_verifier(&VERIFYING_KEY_PRE_NU6_2)); + Lazy::new(|| batch_verifier(&VERIFYING_KEY_PRE_NU6_2, "halo2_pre_nu6_2")); /// Global batch verification context for **NU6.2-until-NU6.3** Halo2 Action proofs. /// @@ -268,7 +343,7 @@ pub static VERIFIER_PRE_NU6_2: Lazy = /// Note that making a `Service` call requires mutable access to the service, so you should call /// `.clone()` on the global handle to create a local, mutable handle. pub static VERIFIER_NU6_2: Lazy = - Lazy::new(|| batch_verifier(&VERIFYING_KEY_NU6_2)); + Lazy::new(|| batch_verifier(&VERIFYING_KEY_NU6_2, "halo2_nu6_2")); /// Global batch verification context for **NU6.3-onward** Halo2 Action proofs. /// @@ -281,7 +356,7 @@ pub static VERIFIER_NU6_2: Lazy = /// Note that making a `Service` call requires mutable access to the service, so you should call /// `.clone()` on the global handle to create a local, mutable handle. pub static VERIFIER_NU6_3_ONWARD: Lazy = - Lazy::new(|| batch_verifier(&VERIFYING_KEY_NU6_3_ONWARD)); + Lazy::new(|| batch_verifier(&VERIFYING_KEY_NU6_3_ONWARD, "halo2_nu6_3_onward")); /// Returns the global Halo2 verifier for the **Orchard-pool** bundle of a **v5** transaction in a /// block at `network_upgrade`. @@ -337,6 +412,15 @@ pub fn orchard_v5_verifier_for(network_upgrade: NetworkUpgrade) -> &'static Veri } } +/// Returns how many times `item` has reached the inner Halo2 verifier of the circuit version +/// `network_upgrade` routes v5 Orchard bundles to. +/// +/// Test-only. See [`Cached::inner_calls_for`]. +#[cfg(test)] +pub(crate) fn inner_calls_for(network_upgrade: NetworkUpgrade, item: &Item) -> usize { + orchard_v5_verifier_for(network_upgrade).inner_calls_for(item) +} + /// Returns the global Halo2 verifier for **v6** Orchard-pool and Ironwood-pool bundles. /// /// v6 Orchard and Ironwood bundles only exist from NU6.3 onward, so they always use the NU6.3 diff --git a/zebra-consensus/src/primitives/halo2/tests.rs b/zebra-consensus/src/primitives/halo2/tests.rs index c1228aed490..189127c9c73 100644 --- a/zebra-consensus/src/primitives/halo2/tests.rs +++ b/zebra-consensus/src/primitives/halo2/tests.rs @@ -14,23 +14,44 @@ //! selects the matching key by block era — in particular a v5 Orchard bundle at NU6.3 routes //! to the NU6.3 cross-address key (the same key as v6 Orchard and Ironwood), not the fixed key. -use std::sync::Arc; +use std::{ + future, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }, + task::{Context, Poll}, + time::Duration, +}; -use orchard::bundle::{Authorized, Bundle}; +use orchard::bundle::{Authorized, Bundle, BundleVersion, Flags}; +use tower::{Service, ServiceExt}; use zcash_protocol::value::ZatBalance; use zebra_chain::{ block::Block, parameters::NetworkUpgrade, - serialization::ZcashDeserializeInto, - transaction::{HashType, SigHash}, + serialization::{BytesInDisplayOrder, ZcashDeserializeInto}, + transaction::{ + arbitrary::with_garbage_orchard_authorization, AuthDigest, Hash, HashType, SigHash, + Transaction, WtxId, + }, transparent, }; +use crate::{error::TransactionError, BoxError}; + use super::{ - orchard_v5_verifier_for, orchard_v6_verifier, Item, VERIFIER_NU6_2, VERIFIER_NU6_3_ONWARD, - VERIFIER_PRE_NU6_2, VERIFYING_KEY_NU6_2, VERIFYING_KEY_PRE_NU6_2, + orchard_v5_verifier_for, orchard_v6_verifier, CacheKey, Cached, CachedItem, Item, + VERIFIER_NU6_2, VERIFIER_NU6_3_ONWARD, VERIFIER_PRE_NU6_2, VERIFYING_KEY_NU6_2, + VERIFYING_KEY_PRE_NU6_2, }; +/// The `verifier` label the test caches report their metrics under. +/// +/// Test caches use their own label so their counts never land in the series the production +/// verifiers report. +const TEST_CACHE_VERIFIER_LABEL: &str = "halo2_test"; + /// Returns one real pre-NU6.2 Orchard bundle and its sighash, extracted from the mainnet test /// blocks. /// @@ -39,6 +60,16 @@ use super::{ /// [`VERIFYING_KEY_PRE_NU6_2`]. Transactions with transparent inputs are skipped because their /// sighash needs the previous outputs they spend, which are not in the test vectors. fn pre_nu6_2_bundle_and_sighash() -> (Bundle, SigHash) { + let (_tx, bundle, sighash) = pre_nu6_2_transaction_bundle_and_sighash(); + (bundle, sighash) +} + +/// Returns one real pre-NU6.2 Orchard transaction, together with its bundle and sighash. +/// +/// The transaction itself is needed by the cache-key tests, which derive the key from its +/// witnessed transaction ID. See [`pre_nu6_2_bundle_and_sighash`] for how it is selected. +fn pre_nu6_2_transaction_bundle_and_sighash( +) -> (Transaction, Bundle, SigHash) { for bytes in zebra_test::vectors::MAINNET_BLOCKS.values() { let block: Block = bytes .zcash_deserialize_into() @@ -58,7 +89,7 @@ fn pre_nu6_2_bundle_and_sighash() -> (Bundle, SigHash) { }; let sighash = sighasher.sighash(HashType::ALL, None); - return (bundle, sighash); + return (tx.as_ref().clone(), bundle, sighash); } } @@ -144,3 +175,603 @@ async fn orchard_verifier_routing_selects_the_correct_key() { "v6 Orchard/Ironwood must route to the NU6.3 verifier" ); } + +// Cache key completeness. +// +// [`Cached`] reuses a previous `Ok` for any item whose key matches, so the key must uniquely +// identify the transaction and its bundle slot. + +/// Returns a deterministic witnessed transaction ID for cache behaviour tests. +fn test_wtx_id(tag: u8) -> WtxId { + WtxId { + id: Hash::from_bytes_in_display_order(&[tag; 32]), + auth_digest: AuthDigest::from_bytes_in_display_order(&[tag.wrapping_add(1); 32]), + } +} + +/// Returns the witnessed transaction ID of `tx`. +fn wtx_id_of(tx: &Transaction) -> WtxId { + WtxId { + id: tx.hash(), + auth_digest: tx + .auth_digest() + .expect("a v5 transaction has an authorizing-data digest"), + } +} + +/// Returns a cacheable verification item. +fn cacheable_item( + bundle: &Bundle, + sighash: SigHash, + wtx_id: WtxId, +) -> Item { + Item::new_with_wtx_id(bundle.clone(), sighash, wtx_id) +} + +/// Returns the cache key for `bundle`'s pool, `sighash`, and `wtx_id`. +fn cache_key(bundle: &Bundle, sighash: SigHash, wtx_id: WtxId) -> CacheKey { + cacheable_item(bundle, sighash, wtx_id) + .cache_key() + .expect("an item constructed with a wtxid is cacheable") +} + +#[test] +fn cache_key_is_deterministic() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let wtx_id = test_wtx_id(1); + + assert_eq!( + cache_key(&bundle, sighash, wtx_id), + cache_key(&bundle, sighash, wtx_id), + "the same wtxid, sighash, and pool must always produce the same key" + ); +} + +#[test] +fn cache_key_commits_to_the_txid_and_authorizing_data() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let original_wtx_id = test_wtx_id(1); + let original = cache_key(&bundle, sighash, original_wtx_id); + + let mut different_txid = original_wtx_id; + different_txid.id.0[0] ^= 1; + assert_ne!( + original, + cache_key(&bundle, sighash, different_txid), + "the transaction ID must be part of the cache key" + ); + + let mut different_authorizing_data = original_wtx_id; + different_authorizing_data.auth_digest.0[0] ^= 1; + assert_ne!( + original, + cache_key(&bundle, sighash, different_authorizing_data), + "the authorizing-data digest must be part of the cache key" + ); +} + +#[test] +fn cache_key_commits_to_the_sighash() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let wtx_id = test_wtx_id(1); + let original = cache_key(&bundle, sighash, wtx_id); + + let mut different_sighash = sighash; + different_sighash.0[0] ^= 1; + + assert_ne!( + original, + cache_key(&bundle, different_sighash, wtx_id), + "different verification contexts must get different cache keys" + ); +} + +/// Corrupting only the authorizing data changes the cache key, even though the txid does not +/// change. +/// +/// That is the shape of CVE-2026-34377: a key derived from the txid alone would collide here, and +/// the corrupt transaction would inherit the valid one's verification. The witnessed ID's ZIP 244 +/// authorizing-data digest is what keeps the two apart. +#[test] +fn cache_key_changes_when_the_authorizing_data_changes() { + let (tx, bundle, sighash) = pre_nu6_2_transaction_bundle_and_sighash(); + let original = cache_key(&bundle, sighash, wtx_id_of(&tx)); + + let garbage = with_garbage_orchard_authorization(tx.clone()); + assert_eq!( + tx.hash(), + garbage.hash(), + "corrupting authorizing data must leave the txid unchanged, or this test proves nothing" + ); + + let garbage_sighasher = garbage + .sighasher(NetworkUpgrade::Nu5, Arc::new(Vec::new())) + .expect("the corrupt transaction still has a sighasher"); + let garbage_bundle = garbage_sighasher + .orchard_bundle() + .expect("the corrupt transaction still has an Orchard bundle"); + let garbage_sighash = garbage_sighasher.sighash(HashType::ALL, None); + + assert_ne!( + original, + cache_key(&garbage_bundle, garbage_sighash, wtx_id_of(&garbage)), + "corrupting the proof and signatures must change the cache key" + ); +} + +/// Returns `bundle`'s parts rebuilt under `flags` and `version`. +fn rebuilt_as( + bundle: &Bundle, + flags: Flags, + version: BundleVersion, +) -> Bundle { + Bundle::try_from_parts( + bundle.actions().clone(), + flags, + *bundle.value_balance(), + *bundle.anchor(), + bundle.authorization().clone(), + version, + ) + .expect("a real mainnet Orchard bundle's parts are representable under the given version") +} + +/// The Orchard and Ironwood pools of one v6 transaction never share a cache key. +/// +/// A v6 transaction gives both bundles the same [`WtxId`]. Both pools also use the NU6.3 circuit +/// and therefore the same cache, so the value-pool tag names which bundle slot earned an entry. +/// +/// The two bundles here are built from identical parts, and with cross-address transfers disabled +/// their flags are identical too. Only the pool tag tells them apart. +#[test] +fn cache_key_distinguishes_the_orchard_and_ironwood_pools() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let wtx_id = test_wtx_id(1); + + // Cross-address transfers are disallowed in the NU6.3 Orchard pool and optional in Ironwood, + // so this is the one flag set both pools can encode. + let orchard = rebuilt_as( + &bundle, + Flags::CROSS_ADDRESS_DISABLED, + BundleVersion::orchard_v3(), + ); + let ironwood = rebuilt_as( + &bundle, + Flags::CROSS_ADDRESS_DISABLED, + BundleVersion::ironwood_v3(), + ); + + assert_eq!( + orchard.flags(), + ironwood.flags(), + "this test is only meaningful if the two pools carry the same flags" + ); + assert_ne!( + cache_key(&orchard, sighash, wtx_id), + cache_key(&ironwood, sighash, wtx_id), + "the Orchard and Ironwood bundles of one transaction must not share a cache key" + ); +} + +// Caching behaviour. + +/// An inner verification service that counts calls and returns a fixed result. +#[derive(Clone)] +struct CountingVerifier { + calls: Arc, + succeeds: bool, +} + +impl CountingVerifier { + fn new(succeeds: bool) -> Self { + Self { + calls: Arc::new(AtomicUsize::new(0)), + succeeds, + } + } + + fn calls(&self) -> usize { + self.calls.load(Ordering::SeqCst) + } +} + +impl Service for CountingVerifier { + type Response = (); + type Error = BoxError; + type Future = future::Ready>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, _item: Item) -> Self::Future { + self.calls.fetch_add(1, Ordering::SeqCst); + + future::ready(if self.succeeds { + Ok(()) + } else { + Err(TransactionError::Halo2VerificationFailed.into()) + }) + } +} + +#[tokio::test] +async fn cache_skips_the_inner_service_for_an_already_verified_item() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let inner = CountingVerifier::new(true); + let mut verifier = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + for _ in 0..3 { + verifier + .ready() + .await + .expect("the cache must become ready") + .call(cacheable_item(&bundle, sighash, test_wtx_id(1))) + .await + .expect("a valid item must verify"); + } + + assert_eq!( + inner.calls(), + 1, + "only the first verification of an item may reach the inner service" + ); +} + +#[tokio::test] +async fn items_without_a_wtxid_are_not_cached() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let inner = CountingVerifier::new(true); + let mut verifier = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + for _ in 0..2 { + verifier + .ready() + .await + .expect("the cache must become ready") + .call(Item::new(bundle.clone(), sighash)) + .await + .expect("an item without a wtxid must still verify"); + } + + assert_eq!( + inner.calls(), + 2, + "an item without a wtxid must never inherit a cached result" + ); +} + +#[tokio::test] +async fn cache_does_not_reuse_a_result_across_items() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + + let inner = CountingVerifier::new(true); + let mut verifier = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + for wtx_id in [test_wtx_id(1), test_wtx_id(2)] { + verifier + .ready() + .await + .expect("the cache must become ready") + .call(cacheable_item(&bundle, sighash, wtx_id)) + .await + .expect("the inner service accepts everything in this test"); + } + + assert_eq!( + inner.calls(), + 2, + "items with different keys must each be verified" + ); +} + +/// A failure is never remembered. +/// +/// A batch error is not per-item evidence — `Fallback` resolves those by re-verifying singly — +/// and an error can report that the batch worker shut down rather than that a proof is invalid. +/// Remembering either as "invalid" would make the node reject valid blocks. +#[tokio::test] +async fn cache_does_not_remember_failures() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let inner = CountingVerifier::new(false); + let mut verifier = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + for _ in 0..3 { + verifier + .ready() + .await + .expect("the cache must become ready") + .call(cacheable_item(&bundle, sighash, test_wtx_id(1))) + .await + .expect_err("the inner service rejects everything in this test"); + } + + assert_eq!( + inner.calls(), + 3, + "a failed verification must be retried, not remembered" + ); +} + +/// Verifies `item` through `verifier`, asserting that it succeeds. +async fn verify_through(verifier: &mut Cached, item: Item) +where + S: Service + Clone + Send + 'static, + S::Future: Send + 'static, +{ + verifier + .ready() + .await + .expect("the cache must become ready") + .call(item) + .await + .expect("the inner service accepts everything in this test"); +} + +#[tokio::test] +async fn cache_evicts_in_insertion_order_and_stays_correct_when_full() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let inner = CountingVerifier::new(true); + let mut verifier = Cached::new(inner.clone(), 2, TEST_CACHE_VERIFIER_LABEL); + + let wtx_ids = [test_wtx_id(1), test_wtx_id(2), test_wtx_id(3)]; + + for wtx_id in &wtx_ids { + verify_through(&mut verifier, cacheable_item(&bundle, sighash, *wtx_id)).await; + } + assert_eq!( + inner.calls(), + 3, + "three distinct items, three verifications" + ); + + // The two most recent are still remembered. + for wtx_id in &wtx_ids[1..] { + verify_through(&mut verifier, cacheable_item(&bundle, sighash, *wtx_id)).await; + } + assert_eq!(inner.calls(), 3, "entries within the capacity must be kept"); + + // The oldest was evicted, so it is verified again rather than silently mis-answered. + verify_through(&mut verifier, cacheable_item(&bundle, sighash, wtx_ids[0])).await; + assert_eq!(inner.calls(), 4, "an evicted entry must be re-verified"); +} + +/// A remembered result is never visible to another circuit version's cache. +/// +/// The cache key deliberately does not name the verifying key. What binds an entry to the key it +/// was produced under is which cache holds it — [`batch_verifier`](super::batch_verifier) builds +/// one per circuit version, and [`orchard_verifier_routing_selects_the_correct_key`] pins the +/// routing. This pins the other half: two cache instances share no state, so an item verified +/// under the pre-NU6.2 insecure key can never be answered from that entry when it is later routed +/// to a different era's verifier. +#[tokio::test] +async fn a_result_cached_under_one_era_is_not_visible_to_another() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let item = cacheable_item(&bundle, sighash, test_wtx_id(9)); + + let inner = CountingVerifier::new(true); + let mut one_era = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + let mut another_era = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + verify_through(&mut one_era, item.clone()).await; + assert_eq!(inner.calls(), 1, "the first verification must be a miss"); + + verify_through(&mut another_era, item).await; + assert_eq!( + inner.calls(), + 2, + "another era's cache must not answer from an entry this one never recorded" + ); +} + +/// Clones of a cache answer from the same set of verified proofs. +/// +/// Production never calls a global verifier directly: the routing functions hand out a `&'static` +/// handle and every request goes through a fresh `.clone()` of it. A cache that lived in the +/// handle rather than behind the shared `Arc` would be empty for every request, so this pins the +/// sharing that makes the cache reachable at all. +#[tokio::test] +async fn cache_is_shared_between_clones() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let item = cacheable_item(&bundle, sighash, test_wtx_id(1)); + + let inner = CountingVerifier::new(true); + let verifier = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + let mut warming_clone = verifier.clone(); + verify_through(&mut warming_clone, item.clone()).await; + assert_eq!(inner.calls(), 1, "the first verification must be a miss"); + + let mut reading_clone = verifier.clone(); + verify_through(&mut reading_clone, item).await; + assert_eq!( + inner.calls(), + 1, + "a clone must answer from the result another clone recorded" + ); +} + +/// An inner service that never returns a result, standing in for a verification in flight. +#[derive(Clone)] +struct PendingVerifier { + calls: Arc, +} + +impl PendingVerifier { + fn new() -> Self { + Self { + calls: Arc::new(AtomicUsize::new(0)), + } + } + + fn calls(&self) -> usize { + self.calls.load(Ordering::SeqCst) + } +} + +impl Service for PendingVerifier { + type Response = (); + type Error = BoxError; + type Future = future::Pending>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, _item: Item) -> Self::Future { + self.calls.fetch_add(1, Ordering::SeqCst); + future::pending() + } +} + +/// A verification that is cancelled before it returns is not remembered. +/// +/// The cache records a key from inside the response future, so dropping that future has to leave +/// the cache untouched. Recording on the way in would remember a proof that was never checked: +/// callers drop these futures routinely, because a block or mempool verification abandons its +/// remaining checks as soon as one of them fails. +#[tokio::test] +async fn cancelling_a_verification_does_not_populate_the_cache() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let item = cacheable_item(&bundle, sighash, test_wtx_id(1)); + + let hanging = PendingVerifier::new(); + let mut verifier = Cached::new(hanging.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + // Start a verification and drop it before the inner service can answer. + let in_flight = verifier + .ready() + .await + .expect("the cache must become ready") + .call(item.clone()); + tokio::time::timeout(Duration::from_millis(50), in_flight) + .await + .expect_err("the inner service never returns, so the verification cannot complete"); + assert_eq!( + hanging.calls(), + 1, + "the cancelled verification must have reached the inner service" + ); + + // Retrying must verify again rather than read an entry the cancelled call never earned. + let mut verifier = verifier.with_inner(CountingVerifier::new(true)); + let counting = verifier.inner().clone(); + verify_through(&mut verifier, item).await; + assert_eq!( + counting.calls(), + 1, + "a cancelled verification must not be remembered as a success" + ); +} + +/// An inner service whose readiness always fails, standing in for a dead batch worker. +/// +/// `Batch::poll_ready` reports an error when its worker has exited, panicked, or closed its +/// channel. `call` panics here because it must never be reached: a service that is not ready must +/// not be called, and the tests below are about what happens *before* that point. +#[derive(Clone)] +struct UnreadyVerifier { + poll_readies: Arc, +} + +impl UnreadyVerifier { + fn new() -> Self { + Self { + poll_readies: Arc::new(AtomicUsize::new(0)), + } + } + + fn poll_readies(&self) -> usize { + self.poll_readies.load(Ordering::SeqCst) + } +} + +impl Service for UnreadyVerifier { + type Response = (); + type Error = BoxError; + type Future = future::Ready>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + self.poll_readies.fetch_add(1, Ordering::SeqCst); + Poll::Ready(Err(BoxError::from("batch worker finished unexpectedly"))) + } + + fn call(&mut self, _item: Item) -> Self::Future { + unreachable!("a service whose poll_ready failed must not be called") + } +} + +/// A cache hit is answered even when the inner service can no longer become ready. +/// +/// `Cached::poll_ready` must not delegate to the inner service. Callers poll readiness before +/// `call`, so delegating would surface a dead batch worker's error for an item whose result the +/// cache already holds — reporting a verified proof as a verification failure, and rejecting a +/// valid block. That is the "an error need not be a verdict" case the module docs are about. +#[tokio::test] +async fn cache_hit_survives_an_inner_service_that_never_becomes_ready() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let item = cacheable_item(&bundle, sighash, test_wtx_id(1)); + + // Warm the cache through a healthy inner service. + let healthy = CountingVerifier::new(true); + let mut verifier = Cached::new(healthy.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + verify_through(&mut verifier, item.clone()).await; + assert_eq!(healthy.calls(), 1, "the first verification must be a miss"); + + // Swap in an inner service that can never become ready, keeping the same cache. + let dead = UnreadyVerifier::new(); + let mut verifier = verifier.with_inner(dead.clone()); + + verifier + .ready() + .await + .expect("the cache must be ready even when the inner service is not") + .call(item) + .await + .expect("a cache hit must be answered from the cache, not from the dead inner service"); + + assert_eq!( + dead.poll_readies(), + 0, + "a hit must not poll the inner service for readiness at all" + ); +} + +/// A miss still propagates an inner readiness failure. +/// +/// Moving readiness off `poll_ready` must not make the cache swallow it: an item that is not in +/// the cache has to reach the inner service, and if that service cannot become ready the request +/// must fail rather than be reported as verified. +#[tokio::test] +async fn cache_miss_propagates_an_inner_readiness_failure() { + let (bundle, sighash) = pre_nu6_2_bundle_and_sighash(); + let dead = UnreadyVerifier::new(); + let mut verifier = Cached::new(dead.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + verifier + .ready() + .await + .expect("the cache itself is always ready") + .call(cacheable_item(&bundle, sighash, test_wtx_id(1))) + .await + .expect_err("a miss must surface the inner service's readiness failure"); + + assert!( + dead.poll_readies() > 0, + "a miss must acquire inner readiness" + ); + + // And the failure must not be remembered as a success. + let mut verifier = verifier.with_inner(CountingVerifier::new(true)); + let counting = verifier.inner().clone(); + verify_through( + &mut verifier, + cacheable_item(&bundle, sighash, test_wtx_id(1)), + ) + .await; + assert_eq!( + counting.calls(), + 1, + "the item must still be verified, so the readiness failure was not recorded as an Ok" + ); +} diff --git a/zebra-consensus/src/primitives/sapling.rs b/zebra-consensus/src/primitives/sapling.rs index 7641b22442b..3bbbf5ed0e3 100644 --- a/zebra-consensus/src/primitives/sapling.rs +++ b/zebra-consensus/src/primitives/sapling.rs @@ -19,10 +19,15 @@ use tower_fallback::Fallback; use sapling_crypto::{bundle::Authorized, BatchValidator, Bundle}; use zcash_proofs::prover::LocalTxProver; use zcash_protocol::value::ZatBalance; -use zebra_chain::transaction::SigHash; +use zebra_chain::transaction::{SigHash, UnminedTxId}; use crate::{error::TransactionError, BoxError}; +use super::cache::{CacheKey, Cached, CachedItem, ShieldedPool, CACHE_CAPACITY}; + +#[cfg(test)] +mod tests; + /// Sapling prover containing spend and output params for the Sapling circuit. /// /// Used to: @@ -39,18 +44,62 @@ pub fn prover() -> &'static LocalTxProver { &SAPLING } +/// A Sapling verification item, used as the request type of the service. +/// +/// Every item carries the cache key its successful verification is remembered under, derived from +/// its transaction's ID and sighash. #[derive(Clone)] pub struct Item { /// The bundle containing the Sapling shielded data to verify. bundle: Bundle, /// The sighash of the transaction that contains the Sapling shielded data. sighash: SigHash, + /// The key this item's successful verification is remembered under. + cache_key: CacheKey, } impl Item { - /// Creates a new [`Item`] from a Sapling bundle and sighash. - pub fn new(bundle: Bundle, sighash: SigHash) -> Self { - Self { bundle, sighash } + /// Creates a new [`Item`] from a Sapling bundle, its sighash, and its transaction's ID. + /// + /// `tx_id` must identify the transaction containing `bundle`, because the cache treats it as + /// determining the bundle — see this type's [`CachedItem`] implementation. The transaction + /// verifier passes the ID it derived from its own request, whose caller must preserve this + /// invariant. + pub fn new( + bundle: Bundle, + sighash: SigHash, + tx_id: UnminedTxId, + ) -> Self { + Self { + bundle, + sighash, + cache_key: CacheKey::new(tx_id, sighash.0, ShieldedPool::Sapling), + } + } +} + +impl CachedItem for Item { + /// Returns the key this item's successful verification is remembered under. + /// + /// The transaction ID commits to the bundle, in both of the forms it takes. A v5 or v6 + /// transaction's [`WtxId`](zebra_chain::transaction::WtxId) pairs a txid over the effecting + /// data with a ZIP 244 authorizing-data digest over the proofs and signatures; a v4 + /// transaction's legacy ID is the hash of its whole serialization, which contains the same + /// authorizing data. So unlike Orchard, which only exists in v5 and v6 transactions, Sapling + /// caches v4 bundles too. + /// + /// The sighash is keyed separately because it is not always a function of the transaction + /// alone. A v5 or v6 sighash also commits to the amounts and scripts of the spent transparent + /// outputs, which the verification context supplies. A v4 shielded sighash does not — it is + /// computed with no input index, so ZIP 143 and ZIP 243 leave the spent output out — but it + /// does commit to the consensus branch id of the block, which the transaction ID of a v4 + /// transaction does not carry. + /// + /// The verifying keys are absent on purpose: Sapling has one spend and one output verifying + /// key for all of history, so unlike Orchard it has no circuit eras to keep apart, and every + /// entry in this cache was written under the same keys it is read back under. + fn cache_key(&self) -> Option { + Some(self.cache_key) } } @@ -210,15 +259,34 @@ pub fn verify_single( .boxed() } +/// The batching-and-fallback stack for Sapling bundle verification, before caching. +type BatchFallbackService = Fallback< + Batch, + ServiceFn BoxFuture<'static, Result<(), Box>>>, +>; + +/// The concrete type of the global Sapling verification service. +pub type VerifierService = Cached; + /// Global batch verification context for Sapling shielded data. -pub static VERIFIER: Lazy< - Fallback< - Batch, - ServiceFn< - fn(Item) -> BoxFuture<'static, Result<(), Box>>, - >, - >, -> = Lazy::new(|| { +/// +/// The stack is wrapped in a [`Cached`] so that a bundle verified when its transaction was +/// gossiped into the mempool does not have to be verified again when the block that mines it +/// arrives. One cache covers all of Sapling: its spend and output verifying keys have never +/// changed, so unlike Orchard there are no circuit eras to keep apart. +pub static VERIFIER: Lazy = + Lazy::new(|| Cached::new(batch_fallback_verifier(), CACHE_CAPACITY, "groth16_sapling")); + +/// Returns how many times `item` has reached the inner Sapling verifier. +/// +/// Test-only. See [`Cached::inner_calls_for`]. +#[cfg(test)] +pub(crate) fn inner_calls_for(item: &Item) -> usize { + VERIFIER.inner_calls_for(item) +} + +/// Builds the uncached batching-and-fallback stack. +fn batch_fallback_verifier() -> BatchFallbackService { Fallback::new( Batch::new( Verifier::default(), @@ -226,6 +294,6 @@ pub static VERIFIER: Lazy< None, super::MAX_BATCH_LATENCY, ), - tower::service_fn(verify_single), + tower::service_fn(verify_single as fn(Item) -> _), ) -}); +} diff --git a/zebra-consensus/src/primitives/sapling/tests.rs b/zebra-consensus/src/primitives/sapling/tests.rs new file mode 100644 index 00000000000..dc794061244 --- /dev/null +++ b/zebra-consensus/src/primitives/sapling/tests.rs @@ -0,0 +1,354 @@ +//! Tests for the Sapling bundle verifier. +//! +//! Most of these are cache-key completeness tests. [`Cached`] reuses a previous `Ok` for any item +//! whose key matches, so a key that misses one of verification's inputs is a consensus bug: it +//! would accept a bundle that was never checked. +//! +//! Sapling keys its entries the same way Halo2 does — transaction ID, sighash and pool — but its +//! bundles also appear in v4 transactions, which have no witnessed ID. A v4 transaction's legacy +//! ID is the hash of its whole serialization, so it covers the proofs and signatures directly, +//! and its sighash is what carries the block's consensus branch id into the key. + +use std::{ + future, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }, + task::{Context, Poll}, +}; + +use tower::{Service, ServiceExt}; +use zebra_chain::{ + block::{Block, Height}, + parameters::{Network, NetworkUpgrade}, + serialization::ZcashDeserializeInto, + transaction::{HashType, Transaction, TxVersion}, + transparent, +}; + +use crate::{error::TransactionError, BoxError}; + +use super::{batch_fallback_verifier, BatchFallbackService, CacheKey, Cached, CachedItem, Item}; + +/// The `verifier` label the test caches report their metrics under. +/// +/// Test caches use their own label so their counts never land in the series the production +/// verifier reports. +const TEST_CACHE_VERIFIER_LABEL: &str = "groth16_sapling_test"; + +/// Returns the mainnet test transactions that carry a Sapling bundle, with the network upgrade +/// each one was mined under. +/// +/// Transactions with transparent inputs are skipped, because their sighash needs the previous +/// outputs they spend, which are not in the test vectors. +/// +/// The upgrade matters for the tests that actually verify: a V4 sighash commits to the consensus +/// branch id, so a real bundle only verifies under the upgrade its block was mined in. The +/// cache-key tests do not need a valid sighash and pass an upgrade of their own. +fn mined_sapling_transactions() -> Vec<(NetworkUpgrade, Transaction)> { + let mut transactions = Vec::new(); + + for (height, bytes) in zebra_test::vectors::MAINNET_BLOCKS.iter() { + let block: Block = bytes + .zcash_deserialize_into() + .expect("hard-coded test vector must deserialize"); + + let nu = NetworkUpgrade::current(&Network::Mainnet, Height(*height)); + + for tx in &block.transactions { + if !tx.inputs().is_empty() { + continue; + } + + if item(tx, nu).is_some() { + transactions.push((nu, tx.as_ref().clone())); + } + } + } + + assert!( + !transactions.is_empty(), + "mainnet test blocks must contain a transparent-input-free Sapling transaction" + ); + + transactions +} + +/// Returns the mainnet test transactions that carry a Sapling bundle. +fn sapling_transactions() -> Vec { + mined_sapling_transactions() + .into_iter() + .map(|(_, tx)| tx) + .collect() +} + +/// Returns one real mainnet Sapling transaction. +fn sapling_transaction() -> Transaction { + sapling_transactions() + .into_iter() + .next() + .expect("there is at least one Sapling transaction") +} + +/// Returns one real mainnet V4 Sapling transaction, with the network upgrade it was mined under. +fn mined_v4_sapling_transaction() -> (NetworkUpgrade, Transaction) { + mined_sapling_transactions() + .into_iter() + .find(|(_, tx)| tx.tx_version() == TxVersion::V4) + .expect("mainnet test blocks must contain a V4 Sapling transaction") +} + +/// Returns the verification item for `tx`'s Sapling bundle under `nu`, if it has one. +fn item(tx: &Transaction, nu: NetworkUpgrade) -> Option { + let all_previous_outputs: Arc> = Arc::new(Vec::new()); + let sighasher = tx.sighasher(nu, all_previous_outputs).ok()?; + let bundle = sighasher.sapling_bundle()?; + + Some(Item::new( + bundle, + sighasher.sighash(HashType::ALL, None), + tx.unmined_id(), + )) +} + +/// Returns the cache key of `tx`'s Sapling bundle under `nu`. +fn cache_key(tx: &Transaction, nu: NetworkUpgrade) -> CacheKey { + item(tx, nu) + .expect("the transaction was selected for having a Sapling bundle") + .cache_key() + .expect("every Sapling item carries a cache key") +} + +#[test] +fn cache_key_is_deterministic() { + let tx = sapling_transaction(); + + assert_eq!( + cache_key(&tx, NetworkUpgrade::Nu5), + cache_key(&tx, NetworkUpgrade::Nu5), + "the same transaction, sighash, and pool must always produce the same key" + ); +} + +#[test] +fn cache_key_distinguishes_different_transactions() { + let transactions = sapling_transactions(); + let first = transactions.first().expect("there is a first transaction"); + let second = transactions + .iter() + .find(|tx| tx.hash() != first.hash()) + .expect("mainnet test blocks must contain two distinct Sapling transactions"); + + assert_ne!( + cache_key(first, NetworkUpgrade::Nu5), + cache_key(second, NetworkUpgrade::Nu5), + "two transactions must not share a cache key" + ); +} + +/// The sighash separates two verifications of one transaction's bundle. +/// +/// One transaction has one transaction ID whatever height it is verified at, but a V4 sighash +/// commits to the consensus branch id of the block that mines it (ZIP 143 and ZIP 243 put it in +/// the BLAKE2b personalization). The sighash is what carries that into the key. +#[test] +fn cache_key_commits_to_the_sighash() { + let (_nu, tx) = mined_v4_sapling_transaction(); + + assert_ne!( + cache_key(&tx, NetworkUpgrade::Canopy), + cache_key(&tx, NetworkUpgrade::Nu5), + "the same bundle verified under two branch ids must not share a cache key" + ); +} + +/// An inner verification service that counts calls and returns a fixed result. +#[derive(Clone)] +struct CountingVerifier { + calls: Arc, + succeeds: bool, +} + +impl CountingVerifier { + fn new(succeeds: bool) -> Self { + Self { + calls: Arc::new(AtomicUsize::new(0)), + succeeds, + } + } + + fn calls(&self) -> usize { + self.calls.load(Ordering::SeqCst) + } +} + +impl Service for CountingVerifier { + type Response = (); + type Error = BoxError; + type Future = future::Ready>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, _item: Item) -> Self::Future { + self.calls.fetch_add(1, Ordering::SeqCst); + + future::ready(if self.succeeds { + Ok(()) + } else { + Err(TransactionError::SaplingVerificationFailed.into()) + }) + } +} + +#[tokio::test] +async fn cache_skips_the_inner_service_for_an_already_verified_bundle() { + let tx = sapling_transaction(); + let inner = CountingVerifier::new(true); + let mut verifier = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + for _ in 0..3 { + verifier + .ready() + .await + .expect("the cache must become ready") + .call(item(&tx, NetworkUpgrade::Nu5).expect("the transaction has a bundle")) + .await + .expect("a valid item must verify"); + } + + assert_eq!( + inner.calls(), + 1, + "only the first verification of a bundle may reach the inner service" + ); +} + +#[tokio::test] +async fn cache_does_not_reuse_a_result_across_bundles() { + let transactions = sapling_transactions(); + let inner = CountingVerifier::new(true); + let mut verifier = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + for tx in transactions.iter().take(2) { + verifier + .ready() + .await + .expect("the cache must become ready") + .call(item(tx, NetworkUpgrade::Nu5).expect("the transaction has a bundle")) + .await + .expect("the inner service accepts everything in this test"); + } + + assert_eq!( + inner.calls(), + 2, + "bundles with different keys must each be verified" + ); +} + +/// A failure is never remembered. +/// +/// A batch error is not per-item evidence — `Fallback` resolves those by re-verifying singly — +/// and an error can report that the batch worker shut down rather than that a proof is invalid. +/// Remembering either as "invalid" would make the node reject valid blocks. +#[tokio::test] +async fn cache_does_not_remember_failures() { + let tx = sapling_transaction(); + let inner = CountingVerifier::new(false); + let mut verifier = Cached::new(inner.clone(), 8, TEST_CACHE_VERIFIER_LABEL); + + for _ in 0..3 { + verifier + .ready() + .await + .expect("the cache must become ready") + .call(item(&tx, NetworkUpgrade::Nu5).expect("the transaction has a bundle")) + .await + .expect_err("the inner service rejects everything in this test"); + } + + assert_eq!( + inner.calls(), + 3, + "a failed verification must not be remembered" + ); +} + +/// The real verification stack, behind a cache private to one test. +/// +/// The production [`VERIFIER`](super::VERIFIER) is a process-wide `Lazy`, so its cache carries +/// whatever every other test in this binary has already verified. This builds the same batch and +/// fallback stack behind a fresh cache, so a test can prove what a cold cache does with real +/// Sapling verification underneath. +fn uncached_verification_behind_a_fresh_cache() -> Cached { + Cached::new(batch_fallback_verifier(), 8, TEST_CACHE_VERIFIER_LABEL) +} + +/// A bundle verified under one network upgrade is not reused under another. +/// +/// The sighash is the only key component that separates these two verifications, so this is the +/// end-to-end evidence for keying on it at all: the same bundle, the same transaction ID, a +/// different branch id, and the remembered `Ok` must not answer it. +/// +/// Real verification runs underneath, so the second call is not merely a miss — the mainnet +/// signatures do not verify against another era's sighash, and the rejection proves the bundle +/// was actually checked rather than answered from the cache. +#[tokio::test(flavor = "multi_thread")] +async fn a_bundle_verified_under_one_upgrade_is_not_reused_under_another() { + let _init_guard = zebra_test::init(); + + let (mined_upgrade, tx) = mined_v4_sapling_transaction(); + + // Any other upgrade that accepts V4 transactions will do: only its branch id matters here. + let other_upgrade = if mined_upgrade == NetworkUpgrade::Nu5 { + NetworkUpgrade::Canopy + } else { + NetworkUpgrade::Nu5 + }; + + let mined_item = item(&tx, mined_upgrade).expect("the transaction has a bundle"); + let other_item = item(&tx, other_upgrade).expect("the transaction has a bundle"); + assert_eq!( + ( + mined_item.bundle.shielded_spends().len(), + mined_item.bundle.shielded_outputs().len(), + *mined_item.bundle.value_balance(), + ), + ( + other_item.bundle.shielded_spends().len(), + other_item.bundle.shielded_outputs().len(), + *other_item.bundle.value_balance(), + ), + "the branch id must not change the parsed bundle, or this test proves nothing" + ); + assert_ne!( + mined_item.cache_key(), + other_item.cache_key(), + "the two sighashes must produce different keys" + ); + + let verifier = uncached_verification_behind_a_fresh_cache(); + + verifier + .clone() + .oneshot(mined_item) + .await + .expect("a real mainnet Sapling bundle must verify under the upgrade that mined it"); + + let error = verifier + .clone() + .oneshot(other_item) + .await + .expect_err("the same bundle must not verify against another upgrade's sighash"); + + let error = error + .downcast::() + .expect("the verifier reports a typed transaction error"); + assert!( + matches!(*error, TransactionError::SaplingVerificationFailed), + "expected SaplingVerificationFailed, got: {error:?}" + ); +} diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index 0b2b5ebeb74..8f3a7afd4f9 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -337,7 +337,8 @@ where tx.as_ref(), nu, script_verifier, - cached_ffi_transaction.clone() + cached_ffi_transaction.clone(), + tx_id, )?; tracing::trace!(?tx_id, "awaiting async checks..."); @@ -532,7 +533,8 @@ where tx.as_ref(), nu, script_verifier, - cached_ffi_transaction.clone() + cached_ffi_transaction.clone(), + tx_id, )?; let check_anchors_and_revealed_nullifiers_query = state @@ -929,6 +931,12 @@ fn check_maturity_height( /// `nu` is the network upgrade active at the transaction's verification height, /// pre-computed by the caller using [`NetworkUpgrade::current`]. /// +/// `tx_id` must be the unmined ID of `tx`: the shielded verifiers key their caches on it, so an +/// ID that does not identify `tx` would let one transaction's bundle be answered from another's +/// verification. A v5 or v6 transaction's witnessed ID is passed down to the Halo2 verifier, +/// which needs the transaction's authorizing-data digest; a legacy ID has none, so those bundles +/// are verified without consulting the Halo2 cache. +/// /// Returns [`TransactionError::WrongVersion`] for V1-V3 transactions, which /// are not supported by any network upgrade Zebra verifies. fn dispatch_version_verification( @@ -936,15 +944,40 @@ fn dispatch_version_verification( nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, + tx_id: UnminedTxId, ) -> Result { + // V5 and V6 transactions always have a witnessed ID, so this is `Some` for the versions that + // reach the Orchard verifiers below. It stays an `Option` rather than an assertion because a + // missing ID only costs a cache miss, and consensus code must not panic on transaction data. + let wtx_id = match tx_id { + UnminedTxId::Witnessed(wtx_id) => Some(wtx_id), + UnminedTxId::Legacy(_) => None, + }; + match tx.tx_version() { TxVersion::Sprout(_) | TxVersion::V3 => { tracing::debug!(?tx, "got transaction with wrong version"); Err(TransactionError::WrongVersion) } - TxVersion::V4 => verify_v4_transaction(tx, nu, script_verifier, cached_ffi_transaction), - TxVersion::V5 => verify_v5_transaction(tx, nu, script_verifier, cached_ffi_transaction), - TxVersion::V6 => verify_v6_transaction(tx, nu, script_verifier, cached_ffi_transaction), + TxVersion::V4 => { + verify_v4_transaction(tx, nu, script_verifier, cached_ffi_transaction, tx_id) + } + TxVersion::V5 => verify_v5_transaction( + tx, + nu, + script_verifier, + cached_ffi_transaction, + tx_id, + wtx_id, + ), + TxVersion::V6 => verify_v6_transaction( + tx, + nu, + script_verifier, + cached_ffi_transaction, + tx_id, + wtx_id, + ), #[allow(unreachable_patterns)] _ => { tracing::debug!(?tx, "got transaction with unsupported version"); @@ -968,12 +1001,14 @@ fn dispatch_version_verification( /// - the `nu` network upgrade active at the transaction's verification height /// - the `script_verifier` to use for verifying the transparent transfers /// - the prepared `cached_ffi_transaction` used by the script verifier +/// - the transaction's `tx_id`, used by the Sapling verification cache #[allow(clippy::unwrap_in_result)] fn verify_v4_transaction( tx: &Transaction, nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, + tx_id: UnminedTxId, ) -> Result { verify_v4_transaction_network_upgrade(tx, nu)?; @@ -986,7 +1021,7 @@ fn verify_v4_transaction( Ok( verify_transparent_inputs_and_outputs(tx, script_verifier, cached_ffi_transaction)? .and(verify_sprout_shielded_data(tx, &sighash)?) - .and(verify_sapling_bundle(sapling_bundle, &sighash)), + .and(verify_sapling_bundle(sapling_bundle, &sighash, tx_id)), ) } @@ -1052,12 +1087,15 @@ fn verify_v4_transaction_network_upgrade( /// - the `nu` network upgrade active at the transaction's verification height /// - the `script_verifier` to use for verifying the transparent transfers /// - the prepared `cached_ffi_transaction` used by the script verifier +/// - the transaction's `tx_id` and `wtx_id`, used by the shielded verification caches #[allow(clippy::unwrap_in_result)] fn verify_v5_transaction( tx: &Transaction, nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, + tx_id: UnminedTxId, + wtx_id: Option, ) -> Result { verify_v5_transaction_network_upgrade(tx, nu)?; @@ -1070,8 +1108,8 @@ fn verify_v5_transaction( 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)), + .and(verify_sapling_bundle(sapling_bundle, &sighash, tx_id)) + .and(verify_orchard_bundle(orchard_bundle, &sighash, nu, wtx_id)), ) } @@ -1127,6 +1165,8 @@ fn verify_v6_transaction( nu: NetworkUpgrade, script_verifier: script::Verifier, cached_ffi_transaction: Arc, + tx_id: UnminedTxId, + wtx_id: Option, ) -> Result { verify_v6_transaction_network_upgrade(tx, nu)?; @@ -1142,9 +1182,9 @@ fn verify_v6_transaction( // 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)), + .and(verify_sapling_bundle(sapling_bundle, &sighash, tx_id)) + .and(verify_orchard_v6_bundle(orchard_bundle, &sighash, wtx_id)) + .and(verify_orchard_v6_bundle(ironwood_bundle, &sighash, wtx_id)), ) } @@ -1281,9 +1321,13 @@ fn verify_sprout_shielded_data( } /// Verifies a transaction's Sapling shielded data. +/// +/// `tx_id` must identify the transaction containing `bundle`; the verifier's cache adds it to the +/// key that lets a mempool verification be reused for the block that mines it. fn verify_sapling_bundle( bundle: Option>, sighash: &SigHash, + tx_id: UnminedTxId, ) -> AsyncChecks { let mut async_checks = AsyncChecks::new(); @@ -1338,7 +1382,7 @@ fn verify_sapling_bundle( async_checks.push( primitives::sapling::VERIFIER .clone() - .oneshot(primitives::sapling::Item::new(bundle, *sighash)), + .oneshot(primitives::sapling::Item::new(bundle, *sighash, tx_id)), ); } @@ -1358,11 +1402,13 @@ fn verify_orchard_bundle( bundle: Option<::orchard::bundle::Bundle<::orchard::bundle::Authorized, ZatBalance>>, sighash: &SigHash, network_upgrade: NetworkUpgrade, + wtx_id: Option, ) -> AsyncChecks { queue_orchard_bundle( || primitives::halo2::orchard_v5_verifier_for(network_upgrade), bundle, sighash, + wtx_id, ) } @@ -1375,8 +1421,14 @@ fn verify_orchard_bundle( fn verify_orchard_v6_bundle( bundle: Option<::orchard::bundle::Bundle<::orchard::bundle::Authorized, ZatBalance>>, sighash: &SigHash, + wtx_id: Option, ) -> AsyncChecks { - queue_orchard_bundle(primitives::halo2::orchard_v6_verifier, bundle, sighash) + queue_orchard_bundle( + primitives::halo2::orchard_v6_verifier, + bundle, + sighash, + wtx_id, + ) } /// Queues an Orchard-shaped bundle's single aggregated Halo2 proof against a verifier. @@ -1395,19 +1447,26 @@ fn verify_orchard_v6_bundle( /// /// `select_verifier` is only invoked when a bundle is present, so a bundle-less transaction /// never forces the (lazily initialized) verifier services. +/// +/// `wtx_id` must identify the transaction containing `bundle`; the verifier's cache keys on it, +/// together with the sighash and the bundle's value pool, so that a bundle verified from the +/// mempool is not verified again in the block that mines it. Without one the item is verified +/// every time. fn queue_orchard_bundle( select_verifier: impl FnOnce() -> &'static primitives::halo2::VerifierService, bundle: Option<::orchard::bundle::Bundle<::orchard::bundle::Authorized, ZatBalance>>, sighash: &SigHash, + wtx_id: Option, ) -> AsyncChecks { let mut async_checks = AsyncChecks::new(); if let Some(bundle) = bundle { - async_checks.push( - select_verifier() - .clone() - .oneshot(primitives::halo2::Item::new(bundle, *sighash)), - ); + let item = match wtx_id { + Some(wtx_id) => primitives::halo2::Item::new_with_wtx_id(bundle, *sighash, wtx_id), + None => primitives::halo2::Item::new(bundle, *sighash), + }; + + async_checks.push(select_verifier().clone().oneshot(item)); } async_checks diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 0772e32b2eb..1905aea7039 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -5018,3 +5018,387 @@ fn script_sig_args_expected_values() { .expect("1-of-1 multisig should be a standard script kind"); assert_eq!(check::script_sig_args_expected(&ms_kind), Some(2)); } + +// The shielded verification caches, exercised end to end through the transaction verifiers. +// +// The unit tests in `primitives::halo2::tests` and `primitives::sapling::tests` pin the cache's +// own behaviour against a stub inner service. These pin what the node actually gets from it: the +// proof and signature verification a mempool transaction paid for is reused by the block that +// mines it, and nothing else is. + +/// The mock state service the cache tests verify against. +type CacheTestState = MockService< + zebra_state::Request, + zebra_state::Response, + zebra_test::mock_service::PropTestAssertion, + zebra_state::BoxError, +>; + +/// Returns one real mainnet transaction that can be verified from the mempool and then in a block. +/// +/// The predicates are what the two verifications need: +/// +/// * an Orchard bundle, and no Sapling bundle, so the Halo2 verifier is the only shielded +/// verifier the transaction reaches; +/// * no transparent inputs, so neither verification queries the state for UTXOs, and the +/// sighash can be computed over an empty set of previous outputs; +/// * no time-based lock time, so the mempool verification makes no median-time-past query; and +/// * a fee at or above the ZIP 317 conventional fee, so the mempool verification is not +/// rejected as under-paying before it reaches the verifier. +fn cacheable_mainnet_orchard_transaction() -> Transaction { + zebra_test::vectors::MAINNET_BLOCKS + .values() + .flat_map(|bytes| { + let block: Block = bytes + .zcash_deserialize_into() + .expect("hard-coded test vector must deserialize"); + block.transactions.clone() + }) + .find(|tx| { + tx.has_orchard_shielded_data() + && tx.inputs().is_empty() + && !tx.has_sapling_shielded_data() + && !tx.lock_time_is_time() + && tx + .value_balance(&HashMap::new()) + .ok() + .and_then(|balance| balance.remaining_transaction_value().ok()) + .is_some_and(|fee| fee >= zip317::conventional_fee(tx)) + }) + .map(|tx| tx.as_ref().clone()) + .expect("the mainnet test blocks must contain a fee-paying Orchard-only transaction") +} + +/// Returns the Orchard verification item the transaction verifier builds for `tx` at +/// `network_upgrade`. +/// +/// Mirrors [`verify_v5_transaction`](super::verify_v5_transaction): the bundle and the sighash +/// come from one sighasher over an empty set of previous outputs, which is correct only because +/// [`cacheable_mainnet_orchard_transaction`] has no transparent inputs. +fn orchard_item( + tx: &Transaction, + network_upgrade: NetworkUpgrade, +) -> crate::primitives::halo2::Item { + let sighasher = tx + .sighasher(network_upgrade, Arc::new(Vec::new())) + .expect("a mainnet Orchard transaction has a sighasher at its own network upgrade"); + let bundle = sighasher + .orchard_bundle() + .expect("the transaction was selected for having an Orchard bundle"); + + crate::primitives::halo2::Item::new_with_wtx_id( + bundle, + sighasher.sighash(HashType::ALL, None), + zebra_chain::transaction::WtxId { + id: tx.hash(), + auth_digest: tx + .auth_digest() + .expect("a v5 transaction has an authorizing-data digest"), + }, + ) +} + +/// Answers the one state query a shielded-only mempool verification makes. +/// +/// Block requests make none: this transaction has no transparent inputs to look up, and the +/// nullifier and anchor check is a mempool-only query. +fn respond_to_nullifier_and_anchor_check(state: &CacheTestState) { + let mut state = state.clone(); + + tokio::spawn(async move { + state + .expect_request_that(|req| { + matches!( + req, + zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_) + ) + }) + .await + .expect("a mempool verification must check nullifiers and anchors") + .respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors); + }); +} + +/// Returns the [`TransactionError`] behind a buffered block verifier's boxed error. +fn transaction_error(error: crate::BoxError) -> TransactionError { + *error + .downcast::() + .expect("the block verifier reports a typed transaction error") +} + +/// Returns a block request that mines `tx` at `height`. +fn cache_test_block_request(tx: &Transaction, height: Height) -> BlockRequest { + BlockRequest { + transaction_hash: tx.hash(), + transaction: Arc::new(tx.clone()), + known_utxos: Arc::new(HashMap::new()), + height, + time: Utc::now(), + } +} + +/// The Halo2 proof cache, exercised end to end through the transaction verifiers. +/// +/// This is one test rather than three because all three claims need the same transaction and a +/// cold cache for it. The Halo2 verifiers are process-wide `Lazy` statics, so only one test can +/// ever see that transaction's cache entry cold. +/// +/// It runs on [`zebra_test::MULTI_THREADED_RUNTIME`] because it reaches a global verifier. +/// `tower-batch-control` spawns that verifier's batch worker on whichever runtime first touches +/// it, so a per-test runtime would leave the worker cancelled for every test that ran afterwards. +/// +/// The three claims, in order: +/// +/// 1. a mempool verification records the proof, and the block that mines the same transaction +/// is answered from that record instead of verifying the proof again; +/// 2. the record does not carry the transaction past the height-dependent checks: the block one +/// height past its expiry is still rejected. That is the mempool bypass Zebra removed as a +/// security fix in PR #10494, and the reason this cache holds a proof rather than a verdict +/// on a transaction; +/// 3. a transaction whose authorizing data was replaced — the same txid, a different +/// authorizing-data digest, which is the shape of CVE-2026-34377 — never inherits the +/// record. +#[test] +fn the_halo2_cache_is_reused_only_for_the_transaction_that_earned_it() { + let _init_guard = zebra_test::init(); + + zebra_test::MULTI_THREADED_RUNTIME.block_on(async { + let state: CacheTestState = MockService::build().for_prop_tests(); + let mempool_verifier = MempoolTxVerifier::new_for_tests(&Network::Mainnet, state.clone()); + let block_verifier = Buffer::new(BlockTxVerifier::new(&Network::Mainnet, state.clone()), 1); + + let tx = cacheable_mainnet_orchard_transaction(); + let expiry_height = tx + .expiry_height() + .expect("a V5 transaction has an expiry height"); + let network_upgrade = NetworkUpgrade::current(&Network::Mainnet, expiry_height); + let item = orchard_item(&tx, network_upgrade); + assert_eq!( + crate::primitives::halo2::inner_calls_for(network_upgrade, &item), + 0, + "this transaction's bundle must not have been verified before this test" + ); + + // 1. The mempool verification is the only one that reaches the Halo2 verifier. + respond_to_nullifier_and_anchor_check(&state); + mempool_verifier + .oneshot(MempoolRequest { + transaction: Arc::new(tx.clone()).into(), + height: expiry_height, + }) + .await + .expect("a real mainnet Orchard transaction must verify at its expiry height"); + + assert_eq!( + crate::primitives::halo2::inner_calls_for(network_upgrade, &item), + 1, + "the mempool verification must reach the inner Halo2 verifier" + ); + + block_verifier + .clone() + .oneshot(cache_test_block_request(&tx, expiry_height)) + .await + .expect("the same transaction must verify in a block"); + + assert_eq!( + crate::primitives::halo2::inner_calls_for(network_upgrade, &item), + 1, + "the block verification must be answered from the cache" + ); + + // 2. The cached proof does not carry the transaction past the expiry check. + let too_late = + (expiry_height + 1).expect("a mainnet expiry height is far below the maximum"); + let error = block_verifier + .clone() + .oneshot(cache_test_block_request(&tx, too_late)) + .await + .expect_err("a transaction mined past its expiry height must be rejected"); + + assert_eq!( + transaction_error(error), + TransactionError::ExpiredTransaction { + expiry_height, + block_height: too_late, + transaction_hash: tx.hash(), + }, + "the rejection must be the expiry rule, not some other failure" + ); + + // 3. The authorizing-data twin does not inherit the cached result. + let twin = with_garbage_orchard_authorization(tx.clone()); + assert_eq!( + tx.hash(), + twin.hash(), + "replacing authorizing data must leave the txid unchanged, or this test proves nothing" + ); + + let twin_item = orchard_item(&twin, network_upgrade); + // A collision here would already be the failure: the twin would inherit the valid + // transaction's result instead of being verified. + assert_eq!( + crate::primitives::halo2::inner_calls_for(network_upgrade, &twin_item), + 0, + "the authorizing-data twin must get a different cache key" + ); + + let error = block_verifier + .clone() + .oneshot(cache_test_block_request(&twin, expiry_height)) + .await + .expect_err("a transaction with replaced authorizing data must be rejected"); + + assert_eq!( + transaction_error(error), + TransactionError::Halo2VerificationFailed, + "the twin must fail Orchard verification" + ); + + assert_eq!( + crate::primitives::halo2::inner_calls_for(network_upgrade, &twin_item), + 1, + "the twin must reach the inner Halo2 verifier" + ); + }); +} + +/// Returns one real mainnet Sapling transaction that can be verified from the mempool and then in +/// a block, with the network upgrade it was mined under. +/// +/// The predicates are the Sapling counterparts of +/// [`cacheable_mainnet_orchard_transaction`]'s: a Sapling bundle and no Orchard bundle, no +/// transparent inputs, no time-based lock time, and a fee at or above the ZIP 317 conventional +/// fee. +fn cacheable_mainnet_sapling_transaction() -> (NetworkUpgrade, Transaction) { + zebra_test::vectors::MAINNET_BLOCKS + .iter() + .flat_map(|(height, bytes)| { + let block: Block = bytes + .zcash_deserialize_into() + .expect("hard-coded test vector must deserialize"); + let nu = NetworkUpgrade::current(&Network::Mainnet, Height(*height)); + + block + .transactions + .clone() + .into_iter() + .map(move |tx| (nu, tx)) + }) + .find(|(_nu, tx)| { + tx.has_sapling_shielded_data() + && !tx.has_orchard_shielded_data() + && tx.inputs().is_empty() + && !tx.lock_time_is_time() + && tx + .value_balance(&HashMap::new()) + .ok() + .and_then(|balance| balance.remaining_transaction_value().ok()) + .is_some_and(|fee| fee >= zip317::conventional_fee(tx)) + }) + .map(|(nu, tx)| (nu, tx.as_ref().clone())) + .expect("the mainnet test blocks must contain a fee-paying Sapling-only transaction") +} + +/// Returns the Sapling verification item the transaction verifier builds for `tx` at +/// `network_upgrade`. +fn sapling_item( + tx: &Transaction, + network_upgrade: NetworkUpgrade, +) -> crate::primitives::sapling::Item { + let sighasher = tx + .sighasher(network_upgrade, Arc::new(Vec::new())) + .expect("a mainnet Sapling transaction has a sighasher at its own network upgrade"); + let bundle = sighasher + .sapling_bundle() + .expect("the transaction was selected for having a Sapling bundle"); + + crate::primitives::sapling::Item::new( + bundle, + sighasher.sighash(HashType::ALL, None), + tx.unmined_id(), + ) +} + +/// The Sapling bundle cache, exercised end to end through the transaction verifiers. +/// +/// The Sapling counterpart of +/// [`the_halo2_cache_is_reused_only_for_the_transaction_that_earned_it`], and the same reasons +/// apply for it being one test on the shared runtime. Sapling bundles also appear in v4 +/// transactions, whose legacy transaction ID is the hash of the whole serialization, so this also +/// covers the key form Orchard never sees. +/// +/// The two claims, in order: +/// +/// 1. a mempool verification records the bundle, and the block that mines the same transaction +/// is answered from that record instead of verifying the proofs and signatures again; +/// 2. the record does not carry the transaction past the height-dependent checks: the block one +/// height past its expiry is still rejected. +#[test] +fn the_sapling_cache_is_reused_only_for_the_transaction_that_earned_it() { + let _init_guard = zebra_test::init(); + + zebra_test::MULTI_THREADED_RUNTIME.block_on(async { + let state: CacheTestState = MockService::build().for_prop_tests(); + let mempool_verifier = MempoolTxVerifier::new_for_tests(&Network::Mainnet, state.clone()); + let block_verifier = Buffer::new(BlockTxVerifier::new(&Network::Mainnet, state.clone()), 1); + + let (network_upgrade, tx) = cacheable_mainnet_sapling_transaction(); + let expiry_height = tx + .expiry_height() + .expect("a V4 or V5 transaction has an expiry height"); + let item = sapling_item(&tx, network_upgrade); + assert_eq!( + crate::primitives::sapling::inner_calls_for(&item), + 0, + "this transaction's bundle must not have been verified before this test" + ); + + // 1. The mempool verification is the only one that reaches the Sapling verifier. + respond_to_nullifier_and_anchor_check(&state); + mempool_verifier + .oneshot(MempoolRequest { + transaction: Arc::new(tx.clone()).into(), + height: expiry_height, + }) + .await + .expect("a real mainnet Sapling transaction must verify at its expiry height"); + + assert_eq!( + crate::primitives::sapling::inner_calls_for(&item), + 1, + "the mempool verification must reach the inner Sapling verifier" + ); + + block_verifier + .clone() + .oneshot(cache_test_block_request(&tx, expiry_height)) + .await + .expect("the same transaction must verify in a block"); + + assert_eq!( + crate::primitives::sapling::inner_calls_for(&item), + 1, + "the block verification must be answered from the cache" + ); + + // 2. The cached verification does not carry the transaction past the expiry check. + let too_late = + (expiry_height + 1).expect("a mainnet expiry height is far below the maximum"); + let error = block_verifier + .clone() + .oneshot(cache_test_block_request(&tx, too_late)) + .await + .expect_err("a transaction mined past its expiry height must be rejected"); + + assert_eq!( + transaction_error(error), + TransactionError::ExpiredTransaction { + expiry_height, + block_height: too_late, + transaction_hash: tx.hash(), + }, + "the rejection must be the expiry rule, not some other failure" + ); + }); +}