diff --git a/docs/zk-trie-architecture.md b/docs/zk-trie-architecture.md index 980c9ace..ddb070bb 100644 --- a/docs/zk-trie-architecture.md +++ b/docs/zk-trie-architecture.md @@ -63,10 +63,46 @@ Depth 3 (capacity: 64 leaves) | 2 | 16 | Grows automatically | | 3 | 64 | | | ... | ... | | -| 32 | ~1.8 × 10^19 | Maximum supported depth | +| 16 | ~4.3 × 10^9 | Max depth the **circuits** accept (see below) | +| 32 | ~1.8 × 10^19 | Max depth the on-chain tree may grow to | The tree grows dynamically -- when the 5th leaf arrives, depth increases from 1 to 2. The old root becomes child[0] of a new root node. +### Circuit depth limit (known, accepted limitation) + +The on-chain tree may grow up to depth 32 (`MAX_TREE_DEPTH` in `pallets/zk-tree`), but the +wormhole circuits only accept Merkle paths up to depth 16 (`MAX_DEPTH` in +`qp-zk-circuits-common/src/zk_merkle.rs`). The circuit pads every proof's witness to the +full `MAX_DEPTH` levels, so **every leaf proof pays the proving cost of a depth-16 path +regardless of the tree's actual depth** -- that is why the circuit constant is kept as +small as safely possible instead of matching the on-chain cap. + +**What happens at the limit:** once leaf 4^16 + 1 (~4.3 billion) is inserted, the tree +grows to depth 17, all Merkle proofs gain a 17th sibling level, and the prover and +on-chain verifier reject them. Existing funds are never lost and nullifier state is +untouched -- wormhole proof *generation* simply halts until the circuit is updated. + +**The plan is to do a circuit update when (long before) that happens.** Rough timeline +to exhaustion at 12-second blocks: + +| Sustained leaf rate | Time to 4.3 B leaves | +|---|---| +| 1 leaf/block (mining-reward floor) | ~1,600 years | +| 10 transfers/sec chain-wide | ~13 years | +| ~50 transfers/sec (permanently full blocks) | ~2.5 years | + +`LeafCount` is public storage, so the approach is observable years ahead; each +1 of +circuit depth quadruples capacity (e.g. 16 → 20 buys ~256× the runway). + +**What the update involves:** bump `MAX_DEPTH` in `qp-zk-circuits-common`, release the +circuit crates, rebuild -- `pallets/wormhole/build.rs` regenerates and embeds the new +verifier binaries automatically -- regenerate the proof test fixtures +(`regenerate_*_fixture` tests), re-benchmark weights, and ship a normal runtime upgrade. +The code change is a one-line constant; the end-to-end effort is on the order of days of +engineering inside a standard release cycle. Proofs built against the old circuit become +invalid at the upgrade (wallets/provers must update in step), but spent nullifiers +persist, so nothing can double-spend across the transition. + ### Hashing Strategy | Layer | Encoding | Felts | Injective? | diff --git a/node/src/zktree_rpc.rs b/node/src/zktree_rpc.rs index a786e225..1b725d05 100644 --- a/node/src/zktree_rpc.rs +++ b/node/src/zktree_rpc.rs @@ -70,6 +70,12 @@ pub trait ZkTreeApi { /// the proving block's hash is still in `frame_system::BlockHash`, a sliding /// window of `BlockHashCount` blocks. Blocks outside that window are rejected. /// +/// The requested hash must also be the *canonical* hash at its height. The backend +/// resolves numbers for any imported block (side forks included), but settlement +/// verifies the claimed hash against `frame_system::BlockHash`, so a proof built on +/// fork state is unusable by construction — reject it here instead of spending +/// state-execution resources producing it. +/// /// The window is `quantus_runtime::configs::BlockHashCount` from the runtime /// crate linked into this node binary — a compile-time constant, not a live /// chain/metadata lookup. After a forkless upgrade that changes @@ -104,6 +110,43 @@ where )), }; + // The backend resolves a number for ANY block it has imported, including + // side-fork blocks — resolvability is not canonicality. On-chain settlement + // compares the proof's claimed hash against `frame_system::BlockHash` (the + // canonical chain), so proof material derived from fork state can never + // settle. Reject anything that is not the canonical hash at its height; + // heights above best (where `best - number` saturates to 0) are rejected + // first for a precise error. + if number > info.best_number { + return Err(jsonrpsee::types::error::ErrorObject::owned( + 9007, + format!( + "Block {hash:?} (#{number}) is above the current best block \ + (#{best}); it is not on the canonical chain", + best = info.best_number, + ), + None::<()>, + )); + } + + let canonical = client.hash(number).map_err(|e| { + jsonrpsee::types::error::ErrorObject::owned( + 9006, + format!("Failed to resolve canonical hash at #{number}: {e}"), + None::<()>, + ) + })?; + if canonical != Some(hash) { + return Err(jsonrpsee::types::error::ErrorObject::owned( + 9008, + format!( + "Block {hash:?} (#{number}) is not on the canonical chain; proofs \ + against fork state cannot be verified on-chain" + ), + None::<()>, + )); + } + // Compile-time constant from the linked runtime crate — see fn docs. let window = >::get(); if info.best_number.saturating_sub(number) > window { @@ -207,18 +250,25 @@ mod tests { H256::from_low_u64_be(u64::from(number) + 1) } - /// Minimal chain view: a best block and a set of known (hash -> number) blocks. + /// Minimal chain view: a best block, the known (hash -> number) blocks the + /// backend has imported (canonical *and* side-fork), and the canonical + /// (number -> hash) index. struct MockChain { best_number: u32, - blocks: HashMap, + /// Every imported block, like the backend's hash->number index. Includes + /// side-fork blocks, which is exactly why resolvability != canonicality. + known: HashMap, + /// The canonical chain's number->hash index. + canonical: HashMap, /// Hashes for which `number()` simulates a backend/DB failure. failing: HashSet, } impl MockChain { fn with_blocks(best_number: u32, numbers: &[u32]) -> Self { - let blocks = numbers.iter().map(|n| (hash_for(*n), *n)).collect(); - Self { best_number, blocks, failing: HashSet::new() } + let known = numbers.iter().map(|n| (hash_for(*n), *n)).collect(); + let canonical = numbers.iter().map(|n| (*n, hash_for(*n))).collect(); + Self { best_number, known, canonical, failing: HashSet::new() } } fn with_number_failure(best_number: u32, failing_hash: H256) -> Self { @@ -226,6 +276,14 @@ mod tests { chain.failing.insert(failing_hash); chain } + + /// Add a block the backend knows about (imported) that is NOT on the + /// canonical chain, at the given height. Returns its hash. + fn add_fork_block(&mut self, number: u32) -> H256 { + let fork_hash = H256::from_low_u64_be(0xF0_0000 + u64::from(number)); + self.known.insert(fork_hash, number); + fork_hash + } } impl HeaderBackend for MockChain { @@ -247,7 +305,7 @@ mod tests { } fn status(&self, hash: H256) -> BlockchainResult { - Ok(if self.blocks.contains_key(&hash) { + Ok(if self.known.contains_key(&hash) { BlockStatus::InChain } else { BlockStatus::Unknown @@ -258,11 +316,11 @@ mod tests { if self.failing.contains(&hash) { return Err(sp_blockchain::Error::Backend("simulated db failure".into())); } - Ok(self.blocks.get(&hash).copied()) + Ok(self.known.get(&hash).copied()) } fn hash(&self, number: NumberFor) -> BlockchainResult> { - Ok(self.blocks.iter().find(|(_, n)| **n == number).map(|(h, _)| *h)) + Ok(self.canonical.get(&number).copied()) } } @@ -305,6 +363,44 @@ mod tests { assert!(resolve_proof_block(&chain, Some(hash_for(ancient))).is_err()); } + /// The backend resolves a number for ANY imported block, including side-fork + /// blocks — resolvability is not canonicality. A proof generated against fork + /// state can never settle (the wormhole pallet compares the claimed hash to + /// `frame_system::BlockHash`, the canonical chain), so the RPC must reject + /// noncanonical hashes instead of burning state-execution resources on them. + #[test] + fn rejects_noncanonical_hashes_within_the_window() { + let best = 10 * window(); + let fork_height = best - 5; + let mut chain = MockChain::with_blocks(best, &[best, fork_height]); + let fork_hash = chain.add_fork_block(fork_height); + + let err = resolve_proof_block(&chain, Some(fork_hash)) + .expect_err("side-fork hash must be rejected even inside the proof window"); + assert_eq!(err.code(), 9008); + + // The canonical block at the same height is still accepted. + assert_eq!( + resolve_proof_block(&chain, Some(hash_for(fork_height))).unwrap(), + hash_for(fork_height) + ); + } + + /// A backend-known block ABOVE the current best (e.g. from a longer side + /// fork that was imported but not chosen) makes `best_number - number` + /// saturate to 0, which the one-sided window check happily accepts. Heights + /// above best have no canonical hash and can never settle. + #[test] + fn rejects_blocks_above_the_best_number() { + let best = 10 * window(); + let mut chain = MockChain::with_blocks(best, &[best]); + let ahead_hash = chain.add_fork_block(best + 5); + + let err = resolve_proof_block(&chain, Some(ahead_hash)) + .expect_err("block above best must be rejected"); + assert_eq!(err.code(), 9007); + } + #[test] fn rejects_unknown_block_hashes() { let best = 10 * window(); diff --git a/pallets/frame-system/src/lib.rs b/pallets/frame-system/src/lib.rs index ed5be494..96c74dc7 100644 --- a/pallets/frame-system/src/lib.rs +++ b/pallets/frame-system/src/lib.rs @@ -735,7 +735,12 @@ pub mod pallet { // 65536 pages (4 GiB) is the wasm32 linear-memory hard maximum. ensure!((64..=65536).contains(&pages), Error::::InvalidHeapPages); storage::unhashed::put_raw(well_known_keys::HEAP_PAGES, &pages.encode()); - Self::deposit_log(generic::DigestItem::RuntimeEnvironmentUpdated); + // NOTE: upstream deposits `DigestItem::RuntimeEnvironmentUpdated` here. This + // fork must not: the QPoW header commits a fixed digest window that the + // pre-runtime item and seal fill exactly, so ANY runtime-deposited digest + // item makes the sealed header unimportable network-wide (see `deposit_log`). + // Nothing in the node stack consumes the item — clients detect environment + // changes from the `:heappages`/`:code` state keys, not the digest. Ok(().into()) } @@ -1622,7 +1627,14 @@ impl Pallet { /// the storage (for instance in case of parachains). pub fn update_code_in_storage(code: &[u8]) { storage::unhashed::put_raw(well_known_keys::CODE, code); - Self::deposit_log(generic::DigestItem::RuntimeEnvironmentUpdated); + // NOTE: upstream deposits `DigestItem::RuntimeEnvironmentUpdated` here. This + // fork must not: the QPoW header commits a fixed digest window that the + // pre-runtime item and seal fill exactly, so ANY runtime-deposited digest + // item makes the sealed header unimportable network-wide (see `deposit_log`). + // A runtime upgrade would then be un-includable through normal block + // production. Clients detect the new code from the `:code` state key (the + // executor's module cache is keyed by code hash); the `CodeUpdated` event + // below remains for observability. Self::deposit_event(Event::CodeUpdated); } @@ -2102,6 +2114,20 @@ impl Pallet { } /// Deposits a log and ensures it matches the block's log data. + /// + /// # WARNING: the QPoW digest window has no spare capacity + /// + /// `qp_header::Header::hash()` commits the digest through a fixed + /// `DIGEST_LOGS_SIZE` window that the client-injected pre-runtime item plus the + /// PoW seal fill **exactly**, and block import rejects any sealed header whose + /// encoded digest exceeds it (truncating would let distinct headers share a + /// hash). A digest item deposited from runtime code therefore does not fail the + /// call — it makes the finished block **unimportable by the entire network**, + /// silently, after mining. This is why the fork's `set_code` / + /// `set_heap_pages` paths do not deposit `RuntimeEnvironmentUpdated` the way + /// upstream does. Do not deposit digest items from runtime logic unless the + /// header format and the wormhole circuit's digest field are resized in the + /// same release. pub fn deposit_log(item: generic::DigestItem) { >::append(item); } diff --git a/pallets/frame-system/src/tests.rs b/pallets/frame-system/src/tests.rs index 35a92df8..e9545a2f 100644 --- a/pallets/frame-system/src/tests.rs +++ b/pallets/frame-system/src/tests.rs @@ -653,7 +653,8 @@ fn set_code_checks_works() { ext.execute_with(|| { let res = System::set_code(RawOrigin::Root.into(), vec![1, 2, 3, 4]); - assert_runtime_updated_digest(if res.is_ok() { 1 } else { 0 }); + // Success or failure, no digest item may be deposited (QPoW window). + assert_no_deposited_digest_items(); assert_eq!(expected.map_err(DispatchErrorWithPostInfo::from), res); }); } @@ -755,15 +756,16 @@ fn validate_unsigned_apply_authorized_upgrade_honors_check_version() { } } -fn assert_runtime_updated_digest(num: usize) { +/// The QPoW header commits a fixed digest window that the pre-runtime item and +/// seal fill exactly, so a runtime-deposited digest item (like upstream's +/// `RuntimeEnvironmentUpdated`) makes the sealed block unimportable +/// network-wide. Environment-changing calls must deposit NO digest items. +fn assert_no_deposited_digest_items() { assert_eq!( - System::digest() - .logs - .into_iter() - .filter(|item| *item == generic::DigestItem::RuntimeEnvironmentUpdated) - .count(), - num, - "Incorrect number of Runtime Updated digest items", + System::digest().logs, + alloc::vec::Vec::new(), + "runtime code must not deposit digest items: the QPoW digest window has \ + no spare capacity and the sealed block would be rejected at import", ); } @@ -810,12 +812,12 @@ fn extrinsics_root_is_calculated_correctly() { } #[test] -fn runtime_updated_digest_emitted_when_heap_pages_changed() { +fn no_digest_item_deposited_when_heap_pages_changed() { new_test_ext().execute_with(|| { System::reset_events(); System::initialize(&1, &[0u8; 32].into(), &Default::default()); System::set_heap_pages(RawOrigin::Root.into(), 64).unwrap(); - assert_runtime_updated_digest(1); + assert_no_deposited_digest_items(); }); } @@ -834,10 +836,11 @@ fn set_heap_pages_validates_range() { ); } - // Both bounds of the allowed range are accepted and still emit the digest item. + // Both bounds of the allowed range are accepted, without depositing any + // digest item (the QPoW digest window has no spare capacity). assert_ok!(System::set_heap_pages(RawOrigin::Root.into(), 64)); assert_ok!(System::set_heap_pages(RawOrigin::Root.into(), 65536)); - assert_runtime_updated_digest(2); + assert_no_deposited_digest_items(); }); } diff --git a/pallets/multisig/src/lib.rs b/pallets/multisig/src/lib.rs index 2c9edb72..3f9659ea 100644 --- a/pallets/multisig/src/lib.rs +++ b/pallets/multisig/src/lib.rs @@ -613,8 +613,11 @@ pub mod pallet { // ===== PHASE 4: High-security whitelist check (if applicable) ===== // (additional read: HighSecurityAccounts) let is_high_security = T::HighSecurity::is_high_security(&multisig_address); - // Use the shared `is_call_allowed` policy so `propose` and `execute` stay consistent. - if !T::HighSecurity::is_call_allowed(&multisig_address, &decoded_call) { + // Apply the shared call policy (the same predicate `execute` consults via + // `is_call_allowed`) using the classification already fetched above for + // weight selection, so the `HighSecurityAccounts` lookup is not repeated — + // the propose weights charge exactly one classification read. + if !T::HighSecurity::is_call_allowed_given(is_high_security, &decoded_call) { // Don't refund after decode - same reasoning as above. return Self::err_burn_full(Error::::CallNotAllowedForHighSecurityMultisig); } diff --git a/pallets/recovery/src/lib.rs b/pallets/recovery/src/lib.rs index 5944c47c..9a24f13f 100644 --- a/pallets/recovery/src/lib.rs +++ b/pallets/recovery/src/lib.rs @@ -433,7 +433,13 @@ pub mod pallet { #[pallet::weight({ let dispatch_info = call.get_dispatch_info(); ( - T::WeightInfo::as_recovered().saturating_add(dispatch_info.call_weight), + T::WeightInfo::as_recovered() + // High-security policy check on the recovered account + // (`is_call_allowed` → one classification read in the runtime + // inspector); the benchmarked base runs with the no-op inspector + // and does not include it. + .saturating_add(T::DbWeight::get().reads(1)) + .saturating_add(dispatch_info.call_weight), dispatch_info.class, )})] pub fn as_recovered( diff --git a/pallets/recovery/src/mock.rs b/pallets/recovery/src/mock.rs index 90509579..b7bd68ec 100644 --- a/pallets/recovery/src/mock.rs +++ b/pallets/recovery/src/mock.rs @@ -37,6 +37,9 @@ construct_runtime!( impl frame_system::Config for Test { type Block = Block; type AccountData = pallet_balances::AccountData; + // A non-zero database weight so tests can observe the db-op components of + // the weights the dispatchables charge (the prelude default is zero). + type DbWeight = frame::deps::frame_support::weights::constants::RocksDbWeight; } parameter_types! { diff --git a/pallets/recovery/src/tests.rs b/pallets/recovery/src/tests.rs index 994bfa2b..15132670 100644 --- a/pallets/recovery/src/tests.rs +++ b/pallets/recovery/src/tests.rs @@ -32,6 +32,31 @@ fn basic_setup_works() { }); } +/// `as_recovered` consults the high-security policy on the recovered account +/// (`T::HighSecurity::is_call_allowed`) before dispatching, which in the runtime +/// costs one `HighSecurityAccounts` storage read. The benchmarked base ran with +/// the no-op inspector, so the declared weight must add that read explicitly. +#[test] +fn as_recovered_weight_charges_high_security_policy_read() { + new_test_ext().execute_with(|| { + let inner = RuntimeCall::System(frame_system::Call::remark { remark: vec![] }); + let inner_weight = inner.get_dispatch_info().call_weight; + let call = + RuntimeCall::Recovery(crate::Call::as_recovered { account: 5, call: Box::new(inner) }); + + let db = ::DbWeight::get(); + let without_policy_read = + ::WeightInfo::as_recovered().saturating_add(inner_weight); + + assert!( + call.get_dispatch_info() + .call_weight + .all_gte(without_policy_read.saturating_add(db.reads(1))), + "declared as_recovered weight must include the high-security policy read" + ); + }); +} + /// A Root-installed proxy must hold the same frame_system consumer reference as a /// `claim_recovery`-created one: the reference keeps the rescuer account alive while the /// proxy exists, and it backs the unconditional `dec_consumers` in `cancel_recovered`, diff --git a/pallets/reversible-transfers/src/lib.rs b/pallets/reversible-transfers/src/lib.rs index 9007468d..18a410d5 100644 --- a/pallets/reversible-transfers/src/lib.rs +++ b/pallets/reversible-transfers/src/lib.rs @@ -329,6 +329,9 @@ pub mod pallet { TooManyGuardianAccounts, /// Asset transfers are not supported. AssetsNotSupported, + /// Zero-amount transfers cannot be scheduled: there is nothing to hold, + /// execute, or reverse. + ZeroAmount, } #[pallet::call] @@ -753,6 +756,10 @@ pub mod pallet { ) -> DispatchResult { let recipient = T::Lookup::lookup(to.clone())?; ensure!(asset_id.is_none(), Error::::AssetsNotSupported); + // A zero-amount schedule is a pure no-op with side effects: it consumes a + // pending-transfer slot and scheduler agenda space, and its execution would + // dispatch a zero-value transfer. Reject it outright. + ensure!(!amount.is_zero(), Error::::ZeroAmount); // Build the transfer call for tx_id computation (not stored) let transfer_call: RuntimeCallOf = diff --git a/pallets/reversible-transfers/src/tests/test_reversible_transfers.rs b/pallets/reversible-transfers/src/tests/test_reversible_transfers.rs index 93fef9d0..dbc37b9a 100644 --- a/pallets/reversible-transfers/src/tests/test_reversible_transfers.rs +++ b/pallets/reversible-transfers/src/tests/test_reversible_transfers.rs @@ -213,6 +213,38 @@ fn set_reversibility_fails_delay_too_short() { }); } +/// A zero-amount schedule is a pure no-op with side effects: it consumes a +/// pending-transfer slot and scheduler agenda space, and its execution would +/// dispatch a zero-value transfer. Both signed scheduling entry points must +/// reject it before any state is written. +#[test] +fn schedule_transfer_rejects_zero_amount() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + + // High-security entry point (alice is high-security from genesis). + assert_err!( + ReversibleTransfers::schedule_transfer(RuntimeOrigin::signed(alice()), bob(), 0), + Error::::ZeroAmount + ); + + // One-time entry point (charlie is a regular account). + assert_err!( + ReversibleTransfers::schedule_transfer_with_delay( + RuntimeOrigin::signed(charlie()), + bob(), + 0, + BlockNumberOrTimestamp::BlockNumber(10), + ), + Error::::ZeroAmount + ); + + // Nothing was scheduled or stored on either path. + assert!(PendingTransfersBySender::::get(&alice()).is_empty()); + assert!(PendingTransfersBySender::::get(&charlie()).is_empty()); + }); +} + #[test] fn schedule_transfer_works() { new_test_ext().execute_with(|| { diff --git a/pallets/reversible-transfers/src/weights.rs b/pallets/reversible-transfers/src/weights.rs index 1d6f3cd4..a0992be4 100644 --- a/pallets/reversible-transfers/src/weights.rs +++ b/pallets/reversible-transfers/src/weights.rs @@ -66,10 +66,15 @@ const EXECUTE_TRANSFER_BASE_WRITES: u64 = 5; /// `execute_transfer`'s weight: the benchmarked base (compute + non-tree storage) /// plus the depth-dependent ZK-tree leaf insert performed by the wormhole proof /// recorder. `insert_leaf` walks the tree leaf-to-root, so DB ops and PoV scale -/// with `tree_ops` via [`pallet_zk_tree::TREE_KEY_POV`]. -fn execute_transfer_weight(db: RuntimeDbWeight, (tree_reads, tree_writes): (u64, u64)) -> Weight { +/// with `tree_ops` via [`pallet_zk_tree::TREE_KEY_POV`], and the path update also +/// computes one Poseidon hash per level (`tree_hash_time`). +fn execute_transfer_weight( + db: RuntimeDbWeight, + (tree_reads, tree_writes): (u64, u64), + tree_hash_time: u64, +) -> Weight { // Minimum execution time: 105_000_000 picoseconds. - Weight::from_parts(110_000_000, 8619) + Weight::from_parts(110_000_000_u64.saturating_add(tree_hash_time), 8619) .saturating_add(Weight::from_parts( 0, tree_reads.saturating_mul(pallet_zk_tree::TREE_KEY_POV), @@ -178,6 +183,7 @@ impl WeightInfo for SubstrateW execute_transfer_weight( T::DbWeight::get(), pallet_zk_tree::Pallet::::insert_leaf_db_ops(), + pallet_zk_tree::Pallet::::insert_leaf_hash_ref_time(), ) } /// Storage: `ReversibleTransfers::HighSecurityAccounts` (r:1 w:0) @@ -311,6 +317,7 @@ impl WeightInfo for () { execute_transfer_weight( RocksDbWeight::get(), pallet_zk_tree::insert_leaf_db_ops_at_depth(pallet_zk_tree::MAX_TREE_DEPTH), + pallet_zk_tree::insert_leaf_hash_ref_time_at_depth(pallet_zk_tree::MAX_TREE_DEPTH), ) } /// Storage: `ReversibleTransfers::HighSecurityAccounts` (r:1 w:0) @@ -383,4 +390,26 @@ mod tests { ); }); } + + /// The leaf insert also computes one Poseidon hash per tree level; that compute + /// must be charged in `ref_time` on top of the DB ops. The mock's `DbWeight` is + /// zero, so any depth-driven `ref_time` growth must come from the hashing term. + #[test] + fn execute_transfer_ref_time_includes_tree_hash_compute() { + crate::tests::mock::new_test_ext().execute_with(|| { + type W = SubstrateWeight; + pallet_zk_tree::Depth::::put(1); + let shallow = W::execute_transfer(); + pallet_zk_tree::Depth::::put(pallet_zk_tree::MAX_TREE_DEPTH); + let deep = W::execute_transfer(); + assert!( + deep.ref_time() > + shallow.ref_time(), + "execute_transfer ref_time must grow with tree depth (Poseidon hashing per level); \ + shallow: {:?}, deep: {:?}", + shallow, + deep, + ); + }); + } } diff --git a/pallets/utility/src/lib.rs b/pallets/utility/src/lib.rs index 6152de57..0c4747aa 100644 --- a/pallets/utility/src/lib.rs +++ b/pallets/utility/src/lib.rs @@ -267,6 +267,10 @@ pub mod pallet { T::WeightInfo::as_derivative() // AccountData for inner call origin accountdata. .saturating_add(T::DbWeight::get().reads_writes(1, 1)) + // High-security policy check on the pseudonym (`is_call_allowed` → + // one classification read in the runtime inspector); the benchmarked + // base runs with the no-op inspector and does not include it. + .saturating_add(T::DbWeight::get().reads(1)) .saturating_add(dispatch_info.call_weight), dispatch_info.class, ) @@ -286,9 +290,11 @@ pub mod pallet { origin.set_caller_from(frame_system::RawOrigin::Signed(pseudonym)); let info = call.get_dispatch_info(); let result = call.dispatch(origin); - // Always take into account the base weight of this call. + // Always take into account the base weight of this call, plus the + // high-security policy read on the pseudonym performed on every invocation. let mut weight = T::WeightInfo::as_derivative() - .saturating_add(T::DbWeight::get().reads_writes(1, 1)); + .saturating_add(T::DbWeight::get().reads_writes(1, 1)) + .saturating_add(T::DbWeight::get().reads(1)); // Add the real weight of the dispatch. weight = weight.saturating_add(extract_actual_weight(&result, &info)); result diff --git a/pallets/utility/src/tests.rs b/pallets/utility/src/tests.rs index de5b3b08..7d9e19d2 100644 --- a/pallets/utility/src/tests.rs +++ b/pallets/utility/src/tests.rs @@ -374,6 +374,35 @@ fn as_derivative_handles_weight_refund() { }); } +/// `as_derivative` consults the high-security policy on the pseudonym +/// (`T::HighSecurity::is_call_allowed`) before dispatching, which in the runtime +/// costs one `HighSecurityAccounts` storage read. The declared weight must +/// charge that read on top of the benchmarked base (which runs with a no-op +/// inspector), the AccountData ops, and the inner call. +#[test] +fn as_derivative_weight_charges_high_security_policy_read() { + new_test_ext().execute_with(|| { + let inner = RuntimeCall::System(frame_system::Call::remark { remark: vec![] }); + let inner_weight = inner.get_dispatch_info().call_weight; + let call = + RuntimeCall::Utility(UtilityCall::as_derivative { index: 0, call: Box::new(inner) }); + + let db = ::DbWeight::get(); + // Everything the declared weight covered before the policy read was + // accounted: benchmarked base + AccountData r/w + the inner call. + let without_policy_read = ::WeightInfo::as_derivative() + .saturating_add(db.reads_writes(1, 1)) + .saturating_add(inner_weight); + + assert!( + call.get_dispatch_info() + .call_weight + .all_gte(without_policy_read.saturating_add(db.reads(1))), + "declared as_derivative weight must include the high-security policy read" + ); + }); +} + #[test] fn as_derivative_filters() { new_test_ext().execute_with(|| { diff --git a/pallets/wormhole/src/lib.rs b/pallets/wormhole/src/lib.rs index c97292a5..7ee0e642 100644 --- a/pallets/wormhole/src/lib.rs +++ b/pallets/wormhole/src/lib.rs @@ -27,6 +27,22 @@ const PRIVATE_BATCH_PI_HEADER_FELTS: usize = 8; /// exit traffic. pub const UNSIGNED_EXIT_PRIORITY: u64 = 1; +/// Hard upper bound on the serialized size of a settlement proof (the `proof_bytes` +/// argument of `verify_private_batch` / `verify_public_batch`), enforced before the +/// blob is copied or parsed. +/// +/// Settlement extrinsics are unsigned and fee-free, and pre-validation runs for every +/// gossiped pool candidate, so without this gate the only bound on the bytes an +/// attacker can make every node copy (`to_vec`) and feed through the plonky2 parser +/// is the block-length limit — megabytes above any real proof. Proof sizes are fixed +/// by the compiled circuit dimensions: the current fixtures serialize to ~151 KB +/// (private batch) and ~224 KB (public batch), so 512 KiB leaves ample headroom for +/// circuit-knob growth (proof size scales only mildly with batch counts) while +/// keeping worst-case admission work near real-proof cost. If a circuit upgrade ever +/// pushes a real proof past this cap, `pre_validation_rejects_oversized_proof_bytes` +/// and every fixture-based settlement test will fail loudly at the same time. +pub const MAX_PROOF_BYTES: usize = 512 * 1024; + /// Expected public-input count of the private-batch circuit compiled into this runtime. fn private_batch_expected_public_inputs() -> usize { PRIVATE_BATCH_PI_HEADER_FELTS + circuit_config::NUM_LEAF_PROOFS * PUBLIC_INPUTS_FELTS_LEN @@ -207,7 +223,7 @@ pub mod pallet { pallet_prelude::*, traits::{ fungible::{Inspect as FungibleInspect, Mutate, Unbalanced}, - BuildGenesisConfig, Currency, + Currency, }, }; use frame_system::pallet_prelude::*; @@ -245,54 +261,6 @@ pub mod pallet { #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); - /// Genesis configuration for recording transfer proofs. - /// - /// This allows addresses to be endowed at genesis with funds that can be spent - /// using ZK proofs. The endowments are stored during genesis and processed in - /// `on_initialize` at block 1, which calls `record_transfer` for each address. - /// This records both the TransferProof in storage AND emits NativeTransferred events. - /// - /// We defer to block 1 because events emitted during genesis_build are not - /// persisted (Substrate limitation). By processing at block 1, indexers like - /// Subsquid can track these transfers. - /// - /// The chain does not distinguish between "wormhole addresses" and regular addresses - - /// any address can have transfer proofs recorded and spend via ZK proofs. - /// - /// Note: The actual balance must also be set via BalancesConfig separately. - #[pallet::genesis_config] - #[derive(frame_support::DefaultNoBound)] - pub struct GenesisConfig { - /// Addresses to record transfer proofs for at genesis: (address, amount). - /// A TransferProof will be recorded for each, enabling ZK spending. - /// Uses u128 for serde compatibility; converted to BalanceOf at build time. - pub endowed_addresses: Vec<(T::WormholeAccountId, u128)>, - } - - #[pallet::genesis_build] - impl BuildGenesisConfig for GenesisConfig { - fn build(&self) { - // Store endowments to be processed in on_initialize at block 1. - // We can't call record_transfer here because events emitted during - // genesis_build are not persisted (Substrate limitation). - // By deferring to block 1, both storage and events are handled correctly. - let pending: Vec<(T::WormholeAccountId, BalanceOf)> = self - .endowed_addresses - .iter() - .map(|(to, amount)| { - let balance: BalanceOf = (*amount).try_into().unwrap_or_else(|_| { - panic!("Genesis endowment amount {} exceeds Balance capacity", amount) - }); - (to.clone(), balance) - }) - .collect(); - - if !pending.is_empty() { - GenesisEndowmentsPending::::put(pending); - } - } - } - #[pallet::config] pub trait Config: frame_system::Config { /// Native balance type for transfer proofs. @@ -396,17 +364,6 @@ pub mod pallet { pub type TransferCount = StorageMap<_, Blake2_128Concat, T::WormholeAccountId, T::TransferCount, ValueQuery>; - /// Genesis endowments pending event emission. - /// Stores (to_address, amount) for each genesis endowment. - /// These are processed in on_initialize at block 1 to emit NativeTransferred events, - /// then cleared. This ensures indexers like Subsquid can track genesis transfers. - /// - /// Unbounded because it's only populated at genesis and cleared on block 1. - #[pallet::storage] - #[pallet::unbounded] - pub type GenesisEndowmentsPending = - StorageValue<_, Vec<(T::WormholeAccountId, BalanceOf)>, ValueQuery>; - #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { @@ -479,6 +436,13 @@ pub mod pallet { BlockNotFound, VerifierNotAvailable, ProofDeserializationFailed, + /// The submitted proof blob exceeds [`crate::MAX_PROOF_BYTES`]. Rejected before + /// any copy or parsing so oversized unsigned spam costs only a length check. + ProofTooLarge, + /// The proof bytes are not the canonical serialization of the decoded proof + /// (e.g. a valid proof with trailing bytes, which the plonky2 parser would + /// silently ignore). Every proof has exactly one accepted byte encoding. + NonCanonicalProofEncoding, ProofVerificationFailed, InvalidProofPublicInputs, /// The volume fee rate in the proof doesn't match the configured rate @@ -489,31 +453,46 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { - /// On block 1, process all genesis endowments by calling record_transfer. - /// This records transfer proofs and emits NativeTransferred events. - /// We defer this from genesis_build because events emitted during genesis - /// are not persisted (Substrate limitation). + /// On block 1, record a transfer proof for every account that exists with a + /// balance — i.e. exactly the genesis balances. + /// + /// The genesis state is the single source of truth: proofs are *derived* from the + /// balances actually issued (there is no separate endowment list that could + /// disagree with them), so an exitable leaf that isn't backed by real issuance is + /// unrepresentable. This runs before any extrinsic has ever executed, so the + /// account set observed here is precisely the genesis set. + /// + /// We do this at block 1 rather than in a genesis build because events emitted + /// during genesis are not persisted (Substrate limitation); recording here emits + /// `NativeTransferred` events that indexers like Subsquid can track. fn on_initialize(n: BlockNumberFor) -> Weight { // Only process on block 1 if n != One::one() { return Weight::zero(); } - let pending = GenesisEndowmentsPending::::take(); - if pending.is_empty() { - return Weight::zero(); - } - let minting_account: T::WormholeAccountId = T::MintingAccount::get().into(); - let num_endowments = pending.len() as u64; + let mut accounts_seen = 0u64; + let mut recorded = 0u64; - for (to, amount) in pending { + for who in frame_system::Account::::iter_keys() { + accounts_seen = accounts_seen.saturating_add(1); + let amount = >::total_balance(&who); + if amount.is_zero() { + continue; + } + let to: T::WormholeAccountId = who.into(); // Record transfer proof and emit event Self::record_transfer(T::AssetId::default(), &minting_account, &to, amount); + recorded = recorded.saturating_add(1); } - // Weight: 1 read (take pending) + N * (2 reads + 2 writes + 1 event) per endowment - T::DbWeight::get().reads_writes(1 + num_endowments * 2, num_endowments * 2) + // Weight: 1 read per iterated account + N * (2 reads + 2 writes + 1 event) + // per recorded proof + T::DbWeight::get().reads_writes( + accounts_seen.saturating_add(recorded.saturating_mul(2)), + recorded.saturating_mul(2), + ) } } @@ -839,6 +818,13 @@ pub mod pallet { for (exit_account, exit_balance) in &processed_accounts { // Skip failed credits (e.g. below ED); nullifier already marked, value // excluded from fee settlement / event. + // + // NOTE: this must stay `Unbalanced::increase_balance` (event-free). The runtime's + // `WormholeProofRecorderExtension` records a transfer proof for every + // `Balances::Minted` event it scans, and this exit already records its own proof + // via `record_transfer` below — switching to `mint_into` (which emits `Minted`) + // could double-record the credit. Pinned by the test + // `exit_credits_emit_no_scannable_transfer_events`. match >::increase_balance( exit_account, *exit_balance, @@ -1123,6 +1109,11 @@ pub mod pallet { ), Error, > { + // Length gate FIRST: `proof_bytes` is attacker-controlled, unsigned and + // fee-free, and everything below copies (`to_vec`) and parses the whole + // blob. Without this bound the only limit is the block-length cap, + // megabytes above any real proof. + ensure!(proof_bytes.len() <= crate::MAX_PROOF_BYTES, Error::::ProofTooLarge); let verifier = crate::get_private_batch_verifier() .map_err(|_| Error::::VerifierNotAvailable)?; let proof = ProofWithPublicInputs::::from_bytes( @@ -1130,6 +1121,16 @@ pub mod pallet { &verifier.circuit_data.common, ) .map_err(|_| Error::::ProofDeserializationFailed)?; + // Exact-framing check: `from_bytes` reads the proof off the front of the + // buffer and silently ignores trailing bytes, so without this a valid + // proof would have unboundedly many accepted byte representations — each + // a distinct tx hash whose copy+parse the pool re-pays at admission. + // Round-tripping pins one canonical encoding per proof (and also rejects + // non-canonical field encodings). + ensure!( + proof.to_bytes().as_slice() == proof_bytes, + Error::::NonCanonicalProofEncoding + ); let inputs = parse_private_batch_public_inputs(&proof) .map_err(|_| Error::::InvalidProofPublicInputs)?; let bundle: ExitBundle = inputs.into(); @@ -1167,6 +1168,9 @@ pub mod pallet { ), Error, > { + // Same gates as `pre_validate_private_batch_proof`: length bound before + // any copy/parse, then exact canonical framing after deserialization. + ensure!(proof_bytes.len() <= crate::MAX_PROOF_BYTES, Error::::ProofTooLarge); let verifier = crate::get_public_batch_verifier().map_err(|_| Error::::VerifierNotAvailable)?; let proof = ProofWithPublicInputs::::from_bytes( @@ -1174,6 +1178,10 @@ pub mod pallet { &verifier.circuit_data.common, ) .map_err(|_| Error::::ProofDeserializationFailed)?; + ensure!( + proof.to_bytes().as_slice() == proof_bytes, + Error::::NonCanonicalProofEncoding + ); let inputs = parse_public_batch_public_inputs( &proof, crate::circuit_config::NUM_PRIVATE_BATCH_PROOFS, @@ -1312,6 +1320,16 @@ pub mod pallet { to: ::WormholeAccountId, amount: BalanceOf, ) -> bool { + // A zero-amount credit moves no value, so a leaf for it is pure state growth: + // it would advance the recipient's transfer count, enlarge the ZK tree, and + // emit a transfer event for nothing. Zero-value `Balances::Transfer` events + // are reachable from permissionless surfaces (plain `transfer_keep_alive(0)`, + // zero-value scheduled transfers, ...), so drop the credit here — the single + // chokepoint every event-scan / call-site recorder goes through — and report + // it as not recorded so weight reconciliation does not count a leaf insert. + if amount.is_zero() { + return false; + } // The wormhole tags native leaves with `asset_id == 0`, but `pallet_assets` uses // id 0 for an unrelated, independently-mintable token. Genuine native reaches us as // `None` (from `Balances` events); a `pallet_assets` asset-0 credit reaches us as diff --git a/pallets/wormhole/src/mock.rs b/pallets/wormhole/src/mock.rs index 4b02c781..12ae71cd 100644 --- a/pallets/wormhole/src/mock.rs +++ b/pallets/wormhole/src/mock.rs @@ -133,13 +133,12 @@ pub fn new_test_ext() -> sp_state_machine::TestExternalities { t.into() } -/// Build test externalities with genesis endowments. -/// Each endowment is (address, amount) and will have both balance and TransferProof recorded -/// (after block 1 initialization), enabling the address to spend via ZK proofs. +/// Build test externalities with genesis balance endowments. /// -/// Note: This sets up the genesis state, but TransferProofs are recorded in on_initialize -/// at block 1. Tests should call `System::set_block_number(1)` and then trigger -/// `Wormhole::on_initialize(1)` to process the endowments. +/// TransferProofs are *derived* from these balances in `on_initialize` at block 1 (the +/// wormhole pallet records a proof for every account existing with a balance), enabling +/// each address to spend via ZK proofs. Tests should call `System::set_block_number(1)` +/// and then trigger `Wormhole::on_initialize(1)` to process them. pub fn new_test_ext_with_endowments( endowments: Vec<(AccountId, Balance)>, ) -> sp_state_machine::TestExternalities { @@ -147,13 +146,8 @@ pub fn new_test_ext_with_endowments( let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - // Set up balances for the endowed accounts - pallet_balances::GenesisConfig:: { balances: endowments.to_vec(), dev_accounts: None } - .assimilate_storage(&mut t) - .unwrap(); - - // Set up endowments to be processed at block 1 - pallet_wormhole::GenesisConfig:: { endowed_addresses: endowments } + // Set up balances for the endowed accounts; wormhole proofs derive from these. + pallet_balances::GenesisConfig:: { balances: endowments, dev_accounts: None } .assimilate_storage(&mut t) .unwrap(); diff --git a/pallets/wormhole/src/tests.rs b/pallets/wormhole/src/tests.rs index 9277910d..d17362d1 100644 --- a/pallets/wormhole/src/tests.rs +++ b/pallets/wormhole/src/tests.rs @@ -83,6 +83,47 @@ mod wormhole_tests { }); } + /// A zero-amount credit moves no value, but recording it would still append a + /// ZK-tree leaf, advance the recipient's transfer count, and emit an event. + /// Zero-value `Balances::Transfer` events are reachable from permissionless + /// surfaces (`transfer_keep_alive(dest, 0)`, zero-value scheduled transfers), + /// so the recorder must drop zero-amount credits and report them as not + /// recorded (so weight reconciliation doesn't count a leaf insert). + #[test] + fn zero_amount_credit_is_not_recorded() { + use qp_wormhole::TransferProofRecorder; + + new_test_ext().execute_with(|| { + System::set_block_number(1); + let from = account_id(1); + let to = account_id(9001); + + assert!( + !>::record_transfer_proof( + None, + from.clone(), + to.clone(), + 0, + ), + "a zero-amount credit must report as not recorded" + ); + assert_eq!(ZkTree::leaf_count(), 0, "no ZK-tree leaf for a zero-amount credit"); + assert_eq!( + Wormhole::transfer_count(&to), + 0, + "the recipient's transfer count must not advance" + ); + + // Sanity: the same credit with a nonzero amount is recorded. + assert!( + >::record_transfer_proof( + None, from, to, 1, + ) + ); + assert_eq!(ZkTree::leaf_count(), 1); + }); + } + #[test] fn record_transfer_increments_count() { new_test_ext().execute_with(|| { @@ -454,6 +495,35 @@ mod wormhole_tests { }); } + // ========================================================================= + // Genesis proofs are derived from real balances (single source of truth) + // ========================================================================= + // + // There is no separate wormhole endowment list at genesis: `on_initialize(1)` derives + // a transfer proof from every account that exists with a balance. An exitable leaf + // that isn't backed by actually-issued value is therefore unrepresentable — the leaf + // amount IS the genesis balance. + + #[test] + fn genesis_proofs_derive_from_balances() { + use frame_support::traits::Hooks; + + let addr1 = account_id(100); + let addr2 = account_id(101); + let amount1 = 100 * UNIT; + let amount2 = 250 * UNIT; + + new_test_ext_with_endowments(vec![(addr1.clone(), amount1), (addr2.clone(), amount2)]) + .execute_with(|| { + System::set_block_number(1); + Wormhole::on_initialize(1); + + // One leaf per funded genesis account, amount = the real balance. + assert_eq!(Wormhole::transfer_count(&addr1), 1); + assert_eq!(Wormhole::transfer_count(&addr2), 1); + }); + } + // ========================================================================= // Soundness counter removal migration // ========================================================================= @@ -700,6 +770,58 @@ mod private_batch_proof_tests { }); } + /// The runtime's `WormholeProofRecorderExtension` records transfer proofs by scanning + /// `Balances::Transfer`/`Minted` events after a transaction. The exit path must therefore + /// never emit either event: `process_exit_bundle` credits exits via + /// `Unbalanced::increase_balance` (event-free) and records its own proof internally via + /// `record_transfer`. If a refactor ever switched the exit credit to `mint_into` (which + /// emits `Minted`), each exit could be recorded twice — inflating `TransferCount` and + /// creating a duplicate, unspendable leaf. + #[test] + fn exit_credits_emit_no_scannable_transfer_events() { + new_test_ext().execute_with(|| { + let proof = deserialize_test_proof(); + let inputs = parse_private_batch_public_inputs(&proof).expect("Should parse"); + + // Set up block state so the proof's cheap bundle checks pass. + let block_number = inputs.block_data.block_number as u64; + let block_hash_bytes: [u8; 32] = + inputs.block_data.block_hash.as_ref().try_into().unwrap(); + frame_system::BlockHash::::insert(block_number, H256::from(block_hash_bytes)); + System::set_block_number(block_number + 10); + + // Guard that the assertion below is meaningful: the proof must credit exits. + let expected_exit: u128 = inputs + .account_data + .iter() + .filter(|a| a.summed_output_amount > 0) + .map(|a| (a.summed_output_amount as u128) * crate::SCALE_DOWN_FACTOR) + .sum(); + assert!(expected_exit > 0, "test proof must credit at least one exit"); + + System::reset_events(); + assert_ok!(Wormhole::verify_private_batch( + RawOrigin::None.into(), + get_test_proof_bytes() + )); + + // No `Transfer`/`Minted` events: nothing for an event-based recorder to pick up. + for record in System::events() { + assert!( + !matches!( + record.event, + RuntimeEvent::Balances( + pallet_balances::Event::::Transfer { .. } | + pallet_balances::Event::::Minted { .. } + ) + ), + "exit processing must not emit scannable Transfer/Minted events: {:?}", + record.event + ); + } + }); + } + /// Sets up the on-chain block state so the test proof's cheap bundle checks pass. fn setup_valid_block_state_for_test_proof() { let proof = deserialize_test_proof(); @@ -710,6 +832,43 @@ mod private_batch_proof_tests { System::set_block_number(block_number + 10); } + /// `ProofWithPublicInputs::from_bytes` reads the proof off the front of the buffer + /// and silently ignores trailing bytes, so without an exact-framing check one valid + /// proof has unboundedly many byte representations — each a distinct transaction + /// hash whose full copy + parse every node re-pays at pool admission, fee-free. + /// Pre-validation must accept exactly one canonical encoding per proof. + #[test] + fn pre_validation_rejects_padded_proof_bytes() { + new_test_ext().execute_with(|| { + setup_valid_block_state_for_test_proof(); + + // The canonical encoding passes pre-validation. + assert!(Wormhole::pre_validate_private_batch_proof(&get_test_proof_bytes()).is_ok()); + + // The same proof with trailing junk must be rejected. + let mut padded = get_test_proof_bytes(); + padded.extend_from_slice(&[0u8; 32]); + assert!(matches!( + Wormhole::pre_validate_private_batch_proof(&padded), + Err(Error::::NonCanonicalProofEncoding) + )); + }); + } + + /// Oversized blobs must be cut off by a length gate BEFORE the byte copy and the + /// parser run — `ProofDeserializationFailed` after the fact means the work was + /// already done. + #[test] + fn pre_validation_rejects_oversized_proof_bytes() { + new_test_ext().execute_with(|| { + let oversized = vec![0u8; crate::MAX_PROOF_BYTES + 1]; + assert!(matches!( + Wormhole::pre_validate_private_batch_proof(&oversized), + Err(Error::::ProofTooLarge) + )); + }); + } + /// The block-inclusion gate (`pre_dispatch`) must reject a proof that cannot be /// verified. Before this was fixed, `pre_dispatch` was a no-op that returned `Ok(())` /// for any `verify_*` call, so junk rode into blocks as failed `Pays::No` extrinsics; @@ -1693,6 +1852,25 @@ mod public_batch_proof_tests { System::set_block_number(block_number + 10); } + /// Public-batch twin of the private-batch exact-framing test: trailing bytes after + /// a valid proof are silently ignored by the plonky2 parser, so they must be + /// rejected by the canonical-encoding check. + #[test] + fn pre_validation_rejects_padded_proof_bytes() { + new_test_ext().execute_with(|| { + setup_matching_block_state(&parse_test_inputs()); + + assert!(Wormhole::pre_validate_public_batch_proof(&get_test_proof_bytes()).is_ok()); + + let mut padded = get_test_proof_bytes(); + padded.extend_from_slice(&[0u8; 32]); + assert!(matches!( + Wormhole::pre_validate_public_batch_proof(&padded), + Err(Error::::NonCanonicalProofEncoding) + )); + }); + } + #[test] fn test_parse_public_batch_public_inputs_succeeds() { let inputs = parse_test_inputs(); @@ -1845,6 +2023,68 @@ mod public_batch_proof_tests { }); } + /// The aggregator rebate is deliberately permissionless: whoever performs the public-batch + /// aggregation names its own payout address as a proof public input. The property that + /// makes this safe is that the address is *bound* by the proof — a third party cannot take + /// someone else's public batch and redirect the rebate to itself, because mutating the + /// aggregator-address public inputs invalidates the proof, and `pre_dispatch` (the + /// block-inclusion gate) runs full ZK verification. + #[test] + fn pre_dispatch_rejects_public_batch_with_redirected_aggregator_address() { + use frame_support::pallet_prelude::ValidateUnsigned; + use qp_plonky2_verifier::field::types::Field; + + new_test_ext().execute_with(|| { + let inputs = parse_test_inputs(); + setup_matching_block_state(&inputs); + + // The genuine proof passes the block-inclusion gate. + let original = get_test_proof_bytes(); + let call = crate::Call::::verify_public_batch { proof_bytes: original.clone() }; + assert!( + ::pre_dispatch(&call).is_ok(), + "the untampered fixture must pass pre_dispatch" + ); + + // An attacker rewrites the aggregator-address public inputs (the first 4 felts + // of the public-batch PI layout) to point at an account they control. + let mut tampered_proof = deserialize_test_proof(); + for felt in tampered_proof.public_inputs.iter_mut().take(4) { + *felt = F::from_canonical_u32(0x42); + } + let tampered_bytes = tampered_proof.to_bytes(); + assert_ne!(tampered_bytes, original, "mutation must change the encoded proof"); + + // The redirected address round-trips through parsing (i.e. the tampering is + // well-formed at the PI level) ... + let tampered_deser = ProofWithPublicInputs::::from_bytes( + tampered_bytes.clone(), + &crate::get_public_batch_verifier().unwrap().circuit_data.common, + ) + .expect("tampered PIs still deserialize"); + let tampered_inputs = parse_public_batch_public_inputs( + &tampered_deser, + crate::circuit_config::NUM_PRIVATE_BATCH_PROOFS, + crate::circuit_config::NUM_LEAF_PROOFS, + ) + .expect("tampered PIs still parse"); + assert_ne!( + tampered_inputs.aggregator_address.as_ref(), + &AGGREGATOR_ADDRESS, + "the payout address was redirected" + ); + + // ... but the proof no longer verifies, so the block-inclusion gate rejects it: + // the rebate cannot be stolen off an existing proof. + let tampered_call = + crate::Call::::verify_public_batch { proof_bytes: tampered_bytes }; + assert!( + ::pre_dispatch(&tampered_call).is_err(), + "pre_dispatch must reject a proof whose aggregator address was redirected" + ); + }); + } + /// Regenerate the public-batch test fixture when circuit parameters change. /// /// Run with: cargo test -p pallet-wormhole --release --lib -- diff --git a/pallets/wormhole/src/weights.rs b/pallets/wormhole/src/weights.rs index 609051d7..7f815b66 100644 --- a/pallets/wormhole/src/weights.rs +++ b/pallets/wormhole/src/weights.rs @@ -163,27 +163,38 @@ impl WeightInfo for SubstrateW .saturating_add(T::DbWeight::get().reads(1_u64.saturating_add(nullifier_reads))) } /// Inclusion path: ZK verify + pre-validation twice (`pre_dispatch` and dispatch - /// body), each charged in full (compute + DB + PoV), plus exit-processing storage. + /// body), each charged in full (compute + DB + PoV), plus exit-processing storage + /// and the per-exit ZK-tree Poseidon hashing (one hash per tree level per insert). fn verify_private_batch() -> Weight { let tree_ops = pallet_zk_tree::Pallet::::insert_leaf_db_ops(); let (reads, writes, proof_size) = storage_tail(private_batch_max_exits(), false, tree_ops); - Weight::from_parts(PRIVATE_BATCH_ZK_VERIFY_REF_TIME_PS, proof_size) - .saturating_add(T::DbWeight::get().reads(reads)) - .saturating_add(T::DbWeight::get().writes(writes)) - .saturating_add(Self::pre_validate_proof()) - .saturating_add(Self::pre_validate_proof()) + let hash_time = private_batch_max_exits() + .saturating_mul(pallet_zk_tree::Pallet::::insert_leaf_hash_ref_time()); + Weight::from_parts( + PRIVATE_BATCH_ZK_VERIFY_REF_TIME_PS.saturating_add(hash_time), + proof_size, + ) + .saturating_add(T::DbWeight::get().reads(reads)) + .saturating_add(T::DbWeight::get().writes(writes)) + .saturating_add(Self::pre_validate_proof()) + .saturating_add(Self::pre_validate_proof()) } /// Same double-prevalidation shape as [`Self::verify_private_batch`], scaled /// across all inner segments plus the aggregator rebate. fn verify_public_batch() -> Weight { let tree_ops = pallet_zk_tree::Pallet::::insert_leaf_db_ops(); let (reads, writes, proof_size) = storage_tail(public_batch_max_exits(), true, tree_ops); - Weight::from_parts(PUBLIC_BATCH_ZK_VERIFY_REF_TIME_PS, proof_size) - .saturating_add(T::DbWeight::get().reads(reads)) - .saturating_add(T::DbWeight::get().writes(writes)) - .saturating_add(Self::pre_validate_public_batch_proof()) - .saturating_add(Self::pre_validate_public_batch_proof()) + let hash_time = public_batch_max_exits() + .saturating_mul(pallet_zk_tree::Pallet::::insert_leaf_hash_ref_time()); + Weight::from_parts( + PUBLIC_BATCH_ZK_VERIFY_REF_TIME_PS.saturating_add(hash_time), + proof_size, + ) + .saturating_add(T::DbWeight::get().reads(reads)) + .saturating_add(T::DbWeight::get().writes(writes)) + .saturating_add(Self::pre_validate_public_batch_proof()) + .saturating_add(Self::pre_validate_public_batch_proof()) } } @@ -204,27 +215,39 @@ impl WeightInfo for () { Weight::from_parts(PUBLIC_BATCH_PRE_VALIDATE_REF_TIME_PS, proof_size) .saturating_add(RocksDbWeight::get().reads(1_u64.saturating_add(nullifier_reads))) } - /// See `SubstrateWeight::verify_private_batch`. Tree component priced at - /// `MAX_TREE_DEPTH` (no runtime type to read live depth). + /// See `SubstrateWeight::verify_private_batch`. Tree component (DB ops and + /// Poseidon hashing) priced at `MAX_TREE_DEPTH` (no runtime type to read live depth). fn verify_private_batch() -> Weight { let tree_ops = pallet_zk_tree::insert_leaf_db_ops_at_depth(pallet_zk_tree::MAX_TREE_DEPTH); let (reads, writes, proof_size) = storage_tail(private_batch_max_exits(), false, tree_ops); - Weight::from_parts(PRIVATE_BATCH_ZK_VERIFY_REF_TIME_PS, proof_size) - .saturating_add(RocksDbWeight::get().reads(reads)) - .saturating_add(RocksDbWeight::get().writes(writes)) - .saturating_add(Self::pre_validate_proof()) - .saturating_add(Self::pre_validate_proof()) + let hash_time = private_batch_max_exits().saturating_mul( + pallet_zk_tree::insert_leaf_hash_ref_time_at_depth(pallet_zk_tree::MAX_TREE_DEPTH), + ); + Weight::from_parts( + PRIVATE_BATCH_ZK_VERIFY_REF_TIME_PS.saturating_add(hash_time), + proof_size, + ) + .saturating_add(RocksDbWeight::get().reads(reads)) + .saturating_add(RocksDbWeight::get().writes(writes)) + .saturating_add(Self::pre_validate_proof()) + .saturating_add(Self::pre_validate_proof()) } /// See `SubstrateWeight::verify_public_batch`. fn verify_public_batch() -> Weight { let tree_ops = pallet_zk_tree::insert_leaf_db_ops_at_depth(pallet_zk_tree::MAX_TREE_DEPTH); let (reads, writes, proof_size) = storage_tail(public_batch_max_exits(), true, tree_ops); - Weight::from_parts(PUBLIC_BATCH_ZK_VERIFY_REF_TIME_PS, proof_size) - .saturating_add(RocksDbWeight::get().reads(reads)) - .saturating_add(RocksDbWeight::get().writes(writes)) - .saturating_add(Self::pre_validate_public_batch_proof()) - .saturating_add(Self::pre_validate_public_batch_proof()) + let hash_time = public_batch_max_exits().saturating_mul( + pallet_zk_tree::insert_leaf_hash_ref_time_at_depth(pallet_zk_tree::MAX_TREE_DEPTH), + ); + Weight::from_parts( + PUBLIC_BATCH_ZK_VERIFY_REF_TIME_PS.saturating_add(hash_time), + proof_size, + ) + .saturating_add(RocksDbWeight::get().reads(reads)) + .saturating_add(RocksDbWeight::get().writes(writes)) + .saturating_add(Self::pre_validate_public_batch_proof()) + .saturating_add(Self::pre_validate_public_batch_proof()) } } @@ -368,6 +391,87 @@ mod tests { ); } + /// Every processed exit inserts a ZK-tree leaf, whose path update computes one + /// Poseidon hash per tree level. That compute must be charged in `ref_time` on + /// top of the DB ops — the mock's `DbWeight` is zero, so any depth-driven + /// `ref_time` growth must come from the hashing term. + #[test] + fn verify_weights_charge_per_exit_hash_compute() { + crate::mock::new_test_ext().execute_with(|| { + type W = SubstrateWeight; + + pallet_zk_tree::Depth::::put(1); + let shallow = W::verify_private_batch(); + pallet_zk_tree::Depth::::put(20); + let deep = W::verify_private_batch(); + assert!( + deep.ref_time() > shallow.ref_time(), + "per-exit Poseidon hashing must make verify ref_time grow with tree depth" + ); + + // Exact floor: ZK verify + both pre-validations + one leaf insert's + // hashing per exit, all at the live depth. + let hash_private = private_batch_max_exits() + .saturating_mul(pallet_zk_tree::insert_leaf_hash_ref_time_at_depth(20)); + assert!( + deep.ref_time() >= + PRIVATE_BATCH_ZK_VERIFY_REF_TIME_PS + + 2 * W::pre_validate_proof().ref_time() + hash_private, + "private verify must charge per-exit hash compute" + ); + + let deep_public = W::verify_public_batch(); + let hash_public = public_batch_max_exits() + .saturating_mul(pallet_zk_tree::insert_leaf_hash_ref_time_at_depth(20)); + assert!( + deep_public.ref_time() >= + PUBLIC_BATCH_ZK_VERIFY_REF_TIME_PS + + 2 * W::pre_validate_public_batch_proof().ref_time() + + hash_public, + "public verify must charge per-exit hash compute" + ); + }); + } + + /// The depth-blind `()` impl must charge the same per-exit hash compute, + /// priced at `MAX_TREE_DEPTH`. + #[test] + fn unit_impl_verify_weights_charge_per_exit_hash_compute() { + let hash_per_insert = pallet_zk_tree::insert_leaf_hash_ref_time_at_depth( + pallet_zk_tree::MAX_TREE_DEPTH, + ); + let tree_ops = + pallet_zk_tree::insert_leaf_db_ops_at_depth(pallet_zk_tree::MAX_TREE_DEPTH); + + let (reads, writes, _) = storage_tail(private_batch_max_exits(), false, tree_ops); + let private_db_time = RocksDbWeight::get() + .reads(reads) + .saturating_add(RocksDbWeight::get().writes(writes)) + .ref_time(); + assert!( + <() as WeightInfo>::verify_private_batch().ref_time() >= + PRIVATE_BATCH_ZK_VERIFY_REF_TIME_PS + + private_db_time + + 2 * <() as WeightInfo>::pre_validate_proof().ref_time() + + private_batch_max_exits().saturating_mul(hash_per_insert), + "() private verify must charge per-exit hash compute on top of DB ops" + ); + + let (reads, writes, _) = storage_tail(public_batch_max_exits(), true, tree_ops); + let public_db_time = RocksDbWeight::get() + .reads(reads) + .saturating_add(RocksDbWeight::get().writes(writes)) + .ref_time(); + assert!( + <() as WeightInfo>::verify_public_batch().ref_time() >= + PUBLIC_BATCH_ZK_VERIFY_REF_TIME_PS + + public_db_time + + 2 * <() as WeightInfo>::pre_validate_public_batch_proof().ref_time() + + public_batch_max_exits().saturating_mul(hash_per_insert), + "() public verify must charge per-exit hash compute on top of DB ops" + ); + } + /// Floor so regenerated weights can't under-price the all-valid worst case. #[test] fn pre_validation_compute_covers_production_path() { diff --git a/pallets/zk-tree/src/lib.rs b/pallets/zk-tree/src/lib.rs index c709f306..efcee98f 100644 --- a/pallets/zk-tree/src/lib.rs +++ b/pallets/zk-tree/src/lib.rs @@ -37,8 +37,29 @@ pub mod tree; #[cfg(test)] mod tests; -/// Maximum depth supported by ZK circuits. -/// A tree of depth 32 can hold 4^32 leaves (more than enough). +/// Maximum depth the on-chain tree may grow to (weight-metering / growth cap). +/// A tree of depth 32 can hold 4^32 leaves. +/// +/// NOTE (known, accepted limitation): this is intentionally *larger* than the depth the +/// wormhole circuits accept. The circuits fix `MAX_DEPTH = 16` (`qp-zk-circuits-common`, +/// `zk_merkle.rs`) because every leaf proof pays the proving cost of a full +/// `MAX_DEPTH`-level Merkle path regardless of the tree's current depth — keeping it at +/// 16 keeps proving fast for everyone. If the tree ever grows past depth 16 +/// (4^16 ≈ 4.3 billion leaves), Merkle proofs gain a 17th sibling level and the prover +/// and verifier reject them, so wormhole proof generation halts until a circuit update +/// raises `MAX_DEPTH` and a runtime upgrade embeds the regenerated verifiers. +/// +/// This is a deliberate "fix it when we get close" trade-off, not an oversight: +/// - Timeline: at one leaf per block (the mining-reward floor, 12s blocks) depth 16 lasts ~1,600 +/// years; at a sustained 10 transfers/sec chain-wide it lasts ~13 years; even permanently +/// saturated blocks (~50 tps) give ~2.5 years. Each +1 of circuit depth quadruples capacity. +/// - Observability: `LeafCount` is public storage, so exhaustion is visible years in advance; alert +/// well before 4^16 leaves. +/// - The update itself: bump `MAX_DEPTH` in `qp-zk-circuits-common`, release the circuit crates, +/// let `pallets/wormhole/build.rs` regenerate the embedded verifier binaries, regenerate proof +/// fixtures, re-benchmark, and ship a runtime upgrade — days of engineering inside a normal +/// release cycle. Old proofs are invalidated by the circuit change; nullifier state is +/// unaffected, so nothing can double-spend across the upgrade. pub const MAX_TREE_DEPTH: u8 = 32; /// Worst-case `(reads, writes)` storage-operation counts for one [`Pallet::insert_leaf`] @@ -223,6 +244,15 @@ pub mod pallet { pub fn insert_leaf_db_ops() -> (u64, u64) { crate::insert_leaf_db_ops_at_depth(Depth::::get()) } + + /// Worst-case Poseidon-hashing `ref_time` for one `insert_leaf` at the tree's + /// *current* depth. See [`insert_leaf_hash_ref_time_at_depth`]. Anything that + /// prices a leaf insert must charge this *in addition to* + /// [`Self::insert_leaf_db_ops`]: the DB ops cover storage I/O only, while the + /// path update also computes one Poseidon hash per tree level. + pub fn insert_leaf_hash_ref_time() -> u64 { + crate::insert_leaf_hash_ref_time_at_depth(Depth::::get()) + } } impl Pallet diff --git a/primitives/header/src/lib.rs b/primitives/header/src/lib.rs index 0525b62b..16d3c0d3 100644 --- a/primitives/header/src/lib.rs +++ b/primitives/header/src/lib.rs @@ -43,6 +43,13 @@ use serde::{Deserialize, Serialize}; /// import rather than silently truncated; see the digest length check in /// `sc-consensus-qpow`. Truncation would let two distinct headers share a block /// hash on the bytes past this window. +/// +/// Because the window has no slack, the runtime must never deposit digest items +/// of its own: even a 1-byte item (e.g. upstream frame-system's +/// `RuntimeEnvironmentUpdated` on `set_code`) pushes the sealed digest to 111 +/// bytes and makes the block unimportable network-wide. The vendored +/// frame-system's deposits were removed for exactly this reason — see the +/// warning on `frame_system::Pallet::deposit_log`. pub const DIGEST_LOGS_SIZE: usize = 110; /// Extension trait for headers that support ZK tree root. diff --git a/primitives/high-security/src/lib.rs b/primitives/high-security/src/lib.rs index d4103aec..c6d32209 100644 --- a/primitives/high-security/src/lib.rs +++ b/primitives/high-security/src/lib.rs @@ -122,13 +122,28 @@ pub trait HighSecurityInspector { /// `Some(guardian_account)` if the account has a guardian, `None` otherwise fn guardian(who: &AccountId) -> Option; + /// Evaluate the call policy for an account whose high-security classification has + /// already been determined. + /// + /// This is the single policy predicate behind [`Self::is_call_allowed`], split out + /// so a caller that already paid the `is_high_security` lookup for another purpose + /// (e.g. weight selection in `pallet_multisig::propose`) can apply the policy + /// without repeating the classification storage read. + fn is_call_allowed_given(is_high_security: bool, call: &RuntimeCall) -> bool { + !is_high_security || Self::is_whitelisted(call) + } + /// Whether `call` may be dispatched with `who` as the effective signed origin. /// /// Non-High-Security accounts may dispatch anything; High-Security accounts are /// restricted to whitelisted calls. Origin-rewriting wrappers (multisig execution, /// `as_recovered`, `as_derivative`) must consult this before dispatching as `who`. + /// + /// NOTE: this performs one `is_high_security` classification lookup — a storage + /// read in the runtime implementation — so every dispatchable that calls it must + /// charge that read in its declared weight. fn is_call_allowed(who: &AccountId, call: &RuntimeCall) -> bool { - !Self::is_high_security(who) || Self::is_whitelisted(call) + Self::is_call_allowed_given(Self::is_high_security(who), call) } // NOTE: No benchmarking-specific methods in the trait! diff --git a/runtime/src/genesis_config_presets.rs b/runtime/src/genesis_config_presets.rs index 2bc94376..5fbf7246 100644 --- a/runtime/src/genesis_config_presets.rs +++ b/runtime/src/genesis_config_presets.rs @@ -148,10 +148,11 @@ fn planck_tech_collective_seed() -> Vec { /// Returns the genesis config populated with given parameters. Treasury is per-profile. /// -/// All endowed addresses automatically get transfer proofs recorded, enabling them to -/// spend their funds via ZK proofs. The chain doesn't distinguish between "wormhole -/// addresses" and regular addresses - any address can spend via ZK proofs if they -/// know the corresponding secret. +/// All endowed addresses automatically get transfer proofs recorded at block 1 (the +/// wormhole pallet derives them from the genesis balances — there is no separate +/// endowment list), enabling them to spend their funds via ZK proofs. The chain doesn't +/// distinguish between "wormhole addresses" and regular addresses - any address can +/// spend via ZK proofs if they know the corresponding secret. fn genesis_template( endowed_accounts: Vec, treasury: TreasuryGenesis, @@ -170,16 +171,11 @@ fn genesis_template( // mining rewards. It is intentionally NOT added to `balances`. let config = RuntimeGenesisConfig { - balances: BalancesConfig { balances: balances.clone(), dev_accounts: None }, + balances: BalancesConfig { balances, dev_accounts: None }, treasury_pallet: pallet_treasury::GenesisConfig:: { treasury_account: Some(treasury.account), treasury_portion: Some(treasury.portion), }, - wormhole: pallet_wormhole::GenesisConfig:: { - // Record transfer proofs for ALL endowed addresses, enabling ZK spending. - // Events are emitted in on_initialize at block 1 for indexer compatibility. - endowed_addresses: balances, - }, ..Default::default() }; @@ -311,6 +307,26 @@ fn planck_treasury_account() -> AccountId { /// Parses genesis JSON, removes [`TECH_COLLECTIVE_SEED_MEMBERS_KEY`] if present, and returns /// serialized config for [`frame_support::genesis_builder_helper::build_state`] plus the optional /// member list. +/// +/// # Trust model (deliberately no size limits) +/// +/// This runs inside the `GenesisBuilder` runtime API, which is only invoked by the node +/// operator's own tooling (chain-spec building / genesis initialization) with the chain +/// spec that operator chose to launch. It is not reachable by network peers or on a +/// running chain. Whoever supplies this JSON already controls *everything* about the +/// chain being built — balances, keys, code — so input-size bounds here would not +/// protect anyone: an oversized or hostile genesis can only stall the chain of the +/// operator who supplied it. This matches upstream Substrate, whose `build_state` +/// helper deserializes the full unbounded config the same way. +/// +/// The same reasoning covers failure semantics: semantically invalid genesis data +/// (duplicate balance entries, sub-ED endowments, ...) *panics* inside the pallets' +/// `BuildGenesisConfig::build` rather than returning `Err`. That is FRAME's design — +/// `build` returns `()` and has no error channel; only JSON deserialization (which runs +/// before the trait) can return `Err`. The panics are inherited verbatim from upstream +/// Substrate and are the intended fail-fast: they abort the operator's own chain-spec +/// build with the assertion message, and the failed build's candidate storage is +/// discarded, so nothing half-built can persist. pub fn prepare_genesis_build_input( config: Vec, ) -> Result<(Vec, Option>), String> { @@ -425,3 +441,27 @@ pub fn preset_names() -> Vec { PresetId::from(PLANCK_RUNTIME_PRESET), ] } + +#[cfg(test)] +mod tests { + use super::*; + use sp_runtime::BuildStorage; + + /// Every shipped preset must actually build genesis storage, i.e. pass every pallet's + /// genesis-build invariants. (Wormhole transfer proofs need no preset entry at all: + /// they are derived from these genesis balances at block 1, so they cannot disagree + /// with the value actually issued.) + #[test] + fn all_presets_build_genesis_storage() { + for id in preset_names() { + let bytes = get_preset(&id).expect("listed preset must resolve"); + let (config_bytes, _members) = prepare_genesis_build_input(bytes) + .unwrap_or_else(|e| panic!("preset {:?}: invalid genesis JSON: {e}", id)); + let config: crate::RuntimeGenesisConfig = serde_json::from_slice(&config_bytes) + .unwrap_or_else(|e| panic!("preset {:?} must deserialize: {e}", id)); + config + .build_storage() + .unwrap_or_else(|e| panic!("preset {:?} must build genesis storage: {e:?}", id)); + } + } +} diff --git a/runtime/src/transaction_extensions.rs b/runtime/src/transaction_extensions.rs index 3bdec54a..c247f333 100644 --- a/runtime/src/transaction_extensions.rs +++ b/runtime/src/transaction_extensions.rs @@ -84,17 +84,42 @@ impl /// Transaction extension that records transfer proofs in the wormhole pallet /// /// This extension uses an EVENT-BASED approach to detect transfers: -/// - After successful execution, scans for Transfer/Transferred/Issued events +/// - After successful execution, scans for `Transfer`, `Minted`, `TransferOnHold` and +/// `ReserveRepatriated` events /// - Records proofs for any transfers that were sent TO a wormhole account -/// - Automatically catches ALL transfers regardless of how they're initiated: +/// - Automatically catches ALL transfers dispatched inside a transaction, regardless of how they're +/// initiated: /// - Direct transfers (transfer, transfer_keep_alive, transfer_all, etc.) /// - Batch transfers (utility.batch, batch_all, force_batch) /// - Multisig transfers (multisig.execute) /// - Recovery transfers (recovery.as_recovered) -/// - Scheduled transfers (scheduler) -/// - Future mechanisms automatically covered +/// - Held-fund seizures/recoveries (reversible_transfers.cancel / recover_funds, which move value +/// with `transfer_on_hold` instead of a free-balance transfer) +/// - Recovery-deposit seizures (recovery.close_recovery, which moves the rescuer's deposit with +/// `repatriate_reserved`) +/// - Future call-based mechanisms automatically covered, since wrapper calls emit their inner +/// events within the same extrinsic's event range /// -/// This addresses audit item EQ-QNT-WORMHOLE-F-05 comprehensively. +/// COVERAGE BOUNDARY: transaction extensions only run for transactions, so this scan +/// never sees events emitted from hooks (`on_initialize` / `on_finalize`). Every +/// hook-context credit therefore needs — and has — an explicit +/// `TransferProofRecorder::record_transfer_proof` call instead: +/// - reversible-transfers' scheduled execution records its transfer in `do_execute_transfer`; +/// - mining rewards and the treasury share record theirs in `on_finalize` +/// (`pallet_mining_rewards`), using eventless `increase_balance` credits. +/// +/// The one remaining hook-context path is a governance-enacted call: referenda enactment +/// dispatches the approved call via the scheduler in `on_initialize` (e.g. a Root +/// `force_transfer`), so its events are not scanned and no leaf is recorded. This is a +/// known, accepted gap rather than an oversight: the scheduler's `ScheduleOrigin` is +/// Root, the tech-referenda track only accepts Root proposal origins, and sudo is +/// removed — so only Root can reach it, and Root can already forge or delete leaves +/// outright (`set_storage`, runtime upgrades), so there is no invariant left to defend +/// against it. The miss is conservative (the credit exists but gains no ZK-spendable +/// leaf; no unbacked exit capacity is created) and repairable (governance can re-issue +/// the credit as an ordinary signed transfer if a leaf is wanted). +/// +/// This addresses audit item EQ-QNT-WORMHOLE-F-05. #[derive(Encode, Decode, Clone, Eq, PartialEq, Default, TypeInfo, Debug, DecodeWithMemTracking)] #[scale_info(skip_type_params(T))] pub struct WormholeProofRecorderExtension(PhantomData); @@ -109,12 +134,37 @@ impl WormholeProofRecorderExtension /// /// Per recorded transfer, `record_transfer` touches one `TransferCount` read and one /// write, plus the ZK-tree leaf insert, whose path update walks the tree leaf-to-root - /// and therefore costs reads/writes proportional to the *current* tree depth (read from - /// storage here, so the charge tracks the tree as it deepens over the chain's life). + /// and therefore costs reads/writes *and* one Poseidon hash per level, both + /// proportional to the *current* tree depth (read from storage here, so the charge + /// tracks the tree as it deepens over the chain's life). fn per_transfer_weight() -> Weight { let (tree_reads, tree_writes) = pallet_zk_tree::Pallet::::insert_leaf_db_ops(); + let hash_time = pallet_zk_tree::Pallet::::insert_leaf_hash_ref_time(); T::DbWeight::get() .reads_writes(1u64.saturating_add(tree_reads), 1u64.saturating_add(tree_writes)) + .saturating_add(Weight::from_parts(hash_time, 0)) + } + + /// Worst-case `ref_time` (picoseconds) to stream-decode one `EventRecord` in + /// [`Self::record_proofs_from_events_since`]. A record is a small SCALE blob + /// (phase + event enum + topics, typically well under ~300 bytes) decoded from an + /// already-fetched storage value — roughly 100–300ns of pure decode on reference + /// hardware; 1µs is a conservative ceiling. + const EVENT_SCAN_DECODE_REF_TIME_PS: u64 = 1_000_000; + + /// Weight of the post-dispatch event scan when `events` records are present at + /// scan time. `Events::stream_iter` fetches the storage value (one read) and the + /// scan then decodes EVERY record present — `Iterator::skip` discards but still + /// decodes the pre-snapshot prefix — so the cost is per record *present*, not per + /// record matched or recorded. + fn event_scan_weight(events: u32) -> Weight { + if events == 0 { + return Weight::zero(); + } + T::DbWeight::get().reads(1).saturating_add(Weight::from_parts( + Self::EVENT_SCAN_DECODE_REF_TIME_PS.saturating_mul(u64::from(events)), + 0, + )) } fn count_transfers(call: &RuntimeCall) -> u64 { @@ -193,6 +243,30 @@ impl WormholeProofRecorderExtension let minting_account = crate::configs::MintingAccount::get(); Some((None, minting_account, who, amount)) }, + // Held-balance transfers. The reversible-transfers pallet releases + // seized/recovered funds to the guardian with `transfer_on_hold` + // (`Restriction::Free`), so the destination receives ordinary free + // balance — a genuine credit that needs a leaf exactly like a + // `Transfer`, it just emits a different event. (`TransferAndHold` + // is deliberately not matched: nothing in the runtime emits it.) + RuntimeEvent::Balances(pallet_balances::Event::TransferOnHold { + source, + dest, + amount, + .. + }) => Some((None, source, dest, amount)), + // Reserved-balance repatriations. `pallet_recovery::close_recovery` + // seizes the rescuer's recovery deposit into the rescued account with + // `repatriate_reserved`, which emits this instead of a `Transfer`. The + // event is only emitted for cross-account moves (self-repatriations + // return early), and the credit belongs to `to` whether it lands free + // or reserved, so record it unconditionally. + RuntimeEvent::Balances(pallet_balances::Event::ReserveRepatriated { + from, + to, + amount, + .. + }) => Some((None, from, to, amount)), _ => None, // Ignore all other events } }) @@ -273,20 +347,33 @@ impl TransactionEx // Use the event count snapshot from prepare() to avoid duplicate recording. if result.is_ok() { let (event_count_before, charged_transfers) = pre; + // Captured BEFORE recording deposits new events: this is exactly the number + // of records the scan below decodes. + let events_at_scan = frame_system::Pallet::::event_count(); let recorded = Self::record_proofs_from_events_since(event_count_before); - // Wrappers that dispatch inner calls stored on-chain (`Multisig::execute`, - // `ReversibleTransfers::recover_funds`, ...) can emit transfer events the static - // `count_transfers` matcher cannot see, so the proof-recording work above may exceed - // the weight reserved by `weight()`. Register the shortfall against the block so - // block-weight based DoS protection stays sound even when the static count drifts. + // Two pieces of caller-influenced work here are invisible to the static + // `weight()` and are therefore registered against the block post-hoc (this + // keeps block-capacity accounting sound; it is not fee-charged): + // + // 1. The event scan itself: any call can emit events the scan must decode (e.g. batched + // `remark_with_event`), and the decode cost is per record present at scan time — see + // `event_scan_weight`. + // + // 2. Recording shortfall: wrappers that dispatch inner calls stored on-chain + // (`Multisig::execute`, `ReversibleTransfers::recover_funds`, ...) can emit transfer + // events the static `count_transfers` matcher cannot see, so the proof-recording + // work above may exceed the weight reserved by `weight()`. + let mut extra = Self::event_scan_weight(events_at_scan); if recorded > charged_transfers { - frame_system::Pallet::::register_extra_weight_unchecked( + extra = extra.saturating_add( Self::per_transfer_weight() .saturating_mul(recorded.saturating_sub(charged_transfers)), - info.class, ); } + if extra != Weight::zero() { + frame_system::Pallet::::register_extra_weight_unchecked(extra, info.class); + } } Ok(()) @@ -782,6 +869,29 @@ mod tests { }); } + #[test] + fn per_transfer_weight_includes_tree_hash_compute() { + new_test_ext().execute_with(|| { + // Recording a transfer inserts a ZK-tree leaf; the path update computes one + // Poseidon hash per tree level. That compute must be charged on top of the + // DB ops, otherwise every recorded transfer under-declares execution work + // by an amount that grows with the tree depth. + pallet_zk_tree::Depth::::put(20); + let weight = WormholeProofRecorderExtension::::per_transfer_weight(); + + let (tree_reads, tree_writes) = pallet_zk_tree::insert_leaf_db_ops_at_depth(20); + let db_time = ::DbWeight::get() + .reads_writes(1u64.saturating_add(tree_reads), 1u64.saturating_add(tree_writes)) + .ref_time(); + assert!( + weight.ref_time() >= + db_time + pallet_zk_tree::insert_leaf_hash_ref_time_at_depth(20), + "per-transfer weight must charge the leaf insert's Poseidon hashing \ + on top of its DB ops" + ); + }); + } + #[test] fn per_transfer_weight_scales_with_tree_depth() { new_test_ext().execute_with(|| { @@ -813,19 +923,59 @@ mod tests { let weight_before = frame_system::Pallet::::block_weight().total(); + let scanned = core::cell::Cell::new(0u32); run_lifecycle(&alice(), opaque_call, || { assert_ok!(Balances::transfer_keep_alive( RuntimeOrigin::signed(alice()), MultiAddress::Id(bob()), EXISTENTIAL_DEPOSIT * 50, )); + scanned.set(frame_system::Pallet::::event_count()); }); let weight_after = frame_system::Pallet::::block_weight().total(); assert_eq!( weight_after.saturating_sub(weight_before), - WormholeProofRecorderExtension::::per_transfer_weight(), - "the uncounted recorded transfer must be registered as extra block weight" + WormholeProofRecorderExtension::::per_transfer_weight().saturating_add( + WormholeProofRecorderExtension::::event_scan_weight(scanned.get()) + ), + "the uncounted recorded transfer must be registered as extra block weight, \ + on top of the always-registered event-scan weight" + ); + }); + } + + /// The post-dispatch scan streams `System::Events` through a decoding iterator — + /// and `skip()` still decodes the records it discards — so every event record + /// present at scan time costs decode work even when nothing is recorded. A signed + /// caller can emit arbitrarily many events with zero-transfer calls (e.g. batched + /// `remark_with_event`), so that work must be registered against the block. + #[test] + fn wormhole_proof_recorder_registers_event_scan_weight() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + + let call = RuntimeCall::System(frame_system::Call::remark { remark: vec![1] }); + assert_eq!(WormholeProofRecorderExtension::::count_transfers(&call), 0); + + let weight_before = frame_system::Pallet::::block_weight().total(); + + // Capture the event count at the end of the dispatch closure: that is + // exactly the number of records the post-dispatch scan decodes. + let scanned = core::cell::Cell::new(0u32); + run_lifecycle(&alice(), call, || { + for i in 0..7u8 { + assert_ok!(System::remark_with_event(RuntimeOrigin::signed(alice()), vec![i],)); + } + scanned.set(frame_system::Pallet::::event_count()); + }); + assert!(scanned.get() >= 7, "the remarks must have emitted events"); + + let weight_after = frame_system::Pallet::::block_weight().total(); + assert_eq!( + weight_after.saturating_sub(weight_before), + WormholeProofRecorderExtension::::event_scan_weight(scanned.get()), + "the per-event decode work of the scan must be registered as block weight" ); }); } @@ -994,6 +1144,87 @@ mod tests { }); } + #[test] + fn event_based_proof_recording_guardian_seizure_via_transfer_on_hold() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + + let amount = EXISTENTIAL_DEPOSIT * 10; + let guardian = alice(); + let count_before = Wormhole::transfer_count(&guardian); + + // charlie is high-security (guardian = alice, from genesis); scheduling a + // transfer places the funds on hold. + assert_ok!(ReversibleTransfers::schedule_transfer( + RuntimeOrigin::signed(charlie()), + MultiAddress::Id(bob()), + amount, + )); + let tx_id = + pallet_reversible_transfers::PendingTransfersBySender::::get(charlie())[0]; + + // The guardian cancels: the held funds (minus the volume fee) are seized to + // the guardian via `transfer_on_hold`, which emits `Balances::TransferOnHold` + // — not a free-balance `Transfer`. The credit is real spendable value landing + // on the guardian's free balance, so the recorder must create a leaf for it + // exactly as it would for a plain transfer. + let events_before = frame_system::Pallet::::event_count(); + assert_ok!(ReversibleTransfers::cancel(RuntimeOrigin::signed(guardian.clone()), tx_id)); + + let recorded = + WormholeProofRecorderExtension::::record_proofs_from_events_since( + events_before, + ); + + assert_eq!(recorded, 1, "hold-transfer seizure must be recorded as a transfer proof"); + assert_eq!(Wormhole::transfer_count(&guardian), count_before + 1); + }); + } + + #[test] + fn event_based_proof_recording_recovery_deposit_repatriation() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + + // alice makes her account recoverable; bob (say, maliciously) initiates a + // recovery, reserving the recovery deposit on his own account. The recovery + // deposits are UNIT-denominated, so fund both well past the genesis balances. + Balances::make_free_balance_be(&alice(), 100 * crate::UNIT); + Balances::make_free_balance_be(&bob(), 100 * crate::UNIT); + assert_ok!(Recovery::create_recovery( + RuntimeOrigin::signed(alice()), + vec![charlie()], + 1, + 0, + )); + assert_ok!(Recovery::initiate_recovery( + RuntimeOrigin::signed(bob()), + MultiAddress::Id(alice()), + )); + + let count_before = Wormhole::transfer_count(&alice()); + let events_before = frame_system::Pallet::::event_count(); + + // Closing the recovery seizes the rescuer's reserved deposit into alice's + // free balance via `repatriate_reserved`, which emits + // `Balances::ReserveRepatriated` — not a free-balance `Transfer`. The + // credit is real spendable value landing on alice, so the recorder must + // create a leaf for it. + assert_ok!(Recovery::close_recovery( + RuntimeOrigin::signed(alice()), + MultiAddress::Id(bob()), + )); + + let recorded = + WormholeProofRecorderExtension::::record_proofs_from_events_since( + events_before, + ); + + assert_eq!(recorded, 1, "reserve repatriation must be recorded as a transfer proof"); + assert_eq!(Wormhole::transfer_count(&alice()), count_before + 1); + }); + } + #[test] fn event_based_proof_recording_no_proof_for_non_transfer() { new_test_ext().execute_with(|| {