From 8d91a8851c191c7fac250731804ca265a7f366a3 Mon Sep 17 00:00:00 2001 From: nol4lej Date: Thu, 8 Oct 2026 18:36:28 -0300 Subject: [PATCH 1/2] feat(shielded-pool)!: private transfer across two trees, spend hardening (circuit v3, spec 18 / tx 6) --- Cargo.lock | 8 +- client/rpc/src/relay/operations.rs | 4 +- client/rpc/src/relay/tests/guard.rs | 39 ++- client/rpc/src/relay/tests/validation.rs | 2 +- client/rpc/src/relay/validation.rs | 2 +- .../evm/precompile/shielded-pool/CHANGELOG.md | 9 + frame/evm/precompile/shielded-pool/Cargo.toml | 2 +- .../src/calls/private_transfer.rs | 31 ++- frame/evm/precompile/shielded-pool/src/lib.rs | 2 +- .../precompile/shielded-pool/src/tests/mod.rs | 38 ++- .../src/tests/private_transfer.rs | 162 +++++++++++- .../shielded-pool/src/tests/relay.rs | 2 +- frame/shielded-pool/CHANGELOG.md | 25 ++ frame/shielded-pool/Cargo.toml | 2 +- frame/shielded-pool/src/benchmarking.rs | 14 +- frame/shielded-pool/src/lib.rs | 10 +- frame/shielded-pool/src/merkle/service.rs | 51 +++- frame/shielded-pool/src/operations/fees.rs | 2 +- frame/shielded-pool/src/operations/mod.rs | 4 +- .../src/operations/private_transfer.rs | 29 ++- frame/shielded-pool/src/operations/shield.rs | 21 +- frame/shielded-pool/src/tests/fees.rs | 28 +- frame/shielded-pool/src/tests/mod.rs | 22 ++ .../src/tests/private_transfer.rs | 244 +++++++++++++++--- frame/shielded-pool/src/tests/unshield.rs | 28 +- .../src/validate_unsigned/tests.rs | 181 ++++++++++++- frame/zk-verifier/CHANGELOG.md | 16 ++ frame/zk-verifier/Cargo.toml | 2 +- frame/zk-verifier/src/encoding.rs | 154 +++++++---- frame/zk-verifier/src/lib.rs | 22 +- frame/zk-verifier/src/port.rs | 72 +++++- frame/zk-verifier/src/tests.rs | 54 +++- frame/zk-verifier/src/verifier.rs | 29 ++- primitives/zk-verifier/CHANGELOG.md | 27 ++ primitives/zk-verifier/Cargo.toml | 2 +- primitives/zk-verifier/src/circuits.rs | 88 +++++-- primitives/zk-verifier/src/lib.rs | 9 +- primitives/zk-verifier/src/types.rs | 156 ++++++++--- scripts/vk/workflows/rotate-dev.sh | 87 +++++-- template/runtime/RUNTIME_VERSIONS.md | 65 ++++- template/runtime/src/lib.rs | 2 +- ts-tests/tests/test-relay-rpc.ts | 6 +- 42 files changed, 1461 insertions(+), 292 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eff625c3..229dc80a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10083,7 +10083,7 @@ dependencies = [ [[package]] name = "orbinum-zk-verifier" -version = "2.1.0" +version = "3.0.0" dependencies = [ "ark-bn254", "ark-ec 0.5.0", @@ -11339,7 +11339,7 @@ dependencies = [ [[package]] name = "pallet-evm-precompile-shielded-pool" -version = "0.8.0" +version = "0.9.0" dependencies = [ "fp-evm", "frame-support", @@ -12408,7 +12408,7 @@ dependencies = [ [[package]] name = "pallet-shielded-pool" -version = "0.21.0" +version = "0.22.0" dependencies = [ "ark-bn254", "ark-ff 0.5.0", @@ -12984,7 +12984,7 @@ dependencies = [ [[package]] name = "pallet-zk-verifier" -version = "0.14.0" +version = "0.15.0" dependencies = [ "ark-bn254", "ark-ec 0.5.0", diff --git a/client/rpc/src/relay/operations.rs b/client/rpc/src/relay/operations.rs index 252c7390..4524e227 100644 --- a/client/rpc/src/relay/operations.rs +++ b/client/rpc/src/relay/operations.rs @@ -101,8 +101,8 @@ impl RelayableOperation for UnshieldOp { } } -/// `privateTransfer(proof, root, nullifiers, commitments, memos, asset_id, fee, -/// circuit_version)` — `0x66ed2cd4` +/// `privateTransfer(proof, merkle_roots, nullifiers, commitments, memos, asset_id, +/// fee, circuit_version)` — `0x63d0b9a0` /// /// Fee is in ABI slot 6: `calldata[196..228]`. The head is 8 slots (256 bytes) /// plus the 4-byte selector = 260 minimum. diff --git a/client/rpc/src/relay/tests/guard.rs b/client/rpc/src/relay/tests/guard.rs index c0dee38c..6e2336ec 100644 --- a/client/rpc/src/relay/tests/guard.rs +++ b/client/rpc/src/relay/tests/guard.rs @@ -27,13 +27,15 @@ fn unshield(nullifier: [u8; 32]) -> Vec { fn transfer(nullifiers: &[[u8; 32]]) -> Vec { let mut data = SELECTOR_PRIVATE_TRANSFER.to_vec(); data.extend_from_slice(&word(256)); // proof offset - data.extend_from_slice(&[0x11; 32]); // root + data.extend_from_slice(&word(256 + 32 + 32 * nullifiers.len())); // roots offset data.extend_from_slice(&word(256)); // nullifiers offset data.resize(4 + 256, 0); data.extend_from_slice(&word(nullifiers.len())); for n in nullifiers { data.extend_from_slice(n); } + data.extend_from_slice(&word(2)); + data.extend_from_slice(&[[0x11; 32], [0x12; 32]].concat()); data } @@ -87,6 +89,41 @@ fn refuses_calldata_that_names_no_note() { assert_eq!(spend_nullifiers(&bad_offset), None, "offset past the input"); } +/// Offsets and lengths at the edge of `usize` read nothing and never overflow: +/// the guard runs on calldata from anyone, before the dry-run. +#[test] +fn hostile_offsets_and_lengths_read_nothing() { + let near_max = U256::from(usize::MAX - 16).to_big_endian(); + let mut offset = transfer(&[[0x33; 32]]); + offset[4 + 64..4 + 96].copy_from_slice(&near_max); + assert_eq!(spend_nullifiers(&offset), None); + + let mut length = transfer(&[[0x33; 32]]); + length[4 + 256..4 + 288].copy_from_slice(&near_max); + assert_eq!(spend_nullifiers(&length), None); + + let mut short = transfer(&[[0x33; 32], [0x44; 32]]); + short.truncate(4 + 256 + 32 + 32); + assert_eq!(spend_nullifiers(&short), None, "second nullifier missing"); +} + +/// The nullifiers come from head slot 2 whatever slot 1 holds: in the +/// two-root layout slot 1 is the roots' offset, never a nullifier. +#[test] +fn the_roots_array_is_never_read_as_nullifiers() { + let mut data = transfer(&[[0x33; 32], [0x44; 32]]); + // Point the roots at the nullifiers and the nullifiers at the roots. + let roots = data[4 + 32..4 + 64].to_vec(); + let nullifiers = data[4 + 64..4 + 96].to_vec(); + data[4 + 32..4 + 64].copy_from_slice(&nullifiers); + data[4 + 64..4 + 96].copy_from_slice(&roots); + assert_eq!( + spend_nullifiers(&data), + Some(vec![[0x11; 32], [0x12; 32]]), + "slot 2 decides, as it does for the precompile" + ); +} + // ── InFlight ───────────────────────────────────────────────────────────────── const TTL: Duration = Duration::from_secs(120); diff --git a/client/rpc/src/relay/tests/validation.rs b/client/rpc/src/relay/tests/validation.rs index 0a687ec7..5ad9c082 100644 --- a/client/rpc/src/relay/tests/validation.rs +++ b/client/rpc/src/relay/tests/validation.rs @@ -78,7 +78,7 @@ fn rejects_calldata_that_ends_before_the_fee_slot() { #[test] fn selectors_are_keccak_of_the_abi_signatures() { let pt = sp_io::hashing::keccak_256( - b"privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)", + b"privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)", ); let un = sp_io::hashing::keccak_256( b"unshield(bytes,bytes32,bytes32,uint32,uint256,bytes32,uint256,bytes32,bytes,uint32)", diff --git a/client/rpc/src/relay/validation.rs b/client/rpc/src/relay/validation.rs index 17675e53..7a803a51 100644 --- a/client/rpc/src/relay/validation.rs +++ b/client/rpc/src/relay/validation.rs @@ -43,7 +43,7 @@ pub(crate) fn compute_effective_min_fee(min_fee_planck: u128, base_fee_wei: u128 /// ```text /// bytes [0..4] selector /// bytes [4..36] slot 0 — offset pointer for proof (bytes/dynamic) -/// bytes [36..68] slot 1 — bytes32 root +/// bytes [36..68] slot 1 — bytes32 root / bytes32[] roots offset /// bytes [68..100] slot 2 — bytes32 nullifier / bytes32[] nullifiers offset /// bytes [100..132] slot 3 — uint32 asset_id / bytes32[] commits offset /// bytes [132..164] slot 4 — uint256 amount / bytes[] memos offset diff --git a/frame/evm/precompile/shielded-pool/CHANGELOG.md b/frame/evm/precompile/shielded-pool/CHANGELOG.md index db32edd8..1a4dca24 100644 --- a/frame/evm/precompile/shielded-pool/CHANGELOG.md +++ b/frame/evm/precompile/shielded-pool/CHANGELOG.md @@ -4,6 +4,15 @@ All notable changes to `pallet-evm-precompile-shielded-pool` will be documented ## [Unreleased] +## [0.9.0] - 2026-10-08 + +### Changed + +- **Breaking:** `privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)`, + selector `0x63d0b9a0`: slot 1 is the offset of `merkle_roots`, exactly two + roots in input order. The single-root selector `0x66ed2cd4` is removed. The fee + stays at slot 6, so the relay RPC's gate is unchanged. + ## [0.8.0] - 2026-10-05 **Breaking** — the shield ABI changes. Requires the matching `pallet-shielded-pool`. diff --git a/frame/evm/precompile/shielded-pool/Cargo.toml b/frame/evm/precompile/shielded-pool/Cargo.toml index f198e253..8e4651aa 100644 --- a/frame/evm/precompile/shielded-pool/Cargo.toml +++ b/frame/evm/precompile/shielded-pool/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-evm-precompile-shielded-pool" -version = "0.8.0" +version = "0.9.0" authors = { workspace = true } edition = "2021" description = "EVM Precompile for Orbinum Shielded Pool Pallet." diff --git a/frame/evm/precompile/shielded-pool/src/calls/private_transfer.rs b/frame/evm/precompile/shielded-pool/src/calls/private_transfer.rs index 73305c95..36c93a85 100644 --- a/frame/evm/precompile/shielded-pool/src/calls/private_transfer.rs +++ b/frame/evm/precompile/shielded-pool/src/calls/private_transfer.rs @@ -1,15 +1,15 @@ //! ABI decoding and call construction for -//! `privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)`. +//! `privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)`. //! //! ## Selector -//! `keccak256("privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)")[0..4]` -//! = `0x66ed2cd4` +//! `keccak256("privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)")[0..4]` +//! = `0x63d0b9a0` //! //! ## ABI layout (`input[4..]`) -//! | Slot (bytes) | Type | Field | -//! |--------------|-----------|-----------------------| -//! | 0..32 | `uint256` | offset → `proof` | -//! | 32..64 | `bytes32` | `merkle_root` | +//! | Slot (bytes) | Type | Field | +//! |--------------|-----------|------------------------| +//! | 0..32 | `uint256` | offset → `proof` | +//! | 32..64 | `uint256` | offset → `merkle_roots`| //! | 64..96 | `uint256` | offset → `nullifiers` | //! | 96..128 | `uint256` | offset → `commitments`| //! | 128..160 | `uint256` | offset → `memos` | @@ -18,7 +18,10 @@ //! | 224..256 | `uint32` | `circuit_version` | //! //! ## Notes -//! The three arrays are parallel: `commitments[i]` and `memos[i]` describe the +//! `merkle_roots` holds exactly two roots, one per input note in input order; +//! they differ when the notes come from different trees. +//! +//! The three other arrays are parallel: `commitments[i]` and `memos[i]` describe the //! output note paid for by `nullifiers[i]`, so all three must have equal length. //! //! The relay fee recipient is not in the ABI: it is the relayer that recorded a @@ -36,7 +39,7 @@ use super::{balance, params, proof}; use crate::{abi, revert}; /// Selector for the signature in this module's header. -pub const SELECTOR: [u8; 4] = [0x66, 0xed, 0x2c, 0xd4]; +pub const SELECTOR: [u8; 4] = [0x63, 0xd0, 0xb9, 0xa0]; /// Maximum number of input nullifiers / output commitments in a single transfer. pub const MAX_NOTES: u32 = 2; @@ -61,8 +64,12 @@ where "privateTransfer: proof must be non-empty", )?; - // Step 3: merkle_root the proof is verified against. - let merkle_root: pallet_shielded_pool::Hash = abi::read_bytes32(params, 32)?; + // Step 3: one root per input note, in input order. The circuit always has two + // inputs (a dummy pads a one-note spend), so exactly two roots. + let merkle_roots: [pallet_shielded_pool::Hash; 2] = + abi::decode_bytes32_array_at_slot(params, 32, 2)? + .try_into() + .map_err(|_| revert("privateTransfer: exactly two merkle roots required"))?; // Step 4: the three parallel arrays — nullifiers spent, commitments created, // and the memo carrying each new note's secrets. The decoders bound count and @@ -127,7 +134,7 @@ where Ok(pallet_shielded_pool::Call::::private_transfer { proof, - merkle_root, + merkle_roots, nullifiers, commitments, encrypted_memos, diff --git a/frame/evm/precompile/shielded-pool/src/lib.rs b/frame/evm/precompile/shielded-pool/src/lib.rs index c33ad327..a0f2668a 100644 --- a/frame/evm/precompile/shielded-pool/src/lib.rs +++ b/frame/evm/precompile/shielded-pool/src/lib.rs @@ -44,7 +44,7 @@ pub use calls::private_transfer::MAX_NOTES as MAX_SPEND_INPUTS; /// | Selector | Solidity signature | /// |--------------|---------------------------------------------------------------------------------------| /// | `0xf25897e0` | `shield(uint32,bytes32,bytes,bytes,uint32)` — payable, amount = `msg.value` | -/// | `0x66ed2cd4` | `privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)` | +/// | `0x63d0b9a0` | `privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)` | /// | `0x4e505348` | `unshield(bytes,bytes32,bytes32,uint32,uint256,bytes32,uint256,bytes32,bytes,uint32)` | /// | `0xc9b235ff` | `commitRelay(bytes32[])` — registered relayers | /// | `0x2a3274dd` | `claimRelayFees(uint32,uint256)` — registered relayers | diff --git a/frame/evm/precompile/shielded-pool/src/tests/mod.rs b/frame/evm/precompile/shielded-pool/src/tests/mod.rs index 5a347923..956b38f4 100644 --- a/frame/evm/precompile/shielded-pool/src/tests/mod.rs +++ b/frame/evm/precompile/shielded-pool/src/tests/mod.rs @@ -158,8 +158,8 @@ fn encode_shield_with( input } -/// `privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)`, -/// selector `0x66ed2cd4`. +/// `privateTransfer` with both inputs proven against `merkle_root`, as a +/// same-tree spend submits it. #[allow(clippy::too_many_arguments)] fn encode_private_transfer( proof: &[u8], @@ -170,8 +170,34 @@ fn encode_private_transfer( asset_id: u32, fee: u128, circuit_version: u32, +) -> Vec { + encode_private_transfer_roots( + proof, + &[merkle_root, merkle_root], + nullifiers, + commitments, + memos, + asset_id, + fee, + circuit_version, + ) +} + +/// `privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)`, +/// selector `0x63d0b9a0`, with any number of roots so malformed counts can be built. +#[allow(clippy::too_many_arguments)] +fn encode_private_transfer_roots( + proof: &[u8], + merkle_roots: &[[u8; 32]], + nullifiers: &[[u8; 32]], + commitments: &[[u8; 32]], + memos: &[Vec], + asset_id: u32, + fee: u128, + circuit_version: u32, ) -> Vec { let proof_enc = encode_bytes(proof); + let roots_enc = encode_bytes32_array(merkle_roots); let nullifiers_enc = encode_bytes32_array(nullifiers); let commitments_enc = encode_bytes32_array(commitments); let memos_enc = encode_bytes_array(memos); @@ -179,14 +205,15 @@ fn encode_private_transfer( // head: 8 slots × 32 = 256 bytes let head_size = 256usize; let off_proof = head_size; - let off_nullifiers = off_proof + proof_enc.len(); + let off_roots = off_proof + proof_enc.len(); + let off_nullifiers = off_roots + roots_enc.len(); let off_commitments = off_nullifiers + nullifiers_enc.len(); let off_memos = off_commitments + commitments_enc.len(); - let mut input = vec![0x66, 0xed, 0x2c, 0xd4]; + let mut input = crate::calls::private_transfer::SELECTOR.to_vec(); let mut head = vec![0u8; head_size]; head[0..32].copy_from_slice(&u256_word(off_proof)); - head[32..64].copy_from_slice(&merkle_root); + head[32..64].copy_from_slice(&u256_word(off_roots)); head[64..96].copy_from_slice(&u256_word(off_nullifiers)); head[96..128].copy_from_slice(&u256_word(off_commitments)); head[128..160].copy_from_slice(&u256_word(off_memos)); @@ -196,6 +223,7 @@ fn encode_private_transfer( input.extend_from_slice(&head); input.extend_from_slice(&proof_enc); + input.extend_from_slice(&roots_enc); input.extend_from_slice(&nullifiers_enc); input.extend_from_slice(&commitments_enc); input.extend_from_slice(&memos_enc); diff --git a/frame/evm/precompile/shielded-pool/src/tests/private_transfer.rs b/frame/evm/precompile/shielded-pool/src/tests/private_transfer.rs index 9544ff31..14b8b257 100644 --- a/frame/evm/precompile/shielded-pool/src/tests/private_transfer.rs +++ b/frame/evm/precompile/shielded-pool/src/tests/private_transfer.rs @@ -17,11 +17,139 @@ fn private_transfer_rejects_truncated_input() { fn private_transfer_selector_matches_signature() { // The constant must be derived from the ABI signature — this is the guard // against a hand-written selector that never matches the code. - let sig = b"privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)"; + let sig = b"privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)"; let hash = sp_io::hashing::keccak_256(sig); assert_eq!(hash[..4], crate::calls::private_transfer::SELECTOR); } +/// Two roots decode into the call in input order, so a spend across two trees +/// reaches the pallet with each input's own root. +#[test] +fn private_transfer_decodes_one_root_per_input() { + let input = super::encode_private_transfer_roots( + &[0x01], + &[[0x0A; 32], [0x0B; 32]], + &[[0x11; 32], [0x22; 32]], + &[canon(0x33), canon(0x44)], + &[vec![0u8; 180], vec![0u8; 180]], + 0, + 0, + 3, + ); + let Some(pallet_shielded_pool::Call::private_transfer { merkle_roots, .. }) = + crate::decode_relayable_call::(&input) + else { + panic!("privateTransfer must decode"); + }; + assert_eq!(merkle_roots, [[0x0A; 32], [0x0B; 32]]); +} + +/// The circuit has two inputs, so anything but exactly two roots is malformed. +#[test] +fn private_transfer_rejects_any_root_count_but_two() { + new_test_ext().execute_with(|| { + for roots in [vec![], vec![[0x0A; 32]], vec![[0x0A; 32]; 3]] { + let input = super::encode_private_transfer_roots( + &[0x01], + &roots, + &[[0x11; 32], [0x22; 32]], + &[canon(0x33), canon(0x44)], + &[vec![0u8; 180], vec![0u8; 180]], + 0, + 0, + 3, + ); + assert!(crate::decode_relayable_call::(&input).is_none()); + let mut h = MockHandle::new(input); + expect_error(ShieldedPoolPrecompile::::execute(&mut h)); + } + }); +} + +/// Hostile `merkle_roots` pointers: an offset past the calldata or at +/// `u256::MAX`, and a length of `u256::MAX` or one that runs past the end, each +/// revert before anything is allocated or read out of bounds. +#[test] +fn private_transfer_rejects_hostile_root_offsets_and_lengths() { + new_test_ext().execute_with(|| { + let valid = super::encode_private_transfer_roots( + &[0x01], + &[[0x0A; 32], [0x0B; 32]], + &[[0x11; 32], [0x22; 32]], + &[canon(0x33), canon(0x44)], + &[vec![0u8; 180], vec![0u8; 180]], + 0, + 0, + 3, + ); + // `input[4..]` is the params; slot 1 (bytes 32..64) holds the roots offset. + let offset_low: [u8; 8] = valid[4 + 56..4 + 64].try_into().unwrap(); + let roots_at = 4 + u64::from_be_bytes(offset_low) as usize; + let mut cases = Vec::new(); + for offset in [vec![0xFF; 32], super::u256_word(valid.len() + 64).to_vec()] { + let mut input = valid.clone(); + input[4 + 32..4 + 64].copy_from_slice(&offset); + cases.push(input); + } + for length in [vec![0xFF; 32], super::u256_word(1 << 20).to_vec()] { + let mut input = valid.clone(); + input[roots_at..roots_at + 32].copy_from_slice(&length); + cases.push(input); + } + for input in cases { + assert!(crate::decode_relayable_call::(&input).is_none()); + let mut h = MockHandle::new(input); + expect_error(ShieldedPoolPrecompile::::execute(&mut h)); + } + }); +} + +/// ABI offsets may alias: pointing `merkle_roots` at the nullifiers array +/// decodes the nullifiers as roots. Harmless: the pallet then needs them to be +/// known roots and the proof to bind them. +#[test] +fn an_aliased_roots_pointer_reads_the_aliased_words_as_roots() { + let mut input = super::encode_private_transfer_roots( + &[0x01], + &[[0x0A; 32], [0x0B; 32]], + &[[0x11; 32], [0x22; 32]], + &[canon(0x33), canon(0x44)], + &[vec![0u8; 180], vec![0u8; 180]], + 0, + 0, + 3, + ); + let nullifiers_offset = input[4 + 64..4 + 96].to_vec(); + input[4 + 32..4 + 64].copy_from_slice(&nullifiers_offset); + let Some(pallet_shielded_pool::Call::private_transfer { merkle_roots, .. }) = + crate::decode_relayable_call::(&input) + else { + panic!("an aliased pointer is still well-formed ABI"); + }; + assert_eq!(merkle_roots, [[0x11; 32], [0x22; 32]]); +} + +/// The single-root ABI is gone: its selector is no longer a call. +#[test] +fn the_single_root_selector_is_refused() { + new_test_ext().execute_with(|| { + let mut input = super::encode_private_transfer( + &[0x01], + canon(0x0A), + &[[0x11; 32], [0x22; 32]], + &[canon(0x33), canon(0x44)], + &[vec![0u8; 180], vec![0u8; 180]], + 0, + 0, + 2, + ); + input[..4].copy_from_slice(&[0x66, 0xed, 0x2c, 0xd4]); + assert!(crate::decode_relayable_call::(&input).is_none()); + let mut h = MockHandle::new(input); + expect_error(ShieldedPoolPrecompile::::execute(&mut h)); + }); +} + #[test] fn private_transfer_rejects_empty_proof() { new_test_ext().execute_with(|| { @@ -107,6 +235,38 @@ fn private_transfer_rejects_mismatched_commitment_memo_count() { }); } +/// Two notes from two trees end to end: one root a sealed tree's, the other the +/// active tree's. Both reach the pallet in order and the spend settles. +#[test] +fn private_transfer_across_two_trees_executes() { + new_test_ext().execute_with(|| { + do_shield(canon(0x56), 5_000); + let sealed = canon(0x5E); + pallet_shielded_pool::SealedRootIndex::::insert(sealed, 0); + let active = current_root(); + let nullifiers = [[0x13; 32], [0x24; 32]]; + + let input = super::encode_private_transfer_roots( + &[0x01, 0x02, 0x03], + &[sealed, active], + &nullifiers, + &[canon(0x35), canon(0x46)], + &[vec![0xAA; 180], vec![0xBB; 180]], + 0, + 0, + 3, + ); + let mut h = MockHandle::new(input); + assert_success(ShieldedPoolPrecompile::::execute(&mut h)); + for n in nullifiers { + assert!(pallet_shielded_pool::NullifierSet::::get( + pallet_shielded_pool::Nullifier::from(n) + ) + .is_some()); + } + }); +} + #[test] fn private_transfer_happy_path() { new_test_ext().execute_with(|| { diff --git a/frame/evm/precompile/shielded-pool/src/tests/relay.rs b/frame/evm/precompile/shielded-pool/src/tests/relay.rs index a54f91ec..38a02438 100644 --- a/frame/evm/precompile/shielded-pool/src/tests/relay.rs +++ b/frame/evm/precompile/shielded-pool/src/tests/relay.rs @@ -161,7 +161,7 @@ fn relayable_transfer_calldata_hashes_like_the_extrinsic() { ); let call = crate::decode_relayable_call::(&input).expect("privateTransfer decodes"); let expected = pallet_shielded_pool::operations::private_transfer::TransferRequest:: { - merkle_root: canon(0xBB), + merkle_roots: [canon(0xBB); 2], nullifiers: vec![pallet_shielded_pool::Nullifier(canon(1))] .try_into() .unwrap(), diff --git a/frame/shielded-pool/CHANGELOG.md b/frame/shielded-pool/CHANGELOG.md index ffcadd4d..a802664c 100644 --- a/frame/shielded-pool/CHANGELOG.md +++ b/frame/shielded-pool/CHANGELOG.md @@ -4,6 +4,31 @@ All notable changes to `pallet-shielded-pool` will be documented in this file. ## [Unreleased] +## [0.22.0] - 2026-10-08 + +### Changed + +- **Breaking:** `private_transfer` takes `merkle_roots: [Hash; 2]` in place of + `merkle_root` — one root per input, so two notes from different trees can be + spent together (transfer circuit v3). `transaction_version` moves. +- Every root must be known (active, recent or sealed); otherwise + `UnknownMerkleRoot`, at dispatch and at pool admission (code 1). +- A dummy input's root must repeat the real input's (the circuit leaves it + free); otherwise `InvalidPublicSignals`, at pool admission code 13. Equal + roots are looked up once. +- The `private_transfer` benchmark spends from two sealed trees, the costliest + root lookups; weights need regenerating. +- `transfer_op_hash` hashes both roots, for every circuit version. + +### Fixed + +- Merkle paths of a sealed tree when `MaxLeavesPerTree` is below 2^depth: the + pruned siblings were recomputed from global leaf indices past the tree's + capacity, which belong to the next tree, so once that tree held leaves the + path no longer matched the sealed root. A subtree past the capacity now reads + as empty. Production uses 2^20 = 2^depth and was not affected; test and dev + configurations with small trees were. + ## [0.21.0] - 2026-10-05 A shield must prove its note is worth exactly the deposit. Closes AL-1: a deposit diff --git a/frame/shielded-pool/Cargo.toml b/frame/shielded-pool/Cargo.toml index 0bdadc45..e66c246a 100644 --- a/frame/shielded-pool/Cargo.toml +++ b/frame/shielded-pool/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-shielded-pool" -version = "0.21.0" +version = "0.22.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/benchmarking.rs b/frame/shielded-pool/src/benchmarking.rs index e0aacd2d..29c2773c 100644 --- a/frame/shielded-pool/src/benchmarking.rs +++ b/frame/shielded-pool/src/benchmarking.rs @@ -173,10 +173,12 @@ mod benchmarks { #[benchmark] fn private_transfer() { let (_caller, _) = setup_benchmark_env::(); - let merkle_root = [1u8; 32]; - - // Setup valid root in storage - crate::storage::MerkleRepository::add_historic_poseidon_root::(merkle_root); + // Two notes from two sealed trees: each root misses the active and + // historic lookups and is found last, in `SealedRootIndex`. + let merkle_roots = [[1u8; 32], [2u8; 32]]; + for (tree_id, root) in (0u32..).zip(merkle_roots) { + crate::storage::MerkleRepository::insert_sealed_root::(tree_id, root); + } let proof: Proof = vec![0u8; 128].try_into().unwrap(); @@ -207,7 +209,7 @@ mod benchmarks { // Must be >= T::Relayer::min_relay_fee() to pass the FeeTooLow check. let fee: BalanceOf = T::Relayer::min_relay_fee().saturated_into(); let op_hash = crate::operations::private_transfer::TransferRequest:: { - merkle_root, + merkle_roots, nullifiers: nullifiers.clone(), commitments: commitments.clone(), memos: encrypted_memos.clone(), @@ -222,7 +224,7 @@ mod benchmarks { private_transfer( crate::RawOrigin::Relayed(relayer), proof, - merkle_root, + merkle_roots, nullifiers, commitments, encrypted_memos, diff --git a/frame/shielded-pool/src/lib.rs b/frame/shielded-pool/src/lib.rs index bed93910..4e21015b 100644 --- a/frame/shielded-pool/src/lib.rs +++ b/frame/shielded-pool/src/lib.rs @@ -857,7 +857,9 @@ pub mod pallet { /// # Arguments /// * `origin` - Unsigned, signed, or the precompile's relayed origin /// * `proof` - The ZK proof of valid transfer - /// * `merkle_root` - The Merkle root the proof was computed against + /// * `merkle_roots` - The root each input is proven against, in input order. + /// Equal unless the notes come from different trees (circuit v3); a + /// dummy input's slot repeats the real root /// * `nullifiers` - Nullifiers for notes being spent /// * `commitments` - Commitments for new notes being created /// * `encrypted_memos` - Encrypted metadata for each new note @@ -867,7 +869,7 @@ pub mod pallet { /// /// # Errors /// * `TooManyInputsOrOutputs` - Not two nullifiers and two commitments - /// * `UnknownMerkleRoot` - Root is not in historic roots + /// * `UnknownMerkleRoot` - Any root is not a known one (active, recent or sealed) /// * `NullifierAlreadyUsed` - Double-spend attempt /// * `ProofVerificationFailed` - ZK proof verification failed /// * `FeeTooLow` - Fee is below `T::Relayer::min_relay_fee()` @@ -881,7 +883,7 @@ pub mod pallet { pub fn private_transfer( origin: OriginFor, proof: Proof, - merkle_root: Hash, + merkle_roots: [Hash; 2], nullifiers: BoundedVec>, commitments: BoundedVec>, encrypted_memos: BoundedVec>, @@ -894,7 +896,7 @@ pub mod pallet { PrivateTransferOperation::execute::( &proof, TransferRequest { - merkle_root, + merkle_roots, nullifiers, commitments, memos: encrypted_memos, diff --git a/frame/shielded-pool/src/merkle/service.rs b/frame/shielded-pool/src/merkle/service.rs index b1d8b90c..db1a63c7 100644 --- a/frame/shielded-pool/src/merkle/service.rs +++ b/frame/shielded-pool/src/merkle/service.rs @@ -263,15 +263,23 @@ impl MerkleTreeService { return get_zero_hash_cached(level); } let span = 1u32 << level; - let base = tree_id - .saturating_mul(cap) - .saturating_add(node_index.saturating_mul(span)); + // 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. + 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. A gap means an empty slot, which the - // tree represents with the level-0 zero hash. + // 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. + let in_tree = span.min(cap - offset); let mut nodes: Vec = (0..span) .map(|i| { - MerkleRepository::get_leaf::(base.saturating_add(i)) + (i < in_tree) + .then(|| MerkleRepository::get_leaf::(base.saturating_add(i))) + .flatten() .map(|c| c.0) .unwrap_or_else(|| get_zero_hash_cached(0)) }) @@ -374,3 +382,34 @@ impl MerkleTreeService { }) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::{Test, new_test_ext}; + + /// 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. + #[test] + fn a_sibling_subtree_past_the_tree_capacity_is_empty() { + new_test_ext().execute_with(|| { + let cap = ::MaxLeavesPerTree::get(); + for i in 0..(2 * cap) { + MerkleTreeService::insert_leaf::(Commitment::new([i as u8 + 1; 32])).unwrap(); + } + // Level log2(cap) node 1 of tree 0 would cover global leaves cap..2·cap, + // all of them tree 1's. + let level = cap.trailing_zeros() as usize; + assert_eq!( + MerkleTreeService::subtree_root::(0, level, 1, cap), + get_zero_hash_cached(level) + ); + // Inside the tree the subtree is the leaves' own fold, not zero. + assert_ne!( + MerkleTreeService::subtree_root::(0, level, 0, cap), + get_zero_hash_cached(level) + ); + }); + } +} diff --git a/frame/shielded-pool/src/operations/fees.rs b/frame/shielded-pool/src/operations/fees.rs index a72c3373..7a369f6b 100644 --- a/frame/shielded-pool/src/operations/fees.rs +++ b/frame/shielded-pool/src/operations/fees.rs @@ -57,7 +57,7 @@ pub fn transfer_op_hash(s: &TransferStatement, circuit_version: u32) -> [u8; 32] sp_io::hashing::blake2_256( &( TRANSFER_OP_DOMAIN, - s.merkle_root, + s.merkle_roots, &s.nullifiers, &s.commitments, s.asset_id, diff --git a/frame/shielded-pool/src/operations/mod.rs b/frame/shielded-pool/src/operations/mod.rs index 060e01c8..9d29fdc4 100644 --- a/frame/shielded-pool/src/operations/mod.rs +++ b/frame/shielded-pool/src/operations/mod.rs @@ -32,7 +32,7 @@ impl SpendRequest { Some(match call.clone() { Call::private_transfer { proof, - merkle_root, + merkle_roots, nullifiers, commitments, encrypted_memos, @@ -42,7 +42,7 @@ impl SpendRequest { } => ( proof.into_inner(), Self::Transfer(TransferRequest { - merkle_root, + merkle_roots, nullifiers, commitments, memos: encrypted_memos, diff --git a/frame/shielded-pool/src/operations/private_transfer.rs b/frame/shielded-pool/src/operations/private_transfer.rs index 7f2fd375..367fd974 100644 --- a/frame/shielded-pool/src/operations/private_transfer.rs +++ b/frame/shielded-pool/src/operations/private_transfer.rs @@ -23,7 +23,8 @@ pub const TRANSFER_OUTPUTS: usize = 2; /// the circuit skips. #[derive(CloneNoBound, PartialEqNoBound, EqNoBound, DebugNoBound)] pub struct TransferRequest { - pub merkle_root: [u8; 32], + /// The root each input is proven against, in input order. + pub merkle_roots: [[u8; 32]; TRANSFER_INPUTS], pub nullifiers: BoundedVec>, pub commitments: BoundedVec>, /// One per commitment, in the same order. @@ -38,7 +39,7 @@ impl TransferRequest { /// What the proof attests to. pub fn statement(&self) -> TransferStatement { TransferStatement { - merkle_root: self.merkle_root, + merkle_roots: self.merkle_roots, nullifiers: self.nullifiers.iter().map(|n| n.0).collect(), commitments: self.commitments.iter().map(|c| c.0).collect(), asset_id: self.asset_id, @@ -56,6 +57,25 @@ impl TransferRequest { pub(crate) fn real_nullifiers(&self) -> impl Iterator { self.nullifiers.iter().filter(|n| n.0 != [0u8; 32]) } + + /// The roots are sound: each one is known, and a dummy input repeats the + /// real input's root. The circuit leaves a dummy's root free, so without the + /// second rule that slot would carry a value of the submitter's choosing. + /// Roots differ only when both inputs are real, each from its own tree. + pub(crate) fn ensure_roots(&self) -> Result<(), Error> { + let [root0, root1] = &self.merkle_roots; + let has_dummy = self.real_nullifiers().count() < TRANSFER_INPUTS; + ensure!( + root0 == root1 || !has_dummy, + Error::::InvalidPublicSignals + ); + ensure!( + MerkleRepository::is_known_root::(root0) + && (root0 == root1 || MerkleRepository::is_known_root::(root1)), + Error::::UnknownMerkleRoot + ); + Ok(()) + } } pub struct PrivateTransferOperation; @@ -98,10 +118,7 @@ impl PrivateTransferOperation { req.fee >= T::Relayer::min_relay_fee().saturated_into(), Error::::FeeTooLow ); - ensure!( - MerkleRepository::is_known_root::(&req.merkle_root), - Error::::UnknownMerkleRoot - ); + req.ensure_roots()?; for nullifier in req.real_nullifiers() { ensure!(nullifier.is_canonical(), Error::::InvalidPublicSignals); diff --git a/frame/shielded-pool/src/operations/shield.rs b/frame/shielded-pool/src/operations/shield.rs index abb52140..24f84e65 100644 --- a/frame/shielded-pool/src/operations/shield.rs +++ b/frame/shielded-pool/src/operations/shield.rs @@ -254,30 +254,11 @@ mod tests { #[test] fn execute_non_canonical_commitment_fails() { new_test_ext().execute_with(|| { - use ark_bn254::Fr; - use ark_ff::{BigInteger, PrimeField}; - let asset_id = setup_asset(); let mut canonical = [0u8; 32]; canonical[0] = 7; - - // n + p: same field element, different bytes. - let p_minus_1 = (-Fr::from(1u64)).into_bigint().to_bytes_le(); - let mut twin = [0u8; 32]; - twin[..p_minus_1.len()].copy_from_slice(&p_minus_1); - let mut carry = 1u16 + 7; - for b in twin.iter_mut() { - let v = *b as u16 + carry; - *b = (v & 0xff) as u8; - carry = v >> 8; - } - - assert_eq!( - Fr::from_le_bytes_mod_order(&canonical), - Fr::from_le_bytes_mod_order(&twin), - "the pair must reduce to one element, or this proves nothing" - ); + let twin = crate::tests::field_twin(canonical); assert_ok!(execute( acc(1), diff --git a/frame/shielded-pool/src/tests/fees.rs b/frame/shielded-pool/src/tests/fees.rs index 2d3ba858..50e1a883 100644 --- a/frame/shielded-pool/src/tests/fees.rs +++ b/frame/shielded-pool/src/tests/fees.rs @@ -43,7 +43,7 @@ fn unshield() -> UnshieldStatement { fn transfer() -> TransferStatement { TransferStatement { - merkle_root: ROOT, + merkle_roots: [ROOT; 2], nullifiers: vec![NULL], commitments: vec![CHANGE], asset_id: 0, @@ -94,12 +94,32 @@ fn every_public_value_changes_the_unshield_hash() { assert_ne!(unshield_op_hash(&unshield(), 2), base, "circuit version"); } +/// The roots are bound in input order: a commit to one order is not a commit +/// to the swapped one, so a relayer cannot claim a spend it did not commit to +/// by reordering its roots. +#[test] +fn the_transfer_hash_binds_the_order_of_the_roots() { + let ab = TransferStatement { + merkle_roots: [[0x0A; 32], [0x0B; 32]], + ..transfer() + }; + let ba = TransferStatement { + merkle_roots: [[0x0B; 32], [0x0A; 32]], + ..transfer() + }; + assert_ne!(transfer_op_hash(&ab, 3), transfer_op_hash(&ba, 3)); +} + #[test] fn every_public_value_changes_the_transfer_hash() { let base = transfer_op_hash(&transfer(), 1); let variants = [ TransferStatement { - merkle_root: [0xFF; 32], + merkle_roots: [[0xFF; 32], ROOT], + ..transfer() + }, + TransferStatement { + merkle_roots: [ROOT, [0xFF; 32]], ..transfer() }, TransferStatement { @@ -203,7 +223,7 @@ fn relay_op_hash_is_the_extrinsics_own_hash() { let transfer_call = Call::::private_transfer { proof: vec![0xAB; 64].try_into().unwrap(), - merkle_root: ROOT, + merkle_roots: [ROOT; 2], nullifiers: vec![Nullifier(NULL)].try_into().unwrap(), commitments: vec![Commitment(CHANGE)].try_into().unwrap(), encrypted_memos: vec![Default::default()].try_into().unwrap(), @@ -212,7 +232,7 @@ fn relay_op_hash_is_the_extrinsics_own_hash() { circuit_version: 1, }; let request = TransferRequest:: { - merkle_root: ROOT, + merkle_roots: [ROOT; 2], nullifiers: vec![Nullifier(NULL)].try_into().unwrap(), commitments: vec![Commitment(CHANGE)].try_into().unwrap(), memos: vec![Default::default()].try_into().unwrap(), diff --git a/frame/shielded-pool/src/tests/mod.rs b/frame/shielded-pool/src/tests/mod.rs index 792fa7a7..6fc2d8b9 100644 --- a/frame/shielded-pool/src/tests/mod.rs +++ b/frame/shielded-pool/src/tests/mod.rs @@ -85,6 +85,28 @@ pub(crate) fn proof() -> BoundedVec> { BoundedVec::try_from(vec![0x01u8; 72]).unwrap() } +/// `x + r`, little-endian: different bytes, the same BN254 field element as +/// `x`. What a copier would submit to give one value a second on-chain identity. +pub(crate) fn field_twin(x: [u8; 32]) -> [u8; 32] { + use ark_bn254::Fr; + use ark_ff::{BigInteger, PrimeField}; + let r = Fr::MODULUS.to_bytes_le(); + let mut out = [0u8; 32]; + let mut carry = 0u16; + for i in 0..32 { + let sum = u16::from(x[i]) + u16::from(r[i]) + carry; + out[i] = sum as u8; + carry = sum >> 8; + } + assert_eq!(carry, 0, "x + r must fit in 32 bytes"); + assert_eq!( + Fr::from_le_bytes_mod_order(&out), + Fr::from_le_bytes_mod_order(&x), + "a twin is the same field element" + ); + out +} + pub(crate) fn evm(byte: u8) -> H160 { H160::repeat_byte(byte) } diff --git a/frame/shielded-pool/src/tests/private_transfer.rs b/frame/shielded-pool/src/tests/private_transfer.rs index e997a2a6..c53411ca 100644 --- a/frame/shielded-pool/src/tests/private_transfer.rs +++ b/frame/shielded-pool/src/tests/private_transfer.rs @@ -44,7 +44,7 @@ fn execute_works_single_note() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x01), commitments: two_outputs(0x02), memos: memos_of(2), @@ -64,7 +64,7 @@ fn execute_works_two_notes() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0xA1, 0xA2]), commitments: commitments_of(&[0xB1, 0xB2]), memos: memos_of(2), @@ -76,6 +76,166 @@ fn execute_works_two_notes() { }); } +/// Two notes from two trees: one proven against a sealed tree's final root, the +/// other against the active tree. Both roots reach the verifier, in input order. +#[test] +fn execute_spends_notes_from_two_trees() { + new_test_ext().execute_with(|| { + use crate::mock::{VerifiedStatement, verified_statements}; + let sealed = [0x5E; 32]; + MerkleRepository::insert_sealed_root::(0, sealed); + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + assert_ok!(PrivateTransferOperation::execute::( + &proof(), + TransferRequest { + merkle_roots: [sealed, KNOWN_ROOT], + nullifiers: nullifiers_of(&[0xA1, 0xA2]), + commitments: commitments_of(&[0xB1, 0xB2]), + memos: memos_of(2), + asset_id: 0u32, + fee: 0u128, + circuit_version: 3, + }, + )); + let Some(VerifiedStatement::Transfer(statement, Some(3))) = verified_statements().pop() + else { + panic!("the transfer statement was not verified"); + }; + assert_eq!(statement.merkle_roots, [sealed, KNOWN_ROOT]); + }); +} + +/// Two real inputs may come from two trees, but each root must be known. +#[test] +fn execute_rejects_an_unknown_root_in_either_slot() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + for roots in [[KNOWN_ROOT, [0xFF; 32]], [[0xFF; 32], KNOWN_ROOT]] { + assert_noop!( + PrivateTransferOperation::execute::( + &proof(), + TransferRequest { + merkle_roots: roots, + nullifiers: nullifiers_of(&[0xA1, 0xA2]), + commitments: two_outputs(0x02), + memos: memos_of(2), + asset_id: 0u32, + fee: 0u128, + circuit_version: 3, + }, + ), + Error::::UnknownMerkleRoot + ); + } + }); +} + +/// The circuit leaves a dummy input's root free, so the pallet pins it to the +/// real input's: neither another known root (a sealed tree's) nor an unknown +/// one is accepted, whichever slot the dummy takes. +#[test] +fn execute_rejects_a_dummy_root_that_differs_from_the_real_one() { + new_test_ext().execute_with(|| { + let sealed = [0x5E; 32]; + MerkleRepository::insert_sealed_root::(0, sealed); + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + let dummy_second = one_input(0x01); + let dummy_first: BoundedVec> = + BoundedVec::try_from(vec![Nullifier::new([0u8; 32]), nullifier(0x01)]).unwrap(); + for (nullifiers, roots) in [ + (dummy_second.clone(), [KNOWN_ROOT, sealed]), + (dummy_second, [KNOWN_ROOT, [0xFF; 32]]), + (dummy_first.clone(), [sealed, KNOWN_ROOT]), + (dummy_first, [[0xFF; 32], KNOWN_ROOT]), + ] { + assert_noop!( + PrivateTransferOperation::execute::( + &proof(), + TransferRequest { + merkle_roots: roots, + nullifiers, + commitments: two_outputs(0x02), + memos: memos_of(2), + asset_id: 0u32, + fee: 0u128, + circuit_version: 3, + }, + ), + Error::::InvalidPublicSignals + ); + } + }); +} + +/// `ensure_roots` over every shape: which slot is the dummy, equal or distinct +/// roots, known or unknown, and two dummies. +#[test] +fn ensure_roots_covers_every_shape() { + new_test_ext().execute_with(|| { + let sealed = [0x5E; 32]; + let unknown = [0xFF; 32]; + MerkleRepository::insert_sealed_root::(0, sealed); + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + let dummy = Nullifier::new([0u8; 32]); + let shape = |nullifiers: [Nullifier; 2], merkle_roots| TransferRequest:: { + merkle_roots, + nullifiers: BoundedVec::try_from(nullifiers.to_vec()).unwrap(), + commitments: two_outputs(0x02), + memos: memos_of(2), + asset_id: 0u32, + fee: 0u128, + circuit_version: 3, + }; + let (a, b) = (nullifier(0x01), nullifier(0x02)); + for (nullifiers, roots, expected) in [ + ([a, b], [KNOWN_ROOT, KNOWN_ROOT], Ok(())), + ([a, b], [sealed, KNOWN_ROOT], Ok(())), + ( + [a, b], + [KNOWN_ROOT, unknown], + Err(Error::::UnknownMerkleRoot), + ), + ( + [a, b], + [unknown, unknown], + Err(Error::::UnknownMerkleRoot), + ), + ([a, dummy], [KNOWN_ROOT, KNOWN_ROOT], Ok(())), + ([dummy, b], [sealed, sealed], Ok(())), + ( + [a, dummy], + [KNOWN_ROOT, sealed], + Err(Error::::InvalidPublicSignals), + ), + ( + [dummy, b], + [unknown, KNOWN_ROOT], + Err(Error::::InvalidPublicSignals), + ), + ( + [a, dummy], + [unknown, unknown], + Err(Error::::UnknownMerkleRoot), + ), + ( + [dummy, dummy], + [KNOWN_ROOT, sealed], + Err(Error::::InvalidPublicSignals), + ), + ] { + let expected: Result<(), Error> = expected; + assert_eq!( + shape(nullifiers, roots) + .ensure_roots() + .map_err(sp_runtime::DispatchError::from), + expected.map_err(sp_runtime::DispatchError::from), + "{nullifiers:?} {roots:?}" + ); + } + }); +} + #[test] fn execute_unknown_root_fails() { new_test_ext().execute_with(|| { @@ -84,7 +244,7 @@ fn execute_unknown_root_fails() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: [0xFFu8; 32], + merkle_roots: [[0xFFu8; 32], [0xFFu8; 32]], nullifiers: one_input(0x01), commitments: two_outputs(0x02), memos: memos_of(2), @@ -109,7 +269,7 @@ fn execute_nullifier_already_used_fails() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x20), commitments: two_outputs(0x30), memos: memos_of(2), @@ -134,7 +294,7 @@ fn execute_two_equal_nullifiers_fails() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0x20, 0x20]), commitments: commitments_of(&[0x30, 0x31]), memos: memos_of(2), @@ -158,7 +318,7 @@ fn execute_memo_commitment_mismatch_fails() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0xA1, 0xA2]), commitments: commitments_of(&[0xB1, 0xB2]), memos: memos_of(1), // mismatch: 1 ≠ 2 @@ -185,7 +345,7 @@ fn execute_invalid_memo_size_fails() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x01), commitments: two_outputs(0x02), memos, @@ -212,7 +372,7 @@ fn execute_marks_nullifiers_used() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0xC1, 0xC2]), commitments: commitments_of(&[0xD1, 0xD2]), memos: memos_of(2), @@ -238,7 +398,7 @@ fn execute_stores_commitment_memos() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0xE0), commitments: two_outputs(0xE1), memos: memos_of(2), @@ -263,7 +423,7 @@ fn execute_emits_private_transfer_event() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers.clone(), commitments: commitments.clone(), memos: memos_of(2), @@ -306,7 +466,7 @@ fn execute_accumulates_fee_to_block_author_when_nonzero() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x10), commitments: two_outputs(0x11), memos: memos_of(2), @@ -330,7 +490,7 @@ fn execute_no_fee_accumulated_when_zero() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x12), commitments: two_outputs(0x13), memos: memos_of(2), @@ -381,7 +541,7 @@ fn execute_with_dummy_nullifier_only_real_inserted() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers, commitments: commitments_of(&[0x56, 0x57]), memos: memos_of(2), @@ -416,7 +576,7 @@ fn execute_dummy_nullifier_not_rejected_as_double_spend() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_tx1, commitments: commitments_of(&[0x62, 0x63]), memos: memos_of(2), @@ -430,7 +590,7 @@ fn execute_dummy_nullifier_not_rejected_as_double_spend() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_tx2, commitments: commitments_of(&[0x72, 0x73]), memos: memos_of(2), @@ -457,7 +617,7 @@ fn execute_rejects_all_dummy_nullifiers() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers, commitments: commitments_of(&[0x10, 0x11]), memos: memos_of(2), @@ -483,7 +643,7 @@ fn execute_nullifier_commitment_count_mismatch_fails() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0xF1]), // 1 nullifier commitments: commitments_of(&[0xF2, 0xF3]), // 2 commitments memos: memos_of(2), @@ -523,7 +683,7 @@ fn transfer_preserves_pool_ledger() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x40), commitments: two_outputs(0x41), memos: memos_of(2), @@ -561,7 +721,7 @@ fn transfer_fee_follows_the_relay_commit() { let nullifiers = one_input(0x70); let commitments = two_outputs(0x71); let op_hash = TransferRequest:: { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers.clone(), commitments: commitments.clone(), memos: memos_of(2), @@ -576,7 +736,7 @@ fn transfer_fee_follows_the_relay_commit() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers, commitments, memos: memos_of(2), @@ -591,7 +751,7 @@ fn transfer_fee_follows_the_relay_commit() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x72), commitments: two_outputs(0x73), memos: memos_of(2), @@ -615,7 +775,7 @@ fn transfer_nonzero_fee_without_recipient_errors() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x80), commitments: two_outputs(0x81), memos: memos_of(2), @@ -643,7 +803,7 @@ fn transfer_frozen_asset_fails() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x90), commitments: two_outputs(0x91), memos: memos_of(2), @@ -667,7 +827,7 @@ fn transfer_unknown_asset_fails() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x92), commitments: two_outputs(0x93), memos: memos_of(2), @@ -706,7 +866,7 @@ fn attack_same_nullifier_twice_in_one_extrinsic_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nulls, commitments: commitments_of(&[0xC1, 0xC2]), memos: memos_of(2), @@ -737,7 +897,7 @@ fn attack_two_dummy_nullifiers_cannot_mint_free_leaves() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nulls, commitments: commitments_of(&[0xD1, 0xD2]), memos: memos_of(2), @@ -760,7 +920,7 @@ fn attack_replaying_a_spent_nullifier_is_refused() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x51), commitments: two_outputs(0x52), memos: memos_of(2), @@ -775,7 +935,7 @@ fn attack_replaying_a_spent_nullifier_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x51), commitments: two_outputs(0x53), memos: memos_of(2), @@ -814,7 +974,7 @@ fn attack_non_canonical_nullifier_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nulls, commitments: two_outputs(0xE1), memos: memos_of(2), @@ -848,7 +1008,7 @@ fn attack_non_canonical_commitment_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x61), commitments: comms, memos: memos_of(2), @@ -873,7 +1033,7 @@ fn attack_unknown_merkle_root_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: [0xEEu8; 32], // never added + merkle_roots: [[0xEEu8; 32], [0xEEu8; 32]], // never added nullifiers: one_input(0x71), commitments: two_outputs(0x72), memos: memos_of(2), @@ -898,7 +1058,7 @@ fn attack_more_commitments_than_nullifiers_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0x81]), // 1 input commitments: commitments_of(&[0x82, 0x83]), // 2 outputs memos: memos_of(2), @@ -924,7 +1084,7 @@ fn attack_memo_count_mismatch_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0x91, 0x92]), commitments: commitments_of(&[0x93, 0x94]), memos: memos_of(1), // one memo for two outputs @@ -953,7 +1113,7 @@ fn attack_undersized_memo_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0xA9), commitments: two_outputs(0xAA), memos, @@ -982,7 +1142,7 @@ fn attack_zero_commitment_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0xB9), commitments: comms, memos: memos_of(2), @@ -1012,7 +1172,7 @@ fn attack_fee_below_minimum_is_refused_without_spending_the_nullifier() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0xC9), commitments: two_outputs(0xCA), memos: memos_of(2), @@ -1057,7 +1217,7 @@ fn attack_duplicate_commitments_in_one_call_cannot_take_two_leaves() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0xF2, 0xF3]), commitments: comms, memos: memos_of(2), @@ -1109,7 +1269,7 @@ fn attack_memo_contents_never_gate_admission() { assert_ok!(PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(seed), commitments: two_outputs(seed + 1), memos, @@ -1126,7 +1286,7 @@ fn attack_memo_contents_never_gate_admission() { fn request(seeds: [u8; 2]) -> TransferRequest { TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[seeds[0], seeds[0] + 1]), commitments: commitments_of(&[seeds[1], seeds[1] + 1]), memos: vec![memo(0xA1), memo(0xA2)].try_into().unwrap(), @@ -1139,7 +1299,7 @@ fn request(seeds: [u8; 2]) -> TransferRequest { fn transfer_call(req: &TransferRequest) -> crate::Call { crate::Call::::private_transfer { proof: proof(), - merkle_root: req.merkle_root, + merkle_roots: req.merkle_roots, nullifiers: req.nullifiers.clone(), commitments: req.commitments.clone(), encrypted_memos: req.memos.clone(), @@ -1276,7 +1436,7 @@ fn a_one_in_one_out_transfer_is_refused() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: nullifiers_of(&[0x01]), commitments: commitments_of(&[0x02]), memos: memos_of(1), @@ -1303,7 +1463,7 @@ fn an_invalid_proof_fails_at_dispatch_and_changes_nothing() { PrivateTransferOperation::execute::( &proof(), TransferRequest { - merkle_root: KNOWN_ROOT, + merkle_roots: [KNOWN_ROOT, KNOWN_ROOT], nullifiers: one_input(0x01), commitments: two_outputs(0x02), memos: memos_of(2), diff --git a/frame/shielded-pool/src/tests/unshield.rs b/frame/shielded-pool/src/tests/unshield.rs index 14143358..95139548 100644 --- a/frame/shielded-pool/src/tests/unshield.rs +++ b/frame/shielded-pool/src/tests/unshield.rs @@ -9,7 +9,7 @@ use crate::{ operations::{assets::AssetOperation, unshield::*}, pallet::{Error, Event as PalletEvent}, storage::{MerkleRepository, NullifierRepository, PoolBalanceRepository}, - types::{Commitment, EncryptedMemo}, + types::{Commitment, EncryptedMemo, Nullifier}, }; use frame_support::{assert_err, assert_noop, assert_ok, traits::Currency}; use sp_runtime::AccountId32; @@ -1114,6 +1114,32 @@ fn a_partial_unshield_without_a_full_change_memo_is_rejected() { }); } +/// A nullifier or change commitment as its modular twin (`x + r`) is the same +/// field element with another on-chain identity: refused at dispatch as well. +#[test] +fn a_non_canonical_nullifier_or_change_commitment_is_refused() { + new_test_ext().execute_with(|| { + let asset_id = setup_spend(); + let twin = super::field_twin; + let mut aliased_nullifier = request(0x73, asset_id); + aliased_nullifier.nullifier = Nullifier::new(twin(aliased_nullifier.nullifier.0)); + assert_noop!( + UnshieldOperation::execute::(&proof(), aliased_nullifier), + Error::::InvalidPublicSignals + ); + let aliased_change = UnshieldRequest { + amount: AMOUNT - 100, + change_commitment: twin(canonical_bytes(0x92)), + change_memo: change_memo(), + ..request(0x74, asset_id) + }; + assert_noop!( + UnshieldOperation::execute::(&proof(), aliased_change), + Error::::InvalidPublicSignals + ); + }); +} + /// The zero account has no known key: an unshield to it burns the withdrawal. #[test] fn an_unshield_to_the_zero_account_is_refused() { diff --git a/frame/shielded-pool/src/validate_unsigned/tests.rs b/frame/shielded-pool/src/validate_unsigned/tests.rs index c3e75744..16d361e4 100644 --- a/frame/shielded-pool/src/validate_unsigned/tests.rs +++ b/frame/shielded-pool/src/validate_unsigned/tests.rs @@ -37,7 +37,7 @@ fn transfer_request( Commitment::new(bytes) }; TransferRequest { - merkle_root: *merkle_root, + merkle_roots: [*merkle_root; 2], nullifiers, commitments: vec![commitment(0), commitment(1)].try_into().unwrap(), memos: vec![full_memo(); 2].try_into().unwrap(), @@ -134,6 +134,47 @@ fn private_transfer_unknown_root_rejected() { }); } +/// A dummy slot must repeat the real input's root: another known root (a sealed +/// tree's) or an unknown one is turned away before the proof is checked. +#[test] +fn private_transfer_dummy_root_that_differs_rejected() { + use sp_runtime::transaction_validity::{InvalidTransaction, TransactionValidityError}; + new_test_ext().execute_with(|| { + MerkleRepository::insert_sealed_root::(0, [0x5E; 32]); + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + for dummy_root in [[0x5E; 32], [0xFF; 32]] { + let mut req = transfer_request(&KNOWN_ROOT, &nullifiers_of(&[0x01]), 0, 3); + req.merkle_roots[1] = dummy_root; + assert_eq!( + super::validate_private_transfer(Some(&proof()), &req), + Err(TransactionValidityError::Invalid( + InvalidTransaction::Custom(codes::INVALID_SPEND) + )) + ); + } + }); +} + +/// Two real inputs from two trees: an unknown root in either slot gets the +/// unknown-root code. +#[test] +fn private_transfer_unknown_root_in_either_real_slot_rejected() { + use sp_runtime::transaction_validity::{InvalidTransaction, TransactionValidityError}; + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + for slot in 0..2 { + let mut req = transfer_request(&KNOWN_ROOT, &nullifiers_of(&[0x01, 0x02]), 0, 3); + req.merkle_roots[slot] = [0xFF; 32]; + assert_eq!( + super::validate_private_transfer(Some(&proof()), &req), + Err(TransactionValidityError::Invalid( + InvalidTransaction::Custom(codes::UNKNOWN_ROOT) + )) + ); + } + }); +} + #[test] fn private_transfer_nullifier_used_rejected() { new_test_ext().execute_with(|| { @@ -794,6 +835,144 @@ fn attack_other_asset_balance_does_not_underwrite_this_one() { }); } +// ── adversarial: spends across two trees ───────────────────────────────────── + +/// A two-tree spend: two real notes, one proven against a sealed tree's root, +/// the other against the active window. +fn cross_tree_request(nullifiers: &[u8], roots: [Hash; 2]) -> TransferRequest { + let mut req = transfer_request(&roots[0], &nullifiers_of(nullifiers), 0, 3); + req.merkle_roots = roots; + req +} + +const SEALED_ROOT: Hash = [0x5E; 32]; + +fn register_two_trees() { + MerkleRepository::insert_sealed_root::(0, SEALED_ROOT); + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); +} + +/// A copy of a pending two-tree spend with re-randomised proof bytes (Groth16 +/// proofs are malleable without the witness) shares its tags and priority, so +/// it can neither sit beside the original nor displace it. +#[test] +fn attack_a_rerandomised_copy_of_a_two_tree_spend_cannot_displace_it() { + new_test_ext().execute_with(|| { + register_two_trees(); + let req = cross_tree_request(&[0x81, 0x82], [SEALED_ROOT, KNOWN_ROOT]); + let other_proof: BoundedVec> = vec![0x02u8; 72].try_into().unwrap(); + let original = super::validate_private_transfer(Some(&proof()), &req).unwrap(); + let copy = super::validate_private_transfer(Some(&other_proof), &req).unwrap(); + assert_eq!(original.provides, copy.provides); + assert_eq!(original.priority, copy.priority); + }); +} + +/// Swapping the inputs together with their roots names the same two notes: +/// one tag set, so no second pool entry. +#[test] +fn attack_swapping_inputs_and_roots_does_not_mint_a_second_pool_entry() { + new_test_ext().execute_with(|| { + register_two_trees(); + let ab = cross_tree_request(&[0x83, 0x84], [SEALED_ROOT, KNOWN_ROOT]); + let ba = cross_tree_request(&[0x84, 0x83], [KNOWN_ROOT, SEALED_ROOT]); + let mut a = super::validate_private_transfer(Some(&proof()), &ab) + .unwrap() + .provides; + let mut b = super::validate_private_transfer(Some(&proof()), &ba) + .unwrap() + .provides; + a.sort(); + b.sort(); + assert_eq!(a, b); + }); +} + +/// The same note in both slots, each slot naming a different tree's root: +/// one note cannot be spent twice in one transfer. +#[test] +fn attack_one_note_in_both_slots_across_two_roots_is_refused() { + new_test_ext().execute_with(|| { + register_two_trees(); + let req = cross_tree_request(&[0x85, 0x85], [SEALED_ROOT, KNOWN_ROOT]); + assert!(super::validate_private_transfer(Some(&proof()), &req).is_err()); + }); +} + +/// A note already spent from one tree cannot be spent again naming another +/// tree's root: nullifiers do not depend on the tree. +#[test] +fn attack_a_spent_note_cannot_be_respent_from_another_tree() { + new_test_ext().execute_with(|| { + register_two_trees(); + NullifierRepository::mark_as_used::(nullifier(0x86), 1u64); + let req = cross_tree_request(&[0x86, 0x87], [KNOWN_ROOT, SEALED_ROOT]); + assert_eq!( + super::validate_private_transfer(Some(&proof()), &req), + Err(sp_runtime::transaction_validity::InvalidTransaction::Stale.into()) + ); + }); +} + +/// A root's non-canonical alias (`root + r`, the same field element) is not +/// a known root: roots compare as bytes, and only canonical ones are stored. +#[test] +fn attack_a_non_canonical_alias_of_a_known_root_is_unknown() { + new_test_ext().execute_with(|| { + let root = crate::tests::canonical_bytes(0x33); + MerkleRepository::add_historic_poseidon_root::(root); + let alias = crate::tests::field_twin(root); + assert_ne!(alias, root); + for roots in [[alias, root], [root, alias]] { + let req = cross_tree_request(&[0x88, 0x89], roots); + assert!(rejected_with( + super::validate_private_transfer(Some(&proof()), &req), + codes::UNKNOWN_ROOT + )); + } + }); +} + +/// An unshield's nullifier or change commitment given as its modular twin +/// (`x + r`): the same field element, so the proof would verify, but another +/// 32-byte identity on chain. Refused before the proof is checked. +#[test] +fn attack_a_non_canonical_unshield_nullifier_or_change_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + PoolBalanceRepository::set_asset_balance::(0, 1_000u128); + let base = unshield_request(&KNOWN_ROOT, &nullifier(0x8B), 0, 500, 0, 2); + + let mut aliased_nullifier = base.clone(); + aliased_nullifier.nullifier = Nullifier::new(crate::tests::field_twin(nullifier(0x8B).0)); + assert!(rejected_with( + super::validate_unshield(Some(&proof()), &aliased_nullifier), + codes::INVALID_SPEND + )); + + let change = crate::tests::canonical_bytes(0x8C); + let mut aliased_change = base; + aliased_change.amount = 400; + aliased_change.change_commitment = crate::tests::field_twin(change); + aliased_change.change_memo = full_memo(); + assert!(rejected_with( + super::validate_unshield(Some(&proof()), &aliased_change), + codes::INVALID_SPEND + )); + }); +} + +/// Two dummy inputs naming two roots spend nothing: refused, nothing inserted. +#[test] +fn attack_two_dummies_with_differing_roots_are_refused() { + new_test_ext().execute_with(|| { + register_two_trees(); + let mut req = cross_tree_request(&[0x8A], [SEALED_ROOT, KNOWN_ROOT]); + req.nullifiers = vec![Nullifier::new([0u8; 32]); 2].try_into().unwrap(); + assert!(super::validate_private_transfer(Some(&proof()), &req).is_err()); + }); +} + // ── memo shape ─────────────────────────────────────────────────────────────── fn rejected_with(result: TransactionValidity, code: u8) -> bool { diff --git a/frame/zk-verifier/CHANGELOG.md b/frame/zk-verifier/CHANGELOG.md index cd566ba3..3dbb766a 100644 --- a/frame/zk-verifier/CHANGELOG.md +++ b/frame/zk-verifier/CHANGELOG.md @@ -6,6 +6,22 @@ All notable changes to this pallet are documented here. ## [Unreleased] +## [0.15.0] - 2026-10-08 + +### Changed + +- **Breaking:** `TransferStatement.merkle_root` → `merkle_roots: [[u8; 32]; 2]`, + the root each input is proven against. +- `encode_transfer` emits both roots under a cross-tree key (circuit 1, v3). Under + a v1 or v2 key, which attests to one root, a statement whose roots differ + fails without a pairing. +- `encode_transfer` / `encode_unshield` return `Option`: `None` when the key + cannot attest to the statement. `verify_statement` takes such an encoder. +- A transfer key of 9 inputs registers (cross-tree layout); the governance rule + on `register_verification_key` documents the exception. +- `verification_weight` covers the 9-input layout (was 8), read from + `max_public_inputs`. + ## [0.14.0] - 2026-10-05 ### Added diff --git a/frame/zk-verifier/Cargo.toml b/frame/zk-verifier/Cargo.toml index 34c5746d..7f9ac7ad 100644 --- a/frame/zk-verifier/Cargo.toml +++ b/frame/zk-verifier/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-zk-verifier" -version = "0.14.0" +version = "0.15.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/encoding.rs b/frame/zk-verifier/src/encoding.rs index d60fcade..b7a60557 100644 --- a/frame/zk-verifier/src/encoding.rs +++ b/frame/zk-verifier/src/encoding.rs @@ -3,26 +3,36 @@ //! //! 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 and a -//! memo-bound key each get the inputs they were built for. +//! [`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. 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]`. -pub fn encode_transfer(s: &TransferStatement, layout: InputLayout) -> Vec<[u8; 32]> { - let mut raw = Vec::with_capacity(4 + s.nullifiers.len() + s.commitments.len()); - raw.push(s.merkle_root); +/// `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. +/// +/// `None` when the key cannot attest to the statement: a key with one root +/// cannot attest to two, so the proof fails without a pairing. +pub fn encode_transfer(s: &TransferStatement, layout: InputLayout) -> Option> { + let [root0, root1] = s.merkle_roots; + let mut raw = Vec::with_capacity(5 + s.nullifiers.len() + s.commitments.len()); + match layout { + InputLayout::CrossTree => raw.extend_from_slice(&s.merkle_roots), + InputLayout::Base | InputLayout::MemoBound if root0 == root1 => raw.push(root0), + InputLayout::Base | InputLayout::MemoBound => 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 layout == InputLayout::MemoBound { + if matches!(layout, InputLayout::MemoBound | InputLayout::CrossTree) { raw.push(to_field_le(&s.memo_digest)); } - raw + Some(raw) } /// Unshield inputs, in circuit order: @@ -32,13 +42,17 @@ pub fn encode_transfer(s: &TransferStatement, layout: InputLayout) -> Vec<[u8; 3 /// 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. -pub fn encode_unshield(s: &UnshieldStatement, layout: InputLayout) -> Vec<[u8; 32]> { +/// +/// `None` for the cross-tree layout: an unshield spends one note, so no such +/// 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![ s.merkle_root, @@ -50,7 +64,7 @@ pub fn encode_unshield(s: &UnshieldStatement, layout: InputLayout) -> Vec<[u8; 3 s.change_commitment, ]; raw.extend(memo_hash); - raw + Some(raw) } /// Shield inputs, in circuit order: `commitment | value | asset_id`. @@ -75,14 +89,16 @@ fn u128_field(v: u128) -> [u8; 32] { #[cfg(test)] mod tests { use super::*; - use orbinum_zk_verifier::{MEMO_HASH_INPUTS, TRANSFER_PUBLIC_INPUTS, UNSHIELD_PUBLIC_INPUTS}; + use orbinum_zk_verifier::{ + CROSS_TREE_INPUTS, MEMO_HASH_INPUTS, TRANSFER_PUBLIC_INPUTS, UNSHIELD_PUBLIC_INPUTS, + }; const NULLIFIERS: [[u8; 32]; 2] = [[0x02; 32], [0x03; 32]]; const COMMITMENTS: [[u8; 32]; 2] = [[0x04; 32], [0x05; 32]]; fn transfer() -> TransferStatement { TransferStatement { - merkle_root: [0x01; 32], + merkle_roots: [[0x01; 32]; 2], nullifiers: NULLIFIERS.to_vec(), commitments: COMMITMENTS.to_vec(), asset_id: 7, @@ -104,8 +120,8 @@ mod tests { } } - /// `R + r`: a different AccountId32 that is the same field element as `R`. - fn alias(recipient: [u8; 32]) -> [u8; 32] { + /// `x + r`, little-endian: different bytes, the same field element as `x`. + fn field_twin(x: [u8; 32]) -> [u8; 32] { // BN254 r, little-endian. const R: [u8; 32] = [ 0x01, 0x00, 0x00, 0xf0, 0x93, 0xf5, 0xe1, 0x43, 0x91, 0x70, 0xb9, 0x79, 0x48, 0xe8, @@ -115,17 +131,18 @@ mod tests { let mut out = [0u8; 32]; let mut carry = 0u16; for i in 0..32 { - let sum = recipient[i] as u16 + R[i] as u16 + carry; + let sum = u16::from(x[i]) + u16::from(R[i]) + carry; out[i] = sum as u8; carry = sum >> 8; } - assert_eq!(carry, 0, "recipient + r must fit in 32 bytes"); + assert_eq!(carry, 0, "x + r must fit in 32 bytes"); + assert_eq!(to_field_le(&out), to_field_le(&x), "same field element"); out } #[test] fn transfer_follows_circuit_order() { - let raw = encode_transfer(&transfer(), InputLayout::Base); + let raw = encode_transfer(&transfer(), InputLayout::Base).unwrap(); assert_eq!(raw.len(), TRANSFER_PUBLIC_INPUTS); assert_eq!(raw[0], [0x01; 32]); assert_eq!(&raw[1..3], &NULLIFIERS); @@ -134,10 +151,66 @@ mod tests { assert_eq!(raw[6], u128_field(500)); } + #[test] + fn cross_tree_transfer_puts_one_root_per_input_first() { + let s = TransferStatement { + merkle_roots: [[0x01; 32], [0x09; 32]], + ..transfer() + }; + let raw = encode_transfer(&s, InputLayout::CrossTree).unwrap(); + assert_eq!( + raw.len(), + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS + CROSS_TREE_INPUTS + ); + assert_eq!(raw[0], [0x01; 32]); + assert_eq!(raw[1], [0x09; 32]); + assert_eq!(&raw[2..4], &NULLIFIERS); + assert_eq!(&raw[4..6], &COMMITMENTS); + assert_eq!(raw[6], u32_field(7)); + assert_eq!(raw[7], u128_field(500)); + assert_eq!(raw[8], to_field_le(&[0xEE; 32])); + } + + #[test] + fn a_single_root_key_cannot_attest_to_two_roots() { + let s = TransferStatement { + merkle_roots: [[0x01; 32], [0x09; 32]], + ..transfer() + }; + assert!(encode_transfer(&s, InputLayout::Base).is_none()); + assert!(encode_transfer(&s, InputLayout::MemoBound).is_none()); + } + + #[test] + fn an_unshield_has_no_cross_tree_encoding() { + assert!(encode_unshield(&unshield(), InputLayout::CrossTree).is_none()); + } + + /// `memo_digest` is a blake2 output, so it is reduced mod r rather than + /// refused: a 256-bit twin would need a blake2 preimage. Two digests that are + /// twins encode the same; two that differ in the field encode differently. + #[test] + fn memo_digest_twins_encode_the_same_and_distinct_digests_differ() { + let digest = [0x11; 32]; + let twin = field_twin(digest); + let encode = |memo_digest| { + encode_transfer( + &TransferStatement { + memo_digest, + ..transfer() + }, + InputLayout::MemoBound, + ) + .unwrap() + }; + assert_eq!(encode(digest), encode(twin)); + assert_ne!(encode(digest), encode([0x12; 32])); + } + #[test] fn memo_bound_transfer_appends_the_reduced_memo_digest() { - let base = encode_transfer(&transfer(), InputLayout::Base); - let bound = encode_transfer(&transfer(), InputLayout::MemoBound); + 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])); @@ -150,18 +223,18 @@ mod tests { ..transfer() }; assert_eq!( - encode_transfer(&transfer(), InputLayout::Base), - encode_transfer(&other, InputLayout::Base) + encode_transfer(&transfer(), InputLayout::Base).unwrap(), + encode_transfer(&other, InputLayout::Base).unwrap() ); assert_ne!( - encode_transfer(&transfer(), InputLayout::MemoBound), - encode_transfer(&other, InputLayout::MemoBound) + 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); + let raw = encode_unshield(&unshield(), InputLayout::Base).unwrap(); assert_eq!(raw.len(), UNSHIELD_PUBLIC_INPUTS); assert_eq!(raw[0], [0x01; 32]); assert_eq!(raw[1], [0x02; 32]); @@ -174,7 +247,7 @@ mod tests { #[test] fn memo_bound_unshield_hashes_the_recipient_and_appends_the_memo_digest() { - let raw = encode_unshield(&unshield(), InputLayout::MemoBound); + let raw = encode_unshield(&unshield(), InputLayout::MemoBound).unwrap(); assert_eq!(raw.len(), UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS); assert_eq!( raw[3], @@ -187,7 +260,7 @@ mod tests { fn a_recipient_alias_encodes_the_same_only_in_the_base_layout() { let recipient = [0x01; 32]; let aliased = UnshieldStatement { - recipient: alias(recipient), + recipient: field_twin(recipient), ..unshield() }; let original = UnshieldStatement { @@ -196,13 +269,13 @@ mod tests { }; assert_ne!(original.recipient, aliased.recipient); assert_eq!( - encode_unshield(&original, InputLayout::Base), - encode_unshield(&aliased, InputLayout::Base), + 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), - encode_unshield(&aliased, InputLayout::MemoBound) + encode_unshield(&original, InputLayout::MemoBound).unwrap(), + encode_unshield(&aliased, InputLayout::MemoBound).unwrap() ); } @@ -220,7 +293,8 @@ mod tests { ..unshield() }, InputLayout::MemoBound, - )[3] + ) + .unwrap()[3] }; assert_eq!( hex(recipient([0x01; 32])), @@ -242,7 +316,7 @@ mod tests { ..unshield() }; for layout in [InputLayout::Base, InputLayout::MemoBound] { - let raw = encode_unshield(&max, layout); + let raw = encode_unshield(&max, layout).unwrap(); assert!( orbinum_zk_verifier::PublicInputs::new(raw) .to_field_elements() @@ -335,21 +409,7 @@ mod tests { /// this shows the verifier refuses it too. #[test] fn rejects_a_non_canonical_encoding_of_the_commitment() { - // BN254 scalar modulus r, little-endian. - const R: [u8; 32] = [ - 0x01, 0x00, 0x00, 0xf0, 0x93, 0xf5, 0xe1, 0x43, 0x91, 0x70, 0xb9, 0x79, 0x48, 0xe8, - 0x33, 0x28, 0x5d, 0x58, 0x81, 0x81, 0xb6, 0x45, 0x50, 0xb8, 0x29, 0xa0, 0x31, 0xe1, - 0x72, 0x4e, 0x64, 0x30, - ]; - let mut shifted = [0u8; 32]; - let mut carry = 0u16; - for i in 0..32 { - let sum = COMMITMENT[i] as u16 + R[i] as u16 + carry; - shifted[i] = sum as u8; - carry = sum >> 8; - } - assert_eq!(carry, 0, "commitment + r fits 256 bits"); - assert_eq!(to_field_le(&shifted), COMMITMENT, "same field element"); + let shifted = super::field_twin(COMMITMENT); assert!(!verifies(ShieldStatement { commitment: shifted, value: 1000, diff --git a/frame/zk-verifier/src/lib.rs b/frame/zk-verifier/src/lib.rs index 987019b3..f96d6268 100644 --- a/frame/zk-verifier/src/lib.rs +++ b/frame/zk-verifier/src/lib.rs @@ -13,8 +13,9 @@ //! ## Key rotation //! //! A known circuit's first version may use its base input layout; every later -//! version must be memo-bound. Rotation is done by Root: register the new -//! version, `set_active_version`, then `retire_version` the old one. +//! 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. //! //! ## Usage //! @@ -273,11 +274,15 @@ pub mod pallet { /// /// 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, or its memo-bound variant (the same constraints plus - /// `memo_hash`, recognised by arity — see `InputLayout`). 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, 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. + /// + /// 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). @@ -603,7 +608,8 @@ 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 or memo-bound). + /// 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. diff --git a/frame/zk-verifier/src/port.rs b/frame/zk-verifier/src/port.rs index 8cef6bd5..d73331ae 100644 --- a/frame/zk-verifier/src/port.rs +++ b/frame/zk-verifier/src/port.rs @@ -18,7 +18,9 @@ use alloc::vec::Vec; /// What a private-transfer proof attests to, as the pallet submits it. #[derive(Clone, PartialEq, Eq, Debug)] pub struct TransferStatement { - pub merkle_root: [u8; 32], + /// The root each input note is proven against. Equal unless the notes come + /// from different trees, which only a cross-tree key can attest to. + pub merkle_roots: [[u8; 32]; 2], /// One per input note. pub nullifiers: Vec<[u8; 32]>, /// One per output note, in the order their memos are submitted. @@ -92,17 +94,22 @@ pub trait ZkVerifierPort { // ─── Implementation ─────────────────────────────────────────────────────────── -/// Public inputs of the largest spend layout: a memo-bound transfer or unshield. +/// Public inputs of the largest spend layout: a cross-tree transfer. const MAX_SPEND_PUBLIC_INPUTS: u32 = { - use orbinum_zk_verifier::{MEMO_HASH_INPUTS, TRANSFER_PUBLIC_INPUTS, UNSHIELD_PUBLIC_INPUTS}; - let base = if TRANSFER_PUBLIC_INPUTS > UNSHIELD_PUBLIC_INPUTS { - TRANSFER_PUBLIC_INPUTS - } else { - UNSHIELD_PUBLIC_INPUTS + use orbinum_zk_verifier::{CIRCUIT_ID_TRANSFER, CIRCUIT_ID_UNSHIELD, max_public_inputs}; + let (Some(transfer), Some(unshield)) = ( + max_public_inputs(CIRCUIT_ID_TRANSFER), + max_public_inputs(CIRCUIT_ID_UNSHIELD), + ) else { + panic!("spend circuits have a known arity") }; - (base + MEMO_HASH_INPUTS) as u32 + max(transfer, unshield) as u32 }; +const fn max(a: usize, b: usize) -> usize { + if a > b { a } else { b } +} + impl ZkVerifierPort for Pallet { fn verify_transfer_proof( proof: &[u8], @@ -139,7 +146,7 @@ impl ZkVerifierPort for Pallet { ) -> Result { // Shield has a single layout; a key of another arity fails the length check. verifier::verify_statement::(CircuitId::SHIELD, version, proof, |_| { - encoding::encode_shield(statement) + Some(encoding::encode_shield(statement)) }) .map(|(ok, _)| ok) } @@ -164,7 +171,8 @@ mod tests { use crate::mock::{Test, activate, insert_vk, new_test_ext}; use frame_support::assert_err; use orbinum_zk_verifier::{ - MEMO_HASH_INPUTS, SHIELD_PUBLIC_INPUTS, TRANSFER_PUBLIC_INPUTS, UNSHIELD_PUBLIC_INPUTS, + CROSS_TREE_INPUTS, MEMO_HASH_INPUTS, SHIELD_PUBLIC_INPUTS, TRANSFER_PUBLIC_INPUTS, + UNSHIELD_PUBLIC_INPUTS, }; // ── Helpers ─────────────────────────────────────────────────────────────── @@ -178,7 +186,7 @@ mod tests { fn transfer() -> TransferStatement { TransferStatement { - merkle_root: [0x03; 32], + merkle_roots: [[0x03; 32]; 2], nullifiers: NULLIFIERS.to_vec(), commitments: COMMITMENTS.to_vec(), asset_id: 1, @@ -276,6 +284,46 @@ mod tests { }); } + /// Every key arity, registered side by side as v1, v2 and v3. + fn insert_all_transfer_keys() { + insert_vk(CircuitId::TRANSFER, 1, TRANSFER_PUBLIC_INPUTS); + insert_vk( + CircuitId::TRANSFER, + 2, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS, + ); + insert_vk( + CircuitId::TRANSFER, + 3, + TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS + CROSS_TREE_INPUTS, + ); + activate(CircuitId::TRANSFER, 3); + } + + #[test] + fn a_same_tree_transfer_verifies_under_every_key() { + new_test_ext().execute_with(|| { + insert_all_transfer_keys(); + for version in 1..=3 { + assert_eq!(verify_transfer(&transfer(), Some(version)), Ok(true)); + } + }); + } + + #[test] + fn only_the_cross_tree_key_verifies_a_spend_across_two_trees() { + new_test_ext().execute_with(|| { + insert_all_transfer_keys(); + let across = TransferStatement { + merkle_roots: [[0x03; 32], [0x07; 32]], + ..transfer() + }; + assert_eq!(verify_transfer(&across, Some(3)), Ok(true)); + assert_eq!(verify_transfer(&across, Some(2)), Ok(false)); + assert_eq!(verify_transfer(&across, Some(1)), Ok(false)); + }); + } + #[test] fn transfer_with_one_input_does_not_fit_the_two_input_key() { new_test_ext().execute_with(|| { @@ -462,7 +510,7 @@ mod tests { #[test] fn verification_weight_covers_the_largest_spend_layout() { use crate::weights::WeightInfo; - assert_eq!(MAX_SPEND_PUBLIC_INPUTS, 8); + assert_eq!(MAX_SPEND_PUBLIC_INPUTS, 9); assert_eq!( as ZkVerifierPort>::verification_weight(), ::WeightInfo::verify_proof(MAX_SPEND_PUBLIC_INPUTS) diff --git a/frame/zk-verifier/src/tests.rs b/frame/zk-verifier/src/tests.rs index b65c4461..9d891733 100644 --- a/frame/zk-verifier/src/tests.rs +++ b/frame/zk-verifier/src/tests.rs @@ -163,6 +163,49 @@ fn register_vk_accepts_the_memo_bound_arity_next_to_the_base_one() { }); } +/// A transfer that proves each input against its own root takes one input more +/// than the memo-bound layout; an unshield has a single input and no such layout. +#[test] +fn register_vk_accepts_the_cross_tree_arity_for_a_transfer_only() { + new_test_ext().execute_with(|| { + assert_ok!(register( + CircuitId::TRANSFER, + 3, + real_vk(TRANSFER_PUBLIC_INPUTS + MEMO_HASH_INPUTS + 1) + )); + assert_noop!( + register( + CircuitId::UNSHIELD, + 3, + real_vk(UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS + 1) + ), + Error::::InvalidVerificationKey + ); + }); +} + +/// A rotation that keeps the layout: unshield v3 (canonical spending key) has +/// the same 8 inputs as v2, so it registers and activates like any later +/// memo-bound version. +#[test] +fn register_vk_accepts_a_memo_bound_rotation_at_the_same_arity() { + new_test_ext().execute_with(|| { + let arity = UNSHIELD_PUBLIC_INPUTS + MEMO_HASH_INPUTS; + assert_ok!(register(CircuitId::UNSHIELD, 2, real_vk(arity))); + assert_ok!(register(CircuitId::UNSHIELD, 3, real_vk(arity))); + assert_ok!(ZkVerifier::set_active_version( + root().into(), + CircuitId::UNSHIELD, + 3 + )); + assert_ok!(ZkVerifier::retire_version( + root().into(), + CircuitId::UNSHIELD, + 2 + )); + }); +} + /// An identity `gamma_abc` point would leave its public input unbound. #[test] fn register_vk_rejects_a_point_at_infinity() { @@ -270,9 +313,14 @@ 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 or base + memo_hash; anything else is rejected. + // TRANSFER takes its base arity, memo-bound (+1) or cross-tree (+2); + // anything else is rejected. + assert_noop!( + register(CircuitId::TRANSFER, 1, real_vk(TRANSFER_PUBLIC_INPUTS + 3)), + Error::::InvalidVerificationKey + ); assert_noop!( - register(CircuitId::TRANSFER, 1, real_vk(TRANSFER_PUBLIC_INPUTS + 2)), + register(CircuitId::UNSHIELD, 1, real_vk(UNSHIELD_PUBLIC_INPUTS + 2)), Error::::InvalidVerificationKey ); // The matching arity is accepted. @@ -1466,7 +1514,7 @@ fn genesis_rejects_a_valid_key_with_the_wrong_arity() { let _ = pallet::GenesisConfig:: { verification_keys: vec![( CircuitId::TRANSFER, - real_vk(TRANSFER_PUBLIC_INPUTS + 2).into_inner(), + real_vk(TRANSFER_PUBLIC_INPUTS + 3).into_inner(), )], _phantom: Default::default(), } diff --git a/frame/zk-verifier/src/verifier.rs b/frame/zk-verifier/src/verifier.rs index 44d6adb3..c1fb3929 100644 --- a/frame/zk-verifier/src/verifier.rs +++ b/frame/zk-verifier/src/verifier.rs @@ -15,17 +15,18 @@ use orbinum_zk_verifier::{Bn254, InputLayout, PreparedVerifyingKey, VerifyingKey /// 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 neither of the -/// circuit's layouts, or inputs that do not fill it, are `valid = false`. +/// 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`. pub fn verify_statement( circuit_id: CircuitId, version: Option, proof: &[u8], - encode: impl FnOnce(InputLayout) -> Vec<[u8; 32]>, + 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)?; - Some(encode(layout)).filter(|raw| raw.len() == arity) + encode(layout).filter(|raw| raw.len() == arity) }) } @@ -148,10 +149,10 @@ mod tests { fn encoder( seen: &mut Option, len: usize, - ) -> impl FnOnce(InputLayout) -> Vec<[u8; 32]> + '_ { + ) -> impl FnOnce(InputLayout) -> Option> + '_ { move |layout| { *seen = Some(layout); - vec![[0x02; 32]; len] + Some(vec![[0x02; 32]; len]) } } @@ -198,16 +199,28 @@ mod tests { }); } + /// An encoder that cannot attest to the statement fails the proof without + /// a pairing, and the attempt is still counted. + #[test] + fn a_statement_the_key_cannot_attest_to_fails_and_is_counted() { + new_test_ext().execute_with(|| { + insert_vk(CircuitId::TRANSFER, 1, BASE); + 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)); + }); + } + #[test] fn a_key_of_foreign_arity_fails_without_encoding() { new_test_ext().execute_with(|| { - insert_vk(CircuitId::TRANSFER, 1, BASE + 2); + insert_vk(CircuitId::TRANSFER, 1, BASE + 3); let mut seen = None; let res = verify_statement::( CircuitId::TRANSFER, Some(1), &proof(), - encoder(&mut seen, BASE + 2), + encoder(&mut seen, BASE + 3), ); assert_eq!(res, Ok((false, 1))); assert_eq!(seen, None); diff --git a/primitives/zk-verifier/CHANGELOG.md b/primitives/zk-verifier/CHANGELOG.md index e94bec22..66da08f2 100644 --- a/primitives/zk-verifier/CHANGELOG.md +++ b/primitives/zk-verifier/CHANGELOG.md @@ -6,6 +6,33 @@ All notable changes to this crate are documented here. ## [Unreleased] +## [3.0.0] - 2026-10-08 + +### Added + +- `InputLayout::CrossTree`: a transfer key with 9 public inputs (the + memo-bound layout with one Merkle root per input, `merkle_roots[0..1]` first). + `input_layout` maps transfer arity 9 to it; an unshield or shield key of that + arity is still refused. `CROSS_TREE_INPUTS` and `has_cross_tree_layout`. +- `max_public_inputs(circuit_id)`: the arity of a circuit's widest layout, the + one place the layouts are added up. The compile-time arity check uses it. + +### Changed + +- **Breaking:** `InputLayout` gains the `CrossTree` variant; an exhaustive + `match` on it must handle it. +- **Breaking:** `MAX_PROOF_BYTES` is removed; `PROOF_BYTES` (128) is the one + accepted proof length. + +### Security + +- A proof must be exactly `PROOF_BYTES` (128) long. `deserialize_compressed` + ignores trailing bytes, so a padded proof was a second byte string for the + same proof — a malleability the pool and the relay op hash already tolerate + (proofs are re-randomisable), but one encoding per proof is the contract. + Found by a live probe. Tests: trailing bytes refused, a G2 point outside the + subgroup refused in proofs and keys, degenerate proof bytes never verify. + ## [2.1.0] - 2026-10-05 ### Added diff --git a/primitives/zk-verifier/Cargo.toml b/primitives/zk-verifier/Cargo.toml index ed8b0125..416dfacf 100644 --- a/primitives/zk-verifier/Cargo.toml +++ b/primitives/zk-verifier/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orbinum-zk-verifier" -version = "2.1.0" +version = "3.0.0" authors = ["Orbinum Network "] edition = "2021" publish = false diff --git a/primitives/zk-verifier/src/circuits.rs b/primitives/zk-verifier/src/circuits.rs index 6125fcdf..b77430eb 100644 --- a/primitives/zk-verifier/src/circuits.rs +++ b/primitives/zk-verifier/src/circuits.rs @@ -24,6 +24,10 @@ pub const SHIELD_PUBLIC_INPUTS: usize = 3; /// `memo_hash`, appended last. pub const MEMO_HASH_INPUTS: usize = 1; +/// Public inputs a cross-tree transfer adds to the memo-bound layout: a second +/// Merkle root, right after the first, so each input has its own. +pub const CROSS_TREE_INPUTS: usize = 1; + // ─── Input layouts ──────────────────────────────────────────────────────────── /// Base public-input count for a known circuit id, or `None` if unknown. A VK @@ -44,6 +48,12 @@ pub const fn has_memo_layout(circuit_id: u8) -> bool { matches!(circuit_id, CIRCUIT_ID_TRANSFER | CIRCUIT_ID_UNSHIELD) } +/// Whether `circuit_id` has a cross-tree layout: only a transfer spends two notes, +/// so only it can take them from two trees. +pub const fn has_cross_tree_layout(circuit_id: u8) -> bool { + circuit_id == CIRCUIT_ID_TRANSFER +} + /// How a circuit version lays out its public inputs, read off its key's arity. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum InputLayout { @@ -52,6 +62,10 @@ pub enum InputLayout { /// The base layout plus `memo_hash` last. How each public value maps to a /// field element per layout is the pallet's encoding. MemoBound, + /// The memo-bound layout with one Merkle root per input + /// (`merkle_roots[0], merkle_roots[1]` in place of `merkle_root`), so the two + /// notes may come from different trees. Transfer only. + CrossTree, } /// The layout a key of `arity` public inputs implies for `circuit_id`, or `None` @@ -64,10 +78,31 @@ pub const fn input_layout(circuit_id: u8, arity: usize) -> Option { Some(base) if arity == base + MEMO_HASH_INPUTS && has_memo_layout(circuit_id) => { Some(InputLayout::MemoBound) } + Some(base) + if arity == base + MEMO_HASH_INPUTS + CROSS_TREE_INPUTS + && has_cross_tree_layout(circuit_id) => + { + Some(InputLayout::CrossTree) + } Some(_) => None, } } +/// Public inputs of `circuit_id`'s widest layout, or `None` if unknown. The one +/// place that adds up the layouts, so weights and caps follow a new one. +pub const fn max_public_inputs(circuit_id: u8) -> Option { + let Some(base) = expected_public_inputs(circuit_id) else { + return None; + }; + Some(if has_cross_tree_layout(circuit_id) { + base + MEMO_HASH_INPUTS + CROSS_TREE_INPUTS + } else if has_memo_layout(circuit_id) { + base + MEMO_HASH_INPUTS + } else { + base + }) +} + // ─── Tests ──────────────────────────────────────────────────────────────────── #[cfg(test)] @@ -112,11 +147,34 @@ mod tests { Some(InputLayout::MemoBound) ); assert_eq!(input_layout(id, 6), None); - assert_eq!(input_layout(id, 9), None); + assert_eq!(input_layout(id, 10), None); } assert_eq!(input_layout(200, 3), Some(InputLayout::Base)); } + #[test] + fn only_a_transfer_has_the_cross_tree_layout() { + assert_eq!( + input_layout(CIRCUIT_ID_TRANSFER, 9), + Some(InputLayout::CrossTree) + ); + assert_eq!(input_layout(CIRCUIT_ID_UNSHIELD, 9), None); + assert_eq!(input_layout(CIRCUIT_ID_SHIELD, 5), None); + } + + #[test] + fn max_public_inputs_is_each_circuits_widest_layout() { + assert_eq!(max_public_inputs(CIRCUIT_ID_TRANSFER), Some(9)); + assert_eq!(max_public_inputs(CIRCUIT_ID_UNSHIELD), Some(8)); + assert_eq!(max_public_inputs(CIRCUIT_ID_SHIELD), Some(3)); + assert_eq!(max_public_inputs(200), None); + for id in [CIRCUIT_ID_TRANSFER, CIRCUIT_ID_UNSHIELD, CIRCUIT_ID_SHIELD] { + let widest = max_public_inputs(id).unwrap(); + assert!(input_layout(id, widest).is_some()); + assert_eq!(input_layout(id, widest + 1), None); + } + } + #[test] fn shield_has_only_the_base_layout() { assert!(!has_memo_layout(CIRCUIT_ID_SHIELD)); @@ -126,24 +184,24 @@ mod tests { assert_eq!(input_layout(CIRCUIT_ID_SHIELD, 2), None); } - /// Every circuit in use sits far below the cap, so enforcing it cannot break - /// a real verification. + /// Every layout in use, each circuit's widest included, sits well below the + /// cap, so enforcing it cannot break a real verification. /// /// A `const` block rather than a `#[test]`: these are all constants, so the /// comparison is decided at compile time either way — this way raising a /// circuit's arity past the cap fails the build instead of a test run. const _: () = { - assert!( - TRANSFER_PUBLIC_INPUTS * 4 < MAX_PUBLIC_INPUTS, - "transfer arity is too close to MAX_PUBLIC_INPUTS" - ); - assert!( - UNSHIELD_PUBLIC_INPUTS * 4 < MAX_PUBLIC_INPUTS, - "unshield arity is too close to MAX_PUBLIC_INPUTS" - ); - assert!( - SHIELD_PUBLIC_INPUTS * 4 < MAX_PUBLIC_INPUTS, - "shield arity is too close to MAX_PUBLIC_INPUTS" - ); + let ids = [CIRCUIT_ID_TRANSFER, CIRCUIT_ID_UNSHIELD, CIRCUIT_ID_SHIELD]; + let mut i = 0; + while i < ids.len() { + match max_public_inputs(ids[i]) { + Some(widest) => assert!( + widest * 3 < MAX_PUBLIC_INPUTS, + "a circuit's arity is too close to MAX_PUBLIC_INPUTS" + ), + None => panic!("a known circuit has no arity"), + } + i += 1; + } }; } diff --git a/primitives/zk-verifier/src/lib.rs b/primitives/zk-verifier/src/lib.rs index c041471e..dcf331f0 100644 --- a/primitives/zk-verifier/src/lib.rs +++ b/primitives/zk-verifier/src/lib.rs @@ -28,15 +28,16 @@ pub use ark_groth16::PreparedVerifyingKey; // ─── Public API ─────────────────────────────────────────────────────────────── pub use circuits::{ - expected_public_inputs, has_memo_layout, input_layout, InputLayout, CIRCUIT_ID_SHIELD, - CIRCUIT_ID_TRANSFER, CIRCUIT_ID_UNSHIELD, MEMO_HASH_INPUTS, SHIELD_PUBLIC_INPUTS, - TRANSFER_PUBLIC_INPUTS, UNSHIELD_PUBLIC_INPUTS, + expected_public_inputs, has_cross_tree_layout, has_memo_layout, input_layout, + max_public_inputs, InputLayout, CIRCUIT_ID_SHIELD, CIRCUIT_ID_TRANSFER, CIRCUIT_ID_UNSHIELD, + CROSS_TREE_INPUTS, MEMO_HASH_INPUTS, SHIELD_PUBLIC_INPUTS, TRANSFER_PUBLIC_INPUTS, + UNSHIELD_PUBLIC_INPUTS, }; pub use snarkjs::SnarkjsProofPoints; #[cfg(feature = "std")] pub use snarkjs::{parse_proof_from_snarkjs, parse_public_inputs_from_snarkjs}; pub use types::{ to_field_le, Proof, PublicInputs, VerifierError, VerifyingKey, BASE_VERIFICATION_COST, - MAX_PUBLIC_INPUTS, MAX_VK_BYTES, PER_INPUT_COST, + MAX_PUBLIC_INPUTS, MAX_VK_BYTES, PER_INPUT_COST, PROOF_BYTES, }; pub use verifier::Groth16Verifier; diff --git a/primitives/zk-verifier/src/types.rs b/primitives/zk-verifier/src/types.rs index 8d95f7fb..d04e69bf 100644 --- a/primitives/zk-verifier/src/types.rs +++ b/primitives/zk-verifier/src/types.rs @@ -34,13 +34,10 @@ pub const MAX_PUBLIC_INPUTS: usize = 32; /// dispatchable. pub const MAX_VK_BYTES: usize = 8192; -/// Largest proof this crate will attempt to deserialize. -/// -/// A compressed Groth16 proof over BN254 is three curve points — 128 bytes, -/// fixed. The margin is for encoding variations, not for growth: anything an -/// order of magnitude past this is malformed, and letting it through only gives -/// the deserializer a length prefix to act on. -pub const MAX_PROOF_BYTES: usize = 1024; +/// Exact size of a compressed Groth16 proof over BN254: `A` (G1, 32) + `B` (G2, +/// 64) + `C` (G1, 32). The deserializer ignores trailing bytes, so without this +/// bound a proof padded out would be a second encoding of the same proof. +pub const PROOF_BYTES: usize = 128; // ─── VerifierError ──────────────────────────────────────────────────────────── /// Errors that can occur during proof verification. @@ -109,7 +106,7 @@ impl Proof { } pub fn to_ark_proof(&self) -> Result, VerifierError> { - if self.bytes.len() > MAX_PROOF_BYTES { + if self.bytes.len() != PROOF_BYTES { return Err(VerifierError::InvalidProof); } ArkProof::::deserialize_compressed(&self.bytes[..]) @@ -490,6 +487,107 @@ mod tests { assert_eq!(result.unwrap().len(), 2); } + /// A point on the twist curve outside the prime-order G2 subgroup. Found by + /// walking x coordinates: the cofactor is huge, so almost any curve point + /// qualifies, and the check says which. + fn off_subgroup_g2() -> ark_bn254::G2Affine { + use ark_bn254::{Fq, Fq2, G2Affine}; + (1u64..) + .filter_map(|i| { + G2Affine::get_point_from_x_unchecked(Fq2::new(Fq::from(i), Fq::from(1u64)), false) + }) + .find(|p| !p.is_in_correct_subgroup_assuming_on_curve()) + .expect("a curve point outside the subgroup exists") + } + + fn proof_bytes( + a: ark_bn254::G1Affine, + b: ark_bn254::G2Affine, + c: ark_bn254::G1Affine, + ) -> Vec { + use ark_serialize::CanonicalSerialize; + let mut bytes = Vec::new(); + a.serialize_compressed(&mut bytes).unwrap(); + b.serialize_compressed(&mut bytes).unwrap(); + c.serialize_compressed(&mut bytes).unwrap(); + bytes + } + + /// Pairings on a G2 point outside the subgroup are not bilinear, which is how + /// a forged proof could pass a verifier that skips the subgroup check. The + /// deserializer must refuse the point, for a proof's `B` and for every G2 + /// element of a key. + #[test] + fn a_g2_point_outside_the_subgroup_is_refused_in_proofs_and_keys() { + use ark_bn254::{G1Affine, G2Affine}; + use ark_ec::AffineRepr; + let off = off_subgroup_g2(); + assert!(off.is_on_curve()); + assert!(!off.is_in_correct_subgroup_assuming_on_curve()); + + let proof = Proof::new(proof_bytes( + G1Affine::generator(), + off, + G1Affine::generator(), + )); + assert!(proof.to_ark_proof().is_err()); + // The same bytes with a subgroup point deserialize: the refusal is the point. + let fine = Proof::new(proof_bytes( + G1Affine::generator(), + G2Affine::generator(), + G1Affine::generator(), + )); + assert!(fine.to_ark_proof().is_ok()); + + assert!(VerifyingKey::new(vk_bytes(3, |vk| vk.beta_g2 = off)) + .to_ark_vk() + .is_err()); + assert!(VerifyingKey::new(vk_bytes(3, |vk| vk.gamma_g2 = off)) + .to_ark_vk() + .is_err()); + assert!(VerifyingKey::new(vk_bytes(3, |vk| vk.delta_g2 = off)) + .to_ark_vk() + .is_err()); + } + + /// Proof points at infinity, all-zero and all-0xff bytes: refused or failing + /// verification, never a panic. + #[test] + fn degenerate_proof_bytes_never_verify_or_panic() { + use ark_bn254::{G1Affine, G2Affine}; + use ark_ec::AffineRepr; + let pvk = VerifyingKey::new(well_formed_vk(2)).prepare().unwrap(); + let inputs = PublicInputs::new(vec![[1u8; 32], [2u8; 32]]); + let len = proof_bytes( + G1Affine::generator(), + G2Affine::generator(), + G1Affine::generator(), + ) + .len(); + let candidates = [ + proof_bytes(G1Affine::zero(), G2Affine::zero(), G1Affine::zero()), + proof_bytes( + G1Affine::zero(), + G2Affine::generator(), + G1Affine::generator(), + ), + vec![0u8; len], + vec![0xffu8; len], + vec![0u8; len - 1], + vec![0u8; len + 1], + ]; + for bytes in candidates { + let proof = Proof::new(bytes); + let res = proof + .to_ark_proof() + .map_err(|_| VerifierError::VerificationFailed) + .and_then(|_| { + crate::Groth16Verifier::verify_with_prepared_vk(&pvk, &inputs, &proof) + }); + assert!(res.is_err()); + } + } + #[test] fn to_field_elements_rejects_non_canonical() { use ark_ff::{BigInteger, PrimeField}; @@ -662,32 +760,27 @@ mod tests { assert!(vk.num_public_inputs().is_err()); } - /// A proof past the bound is refused on size alone. - /// /// `deserialize_compressed` ignores trailing bytes, so a real proof padded - /// out still deserializes — which makes it a fixture the guard is the only - /// thing rejecting. + /// out would still deserialize: another byte string for the same proof. Only + /// the exact compressed size is a proof. #[test] - fn oversized_proof_is_rejected_on_size() { + fn a_proof_with_trailing_bytes_is_refused() { use ark_bn254::{G1Affine, G2Affine}; use ark_ec::AffineRepr; - - let proof = ArkProof:: { - a: G1Affine::generator(), - b: G2Affine::generator(), - c: G1Affine::generator(), - }; - let mut bytes = Proof::from_ark_proof(&proof).expect("serializes").bytes; - assert!( - Proof::new(bytes.clone()).to_ark_proof().is_ok(), - "fixture must be valid first" - ); - - bytes.resize(MAX_PROOF_BYTES + 1, 0u8); - assert_eq!( - Proof::new(bytes).to_ark_proof(), - Err(VerifierError::InvalidProof) + let bytes = proof_bytes( + G1Affine::generator(), + G2Affine::generator(), + G1Affine::generator(), ); + assert_eq!(bytes.len(), PROOF_BYTES); + assert!(Proof::new(bytes.clone()).to_ark_proof().is_ok()); + for padding in [vec![0u8], vec![0xffu8; 8], vec![0u8; 896]] { + let padded = [bytes.clone(), padding].concat(); + assert_eq!( + Proof::new(padded).to_ark_proof(), + Err(VerifierError::InvalidProof) + ); + } } /// The limits must not shadow real input: a genuine BN254 key is ~488 bytes @@ -713,10 +806,7 @@ mod tests { .expect("serializes") .bytes .len(); - assert!( - real * 4 < MAX_PROOF_BYTES, - "proof bound too tight: {real} bytes" - ); + assert_eq!(real, PROOF_BYTES, "proof bound too tight: {real} bytes"); } /// `MAX_PUBLIC_INPUTS` is enforced here: the pallet bounds its extrinsic diff --git a/scripts/vk/workflows/rotate-dev.sh b/scripts/vk/workflows/rotate-dev.sh index 42ae1aeb..d6e59c27 100755 --- a/scripts/vk/workflows/rotate-dev.sh +++ b/scripts/vk/workflows/rotate-dev.sh @@ -3,17 +3,18 @@ set -euo pipefail # ============================================================================ # scripts/vk/workflows/rotate-dev.sh -# DEV on-chain VK rotation for the 2 supported circuits. +# DEV on-chain VK rotation for the spend circuits (transfer, unshield). # # The spec-16 v1 → v2 rotation is done by Root after `setCode`, with services # stopped (docs/DEPLOYMENT_FLOW.md, runbook D): the calls below, in this order. # # Flow: -# 1) check the new keys: memo-bound arity, hash ≠ old version's, and equal to -# the manifest's vk_hash when a manifest is given -# 2) register NEW_VERSION on 1,2 and make it active (one tx) — skipped for a -# circuit that already has it, so a failed run can be repeated -# 3) retire OLD_VERSION on 1,2 right away (unless --keep-old) +# 1) check the new keys: arity (memo-bound, or cross-tree for a transfer), +# hash ≠ old version's, and equal to the manifest's vk_hash when a manifest +# is given +# 2) register NEW_VERSION and make it active (one tx for both circuits) — +# skipped for a circuit that already has it, so a failed run can be repeated +# 3) retire OLD_VERSION right away (unless --keep-old) # 4) optional: remove OLD_VERSION (--remove-old) # 5) check the final state: active = NEW, supported = [NEW] (or [OLD,NEW] # with --keep-old), on-chain hashes = the new keys' @@ -29,6 +30,8 @@ set -euo pipefail # --keep-old leave old_version live # --remove-old also remove old_version at the end # --manifest circuits manifest.json to check the keys' vk_hash against +# --circuits circuits to rotate, comma-separated (default transfer,unshield); +# e.g. `--circuits transfer 3 ws://... //Alice 2` for transfer v3 # # Keys are read from artifacts/verification_key__v.json # (override the directory with VK_ARTIFACTS_DIR). @@ -46,12 +49,14 @@ log() { REMOVE_OLD=false KEEP_OLD=false MANIFEST="" +CIRCUITS="transfer unshield" POSITIONAL=() while [[ $# -gt 0 ]]; do case "$1" in --remove-old) REMOVE_OLD=true; shift ;; --keep-old) KEEP_OLD=true; shift ;; --manifest) MANIFEST="${2:-}"; [[ -f "$MANIFEST" ]] || err "--manifest needs a file"; shift 2 ;; + --circuits) CIRCUITS="${2//,/ }"; shift 2 ;; --*) err "unknown flag $1" ;; *) POSITIONAL+=("$1"); shift ;; esac @@ -111,15 +116,29 @@ v=int(sys.argv[1]); r=(json.load(sys.stdin).get("result") or {}) print(next((h["vk_hash"].removeprefix("0x") for h in r.get("vk_hashes",[]) if h["version"]==v), ""))' "$2" } +CID_transfer=1 +CID_unshield=2 +for circuit in $CIRCUITS; do + [[ "$circuit" == transfer || "$circuit" == unshield ]] || err "unknown circuit $circuit" +done + +# Public inputs a version past 1 may have: memo-bound (base 7 + memo_hash), or +# for a transfer also cross-tree (+ a second root). The runtime refuses anything +# else, but failing here names the file. +accepted_arity() { + case "$1" in + transfer) echo "8 9" ;; + unshield) echo "8" ;; + esac +} + # ── 1. check the new keys ──────────────────────────────────────────────────── -for circuit in transfer unshield; do +for circuit in $CIRCUITS; do json="$ARTIFACTS_DIR/verification_key_${circuit}_v${NEW_VERSION}.json" [[ -f "$json" ]] || err "missing $json" n_public=$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["nPublic"])' "$json") - # Versions past 1 must be memo-bound: base 7 inputs + memo_hash. The runtime - # refuses anything else, but failing here names the file. - if [[ "$NEW_VERSION" != 1 && "$n_public" != 8 ]]; then - err "$circuit v$NEW_VERSION has nPublic=$n_public; a memo-bound key has 8" + if [[ "$NEW_VERSION" != 1 && " $(accepted_arity "$circuit") " != *" $n_public "* ]]; then + err "$circuit v$NEW_VERSION has nPublic=$n_public; expected one of: $(accepted_arity "$circuit")" fi bin=$(pack_verifying_key "$json") eval "BIN_$circuit=\"\$bin\"" @@ -131,30 +150,41 @@ for circuit in transfer unshield; do fi done -CID_transfer=1 -CID_unshield=2 -for circuit in transfer unshield; do +for circuit in $CIRCUITS; do cid_var="CID_$circuit" hash_var="HASH_$circuit" old=$(chain_hash "${!cid_var}" "$OLD_VERSION") [[ "${!hash_var}" != "$old" ]] || err "$circuit: the new key is the old version's key" done -log "RPC: $RPC_WS | old v$OLD_VERSION → new v$NEW_VERSION | keep_old=$KEEP_OLD remove_old=$REMOVE_OLD" +CIDS="" +for circuit in $CIRCUITS; do + cid_var="CID_$circuit" + CIDS="$CIDS ${!cid_var}" +done + +log "RPC: $RPC_WS | circuits: $CIRCUITS | old v$OLD_VERSION → new v$NEW_VERSION | keep_old=$KEEP_OLD remove_old=$REMOVE_OLD" # ── 2. register + activate (skipped when already there) ────────────────────── -existing_transfer=$(chain_hash 1 "$NEW_VERSION") -existing_unshield=$(chain_hash 2 "$NEW_VERSION") -if [[ -z "$existing_transfer" && -z "$existing_unshield" ]]; then +if [[ "$CIRCUITS" == "transfer unshield" \ + && -z "$(chain_hash 1 "$NEW_VERSION")" && -z "$(chain_hash 2 "$NEW_VERSION")" ]]; then log "Batch registering + activating both circuits (v$NEW_VERSION)..." bash "$VK_REGISTRY_SCRIPT" batch-register "$NEW_VERSION" 1 \ "$BIN_transfer" "$BIN_unshield" "$RPC_WS" "$SUDO_SEED" else - [[ "$existing_transfer" == "$HASH_transfer" && "$existing_unshield" == "$HASH_unshield" ]] \ - || err "v$NEW_VERSION is already registered with different keys" - log "v$NEW_VERSION already registered; making it active" - for cid in 1 2; do - bash "$VK_REGISTRY_SCRIPT" set-active "$cid" "$NEW_VERSION" "$RPC_WS" "$SUDO_SEED" + for circuit in $CIRCUITS; do + cid_var="CID_$circuit" + bin_var="BIN_$circuit" + hash_var="HASH_$circuit" + existing=$(chain_hash "${!cid_var}" "$NEW_VERSION") + if [[ -z "$existing" ]]; then + log "[$circuit] register v$NEW_VERSION" + bash "$VK_REGISTRY_SCRIPT" register "${!cid_var}" "$NEW_VERSION" "${!bin_var}" "$RPC_WS" "$SUDO_SEED" + else + [[ "$existing" == "${!hash_var}" ]] || err "$circuit v$NEW_VERSION is already registered with a different key" + fi + log "[$circuit] make v$NEW_VERSION active" + bash "$VK_REGISTRY_SCRIPT" set-active "${!cid_var}" "$NEW_VERSION" "$RPC_WS" "$SUDO_SEED" done fi @@ -165,7 +195,7 @@ import json,sys r=(json.load(sys.stdin).get("result") or {}); print(int(sys.argv[1]) in (r.get("supported_versions") or []))' "$2" } if [[ "$KEEP_OLD" != true ]]; then - for cid in 1 2; do + for cid in $CIDS; do if [[ "$(supported_has "$cid" "$OLD_VERSION")" == True ]]; then log "[circuit $cid] retire v$OLD_VERSION" bash "$VK_REGISTRY_SCRIPT" retire "$cid" "$OLD_VERSION" "$RPC_WS" "$SUDO_SEED" @@ -173,7 +203,7 @@ if [[ "$KEEP_OLD" != true ]]; then done fi if [[ "$REMOVE_OLD" == true ]]; then - for cid in 1 2; do + for cid in $CIDS; do if [[ -n "$(chain_hash "$cid" "$OLD_VERSION")" ]]; then log "[circuit $cid] remove v$OLD_VERSION" bash "$VK_REGISTRY_SCRIPT" remove "$cid" "$OLD_VERSION" "$RPC_WS" "$SUDO_SEED" @@ -182,7 +212,7 @@ if [[ "$REMOVE_OLD" == true ]]; then fi # ── 5. final state ─────────────────────────────────────────────────────────── -for circuit in transfer unshield; do +for circuit in $CIRCUITS; do cid_var="CID_$circuit" cid="${!cid_var}" info=$(rpc zkVerifier_getCircuitVersionInfo "$cid") @@ -200,5 +230,8 @@ PY [[ "$(chain_hash "$cid" "$NEW_VERSION")" == "${!hash_var}" ]] || err "$circuit: on-chain v$NEW_VERSION hash differs" done -rm -f "$BIN_transfer" "$BIN_unshield" +for circuit in $CIRCUITS; do + bin_var="BIN_$circuit" + rm -f "${!bin_var}" +done log "✅ VK rotation completed" diff --git a/template/runtime/RUNTIME_VERSIONS.md b/template/runtime/RUNTIME_VERSIONS.md index f285c555..12ca4b72 100644 --- a/template/runtime/RUNTIME_VERSIONS.md +++ b/template/runtime/RUNTIME_VERSIONS.md @@ -20,11 +20,15 @@ to `spec_version` / `transaction_version` must add a row here in the same PR. The genesis reset (`69d1b837`) set `spec_version` back to 1 and `transaction_version` to 1 for the public testnet launch. -### spec 18 — tx 5 — [Unreleased] - -Governance can require a minimum node version from block authors. -`transaction_version` stays: validator-set only gains calls 6 and 7. Ships with -`orbinum-runtime` / `orbinum-node` 0.4.0 and `pallet-validator-set` 0.4.0; +### spec 18 — tx 6 — [Unreleased] + +Governance can require a minimum node version from block authors, and a private +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, +`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. #### 1 · Minimum author version @@ -43,6 +47,57 @@ authoring a block without one, drops them. Old binaries run spec 18 unchanged (no new host function). Setting a minimum is what removes them from authoring; they keep importing and voting in GRANDPA. +#### 2 · Transfer across trees (circuit 1, v3) + +The transfer circuit proved both inputs against one root, so two notes from +different trees of the forest could not be spent together and wallets had to +consolidate first. Transfer v3 (9 public inputs: `merkle_roots[0..1]`, then the +v2 inputs) proves each input against its own root. + +- **Call:** `private_transfer` takes `merkle_roots: [Hash; 2]`, in input order. + Every root must be known (active, recent or a sealed tree's final root). The + circuit leaves a dummy input's root free, so the pallet requires it to repeat + the real input's (`InvalidPublicSignals`, pool code 13). +- **Older keys:** a v1 or v2 key attests to one root, so a spend whose roots + differ fails verification under it (`ProofVerificationFailed`). Same-tree spends + submit `[r, r]` and verify under any version. +- **zk-verifier:** a transfer key of 9 inputs registers as the cross-tree layout; + an unshield key of 9 is still refused. `verification_weight` covers 9 inputs. +- **Relay op hash:** `transfer_op_hash` hashes both roots, for every version. +- **EVM precompile:** `privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)`, + selector `0x63d0b9a0`; slot 1 becomes the offset of a two-root array, the fee + stays at slot 6. The single-root selector `0x66ed2cd4` is gone. If + `pallet_relayer::AllowedSelectors` is set on the chain, Root adds the new one. + +**Also:** sealed-tree Merkle paths no longer read leaves of the next tree when +`MaxLeavesPerTree` is below 2^20 (not the case on testnet; found with 8-leaf test +trees). + +#### 3 · Canonical spending key (transfer v3, unshield v3) + +Transfer v1/v2 and unshield v1/v2 accept spending keys that are not reduced +modulo the Baby JubJub subgroup order, so one note admits several valid +nullifiers. Transfer v3 and unshield v3 (`@orbinum/circuits`, `SpendingKeyOwner`) +require a canonical key, which gives each note exactly one nullifier. No runtime +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.** + +**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)`. + +| Circuit | Version | `vk_hash` | +|---|---|---| +| transfer (1) | 3 | `0x04e05d74b320601fe1630ffa5b07c617117340c71d9f8d7b639a479f621a769a` | +| unshield (2) | 3 | `0x24fb6a97b53300b2cf2effa83547ac5a913c0b83f99788dd428ce54498f4e9f8` | + +Do **not** register the v3 keys of circuits 0.17.0: they come from a development +ceremony. Until v3 is registered, same-tree transfers keep working under v2 and +cross-tree ones are refused. Clients +(`@orbinum/protocol`, wallet-sdk, app) ship with the upgrade: the new +transaction version invalidates the old call encoding. + ### spec 17 — tx 5 — [Unreleased] A shield must prove its note is worth exactly the deposit. `transaction_version` diff --git a/template/runtime/src/lib.rs b/template/runtime/src/lib.rs index 64c4cb5b..86cf6041 100644 --- a/template/runtime/src/lib.rs +++ b/template/runtime/src/lib.rs @@ -163,7 +163,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_version: 18, impl_version: 1, apis: RUNTIME_API_VERSIONS, - transaction_version: 5, + transaction_version: 6, system_version: 1, }; diff --git a/ts-tests/tests/test-relay-rpc.ts b/ts-tests/tests/test-relay-rpc.ts index 52ef8fb7..da412d1a 100644 --- a/ts-tests/tests/test-relay-rpc.ts +++ b/ts-tests/tests/test-relay-rpc.ts @@ -23,7 +23,7 @@ let MIN_RELAY_FEE: bigint; /// a wrong selector still produces "unsupported selector", so the negative /// cases go green while the positive ones silently test nothing. const SIG_UNSHIELD = "unshield(bytes,bytes32,bytes32,uint32,uint256,bytes32,uint256,bytes32,bytes,uint32)"; -const SIG_PRIVATE_TRANSFER = "privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)"; +const SIG_PRIVATE_TRANSFER = "privateTransfer(bytes,bytes32[],bytes32[],bytes32[],bytes[],uint32,uint256,uint32)"; const SEL_UNSHIELD = ethers.id(SIG_UNSHIELD).slice(2, 10); const SEL_PRIVATE_TRANSFER = ethers.id(SIG_PRIVATE_TRANSFER).slice(2, 10); @@ -73,10 +73,10 @@ function buildUnshieldCalldata(fee: bigint): string { */ function buildPrivateTransferCalldata(fee: bigint): string { const encoded = abiCoder.encode( - ["bytes", "bytes32", "bytes32[]", "bytes32[]", "bytes[]", "uint32", "uint256", "uint32"], + ["bytes", "bytes32[]", "bytes32[]", "bytes32[]", "bytes[]", "uint32", "uint256", "uint32"], [ "0x" + "aa".repeat(32), // proof - "0x" + "bb".repeat(32), // merkle root + ["0x" + "bb".repeat(32), "0x" + "bb".repeat(32)], // merkle roots, one per input ["0x" + "cc".repeat(32)], // nullifiers[] ["0x" + "dd".repeat(32)], // output commitments[] ["0x" + "ee".repeat(180)], // encrypted memos[] (must be exactly 180 bytes) From 6f4e26b493ef6147dae32d8a1cb0aa8c209e4f63 Mon Sep 17 00:00:00 2001 From: nol4lej Date: Thu, 8 Oct 2026 19:22:32 -0300 Subject: [PATCH 2/2] test(shielded-pool): don't read the verifier recorder under skip-proof-verification --- .../src/tests/private_transfer.rs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/frame/shielded-pool/src/tests/private_transfer.rs b/frame/shielded-pool/src/tests/private_transfer.rs index c53411ca..bbad5fc5 100644 --- a/frame/shielded-pool/src/tests/private_transfer.rs +++ b/frame/shielded-pool/src/tests/private_transfer.rs @@ -77,11 +77,11 @@ fn execute_works_two_notes() { } /// Two notes from two trees: one proven against a sealed tree's final root, the -/// other against the active tree. Both roots reach the verifier, in input order. +/// other against the active tree. Both are spent, and both roots reach the +/// verifier in input order (nothing does under `skip-proof-verification`). #[test] fn execute_spends_notes_from_two_trees() { new_test_ext().execute_with(|| { - use crate::mock::{VerifiedStatement, verified_statements}; let sealed = [0x5E; 32]; MerkleRepository::insert_sealed_root::(0, sealed); MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); @@ -98,11 +98,18 @@ fn execute_spends_notes_from_two_trees() { circuit_version: 3, }, )); - let Some(VerifiedStatement::Transfer(statement, Some(3))) = verified_statements().pop() - else { - panic!("the transfer statement was not verified"); - }; - assert_eq!(statement.merkle_roots, [sealed, KNOWN_ROOT]); + for seed in [0xA1, 0xA2] { + assert!(NullifierRepository::is_used::(&nullifier(seed))); + } + #[cfg(not(feature = "skip-proof-verification"))] + { + use crate::mock::{VerifiedStatement, verified_statements}; + let Some(VerifiedStatement::Transfer(statement, Some(3))) = verified_statements().pop() + else { + panic!("the transfer statement was not verified"); + }; + assert_eq!(statement.merkle_roots, [sealed, KNOWN_ROOT]); + } }); }