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" + ); + }); +}