diff --git a/Cargo.lock b/Cargo.lock index 229dc80a..bcda7283 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12408,7 +12408,7 @@ dependencies = [ [[package]] name = "pallet-shielded-pool" -version = "0.22.0" +version = "0.23.0" dependencies = [ "ark-bn254", "ark-ff 0.5.0", @@ -12984,7 +12984,7 @@ dependencies = [ [[package]] name = "pallet-zk-verifier" -version = "0.15.0" +version = "0.16.0" dependencies = [ "ark-bn254", "ark-ec 0.5.0", diff --git a/client/rpc-v2/src/privacy.rs b/client/rpc-v2/src/privacy.rs index 56f3ac38..74ea4885 100644 --- a/client/rpc-v2/src/privacy.rs +++ b/client/rpc-v2/src/privacy.rs @@ -1,10 +1,16 @@ -//! Privacy RPC — Orbinum shielded pool (4 endpoints, 1 file) +//! Privacy RPC — Orbinum shielded pool. //! //! Endpoints: -//! - `privacy_getMerkleRoot` — current Merkle root as hex -//! - `privacy_getMerkleProof` — sibling path for a leaf index -//! - `privacy_getNullifierStatus` — whether a nullifier has been spent -//! - `privacy_getPoolStats` — aggregate pool statistics +//! - `privacy_getMerkleRoot` — current Merkle root as hex +//! - `privacy_getMerkleProof` — sibling path for a leaf index +//! - `privacy_getMerkleProofByCommitment` — sibling path for a commitment +//! - `privacy_getNullifierStatus` — whether a nullifier has been spent +//! - `privacy_getPoolStats` — aggregate pool statistics +//! +//! The two proof endpoints run on blocking threads and at most +//! `MAX_CONCURRENT_PROOFS` at once: a sealed tree's path is rebuilt from +//! leaves, and inline on the connection task a few of them would stall every +//! other RPC. use jsonrpsee::{ core::RpcResult, @@ -37,7 +43,13 @@ use sp_blockchain::HeaderBackend; use sp_core::{storage::StorageKey, H256}; use sp_crypto_hashing::{blake2_128, twox_128}; use sp_runtime::traits::Block as BlockT; -use std::{marker::PhantomData, sync::Arc}; +use std::{ + marker::PhantomData, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }, +}; // ============================================================================ // Storage key helpers @@ -123,14 +135,15 @@ pub trait PrivacyApi { fn get_merkle_root(&self) -> RpcResult; /// Returns a Merkle sibling-path proof for the leaf at `leaf_index`. - #[method(name = "privacy_getMerkleProof")] + #[method(name = "privacy_getMerkleProof", blocking)] fn get_merkle_proof(&self, leaf_index: u32) -> RpcResult; /// Returns a Merkle sibling-path proof for the given commitment (`0x`-prefixed hex, 32 bytes). - /// Resolves the leaf index via the on-chain reverse index (O(1)) and reads the - /// stored sibling path (O(depth)). Root and path come from the same block. - /// Returns an error if the commitment is not found in the tree. - #[method(name = "privacy_getMerkleProofByCommitment")] + /// Resolves the leaf index via the on-chain reverse index (O(1)), then reads + /// the stored siblings, rebuilding a sealed tree's pruned ones from its + /// leaves. Root and path come from the same block. Returns an error if the + /// commitment is not found in the tree. + #[method(name = "privacy_getMerkleProofByCommitment", blocking)] fn get_merkle_proof_by_commitment(&self, commitment: String) -> RpcResult; /// Returns whether the nullifier (`0x`-prefixed hex, 32 bytes) has been spent. @@ -146,9 +159,15 @@ pub trait PrivacyApi { // RPC server // ============================================================================ +/// Merkle proofs computed at once, at most. A sealed tree's path rebuilds +/// pruned siblings from its leaves, so a proof costs real CPU; past this, a +/// request is refused as busy rather than queued behind the others. +const MAX_CONCURRENT_PROOFS: usize = 4; + /// Privacy RPC server for the Orbinum shielded pool. pub struct PrivacyRpc { client: Arc, + proofs: ProofGate, _ph: PhantomData<(B, BE)>, } @@ -156,11 +175,46 @@ impl PrivacyRpc { pub fn new(client: Arc) -> Self { Self { client, + proofs: ProofGate::new(MAX_CONCURRENT_PROOFS), _ph: PhantomData, } } } +/// Caps how many proofs run at once. A permit is held for the duration of one +/// request and released when dropped. +struct ProofGate { + in_flight: AtomicUsize, + max: usize, +} + +struct ProofPermit<'a>(&'a AtomicUsize); + +impl ProofGate { + fn new(max: usize) -> Self { + Self { + in_flight: AtomicUsize::new(0), + max, + } + } + + /// A permit, or `None` when `max` proofs are already running. + fn enter(&self) -> Option> { + self.in_flight + .fetch_update(Ordering::AcqRel, Ordering::Acquire, |n| { + (n < self.max).then_some(n + 1) + }) + .ok() + .map(|_| ProofPermit(&self.in_flight)) + } +} + +impl Drop for ProofPermit<'_> { + fn drop(&mut self) { + self.0.fetch_sub(1, Ordering::AcqRel); + } +} + // ============================================================================ // Error helpers // ============================================================================ @@ -189,6 +243,14 @@ fn pool_not_initialized() -> ErrorObject<'static> { ) } +fn busy() -> ErrorObject<'static> { + ErrorObject::owned( + ErrorCode::ServerIsBusy.code(), + "Too many Merkle proofs in flight, try again later", + None::<()>, + ) +} + fn pool_is_empty() -> ErrorObject<'static> { ErrorObject::owned( ErrorCode::InternalError.code(), @@ -233,6 +295,7 @@ where } fn get_merkle_proof(&self, leaf_index: u32) -> RpcResult { + let _permit = self.proofs.enter().ok_or_else(busy)?; // Both runtime-API calls execute at the same block, so root and path // can never mismatch. let best_hash = self.client.info().best_hash; @@ -280,6 +343,7 @@ where } fn get_merkle_proof_by_commitment(&self, commitment: String) -> RpcResult { + let _permit = self.proofs.enter().ok_or_else(busy)?; let best_hash = self.client.info().best_hash; let api = self.client.runtime_api(); @@ -430,6 +494,31 @@ mod tests { use super::*; use sp_crypto_hashing::twox_128; + // ------------------------------------------------------------------------- + // Proof concurrency gate + // ------------------------------------------------------------------------- + + mod proof_gate { + use super::*; + + #[test] + fn refuses_past_the_cap_and_frees_a_slot_on_drop() { + let gate = ProofGate::new(2); + let a = gate.enter().expect("first permit"); + let b = gate.enter().expect("second permit"); + assert!(gate.enter().is_none(), "a third proof must be refused"); + drop(a); + let c = gate.enter().expect("a slot frees when a permit drops"); + drop((b, c)); + assert_eq!(gate.in_flight.load(Ordering::Acquire), 0); + } + + #[test] + fn the_busy_error_is_the_standard_server_busy_code() { + assert_eq!(busy().code(), ErrorCode::ServerIsBusy.code()); + } + } + // ------------------------------------------------------------------------- // Storage key helpers // ------------------------------------------------------------------------- diff --git a/frame/shielded-pool/CHANGELOG.md b/frame/shielded-pool/CHANGELOG.md index a802664c..cff5afcb 100644 --- a/frame/shielded-pool/CHANGELOG.md +++ b/frame/shielded-pool/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to `pallet-shielded-pool` will be documented in this file. ## [Unreleased] +## [0.23.0] - 2026-10-08 + +### Changed + +- `get_merkle_path`: a sealed tree's node missing at or above the prune cut + (pruned under an earlier, higher cut) is rebuilt from the leaves instead of + read as zero, so lowering `SealedTreePrunedBelowLevel` on a live chain keeps + every path valid. The runtime lowers it from 10 to 6 in spec 18: a path now + rebuilds its pruned siblings from 62 leaves instead of 1_022. +- `SealedTreePrunedBelowLevel` documents the trade per cut (`2^(21−c) − 2` + stored nodes, `2^c − 2` leaves per path) and why lowering it is safe. + ## [0.22.0] - 2026-10-08 ### Changed diff --git a/frame/shielded-pool/Cargo.toml b/frame/shielded-pool/Cargo.toml index e66c246a..06e5d75d 100644 --- a/frame/shielded-pool/Cargo.toml +++ b/frame/shielded-pool/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-shielded-pool" -version = "0.22.0" +version = "0.23.0" description = "Shielded pool pallet for private transactions using ZK proofs" authors = ["Orbinum Team"] license = "GPL-3.0-or-later" diff --git a/frame/shielded-pool/src/lib.rs b/frame/shielded-pool/src/lib.rs index 4e21015b..674343cd 100644 --- a/frame/shielded-pool/src/lib.rs +++ b/frame/shielded-pool/src/lib.rs @@ -245,14 +245,15 @@ pub mod pallet { /// tree is immutable, so anything dropped here is recomputed from /// `MerkleLeaves` on demand. /// - /// The trade is storage against query latency, and it is lopsided: nodes - /// concentrate at the bottom, so cutting at level 10 drops 99.8% of the - /// entries (1_048_574 -> 2_046 per tree) while a path costs 2^10 leaf - /// reads and 1_023 Poseidon hashes — about 60ms native, ~180ms in Wasm. - /// Cutting at 12 frees only 0.15% more for four times the work. + /// The trade is storage against query latency. A cut `c` keeps + /// 2^(21−c) − 2 nodes per tree and makes a path read 2^c − 2 leaves: + /// nodes concentrate at the bottom, so each level less halves the reads and + /// doubles what is stored. Paths are served by the public RPC, so the read + /// cost is also what an attacker can make a node pay per request. /// - /// Configurable rather than fixed: the recompute cost tracks validator - /// hardware. Must be non-zero and below the tree depth (`integrity_test`). + /// Lowering it on a live chain is safe: a sealed tree's node pruned under an + /// earlier, higher cut is rebuilt from the leaves when a path needs it. + /// Must be non-zero and below the tree depth (`integrity_test`). #[pallet::constant] type SealedTreePrunedBelowLevel: Get; diff --git a/frame/shielded-pool/src/merkle/batch.rs b/frame/shielded-pool/src/merkle/batch.rs index 1673b54b..3b274320 100644 --- a/frame/shielded-pool/src/merkle/batch.rs +++ b/frame/shielded-pool/src/merkle/batch.rs @@ -8,6 +8,8 @@ use super::hashing::{hash_pair, hash_pair_poseidon}; use crate::types::Hash; use sp_std::vec::Vec; +/// Root of a `DEPTH`-level tree holding `leaves` from index 0, the rest empty. +/// Empty input yields the zero leaf, not the empty tree's root. pub fn compute_root_from_leaves_poseidon(leaves: &[Hash]) -> Hash { if leaves.is_empty() { return [0u8; 32]; @@ -41,6 +43,8 @@ pub fn compute_root_from_leaves_poseidon(leaves: &[Hash]) -> current_level.first().copied().unwrap_or([0u8; 32]) } +/// Root of a `DEPTH`-level tree holding `leaves` from index 0, the rest empty. +/// Empty input yields the empty tree's root. pub fn compute_root_from_leaves(leaves: &[Hash]) -> Hash { if leaves.is_empty() { let mut current = [0u8; 32]; diff --git a/frame/shielded-pool/src/merkle/hashing.rs b/frame/shielded-pool/src/merkle/hashing.rs index 799f1481..eb60405c 100644 --- a/frame/shielded-pool/src/merkle/hashing.rs +++ b/frame/shielded-pool/src/merkle/hashing.rs @@ -9,12 +9,9 @@ use ark_ff::BigInteger; /// Digest of an empty subtree rooted at `level`. /// -/// Iterative rather than recursive. The recursion this replaces spent one stack -/// frame per level, and `get_zero_hash_cached` falls through to here for any -/// level past its 21-entry table — with `usize` being 32 bits under Wasm, a -/// caller passing a large level would exhaust the runtime's fixed 1 MB stack. -/// A stack overflow there takes the node down rather than failing a call, while -/// a loop just runs long: slow is recoverable, overflowing is not. +/// A loop, not recursion: `get_zero_hash_cached` falls through to here past its +/// table, and recursing once per level could exhaust the runtime's fixed stack — +/// a node-level failure, where a long loop is only slow. pub fn zero_hash_at_level(level: usize) -> [u8; 32] { let mut current = [0u8; 32]; for _ in 0..level { @@ -23,11 +20,11 @@ pub fn zero_hash_at_level(level: usize) -> [u8; 32] { current } -/// Cached zero hashes for Poseidon (lazy-initialized, thread-safe). +/// Zero hashes for levels 0..=20, computed once. static ZERO_HASHES_POSEIDON: once_cell::race::OnceBox<[[u8; 32]; 21]> = once_cell::race::OnceBox::new(); -/// Get precomputed zero hash at level (optimized with cache). +/// [`zero_hash_at_level`], from the table for the tree's own levels. #[inline] pub fn get_zero_hash_cached(level: usize) -> [u8; 32] { if level < 21 { @@ -44,7 +41,7 @@ pub fn get_zero_hash_cached(level: usize) -> [u8; 32] { zero_hash_at_level(level) } -/// Hash two nodes together using Poseidon (ZK-friendly, ~300 constraints). +/// Poseidon over two field elements, as the circuits hash Merkle nodes. #[inline] pub fn hash_pair_poseidon(left: &[u8; 32], right: &[u8; 32]) -> [u8; 32] { use ark_bn254::Fr as Bn254Fr; @@ -61,10 +58,8 @@ pub fn hash_pair_poseidon(left: &[u8; 32], right: &[u8; 32]) -> [u8; 32] { let hash_fr = hasher.hash_2([FieldElement::new(left_fr), FieldElement::new(right_fr)]); - // BN254 `Fr` always yields 32 bytes, so the clamp never binds today. It is - // here because this runs on the block-import path, where slicing past the - // end would panic the node rather than fail a call — the same reason - // `recipient_to_field` is written this way. + // `Fr` always yields 32 bytes, so the clamp never binds; it stays because a + // slice past the end here would panic block import. let mut hash_bytes = [0u8; 32]; let bigint = hash_fr.inner().into_bigint(); let bytes = bigint.to_bytes_le(); @@ -73,7 +68,7 @@ pub fn hash_pair_poseidon(left: &[u8; 32], right: &[u8; 32]) -> [u8; 32] { hash_bytes } -/// Hash pair — always uses Poseidon. +/// The tree's node hash. pub fn hash_pair(left: &[u8; 32], right: &[u8; 32]) -> [u8; 32] { hash_pair_poseidon(left, right) } diff --git a/frame/shielded-pool/src/merkle/mod.rs b/frame/shielded-pool/src/merkle/mod.rs index 3e68b883..beedd8e8 100644 --- a/frame/shielded-pool/src/merkle/mod.rs +++ b/frame/shielded-pool/src/merkle/mod.rs @@ -50,7 +50,7 @@ mod tests { types::{Commitment, Hash}, }; - // ── hash functions ────────────────────────────────────────────────────── + // ── Hash functions ────────────────────────────────────────────────────── #[test] fn hash_pair_poseidon_deterministic() { @@ -164,7 +164,7 @@ mod tests { assert_ne!(deep, [0u8; 32]); } - // ── IncrementalMerkleTree ──────────────────────────────────────────────── + // ── IncrementalMerkleTree ─────────────────────────────────────────────── #[test] fn tree_new_has_zero_size() { @@ -278,7 +278,7 @@ mod tests { assert!(tree.generate_proof(5, &leaves).is_err()); } - // ── compute_root_from_leaves_poseidon ──────────────────────────────────── + // ── compute_root_from_leaves_poseidon ─────────────────────────────────── #[test] fn compute_root_poseidon_empty_is_zero() { @@ -307,7 +307,7 @@ mod tests { assert_ne!(r1, r2); } - // ── MerkleTreeService (FRAME-backed) ───────────────────────────────────── + // ── MerkleTreeService (FRAME-backed) ──────────────────────────────────── #[test] fn service_insert_leaf_returns_sequential_indices() { @@ -324,8 +324,8 @@ mod tests { new_test_ext().execute_with(|| { let c = Commitment::new([0x01u8; 32]); MerkleTreeService::insert_leaf::(c).unwrap(); - // Duplicate detection is based on CommitmentMemos; simulate a prior memo insert - // (operations layer stores the memo when shielding/transferring) + // Duplicates are detected through `CommitmentMemos`, which the operations + // layer fills on shield and transfer: simulate that. use crate::storage::CommitmentRepository; use crate::types::{EncryptedMemo, MAX_ENCRYPTED_MEMO_SIZE}; CommitmentRepository::store_memo::( @@ -479,7 +479,7 @@ mod tests { }); } - // ── Stored-node path reads vs recomputed reference ─────────────────────── + // ── Stored-node path reads vs recomputed reference ────────────────────── /// Reference sibling-path builder: recomputes every level from the full /// leaf set. Oracle for the O(depth) stored-node read path. @@ -588,7 +588,7 @@ mod tests { }); } - // ── Incremental frontier vs batch consistency ──────────────────────────── + // ── Incremental frontier vs batch consistency ─────────────────────────── #[test] fn incremental_root_matches_batch_root_after_single_insert() { @@ -675,9 +675,8 @@ mod tests { }); } - // Simulates storage round-trip across multiple separate execute_with calls, - // mimicking the frontier being persisted between blocks. - // Verifies SCALE serialization of [[u8; 32]; 20] survives storage read/write cycles. + // The frontier, persisted between blocks (separate `execute_with` calls), + // survives the SCALE round trip of `[[u8; 32]; 20]`. #[test] fn frontier_survives_storage_round_trip_across_separate_calls() { use crate::pallet::MerkleTreeFrontier; @@ -716,7 +715,7 @@ mod tests { ); } - // ── tree-depth consistency ──────────────────────────────────────────────── + // ── Tree-depth consistency ────────────────────────────────────────────── /// integrity_test passes when MaxTreeDepth equals the fixed tree depth. The /// mock is aligned to MAX_TREE_DEPTH, so construction must not panic; a @@ -739,7 +738,7 @@ mod tests { assert!(cap.is_power_of_two() && cap <= 1 << MAX_TREE_DEPTH); } - // ── Multi-tree forest: sealing and rollover ────────────────────────────── + // ── Multi-tree forest: sealing and rollover ───────────────────────────── fn fill_leaves(from: u8, count: u8) { for i in 0..count { @@ -946,7 +945,7 @@ mod tests { }); } - // ── historic-root window ────────────────────────────────────────────────── + // ── Historic-root window ──────────────────────────────────────────────── /// integrity_test rejects a zero root window (checked via the mock's non-zero /// MaxHistoricRoots passing construction). @@ -1444,7 +1443,7 @@ mod prune_tests { crate::pallet::MerkleNodes::::iter().count() } - // ── adversarial: can anything reintroduce the divergence? ───────────────── + // ── Adversarial: can anything reintroduce the divergence? ─────────────── // // The fix is only worth what it survives. Each of these attacks the sweep // from a different angle, trying to make two nodes running the same block diff --git a/frame/shielded-pool/src/merkle/service.rs b/frame/shielded-pool/src/merkle/service.rs index db1a63c7..9e97a58b 100644 --- a/frame/shielded-pool/src/merkle/service.rs +++ b/frame/shielded-pool/src/merkle/service.rs @@ -18,18 +18,24 @@ use frame_support::{ensure, pallet_prelude::*, traits::Get}; use sp_runtime::traits::Saturating; use sp_std::vec::Vec; +/// The forest's storage operations. Stateless: every method reads and writes +/// through [`MerkleRepository`]. pub struct MerkleTreeService; impl MerkleTreeService { - /// Insert a new leaf into the Merkle tree. + // ── Leaves and sealing ────────────────────────────────────────────────── + + /// Insert a leaf into the active tree and return its global index. /// - /// Uses an incremental frontier algorithm: O(depth) hashes per insert, - /// replacing the former O(n) full recomputation from all leaves. + /// 1. Bounds: the global index fits a `u32`, the commitment is new. + /// 2. Walk up the stored frontier — O(depth) — persisting each new node so + /// path reads stay point lookups. + /// 3. Store the leaf, the new root and its historic entry; emit the update. + /// 4. Seal the tree if this leaf filled it. pub fn insert_leaf(commitment: Commitment) -> Result { let index = MerkleRepository::get_tree_size::(); - // Absolute forest ceiling: the global u32 leaf index must stay - // representable (4096 trees at depth 20). Per-tree fullness rolls - // over to a fresh tree below instead of erroring. + // 1. Bounds. Only the forest has a ceiling (4096 trees at depth 20); a + // full tree rolls over to a fresh one in step 4. ensure!(index < u32::MAX, Error::::MerkleTreeFull); ensure!( !CommitmentRepository::exists::(&commitment), @@ -40,25 +46,24 @@ impl MerkleTreeService { let tree_id = index / cap; let local = index % cap; - // Load frontier from storage and run one incremental update. - // Depth is always DEFAULT_TREE_DEPTH (20) — matches the fixed-size frontier array. + // 2. Frontier walk, at the depth the frontier array is sized for. let mut frontier = MerkleRepository::get_frontier::(); let mut current_hash = commitment.0; let mut current_index = local; for (level, frontier_slot) in frontier.iter_mut().enumerate() { if current_index.is_multiple_of(2) { - // Left node: save in frontier, pair with zero-sibling + // Left child: remember it, pair it with the empty right subtree. *frontier_slot = current_hash; let zero = get_zero_hash_cached(level); current_hash = hash_pair(¤t_hash, &zero); } else { - // Right node: combine with stored left sibling + // Right child: pair it with the stored left sibling. current_hash = hash_pair(frontier_slot, ¤t_hash); } current_index /= 2; - // current_hash is now the node at (level + 1, current_index). Persist - // levels 1..=19 so proof reads are O(depth); level 20 is PoseidonRoot. + // `current_hash` is now node (level + 1, current_index). Levels 1..=19 + // are stored; level 20 is `PoseidonRoot`. if level + 1 < crate::types::DEFAULT_TREE_DEPTH { MerkleRepository::set_node::( tree_id, @@ -69,6 +74,7 @@ impl MerkleTreeService { } } + // 3. Store. let new_poseidon_root = current_hash; let old_poseidon_root = MerkleRepository::get_poseidon_root::(); @@ -80,27 +86,27 @@ impl MerkleTreeService { MerkleRepository::set_poseidon_root::(new_poseidon_root); Self::add_poseidon_historic_root::(new_poseidon_root); - // The freshly inserted leaf belongs to `new_poseidon_root`, so this - // event fires before any seal resets the active root. + // Emitted before any seal resets the active root: the leaf belongs to this one. Pallet::::deposit_event(Event::MerkleRootUpdated { old_root: old_poseidon_root, new_root: new_poseidon_root, tree_size: index.saturating_add(1), }); + // 4. Seal. if local + 1 == cap { Self::seal_tree::(tree_id, new_poseidon_root, cap); } Ok(index) } - /// Seal a full tree and open a fresh one, eagerly in the same insert. + /// Seal a full tree and open a fresh one, in the same insert. /// - /// The final root becomes a permanent anchor (`SealedTreeRoots` / - /// `SealedRootIndex`) — unlike the historic ring it never expires, so - /// notes in sealed trees stay spendable forever. The active tree resets - /// to the empty state; the empty root joins the historic ring to keep - /// the `PoseidonRoot ∈ known roots` invariant. + /// 1. The final root becomes a permanent anchor (`SealedTreeRoots`, + /// `SealedRootIndex`): it never expires, so the tree's notes stay + /// spendable forever. + /// 2. The active tree resets to empty, and the empty root joins the historic + /// window so `PoseidonRoot` is always a known root. fn seal_tree(tree_id: u32, final_root: Hash, cap: u32) { MerkleRepository::insert_sealed_root::(tree_id, final_root); MerkleRepository::set_frontier::([[0u8; 32]; crate::types::DEFAULT_TREE_DEPTH]); @@ -116,11 +122,17 @@ impl MerkleTreeService { }); } - /// Record the new root and drop the ones whose retention window has passed. + // ── Historic roots ────────────────────────────────────────────────────── + + /// Record a new root, dropping the ones whose retention window has passed. + /// + /// The window is measured in blocks (`RootRetentionBlocks`); the queue only + /// orders roots by expiry. /// - /// Retention is measured in blocks (`RootRetentionBlocks`), so the window - /// always outlives the mempool longevity a transaction was admitted with. - /// `MaxHistoricRoots` is only a safety cap on queue length. + /// 1. Drain expired slots from the tail, at most `MAX_ROOTS_PRUNED_PER_INSERT`. + /// 2. If the queue still reaches `MaxHistoricRoots` — a safety cap, not the + /// window — evict the oldest and log the misconfiguration. + /// 3. Append the new root. pub(crate) fn add_poseidon_historic_root(poseidon_root: Hash) { let now = frame_system::Pallet::::block_number(); let expires_at = now.saturating_add(T::RootRetentionBlocks::get()); @@ -128,48 +140,38 @@ impl MerkleTreeService { let mut head = MerkleRepository::get_historic_roots_head::(); let mut tail = MerkleRepository::get_historic_roots_tail::(); - // Drain expired slots from the tail. Slots sit in expiry order because - // every insert stores `now + retention` with a non-decreasing `now`, so - // the first live slot ends the scan. + // 1. Drain. Slots are in expiry order (each stores `now + retention`), so + // the first live one ends the scan. let mut pruned = 0usize; while tail < head && pruned < MAX_ROOTS_PRUNED_PER_INSERT { let Some((root, slot_expiry)) = MerkleRepository::get_historic_root_slot::(tail) else { - // Defensive: this path never leaves holes, but skipping keeps the - // queue draining instead of wedging on one forever. Counted against - // the cap so a long run of holes cannot turn into an unbounded read - // loop inside a dispatchable. + // A hole, which this code never leaves: skip it, and count it so the + // scan stays bounded. tail = tail.saturating_add(1); pruned = pruned.saturating_add(1); continue; }; if slot_expiry >= now { - break; // still live — and so is every slot behind it + break; // live, and so is every slot after it } MerkleRepository::remove_historic_root_slot::(tail); tail = tail.saturating_add(1); pruned = pruned.saturating_add(1); - // A root can occupy several slots (re-inserted, or an insert that left - // the root unchanged). The map holds one expiry per root — the latest, - // since `add_historic_poseidon_root_until` only extends it — so it is - // the authority on whether the root is still spendable. O(1) per slot, - // which keeps this loop linear. + // A root can sit in several slots; its map entry holds the latest expiry + // and decides whether it is still spendable. match MerkleRepository::get_historic_root_expiry::(&root) { Some(expiry) if expiry >= now => {} _ => MerkleRepository::remove_poseidon_historic_root::(&root), } } - // The cap is a backstop, not the window: reaching it means the window - // holds more roots than `MaxHistoricRoots` allows, which would silently - // shorten it. Evict the oldest so inserts keep working, and log the - // misconfiguration rather than failing quietly. + // 2. Cap. Reaching it means the window holds more roots than allowed: + // evict the oldest so inserts keep working, and say so. if head.saturating_sub(tail) >= T::MaxHistoricRoots::get() as u64 { - // Not `defensive!`: that expands to `debug_assert!(false)` and panics in - // any debug-assertions build. This branch is a reachable operational - // state, so a validator on a debug build must not halt where a release - // build keeps producing. + // A log, not `defensive!`: that panics in debug builds, and this state + // is reachable. frame_support::__private::log::warn!( target: "runtime::shielded-pool", "historic-root cap reached before retention elapsed; \ @@ -179,11 +181,8 @@ impl MerkleTreeService { MerkleRepository::remove_historic_root_slot::(tail); match MerkleRepository::get_historic_root_expiry::(&root) { Some(expiry) if expiry >= now => { - // Still live: re-queue at the head rather than drop it. - // Deleting the slot while keeping the map entry would strand - // that entry outside `[tail, head)` — unreachable by the drain - // loop, so never prunable and permanently spendable. Forgetting - // it instead would reject spends that are still valid. + // Still live: re-queue it. Dropping only the slot would leave the + // entry unprunable; dropping both would refuse valid spends. MerkleRepository::set_historic_root_slot::(head, root, slot_expiry); head = head.saturating_add(1); } @@ -193,6 +192,7 @@ impl MerkleTreeService { tail = tail.saturating_add(1); } + // 3. Append. MerkleRepository::set_historic_root_slot::(head, poseidon_root, expires_at); head = head.saturating_add(1); @@ -201,6 +201,10 @@ impl MerkleTreeService { MerkleRepository::set_historic_roots_tail::(tail); } + // ── Roots and paths ───────────────────────────────────────────────────── + + /// Whether a spend may be proven against `root`: the active root, a root + /// still in its retention window, or a sealed tree's final root. pub fn is_known_root(root: &Hash) -> bool { MerkleRepository::is_known_root::(root) } @@ -212,9 +216,10 @@ impl MerkleTreeService { /// empty subtree, so the canonical zero hash for that level is used. /// /// A **sealed** tree has its levels below `SealedTreePrunedBelowLevel` dropped - /// (see [`Self::prune_sealed_nodes`]), so those siblings are recomputed from - /// the leaves instead. Only the sibling subtree is rebuilt, not the whole - /// tree: at a cut of 10 that is 2^10 leaf reads and 1_023 hashes. + /// (see [`Self::prune_sealed_nodes`]), so those siblings are rebuilt from the + /// leaves: only each sibling subtree, `2^cut − 2` leaf reads in all. A node + /// missing at or above the cut — pruned under an earlier, higher cut — is + /// rebuilt the same way. pub fn get_merkle_path(leaf_index: u32) -> Option { let size = MerkleRepository::get_tree_size::(); if leaf_index >= size { @@ -228,7 +233,7 @@ impl MerkleTreeService { let mut siblings = [[0u8; 32]; crate::types::DEFAULT_TREE_DEPTH]; let mut indices = [0u8; crate::types::DEFAULT_TREE_DEPTH]; - // Only sealed trees are pruned; the active one always has every node. + // Only sealed trees are pruned; the active one keeps every node. let is_sealed = tree_id < size / cap; let cut = T::SealedTreePrunedBelowLevel::get() as usize; @@ -237,12 +242,18 @@ impl MerkleTreeService { indices[level] = (node_index & 1) as u8; let sibling_index = node_index ^ 1; let sibling = if level == 0 { - // Level-0 nodes are the leaves; map the tree-local sibling - // back to its global MerkleLeaves index. + // Leaves: the tree-local sibling, at its global index. MerkleRepository::get_leaf::(tree_id * cap + sibling_index).map(|c| c.0) } else if is_sealed && level < cut { + // Pruned level of a sealed tree: rebuilt from the leaves. Some(Self::subtree_root::(tree_id, level, sibling_index, cap)) + } else if is_sealed { + // Kept level of a sealed tree. A node pruned under an earlier, higher + // cut is rebuilt; past the capacity that yields the zero hash. + MerkleRepository::get_node::(tree_id, level as u8, sibling_index) + .or_else(|| Some(Self::subtree_root::(tree_id, level, sibling_index, cap))) } else { + // Active tree: a missing node is an empty subtree. MerkleRepository::get_node::(tree_id, level as u8, sibling_index) }; siblings[level] = sibling.unwrap_or_else(|| get_zero_hash_cached(level)); @@ -253,27 +264,23 @@ impl MerkleTreeService { /// Rebuild the node at `(level, node_index)` from the leaves beneath it. /// /// Reads the `2^level` leaves the node spans and folds them pairwise. Used only - /// for pruned levels of sealed trees, where the leaves are immutable, so the - /// result is exactly what was stored before pruning. + /// for pruned nodes of sealed trees, whose leaves are immutable, so the result + /// is exactly what was stored before pruning. fn subtree_root(tree_id: u32, level: usize, node_index: u32, cap: u32) -> Hash { - // `level` is bounded by DEFAULT_TREE_DEPTH at every call site, but the - // shift below would overflow if that ever changed, so fail soft instead of - // wrapping into a wrong-but-plausible root. + // Callers stay below the tree depth; past 31 the shift would wrap into a + // plausible but wrong root, so fail to the zero hash instead. if level >= 32 { return get_zero_hash_cached(level); } let span = 1u32 << level; - // Past the tree's capacity the subtree holds none of its leaves: with - // `MaxLeavesPerTree` below 2^depth those global indices are the next - // tree's. + // Past the tree's capacity the span is the next tree's leaves: empty here. let offset = node_index.saturating_mul(span); if offset >= cap { return get_zero_hash_cached(level); } let base = tree_id.saturating_mul(cap).saturating_add(offset); - // Read the leaves this node spans, none past the tree's capacity. A gap - // means an empty slot, which the tree represents with the level-0 zero hash. + // Read the span, none past the capacity; a gap is an empty leaf. let in_tree = span.min(cap - offset); let mut nodes: Vec = (0..span) .map(|i| { @@ -285,9 +292,8 @@ impl MerkleTreeService { }) .collect(); - // Fold pairwise up to `level`; the vector halves each round, so one node - // remains. A missing right sibling pairs with its level's zero hash, - // matching what `insert_leaf` stored. + // Fold pairwise up to `level`; a missing right sibling is that level's + // zero hash, as `insert_leaf` stored it. for lvl in 0..level { let zero = get_zero_hash_cached(lvl); nodes = nodes @@ -301,23 +307,25 @@ impl MerkleTreeService { .unwrap_or_else(|| get_zero_hash_cached(level)) } + /// Whether `path` takes `leaf` to `root`. pub fn verify_merkle_proof(root: &Hash, leaf: &Hash, path: &DefaultMerklePath) -> bool { IncrementalMerkleTree::<20>::verify_proof(root, leaf, path) } + /// The global leaf index of `commitment`, if it is in the forest. pub fn find_leaf_index(commitment: &Commitment) -> Option { MerkleRepository::find_leaf_index::(commitment) } - /// Drop internal nodes below the cut level for trees that have already sealed. - /// - /// Returns the number of keys removed. Bounded by `budget`: a full tree holds - /// ~1M prunable nodes, far past what one block can absorb, so the sweep parks - /// its position in `SealedPruneCursor` and resumes on the next call. + // ── Sealed-tree pruning ───────────────────────────────────────────────── + + /// Drop sealed trees' nodes below the cut, at most `budget` probes per call; + /// returns how many were removed. /// - /// Safe because `MerkleNodes` serves Merkle paths only — no dispatchable reads - /// it — and a sealed tree's leaves never change, so anything dropped here is - /// reproducible by [`Self::subtree_root`]. + /// Safe: `MerkleNodes` serves paths only — no dispatchable reads it — and a + /// sealed tree's leaves never change, so [`Self::subtree_root`] rebuilds + /// anything dropped. A full tree holds ~1M prunable nodes, so the sweep parks + /// in `SealedPruneCursor` and resumes on the next call. pub(crate) fn prune_sealed_nodes(budget: u32) -> u32 { if budget == 0 { return 0; @@ -332,26 +340,26 @@ impl MerkleTreeService { let (mut tree, mut level, mut index) = Self::prune_resume_point::(); let mut removed = 0u32; - // Every probe is charged, not just the ones that remove something: a level - // already swept is all misses, and an uncharged scan could walk hundreds of - // thousands of keys inside one block. + // Every probe counts against the budget, hit or miss, so an already swept + // level cannot turn into an unbounded scan. let mut probed = 0u32; while probed < budget { if tree >= active_tree { - // Caught up with the active tree — which is never pruned. Nothing - // more to do until another tree seals. + // Caught up: the active tree is never pruned. crate::pallet::SealedPruneCursor::::kill(); return removed; } if level >= cut { + // This tree is done: everything below the cut is gone. crate::pallet::LastPrunedTree::::put(tree); tree = tree.saturating_add(1); level = 1; index = 0; continue; } - // A node at `level` spans 2^level leaves, so the level holds cap >> level. + // Level done: it holds cap >> level nodes. Levels above the tree's height + // hold none here; their nodes lead to the forest root and are kept. if index >= (cap >> level) { level = level.saturating_add(1); index = 0; @@ -371,8 +379,7 @@ impl MerkleTreeService { } /// Where the next sweep starts: the parked cursor, or the tree after the last - /// one fully swept. Starting past `LastPrunedTree` keeps a restart from - /// re-walking trees that are already clean. + /// one fully swept, so a restart never re-walks clean trees. fn prune_resume_point() -> (u32, u8, u32) { crate::pallet::SealedPruneCursor::::get().unwrap_or_else(|| { let next = crate::pallet::LastPrunedTree::::get() @@ -388,6 +395,40 @@ mod tests { use super::*; use crate::mock::{Test, new_test_ext}; + /// A sealed tree pruned under an earlier, higher cut is missing nodes at + /// levels the current cut keeps. Its paths must still verify against the + /// sealed root: the missing siblings are rebuilt from the leaves. This is what + /// makes lowering `SealedTreePrunedBelowLevel` safe on a live chain. + #[test] + fn a_tree_pruned_under_a_higher_cut_still_serves_valid_paths() { + new_test_ext().execute_with(|| { + let cap = ::MaxLeavesPerTree::get(); + let leaves: Vec = (0..cap) + .map(|i| Commitment::new([i as u8 + 1; 32])) + .collect(); + for leaf in &leaves { + MerkleTreeService::insert_leaf::(*leaf).unwrap(); + } + MerkleTreeService::insert_leaf::(Commitment::new([0xEE; 32])).unwrap(); + let sealed = MerkleRepository::get_sealed_root::(0).expect("tree 0 sealed"); + + // Prune as a cut one level above the mock's would have: the mock keeps + // this level, so only the fallback can supply it. + let kept = ::SealedTreePrunedBelowLevel::get(); + for index in 0..(cap >> kept) { + crate::pallet::MerkleNodes::::remove((0u32, kept, index)); + } + + for (i, leaf) in leaves.iter().enumerate() { + let path = MerkleTreeService::get_merkle_path::(i as u32).unwrap(); + assert!( + MerkleTreeService::verify_merkle_proof(&sealed, &leaf.0, &path), + "leaf {i}" + ); + } + }); + } + /// With `MaxLeavesPerTree` below 2^depth, a sealed tree's sibling subtree past /// its capacity spans global leaf indices of the NEXT tree. It must still read /// as empty: those leaves are not this tree's. diff --git a/frame/shielded-pool/src/merkle/tree.rs b/frame/shielded-pool/src/merkle/tree.rs index f662c700..6299fab6 100644 --- a/frame/shielded-pool/src/merkle/tree.rs +++ b/frame/shielded-pool/src/merkle/tree.rs @@ -9,10 +9,15 @@ use crate::types::MerklePath; use frame_support::pallet_prelude::*; use sp_std::vec::Vec; +/// An append-only Merkle tree of depth `DEPTH`, kept as its frontier: the last +/// left child seen at each level is all an insert needs. #[derive(Clone, Encode, Decode, TypeInfo, MaxEncodedLen, Debug)] pub struct IncrementalMerkleTree { + /// The last left child at each level, waiting for its right sibling. pub frontier: [[u8; 32]; DEPTH], + /// Index the next leaf takes; also the number of leaves. pub next_index: u32, + /// Current root. pub root: [u8; 32], } @@ -23,19 +28,15 @@ impl Default for IncrementalMerkleTree { } impl IncrementalMerkleTree { - /// `capacity()` shifts into a `u32`, so a depth of 32 or more is undefined: - /// debug builds panic, release builds wrap to 1 and the tree reports itself - /// full after a single leaf. - /// - /// The bound belongs on the type rather than in the runtime config. The - /// pallet's `integrity_test` pins `MaxTreeDepth` to 20, but this struct is - /// `pub` and generic, so nothing stopped a downstream caller from picking - /// its own depth. Instantiating past the limit now fails to compile. + /// `capacity()` shifts into a `u32`, so a depth of 32 or more would wrap. The + /// bound sits on the type, not in the runtime config, because the struct is + /// public and generic: instantiating it past the limit fails to compile. const _DEPTH_FITS_IN_U32: () = assert!( DEPTH < 32, "IncrementalMerkleTree DEPTH must be below 32: capacity() shifts into a u32" ); + /// An empty tree. pub fn new() -> Self { let root = Self::compute_empty_root(); Self { @@ -45,6 +46,7 @@ impl IncrementalMerkleTree { } } + /// Root of the tree with no leaves. fn compute_empty_root() -> [u8; 32] { let mut current = [0u8; 32]; for _ in 0..DEPTH { @@ -53,18 +55,22 @@ impl IncrementalMerkleTree { current } + /// Digest of an empty subtree at `level`. fn zero_hash(level: usize) -> [u8; 32] { get_zero_hash_cached(level) } + /// Leaves the tree can hold: `2^DEPTH`. pub fn capacity(&self) -> u32 { let () = Self::_DEPTH_FITS_IN_U32; 1u32 << DEPTH } + /// Whether no further leaf fits. pub fn is_full(&self) -> bool { self.next_index >= self.capacity() } + /// Append `leaf` and return its index: O(DEPTH), one walk up the frontier. pub fn insert(&mut self, leaf: [u8; 32]) -> Result { if self.is_full() { return Err("Merkle tree is full"); @@ -89,13 +95,17 @@ impl IncrementalMerkleTree { Ok(index) } + /// Current root. pub fn root(&self) -> [u8; 32] { self.root } + /// Number of leaves inserted. pub fn size(&self) -> u32 { self.next_index } + /// The path of `leaf_index`, rebuilt from every leaf: O(n), for tests and + /// off-chain use. `leaves` must be exactly the leaves inserted so far. pub fn generate_proof( &self, leaf_index: u32, @@ -145,6 +155,8 @@ impl IncrementalMerkleTree { Ok(MerklePath { siblings, indices }) } + /// Whether `path` takes `leaf` to `root`; `indices[level]` is 1 when the + /// node is a right child. pub fn verify_proof(root: &[u8; 32], leaf: &[u8; 32], path: &MerklePath) -> bool { let mut current = *leaf; for level in 0..DEPTH { diff --git a/frame/zk-verifier/CHANGELOG.md b/frame/zk-verifier/CHANGELOG.md index 3dbb766a..815523d2 100644 --- a/frame/zk-verifier/CHANGELOG.md +++ b/frame/zk-verifier/CHANGELOG.md @@ -6,6 +6,28 @@ All notable changes to this pallet are documented here. ## [Unreleased] +## [0.16.0] - 2026-10-08 + +### Security + +- **Breaking:** a spend circuit (transfer, unshield) never takes a key of the + base layout, at any version: such a key binds neither the memos nor the full + recipient. The `FIRST_VERSION` exception is gone; genesis keys for spend + circuits must be memo-bound. Shield is unchanged. +- `unretire_version` re-checks the key against the same rule, so a base spend + key retired for being unsafe cannot come back. It now reads the key: + `unretire_version`'s weight needs regenerating. +- A base spend key already in storage — left by an older runtime or a raw + storage write — verifies nothing: the verifier refuses the layout on every + path, so a proof under it cannot spend with swapped memos. `set_active_version` + refuses such a key too; it now reads the key, so its weight needs + regenerating as well. +- `encode_transfer` / `encode_unshield` return `None` for the base layout: the + base encoding of a spend, and the raw-recipient path with it, is gone. +- The `verify_proof` benchmark seeds its key under an id outside the known + table: a known circuit admits only its own arities, so most `n` would skip + the pairing and the regenerated weight would fall far below the real cost. + ## [0.15.0] - 2026-10-08 ### Changed diff --git a/frame/zk-verifier/Cargo.toml b/frame/zk-verifier/Cargo.toml index 7f9ac7ad..118c57dc 100644 --- a/frame/zk-verifier/Cargo.toml +++ b/frame/zk-verifier/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-zk-verifier" -version = "0.15.0" +version = "0.16.0" description = "Zero-Knowledge proof verification pallet for Orbinum" authors = ["Orbinum Team"] license = "GPL-3.0-or-later" diff --git a/frame/zk-verifier/src/benchmarking.rs b/frame/zk-verifier/src/benchmarking.rs index 7426adc8..abc22456 100644 --- a/frame/zk-verifier/src/benchmarking.rs +++ b/frame/zk-verifier/src/benchmarking.rs @@ -57,10 +57,12 @@ mod benchmarks { .bytes } - /// VK for the TRANSFER circuit (arity 7) — used by the storage benchmarks, - /// which validate that a registered VK deserializes and matches circuit arity. + /// A memo-bound TRANSFER key (arity 8), the layout registration and + /// `unretire_version` accept for a spend circuit. fn sample_verification_key() -> Vec { - synthetic_vk(orbinum_zk_verifier::TRANSFER_PUBLIC_INPUTS) + synthetic_vk( + orbinum_zk_verifier::TRANSFER_PUBLIC_INPUTS + orbinum_zk_verifier::MEMO_HASH_INPUTS, + ) } /// Store the sample key for `(circuit_id, version)`, bypassing registration. @@ -85,7 +87,10 @@ mod benchmarks { /// `Ok`, so the full verification cost is still recorded. #[benchmark] fn verify_proof(n: Linear<1, 16>) { - let circuit_id = CircuitId::TRANSFER; + // An id outside the known table: it takes the base layout at any arity. A + // known circuit admits only its own arities, so every other `n` would skip + // the pairing and the fit would come out far below the real cost. + let circuit_id = CircuitId(200); // Seed storage with an arity-`n` VK so `do_verify` runs `n` scalar-muls + pairing. let vk_info = VerificationKeyInfo { diff --git a/frame/zk-verifier/src/encoding.rs b/frame/zk-verifier/src/encoding.rs index b7a60557..6abe50d1 100644 --- a/frame/zk-verifier/src/encoding.rs +++ b/frame/zk-verifier/src/encoding.rs @@ -3,15 +3,16 @@ //! //! The one place that knows each circuit's input order and how a domain value //! becomes a BN254 field element (32 bytes, little-endian, canonical). The -//! [`InputLayout`] comes from the key being verified against, so a v1, a -//! memo-bound and a cross-tree key each get the inputs they were built for. +//! [`InputLayout`] comes from the key being verified against, so a memo-bound +//! and a cross-tree key each get the inputs they were built for. A base spend +//! key gets none: it binds no memo, and the verifier never admits it. use crate::port::{ShieldStatement, TransferStatement, UnshieldStatement}; use alloc::vec::Vec; use orbinum_zk_verifier::{InputLayout, to_field_le}; /// Transfer inputs, in circuit order: -/// `merkle_root | nullifiers.. | commitments.. | asset_id | fee [| memo_hash]`, or +/// `merkle_root | nullifiers.. | commitments.. | asset_id | fee | memo_hash`, or /// for the cross-tree layout `merkle_roots[0] | merkle_roots[1] | ..` in place of /// the single root. /// @@ -22,49 +23,40 @@ pub fn encode_transfer(s: &TransferStatement, layout: InputLayout) -> Option raw.extend_from_slice(&s.merkle_roots), - InputLayout::Base | InputLayout::MemoBound if root0 == root1 => raw.push(root0), - InputLayout::Base | InputLayout::MemoBound => return None, + InputLayout::MemoBound if root0 == root1 => raw.push(root0), + InputLayout::MemoBound | InputLayout::Base => return None, } raw.extend_from_slice(&s.nullifiers); raw.extend_from_slice(&s.commitments); raw.push(u32_field(s.asset_id)); raw.push(u128_field(s.fee)); - if matches!(layout, InputLayout::MemoBound | InputLayout::CrossTree) { - raw.push(to_field_le(&s.memo_digest)); - } + raw.push(to_field_le(&s.memo_digest)); Some(raw) } /// Unshield inputs, in circuit order: -/// `merkle_root | nullifier | amount | recipient | asset_id | fee | change_commitment [| memo_hash]`. +/// `merkle_root | nullifier | amount | recipient | asset_id | fee | change_commitment | memo_hash`. /// -/// `recipient` is an AccountId32, wider than the field. The base layout takes -/// it mod r, which maps `R` and `R ± r` to the same input — a copier could -/// redirect the withdrawal to an alias nobody controls. The memo-bound layout -/// hashes it first, so an alias would need a blake2 collision. +/// `recipient` is an AccountId32, wider than the field, so it is hashed first: +/// taken mod r, `R` and `R ± r` would be the same input, and a copier could +/// redirect the withdrawal to an alias nobody controls. /// -/// `None` for the cross-tree layout: an unshield spends one note, so no such -/// key can exist for it (`input_layout`). +/// `None` for any layout but the memo-bound one: an unshield spends one note, +/// so no cross-tree key can exist for it (`input_layout`). pub fn encode_unshield(s: &UnshieldStatement, layout: InputLayout) -> Option> { - let (recipient, memo_hash) = match layout { - InputLayout::Base => (to_field_le(&s.recipient), None), - InputLayout::MemoBound => ( - to_field_le(&sp_io::hashing::blake2_256(&s.recipient)), - Some(to_field_le(&s.memo_digest)), - ), - InputLayout::CrossTree => return None, - }; - let mut raw = alloc::vec![ + if layout != InputLayout::MemoBound { + return None; + } + Some(alloc::vec![ s.merkle_root, s.nullifier, u128_field(s.amount), - recipient, + to_field_le(&sp_io::hashing::blake2_256(&s.recipient)), u32_field(s.asset_id), u128_field(s.fee), s.change_commitment, - ]; - raw.extend(memo_hash); - Some(raw) + to_field_le(&s.memo_digest), + ]) } /// Shield inputs, in circuit order: `commitment | value | asset_id`. @@ -142,13 +134,20 @@ mod tests { #[test] fn transfer_follows_circuit_order() { - let raw = encode_transfer(&transfer(), InputLayout::Base).unwrap(); - assert_eq!(raw.len(), TRANSFER_PUBLIC_INPUTS); + let raw = encode_transfer(&transfer(), InputLayout::MemoBound).unwrap(); + assert_eq!(raw.len(), TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS); assert_eq!(raw[0], [0x01; 32]); assert_eq!(&raw[1..3], &NULLIFIERS); assert_eq!(&raw[3..5], &COMMITMENTS); assert_eq!(raw[5], u32_field(7)); assert_eq!(raw[6], u128_field(500)); + assert_eq!(raw[7], to_field_le(&[0xEE; 32])); + } + + #[test] + fn a_base_spend_key_gets_no_encoding() { + assert!(encode_transfer(&transfer(), InputLayout::Base).is_none()); + assert!(encode_unshield(&unshield(), InputLayout::Base).is_none()); } #[test] @@ -177,7 +176,6 @@ mod tests { merkle_roots: [[0x01; 32], [0x09; 32]], ..transfer() }; - assert!(encode_transfer(&s, InputLayout::Base).is_none()); assert!(encode_transfer(&s, InputLayout::MemoBound).is_none()); } @@ -208,56 +206,25 @@ mod tests { } #[test] - fn memo_bound_transfer_appends_the_reduced_memo_digest() { - let base = encode_transfer(&transfer(), InputLayout::Base).unwrap(); - let bound = encode_transfer(&transfer(), InputLayout::MemoBound).unwrap(); - assert_eq!(bound.len(), TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS); - assert_eq!(&bound[..TRANSFER_PUBLIC_INPUTS], &base[..]); - assert_eq!(bound[TRANSFER_PUBLIC_INPUTS], to_field_le(&[0xEE; 32])); - } - - #[test] - fn memo_digest_only_matters_when_bound() { - let other = TransferStatement { - memo_digest: [0x11; 32], - ..transfer() - }; - assert_eq!( - encode_transfer(&transfer(), InputLayout::Base).unwrap(), - encode_transfer(&other, InputLayout::Base).unwrap() - ); - assert_ne!( - encode_transfer(&transfer(), InputLayout::MemoBound).unwrap(), - encode_transfer(&other, InputLayout::MemoBound).unwrap() - ); - } - - #[test] - fn unshield_follows_circuit_order() { - let raw = encode_unshield(&unshield(), InputLayout::Base).unwrap(); - assert_eq!(raw.len(), UNSHIELD_PUBLIC_INPUTS); + fn unshield_follows_circuit_order_with_a_hashed_recipient() { + let raw = encode_unshield(&unshield(), InputLayout::MemoBound).unwrap(); + assert_eq!(raw.len(), UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS); assert_eq!(raw[0], [0x01; 32]); assert_eq!(raw[1], [0x02; 32]); assert_eq!(raw[2], u128_field(100)); - assert_eq!(raw[3], to_field_le(&[0xFF; 32])); - assert_eq!(raw[4], u32_field(7)); - assert_eq!(raw[5], u128_field(5)); - assert_eq!(raw[6], [0x06; 32]); - } - - #[test] - fn memo_bound_unshield_hashes_the_recipient_and_appends_the_memo_digest() { - let raw = encode_unshield(&unshield(), InputLayout::MemoBound).unwrap(); - assert_eq!(raw.len(), UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS); assert_eq!( raw[3], to_field_le(&sp_io::hashing::blake2_256(&[0xFF; 32])) ); - assert_eq!(raw[UNSHIELD_PUBLIC_INPUTS], to_field_le(&[0xEE; 32])); + assert_eq!(raw[4], u32_field(7)); + assert_eq!(raw[5], u128_field(5)); + assert_eq!(raw[6], [0x06; 32]); + assert_eq!(raw[7], to_field_le(&[0xEE; 32])); } + /// `R + r` is the same field element as `R`; hashed first, it is another input. #[test] - fn a_recipient_alias_encodes_the_same_only_in_the_base_layout() { + fn a_recipient_alias_encodes_differently() { let recipient = [0x01; 32]; let aliased = UnshieldStatement { recipient: field_twin(recipient), @@ -268,11 +235,6 @@ mod tests { ..unshield() }; assert_ne!(original.recipient, aliased.recipient); - assert_eq!( - encode_unshield(&original, InputLayout::Base).unwrap(), - encode_unshield(&aliased, InputLayout::Base).unwrap(), - "v1 cannot tell R from R + r" - ); assert_ne!( encode_unshield(&original, InputLayout::MemoBound).unwrap(), encode_unshield(&aliased, InputLayout::MemoBound).unwrap() @@ -315,15 +277,12 @@ mod tests { fee: u128::MAX, ..unshield() }; - for layout in [InputLayout::Base, InputLayout::MemoBound] { - let raw = encode_unshield(&max, layout).unwrap(); - assert!( - orbinum_zk_verifier::PublicInputs::new(raw) - .to_field_elements() - .is_ok(), - "{layout:?}" - ); - } + let raw = encode_unshield(&max, InputLayout::MemoBound).unwrap(); + assert!( + orbinum_zk_verifier::PublicInputs::new(raw) + .to_field_elements() + .is_ok() + ); } #[test] @@ -342,7 +301,7 @@ mod tests { assert_eq!(raw[2], u32_field(7)); } - // ── shield against a real proof ─────────────────────────────────────────── + // ── Shield against a real proof ─────────────────────────────────────────── /// A real shield proof of `@orbinum/circuits` 0.16.0 (`fixtures/shield.input.json`: /// a 1000-unit note of asset 0), checked through `encode_shield` against the diff --git a/frame/zk-verifier/src/lib.rs b/frame/zk-verifier/src/lib.rs index f96d6268..ddb7ff07 100644 --- a/frame/zk-verifier/src/lib.rs +++ b/frame/zk-verifier/src/lib.rs @@ -12,10 +12,9 @@ //! //! ## Key rotation //! -//! A known circuit's first version may use its base input layout; every later -//! version must be memo-bound, and a transfer may also be cross-tree (one root -//! per input). Rotation is done by Root: register the new version, -//! `set_active_version`, then `retire_version` the old one. +//! A spend circuit's keys are memo-bound or, for a transfer, cross-tree (one +//! root per input); shield has one layout. Rotation is done by Root: register +//! the new version, `set_active_version`, then `retire_version` the old one. //! //! ## Usage //! @@ -73,12 +72,13 @@ pub mod pallet { #[pallet::config] pub trait Config: frame_system::Config>> { + /// Largest proof accepted, in bytes. #[pallet::constant] type MaxProofSize: Get; - + /// Most public inputs `verify_proof` takes. #[pallet::constant] type MaxPublicInputs: Get; - + /// Benchmarked weights. type WeightInfo: WeightInfo; } @@ -137,6 +137,7 @@ pub mod pallet { #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] pub struct GenesisConfig { + /// One key per circuit, registered as version 1 and activated. pub verification_keys: Vec<(CircuitId, Vec)>, #[serde(skip)] pub _phantom: PhantomData, @@ -165,10 +166,9 @@ pub mod pallet { /// (the benchmark runner). Enabling it alone means a release runtime with no /// verification, so abort construction in that case. fn integrity_test() { - // `const` block: both operands are `cfg!`, so this resolves at compile time - // and a bad feature combination fails the build rather than the runtime's - // integrity check. Strictly stronger than asserting at runtime, and it is - // what clippy::assertions_on_constants asks for. + // A `const` block: both operands are `cfg!`, so a bad feature set fails + // the build, not the runtime check (what clippy::assertions_on_constants + // asks for). const { assert!( !cfg!(feature = "skip-proof-verification") @@ -186,42 +186,24 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { - VerificationKeyRegistered { - circuit_id: CircuitId, - version: u32, - }, - ActiveVersionSet { - circuit_id: CircuitId, - version: u32, - }, - VerificationKeyRemoved { - circuit_id: CircuitId, - version: u32, - }, - VersionRetired { - circuit_id: CircuitId, - version: u32, - }, - VersionUnretired { - circuit_id: CircuitId, - version: u32, - }, - ProofVerified { - circuit_id: CircuitId, - version: u32, - }, - ProofVerificationFailed { - circuit_id: CircuitId, - version: u32, - }, - BatchVerificationKeysRegistered { - count: u32, - }, + /// A key was registered for `(circuit_id, version)`. + VerificationKeyRegistered { circuit_id: CircuitId, version: u32 }, + /// `version` is now the circuit's active one. + ActiveVersionSet { circuit_id: CircuitId, version: u32 }, + /// A version's key and its data were removed. + VerificationKeyRemoved { circuit_id: CircuitId, version: u32 }, + /// A version stopped verifying; its data is kept. + VersionRetired { circuit_id: CircuitId, version: u32 }, + /// A retired version verifies again. + VersionUnretired { circuit_id: CircuitId, version: u32 }, + /// `verify_proof` accepted a proof. + ProofVerified { circuit_id: CircuitId, version: u32 }, + /// `verify_proof` rejected a proof. + ProofVerificationFailed { circuit_id: CircuitId, version: u32 }, + /// `count` keys were registered in one batch. + BatchVerificationKeysRegistered { count: u32 }, /// Every version of a retired circuit was purged from storage. - CircuitPurged { - circuit_id: CircuitId, - removed: u32, - }, + CircuitPurged { circuit_id: CircuitId, removed: u32 }, } // ── Errors ──────────────────────────────────────────────────────────────── @@ -272,20 +254,17 @@ pub mod pallet { impl Pallet { /// Register a verification key version for a circuit (Root only). /// - /// SECURITY INVARIANT: a new version of an EXISTING circuit id must accept - /// the same notes under rules at least as strict: a key rotation of the same - /// circuit, its memo-bound variant (the same constraints plus `memo_hash`), - /// or, for a transfer, its cross-tree variant (memo-bound with one Merkle - /// root per input instead of one shared root), each recognised by arity — - /// see `InputLayout`. The cross-tree variant proves each note against its - /// own root under the same constraints; the pool checks every root is known. + /// Refused when the key does not deserialize, its arity gives no admitted + /// layout (a spend key is memo-bound or, for a transfer, cross-tree — never + /// base), the version exists, or the circuit is at its version cap. /// - /// A note's circuit version is NOT bound into its commitment, so the - /// submitter picks the version freely; a version with weaker constraints at - /// a known arity would let any note be spent under it. Any other semantic change MUST use a NEW - /// circuit id. `ensure_vk_arity` enforces arity, NOT semantics — that is a - /// governance responsibility. Retire a superseded version with - /// `retire_version` (a v1 key binds neither memos nor the full recipient). + /// SECURITY INVARIANT: a note's circuit version is not bound into its + /// commitment, so the submitter picks it freely. Every version of a circuit + /// must therefore accept the same notes under rules at least as strict: a + /// key rotation, its memo-bound variant, or its cross-tree variant (one root + /// per input; the pool checks each is known). The pallet checks arity, not + /// semantics: any other change takes a new circuit id, and a superseded + /// version is retired with `retire_version`. #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::register_verification_key())] pub fn register_verification_key( @@ -298,7 +277,11 @@ pub mod pallet { Self::register_vk(circuit_id, version, verification_key, false) } - /// Set the active verification key version for a circuit (Root only). + /// Make `version` the circuit's active one (Root only). + /// + /// Refused for a missing key, a retired version, or a key of a layout the + /// pallet does not admit: wallets would prove against a key every proof + /// then fails. #[pallet::call_index(1)] #[pallet::weight(T::WeightInfo::set_active_version())] pub fn set_active_version( @@ -307,22 +290,19 @@ pub mod pallet { version: u32, ) -> DispatchResult { ensure_root(origin)?; - ensure!( - VerificationKeys::::contains_key(circuit_id, version), - Error::::VerificationKeyNotFound - ); - // A retired version cannot verify: making it active would leave wallets - // proving against a key every proof then fails. + let info = VerificationKeys::::get(circuit_id, version) + .ok_or(Error::::VerificationKeyNotFound)?; ensure!( !RetiredVersions::::contains_key(circuit_id, version), Error::::UnsupportedCircuitVersion ); + Self::ensure_vk_arity(circuit_id, &info.key_data)?; Self::activate(circuit_id, version); Ok(()) } - /// Remove a verification key version (Root only, cannot remove active version). + /// Remove a version and all its data (Root only); never the active one. #[pallet::call_index(2)] #[pallet::weight(T::WeightInfo::remove_verification_key())] pub fn remove_verification_key( @@ -351,7 +331,8 @@ pub mod pallet { Ok(()) } - /// Verify a zero-knowledge proof (any signed origin). + /// Verify a proof against caller-encoded inputs, under the circuit's active + /// key (any signed origin). #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::verify_proof(public_inputs.len() as u32))] pub fn verify_proof( @@ -382,10 +363,9 @@ pub mod pallet { circuit_id, version, }); - // Benchmarks feed dummy proofs that never verify; skipping the error - // return lets the runner record the weight (the pairing already ran). - // Gated on `skip-proof-verification`, NOT `runtime-benchmarks`, so a - // release runtime never disables the error path. See integrity_test. + // Benchmarks feed proofs that never verify: skipping the error lets the + // runner record the weight. Gated on `skip-proof-verification`, which + // `integrity_test` keeps out of any non-benchmark build. #[cfg(not(feature = "skip-proof-verification"))] return Err(Error::::VerificationFailed.into()); } @@ -418,7 +398,8 @@ pub mod pallet { Ok(()) } - /// Retires a verification key version while preserving its data. + /// Retire a version (Root only): it stops verifying, its data is kept. + /// Never the active one. #[pallet::call_index(5)] #[pallet::weight(T::WeightInfo::retire_version())] pub fn retire_version( @@ -447,7 +428,10 @@ pub mod pallet { Ok(()) } - /// Reverse `retire_version`: allow proofs for `(circuit_id, version)` again. + /// Undo `retire_version` (Root only). + /// + /// The key is held to the registration rules, so one retired for being + /// unsafe cannot come back. #[pallet::call_index(6)] #[pallet::weight(T::WeightInfo::unretire_version())] pub fn unretire_version( @@ -460,6 +444,9 @@ pub mod pallet { RetiredVersions::::contains_key(circuit_id, version), Error::::VersionNotRetired ); + let info = VerificationKeys::::get(circuit_id, version) + .ok_or(Error::::VerificationKeyNotFound)?; + Self::ensure_vk_arity(circuit_id, &info.key_data)?; RetiredVersions::::remove(circuit_id, version); Self::deposit_event(Event::VersionUnretired { circuit_id, @@ -468,32 +455,22 @@ pub mod pallet { Ok(()) } - /// Erase every trace of a circuit the runtime no longer implements. - /// - /// `remove_verification_key` and `retire_version` both refuse to touch the - /// active version, which is what keeps a live circuit from ending up with - /// no key to verify against. That guard also makes them unable to retire a - /// circuit as a whole: its last version is, by construction, the active one. - /// This call covers that gap — and only that gap. + /// Erase every trace of a circuit the runtime no longer implements (Root + /// only). /// - /// The one guard: the runtime must no longer know the id — - /// `expected_public_inputs` returns `None`. An id above `u8::MAX` is - /// rejected outright rather than truncated into that lookup, so a future - /// circuit numbered past 255 cannot alias its way past this check. + /// `retire_version` and `remove_verification_key` never touch the active + /// version, so neither can retire a circuit whose last version is active. + /// This call covers that gap, and only it. /// - /// `ActiveCircuitVersion` is cleared here rather than required to be empty - /// beforehand. Requiring it would make the call unreachable: the first - /// `register_verification_key` for a circuit activates the version it - /// registers, and no extrinsic ever clears that entry — `set_active_version` - /// only overwrites, and both `retire_version` and `remove_verification_key` - /// refuse to touch whichever version is active. An unknown id has no - /// verification route regardless of what the entry says, so it carries no - /// authority worth guarding. + /// 1. The id must be unknown to the runtime (`expected_public_inputs` is + /// `None`). + /// 2. Count every map's entries, plus a stale `ActiveCircuitVersion`. + /// 3. Refuse a map wider than `MAX_VERSIONS_PER_CIRCUIT`. + /// 4. Clear all five maps by prefix — so entries without a + /// `VerificationKeys` row go too — and report the entries cleared. /// - /// Clears all five maps by prefix — `VerificationKeys`, `VkHashes`, - /// `VerificationStats`, `RetiredVersions`, `ActiveCircuitVersion` — rather - /// than iterating one map's versions, so satellite entries left without a - /// `VerificationKeys` row are collected too. + /// `ActiveCircuitVersion` is cleared, not required empty: no extrinsic ever + /// clears it, and an unknown id has no verification route whatever it says. #[pallet::call_index(7)] #[pallet::weight(T::WeightInfo::purge_circuit(Pallet::::MAX_VERSIONS_PER_CIRCUIT))] pub fn purge_circuit( @@ -502,73 +479,62 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - // Fail closed on ids that do not fit the lookup: `as u8` would alias - // e.g. 257 onto 1, so an id past 255 must never reach the table. + // 1. Unknown id. One past 255 is refused: `as u8` would alias 257 onto 1. let id = u8::try_from(circuit_id.0).map_err(|_| Error::::CircuitStillInUse)?; ensure!( orbinum_zk_verifier::expected_public_inputs(id).is_none(), Error::::CircuitStillInUse ); - // Counted by iterating rather than from `clear_prefix`'s result: its - // counters only report keys committed to the backend, and everything - // written earlier in the same block still lives in the overlay. - // - // Each map is counted on its own and the totals summed, not maxed: the - // four normally share a version set, but nothing in the type system says - // they must, and the sum is what the clear below actually pays for. + // 2. Count, by iterating: `clear_prefix`'s counters miss this block's + // overlay writes. Summed per map, as nothing forces the maps to share a + // version set. let per_map = [ VerificationKeys::::iter_key_prefix(circuit_id).count(), VkHashes::::iter_key_prefix(circuit_id).count(), VerificationStats::::iter_key_prefix(circuit_id).count(), RetiredVersions::::iter_key_prefix(circuit_id).count(), ]; - // A stale `ActiveCircuitVersion` with no versions behind it is still an - // entry to clear, so it belongs in the total the event reports. + // A stale active pointer is an entry to clear too. let active_entries = usize::from(ActiveCircuitVersion::::contains_key(circuit_id)); let entries = per_map.iter().sum::().saturating_add(active_entries); ensure!(entries > 0, Error::::CircuitHasNoStorage); - // The cap bounds versions per map, so the clear is bounded by the widest - // map, not by the total. More than that means storage was built outside - // `store_vk`; bail rather than clear part of it — the extrinsic reverts - // as a whole, so nothing is left half-done. + // 3. Bound. The cap is per map, so the widest map bounds the clear; a + // wider one was built outside `store_vk`. let widest = per_map.iter().copied().max().unwrap_or(0); ensure!( widest <= Self::MAX_VERSIONS_PER_CIRCUIT as usize, Error::::TooManyVersions ); - // `u32::MAX` as the limit: the count above already proved the prefix - // fits within the cap, so this only has to clear everything in one pass. + // 4. Clear, in one pass: the count proved every prefix fits the cap. let _ = VerificationKeys::::clear_prefix(circuit_id, u32::MAX, None); let _ = VkHashes::::clear_prefix(circuit_id, u32::MAX, None); let _ = VerificationStats::::clear_prefix(circuit_id, u32::MAX, None); let _ = RetiredVersions::::clear_prefix(circuit_id, u32::MAX, None); ActiveCircuitVersion::::remove(circuit_id); - // Reports entries cleared across every map, not versions: an indexer - // reconciling against its own view needs the real figure, and a stale - // active pointer with no versions behind it still cleared something. + // Entries, not versions: the figure an indexer reconciles against. Self::deposit_event(Event::CircuitPurged { circuit_id, removed: entries as u32, }); - // Weighed by versions, not entries: the benchmark's linear component - // charges one read and four writes per version, so it is already the - // per-version cost of clearing all four maps. + // Weighed by versions: the benchmark charges one read and four writes + // per version, the cost of clearing all four maps. Ok(Some(T::WeightInfo::purge_circuit(widest as u32)).into()) } } + // ── Key rules ───────────────────────────────────────────────────────────── + impl Pallet { /// Upper bound on stored versions per circuit — keeps the versions DoubleMap /// and the runtime-API iteration provably bounded. Registration is Root-only, /// so this is operator-discipline, not an attacker limit. pub(crate) const MAX_VERSIONS_PER_CIRCUIT: u32 = 64; - /// The version genesis registers, and the only version of a known circuit - /// allowed the base input layout. + /// The version genesis registers. pub(crate) const FIRST_VERSION: u32 = 1; /// Validate and store a new key for `(circuit_id, version)`, then activate @@ -581,7 +547,7 @@ pub mod pallet { set_active: bool, ) -> DispatchResult { ensure!(!key_data.is_empty(), Error::::EmptyVerificationKey); - Self::ensure_vk_arity(circuit_id, version, &key_data)?; + Self::ensure_vk_arity(circuit_id, &key_data)?; // Never overwrite: a replaced key would silently change what verifies. ensure!( !VerificationKeys::::contains_key(circuit_id, version), @@ -607,39 +573,17 @@ pub mod pallet { }); } - /// Verify the VK deserializes as a BN254 Groth16 key and that its arity - /// fits one of the circuit's input layouts (base, memo-bound or, for a - /// transfer, cross-tree). - /// - /// For a spend circuit, only [`Self::FIRST_VERSION`] may use the base - /// layout: every later version must bind its memos and the full recipient. - /// That keeps a v1 key from being registered again as "v2" — a rotation - /// that would retire nothing but the version number. Shield has one layout - /// at every version. - /// - /// Only ids in the known table carry an expected arity; the rest are - /// checked to deserialize and nothing more. An id that does not fit a - /// `u8` is rejected outright rather than truncated — `as u8` would alias - /// 257 onto 1 and silently validate a key against the wrong circuit's - /// arity. `purge_circuit` guards the same lookup the same way. - fn ensure_vk_arity(circuit_id: CircuitId, version: u32, key_data: &[u8]) -> DispatchResult { - use orbinum_zk_verifier::{InputLayout, VerifyingKey, has_memo_layout, input_layout}; - - let id = u8::try_from(circuit_id.0).map_err(|_| Error::::InvalidVerificationKey)?; - - let vk = VerifyingKey::new(key_data.to_vec()); - let arity = vk + /// The rule every stored key is held to: it deserializes as a BN254 Groth16 + /// key and its arity gives an admitted layout + /// ([`verifier::admitted_layout`]). Ids outside the known table are only + /// checked to deserialize. + fn ensure_vk_arity(circuit_id: CircuitId, key_data: &[u8]) -> DispatchResult { + let arity = orbinum_zk_verifier::VerifyingKey::new(key_data.to_vec()) .num_public_inputs() .map_err(|_| Error::::InvalidVerificationKey)?; - - let layout = input_layout(id, arity).ok_or(Error::::InvalidVerificationKey)?; - ensure!( - !(has_memo_layout(id) - && version != Self::FIRST_VERSION - && layout == InputLayout::Base), - Error::::InvalidVerificationKey - ); - Ok(()) + verifier::admitted_layout(circuit_id, arity) + .map(|_| ()) + .ok_or(Error::::InvalidVerificationKey.into()) } /// Insert a validated VK for `(circuit_id, version)` and store its hash, diff --git a/frame/zk-verifier/src/port.rs b/frame/zk-verifier/src/port.rs index d73331ae..b64dee99 100644 --- a/frame/zk-verifier/src/port.rs +++ b/frame/zk-verifier/src/port.rs @@ -25,25 +25,32 @@ pub struct TransferStatement { pub nullifiers: Vec<[u8; 32]>, /// One per output note, in the order their memos are submitted. pub commitments: Vec<[u8; 32]>, + /// The asset of every note. pub asset_id: u32, + /// Relay fee, taken from the inputs. pub fee: u128, - /// `blake2_256` of the SCALE-encoded output memos. Bound by memo-bound versions. + /// `blake2_256` of the SCALE-encoded output memos; every transfer key binds it. pub memo_digest: [u8; 32], } /// What an unshield proof attests to, as the pallet submits it. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct UnshieldStatement { + /// The root the spent note is proven against. pub merkle_root: [u8; 32], + /// The spent note's nullifier. pub nullifier: [u8; 32], + /// What the recipient receives. pub amount: u128, /// The recipient account's raw 32 bytes. See [`encoding::encode_unshield`]. pub recipient: [u8; 32], + /// The asset of the spent note. pub asset_id: u32, + /// Relay fee, taken from the note. pub fee: u128, /// Zero for a total unshield. pub change_commitment: [u8; 32], - /// `blake2_256` of the SCALE-encoded `[change_memo]`. Bound by memo-bound versions. + /// `blake2_256` of the SCALE-encoded `[change_memo]`; every unshield key binds it. pub memo_digest: [u8; 32], } @@ -51,8 +58,11 @@ pub struct UnshieldStatement { /// deposited value and asset. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct ShieldStatement { + /// The commitment inserted into the tree. pub commitment: [u8; 32], + /// The deposited value. pub value: u128, + /// The deposited asset. pub asset_id: u32, } @@ -269,8 +279,10 @@ mod tests { }); } + /// A base key in storage — never registrable, but left by an older runtime + /// or a raw storage write — verifies nothing, active or named. #[test] - fn transfer_verifies_under_the_base_and_the_memo_bound_key() { + fn transfer_verifies_under_the_memo_bound_key_only() { new_test_ext().execute_with(|| { insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); insert_vk( @@ -279,7 +291,8 @@ mod tests { TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, ); activate(CircuitId::TRANSFER, 1); - assert_eq!(verify_transfer(&transfer(), None), Ok(true)); + assert_eq!(verify_transfer(&transfer(), None), Ok(false)); + assert_eq!(verify_transfer(&transfer(), Some(1)), Ok(false)); assert_eq!(verify_transfer(&transfer(), Some(2)), Ok(true)); }); } @@ -301,10 +314,11 @@ mod tests { } #[test] - fn a_same_tree_transfer_verifies_under_every_key() { + fn a_same_tree_transfer_verifies_under_every_admitted_key() { new_test_ext().execute_with(|| { insert_all_transfer_keys(); - for version in 1..=3 { + assert_eq!(verify_transfer(&transfer(), Some(1)), Ok(false)); + for version in 2..=3 { assert_eq!(verify_transfer(&transfer(), Some(version)), Ok(true)); } }); @@ -403,8 +417,10 @@ mod tests { }); } + /// A base key in storage — never registrable, but left by an older runtime + /// or a raw storage write — verifies nothing, active or named. #[test] - fn unshield_verifies_under_the_base_and_the_memo_bound_key() { + fn unshield_verifies_under_the_memo_bound_key_only() { new_test_ext().execute_with(|| { insert_vk(CircuitId::UNSHIELD, 1, UNSHIELD_PUBLIC_INPUTS); insert_vk( @@ -413,7 +429,8 @@ mod tests { UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS, ); activate(CircuitId::UNSHIELD, 1); - assert_eq!(verify_unshield(&unshield(), None), Ok(true)); + assert_eq!(verify_unshield(&unshield(), None), Ok(false)); + assert_eq!(verify_unshield(&unshield(), Some(1)), Ok(false)); assert_eq!(verify_unshield(&unshield(), Some(2)), Ok(true)); }); } diff --git a/frame/zk-verifier/src/tests.rs b/frame/zk-verifier/src/tests.rs index 9d891733..30a0ac19 100644 --- a/frame/zk-verifier/src/tests.rs +++ b/frame/zk-verifier/src/tests.rs @@ -43,8 +43,9 @@ fn register(circuit_id: CircuitId, version: u32, key: VkBytes) -> DispatchResult ZkVerifier::register_verification_key(root().into(), circuit_id, version, key) } +/// A transfer key registration accepts: the memo-bound layout. fn vk_bytes() -> VkBytes { - real_vk(TRANSFER_PUBLIC_INPUTS) + real_vk(TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS) } fn vk_too_short() -> VkBytes { @@ -55,17 +56,13 @@ fn vk_empty() -> VkBytes { BoundedVec::default() } -/// A key registration accepts for `(circuit_id, version)`: the base layout for -/// the first version, the memo-bound one for any later version of a spend circuit. -fn vk_for(circuit_id: CircuitId, version: u32) -> VkBytes { +/// A key registration accepts for `circuit_id`: memo-bound for a spend +/// circuit, the base layout for shield and unknown ids. The version does not +/// matter. +fn vk_for(circuit_id: CircuitId, _version: u32) -> VkBytes { let id = circuit_id.0 as u8; let arity = match orbinum_zk_verifier::expected_public_inputs(id) { - Some(base) - if version > Pallet::::FIRST_VERSION - && orbinum_zk_verifier::has_memo_layout(id) => - { - base + MEMO_HASH_INPUTS - } + Some(base) if orbinum_zk_verifier::has_memo_layout(id) => base + MEMO_HASH_INPUTS, Some(base) => base, None => TRANSFER_PUBLIC_INPUTS, }; @@ -140,25 +137,56 @@ fn register_vk_rejects_key_shorter_than_256_bytes() { }); } -/// A version that binds its memos takes one input more; it registers -/// alongside the base version, which keeps verifying until retired. +/// A spend circuit's key must bind its memos at every version, the first +/// included: a base key would let a copier swap memos or alias the recipient. #[test] -fn register_vk_accepts_the_memo_bound_arity_next_to_the_base_one() { +fn register_vk_refuses_a_base_spend_key_at_every_version() { new_test_ext().execute_with(|| { + for (cid, base) in [ + (CircuitId::TRANSFER, TRANSFER_PUBLIC_INPUTS), + (CircuitId::UNSHIELD, UNSHIELD_PUBLIC_INPUTS), + ] { + for version in [1, 2] { + assert_noop!( + register(cid, version, real_vk(base)), + Error::::InvalidVerificationKey + ); + assert_ok!(register(cid, version, real_vk(base + MEMO_HASH_INPUTS))); + } + } + // Shield has only the base layout. assert_ok!(register( - CircuitId::TRANSFER, + CircuitId::SHIELD, 1, - real_vk(TRANSFER_PUBLIC_INPUTS) + real_vk(SHIELD_PUBLIC_INPUTS) )); - assert_ok!(register( + }); +} + +/// A key retired for being unsafe cannot come back: `unretire_version` holds +/// it to the rules a new registration faces. +#[test] +fn unretire_refuses_a_base_spend_key() { + new_test_ext().execute_with(|| { + insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); + insert_vk( CircuitId::TRANSFER, 2, - real_vk(TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS) + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); + activate(CircuitId::TRANSFER, 2); + assert_ok!(ZkVerifier::retire_version( + root().into(), + CircuitId::TRANSFER, + 1 )); - assert_ok!(register( - CircuitId::UNSHIELD, - 2, - real_vk(UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS) + assert_noop!( + ZkVerifier::unretire_version(root().into(), CircuitId::TRANSFER, 1), + Error::::InvalidVerificationKey + ); + assert!(RetiredVersions::::contains_key( + CircuitId::TRANSFER, + 1 )); }); } @@ -233,28 +261,6 @@ fn register_vk_rejects_a_point_at_infinity() { }); } -/// A later version of a known circuit must be memo-bound: re-registering a -/// base (v1-layout) key as "v2" would rotate nothing but the number. -#[test] -fn register_vk_refuses_the_base_layout_past_version_one() { - new_test_ext().execute_with(|| { - for (version, key) in [ - (2, real_vk(TRANSFER_PUBLIC_INPUTS)), - (7, real_vk(TRANSFER_PUBLIC_INPUTS)), - ] { - assert_noop!( - register(CircuitId::TRANSFER, version, key), - Error::::InvalidVerificationKey - ); - } - assert_ok!(register( - CircuitId::TRANSFER, - 2, - vk_for(CircuitId::TRANSFER, 2) - )); - }); -} - /// Shield has no memo-bound layout: every version takes the base arity, and one /// input more — memo-bound for a spend circuit — is refused. #[test] @@ -284,7 +290,8 @@ fn register_vk_takes_shield_at_its_base_arity_for_every_version() { } /// The v1 → v2 rotation, done by Root extrinsics after the upgrade: register -/// the memo-bound keys as version 2, make them active, retire version 1. +/// the memo-bound keys as version 2, make them active, retire version 1. The v1 +/// key is one registered under the old rules, so it is placed in storage. #[test] fn v1_rotates_to_memo_bound_v2_by_extrinsic() { new_test_ext().execute_with(|| { @@ -292,7 +299,8 @@ fn v1_rotates_to_memo_bound_v2_by_extrinsic() { (CircuitId::TRANSFER, TRANSFER_PUBLIC_INPUTS), (CircuitId::UNSHIELD, UNSHIELD_PUBLIC_INPUTS), ] { - assert_ok!(register(cid, 1, real_vk(base))); + insert_vk(cid, 1, base); + activate(cid, 1); assert_ok!(register(cid, 2, vk_for(cid, 2))); // Registering v2 leaves v1 active until Root switches. assert_eq!(ActiveCircuitVersion::::get(cid), Some(1)); @@ -313,8 +321,8 @@ fn v1_rotates_to_memo_bound_v2_by_extrinsic() { #[test] fn register_vk_rejects_wrong_arity() { new_test_ext().execute_with(|| { - // TRANSFER takes its base arity, memo-bound (+1) or cross-tree (+2); - // anything else is rejected. + // TRANSFER takes memo-bound (+1) or cross-tree (+2); anything else, + // the base arity included, is rejected. assert_noop!( register(CircuitId::TRANSFER, 1, real_vk(TRANSFER_PUBLIC_INPUTS + 3)), Error::::InvalidVerificationKey @@ -323,12 +331,8 @@ fn register_vk_rejects_wrong_arity() { register(CircuitId::UNSHIELD, 1, real_vk(UNSHIELD_PUBLIC_INPUTS + 2)), Error::::InvalidVerificationKey ); - // The matching arity is accepted. - assert_ok!(register( - CircuitId::TRANSFER, - 1, - real_vk(TRANSFER_PUBLIC_INPUTS) - )); + // A matching arity is accepted. + assert_ok!(register(CircuitId::TRANSFER, 1, vk_bytes())); }); } @@ -420,7 +424,7 @@ fn register_vk_different_circuits_are_independent() { assert_ok!(register( CircuitId::UNSHIELD, 1, - real_vk(UNSHIELD_PUBLIC_INPUTS) + real_vk(UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS) )); assert!(VerificationKeys::::contains_key( CircuitId::TRANSFER, @@ -450,7 +454,7 @@ fn register_vk_rejects_duplicate_circuit_version() { #[test] fn register_stores_the_vk_hash() { new_test_ext().execute_with(|| { - let vk = real_vk(TRANSFER_PUBLIC_INPUTS); + let vk = vk_bytes(); assert_ok!(register(CircuitId::TRANSFER, 1, vk.clone())); let expected = sp_io::hashing::blake2_256(vk.as_slice()); assert_eq!( @@ -495,8 +499,16 @@ fn set_active_version_requires_root() { #[test] fn set_active_version_rejects_a_retired_version() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); - insert_vk(CircuitId::TRANSFER, 2, TRANSFER_PUBLIC_INPUTS); + insert_vk( + CircuitId::TRANSFER, + 1, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); + insert_vk( + CircuitId::TRANSFER, + 2, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); RetiredVersions::::insert(CircuitId::TRANSFER, 1, ()); assert_noop!( ZkVerifier::set_active_version(root().into(), CircuitId::TRANSFER, 1), @@ -510,6 +522,19 @@ fn set_active_version_rejects_a_retired_version() { }); } +/// A base spend key already in storage — never registrable, but left by an +/// older runtime or a raw storage write — cannot be made active. +#[test] +fn set_active_version_refuses_a_base_spend_key() { + new_test_ext().execute_with(|| { + insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); + assert_noop!( + ZkVerifier::set_active_version(root().into(), CircuitId::TRANSFER, 1), + Error::::InvalidVerificationKey + ); + }); +} + #[test] fn set_active_version_rejects_non_existent_vk() { new_test_ext().execute_with(|| { @@ -523,8 +548,16 @@ fn set_active_version_rejects_non_existent_vk() { #[test] fn set_active_version_updates_storage_and_emits_event() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); - insert_vk(CircuitId::TRANSFER, 2, TRANSFER_PUBLIC_INPUTS); + insert_vk( + CircuitId::TRANSFER, + 1, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); + insert_vk( + CircuitId::TRANSFER, + 2, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); activate(CircuitId::TRANSFER, 1); assert_ok!(ZkVerifier::set_active_version( @@ -546,8 +579,16 @@ fn set_active_version_updates_storage_and_emits_event() { #[test] fn set_active_version_can_downgrade() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); - insert_vk(CircuitId::TRANSFER, 2, TRANSFER_PUBLIC_INPUTS); + insert_vk( + CircuitId::TRANSFER, + 1, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); + insert_vk( + CircuitId::TRANSFER, + 2, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); activate(CircuitId::TRANSFER, 2); assert_ok!(ZkVerifier::set_active_version( @@ -791,7 +832,11 @@ fn verify_proof_empty_public_inputs_returns_error() { fn verify_proof_happy_path_emits_proof_verified_event() { // In test cfg, do_verify always returns true. new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); + insert_vk( + CircuitId::TRANSFER, + 1, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); activate(CircuitId::TRANSFER, 1); assert_ok!(ZkVerifier::verify_proof( signed().into(), @@ -862,12 +907,13 @@ fn batch_register_rejects_empty_vk() { }); } +/// The batch path holds keys to the same rule: no base spend key, even as v1. #[test] -fn batch_register_refuses_the_base_layout_past_version_one() { +fn batch_register_refuses_a_base_spend_key() { new_test_ext().execute_with(|| { let bad = VkEntry { circuit_id: CircuitId::TRANSFER, - version: 2, + version: 1, verification_key: real_vk(TRANSFER_PUBLIC_INPUTS), set_active: true, }; @@ -1044,8 +1090,16 @@ fn retire_version_rejects_active_and_unknown() { #[test] fn retire_then_unretire_toggles_the_flag() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); - insert_vk(CircuitId::TRANSFER, 2, TRANSFER_PUBLIC_INPUTS); + insert_vk( + CircuitId::TRANSFER, + 1, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); + insert_vk( + CircuitId::TRANSFER, + 2, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); activate(CircuitId::TRANSFER, 1); assert_ok!(ZkVerifier::retire_version( @@ -1383,7 +1437,7 @@ fn runtime_api_circuit_version_info_vk_hashes_are_blake2_256_of_key_data() { activate(CircuitId::TRANSFER, 1); let info = Pallet::::runtime_api_get_circuit_version_info(CircuitId::TRANSFER.0).unwrap(); - let expected_hash = sp_io::hashing::blake2_256(&vk_bytes()); + let expected_hash = sp_io::hashing::blake2_256(&real_vk(TRANSFER_PUBLIC_INPUTS)); assert_eq!(info.vk_hashes[0].vk_hash, expected_hash); assert_eq!(info.vk_hashes[0].version, 1u32); }); @@ -1434,7 +1488,7 @@ fn genesis_registers_vk_at_version_1_and_activates_it() { // Genesis runs the registration checks, so the key must be a real one. verification_keys: vec![( CircuitId::TRANSFER, - real_vk(TRANSFER_PUBLIC_INPUTS).into_inner(), + real_vk(TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS).into_inner(), )], _phantom: Default::default(), } @@ -1459,11 +1513,11 @@ fn genesis_multiple_circuits_are_all_registered() { verification_keys: vec![ ( CircuitId::TRANSFER, - real_vk(TRANSFER_PUBLIC_INPUTS).into_inner(), + real_vk(TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS).into_inner(), ), ( CircuitId::UNSHIELD, - real_vk(UNSHIELD_PUBLIC_INPUTS).into_inner(), + real_vk(UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS).into_inner(), ), ], _phantom: Default::default(), @@ -1521,6 +1575,28 @@ fn genesis_rejects_a_valid_key_with_the_wrong_arity() { .build_storage(); } +/// Genesis with one key of `arity` inputs for `circuit_id`. +fn genesis_with(circuit_id: CircuitId, arity: usize) { + let _ = pallet::GenesisConfig:: { + verification_keys: vec![(circuit_id, real_vk(arity).into_inner())], + _phantom: Default::default(), + } + .build_storage(); +} + +/// A network cannot start with a spend key that binds no memo. +#[test] +#[should_panic(expected = "Genesis VK must deserialize")] +fn genesis_rejects_a_base_transfer_key() { + genesis_with(CircuitId::TRANSFER, TRANSFER_PUBLIC_INPUTS); +} + +#[test] +#[should_panic(expected = "Genesis VK must deserialize")] +fn genesis_rejects_a_base_unshield_key() { + genesis_with(CircuitId::UNSHIELD, UNSHIELD_PUBLIC_INPUTS); +} + // ── integrity_test ──────────────────────────────────────────────────────────── /// The integrity_test must abort when verification is compiled out WITHOUT the diff --git a/frame/zk-verifier/src/verifier.rs b/frame/zk-verifier/src/verifier.rs index c1fb3929..28e9024a 100644 --- a/frame/zk-verifier/src/verifier.rs +++ b/frame/zk-verifier/src/verifier.rs @@ -1,8 +1,8 @@ //! Core Groth16 proof verification. //! //! Resolves the circuit version, loads and prepares its key once, picks the -//! [`InputLayout`] the key implies, and records statistics. Encoding the inputs -//! is the caller's (see [`crate::encoding`]). +//! [`InputLayout`] the key implies — only an admitted one — and records +//! statistics. Encoding the inputs is the caller's (see [`crate::encoding`]). use crate::{ Error, @@ -10,22 +10,23 @@ use crate::{ types::CircuitId, }; use alloc::vec::Vec; -use orbinum_zk_verifier::{Bn254, InputLayout, PreparedVerifyingKey, VerifyingKey, input_layout}; +use orbinum_zk_verifier::{ + Bn254, InputLayout, PreparedVerifyingKey, VerifyingKey, has_memo_layout, input_layout, +}; /// Verify a proof of a circuit statement, encoded for the layout of the key it /// is checked against. /// -/// Returns `(valid, resolved_version)`. A key whose arity fits none of the -/// circuit's layouts, a statement the key cannot attest to (`encode` returns -/// `None`), or inputs that do not fill it, are `valid = false`. +/// Returns `(valid, resolved_version)`. A key of a layout the pallet does not +/// admit, a statement the key cannot attest to (`encode` returns `None`), or +/// inputs that do not fill it, are `valid = false`. pub fn verify_statement( circuit_id: CircuitId, version: Option, proof: &[u8], encode: impl FnOnce(InputLayout) -> Option>, ) -> Result<(bool, u32), sp_runtime::DispatchError> { - check::(circuit_id, version, proof, |arity| { - let layout = input_layout(u8::try_from(circuit_id.0).ok()?, arity)?; + check::(circuit_id, version, proof, |layout, arity| { encode(layout).filter(|raw| raw.len() == arity) }) } @@ -39,16 +40,27 @@ pub fn verify_raw( raw_inputs: Vec<[u8; 32]>, ) -> Result<(bool, u32), sp_runtime::DispatchError> { frame_support::ensure!(!raw_inputs.is_empty(), Error::::EmptyPublicInputs); - check::(circuit_id, version, proof, |_| Some(raw_inputs)) + check::(circuit_id, version, proof, |_, _| Some(raw_inputs)) +} + +/// The layout a key of `arity` inputs gives `circuit_id`, if admitted. +/// +/// A spend circuit never takes the base layout, which binds neither its memos +/// nor the full recipient: such a key verifies nothing, however it reached +/// storage. An id past `u8::MAX` gets none rather than aliasing a known one +/// (`as u8` maps 257 to 1). +pub(crate) fn admitted_layout(circuit_id: CircuitId, arity: usize) -> Option { + let id = u8::try_from(circuit_id.0).ok()?; + input_layout(id, arity).filter(|layout| !(has_memo_layout(id) && *layout == InputLayout::Base)) } -/// Shared path: resolve and prepare the key, build the inputs for its arity, +/// Shared path: resolve and prepare the key, build the inputs for its layout, /// verify, record. `inputs` returns `None` to fail the proof without verifying. fn check( circuit_id: CircuitId, version: Option, proof: &[u8], - inputs: impl FnOnce(usize) -> Option>, + inputs: impl FnOnce(InputLayout, usize) -> Option>, ) -> Result<(bool, u32), sp_runtime::DispatchError> { frame_support::ensure!(!proof.is_empty(), Error::::EmptyProof); let (key_data, resolved) = resolve_key::(circuit_id, version)?; @@ -58,7 +70,9 @@ fn check( let result = match VerifyingKey::new(key_data).prepare() { Ok(pvk) => { let arity = pvk.vk.gamma_abc_g1.len().saturating_sub(1); - inputs(arity).is_some_and(|raw| do_verify(&pvk, proof, raw)) + admitted_layout(circuit_id, arity) + .and_then(|layout| inputs(layout, arity)) + .is_some_and(|raw| do_verify(&pvk, proof, raw)) } Err(_) => false, }; @@ -86,13 +100,11 @@ fn resolve_key( Ok((info.key_data.into_inner(), resolved)) } -/// Record a verification outcome into `VerificationStats`. +/// Count a verification outcome in `VerificationStats`, failures too. /// -/// Failed attempts are counted too. Note the asymmetry between call paths: -/// the `verify_proof` extrinsic returns `Err` on failure, so the runtime -/// reverts this write; the Port path returns `Ok((false, _))`, so the write -/// **persists**. This is deliberate — persisting Port-side failures gives -/// observability into invalid proofs reaching the pool (client bugs / attacks). +/// `verify_proof` returns `Err` on failure, so its write reverts; the port +/// returns `Ok(false)`, so its write persists — on purpose: invalid proofs +/// reaching the pool stay observable. fn record_stats(circuit_id: CircuitId, version: u32, result: bool) { VerificationStats::::mutate(circuit_id, version, |s| { s.total_verifications = s.total_verifications.saturating_add(1); @@ -137,9 +149,11 @@ mod tests { pallet::VerificationStats, }; use frame_support::assert_err; - use orbinum_zk_verifier::{MEMO_HASH_INPUTS, TRANSFER_PUBLIC_INPUTS}; + use orbinum_zk_verifier::{MEMO_HASH_INPUTS, SHIELD_PUBLIC_INPUTS, TRANSFER_PUBLIC_INPUTS}; const BASE: usize = TRANSFER_PUBLIC_INPUTS; + /// A transfer key the pallet admits: the memo-bound layout. + const KEY: usize = BASE + MEMO_HASH_INPUTS; fn proof() -> Vec { vec![0x01; 128] @@ -168,21 +182,40 @@ mod tests { // ── verify_statement ────────────────────────────────────────────────────── #[test] - fn a_base_key_gets_the_base_layout() { + fn a_base_shield_key_gets_the_base_layout() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, BASE); + insert_vk(CircuitId::SHIELD, 1, SHIELD_PUBLIC_INPUTS); let mut seen = None; let res = verify_statement::( - CircuitId::TRANSFER, + CircuitId::SHIELD, Some(1), &proof(), - encoder(&mut seen, BASE), + encoder(&mut seen, SHIELD_PUBLIC_INPUTS), ); assert_eq!(res, Ok((true, 1))); assert_eq!(seen, Some(InputLayout::Base)); }); } + /// A base spend key binds no memo: wherever it came from, it verifies + /// nothing, on either path, and the attempt is counted. + #[test] + fn a_base_spend_key_verifies_nothing() { + new_test_ext().execute_with(|| { + for cid in [CircuitId::TRANSFER, CircuitId::UNSHIELD] { + insert_vk(cid, 1, BASE); + let mut seen = None; + let res = + verify_statement::(cid, Some(1), &proof(), encoder(&mut seen, BASE)); + assert_eq!(res, Ok((false, 1))); + assert_eq!(seen, None); + let raw = verify_raw::(cid, Some(1), &proof(), vec![[0x02; 32]; BASE]); + assert_eq!(raw, Ok((false, 1))); + assert_eq!(stats(cid, 1), (2, 0, 2)); + } + }); + } + #[test] fn a_memo_bound_key_gets_the_memo_bound_layout() { new_test_ext().execute_with(|| { @@ -204,7 +237,7 @@ mod tests { #[test] fn a_statement_the_key_cannot_attest_to_fails_and_is_counted() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, BASE); + insert_vk(CircuitId::TRANSFER, 1, KEY); let res = verify_statement::(CircuitId::TRANSFER, Some(1), &proof(), |_| None); assert_eq!(res, Ok((false, 1))); assert_eq!(stats(CircuitId::TRANSFER, 1), (1, 0, 1)); @@ -231,13 +264,13 @@ mod tests { #[test] fn inputs_that_do_not_fill_the_key_fail() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, BASE); + insert_vk(CircuitId::TRANSFER, 1, KEY); let mut seen = None; let res = verify_statement::( CircuitId::TRANSFER, Some(1), &proof(), - encoder(&mut seen, BASE - 2), + encoder(&mut seen, KEY - 2), ); assert_eq!(res, Ok((false, 1))); }); @@ -298,7 +331,7 @@ mod tests { #[test] fn verify_raw_does_not_encode_inputs() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, BASE); + insert_vk(CircuitId::TRANSFER, 1, KEY); activate(CircuitId::TRANSFER, 1); assert_eq!( verify_raw::(CircuitId::TRANSFER, None, &proof(), vec![[0x02; 32]]), @@ -307,7 +340,7 @@ mod tests { }); } - // ── version resolution ──────────────────────────────────────────────────── + // ── Version resolution ──────────────────────────────────────────────────── #[test] fn no_active_version_is_circuit_not_found() { @@ -343,8 +376,8 @@ mod tests { #[test] fn an_explicit_version_overrides_the_active_one() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, BASE); - insert_vk(CircuitId::TRANSFER, 2, BASE); + insert_vk(CircuitId::TRANSFER, 1, KEY); + insert_vk(CircuitId::TRANSFER, 2, KEY); activate(CircuitId::TRANSFER, 1); let (_, version) = verify_raw::(CircuitId::TRANSFER, Some(2), &proof(), vec![[0x02; 32]]) @@ -353,13 +386,13 @@ mod tests { }); } - // ── statistics ──────────────────────────────────────────────────────────── + // ── Statistics ──────────────────────────────────────────────────────────── #[test] fn stats_count_per_circuit_and_version() { new_test_ext().execute_with(|| { for cid in [CircuitId::TRANSFER, CircuitId::UNSHIELD] { - insert_vk(cid, 1, BASE); + insert_vk(cid, 1, KEY); } let raw = || vec![[0x02; 32]]; verify_raw::(CircuitId::TRANSFER, Some(1), &proof(), raw()).unwrap(); diff --git a/scripts/vk/workflows/setup-dev.sh b/scripts/vk/workflows/setup-dev.sh index 37afc059..61f520cb 100755 --- a/scripts/vk/workflows/setup-dev.sh +++ b/scripts/vk/workflows/setup-dev.sh @@ -21,7 +21,7 @@ set -euo pipefail # # EXAMPLES: # bash scripts/vk/workflows/setup-dev.sh -# bash scripts/vk/workflows/setup-dev.sh ws://127.0.0.1:9944 "//Alice" 1 +# bash scripts/vk/workflows/setup-dev.sh ws://127.0.0.1:9944 "//Alice" 3 # bash scripts/vk/workflows/setup-dev.sh ws://10.0.0.5:9944 "" 2 "@orbinum/circuits@0.4.4" # ============================================================================ diff --git a/template/runtime/RUNTIME_VERSIONS.md b/template/runtime/RUNTIME_VERSIONS.md index 12ca4b72..64eb05f1 100644 --- a/template/runtime/RUNTIME_VERSIONS.md +++ b/template/runtime/RUNTIME_VERSIONS.md @@ -27,7 +27,7 @@ transfer can spend two notes from different trees. `transaction_version` moves: `private_transfer` (call 1) takes `merkle_roots: [Hash; 2]` in place of `merkle_root`. Validator-set only gains calls 6 and 7. Ships with `orbinum-runtime` / `orbinum-node` 0.4.0, `pallet-validator-set` 0.4.0, -`pallet-shielded-pool` 0.22.0, `pallet-zk-verifier` 0.15.0, +`pallet-shielded-pool` 0.23.0, `pallet-zk-verifier` 0.16.0, `orbinum-zk-verifier` 3.0.0 and `pallet-evm-precompile-shielded-pool` 0.9.0; node 0.4.0 is the first binary that declares its version. @@ -83,6 +83,21 @@ code changes: unshield v3 keeps the 8-input memo-bound layout of v2. The fix is in the keys, so **transfer v2 and unshield v2 must be retired as soon as v3 is active.** +#### 4 · Cheaper sealed-tree paths, safer keys + +- **Prune cut 10 → 6** (`SealedTreePrunedBelowLevel`). A sealed tree's Merkle path + rebuilds its pruned siblings from leaves: 62 leaf reads (~11ms in Wasm) instead + of 1_022 (~180ms), at 32_766 stored nodes per tree instead of 2_046. Paths are + public RPC, so that cost was what one request could make a node pay. Lowering + the cut is safe whenever it lands: a node pruned under the old cut is rebuilt + on demand. +- **Node:** the `privacy_getMerkleProof*` RPCs run on blocking threads, at most 4 + at once; the excess is refused as busy (`-32009`) instead of stalling every + other RPC. +- **zk-verifier:** a spend circuit never takes a base-layout key (no memo + binding), at any version: not on registration, activation or unretiring, and a + base key already in storage verifies nothing. + **After the upgrade, by Root:** register the v3 keys from `@orbinum/circuits` **0.17.1** (release ceremony, beacon = testnet block #1190708), activate both with `set_active`, then `retire_version(1, 2)` and `retire_version(2, 2)`. diff --git a/template/runtime/src/configs/privacy.rs b/template/runtime/src/configs/privacy.rs index d2ff5acc..b5be8629 100644 --- a/template/runtime/src/configs/privacy.rs +++ b/template/runtime/src/configs/privacy.rs @@ -1,14 +1,24 @@ //! Orbinum privacy stack: ZK verifier, relayer, and the shielded pool. +//! +//! One `///` line per associated type: what it is wired to, or what the value +//! means. The reasoning behind each bound lives on the pallet's `Config`. use crate::*; use frame_support::parameter_types; +// ─── ZK verifier ────────────────────────────────────────────────────────────── + impl pallet_zk_verifier::Config for Runtime { + /// A compressed Groth16 proof over BN254: exactly 128 bytes. type MaxProofSize = ConstU32<128>; + /// Public inputs `verify_proof` takes at most; the widest circuit has 9. type MaxPublicInputs = ConstU32<32>; + /// Benchmarked weights. type WeightInfo = pallet_zk_verifier::weights::SubstrateWeight; } +// ─── Relayer ────────────────────────────────────────────────────────────────── + /// The current block's author, as `pallet_authorship` reports it. pub struct RelayerBlockAuthor; @@ -19,24 +29,28 @@ impl frame_support::traits::Get> for RelayerBlockAuthor { } impl pallet_relayer::Config for Runtime { + /// Who earns a spend's fee when no relayer committed to it. type BlockAuthor = RelayerBlockAuthor; - /// 0.001 ORB, anti-spam floor. Overridable via `set_min_relay_fee`. + /// The validator set relayers are registered against. + type ValidatorSet = ValidatorSet; + /// Root sets the fee floor and the allowed selectors. + type ManageOrigin = frame_system::EnsureRoot; + /// Fee floor until Root sets one: 0.001 ORB, against spam. type DefaultMinRelayFee = ConstU128<1_000_000_000_000_000>; - /// Ceiling for `set_min_relay_fee`: 1 ORB. Room to react to price swings, far below - /// where a typo would brick relaying until the next runtime upgrade. + /// Highest floor Root may set: 1 ORB, room for price swings but not for typos. type MaxMinRelayFee = ConstU128<1_000_000_000_000_000_000>; - type ManageOrigin = frame_system::EnsureRoot; + /// Precompile selectors the relay may be allowed to call. type MaxAllowedSelectors = ConstU32<16>; - type ValidatorSet = ValidatorSet; - /// A relay commit credits spends in the next 19 blocks (~2 min at 6 s), then - /// expires. Short, since a relayer that commits and never submits holds that - /// spend's fee until then. + /// A commit credits spends for the next 19 blocks (~2 min), then expires. type CommitTtl = ConstU32<20>; - /// Covers a busy relayer's block; keeps the expiry index at ≤ 32 × 64 entries. + /// Commits one relayer may record per block. type MaxCommitsPerRelayerPerBlock = ConstU32<64>; + /// No benchmarked weights yet: the pallet's defaults. type WeightInfo = (); } +// ─── Shielded pool ──────────────────────────────────────────────────────────── + parameter_types! { pub const ShieldedPoolPalletId: PalletId = PalletId(*b"shld/pol"); } @@ -51,26 +65,26 @@ impl sp_runtime::traits::Convert for EvmAccount { } impl pallet_shielded_pool::Config for Runtime { + /// The native token: the pool holds only asset 0. type Currency = Balances; + /// Verifies every shield, transfer and unshield proof. type ZkVerifier = ZkVerifier; + /// Fee floor, relay commits and fee crediting. type Relayer = pallet_relayer::Pallet; + /// Maps an EVM caller to the account it pays from or is paid to. type EvmAccount = EvmAccount; + /// Derives the pool's account, which holds every shielded deposit. type PalletId = ShieldedPoolPalletId; + /// Tree depth, fixed by the circuits (`integrity_test` pins it). type MaxTreeDepth = ConstU32<20>; - /// Safety cap on the historic-root queue, not the retention window: a root expires by - /// elapsed blocks, so `RootRetentionBlocks` is what frees one. Sized for ~27 - /// transfers/block sustained across a full window, well past the ~127 proof - /// verifications a block can fit. - type MaxHistoricRoots = ConstU32<16384>; - /// Roots stay spendable for 300 blocks (~30 min at 6s), comfortably above - /// the 64-block mempool longevity of an unsigned transaction. - type RootRetentionBlocks = ConstU32<300>; - /// Prune sealed trees below level 10: drops 99.8% of their internal nodes - /// (1_048_574 -> 2_046 each) while a Merkle path costs 2^10 leaf reads and - /// 1_023 Poseidon hashes — ~60ms native, ~180ms in Wasm. Level 12 would free - /// only 0.15% more for four times the work. Active trees are never pruned. - type SealedTreePrunedBelowLevel = ConstU8<10>; - /// Pinned to 2^20: clients derive tree_id = leaf_index >> 20 from this. + /// Leaves per tree, pinned to 2^20: clients derive `tree_id = leaf_index >> 20`. type MaxLeavesPerTree = ConstU32<1_048_576>; + /// Sealed trees keep nodes from level 6 up: a path reads 62 leaves, not 1_022. + type SealedTreePrunedBelowLevel = ConstU8<6>; + /// A root stays spendable for 300 blocks (~30 min), past the mempool's 64. + type RootRetentionBlocks = ConstU32<300>; + /// Safety cap on the historic-root queue: ~27 inserts per block over a window. + type MaxHistoricRoots = ConstU32<16384>; + /// Benchmarked weights. type WeightInfo = pallet_shielded_pool::weights::SubstrateWeight; }