diff --git a/Cargo.lock b/Cargo.lock index 68664480..61647ad0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3365,6 +3365,7 @@ dependencies = [ "libsecp256k1", "log", "pallet-evm", + "pallet-evm-precompile-shielded-pool", "pallet-relayer-runtime-api", "pallet-shielded-pool-runtime-api", "parity-scale-codec", @@ -7894,7 +7895,7 @@ dependencies = [ [[package]] name = "pallet-evm-precompile-shielded-pool" -version = "0.4.0" +version = "0.5.1" dependencies = [ "fp-evm", "frame-support", @@ -8103,7 +8104,7 @@ dependencies = [ [[package]] name = "pallet-shielded-pool" -version = "0.17.0" +version = "0.17.1" dependencies = [ "ark-bn254", "ark-ff 0.5.0", diff --git a/client/rpc/Cargo.toml b/client/rpc/Cargo.toml index 4b3562ae..37ee1f34 100644 --- a/client/rpc/Cargo.toml +++ b/client/rpc/Cargo.toml @@ -61,6 +61,7 @@ fp-evm = { workspace = true, features = ["default"] } fp-rpc = { workspace = true, features = ["default"] } fp-storage = { workspace = true, features = ["default"] } pallet-evm = { workspace = true, features = ["default"] } +pallet-evm-precompile-shielded-pool = { workspace = true, features = ["std"] } pallet-relayer-runtime-api = { workspace = true, features = ["std"] } pallet-shielded-pool-runtime-api = { workspace = true, features = ["std"] } diff --git a/client/rpc/src/relay/config.rs b/client/rpc/src/relay/config.rs new file mode 100644 index 00000000..daeb14ca --- /dev/null +++ b/client/rpc/src/relay/config.rs @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +//! Relay constants, grouped by what they govern. +//! +//! Three roles are kept apart because they answer to different authorities: the +//! admission limits are the relay's own policy, the transaction parameters are +//! what it signs and pays for, and the fallbacks are last-resort copies of state +//! that normally lives on-chain. +//! +//! `RELAY_GAS_LIMIT` deliberately spans two of them — it bounds the transaction +//! the relay signs and, through the 2× gas floor, the fee it demands in return. + +use super::operations::{SELECTOR_PRIVATE_TRANSFER, SELECTOR_UNSHIELD}; + +// --------------------------------------------------------------------------- +// Target +// --------------------------------------------------------------------------- + +/// ShieldedPool precompile: `0x0000000000000000000000000000000000000801`. +/// +/// The only address the relay will call. Anything else is rejected before the +/// selector is even read. +pub(crate) const SHIELDED_POOL_PRECOMPILE: [u8; 20] = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x08, 0x01, +]; + +// --------------------------------------------------------------------------- +// Admission limits +// --------------------------------------------------------------------------- + +/// Maximum calldata size accepted by the relay (32 KB). +/// +/// A realistic shielded-pool calldata is ~2–5 KB: a 256 B Groth16 proof plus the +/// ABI head and Merkle path. The cap prevents an attacker from passing the +/// selector and fee checks with megabytes of data the relayer would then pay +/// calldata gas for. +pub(crate) const MAX_CALLDATA_BYTES: usize = 32_768; + +// --------------------------------------------------------------------------- +// Transaction parameters +// --------------------------------------------------------------------------- + +/// Maximum fee per gas paid by the relay tx (10 gwei). +pub(crate) const MAX_FEE_PER_GAS_WEI: u64 = 10_000_000_000; + +/// Gas limit used for relay transactions, and the basis of the 2× gas floor in +/// [`super::validation::compute_effective_min_fee`]. +pub(crate) const RELAY_GAS_LIMIT: u64 = 2_000_000; + +// --------------------------------------------------------------------------- +// Runtime API fallbacks +// --------------------------------------------------------------------------- + +/// Last-resort minimum fee, used ONLY when the `relay_config()` Runtime API call +/// fails entirely — that is, on a node running a pre-API runtime. +/// +/// The authoritative value lives in `pallet-relayer::MinRelayFee` and is +/// governance-modifiable via `set_min_relay_fee`. This matches +/// `pallet-relayer::DefaultMinRelayFee` (0.001 ORB) so all three sources agree +/// out of the box. +pub(crate) const MIN_RELAY_FEE_FALLBACK: u128 = 1_000_000_000_000_000; // 0.001 ORB in planck + +/// Selector whitelist used on the same fallback path. +/// +/// Built from the operation constants — themselves re-exported from the +/// precompile — so this list cannot drift from what the decoder accepts. +pub(crate) const SELECTORS_FALLBACK: [[u8; 4]; 2] = [SELECTOR_UNSHIELD, SELECTOR_PRIVATE_TRANSFER]; diff --git a/client/rpc/src/relay/mod.rs b/client/rpc/src/relay/mod.rs index 28d174cf..8f1fbed4 100644 --- a/client/rpc/src/relay/mod.rs +++ b/client/rpc/src/relay/mod.rs @@ -7,294 +7,35 @@ //! - `orbinum_relayShieldedCall(calldata: "0x...")` → `txHash` //! - `orbinum_relayerStatus()` → `{ address, minFee, balanceWei, enabled, isRegistered }` //! -//! The relay only accepts calls to the ShieldedPool precompile -//! (`0x0000000000000000000000000000000000000801`) with selector -//! `0x47fc44a2` (unshield) or `0x8c0f5d24` (privateTransfer). -//! It checks the fee embedded in ABI slot 6 is ≥ the current `min_relay_fee` from -//! `pallet-relayer` (queried dynamically via Runtime API so forkless upgrades take effect immediately). +//! The relay pays gas on a user's behalf, so it will only call the ShieldedPool +//! precompile (`0x…0801`), only with a whitelisted selector, and only when the +//! fee in ABI slot 6 covers what the transaction will cost it. The whitelist and +//! fee floor come from `pallet-relayer` via Runtime API, read on every call so +//! governance changes apply without a node restart. //! //! # Module layout //! -//! | Sub-module | Responsibility | -//! |-----------------|-------------------------------------------------------------| -//! | [`operations`] | `RelayableOperation` trait + `UnshieldOp` + `PrivateTransferOp` (Capa 3) | -//! | [`types`] | `OrbinumRelayApi` RPC trait + `RelayerStatus` response type | -//! | [`validation`] | Pure calldata validation + fee-floor computation + tests | +//! The split is by what each part needs in order to run: +//! +//! | Sub-module | Responsibility | +//! |----------------|-----------------------------------------------------------| +//! | [`config`] | Constants: the target, admission limits, tx parameters, and Runtime API fallbacks | +//! | [`types`] | `OrbinumRelayApi` RPC trait + `RelayerStatus` response | +//! | [`operations`] | Per-operation selector, length, and fee extraction | +//! | [`validation`] | Pure calldata checks — bytes in, verdict out; no chain state | +//! | [`rpc`] | `OrbinumRelay`: dry run, nonce, signing, pool submission | +//! +//! Keeping [`validation`] free of chain state is what lets `tests/adversarial.rs` +//! throw hostile calldata at it without a node. +pub mod config; pub mod operations; +pub mod rpc; pub mod types; pub mod validation; -pub use types::{OrbinumRelayApiServer, RelayerStatus}; - -use std::sync::Arc; - -use ethereum::TransactionAction; -use ethereum_types::{H160, H256, U256}; -use jsonrpsee::core::RpcResult; -// Substrate -use sc_client_api::backend::{Backend, StorageProvider}; -use sc_transaction_pool_api::{TransactionPool, TransactionSource}; -use sp_api::ProvideRuntimeApi; -use sp_blockchain::HeaderBackend; -use sp_runtime::traits::Block as BlockT; -// Frontier -use fc_rpc_core::types::{Bytes, TransactionMessage}; -use fp_rpc::{ConvertTransactionRuntimeApi, EthereumRuntimeRPCApi}; -// Orbinum -use pallet_relayer_runtime_api::RelayerRuntimeApi; -use pallet_shielded_pool_runtime_api::ShieldedPoolRuntimeApi; - -use crate::{internal_err, signer::EthValidatorSigner}; - -use validation::{ - check_dry_run_exit, compute_effective_min_fee, validate_relay_calldata, MAX_FEE_PER_GAS_WEI, - MIN_RELAY_FEE_FALLBACK, RELAY_GAS_LIMIT, SELECTORS_FALLBACK, SHIELDED_POOL_PRECOMPILE, -}; - -// --------------------------------------------------------------------------- -// Server struct -// --------------------------------------------------------------------------- - -pub struct OrbinumRelay { - client: Arc, - pool: Arc

, - signer: EthValidatorSigner, - /// Serializes relay submissions AND tracks the optimistic next-nonce. - /// - /// Why `Option` instead of `()`: - /// - /// Each submission is: (1) read confirmed nonce, (2) sign, (3) submit. With a plain - /// Mutex<()> two requests in the same block both read `confirmed = N`, sign with N, - /// and the second submission fails with "nonce already used", burning gas for nothing. - /// - /// By storing the last submitted nonce we can compute: - /// actual_nonce = max(confirmed_nonce, memory_nonce) - /// which allows multiple submissions within the same block to use N, N+1, N+2… - /// On the next block `confirmed_nonce` catches up and `memory_nonce` resets naturally. - /// - /// `None` = no submission has been made yet; read from runtime. - submit_lock: Arc>>, - _phantom: std::marker::PhantomData<(B, BE)>, -} - -impl OrbinumRelay -where - B: BlockT, -{ - pub fn new(client: Arc, pool: Arc

, signer: EthValidatorSigner) -> Self { - Self { - client, - pool, - signer, - submit_lock: Arc::new(tokio::sync::Mutex::new(None)), - _phantom: Default::default(), - } - } -} - -#[jsonrpsee::core::async_trait] -impl OrbinumRelayApiServer for OrbinumRelay -where - B: BlockT, - C: ProvideRuntimeApi + HeaderBackend + StorageProvider + 'static, - C::Api: EthereumRuntimeRPCApi - + ConvertTransactionRuntimeApi - + ShieldedPoolRuntimeApi - + RelayerRuntimeApi, - BE: Backend + 'static, - P: TransactionPool + 'static, -{ - async fn relay_shielded_call(&self, calldata: Bytes) -> RpcResult { - let data = calldata.into_vec(); - - // Load fee/selector config from the runtime on every call so governance changes - // (set_min_relay_fee extrinsic) take immediate effect without a node restart. - // MIN_RELAY_FEE_FALLBACK is only used when the Runtime API call fails entirely. - let best_hash = self.client.info().best_hash; - let (min_fee_planck, allowed_selectors) = { - match self.client.runtime_api().relay_config(best_hash) { - Ok(cfg) => (cfg.min_fee_planck, cfg.allowed_selectors), - Err(e) => { - log::warn!( - target: "orbinum-relay", - "relay_config Runtime API unavailable, using fallback: {e}" - ); - (MIN_RELAY_FEE_FALLBACK, SELECTORS_FALLBACK.to_vec()) - } - } - }; - - // Compute 2× gas floor: the relay must earn at least twice what it spends on EVM gas. - // 1 wei == 1 plank in Orbinum, so no unit conversion is required. - let base_fee_wei: u128 = self - .client - .runtime_api() - .gas_price(best_hash) - .map(|p| p.as_u128()) - .unwrap_or(0); - let effective_min_fee = compute_effective_min_fee(min_fee_planck, base_fee_wei); - - if let Err(e) = validate_relay_calldata(&data, effective_min_fee, &allowed_selectors) { - if e == "fee below minimum" { - // Extract the fee from slot 6 for the log (validated length already). - let provided_fee = if data.len() >= 228 { - U256::from_big_endian(&data[196..228]) - } else { - U256::zero() - }; - log::warn!( - target: "orbinum-relay", - "relay rejected: fee below minimum — provided={provided_fee} required={effective_min_fee} (base_fee={base_fee_wei} governance={min_fee_planck})", - ); - } - return Err(internal_err(e)); - } +#[cfg(test)] +mod tests; - // Dry-run: simulate the EVM call without broadcasting a transaction. - // This catches invalid ZK proofs, already-spent nullifiers, and any other - // on-chain rejection BEFORE the relayer signs and pays gas. - { - let relayer_addr = self.signer.address(); - let dry_result = self.client.runtime_api().call( - best_hash, - relayer_addr, - H160::from(SHIELDED_POOL_PRECOMPILE), - data.clone(), - U256::zero(), - U256::from(RELAY_GAS_LIMIT), - Some(U256::from(MAX_FEE_PER_GAS_WEI)), - Some(U256::from(1_000_000_000u64)), - None, // nonce — not needed for simulation - false, // estimate = false: real execution semantics - None, // access_list - None, // authorization_list - ); - match dry_result { - Err(e) => { - log::warn!( - target: "orbinum-relay", - "dry-run Runtime API error: {e}" - ); - return Err(internal_err(format!("dry-run runtime error: {e}"))); - } - Ok(Err(dispatch_err)) => { - log::warn!( - target: "orbinum-relay", - "dry-run dispatch error: {dispatch_err:?}" - ); - return Err(internal_err(format!( - "calldata rejected by runtime: {dispatch_err:?}" - ))); - } - Ok(Ok(info)) => { - if let Err(e) = check_dry_run_exit(&info.exit_reason) { - log::warn!( - target: "orbinum-relay", - "dry-run EVM execution failed — exit={:?} revert_data={:?}", - info.exit_reason, - info.value - ); - return Err(internal_err(e)); - } - } - } - } - - // Hold the lock for the full sign-and-submit sequence. - // The lock also carries the optimistic next-nonce so multiple submissions - // within the same block don't collide on the same confirmed nonce. - let mut nonce_guard = self.submit_lock.lock().await; - - let relayer_addr = self.signer.address(); - - let (chain_id, nonce) = { - let api = self.client.runtime_api(); - let chain_id = api - .chain_id(best_hash) - .map_err(|e| internal_err(format!("chain_id: {e}")))?; - let confirmed_nonce = api - .account_basic(best_hash, relayer_addr) - .map_err(|e| internal_err(format!("account_basic: {e}")))? - .nonce; - // Use the in-memory nonce when it's ahead of the confirmed one. - // This lets us submit N txs within a single block using nonces N, N+1, N+2… - // Once the block is imported the confirmed nonce catches up naturally. - let nonce = match *nonce_guard { - Some(mem) if mem > confirmed_nonce => mem, - _ => confirmed_nonce, - }; - (chain_id, nonce) - }; - - let message = TransactionMessage::EIP1559(ethereum::EIP1559TransactionMessage { - chain_id, - nonce, - max_priority_fee_per_gas: U256::from(1_000_000_000u64), - max_fee_per_gas: U256::from(MAX_FEE_PER_GAS_WEI), - gas_limit: U256::from(RELAY_GAS_LIMIT), - action: TransactionAction::Call(H160::from(SHIELDED_POOL_PRECOMPILE)), - value: U256::zero(), - input: data, - access_list: vec![], - }); - - use crate::signer::EthSigner as _; - let transaction = self.signer.sign(message, &relayer_addr)?; - let tx_hash = transaction.hash(); - - let extrinsic = { - let api = self.client.runtime_api(); - api.convert_transaction(best_hash, transaction) - .map_err(|e| internal_err(format!("convert_transaction: {e}")))? - }; - - let submit_result = self - .pool - .submit_one(best_hash, TransactionSource::Local, extrinsic) - .await - .map(|_| tx_hash) - .map_err(|e| internal_err(format!("pool submit: {e}"))); - - // Advance the in-memory nonce only after a successful submit. - // On failure the nonce slot is still free and the next call will retry with - // the same (or a freshly confirmed) nonce. - if submit_result.is_ok() { - *nonce_guard = Some(nonce + U256::one()); - } - - submit_result - } - - async fn relayer_status(&self) -> RpcResult { - let best_hash = self.client.info().best_hash; - let api = self.client.runtime_api(); - - let min_fee_planck = api - .relay_config(best_hash) - .map(|cfg| cfg.min_fee_planck) - .unwrap_or(MIN_RELAY_FEE_FALLBACK); - - let base_fee_wei: u128 = api.gas_price(best_hash).map(|p| p.as_u128()).unwrap_or(0); - let min_fee = compute_effective_min_fee(min_fee_planck, base_fee_wei); - - let balance = { - api.account_basic(best_hash, self.signer.address()) - .map_err(|e| internal_err(format!("account_basic: {e}")))? - .balance - }; - // Capa 2: check whether this relay's EVM address is registered on-chain. - let is_registered = api - .is_relayer_evm(best_hash, self.signer.address().0) - .unwrap_or(false); - // Consider relay operational when it can cover at least one worst-case tx. - let min_operational = U256::from(min_fee); - Ok(RelayerStatus { - address: self.signer.address(), - min_fee: format!("{min_fee}"), - balance_wei: format!("{balance}"), - enabled: balance >= min_operational, - is_registered, - }) - } -} +pub use rpc::OrbinumRelay; +pub use types::{OrbinumRelayApiServer, RelayerStatus}; diff --git a/client/rpc/src/relay/operations.rs b/client/rpc/src/relay/operations.rs index 05feab1b..5c83534a 100644 --- a/client/rpc/src/relay/operations.rs +++ b/client/rpc/src/relay/operations.rs @@ -10,11 +10,14 @@ use ethereum_types::U256; -/// 4-byte ABI selector for `unshield(...)`. -pub(crate) const SELECTOR_UNSHIELD: [u8; 4] = [0x47, 0xfc, 0x44, 0xa2]; - -/// 4-byte ABI selector for `privateTransfer(...)`. -pub(crate) const SELECTOR_PRIVATE_TRANSFER: [u8; 4] = [0x8c, 0x0f, 0x5d, 0x24]; +/// Selectors the relay accepts, re-exported from the precompile that decodes +/// them. Aliases rather than literals: the whitelist and the decoder cannot +/// disagree if there is only one definition. A hand-kept copy drifting from the +/// decoder fails silently — a wrong selector is merely "unsupported", so the +/// rejection tests stay green while the accept path stops working. +pub(crate) use pallet_evm_precompile_shielded_pool::selectors::{ + PRIVATE_TRANSFER as SELECTOR_PRIVATE_TRANSFER, UNSHIELD as SELECTOR_UNSHIELD, +}; /// Describes how to validate calldata for a specific relayable on-chain operation. /// @@ -38,9 +41,32 @@ pub(crate) trait RelayableOperation: Send + Sync { fn extract_fee(&self, calldata: &[u8]) -> u128; } -/// `unshield(proof, root, nullifier, asset_id, amount, recipient, fee)` — `0x47fc44a2` +/// Reads the relay fee from ABI slot 6 (`calldata[196..228]`), the position both +/// operations share. +/// +/// Saturates instead of panicking on a value above `u128::MAX`. Calldata reaches +/// this from an unauthenticated RPC call, and `U256::as_u128` panics outright on +/// anything wider — one crafted 32-byte word would take down the handler. +/// Saturating is safe because the result is only ever compared against the fee +/// floor: an absurd fee clears it here and is then rejected by the EVM dry-run, +/// which is what would have happened anyway. +/// +/// The slice is bounds-checked by the caller's `min_calldata_len()` gate (260 or +/// 324, both well past 228). +fn fee_at_slot_6(calldata: &[u8]) -> u128 { + let Ok(bytes) = <[u8; 32]>::try_from(&calldata[196..228]) else { + return 0; // unreachable behind the length gate; a zero fee fails the floor + }; + U256::from_big_endian(&bytes) + .try_into() + .unwrap_or(u128::MAX) +} + +/// `unshield(proof, root, nullifier, asset_id, amount, recipient, fee, +/// change_commitment, change_encrypted_memo, circuit_version)` — `0x4e505348` /// -/// Fee is in ABI slot 6: `calldata[196..228]`. +/// Fee is in ABI slot 6: `calldata[196..228]`. The head is 10 slots (320 bytes) +/// plus the 4-byte selector = 324 minimum. pub(crate) struct UnshieldOp; impl RelayableOperation for UnshieldOp { @@ -53,18 +79,19 @@ impl RelayableOperation for UnshieldOp { } fn min_calldata_len(&self) -> usize { - 228 + 324 } fn extract_fee(&self, calldata: &[u8]) -> u128 { - let bytes: [u8; 32] = calldata[196..228].try_into().unwrap(); - U256::from_big_endian(&bytes).as_u128() + fee_at_slot_6(calldata) } } -/// `privateTransfer(proof, root, nullifiers, commitments, memos, asset_id, fee)` — `0x8c0f5d24` +/// `privateTransfer(proof, root, nullifiers, commitments, memos, asset_id, fee, +/// circuit_version)` — `0x66ed2cd4` /// -/// Fee is in ABI slot 6: `calldata[196..228]`. +/// Fee is in ABI slot 6: `calldata[196..228]`. The head is 8 slots (256 bytes) +/// plus the 4-byte selector = 260 minimum. pub(crate) struct PrivateTransferOp; impl RelayableOperation for PrivateTransferOp { @@ -77,12 +104,11 @@ impl RelayableOperation for PrivateTransferOp { } fn min_calldata_len(&self) -> usize { - 228 + 260 } fn extract_fee(&self, calldata: &[u8]) -> u128 { - let bytes: [u8; 32] = calldata[196..228].try_into().unwrap(); - U256::from_big_endian(&bytes).as_u128() + fee_at_slot_6(calldata) } } diff --git a/client/rpc/src/relay/rpc.rs b/client/rpc/src/relay/rpc.rs new file mode 100644 index 00000000..2a17d00e --- /dev/null +++ b/client/rpc/src/relay/rpc.rs @@ -0,0 +1,296 @@ +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +//! The RPC server itself: everything that needs chain state. +//! +//! [`super::validation`] decides whether calldata is admissible from the bytes +//! alone. What is left here is the part that cannot be answered offline — +//! querying governance config, simulating the call, allocating a nonce, signing, +//! and submitting to the pool. +//! +//! Both handlers read `relay_config()` on every call rather than caching it, so +//! a governance change takes effect without restarting the node. + +use std::sync::Arc; + +use ethereum::TransactionAction; +use ethereum_types::{H160, H256, U256}; +use jsonrpsee::core::RpcResult; +// Substrate +use sc_client_api::backend::{Backend, StorageProvider}; +use sc_transaction_pool_api::{TransactionPool, TransactionSource}; +use sp_api::ProvideRuntimeApi; +use sp_blockchain::HeaderBackend; +use sp_runtime::traits::Block as BlockT; +// Frontier +use fc_rpc_core::types::{Bytes, TransactionMessage}; +use fp_rpc::{ConvertTransactionRuntimeApi, EthereumRuntimeRPCApi}; +// Orbinum +use pallet_relayer_runtime_api::RelayerRuntimeApi; +use pallet_shielded_pool_runtime_api::ShieldedPoolRuntimeApi; + +use crate::{internal_err, signer::EthValidatorSigner}; + +use super::{ + config::{ + MAX_FEE_PER_GAS_WEI, MIN_RELAY_FEE_FALLBACK, RELAY_GAS_LIMIT, SELECTORS_FALLBACK, + SHIELDED_POOL_PRECOMPILE, + }, + types::{OrbinumRelayApiServer, RelayerStatus}, + validation::{check_dry_run_exit, compute_effective_min_fee, validate_relay_calldata}, +}; + +// --------------------------------------------------------------------------- +// Server struct +// --------------------------------------------------------------------------- + +pub struct OrbinumRelay { + client: Arc, + pool: Arc

, + signer: EthValidatorSigner, + /// Serializes relay submissions AND tracks the optimistic next-nonce. + /// + /// Why `Option` instead of `()`: + /// + /// Each submission is: (1) read confirmed nonce, (2) sign, (3) submit. With a plain + /// Mutex<()> two requests in the same block both read `confirmed = N`, sign with N, + /// and the second submission fails with "nonce already used", burning gas for nothing. + /// + /// By storing the last submitted nonce we can compute: + /// actual_nonce = max(confirmed_nonce, memory_nonce) + /// which allows multiple submissions within the same block to use N, N+1, N+2… + /// On the next block `confirmed_nonce` catches up and `memory_nonce` resets naturally. + /// + /// `None` = no submission has been made yet; read from runtime. + submit_lock: Arc>>, + _phantom: std::marker::PhantomData<(B, BE)>, +} + +impl OrbinumRelay +where + B: BlockT, +{ + pub fn new(client: Arc, pool: Arc

, signer: EthValidatorSigner) -> Self { + Self { + client, + pool, + signer, + submit_lock: Arc::new(tokio::sync::Mutex::new(None)), + _phantom: Default::default(), + } + } +} + +#[jsonrpsee::core::async_trait] +impl OrbinumRelayApiServer for OrbinumRelay +where + B: BlockT, + C: ProvideRuntimeApi + HeaderBackend + StorageProvider + 'static, + C::Api: EthereumRuntimeRPCApi + + ConvertTransactionRuntimeApi + + ShieldedPoolRuntimeApi + + RelayerRuntimeApi, + BE: Backend + 'static, + P: TransactionPool + 'static, +{ + async fn relay_shielded_call(&self, calldata: Bytes) -> RpcResult { + let data = calldata.into_vec(); + + // Load fee/selector config from the runtime on every call so governance changes + // (set_min_relay_fee extrinsic) take immediate effect without a node restart. + // MIN_RELAY_FEE_FALLBACK is only used when the Runtime API call fails entirely. + let best_hash = self.client.info().best_hash; + let (min_fee_planck, allowed_selectors) = { + match self.client.runtime_api().relay_config(best_hash) { + Ok(cfg) => (cfg.min_fee_planck, cfg.allowed_selectors), + Err(e) => { + log::warn!( + target: "orbinum-relay", + "relay_config Runtime API unavailable, using fallback: {e}" + ); + (MIN_RELAY_FEE_FALLBACK, SELECTORS_FALLBACK.to_vec()) + } + } + }; + + // Compute 2× gas floor: the relay must earn at least twice what it spends on EVM gas. + // 1 wei == 1 plank in Orbinum, so no unit conversion is required. + // Saturate rather than `as_u128()`, which panics on a gas_price ≥ 2^128. + // The value comes from the runtime, not calldata, but a panic here would + // still take down the relay RPC — mirror the fee-word hardening in + // operations.rs::fee_at_slot_6. + let base_fee_wei: u128 = self + .client + .runtime_api() + .gas_price(best_hash) + .map(|p| p.try_into().unwrap_or(u128::MAX)) + .unwrap_or(0); + let effective_min_fee = compute_effective_min_fee(min_fee_planck, base_fee_wei); + + if let Err(e) = validate_relay_calldata(&data, effective_min_fee, &allowed_selectors) { + if e == "fee below minimum" { + // Extract the fee from slot 6 for the log (validated length already). + let provided_fee = if data.len() >= 228 { + U256::from_big_endian(&data[196..228]) + } else { + U256::zero() + }; + log::warn!( + target: "orbinum-relay", + "relay rejected: fee below minimum — provided={provided_fee} required={effective_min_fee} (base_fee={base_fee_wei} governance={min_fee_planck})", + ); + } + return Err(internal_err(e)); + } + + // Dry-run: simulate the EVM call without broadcasting a transaction. + // This catches invalid ZK proofs, already-spent nullifiers, and any other + // on-chain rejection BEFORE the relayer signs and pays gas. + { + let relayer_addr = self.signer.address(); + let dry_result = self.client.runtime_api().call( + best_hash, + relayer_addr, + H160::from(SHIELDED_POOL_PRECOMPILE), + data.clone(), + U256::zero(), + U256::from(RELAY_GAS_LIMIT), + Some(U256::from(MAX_FEE_PER_GAS_WEI)), + Some(U256::from(1_000_000_000u64)), + None, // nonce — not needed for simulation + false, // estimate = false: real execution semantics + None, // access_list + None, // authorization_list + ); + match dry_result { + Err(e) => { + log::warn!( + target: "orbinum-relay", + "dry-run Runtime API error: {e}" + ); + return Err(internal_err(format!("dry-run runtime error: {e}"))); + } + Ok(Err(dispatch_err)) => { + log::warn!( + target: "orbinum-relay", + "dry-run dispatch error: {dispatch_err:?}" + ); + return Err(internal_err(format!( + "calldata rejected by runtime: {dispatch_err:?}" + ))); + } + Ok(Ok(info)) => { + if let Err(e) = check_dry_run_exit(&info.exit_reason) { + log::warn!( + target: "orbinum-relay", + "dry-run EVM execution failed — exit={:?} revert_data={:?}", + info.exit_reason, + info.value + ); + return Err(internal_err(e)); + } + } + } + } + + // Hold the lock for the full sign-and-submit sequence. + // The lock also carries the optimistic next-nonce so multiple submissions + // within the same block don't collide on the same confirmed nonce. + let mut nonce_guard = self.submit_lock.lock().await; + + let relayer_addr = self.signer.address(); + + let (chain_id, nonce) = { + let api = self.client.runtime_api(); + let chain_id = api + .chain_id(best_hash) + .map_err(|e| internal_err(format!("chain_id: {e}")))?; + let confirmed_nonce = api + .account_basic(best_hash, relayer_addr) + .map_err(|e| internal_err(format!("account_basic: {e}")))? + .nonce; + // Use the in-memory nonce when it's ahead of the confirmed one. + // This lets us submit N txs within a single block using nonces N, N+1, N+2… + // Once the block is imported the confirmed nonce catches up naturally. + let nonce = match *nonce_guard { + Some(mem) if mem > confirmed_nonce => mem, + _ => confirmed_nonce, + }; + (chain_id, nonce) + }; + + let message = TransactionMessage::EIP1559(ethereum::EIP1559TransactionMessage { + chain_id, + nonce, + max_priority_fee_per_gas: U256::from(1_000_000_000u64), + max_fee_per_gas: U256::from(MAX_FEE_PER_GAS_WEI), + gas_limit: U256::from(RELAY_GAS_LIMIT), + action: TransactionAction::Call(H160::from(SHIELDED_POOL_PRECOMPILE)), + value: U256::zero(), + input: data, + access_list: vec![], + }); + + use crate::signer::EthSigner as _; + let transaction = self.signer.sign(message, &relayer_addr)?; + let tx_hash = transaction.hash(); + + let extrinsic = { + let api = self.client.runtime_api(); + api.convert_transaction(best_hash, transaction) + .map_err(|e| internal_err(format!("convert_transaction: {e}")))? + }; + + let submit_result = self + .pool + .submit_one(best_hash, TransactionSource::Local, extrinsic) + .await + .map(|_| tx_hash) + .map_err(|e| internal_err(format!("pool submit: {e}"))); + + // Advance the in-memory nonce only after a successful submit. + // On failure the nonce slot is still free and the next call will retry with + // the same (or a freshly confirmed) nonce. + if submit_result.is_ok() { + *nonce_guard = Some(nonce + U256::one()); + } + + submit_result + } + + async fn relayer_status(&self) -> RpcResult { + let best_hash = self.client.info().best_hash; + let api = self.client.runtime_api(); + + let min_fee_planck = api + .relay_config(best_hash) + .map(|cfg| cfg.min_fee_planck) + .unwrap_or(MIN_RELAY_FEE_FALLBACK); + + // Saturate rather than `as_u128()` (panics ≥ 2^128) — see the sibling + // call above; runtime-sourced, but a panic still kills the relay RPC. + let base_fee_wei: u128 = api + .gas_price(best_hash) + .map(|p| p.try_into().unwrap_or(u128::MAX)) + .unwrap_or(0); + let min_fee = compute_effective_min_fee(min_fee_planck, base_fee_wei); + + let balance = { + api.account_basic(best_hash, self.signer.address()) + .map_err(|e| internal_err(format!("account_basic: {e}")))? + .balance + }; + // Capa 2: check whether this relay's EVM address is registered on-chain. + let is_registered = api + .is_relayer_evm(best_hash, self.signer.address().0) + .unwrap_or(false); + // Consider relay operational when it can cover at least one worst-case tx. + let min_operational = U256::from(min_fee); + Ok(RelayerStatus { + address: self.signer.address(), + min_fee: format!("{min_fee}"), + balance_wei: format!("{balance}"), + enabled: balance >= min_operational, + is_registered, + }) + } +} diff --git a/client/rpc/src/relay/tests/adversarial.rs b/client/rpc/src/relay/tests/adversarial.rs new file mode 100644 index 00000000..77272daf --- /dev/null +++ b/client/rpc/src/relay/tests/adversarial.rs @@ -0,0 +1,400 @@ +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +//! Adversarial tests: malformed, hostile, and boundary calldata. +//! +//! These assert the relay rejects rather than panics. Every input here is one a +//! caller can send over the unauthenticated RPC. + +use crate::relay::{ + config::*, + operations::{SELECTOR_PRIVATE_TRANSFER, SELECTOR_UNSHIELD}, + validation::*, +}; + +/// Well-formed base calldata for privateTransfer, long enough to pass the +/// length gates so the later checks are actually reached. +fn base_private_transfer(fee: u128) -> Vec { + let mut d = SELECTOR_PRIVATE_TRANSFER.to_vec(); + d.resize(4 + 256, 0); + let mut fee_word = [0u8; 32]; + fee_word[16..32].copy_from_slice(&fee.to_be_bytes()); + d[4 + 192..4 + 224].copy_from_slice(&fee_word); + d +} + +/// Every length from 0 to just past the minimum: no panic, and the boundary +/// must be exact (227 rejected, 228 reaches the selector check). +#[test] +fn attack_every_calldata_length_is_handled_without_panic() { + for len in 0..400usize { + let data = vec![0xAAu8; len]; + let _ = validate_relay_calldata(&data, 0, &SELECTORS_FALLBACK); + } + // Boundary is exact. + assert_eq!( + validate_relay_calldata(&vec![0u8; 227], 0, &SELECTORS_FALLBACK), + Err("calldata too short") + ); + // 228 bytes of zeros passes the length gate and dies on the selector. + assert_eq!( + validate_relay_calldata(&vec![0u8; 228], 0, &SELECTORS_FALLBACK), + Err("unsupported selector") + ); +} + +/// A valid selector with calldata between the global 228 gate and the +/// operation's own minimum must be refused by the per-op gate, not read +/// past its end. +#[test] +fn attack_length_between_global_and_per_op_minimum_is_refused() { + for len in 228..260usize { + let mut d = SELECTOR_PRIVATE_TRANSFER.to_vec(); + d.resize(len, 0); + assert_eq!( + validate_relay_calldata(&d, 0, &SELECTORS_FALLBACK), + Err("calldata too short"), + "privateTransfer at {len} bytes must be refused" + ); + } + for len in 228..324usize { + let mut d = SELECTOR_UNSHIELD.to_vec(); + d.resize(len, 0); + assert_eq!( + validate_relay_calldata(&d, 0, &SELECTORS_FALLBACK), + Err("calldata too short"), + "unshield at {len} bytes must be refused" + ); + } +} + +/// The calldata cap must hold exactly: one byte over is refused, and the +/// oversized buffer must never be walked. +#[test] +fn attack_oversized_calldata_is_refused_at_the_exact_boundary() { + let mut ok = base_private_transfer(0); + ok.resize(MAX_CALLDATA_BYTES, 0); + // At the cap: passes the size gate (fails later or succeeds, but not "too large"). + assert_ne!( + validate_relay_calldata(&ok, 0, &SELECTORS_FALLBACK), + Err("calldata too large") + ); + + let mut over = base_private_transfer(0); + over.resize(MAX_CALLDATA_BYTES + 1, 0); + assert_eq!( + validate_relay_calldata(&over, 0, &SELECTORS_FALLBACK), + Err("calldata too large") + ); +} + +/// A fee word of all 0xFF (u256::MAX) must saturate, never panic, and must +/// COMPARE as above any minimum — a panic here is remote node death. +#[test] +fn attack_max_fee_word_saturates_and_passes_the_floor() { + let mut d = base_private_transfer(0); + d[4 + 192..4 + 224].copy_from_slice(&[0xFFu8; 32]); + assert_eq!( + validate_relay_calldata(&d, u128::MAX, &SELECTORS_FALLBACK), + Ok(()), + "a saturated fee must clear even the maximum floor" + ); +} + +/// A fee one planck below the floor must be refused; exactly at the floor +/// must pass. Off-by-one here is free money for the attacker or a broken relay. +#[test] +fn attack_fee_floor_boundary_is_exact() { + let floor = 1_000_000_000_000_000u128; + assert_eq!( + validate_relay_calldata( + &base_private_transfer(floor - 1), + floor, + &SELECTORS_FALLBACK + ), + Err("fee below minimum") + ); + assert_eq!( + validate_relay_calldata(&base_private_transfer(floor), floor, &SELECTORS_FALLBACK), + Ok(()) + ); +} + +/// An empty whitelist must reject everything — a governance misconfiguration +/// must fail closed, never open. +#[test] +fn attack_empty_whitelist_fails_closed() { + assert_eq!( + validate_relay_calldata(&base_private_transfer(0), 0, &[]), + Err("unsupported selector") + ); +} + +/// A selector the governance whitelist allows but the node does not +/// implement must be refused, not dispatched to a wrong decoder. +#[test] +fn attack_whitelisted_but_unimplemented_selector_is_refused() { + let mut d = base_private_transfer(0); + d[..4].copy_from_slice(&[0xDE, 0xAD, 0xBE, 0xEF]); + assert_eq!( + validate_relay_calldata(&d, 0, &[[0xDE, 0xAD, 0xBE, 0xEF]]), + Err("unsupported selector"), + "a selector with no registered operation must fail closed" + ); +} + +/// The gas floor must saturate rather than overflow: base_fee near u128::MAX +/// multiplied by 2×gas_limit would wrap and produce a floor of ~0, letting +/// every transfer through for free. +#[test] +fn attack_gas_floor_saturates_instead_of_wrapping() { + let floor = compute_effective_min_fee(1, u128::MAX); + assert_eq!( + floor, + u128::MAX, + "a wrapped multiplication would collapse the floor to near zero" + ); + // And a realistic value still behaves. + let normal = compute_effective_min_fee(1_000_000_000_000_000, 1_000_000_000); + assert!(normal >= 1_000_000_000_000_000); +} + +/// Deterministic byte fuzz over the whole calldata: the only requirement is +/// that no input, however malformed, panics the validator. +#[test] +fn attack_calldata_fuzz_never_panics() { + let base = base_private_transfer(1_000_000_000_000_000); + let mut seed: u64 = 0xDEADBEEFCAFEBABE; + for _ in 0..20_000 { + let mut data = base.clone(); + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + let muts = 1 + (seed >> 60) as usize % 12; + for _ in 0..muts { + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + let pos = (seed >> 33) as usize % data.len(); + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + data[pos] = (seed >> 40) as u8; + } + // Sometimes truncate too. + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + if seed % 3 == 0 { + let cut = (seed >> 33) as usize % data.len().max(1); + data.truncate(cut); + } + let _ = validate_relay_calldata(&data, 1_000_000_000_000_000, &SELECTORS_FALLBACK); + } +} + +/// Fuzz the fee slot specifically with full-width random words — this is the +/// field that historically panicked via `U256::as_u128()`. +#[test] +fn attack_fee_slot_fuzz_never_panics() { + let mut seed: u64 = 0x1234_5678_9ABC_DEF0; + for _ in 0..20_000 { + let mut d = base_private_transfer(0); + for byte in d[4 + 192..4 + 224].iter_mut() { + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + *byte = (seed >> 40) as u8; + } + let _ = validate_relay_calldata(&d, 1_000_000_000_000_000, &SELECTORS_FALLBACK); + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Whitelist integrity +// +// `allowed_selectors` comes from governance storage over a Runtime API. These +// cover what happens when that list is itself hostile or malformed, which the +// node cannot prevent — only survive. +// ───────────────────────────────────────────────────────────────────────────── + +/// A whitelist entry for an operation the node does not implement must not +/// promote the call: the selector matches the whitelist but no decoder claims +/// it, so `default_operations()` finds nothing and the call is refused. +/// +/// This is the forward-compatibility path — governance enabling an operation +/// before the node ships it. Failing open here would relay calldata whose fee +/// slot has never been located, i.e. an unpriced call. +#[test] +fn attack_whitelist_full_of_unimplemented_selectors_fails_closed() { + let hostile: Vec<[u8; 4]> = (0u8..64).map(|i| [i, i, i, i]).collect(); + for sel in &hostile { + let mut data = base_private_transfer(u128::MAX); + data[..4].copy_from_slice(sel); + assert_eq!( + validate_relay_calldata(&data, 1, &hostile), + Err("unsupported selector"), + "selector {sel:?} is whitelisted but unimplemented — must not relay" + ); + } +} + +/// A whitelist containing a real selector many times over must behave exactly +/// as if it appeared once. Guards the `contains` lookup against a governance +/// list padded to provoke quadratic scanning or an early-exit mistake. +#[test] +fn attack_whitelist_with_duplicate_entries_behaves_identically() { + let padded = vec![SELECTOR_PRIVATE_TRANSFER; 4096]; + let data = base_private_transfer(u128::MAX); + assert_eq!(validate_relay_calldata(&data, 1, &padded), Ok(())); + assert_eq!( + validate_relay_calldata(&data, u128::MAX, &padded), + Ok(()), + "a saturated fee word clears any floor, duplicates or not" + ); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Fee floor arithmetic +// +// `compute_effective_min_fee` multiplies runtime-sourced values. Wrapping here +// would invert the comparison and let a zero-fee call through. +// ───────────────────────────────────────────────────────────────────────────── + +/// The 2× gas floor must saturate, never wrap. A wrapped product would come out +/// *small*, and a small floor is one an attacker can clear with a nominal fee +/// while the relay pays real gas. +#[test] +fn attack_gas_floor_never_wraps_below_governance_minimum() { + for base_fee in [ + u128::MAX, + u128::MAX / 2, + u128::MAX / RELAY_GAS_LIMIT as u128, + 1 << 127, + ] { + let floor = compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, base_fee); + assert!( + floor >= MIN_RELAY_FEE_FALLBACK, + "floor {floor} fell below governance minimum at base_fee={base_fee}" + ); + } +} + +/// Governance setting `min_fee_planck` to zero must not disable the gas floor: +/// the relay still has to earn back the gas it spends. +#[test] +fn attack_zero_governance_fee_still_charges_the_gas_floor() { + let floor = compute_effective_min_fee(0, 1_000_000_000); + assert_eq!(floor, 2 * RELAY_GAS_LIMIT as u128 * 1_000_000_000); + assert!( + floor > 0, + "a zero governance fee must not mean a free relay" + ); + + // And with no gas price either, the floor is genuinely zero — documenting + // that the free-relay case requires BOTH to be zero. + assert_eq!(compute_effective_min_fee(0, 0), 0); +} + +/// A fee exactly one planck below the floor is refused; exactly at it passes. +/// Pins the comparison as `<` rather than `<=`, at a boundary an attacker +/// controls precisely. +#[test] +fn attack_fee_one_below_floor_is_refused() { + let floor = 1_000_000u128; + let at = base_private_transfer(floor); + let below = base_private_transfer(floor - 1); + + assert_eq!( + validate_relay_calldata(&at, floor, &SELECTORS_FALLBACK), + Ok(()) + ); + assert_eq!( + validate_relay_calldata(&below, floor, &SELECTORS_FALLBACK), + Err("fee below minimum") + ); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Selector confusion +// +// The two operations share a head up to slot 6 but diverge past it. A call +// must be measured against the length of the operation it claims to be. +// ───────────────────────────────────────────────────────────────────────────── + +/// privateTransfer-length calldata (260) carrying the unshield selector must be +/// refused: unshield needs 324. Otherwise the shorter layout would be read +/// against the longer one's expectations. +#[test] +fn attack_unshield_selector_on_private_transfer_length_is_refused() { + let mut data = base_private_transfer(u128::MAX); + data[..4].copy_from_slice(&SELECTOR_UNSHIELD); + assert_eq!(data.len(), 260); + assert_eq!( + validate_relay_calldata(&data, 1, &SELECTORS_FALLBACK), + Err("calldata too short"), + "260 bytes is valid for privateTransfer but 64 short for unshield" + ); +} + +/// The fee slot must be read from the same offset regardless of which selector +/// is claimed — both layouts agree up to slot 6, and that agreement is what +/// makes a single `fee_at_slot_6` correct. +#[test] +fn attack_fee_slot_is_stable_across_both_selectors() { + let fee = 12_345_678u128; + let mut pt = base_private_transfer(fee); + let mut un = pt.clone(); + un.resize(324, 0); + un[..4].copy_from_slice(&SELECTOR_UNSHIELD); + pt[..4].copy_from_slice(&SELECTOR_PRIVATE_TRANSFER); + + // Both must accept at exactly `fee` and refuse at `fee + 1`. + for data in [&pt, &un] { + assert_eq!( + validate_relay_calldata(data, fee, &SELECTORS_FALLBACK), + Ok(()) + ); + assert_eq!( + validate_relay_calldata(data, fee + 1, &SELECTORS_FALLBACK), + Err("fee below minimum") + ); + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Size boundary +// ───────────────────────────────────────────────────────────────────────────── + +/// Calldata at exactly the cap is accepted, one byte over is refused, and +/// neither allocates proportionally to the claimed size. Pins `>` rather than +/// `>=` at the one boundary an attacker pays nothing to probe. +#[test] +fn attack_calldata_cap_boundary_is_exact() { + let mut at_cap = base_private_transfer(u128::MAX); + at_cap.resize(MAX_CALLDATA_BYTES, 0); + assert_eq!( + validate_relay_calldata(&at_cap, 1, &SELECTORS_FALLBACK), + Ok(()), + "exactly at the cap must be accepted" + ); + + let mut over = at_cap.clone(); + over.push(0); + assert_eq!( + validate_relay_calldata(&over, 1, &SELECTORS_FALLBACK), + Err("calldata too large") + ); +} + +/// The size check must come before any per-operation work: an oversized body +/// is refused for its size even when its selector is unknown, so a huge +/// unknown-selector call cannot be used to force extra scanning. +#[test] +fn attack_oversized_unknown_selector_is_refused_on_size_first() { + let mut data = vec![0xFFu8; MAX_CALLDATA_BYTES + 1]; + data[..4].copy_from_slice(&[0xDE, 0xAD, 0xBE, 0xEF]); + assert_eq!( + validate_relay_calldata(&data, 1, &SELECTORS_FALLBACK), + Err("calldata too large") + ); +} diff --git a/client/rpc/src/relay/tests/mod.rs b/client/rpc/src/relay/tests/mod.rs new file mode 100644 index 00000000..bde760af --- /dev/null +++ b/client/rpc/src/relay/tests/mod.rs @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +//! Tests for the relay's admission gate. +//! +//! Split by intent: [`validation`] covers the branches a well-formed request +//! takes, [`adversarial`] covers what a hostile caller can send over the +//! unauthenticated RPC and asserts the relay rejects rather than panics. + +mod adversarial; +mod validation; diff --git a/client/rpc/src/relay/tests/validation.rs b/client/rpc/src/relay/tests/validation.rs new file mode 100644 index 00000000..84242ada --- /dev/null +++ b/client/rpc/src/relay/tests/validation.rs @@ -0,0 +1,415 @@ +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +//! Unit tests for the pure validation functions — no runtime, no async. +//! +//! Covers every branch of `validate_relay_calldata` and every fee-floor case. + +use ethereum_types::U256; +use fp_evm::ExitReason; + +use crate::relay::{ + config::*, + operations::{SELECTOR_PRIVATE_TRANSFER, SELECTOR_UNSHIELD}, + validation::*, +}; + +/// Build minimal valid calldata for the given selector and fee. +/// +/// Head layout (first 228 bytes; the buffer is padded to the largest op's head): +/// ```text +/// [0..4] selector +/// [4..36] slot 0 — proof offset: 0xE0 (= 7×32 = 224, past all head slots) +/// [36..68] slot 1 — bytes32 zeroes (root) +/// [68..100] slot 2 — bytes32 zeroes (nullifier / nullifiers offset) +/// [100..132] slot 3 — uint32 zeroes (assetId / commitments offset) +/// [132..164] slot 4 — uint256 zeroes (amount / memos offset) +/// [164..196] slot 5 — bytes32 zeroes (recipient / assetId) +/// [196..228] slot 6 — uint256 fee +/// ``` +fn build_calldata(selector: [u8; 4], fee_wei: u128) -> Vec { + // unshield's head is 10 slots (4 + 320 = 324 bytes); privateTransfer's is + // 8 slots (260). Build the larger of the two — extra zero head slots are + // harmless, the fee stays at slot 6 either way. + let mut data = vec![0u8; 324]; + data[..4].copy_from_slice(&selector); + // Proof-bytes offset: 7×32 = 224 = 0xE0 (big-endian U256 → only last byte set) + data[35] = 0xE0; + // Fee at slot 6 = data[196..228] + data[196..228].copy_from_slice(&U256::from(fee_wei).to_big_endian()); + data +} + +// ── Length checks ────────────────────────────────────────────────────── + +#[test] +fn rejects_empty_calldata() { + assert_eq!( + validate_relay_calldata(&[], MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("calldata too short") + ); +} + +#[test] +fn rejects_calldata_227_bytes() { + assert_eq!( + validate_relay_calldata(&[0u8; 227], MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("calldata too short") + ); +} + +#[test] +fn rejects_calldata_196_bytes_old_wrong_limit() { + // Ensure the old (incorrect) limit of 196 is no longer accepted + let data = vec![0u8; 196]; + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("calldata too short") + ); +} + +// ── Selector checks ──────────────────────────────────────────────────── + +/// The whitelist re-exports the decoder's constants, so the two cannot +/// disagree. What still needs proving is that those constants are keccak of +/// the signatures the client encodes against — a decoder renamed in lockstep +/// with this file would satisfy equality while rejecting every real call. +#[test] +fn selectors_are_keccak_of_the_abi_signatures() { + let pt = sp_core::hashing::keccak_256( + b"privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)", + ); + let un = sp_core::hashing::keccak_256( + b"unshield(bytes,bytes32,bytes32,uint32,uint256,bytes32,uint256,bytes32,bytes,uint32)", + ); + assert_eq!(pt[..4], SELECTOR_PRIVATE_TRANSFER); + assert_eq!(un[..4], SELECTOR_UNSHIELD); +} + +/// The runtime fallback list must agree with the client fallback list. +#[test] +fn fallback_selectors_contain_both_operations() { + assert!(SELECTORS_FALLBACK.contains(&SELECTOR_UNSHIELD)); + assert!(SELECTORS_FALLBACK.contains(&SELECTOR_PRIVATE_TRANSFER)); +} + +/// privateTransfer calldata shorter than its 8-slot head (260 bytes) is +/// rejected even though it clears the global 228-byte minimum. +#[test] +fn rejects_private_transfer_calldata_between_228_and_260() { + let mut data = build_calldata(SELECTOR_PRIVATE_TRANSFER, MIN_RELAY_FEE_FALLBACK); + data.truncate(259); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("calldata too short") + ); +} + +#[test] +fn rejects_unknown_selector() { + let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); + data[..4].copy_from_slice(&[0xde, 0xad, 0xbe, 0xef]); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("unsupported selector") + ); +} + +#[test] +fn rejects_shield_selector() { + // shield = 0x781442b9 — NOT in relay whitelist + let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); + data[..4].copy_from_slice(&[0x78, 0x14, 0x42, 0xb9]); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("unsupported selector") + ); +} + +// ── Fee checks ───────────────────────────────────────────────────────── + +#[test] +fn rejects_zero_fee() { + let data = build_calldata(SELECTOR_UNSHIELD, 0); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("fee below minimum") + ); +} + +#[test] +fn rejects_fee_one_wei_below_minimum() { + let data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK - 1); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("fee below minimum") + ); +} + +#[test] +fn rejects_fee_one_wei_below_minimum_private_transfer() { + let data = build_calldata(SELECTOR_PRIVATE_TRANSFER, MIN_RELAY_FEE_FALLBACK - 1); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("fee below minimum") + ); +} + +// ── Valid calldata ───────────────────────────────────────────────────── + +#[test] +fn accepts_unshield_with_exact_minimum_fee() { + let data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Ok(()) + ); +} + +#[test] +fn accepts_private_transfer_with_exact_minimum_fee() { + let data = build_calldata(SELECTOR_PRIVATE_TRANSFER, MIN_RELAY_FEE_FALLBACK); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Ok(()) + ); +} + +#[test] +fn accepts_large_fee() { + let data = build_calldata(SELECTOR_UNSHIELD, u128::MAX); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Ok(()) + ); +} + +#[test] +fn accepts_calldata_longer_than_228_bytes() { + let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); + // Append tail bytes (proof data and dynamic arrays) + data.extend_from_slice(&[0xaa; 128]); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Ok(()) + ); +} + +// ── Fee is read from the correct position ────────────────────────────── + +#[test] +fn fee_at_slot_5_is_not_read_as_fee() { + // Put a value >= MIN_RELAY_FEE_FALLBACK in slot 5 (data[164..196]) but zero in slot 6 + let mut data = build_calldata(SELECTOR_UNSHIELD, 0); + // Overwrite slot 5 with MIN_RELAY_FEE_FALLBACK (this is recipient in unshield — NOT the fee) + data[164..196].copy_from_slice(&U256::from(MIN_RELAY_FEE_FALLBACK).to_big_endian()); + // Fee (slot 6, data[196..228]) is still zero → should reject + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("fee below minimum") + ); +} + +#[test] +fn fee_at_slot_6_is_correctly_read() { + // Slot 5 = zero, slot 6 = MIN_RELAY_FEE_FALLBACK → should accept + let data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Ok(()) + ); +} + +// ── Calldata size upper bound ────────────────────────────────────────── + +#[test] +fn rejects_calldata_above_max_size() { + let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); + data.resize(MAX_CALLDATA_BYTES + 1, 0u8); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Err("calldata too large") + ); +} + +#[test] +fn accepts_calldata_at_exact_max_size() { + let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); + data.resize(MAX_CALLDATA_BYTES, 0u8); + assert_eq!( + validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), + Ok(()) + ); +} + +// ── compute_effective_min_fee ────────────────────────────────────────── + +/// At zero base_fee the governance floor must win. +#[test] +fn effective_min_fee_zero_base_fee_returns_governance_floor() { + assert_eq!( + compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, 0), + MIN_RELAY_FEE_FALLBACK + ); +} + +/// At 10 gwei the 2× gas floor (40_000_000_000_000_000) must beat governance (0.001 ORB). +#[test] +fn effective_min_fee_gas_floor_dominates_at_10_gwei() { + let base_fee = 10_000_000_000u128; // 10 gwei + let expected = (RELAY_GAS_LIMIT as u128) * 2 * base_fee; + assert!( + expected > MIN_RELAY_FEE_FALLBACK, + "test precondition: gas floor must exceed governance" + ); + assert_eq!( + compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, base_fee), + expected + ); +} + +/// At 1 wei base_fee the gas floor (4_000_000) is far below governance (0.001 ORB). +#[test] +fn effective_min_fee_governance_floor_dominates_at_negligible_base_fee() { + let base_fee = 1u128; + let gas_floor = (RELAY_GAS_LIMIT as u128) * 2 * base_fee; + assert!( + MIN_RELAY_FEE_FALLBACK > gas_floor, + "test precondition: governance must exceed gas floor" + ); + assert_eq!( + compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, base_fee), + MIN_RELAY_FEE_FALLBACK + ); +} + +/// The crossover point is at base_fee = governance / (gas_limit × 2) = 250_000_000 (0.25 gwei). +/// Below this threshold governance wins; at or above, the gas floor wins. +#[test] +fn effective_min_fee_crossover_at_250_mwei() { + // threshold = 1_000_000_000_000_000 / (2_000_000 × 2) = 250_000_000 + let threshold = MIN_RELAY_FEE_FALLBACK / (RELAY_GAS_LIMIT as u128 * 2); + // At threshold: 2×gas == governance, max returns governance. + let at = compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, threshold); + // One wei above: 2×gas > governance. + let above = compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, threshold + 1); + assert_eq!(at, MIN_RELAY_FEE_FALLBACK); + assert!(above > MIN_RELAY_FEE_FALLBACK); +} + +/// A governance floor higher than the gas floor must win regardless of base_fee. +#[test] +fn effective_min_fee_custom_high_governance_beats_gas_floor() { + let governance = 100_000_000_000_000_000u128; // 0.1 ORB + let base_fee = 1_000_000_000u128; // 1 gwei + let gas_floor = (RELAY_GAS_LIMIT as u128) * 2 * base_fee; + assert!( + governance > gas_floor, + "test precondition: governance must exceed gas floor" + ); + assert_eq!(compute_effective_min_fee(governance, base_fee), governance); +} + +/// Saturating arithmetic must not panic on u128::MAX inputs. +#[test] +fn effective_min_fee_saturates_without_panic() { + let result = compute_effective_min_fee(u128::MAX, u128::MAX); + assert_eq!(result, u128::MAX); +} + +/// The calldata fee must be validated against `effective_min_fee`, not raw `min_fee_planck`. +/// Simulates the scenario where the gas floor is the active minimum. +#[test] +fn validate_calldata_rejects_fee_below_gas_floor_even_above_governance() { + let governance = MIN_RELAY_FEE_FALLBACK; + let base_fee = 10_000_000_000u128; // 10 gwei + let gas_floor = (RELAY_GAS_LIMIT as u128) * 2 * base_fee; // 40_000_000_000_000_000 + assert!(gas_floor > governance); + let effective = compute_effective_min_fee(governance, base_fee); + // A fee that clears governance but not the gas floor is rejected. + let below_gas_floor = gas_floor - 1; + let data = build_calldata(SELECTOR_UNSHIELD, below_gas_floor); + assert_eq!( + validate_relay_calldata(&data, effective, &SELECTORS_FALLBACK), + Err("fee below minimum") + ); +} + +/// Fee that exactly equals the gas floor (when it dominates) must be accepted. +#[test] +fn validate_calldata_accepts_exact_gas_floor() { + let governance = MIN_RELAY_FEE_FALLBACK; + let base_fee = 10_000_000_000u128; // 10 gwei + let effective = compute_effective_min_fee(governance, base_fee); + let data = build_calldata(SELECTOR_UNSHIELD, effective); + assert_eq!( + validate_relay_calldata(&data, effective, &SELECTORS_FALLBACK), + Ok(()) + ); +} + +// ── check_dry_run_exit ───────────────────────────────────────────────── + +/// A successful EVM execution must pass the dry-run check. +#[test] +fn dry_run_exit_succeed_returns_ok() { + use evm::ExitSucceed; + assert!(check_dry_run_exit(&ExitReason::Succeed(ExitSucceed::Returned)).is_ok()); + assert!(check_dry_run_exit(&ExitReason::Succeed(ExitSucceed::Stopped)).is_ok()); + assert!(check_dry_run_exit(&ExitReason::Succeed(ExitSucceed::Suicided)).is_ok()); +} + +/// A revert (invalid ZK proof, double-spend nullifier, etc.) must be rejected. +#[test] +fn dry_run_exit_revert_returns_err() { + use evm::ExitRevert; + let result = check_dry_run_exit(&ExitReason::Revert(ExitRevert::Reverted)); + assert!(result.is_err()); + let msg = result.unwrap_err(); + assert!( + msg.contains("calldata would fail on-chain"), + "expected 'calldata would fail on-chain' in: {msg}" + ); + assert!(msg.contains("Revert"), "expected 'Revert' in: {msg}"); +} + +/// An EVM error (OutOfGas, etc.) must be rejected. +#[test] +fn dry_run_exit_error_out_of_gas_returns_err() { + use evm::ExitError; + let result = check_dry_run_exit(&ExitReason::Error(ExitError::OutOfGas)); + assert!(result.is_err()); + let msg = result.unwrap_err(); + assert!(msg.contains("calldata would fail on-chain"), "{msg}"); + assert!(msg.contains("OutOfGas"), "expected 'OutOfGas' in: {msg}"); +} + +/// A call-too-deep EVM error (stack overflow) must be rejected. +#[test] +fn dry_run_exit_error_call_too_deep_returns_err() { + use evm::ExitError; + let result = check_dry_run_exit(&ExitReason::Error(ExitError::CallTooDeep)); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("CallTooDeep")); +} + +/// A fatal EVM error must be rejected. +#[test] +fn dry_run_exit_fatal_returns_err() { + use evm::ExitFatal; + let result = check_dry_run_exit(&ExitReason::Fatal(ExitFatal::NotSupported)); + assert!(result.is_err()); + let msg = result.unwrap_err(); + assert!(msg.contains("calldata would fail on-chain"), "{msg}"); + assert!(msg.contains("Fatal"), "expected 'Fatal' in: {msg}"); +} + +/// The error message must embed the exit reason so callers can surface it to users. +#[test] +fn dry_run_exit_error_message_includes_reason() { + use evm::ExitError; + let reason = ExitReason::Error(ExitError::OutOfFund); + let err = check_dry_run_exit(&reason).unwrap_err(); + // The message should contain both the prefix and the specific variant. + assert!(err.starts_with("calldata would fail on-chain:"), "{err}"); + assert!(err.contains("OutOfFund"), "{err}"); +} diff --git a/client/rpc/src/relay/validation.rs b/client/rpc/src/relay/validation.rs index 3793af5b..b1f09d16 100644 --- a/client/rpc/src/relay/validation.rs +++ b/client/rpc/src/relay/validation.rs @@ -1,58 +1,21 @@ // SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 -//! Pure calldata validation — no runtime or async dependencies. +//! Pure calldata validation — no runtime, no async, no I/O. //! -//! All functions here are synchronous and fully unit-testable without spinning up -//! a Substrate node. The tests at the bottom of this file cover every validation -//! branch and fee-floor scenario. +//! This is the relay's admission gate: everything it can decide about a request +//! by looking at the bytes alone. Anything needing chain state (the dry run, the +//! nonce, the signature) lives in [`super::rpc`], and the limits these functions +//! enforce live in [`super::config`]. +//! +//! Being free of those dependencies is what makes the hostile cases in +//! `tests/adversarial.rs` cheap to write: no node, no async runtime, just bytes. use fp_evm::ExitReason; -use super::operations::default_operations; - -// --------------------------------------------------------------------------- -// Constants -// --------------------------------------------------------------------------- - -/// Maximum fee per gas paid by the relay tx (10 gwei). -/// Used when building the EIP-1559 transaction. -pub(crate) const MAX_FEE_PER_GAS_WEI: u64 = 10_000_000_000; - -/// Gas limit used for relay transactions. -pub(crate) const RELAY_GAS_LIMIT: u64 = 2_000_000; - -/// Last-resort fallback minimum fee used ONLY when the Runtime API call fails entirely. -/// -/// The authoritative value lives in `pallet-relayer::MinRelayFee` storage and is -/// modifiable by governance via `set_min_relay_fee`. This constant is never used in -/// normal operation — it only kicks in if the node runs a pre-API runtime that does -/// not expose `relay_config()`. -/// -/// Set to the same default as `pallet-relayer::DefaultMinRelayFee` (0.001 ORB) so all -/// three sources are identical out of the box. -pub(crate) const MIN_RELAY_FEE_FALLBACK: u128 = 1_000_000_000_000_000; // 0.001 ORB in planck - -/// Static fallback selector whitelist for when the Runtime API is unavailable. -pub(crate) const SELECTORS_FALLBACK: [[u8; 4]; 2] = [ - [0x47, 0xfc, 0x44, 0xa2], // unshield - [0x8c, 0x0f, 0x5d, 0x24], // privateTransfer -]; - -/// Maximum calldata size accepted by the relay (32 KB). -/// -/// A realistic shielded-pool calldata is ~2–5 KB (256 B Groth16 proof + ABI head + Merkle path). -/// This cap prevents DoS: an attacker could craft calldata that passes selector/fee checks -/// but carries megabytes of data the relayer would have to pay calldata gas for. -pub(crate) const MAX_CALLDATA_BYTES: usize = 32_768; - -/// ShieldedPool precompile: 0x0000000000000000000000000000000000000801 -pub(crate) const SHIELDED_POOL_PRECOMPILE: [u8; 20] = [ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x08, 0x01, -]; - -// --------------------------------------------------------------------------- -// Pure validation functions -// --------------------------------------------------------------------------- +use super::{ + config::{MAX_CALLDATA_BYTES, RELAY_GAS_LIMIT}, + operations::default_operations, +}; /// Computes the effective minimum fee the user must include in their calldata. /// @@ -75,7 +38,8 @@ pub(crate) fn compute_effective_min_fee(min_fee_planck: u128, base_fee_wei: u128 /// (or `MIN_RELAY_FEE_FALLBACK` if the API is unavailable). /// `allowed_selectors` — from `relay_config().allowed_selectors`. /// -/// Both `unshield` and `privateTransfer` share the same ABI head layout: +/// Both `unshield` and `privateTransfer` agree up to slot 6, which is all this +/// function reads: /// ```text /// bytes [0..4] selector /// bytes [4..36] slot 0 — offset pointer for proof (bytes/dynamic) @@ -86,7 +50,9 @@ pub(crate) fn compute_effective_min_fee(min_fee_planck: u128, base_fee_wei: u128 /// bytes [164..196] slot 5 — bytes32 recipient / uint32 asset_id /// bytes [196..228] slot 6 — uint256 fee ← checked here /// ``` -/// Minimum head size = 4 + 7 × 32 = 228 bytes. +/// Past slot 6 the layouts diverge, so the 228-byte minimum above is only a +/// cheap first gate: each operation declares its own `min_calldata_len()` +/// (unshield 324, privateTransfer 260), checked after selector dispatch. pub(crate) fn validate_relay_calldata( data: &[u8], min_fee_wei: u128, @@ -138,378 +104,3 @@ pub(crate) fn check_dry_run_exit(exit_reason: &ExitReason) -> Result<(), String> other => Err(format!("calldata would fail on-chain: {other:?}")), } } - -// --------------------------------------------------------------------------- -// Unit tests — pure validation logic, no runtime required -// --------------------------------------------------------------------------- - -#[cfg(test)] -mod tests { - use ethereum_types::U256; - - use super::super::operations::{SELECTOR_PRIVATE_TRANSFER, SELECTOR_UNSHIELD}; - use super::*; - - /// Build minimal valid calldata for the given selector and fee. - /// - /// Head layout (228 bytes total): - /// ```text - /// [0..4] selector - /// [4..36] slot 0 — proof offset: 0xE0 (= 7×32 = 224, past all head slots) - /// [36..68] slot 1 — bytes32 zeroes (root) - /// [68..100] slot 2 — bytes32 zeroes (nullifier / nullifiers offset) - /// [100..132] slot 3 — uint32 zeroes (assetId / commitments offset) - /// [132..164] slot 4 — uint256 zeroes (amount / memos offset) - /// [164..196] slot 5 — bytes32 zeroes (recipient / assetId) - /// [196..228] slot 6 — uint256 fee - /// ``` - fn build_calldata(selector: [u8; 4], fee_wei: u128) -> Vec { - let mut data = vec![0u8; 228]; - data[..4].copy_from_slice(&selector); - // Proof-bytes offset: 7×32 = 224 = 0xE0 (big-endian U256 → only last byte set) - data[35] = 0xE0; - // Fee at slot 6 = data[196..228] - data[196..228].copy_from_slice(&U256::from(fee_wei).to_big_endian()); - data - } - - // ── Length checks ────────────────────────────────────────────────────── - - #[test] - fn rejects_empty_calldata() { - assert_eq!( - validate_relay_calldata(&[], MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("calldata too short") - ); - } - - #[test] - fn rejects_calldata_227_bytes() { - assert_eq!( - validate_relay_calldata(&[0u8; 227], MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("calldata too short") - ); - } - - #[test] - fn rejects_calldata_196_bytes_old_wrong_limit() { - // Ensure the old (incorrect) limit of 196 is no longer accepted - let data = vec![0u8; 196]; - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("calldata too short") - ); - } - - // ── Selector checks ──────────────────────────────────────────────────── - - #[test] - fn rejects_unknown_selector() { - let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); - data[..4].copy_from_slice(&[0xde, 0xad, 0xbe, 0xef]); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("unsupported selector") - ); - } - - #[test] - fn rejects_shield_selector() { - // shield = 0x781442b9 — NOT in relay whitelist - let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); - data[..4].copy_from_slice(&[0x78, 0x14, 0x42, 0xb9]); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("unsupported selector") - ); - } - - // ── Fee checks ───────────────────────────────────────────────────────── - - #[test] - fn rejects_zero_fee() { - let data = build_calldata(SELECTOR_UNSHIELD, 0); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("fee below minimum") - ); - } - - #[test] - fn rejects_fee_one_wei_below_minimum() { - let data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK - 1); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("fee below minimum") - ); - } - - #[test] - fn rejects_fee_one_wei_below_minimum_private_transfer() { - let data = build_calldata(SELECTOR_PRIVATE_TRANSFER, MIN_RELAY_FEE_FALLBACK - 1); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("fee below minimum") - ); - } - - // ── Valid calldata ───────────────────────────────────────────────────── - - #[test] - fn accepts_unshield_with_exact_minimum_fee() { - let data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Ok(()) - ); - } - - #[test] - fn accepts_private_transfer_with_exact_minimum_fee() { - let data = build_calldata(SELECTOR_PRIVATE_TRANSFER, MIN_RELAY_FEE_FALLBACK); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Ok(()) - ); - } - - #[test] - fn accepts_large_fee() { - let data = build_calldata(SELECTOR_UNSHIELD, u128::MAX); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Ok(()) - ); - } - - #[test] - fn accepts_calldata_longer_than_228_bytes() { - let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); - // Append tail bytes (proof data and dynamic arrays) - data.extend_from_slice(&[0xaa; 128]); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Ok(()) - ); - } - - // ── Fee is read from the correct position ────────────────────────────── - - #[test] - fn fee_at_slot_5_is_not_read_as_fee() { - // Put a value >= MIN_RELAY_FEE_FALLBACK in slot 5 (data[164..196]) but zero in slot 6 - let mut data = build_calldata(SELECTOR_UNSHIELD, 0); - // Overwrite slot 5 with MIN_RELAY_FEE_FALLBACK (this is recipient in unshield — NOT the fee) - data[164..196].copy_from_slice(&U256::from(MIN_RELAY_FEE_FALLBACK).to_big_endian()); - // Fee (slot 6, data[196..228]) is still zero → should reject - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("fee below minimum") - ); - } - - #[test] - fn fee_at_slot_6_is_correctly_read() { - // Slot 5 = zero, slot 6 = MIN_RELAY_FEE_FALLBACK → should accept - let data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Ok(()) - ); - } - - // ── Calldata size upper bound ────────────────────────────────────────── - - #[test] - fn rejects_calldata_above_max_size() { - let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); - data.resize(MAX_CALLDATA_BYTES + 1, 0u8); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Err("calldata too large") - ); - } - - #[test] - fn accepts_calldata_at_exact_max_size() { - let mut data = build_calldata(SELECTOR_UNSHIELD, MIN_RELAY_FEE_FALLBACK); - data.resize(MAX_CALLDATA_BYTES, 0u8); - assert_eq!( - validate_relay_calldata(&data, MIN_RELAY_FEE_FALLBACK, &SELECTORS_FALLBACK), - Ok(()) - ); - } - - // ── compute_effective_min_fee ────────────────────────────────────────── - - /// At zero base_fee the governance floor must win. - #[test] - fn effective_min_fee_zero_base_fee_returns_governance_floor() { - assert_eq!( - compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, 0), - MIN_RELAY_FEE_FALLBACK - ); - } - - /// At 10 gwei the 2× gas floor (40_000_000_000_000_000) must beat governance (0.001 ORB). - #[test] - fn effective_min_fee_gas_floor_dominates_at_10_gwei() { - let base_fee = 10_000_000_000u128; // 10 gwei - let expected = (RELAY_GAS_LIMIT as u128) * 2 * base_fee; - assert!( - expected > MIN_RELAY_FEE_FALLBACK, - "test precondition: gas floor must exceed governance" - ); - assert_eq!( - compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, base_fee), - expected - ); - } - - /// At 1 wei base_fee the gas floor (4_000_000) is far below governance (0.001 ORB). - #[test] - fn effective_min_fee_governance_floor_dominates_at_negligible_base_fee() { - let base_fee = 1u128; - let gas_floor = (RELAY_GAS_LIMIT as u128) * 2 * base_fee; - assert!( - MIN_RELAY_FEE_FALLBACK > gas_floor, - "test precondition: governance must exceed gas floor" - ); - assert_eq!( - compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, base_fee), - MIN_RELAY_FEE_FALLBACK - ); - } - - /// The crossover point is at base_fee = governance / (gas_limit × 2) = 250_000_000 (0.25 gwei). - /// Below this threshold governance wins; at or above, the gas floor wins. - #[test] - fn effective_min_fee_crossover_at_250_mwei() { - // threshold = 1_000_000_000_000_000 / (2_000_000 × 2) = 250_000_000 - let threshold = MIN_RELAY_FEE_FALLBACK / (RELAY_GAS_LIMIT as u128 * 2); - // At threshold: 2×gas == governance, max returns governance. - let at = compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, threshold); - // One wei above: 2×gas > governance. - let above = compute_effective_min_fee(MIN_RELAY_FEE_FALLBACK, threshold + 1); - assert_eq!(at, MIN_RELAY_FEE_FALLBACK); - assert!(above > MIN_RELAY_FEE_FALLBACK); - } - - /// A governance floor higher than the gas floor must win regardless of base_fee. - #[test] - fn effective_min_fee_custom_high_governance_beats_gas_floor() { - let governance = 100_000_000_000_000_000u128; // 0.1 ORB - let base_fee = 1_000_000_000u128; // 1 gwei - let gas_floor = (RELAY_GAS_LIMIT as u128) * 2 * base_fee; - assert!( - governance > gas_floor, - "test precondition: governance must exceed gas floor" - ); - assert_eq!(compute_effective_min_fee(governance, base_fee), governance); - } - - /// Saturating arithmetic must not panic on u128::MAX inputs. - #[test] - fn effective_min_fee_saturates_without_panic() { - let result = compute_effective_min_fee(u128::MAX, u128::MAX); - assert_eq!(result, u128::MAX); - } - - /// The calldata fee must be validated against `effective_min_fee`, not raw `min_fee_planck`. - /// Simulates the scenario where the gas floor is the active minimum. - #[test] - fn validate_calldata_rejects_fee_below_gas_floor_even_above_governance() { - let governance = MIN_RELAY_FEE_FALLBACK; - let base_fee = 10_000_000_000u128; // 10 gwei - let gas_floor = (RELAY_GAS_LIMIT as u128) * 2 * base_fee; // 40_000_000_000_000_000 - assert!(gas_floor > governance); - let effective = compute_effective_min_fee(governance, base_fee); - // A fee that clears governance but not the gas floor is rejected. - let below_gas_floor = gas_floor - 1; - let data = build_calldata(SELECTOR_UNSHIELD, below_gas_floor); - assert_eq!( - validate_relay_calldata(&data, effective, &SELECTORS_FALLBACK), - Err("fee below minimum") - ); - } - - /// Fee that exactly equals the gas floor (when it dominates) must be accepted. - #[test] - fn validate_calldata_accepts_exact_gas_floor() { - let governance = MIN_RELAY_FEE_FALLBACK; - let base_fee = 10_000_000_000u128; // 10 gwei - let effective = compute_effective_min_fee(governance, base_fee); - let data = build_calldata(SELECTOR_UNSHIELD, effective); - assert_eq!( - validate_relay_calldata(&data, effective, &SELECTORS_FALLBACK), - Ok(()) - ); - } - - // ── check_dry_run_exit ───────────────────────────────────────────────── - - /// A successful EVM execution must pass the dry-run check. - #[test] - fn dry_run_exit_succeed_returns_ok() { - use evm::ExitSucceed; - assert!(check_dry_run_exit(&ExitReason::Succeed(ExitSucceed::Returned)).is_ok()); - assert!(check_dry_run_exit(&ExitReason::Succeed(ExitSucceed::Stopped)).is_ok()); - assert!(check_dry_run_exit(&ExitReason::Succeed(ExitSucceed::Suicided)).is_ok()); - } - - /// A revert (invalid ZK proof, double-spend nullifier, etc.) must be rejected. - #[test] - fn dry_run_exit_revert_returns_err() { - use evm::ExitRevert; - let result = check_dry_run_exit(&ExitReason::Revert(ExitRevert::Reverted)); - assert!(result.is_err()); - let msg = result.unwrap_err(); - assert!( - msg.contains("calldata would fail on-chain"), - "expected 'calldata would fail on-chain' in: {msg}" - ); - assert!(msg.contains("Revert"), "expected 'Revert' in: {msg}"); - } - - /// An EVM error (OutOfGas, etc.) must be rejected. - #[test] - fn dry_run_exit_error_out_of_gas_returns_err() { - use evm::ExitError; - let result = check_dry_run_exit(&ExitReason::Error(ExitError::OutOfGas)); - assert!(result.is_err()); - let msg = result.unwrap_err(); - assert!(msg.contains("calldata would fail on-chain"), "{msg}"); - assert!(msg.contains("OutOfGas"), "expected 'OutOfGas' in: {msg}"); - } - - /// A call-too-deep EVM error (stack overflow) must be rejected. - #[test] - fn dry_run_exit_error_call_too_deep_returns_err() { - use evm::ExitError; - let result = check_dry_run_exit(&ExitReason::Error(ExitError::CallTooDeep)); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("CallTooDeep")); - } - - /// A fatal EVM error must be rejected. - #[test] - fn dry_run_exit_fatal_returns_err() { - use evm::ExitFatal; - let result = check_dry_run_exit(&ExitReason::Fatal(ExitFatal::NotSupported)); - assert!(result.is_err()); - let msg = result.unwrap_err(); - assert!(msg.contains("calldata would fail on-chain"), "{msg}"); - assert!(msg.contains("Fatal"), "expected 'Fatal' in: {msg}"); - } - - /// The error message must embed the exit reason so callers can surface it to users. - #[test] - fn dry_run_exit_error_message_includes_reason() { - use evm::ExitError; - let reason = ExitReason::Error(ExitError::OutOfFund); - let err = check_dry_run_exit(&reason).unwrap_err(); - // The message should contain both the prefix and the specific variant. - assert!(err.starts_with("calldata would fail on-chain:"), "{err}"); - assert!(err.contains("OutOfFund"), "{err}"); - } -} diff --git a/frame/evm/precompile/shielded-pool/CHANGELOG.md b/frame/evm/precompile/shielded-pool/CHANGELOG.md index 055c8f4e..3bbe65d5 100644 --- a/frame/evm/precompile/shielded-pool/CHANGELOG.md +++ b/frame/evm/precompile/shielded-pool/CHANGELOG.md @@ -2,6 +2,33 @@ All notable changes to `pallet-evm-precompile-shielded-pool` will be documented in this file. +## [0.5.1] - 2026-08-11 + +### Added + +- **Selectors are exported** (`selectors::{SHIELD, PRIVATE_TRANSFER, UNSHIELD, + CLAIM_SHIELDED_FEES}`) so the relay whitelist can be pinned against the + decoder's own constants in a test rather than kept in sync by hand. That copy + drifting is the ME-8 class of bug, and it fails silently: a wrong selector is + merely "unsupported", so the rejection tests stay green while the accept path + quietly stops working. + +### Fixed + +- **`private_transfer_rejects_truncated_input` used a stale selector** + (`0x8c0f5d24`, from a signature two versions old). It passed only because a + wrong selector is rejected anyway — it was testing nothing about truncation. + Now taken from the decoder constant, with `private_transfer_selector_matches_signature` + and `unshield_selector_matches_signature` deriving both from keccak of the ABI + signature so neither can drift again. + +### Tests + +- Nine adversarial decoder tests: truncation at every offset, self-referential + and `u256::MAX` offsets, offsets above `u32` that look benign, huge array + counts that must not allocate, oversized `u32` slots, a maximal fee word, and + two fuzz sweeps (random bytes and random truncations) that must never panic. + ## [0.5.0] - 2026-08-07 ### Security diff --git a/frame/evm/precompile/shielded-pool/Cargo.toml b/frame/evm/precompile/shielded-pool/Cargo.toml index c589b199..4fa4e6bb 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.5.0" +version = "0.5.1" authors = { workspace = true } edition = "2021" description = "EVM Precompile for Orbinum Shielded Pool Pallet." diff --git a/frame/evm/precompile/shielded-pool/src/calls/claim_shielded_fees.rs b/frame/evm/precompile/shielded-pool/src/calls/claim_shielded_fees.rs index 52e6882e..b65ced75 100644 --- a/frame/evm/precompile/shielded-pool/src/calls/claim_shielded_fees.rs +++ b/frame/evm/precompile/shielded-pool/src/calls/claim_shielded_fees.rs @@ -6,23 +6,23 @@ //! = `0x88d9deba` //! //! ## ABI layout (`input[4..]`) -//! | Slot (bytes) | Type | Field | -//! |-------------|-----------|--------------------| -//! | 0..32 | `bytes32` | `commitment` | -//! | 32..64 | `uint256` | `amount` | -//! | 64..96 | `uint32` | `asset_id` | -//! | 96..128 | `uint256` | offset → `memo` | -//! | 128..160 | `uint256` | offset → `proof` | -//! | 160..192 | `uint256` | offset → `public_signals` | -//! | 192..224 | `uint32` | `circuit_version` | +//! | Slot (bytes) | Type | Field | +//! |--------------|-----------|---------------------------| +//! | 0..32 | `bytes32` | `commitment` | +//! | 32..64 | `uint256` | `amount` | +//! | 64..96 | `uint32` | `asset_id` | +//! | 96..128 | `uint256` | offset → `memo` | +//! | 128..160 | `uint256` | offset → `proof` | +//! | 160..192 | `uint256` | offset → `public_signals` | +//! | 192..224 | `uint32` | `circuit_version` | //! -//! The **validator** origin is derived from `handle.context().caller` -//! (the EVM address that sent the transaction), mapped to an `AccountId` -//! via `AddressMapping`. It must match the address registered in -//! `pallet-relayer` that has accumulated pending fees. +//! ## Notes +//! `public_signals` is a fixed 76-byte blob encoded off-chain: +//! `commitment[0..32] | value[32..40] | asset_id[40..44] | owner_hash[44..76]`. //! -//! ## public_signals layout (76 bytes, off-chain encoded) -//! `commitment[0..32] | value[32..40] | asset_id[40..44] | owner_hash[44..76]` +//! The validator is not in the ABI: it comes from `handle.context().caller`, +//! mapped to an `AccountId` via `AddressMapping`. It must match the address +//! registered in `pallet-relayer` that accumulated the pending fees. use alloc::vec::Vec; @@ -31,20 +31,16 @@ use sp_core::U256; use crate::abi; -/// `keccak256("claimShieldedFees(bytes32,uint256,uint32,bytes,bytes,bytes,uint32)")[0..4]` -/// The trailing `uint32` is `circuitVersion` — the circuit version the spent -/// notes were created under, so the proof is verified against that version's VK. +/// Selector for the signature in this module's header. pub const SELECTOR: [u8; 4] = [0x88, 0xd9, 0xde, 0xba]; /// Maximum byte length of a serialised Groth16 proof accepted by the pallet. const MAX_PROOF_LEN: u32 = 512; -/// Decodes the ABI-encoded `input` and returns a ready-to-dispatch -/// `claim_shielded_fees` call. +/// Decodes `input` into a ready-to-dispatch `claim_shielded_fees` call. /// -/// The validator `AccountId` is NOT part of the ABI — it is derived from -/// `handle.context().caller` so the pallet can look up the correct pending -/// fee balance in `pallet-relayer`. +/// The handle is unused: unlike the other calls, the validator origin is mapped +/// from the caller by the dispatch layer rather than read here. pub fn decode( _handle: &impl PrecompileHandle, input: &[u8], @@ -53,15 +49,19 @@ where T: pallet_shielded_pool::Config, pallet_shielded_pool::BalanceOf: TryFrom, { - // Minimum head section: 6 fixed slots × 32 bytes = 192 bytes. let params = &input[4..]; + + // Step 1: require the first six head slots. `circuit_version` is the seventh + // and is checked in step 7, so calldata predating it reports the missing field + // rather than a bad length. if params.len() < 192 { return Err(err("claimShieldedFees: input too short")); } + // Step 2: commitment of the note the claimed fees are paid into. let commitment = pallet_shielded_pool::Commitment::from(abi::read_bytes32(params, 0)?); - // Reject zero-amount calls early. + // Step 3: amount. Zero would mint a worthless note while consuming the proof. let amount_u256 = U256::from_big_endian(¶ms[32..64]); if amount_u256.is_zero() { return Err(err("claimShieldedFees: amount must be non-zero")); @@ -74,12 +74,16 @@ where .map_err(|_| err("claimShieldedFees: amount conversion failed"))? }; + // Step 4: asset_id. let asset_id = abi::decode_u32(¶ms[64..96])?; + // Step 5: memo — dynamic, offset at slot 96. It carries the only copy of the + // new note's secrets. let memo_bytes: Vec = abi::decode_bytes_at_slot(params, 96)?; let memo = pallet_shielded_pool::FrameEncryptedMemo::new(memo_bytes) .map_err(|_| err("claimShieldedFees: memo too long or wrong size"))?; + // Step 6: proof — dynamic, offset at slot 128. let proof: frame_support::BoundedVec> = abi::decode_bytes_at_slot(params, 128)? .try_into() @@ -89,6 +93,9 @@ where return Err(err("claimShieldedFees: proof must be non-empty")); } + // Step 7: public_signals — dynamic, offset at slot 160. Exactly 76 bytes, the + // fixed layout the circuit was compiled against; any other length cannot be a + // valid public input, so it is rejected before the pallet verifies the proof. let public_signals_raw: Vec = abi::decode_bytes_at_slot(params, 160)?; if public_signals_raw.len() != 76 { @@ -98,6 +105,7 @@ where .try_into() .map_err(|_| err("claimShieldedFees: public_signals too long"))?; + // Step 8: circuit_version, selecting the VK the proof is checked against. if params.len() < 224 { return Err(err( "claimShieldedFees: input too short (missing circuitVersion)", diff --git a/frame/evm/precompile/shielded-pool/src/calls/mod.rs b/frame/evm/precompile/shielded-pool/src/calls/mod.rs index 4cc153aa..8c3a09ee 100644 --- a/frame/evm/precompile/shielded-pool/src/calls/mod.rs +++ b/frame/evm/precompile/shielded-pool/src/calls/mod.rs @@ -7,6 +7,12 @@ //! //! The precompile router in `lib.rs` only needs to match on `SELECTOR`s and //! forward to the appropriate `decode`, then hand the call to `dispatch`. +//! +//! Each module header documents the selector and the ABI slot layout; `decode` +//! walks that layout in numbered steps. Decoding is the trust boundary, so a +//! `decode` also rejects input that is well-formed but irrecoverable — a zero +//! amount, a burn address, a memo without which a note could never be spent — +//! rather than leaving it to the pallet. pub mod claim_shielded_fees; pub mod private_transfer; 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 5a1862fd..213f70da 100644 --- a/frame/evm/precompile/shielded-pool/src/calls/private_transfer.rs +++ b/frame/evm/precompile/shielded-pool/src/calls/private_transfer.rs @@ -5,19 +5,23 @@ //! `keccak256("privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)")[0..4]` //! = `0x66ed2cd4` //! -//! ## ABI layout (`input[4..]`) — standard head/tail encoding -//! | Slot (bytes) | Type | Field | -//! |-------------|-------------|--------------------| -//! | 0..32 | `uint256` | offset → `proof` | -//! | 32..64 | `bytes32` | `merkle_root` | -//! | 64..96 | `uint256` | offset → nullifiers| -//! | 96..128 | `uint256` | offset → commitments| -//! | 128..160 | `uint256` | offset → memos | -//! | 160..192 | `uint32` | `asset_id` | -//! | 192..224 | `uint256` | `fee` | -//! | 224..256 | `uint32` | `circuit_version` | +//! ## ABI layout (`input[4..]`) +//! | Slot (bytes) | Type | Field | +//! |--------------|-----------|-----------------------| +//! | 0..32 | `uint256` | offset → `proof` | +//! | 32..64 | `bytes32` | `merkle_root` | +//! | 64..96 | `uint256` | offset → `nullifiers` | +//! | 96..128 | `uint256` | offset → `commitments`| +//! | 128..160 | `uint256` | offset → `memos` | +//! | 160..192 | `uint32` | `asset_id` | +//! | 192..224 | `uint256` | `fee` | +//! | 224..256 | `uint32` | `circuit_version` | //! -//! `relayer` is derived from `handle.context().caller` — not part of the ABI. +//! ## Notes +//! The three arrays are parallel: `commitments[i]` and `memos[i]` describe the +//! output note paid for by `nullifiers[i]`, so all three must have equal length. +//! +//! `relayer` is not in the ABI: it comes from `handle.context().caller`. use fp_evm::{ExitError, PrecompileFailure, PrecompileHandle}; use frame_support::BoundedVec; @@ -25,9 +29,7 @@ use sp_core::U256; use crate::abi; -/// `keccak256("privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256,uint32)")[0..4]` -/// The trailing `uint32` is `circuitVersion` — the circuit version the spent -/// notes were created under, so the proof is verified against that version's VK. +/// Selector for the signature in this module's header. pub const SELECTOR: [u8; 4] = [0x66, 0xed, 0x2c, 0xd4]; /// Maximum byte length of a serialised Groth16 proof accepted by the pallet. @@ -35,11 +37,10 @@ const MAX_PROOF_LEN: u32 = 512; /// Maximum number of input nullifiers / output commitments in a single transfer. const MAX_NOTES: u32 = 2; -/// Decodes the ABI-encoded `input` and returns a ready-to-dispatch -/// `private_transfer` call. +/// Decodes `input` into a ready-to-dispatch `private_transfer` call. /// -/// `handle.context().caller` is forwarded as the `relayer` field so -/// `pallet-relayer` can route fees to the registered Substrate account. +/// Beyond the ABI itself, this enforces the structural invariant the proof does +/// not cover: at least one input note, and the three arrays equal in length. pub fn decode( handle: &impl PrecompileHandle, input: &[u8], @@ -49,10 +50,14 @@ where pallet_shielded_pool::BalanceOf: TryFrom, { let params = &input[4..]; + + // Step 1: require all eight head slots. The array offsets they carry point + // into the tail, whose bounds each decoder validates on its own. if params.len() < 256 { return Err(err("privateTransfer: input too short")); } + // Step 2: proof — dynamic, offset at slot 0. let proof: BoundedVec> = abi::decode_bytes_at_slot(params, 0)? .try_into() @@ -62,8 +67,11 @@ where return Err(err("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 4: the three parallel arrays — nullifiers spent, commitments created, + // and the memo carrying each new note's secrets. let nullifiers: BoundedVec< pallet_shielded_pool::Nullifier, frame_support::traits::ConstU32, @@ -97,10 +105,10 @@ where .try_into() .map_err(|_| err("privateTransfer: too many memos"))?; - // Structural consistency: at least one real input note is required, and the three - // parallel arrays must have the same length. The ZK proof enforces value balance, - // but mismatched array lengths would produce a nonsensical call that reaches the - // pallet unnecessarily. + // Step 5: the arrays must line up. A length mismatch is malformed input, not a + // balance question, and the proof cannot catch it — it constrains values, not + // how many memos were attached, so a short memo array would silently drop the + // secrets for an output note that still gets created. if nullifiers.is_empty() { return Err(err("privateTransfer: at least one nullifier required")); } @@ -111,8 +119,10 @@ where return Err(err("privateTransfer: commitment/memo count mismatch")); } + // Step 6: asset_id. let asset_id = abi::decode_u32(¶ms[160..192])?; + // Step 7: fee paid to the relayer. let fee: pallet_shielded_pool::BalanceOf = { let raw: u128 = U256::from_big_endian(¶ms[192..224]) .try_into() @@ -121,8 +131,11 @@ where .map_err(|_| err("privateTransfer: fee conversion failed"))? }; + // Step 8: relayer. Not an ABI field — whoever submits the EVM transaction is + // the relayer, so the calldata cannot spoof it. let relayer = Some(handle.context().caller); + // Step 9: circuit_version, selecting the VK the proof is checked against. let circuit_version = abi::decode_u32(¶ms[224..256])?; Ok(pallet_shielded_pool::Call::::private_transfer { diff --git a/frame/evm/precompile/shielded-pool/src/calls/shield.rs b/frame/evm/precompile/shielded-pool/src/calls/shield.rs index 9ec0ef80..b9228f3a 100644 --- a/frame/evm/precompile/shielded-pool/src/calls/shield.rs +++ b/frame/evm/precompile/shielded-pool/src/calls/shield.rs @@ -4,26 +4,27 @@ //! `keccak256("shield(uint32,bytes32,bytes)")[0..4]` = `0x9feb22ea` //! //! ## ABI layout (`input[4..]`) -//! | Slot (bytes) | Type | Field | -//! |-------------|-----------|-----------------| -//! | 0..32 | `uint32` | `asset_id` | -//! | 32..64 | `bytes32` | `commitment` | -//! | 64..96 | `uint256` | offset → memo | -//! | at offset | `bytes` | `encrypted_memo`| +//! | Slot (bytes) | Type | Field | +//! |--------------|-----------|------------------| +//! | 0..32 | `uint32` | `asset_id` | +//! | 32..64 | `bytes32` | `commitment` | +//! | 64..96 | `uint256` | offset → memo | //! -//! The token **amount** is read from `msg.value` — the EVM executor transfers it to -//! the precompile's address before `execute` runs, so no explicit amount slot is needed. +//! ## Notes +//! `amount` is not in the ABI: it is `msg.value`, which the EVM executor has +//! already transferred to the precompile's address before `execute` runs. use fp_evm::{ExitError, PrecompileFailure, PrecompileHandle}; use crate::abi; -/// `keccak256("shield(uint32,bytes32,bytes)")[0..4]` +/// Selector for the signature in this module's header. pub const SELECTOR: [u8; 4] = [0x9f, 0xeb, 0x22, 0xea]; -/// Decodes the ABI-encoded `input` and returns a ready-to-dispatch `shield` call. +/// Decodes `input` into a ready-to-dispatch `shield` call. /// -/// `handle` is consulted only for `apparent_value` (the `msg.value` ETH amount). +/// `handle` supplies the amount via `apparent_value` (`msg.value`); the calldata +/// carries only the asset, the commitment, and the memo. pub fn decode( handle: &impl PrecompileHandle, input: &[u8], @@ -33,15 +34,19 @@ where pallet_shielded_pool::BalanceOf: TryFrom, { let params = &input[4..]; + + // Step 1: require the three-slot head. The memo offset it carries is bounds + // checked by the tail decoder in step 5. if params.len() < 96 { return Err(err("shield: input too short")); } + // Step 2: asset_id. let asset_id = abi::decode_u32(¶ms[0..32])?; - // Reject zero-value calls at the precompile boundary (defense-in-depth; - // the pallet also rejects them, but this produces a cleaner error before - // reaching the dispatch layer). + // Step 3: amount, taken from msg.value rather than the calldata. Zero is + // rejected here as well as in the pallet — it would mint a commitment backed + // by no funds. let apparent_value = handle.context().apparent_value; if apparent_value.is_zero() { return Err(err("shield: amount must be non-zero")); @@ -55,8 +60,11 @@ where .map_err(|_| err("shield: amount conversion failed"))? }; + // Step 4: commitment of the note being created. let commitment = pallet_shielded_pool::Commitment::from(abi::read_bytes32(params, 32)?); + // Step 5: encrypted_memo — dynamic, offset at slot 64. It carries the only + // copy of the new note's secrets, so a malformed one fails the call. let memo_bytes = abi::decode_bytes_at_slot(params, 64)?; let encrypted_memo = pallet_shielded_pool::FrameEncryptedMemo::new(memo_bytes) .map_err(|_| err("shield: memo too long or wrong size"))?; diff --git a/frame/evm/precompile/shielded-pool/src/calls/unshield.rs b/frame/evm/precompile/shielded-pool/src/calls/unshield.rs index ddd0fc46..4ef9fb20 100644 --- a/frame/evm/precompile/shielded-pool/src/calls/unshield.rs +++ b/frame/evm/precompile/shielded-pool/src/calls/unshield.rs @@ -6,30 +6,28 @@ //! = `0x4e505348` //! //! ## ABI layout (`input[4..]`) -//! | Slot (bytes) | Type | Field | -//! |-------------|-----------|-----------------| -//! | 0..32 | `uint256` | offset → `proof`| -//! | 32..64 | `bytes32` | `merkle_root` | -//! | 64..96 | `bytes32` | `nullifier` | -//! | 96..128 | `uint32` | `asset_id` | -//! | 128..160 | `uint256` | `amount` | -//! | 160..192 | `bytes32` | `recipient` (AccountId32) | -//! | 192..224 | `uint256` | `fee` | -//! | 224..256 | `bytes32` | `change_commitment` | -//! | 256..288 | `uint256` | offset → `change_encrypted_memo` | -//! | 288..320 | `uint32` | `circuit_version` | +//! | Slot (bytes) | Type | Field | +//! |--------------|-----------|----------------------------------| +//! | 0..32 | `uint256` | offset → `proof` | +//! | 32..64 | `bytes32` | `merkle_root` | +//! | 64..96 | `bytes32` | `nullifier` | +//! | 96..128 | `uint32` | `asset_id` | +//! | 128..160 | `uint256` | `amount` | +//! | 160..192 | `bytes32` | `recipient` (AccountId32) | +//! | 192..224 | `uint256` | `fee` | +//! | 224..256 | `bytes32` | `change_commitment` | +//! | 256..288 | `uint256` | offset → `change_encrypted_memo` | +//! | 288..320 | `uint32` | `circuit_version` | //! -//! `recipient` is an `AccountId32` encoded as a 32-byte ABI `bytes32` slot. -//! This can be a Substrate-native account or the `AccountId32` derived from -//! an H160 address (`H160 ++ [0x00; 12]`). +//! ## Notes +//! `recipient` is an `AccountId32` in a `bytes32` slot — either a Substrate-native +//! account or one derived from an H160 (`H160 ++ [0x00; 12]`). //! -//! `change_commitment` is `[0u8; 32]` for a total unshield (no change note). -//! For a partial unshield it is `NoteCommitment(change_value, asset_id, change_owner_pk, change_blinding)`. +//! `change_commitment` is all-zero for a total unshield. For a partial one it is +//! `NoteCommitment(change_value, asset_id, change_owner_pk, change_blinding)`, and +//! `change_encrypted_memo` then holds `nonce(12) || ciphertext(132) || ephPk(32)`. //! -//! `change_encrypted_memo` is a dynamic `bytes` field (176 bytes for partial unshield, 0 bytes for total). -//! For a partial unshield it contains: nonce(12) || ciphertext(132) || ephPk(32). -//! -//! `relayer` is derived from `handle.context().caller` — not part of the ABI. +//! `relayer` is not in the ABI: it comes from `handle.context().caller`. use alloc::vec::Vec; @@ -39,18 +37,17 @@ use sp_core::U256; use crate::abi; -/// `keccak256("unshield(bytes,bytes32,bytes32,uint32,uint256,bytes32,uint256,bytes32,bytes,uint32)")[0..4]` -/// The trailing `uint32` is `circuitVersion` — the circuit version the spent -/// notes were created under, so the proof is verified against that version's VK. +/// Selector for the signature in this module's header. pub const SELECTOR: [u8; 4] = [0x4e, 0x50, 0x53, 0x48]; /// Maximum byte length of a serialised Groth16 proof accepted by the pallet. const MAX_PROOF_LEN: u32 = 512; -/// Decodes the ABI-encoded `input` and returns a ready-to-dispatch `unshield` call. +/// Decodes `input` into a ready-to-dispatch `unshield` call. /// -/// `handle.context().caller` is forwarded as the `relayer` field so -/// `pallet-relayer` can route fees to the registered Substrate account. +/// Rejects anything the pallet would have to reject anyway, plus the inputs that +/// are irrecoverable rather than merely invalid: a zero amount, the zero +/// recipient, and a malformed change memo. pub fn decode( handle: &impl PrecompileHandle, input: &[u8], @@ -61,10 +58,15 @@ where ::AccountId: From<[u8; 32]>, { let params = &input[4..]; + + // Step 1: require the head through `change_commitment`. The last two slots are + // checked later — the memo offset only when a change note exists (step 9), and + // `circuit_version` in step 11. if params.len() < 256 { return Err(err("unshield: input too short")); } + // Step 2: proof — dynamic, offset at slot 0. let proof: BoundedVec> = abi::decode_bytes_at_slot(params, 0)? .try_into() @@ -74,13 +76,16 @@ where return Err(err("unshield: proof must be non-empty")); } + // Step 3: merkle_root and the nullifier of the note being spent. let merkle_root: pallet_shielded_pool::Hash = abi::read_bytes32(params, 32)?; let nullifier = pallet_shielded_pool::Nullifier::from(abi::read_bytes32(params, 64)?); + // Step 4: asset_id. let asset_id = abi::decode_u32(¶ms[96..128])?; - // Reject zero-amount unshield early (defense-in-depth before dispatch). + // Step 5: amount. Zero is rejected here as well as in the pallet — it is a + // no-op that still burns the nullifier, destroying the note it spends. let amount_u256 = U256::from_big_endian(¶ms[128..160]); if amount_u256.is_zero() { return Err(err("unshield: amount must be non-zero")); @@ -93,14 +98,15 @@ where .map_err(|_| err("unshield: amount conversion failed"))? }; - // Reject the zero AccountId32 (all-zeros): transferring to this address - // permanently destroys tokens with no possibility of recovery. + // Step 6: recipient. The all-zero AccountId32 has no known private key, so + // unshielding to it destroys the funds with no possibility of recovery. let recipient_bytes = abi::read_bytes32(params, 160)?; if recipient_bytes == [0u8; 32] { return Err(err("unshield: recipient must not be the zero address")); } let recipient: ::AccountId = recipient_bytes.into(); + // Step 7: fee paid to the relayer. let fee: pallet_shielded_pool::BalanceOf = { let raw: u128 = U256::from_big_endian(¶ms[192..224]) .try_into() @@ -109,13 +115,16 @@ where .map_err(|_| err("unshield: fee conversion failed"))? }; + // Step 8: change_commitment. All-zero means a total unshield, which leaves no + // change note behind. let change_commitment: pallet_shielded_pool::Hash = abi::read_bytes32(params, 224)?; let is_total_unshield = change_commitment == [0u8; 32]; - // Decode change_encrypted_memo as a dynamic bytes field (offset pointer at - // slot 256). For a partial unshield (non-zero change_commitment) a malformed - // offset must fail loudly — silently defaulting to an empty memo would make - // the change note unrecoverable. A total unshield legitimately has no memo. + // Step 9: change_encrypted_memo — dynamic, offset at slot 256. A total + // unshield carries no memo, so a missing or malformed offset is expected + // there. A partial one must fail loudly: the memo is the only copy of the + // change note's secrets, and defaulting to an empty one would leave the + // change permanently unspendable. let change_encrypted_memo_bytes = if params.len() >= 288 { match abi::decode_bytes_at_slot(params, 256) { Ok(bytes) => bytes, @@ -129,20 +138,21 @@ where Vec::new() }; - // Convert to EncryptedMemo (max 176 bytes per pallet definition). - // Empty bytes (total unshield) is allowed and results in an empty EncryptedMemo. let change_encrypted_memo: pallet_shielded_pool::types::EncryptedMemo = if change_encrypted_memo_bytes.is_empty() { - // Total unshield: empty memo pallet_shielded_pool::types::EncryptedMemo::default() } else { - // Partial unshield: create from bytes pallet_shielded_pool::types::EncryptedMemo::new(change_encrypted_memo_bytes) .map_err(|_| err("unshield: invalid change_encrypted_memo"))? }; + // Step 10: relayer. Not an ABI field — whoever submits the EVM transaction is + // the relayer, so the calldata cannot spoof it. let relayer = Some(handle.context().caller); + // Step 11: circuit_version. Checked here rather than in the step 1 guard so + // that calldata predating this slot reports the missing field instead of a + // generic length error. if params.len() < 320 { return Err(err("unshield: input too short (missing circuitVersion)")); } diff --git a/frame/evm/precompile/shielded-pool/src/lib.rs b/frame/evm/precompile/shielded-pool/src/lib.rs index 570872b3..28a51883 100644 --- a/frame/evm/precompile/shielded-pool/src/lib.rs +++ b/frame/evm/precompile/shielded-pool/src/lib.rs @@ -6,6 +6,19 @@ pub(crate) mod abi; pub(crate) mod calls; pub(crate) mod dispatch; +/// The ABI selectors this precompile answers to. +/// +/// Exported so the relay whitelist can be pinned against them in a test rather +/// than kept in sync by hand. A hand-kept copy drifting from this one fails +/// silently: a wrong selector still yields "unsupported selector", so the +/// rejection tests stay green while the accept path quietly stops working. +pub mod selectors { + pub use crate::calls::claim_shielded_fees::SELECTOR as CLAIM_SHIELDED_FEES; + pub use crate::calls::private_transfer::SELECTOR as PRIVATE_TRANSFER; + pub use crate::calls::shield::SELECTOR as SHIELD; + pub use crate::calls::unshield::SELECTOR as UNSHIELD; +} + use core::marker::PhantomData; use fp_evm::{ExitError, Precompile, PrecompileFailure, PrecompileHandle, PrecompileResult}; diff --git a/frame/evm/precompile/shielded-pool/src/tests.rs b/frame/evm/precompile/shielded-pool/src/tests.rs index 67e4f7f2..1b4864d6 100644 --- a/frame/evm/precompile/shielded-pool/src/tests.rs +++ b/frame/evm/precompile/shielded-pool/src/tests.rs @@ -37,6 +37,21 @@ fn expect_error(result: Result) { ); } +/// Like [`expect_error`], but pins WHICH rejection fired. The adversarial tests +/// need this: a decoder that refuses everything would pass a bare `is_err`, so +/// the message is what proves the intended check ran. +fn expect_error_msg(result: Result, needle: &str) { + match result { + Err(PrecompileFailure::Error { + exit_status: ExitError::Other(msg), + }) => assert!( + msg.contains(needle), + "expected an error containing {needle:?}, got: {msg:?}" + ), + other => panic!("expected PrecompileFailure::Error(Other), got: {other:?}"), + } +} + fn assert_success(result: Result) { match result { Ok(out) => assert_eq!(out.exit_status, fp_evm::ExitSucceed::Stopped), @@ -459,11 +474,33 @@ fn shield_with_zero_value_rejected() { #[test] fn private_transfer_rejects_truncated_input() { new_test_ext().execute_with(|| { - let mut h = MockHandle::new(vec![0x8c, 0x0f, 0x5d, 0x24]); + // The REAL selector, taken from the decoder. A stale literal here would + // still make this test pass — a wrong selector is rejected as + // "unsupported" — while testing nothing about truncation. + let mut h = MockHandle::new(crate::calls::private_transfer::SELECTOR.to_vec()); expect_error(ShieldedPoolPrecompile::::execute(&mut h)); }); } +#[test] +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 hash = sp_io::hashing::keccak_256(sig); + assert_eq!(hash[..4], crate::calls::private_transfer::SELECTOR); +} + +#[test] +fn unshield_selector_matches_signature() { + // Both relay selectors were stale on main, not just privateTransfer's, so + // unshield gets the same guard. + let sig = + b"unshield(bytes,bytes32,bytes32,uint32,uint256,bytes32,uint256,bytes32,bytes,uint32)"; + let hash = sp_io::hashing::keccak_256(sig); + assert_eq!(hash[..4], crate::calls::unshield::SELECTOR); +} + #[test] fn private_transfer_rejects_empty_proof() { new_test_ext().execute_with(|| { @@ -888,3 +925,255 @@ fn shield_asset_id_round_trips_through_abi() { ); } } +// ───────────────────────────────────────────────────────────────────────────── +// Adversarial ABI battery — the attacker controls every byte of `input` +// +// This is the only surface where untrusted bytes reach the node directly: an +// EVM caller can send arbitrary calldata to the precompile address. Each test +// below is an attempt to make the decoder panic, over-allocate, or read out of +// bounds. A panic here is a node crash, not a rejected transaction. +// ───────────────────────────────────────────────────────────────────────────── + +/// Truncated calldata at every length from the selector to a full head. None of +/// these may panic — the decoder must reject each one cleanly. +#[test] +fn attack_truncation_at_every_offset_never_panics() { + new_test_ext().execute_with(|| { + let full = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + for len in 0..full.len().min(600) { + let mut h = MockHandle::new(full[..len].to_vec()); + // Must not panic. Any Result is acceptable. + let _ = ShieldedPoolPrecompile::::execute(&mut h); + } + }); +} + +/// An offset pointing back into the head makes the "length" word overlap the +/// caller-controlled head — a classic way to fabricate a huge length. +#[test] +fn attack_self_referential_offset_is_refused() { + new_test_ext().execute_with(|| { + let mut input = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + // Point the proof offset at slot 0 of the head (offset 0 → itself). + input[4..36].copy_from_slice(&[0u8; 32]); + let mut h = MockHandle::new(input); + let _ = ShieldedPoolPrecompile::::execute(&mut h); + }); +} + +/// Every dynamic offset set to u256::MAX. word_to_usize must reject before any +/// slicing arithmetic happens. +#[test] +fn attack_max_u256_offsets_are_refused_not_truncated() { + new_test_ext().execute_with(|| { + for slot in [0usize, 64, 96, 128, 256] { + let mut input = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + input[4 + slot..4 + slot + 32].copy_from_slice(&[0xFFu8; 32]); + let mut h = MockHandle::new(input); + expect_error(ShieldedPoolPrecompile::::execute(&mut h)); + } + }); +} + +/// 2^32 + small: the low 32 bits look like a valid offset while the value is +/// astronomically out of range. This is the exact shape that `low_u32` let +/// through historically. +#[test] +fn attack_offset_above_u32_that_looks_benign_is_refused() { + new_test_ext().execute_with(|| { + let mut input = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + // 2^32 + 288 — low 32 bits read as 288, a perfectly plausible offset. + let sneaky = U256::from(1u64 << 32) + U256::from(288u64); + let word = sneaky.to_big_endian(); + input[4..36].copy_from_slice(&word); + let mut h = MockHandle::new(input); + expect_error(ShieldedPoolPrecompile::::execute(&mut h)); + }); +} + +/// A declared array count of ~1e9 must be refused BEFORE Vec::with_capacity +/// reserves for it — otherwise one call OOMs the node. +#[test] +fn attack_huge_array_count_does_not_allocate() { + new_test_ext().execute_with(|| { + let mut input = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + // Find the nullifiers array offset and overwrite its count word. + let off = U256::from_big_endian(&input[4 + 64..4 + 96]).as_usize(); + let count_at = 4 + off; + if count_at + 32 <= input.len() { + let huge = U256::from(1u64 << 30).to_big_endian(); + input[count_at..count_at + 32].copy_from_slice(&huge); + } + let mut h = MockHandle::new(input); + expect_error(ShieldedPoolPrecompile::::execute(&mut h)); + }); +} + +/// A fee word of u256::MAX must not panic converting to u128 — it must be +/// rejected as an overflow. +#[test] +fn attack_max_fee_word_is_refused_not_panicking() { + new_test_ext().execute_with(|| { + let mut input = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + input[4 + 192..4 + 224].copy_from_slice(&[0xFFu8; 32]); + let mut h = MockHandle::new(input); + expect_error_msg( + ShieldedPoolPrecompile::::execute(&mut h), + "fee overflow", + ); + }); +} + +/// asset_id and circuit_version live in u32 slots. A word with high bits set +/// must be rejected, never truncated to a plausible small number. +#[test] +fn attack_oversized_u32_slots_are_refused_not_truncated() { + new_test_ext().execute_with(|| { + for slot in [160usize, 224] { + let mut input = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + // 2^32 exactly: truncates to 0 if the decoder uses low_u32. + let word = U256::from(1u64 << 32).to_big_endian(); + input[4 + slot..4 + slot + 32].copy_from_slice(&word); + let mut h = MockHandle::new(input); + expect_error(ShieldedPoolPrecompile::::execute(&mut h)); + } + }); +} + +/// Random fuzz over the whole calldata: flip bytes everywhere and assert the +/// decoder never panics. Deterministic (fixed LCG) so a failure reproduces. +#[test] +fn attack_byte_fuzz_never_panics() { + new_test_ext().execute_with(|| { + let base = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + let mut seed: u64 = 0x2545F4914F6CDD1D; + for _ in 0..3000 { + let mut input = base.clone(); + // 1–8 mutations per round. + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + let muts = 1 + (seed >> 60) as usize % 8; + for _ in 0..muts { + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + let pos = (seed >> 33) as usize % input.len(); + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + input[pos] = (seed >> 40) as u8; + } + let mut h = MockHandle::new(input); + // Only requirement: no panic. + let _ = ShieldedPoolPrecompile::::execute(&mut h); + } + }); +} + +/// Fuzz with truncation AND mutation combined — the shape most likely to hit an +/// unchecked slice near a boundary. +#[test] +fn attack_truncated_fuzz_never_panics() { + new_test_ext().execute_with(|| { + let base = encode_private_transfer( + &[0x01u8; 72], + canon(0xBB), + &[canon(1)], + &[canon(2)], + &[vec![0x01u8; 180]], + 0, + 0, + 1, + ); + let mut seed: u64 = 0x9E3779B97F4A7C15; + for _ in 0..2000 { + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + let cut = 4 + (seed >> 33) as usize % base.len().max(1); + let mut input = base[..cut.min(base.len())].to_vec(); + if !input.is_empty() { + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + let pos = (seed >> 33) as usize % input.len(); + input[pos] = (seed >> 40) as u8; + } + let mut h = MockHandle::new(input); + let _ = ShieldedPoolPrecompile::::execute(&mut h); + } + }); +} diff --git a/frame/shielded-pool/CHANGELOG.md b/frame/shielded-pool/CHANGELOG.md index a5ac0713..6ea1c580 100644 --- a/frame/shielded-pool/CHANGELOG.md +++ b/frame/shielded-pool/CHANGELOG.md @@ -2,6 +2,44 @@ All notable changes to `pallet-shielded-pool` will be documented in this file. +## [0.17.1] - 2026-08-11 + +### Security + +- **Pool admission tags one entry per NULLIFIER, in a namespace shared with + `unshield` (`ShieldedPoolSpend`).** `and_provides` contributes exactly ONE + tag, so passing it a `Vec` encoded the whole nullifier set plus the relayer + into a single blob. Three consequences, each free for an attacker since the + fee is only charged on execution: + + - reordering the two inputs produced a different tag, minting a **second + admissible pool entry for the same spend**; + - two transfers sharing only ONE note (`A+B` and `A+C`) did not collide at + all, so one note could back an unbounded number of pool entries; + - `private_transfer` and `unshield` used different tag prefixes, so the same + note could back one of each simultaneously. + + Every variant propagates and is revalidated network-wide while at most one + can ever execute. + + `relayer` deliberately leaves the tag. Binding it made a copy with a swapped + fee recipient a *separate* entry, so anyone could rebroadcast another user's + spend pointed at their own account and have both sit in the pool; keyed on + the nullifier the two are mutually exclusive, so taking the fee requires + out-bidding, which means paying it. + + **Admission policy, not state transition — consensus is unaffected.** Nodes + on the old logic keep accepting the duplicate variants, so the mitigation + completes as the network updates. + +### Tests + +- Adversarial batteries rather than happy-path coverage: 13 attacks on the + transfer operation (double-spend within one extrinsic, non-canonical field + elements, forged roots, array-arity mismatch, undersized memo, duplicate + commitments) and 11 on pool admission (tag collisions, relayer swap, + reordered inputs, cross-call exclusivity with `unshield`). + ## [0.17.0] - 2026-08-07 ### Security diff --git a/frame/shielded-pool/Cargo.toml b/frame/shielded-pool/Cargo.toml index ae846927..30642092 100644 --- a/frame/shielded-pool/Cargo.toml +++ b/frame/shielded-pool/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-shielded-pool" -version = "0.17.0" +version = "0.17.1" 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/operations/private_transfer.rs b/frame/shielded-pool/src/operations/private_transfer.rs index 7832b206..3d21f729 100644 --- a/frame/shielded-pool/src/operations/private_transfer.rs +++ b/frame/shielded-pool/src/operations/private_transfer.rs @@ -865,4 +865,426 @@ mod tests { "two outputs must cost more proof_size than one" ); } + + // ── adversarial battery ────────────────────────────────────────────────── + // + // Each of these is an attempt to BREAK an invariant, not a demonstration + // that it holds. They are written from the attacker's side: assume the ZK + // proof is satisfiable (the mock skips verification) and ask what the + // non-cryptographic checks still have to stop on their own. + + /// Double-spend inside ONE extrinsic, same nullifier twice. + /// + /// The set check cannot catch this: neither nullifier is in storage yet when + /// the loop runs, so only the explicit pairwise comparison stands between + /// this and spending one note twice in a single call. + #[test] + fn attack_same_nullifier_twice_in_one_extrinsic_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let mut nulls: BoundedVec> = BoundedVec::new(); + nulls.try_push(make_nullifier(0x77)).unwrap(); + nulls.try_push(make_nullifier(0x77)).unwrap(); // same note, twice + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nulls, + commitments_of(&[0xC1, 0xC2]), + memos_of(2), + 0u32, + 0u128, + None, + 1, + ), + Error::::NullifierAlreadyUsed + ); + }); + } + + /// The dummy nullifier is exempt from the "already used" check by design. + /// Two dummies in one call must therefore NOT be readable as a duplicate + /// pair — but the all-dummy guard has to reject the call outright, or a + /// transfer with no real input mints two free leaves. + #[test] + fn attack_two_dummy_nullifiers_cannot_mint_free_leaves() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let mut nulls: BoundedVec> = BoundedVec::new(); + nulls.try_push(Nullifier::new([0u8; 32])).unwrap(); + nulls.try_push(Nullifier::new([0u8; 32])).unwrap(); + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nulls, + commitments_of(&[0xD1, 0xD2]), + memos_of(2), + 0u32, + 0u128, + None, + 1, + ), + Error::::InvalidAmount + ); + }); + } + + /// Replay of a nullifier already spent in an EARLIER block. + #[test] + fn attack_replaying_a_spent_nullifier_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + assert_ok!(PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0x51]), + commitments_of(&[0x52]), + memos_of(1), + 0u32, + 0u128, + None, + 1, + )); + + // Same nullifier, different outputs — the note is already gone. + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0x51]), + commitments_of(&[0x53]), + memos_of(1), + 0u32, + 0u128, + None, + 1, + ), + Error::::NullifierAlreadyUsed + ); + }); + } + + /// Non-canonical field elements: bytes above the BN254 modulus that reduce + /// to a DIFFERENT, already-spent value. Accepting them would give every + /// nullifier a second spelling and defeat the double-spend set entirely. + #[test] + fn attack_non_canonical_nullifier_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + // modulus + 1, little-endian — reduces to 1, which is canonical. + let mut over = [0u8; 32]; + over[0] = 0x02; + over[31] = 0xFF; + assert!( + !Nullifier::new(over).is_canonical(), + "fixture must actually be non-canonical or the test proves nothing" + ); + + let mut nulls: BoundedVec> = BoundedVec::new(); + nulls.try_push(Nullifier::new(over)).unwrap(); + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nulls, + commitments_of(&[0xE1]), + memos_of(1), + 0u32, + 0u128, + None, + 1, + ), + Error::::InvalidPublicSignals + ); + }); + } + + /// Same, on the output side: a non-canonical commitment would land a leaf + /// whose second spelling could collide with a real one. + #[test] + fn attack_non_canonical_commitment_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let mut over = [0u8; 32]; + over[0] = 0x02; + over[31] = 0xFF; + assert!(!Commitment::new(over).is_canonical()); + + let mut comms: BoundedVec> = BoundedVec::new(); + comms.try_push(Commitment::new(over)).unwrap(); + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0x61]), + comms, + memos_of(1), + 0u32, + 0u128, + None, + 1, + ), + Error::::InvalidPublicSignals + ); + }); + } + + /// A forged Merkle root the attacker made up: it lets them prove membership + /// of a note that was never in the tree. + #[test] + fn attack_unknown_merkle_root_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + [0xEEu8; 32], // never added + nullifiers_of(&[0x71]), + commitments_of(&[0x72]), + memos_of(1), + 0u32, + 0u128, + None, + 1, + ), + Error::::UnknownMerkleRoot + ); + }); + } + + /// Array-length confusion: more commitments than nullifiers would insert an + /// output nothing paid for. + #[test] + fn attack_more_commitments_than_nullifiers_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0x81]), // 1 input + commitments_of(&[0x82, 0x83]), // 2 outputs + memos_of(2), + 0u32, + 0u128, + None, + 1, + ), + Error::::TooManyInputsOrOutputs + ); + }); + } + + /// Memo count out of step with the outputs: a missing memo would leave a + /// commitment nobody can ever open, and the zip() that stores them would + /// silently drop the extra output. + #[test] + fn attack_memo_count_mismatch_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0x91, 0x92]), + commitments_of(&[0x93, 0x94]), + memos_of(1), // one memo for two outputs + 0u32, + 0u128, + None, + 1, + ), + Error::::MemoCommitmentMismatch + ); + }); + } + + /// A wrong-sized memo must not reach storage: the wallet's decrypt path + /// slices fixed offsets, so a short memo is a note nobody can open. + #[test] + fn attack_undersized_memo_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let mut memos: BoundedVec> = BoundedVec::new(); + memos.try_push(short_memo()).unwrap(); + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0xA9]), + commitments_of(&[0xAA]), + memos, + 0u32, + 0u128, + None, + 1, + ), + Error::::InvalidMemoSize + ); + }); + } + + /// The zero commitment is the tree's empty-leaf sentinel. Inserting it as a + /// real output would corrupt the Merkle structure. + #[test] + fn attack_zero_commitment_is_refused() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let mut comms: BoundedVec> = BoundedVec::new(); + comms.try_push(Commitment::new([0u8; 32])).unwrap(); + + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0xB9]), + comms, + memos_of(1), + 0u32, + 0u128, + None, + 1, + ), + Error::::InvalidPublicSignals + ); + }); + } + + /// A fee below the relay minimum must be refused BEFORE any state changes — + /// otherwise the pool subsidizes the spam it is meant to price out. + #[test] + fn attack_fee_below_minimum_is_refused_without_spending_the_nullifier() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + let min = ::Relayer::min_relay_fee(); + if min == 0 { + return; // mock has no minimum; nothing to prove here + } + + let n = make_nullifier(0xC9); + assert_err!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0xC9]), + commitments_of(&[0xCA]), + memos_of(1), + 0u32, + min.saturating_sub(1), + None, + 1, + ), + Error::::FeeTooLow + ); + // And the note must still be spendable — a rejected call that burned + // the nullifier would destroy funds. + assert!(!NullifierRepository::is_used::(&n)); + }); + } + + /// Duplicate commitments inside ONE call, checked for real. + /// + /// The duplicate guard reads `CommitmentMemos`, which is only populated + /// AFTER each insert by `store_memo`. Within a single call the loop runs + /// insert→store_memo per output, so by the time the second (identical) + /// output is inserted the first one's memo IS stored and the guard fires. + /// If that ordering ever changes, one note would take two leaves in one + /// transaction — this pins the outcome, not the mechanism. + #[test] + fn attack_duplicate_commitments_in_one_call_cannot_take_two_leaves() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let mut comms: BoundedVec> = BoundedVec::new(); + comms.try_push(make_commitment(0xF1)).unwrap(); + comms.try_push(make_commitment(0xF1)).unwrap(); // same leaf twice + + let before = MerkleRepository::get_tree_size::(); + + // Run inside a storage transaction, the way a dispatchable executes: + // FRAME rolls the whole extrinsic back on error, so the partial leaf + // from the first (accepted) output must not survive. Calling + // `execute` bare would leave that write in place — an artefact of the + // test harness, not of the runtime. + let result = frame_support::storage::with_storage_layer(|| { + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0xF2, 0xF3]), + comms, + memos_of(2), + 0u32, + 0u128, + None, + 1, + ) + }); + + assert!(result.is_err(), "a duplicated output must not be accepted"); + let after = MerkleRepository::get_tree_size::(); + assert_eq!( + before, after, + "the rejected call must leave no leaf behind once rolled back" + ); + }); + } + + /// The memo is opaque to the chain, and must stay that way. + /// + /// `sourcePk` lives at plaintext bytes [84,116) INSIDE the ciphertext — the + /// pallet holds no key and must never gate on memo contents. This pins that: + /// two transfers whose memos differ only in those bytes are equally valid on + /// chain. A pallet that could tell them apart would mean the memo was not + /// actually encrypted. + #[test] + fn attack_memo_contents_never_gate_admission() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + // Two memos, same length, different bytes where sourcePk would sit. + let mut a = [0x01u8; MAX_ENCRYPTED_MEMO_SIZE as usize]; + let mut b = [0x01u8; MAX_ENCRYPTED_MEMO_SIZE as usize]; + for byte in a[84..116].iter_mut() { + *byte = 0x00; + } + for byte in b[84..116].iter_mut() { + *byte = 0xAB; + } + + for (i, bytes) in [a, b].into_iter().enumerate() { + let mut memos: BoundedVec> = BoundedVec::new(); + memos + .try_push(EncryptedMemo::from_bytes(&bytes).unwrap()) + .unwrap(); + let seed = 0x80 + i as u8 * 2; + assert_ok!(PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[seed]), + commitments_of(&[seed + 1]), + memos, + 0u32, + 0u128, + None, + 1, + )); + } + }); + } } diff --git a/frame/shielded-pool/src/validate_unsigned/mod.rs b/frame/shielded-pool/src/validate_unsigned/mod.rs index 96417673..480be9e3 100644 --- a/frame/shielded-pool/src/validate_unsigned/mod.rs +++ b/frame/shielded-pool/src/validate_unsigned/mod.rs @@ -28,9 +28,19 @@ pub use unshield::validate_unshield; /// pass admission, propagate, and only then revert with `UnknownMerkleRoot`. pub(crate) const TX_LONGEVITY: u64 = 64; +/// ONE tag namespace for every operation that spends a note. +/// +/// A nullifier identifies a NOTE, not an operation, and the on-chain rule is +/// simply "each note is spent once" — whether by a transfer or an unshield. +/// While transfer and unshield used separate prefixes, the same note could back +/// one of each in the pool at the same time: both propagate and get revalidated +/// network-wide, only one can ever execute. Sharing the namespace makes pool +/// admission mirror the chain: one note, one entry. +pub(crate) const SPEND_TAG_PREFIX: &str = "ShieldedPoolSpend"; + #[cfg(test)] mod tests { - use super::{TX_LONGEVITY, validate_private_transfer, validate_unshield}; + use super::{TX_LONGEVITY, codes, validate_private_transfer, validate_unshield}; use crate::{ mock::{Test, new_test_ext}, storage::{MerkleRepository, NullifierRepository, PoolBalanceRepository}, @@ -407,11 +417,18 @@ mod tests { sp_core::H160::from([byte; 20]) } - /// Two unshield variants differing only in `relayer` produce different - /// `provides` tag sets, so a spoofed variant is a distinct pool entry and - /// cannot silently replace the honest one. + /// Two unshield variants differing only in `relayer` COLLIDE — they are the + /// same spend of the same note. + /// + /// This inverts the earlier expectation on purpose. Binding the relayer into + /// the tag made a spoofed copy a SEPARATE pool entry, so anyone could + /// rebroadcast an honest unshield pointed at their own account and have both + /// live in the pool: duplicate propagation and revalidation across the whole + /// network, for a copy that cost the attacker nothing. Keyed on the nullifier + /// alone the two are mutually exclusive, and taking the fee requires + /// out-bidding — which means actually paying it. #[test] - fn unshield_relayer_changes_provides_tag() { + fn unshield_relayer_swap_collides_with_the_original() { new_test_ext().execute_with(|| { MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); PoolBalanceRepository::set_asset_balance::(0, 1000u128); @@ -441,8 +458,14 @@ mod tests { validate_unshield::(&KNOWN_ROOT, &n, &0u32, &100u128, &10u128, &None, 1) .unwrap(); - assert_ne!(a.provides, b.provides, "different relayer → different tags"); - assert_ne!(a.provides, none.provides, "Some vs None → different tags"); + assert_eq!( + a.provides, b.provides, + "a relayer-swapped copy is the same spend and must collide" + ); + assert_eq!( + a.provides, none.provides, + "Some vs None relayer is still the same note being spent" + ); // Fee steers priority, not the relayer field. assert_eq!(a.priority, b.priority); }); @@ -481,9 +504,20 @@ mod tests { }); } - /// Same for private_transfer. + /// A relayer-swapped copy of a transfer COLLIDES with the original. + /// + /// This inverts the earlier expectation, deliberately. Binding the relayer + /// into the tag made a copy with a different fee recipient a *separate* pool + /// entry, so a third party could rebroadcast someone else's spend pointed at + /// their own account and have both sit in the pool at once — duplicate load, + /// and a race for the fee that cost the attacker nothing. + /// + /// Tagging per nullifier makes the two mutually exclusive: the higher fee + /// wins (first-seen at equal fee), so out-bidding is the only way to take + /// the fee, and out-bidding means actually paying it. The pool now mirrors + /// the on-chain rule — one note, one spend. #[test] - fn transfer_relayer_changes_provides_tag() { + fn transfer_relayer_swap_collides_with_the_original() { new_test_ext().execute_with(|| { MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); let ns = nullifiers_of(&[0x63]); @@ -494,7 +528,7 @@ mod tests { let b = validate_private_transfer::(&KNOWN_ROOT, &ns, &10u128, &Some(evm(0xBB)), 1) .unwrap(); - assert_ne!(a.provides, b.provides); + assert_eq!(a.provides, b.provides); }); } @@ -579,4 +613,397 @@ mod tests { ); }); } + + // ── adversarial: mempool tag manipulation ──────────────────────────────── + // + // The `provides` tag decides which pool entries are mutually exclusive. + // Getting it wrong is not a crash — it is censorship or fee theft: an + // attacker who can mint a colliding variant of someone else's transaction + // can displace it, and one who can mint NON-colliding variants of the same + // spend can flood the pool with entries that all spend one note. + + /// Two transactions spending the SAME note must be mutually exclusive in the + /// pool. If their tags differ, both sit in the pool and the second is dead + /// weight the node still gossips and validates. + #[test] + fn attack_same_nullifier_different_root_still_collides_in_the_pool() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + let other_root = [0x22u8; 32]; + MerkleRepository::add_historic_poseidon_root::(other_root); + + let nulls = nullifiers_of(&[0x42]); + let a = + validate_private_transfer::(&KNOWN_ROOT, &nulls, &0u128, &None, 1).unwrap(); + let b = + validate_private_transfer::(&other_root, &nulls, &0u128, &None, 1).unwrap(); + + assert_eq!( + a.provides, b.provides, + "same note spent twice must produce the same tag, whatever the root" + ); + }); + } + + /// Fee-hijack attempt: a third party rebroadcasts someone else's spend with + /// the relayer swapped to themselves. The two must be MUTUALLY EXCLUSIVE in + /// the pool (same nullifier tag) so both can never sit there at once — + /// otherwise the network carries a duplicate of every transfer. + #[test] + fn attack_swapping_the_relayer_cannot_add_a_second_pool_entry() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + let nulls = nullifiers_of(&[0x43]); + + let honest = + validate_private_transfer::(&KNOWN_ROOT, &nulls, &0u128, &None, 1).unwrap(); + let hijacked = validate_private_transfer::( + &KNOWN_ROOT, + &nulls, + &0u128, + &Some(sp_core::H160::repeat_byte(0xEE)), + 1, + ) + .unwrap(); + + assert_eq!( + honest.provides, hijacked.provides, + "a relayer-swapped copy must collide with the original, not coexist" + ); + }); + } + + /// Dummy nullifiers carry no identity. Two DIFFERENT real spends that each + /// pad with a dummy must not be forced to collide through the dummy. + #[test] + fn attack_dummy_padding_does_not_make_unrelated_spends_collide() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let mut a_nulls: BoundedVec> = BoundedVec::new(); + a_nulls.try_push(make_nullifier(0x51)).unwrap(); + a_nulls.try_push(Nullifier::new([0u8; 32])).unwrap(); + + let mut b_nulls: BoundedVec> = BoundedVec::new(); + b_nulls.try_push(make_nullifier(0x52)).unwrap(); + b_nulls.try_push(Nullifier::new([0u8; 32])).unwrap(); + + let a = + validate_private_transfer::(&KNOWN_ROOT, &a_nulls, &0u128, &None, 1).unwrap(); + let b = + validate_private_transfer::(&KNOWN_ROOT, &b_nulls, &0u128, &None, 1).unwrap(); + + assert_ne!( + a.provides, b.provides, + "unrelated spends must not collide just because both padded with a dummy" + ); + }); + } + + /// Reordering the two inputs of the SAME spend must not mint a second pool + /// entry — otherwise one note yields two admissible transactions. + #[test] + fn attack_reordering_inputs_does_not_mint_a_second_pool_entry() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let ab = nullifiers_of(&[0x61, 0x62]); + let ba = nullifiers_of(&[0x62, 0x61]); + + let a = validate_private_transfer::(&KNOWN_ROOT, &ab, &0u128, &None, 1).unwrap(); + let b = validate_private_transfer::(&KNOWN_ROOT, &ba, &0u128, &None, 1).unwrap(); + + let mut a_tags = a.provides.clone(); + let mut b_tags = b.provides.clone(); + a_tags.sort(); + b_tags.sort(); + assert_eq!( + a_tags, b_tags, + "the same pair of notes must produce the same tag set in any order" + ); + }); + } + + /// Priority is the fee. An attacker must not be able to outrank an honest + /// transaction without actually paying more. + #[test] + fn attack_priority_tracks_the_fee_and_cannot_be_forged() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + let nulls = nullifiers_of(&[0x71]); + + let cheap = + validate_private_transfer::(&KNOWN_ROOT, &nulls, &10u128, &None, 1).unwrap(); + let rich = validate_private_transfer::(&KNOWN_ROOT, &nulls, &1_000u128, &None, 1) + .unwrap(); + + assert!( + rich.priority > cheap.priority, + "a higher fee must buy higher priority, or fee bidding is broken" + ); + assert_eq!( + cheap.longevity, TX_LONGEVITY, + "longevity must not vary with fee" + ); + assert_eq!(rich.longevity, TX_LONGEVITY); + }); + } + + /// A spent note must be refused at ADMISSION, not merely at execution: + /// otherwise every node re-validates and gossips a transaction that can + /// never succeed. + #[test] + fn attack_spent_note_is_refused_at_pool_admission() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + let n = make_nullifier(0x81); + NullifierRepository::mark_as_used::(n, 1u64); + + let result = validate_private_transfer::( + &KNOWN_ROOT, + &nullifiers_of(&[0x81]), + &0u128, + &None, + 1, + ); + assert_eq!( + result.unwrap_err(), + sp_runtime::transaction_validity::TransactionValidityError::Invalid( + sp_runtime::transaction_validity::InvalidTransaction::Stale + ), + ); + }); + } + + /// THE REGRESSION THIS SUITE EXISTS FOR. + /// + /// Two transfers that share only ONE input note (A+B and A+C) must be + /// mutually exclusive: note A can back exactly one pool entry. When the tag + /// was a single blob over the whole nullifier set, these did not collide, + /// so one note could back unboundedly many admissible transactions — free + /// mempool amplification, since the fee is only charged on execution. + #[test] + fn attack_transfers_sharing_one_note_are_mutually_exclusive() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let ab = nullifiers_of(&[0x61, 0x62]); + let ac = nullifiers_of(&[0x61, 0x63]); + + let a = validate_private_transfer::(&KNOWN_ROOT, &ab, &0u128, &None, 1).unwrap(); + let b = validate_private_transfer::(&KNOWN_ROOT, &ac, &0u128, &None, 1).unwrap(); + + let shared = a.provides.iter().any(|t| b.provides.contains(t)); + assert!( + shared, + "spends sharing note A must share a tag, or A backs two pool entries" + ); + }); + } + + /// Each real nullifier contributes its OWN tag — the property every + /// exclusion guarantee above rests on. A single concatenated tag silently + /// breaks all of them, so pin the cardinality directly. + #[test] + fn attack_each_nullifier_contributes_an_independent_tag() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + let one = validate_private_transfer::( + &KNOWN_ROOT, + &nullifiers_of(&[0x91]), + &0u128, + &None, + 1, + ) + .unwrap(); + assert_eq!(one.provides.len(), 1, "one real input → one tag"); + + let two = validate_private_transfer::( + &KNOWN_ROOT, + &nullifiers_of(&[0x92, 0x93]), + &0u128, + &None, + 1, + ) + .unwrap(); + assert_eq!( + two.provides.len(), + 2, + "two real inputs → two independent tags" + ); + + // A dummy-padded single input must still yield exactly one tag. + let mut padded: BoundedVec> = BoundedVec::new(); + padded.try_push(make_nullifier(0x94)).unwrap(); + padded.try_push(Nullifier::new([0u8; 32])).unwrap(); + let p = + validate_private_transfer::(&KNOWN_ROOT, &padded, &0u128, &None, 1).unwrap(); + assert_eq!(p.provides.len(), 1, "the dummy must not contribute a tag"); + }); + } + /// A transfer and an unshield spending the SAME note must be mutually + /// exclusive in the pool. + /// + /// They used to carry different tag prefixes, so one of each could sit in + /// the pool for a single note: both propagate and get revalidated by every + /// node, while at most one can execute. A nullifier names a NOTE, not an + /// operation, so both now share one tag namespace. + #[test] + fn attack_transfer_and_unshield_of_the_same_note_are_mutually_exclusive() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + PoolBalanceRepository::set_asset_balance::(0, 100_000u128); + let n = make_nullifier(0x77); + + let transfer = validate_private_transfer::( + &KNOWN_ROOT, + &nullifiers_of(&[0x77]), + &10u128, + &None, + 1, + ) + .unwrap(); + let unshield = + validate_unshield::(&KNOWN_ROOT, &n, &0u32, &100u128, &10u128, &None, 1) + .unwrap(); + + assert_eq!( + transfer.provides, unshield.provides, + "one note must back one pool entry, whichever operation spends it" + ); + }); + } + + // ── Solvency arithmetic ────────────────────────────────────────────────── + // + // `amount + fee` is attacker-chosen and summed before the balance compare. + // A wrapping sum comes out SMALL, which passes the compare — so the overflow + // branch is what stops an unbackable spend from being gossiped. + + /// `amount + fee` overflowing `Balance` must be refused, not wrapped. + /// + /// Both operands come from the caller, so the sum is reachable: picking + /// `amount = MAX` and any non-zero fee wraps to a tiny total that clears the + /// pool-balance check. The result must be AMOUNT_OVERFLOW, never admission. + #[test] + fn attack_amount_plus_fee_overflow_is_refused_not_wrapped() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + // A pool holding almost nothing — a wrapped total would still clear it. + PoolBalanceRepository::set_asset_balance::(0, 1u128); + let n = make_nullifier(0x99); + + let got = validate_unshield::( + &KNOWN_ROOT, + &n, + &0u32, + &u128::MAX, + &1u128, // MAX + 1 wraps + &None, + 1, + ); + + assert_eq!( + got, + Err(codes::reject(codes::AMOUNT_OVERFLOW).into()), + "a wrapping sum would admit a spend the pool cannot cover" + ); + }); + } + + /// The solvency check is `<`, so a spend of exactly the pool balance is + /// admissible and one planck more is not. Pins the boundary an attacker + /// probes for free, since admission costs nothing until execution. + #[test] + fn attack_solvency_boundary_is_exact() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + PoolBalanceRepository::set_asset_balance::(0, 1_000u128); + + // amount + fee == balance exactly. + let exact = validate_unshield::( + &KNOWN_ROOT, + &make_nullifier(0xA1), + &0u32, + &900u128, + &100u128, + &None, + 1, + ); + assert!( + exact.is_ok(), + "draining the pool exactly must be admissible" + ); + + // One planck over. + let over = validate_unshield::( + &KNOWN_ROOT, + &make_nullifier(0xA2), + &0u32, + &901u128, + &100u128, + &None, + 1, + ); + assert_eq!( + over, + Err(codes::reject(codes::INSUFFICIENT_POOL_BALANCE).into()) + ); + }); + } + + /// The fee counts against the pool, not just the amount. + /// + /// Both leave the pool on execution, so a spend whose amount alone fits but + /// whose amount+fee does not must be refused — otherwise the fee is paid out + /// of a balance that was never there. + #[test] + fn attack_fee_counts_against_pool_solvency() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + PoolBalanceRepository::set_asset_balance::(0, 1_000u128); + + // amount == balance, leaving nothing for the fee. + let got = validate_unshield::( + &KNOWN_ROOT, + &make_nullifier(0xA3), + &0u32, + &1_000u128, + &100u128, + &None, + 1, + ); + assert_eq!( + got, + Err(codes::reject(codes::INSUFFICIENT_POOL_BALANCE).into()), + "amount alone fits, but the fee also leaves the pool" + ); + }); + } + + /// Solvency is tracked per asset: a rich asset must not underwrite a spend + /// against an empty one. + #[test] + fn attack_other_asset_balance_does_not_underwrite_this_one() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + PoolBalanceRepository::set_asset_balance::(0, u128::MAX / 2); + // Asset 1 holds nothing. + + let got = validate_unshield::( + &KNOWN_ROOT, + &make_nullifier(0xA4), + &1u32, + &1_000u128, + &100u128, + &None, + 1, + ); + assert_eq!( + got, + Err(codes::reject(codes::INSUFFICIENT_POOL_BALANCE).into()) + ); + }); + } } diff --git a/frame/shielded-pool/src/validate_unsigned/transfer.rs b/frame/shielded-pool/src/validate_unsigned/transfer.rs index 400ae412..294e8228 100644 --- a/frame/shielded-pool/src/validate_unsigned/transfer.rs +++ b/frame/shielded-pool/src/validate_unsigned/transfer.rs @@ -1,8 +1,27 @@ //! Pool admission for `private_transfer`. //! -//! Checks run cheapest-first — a version lookup, a fee compare, then two point -//! reads — so flooding the pool with invalid transactions stays cheap to reject. -//! No ZK verification happens here; that is the extrinsic's job. +//! No ZK verification happens here; that is the extrinsic's job. Verifying a +//! proof at admission would let anyone burn a node's CPU for free, since +//! unsigned submissions cost nothing to make. +//! +//! ## Order of checks +//! +//! The steps below are numbered, and the order is the anti-spam property, not a +//! style choice: each step is more expensive than the last, so a junk +//! transaction is rejected as early — and as cheaply — as possible. +//! +//! | # | Check | Cost | +//! |---|------------------------|-----------------------------| +//! | 1 | circuit version | in-memory lookup | +//! | 2 | fee floor | one storage read + compare | +//! | 3 | Merkle root known | one storage read | +//! | 4 | nullifiers not spent | up to two storage reads | +//! | 5 | not all-dummy | in-memory scan | +//! | 6 | build the pool tags | no reads | +//! +//! Every check here is ALSO re-done in the dispatchable. That is deliberate: a +//! check performed only at admission could be skipped by a malicious block +//! author, so admission may reject more than execution — never less. use super::{ TX_LONGEVITY, @@ -16,7 +35,6 @@ use crate::{ use frame_support::pallet_prelude::*; use pallet_relayer::RelayerInterface as _; use pallet_zk_verifier::ZkVerifierPort as _; -use parity_scale_codec::Encode; use sp_runtime::{ SaturatedConversion, transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction}, @@ -26,61 +44,89 @@ use sp_runtime::{ /// zk-verifier's `CircuitId` constants). const CIRCUIT_TRANSFER: u32 = 1; +/// `_relayer` is intentionally unused: it is part of the call and reaches the +/// dispatchable, but it must NOT enter the pool tag. Binding it made a copy with +/// a swapped fee recipient a separate pool entry, so anyone could duplicate an +/// honest transaction at no cost. Kept in the signature so the caller in +/// `lib.rs` stays a faithful mirror of the call's fields. pub fn validate_private_transfer( merkle_root: &Hash, nullifiers: &BoundedVec>, fee: &BalanceOf, - relayer: &Option, + _relayer: &Option, circuit_version: u32, ) -> TransactionValidity { - // Anti-spam: reject an unsupported circuit version before pool admission. + // ── 1. Circuit version ─────────────────────────────────────────────────── + // Cheapest gate first: a transaction proving against a retired circuit can + // never execute, so it must not reach the pool at all. if !T::ZkVerifier::is_supported_version(CIRCUIT_TRANSFER, circuit_version) { return reject(codes::UNSUPPORTED_CIRCUIT_VERSION).into(); } - // Anti-spam: fee must meet minimum relay fee + // ── 2. Fee floor ───────────────────────────────────────────────────────── + // The pool's price of entry. Submissions are unsigned and gasless, so this + // is what stops an attacker from filling it for nothing. let min_fee: BalanceOf = T::Relayer::min_relay_fee().saturated_into(); if *fee < min_fee { return InvalidTransaction::Payment.into(); } - // Reject unknown Merkle roots + // ── 3. Merkle root ─────────────────────────────────────────────────────── + // An unknown root cannot verify, and the retention window is sized to + // outlive `TX_LONGEVITY` so a root accepted here stays valid until the + // transaction expires. if !MerkleRepository::is_known_root::(merkle_root) { return reject(codes::UNKNOWN_ROOT).into(); } - // Reject already-spent nullifiers (skip dummy nullifiers — value zero, forced by circuit) + // ── 4. Nullifiers not already spent ────────────────────────────────────── + // The dummy nullifier (all zeros) pads a single-input spend. It is never + // inserted into the set, so it can never be stale — skipping it is required, + // not an optimisation: treating it as spent would reject every one-input + // transfer after the first. for nullifier in nullifiers.iter() { if nullifier.0 == [0u8; 32] { - continue; // dummy input — never inserted in the set, cannot be stale + continue; } if NullifierSet::::contains_key(nullifier) { return InvalidTransaction::Stale.into(); } } - // Reject transactions where all nullifiers are dummy (both inputs value=0). - // This prevents free Merkle tree spam (2 commitments inserted at zero cost). + // ── 5. At least one real input ─────────────────────────────────────────── + // All-dummy means no note is being spent, so the transfer would insert two + // commitments at zero cost — free Merkle tree growth. if nullifiers.iter().all(|n| n.0 == [0u8; 32]) { return reject(codes::ALL_INPUTS_DUMMY).into(); } - // Exclude dummy nullifiers (zero) from provides — they carry no identity. - // Bind the fee recipient (`relayer`) into the tag so a variant differing only - // in `relayer` is a distinct pool entry and cannot silently replace the honest - // tx. The shared nullifier tag already makes same-nullifier variants mutually - // exclusive (first-seen wins at equal fee); this hardens that boundary. - let mut provides: alloc::vec::Vec> = nullifiers - .iter() - .filter(|n| n.0 != [0u8; 32]) - .map(|n| n.encode()) - .collect(); - provides.push(relayer.encode()); - - ValidTransaction::with_tag_prefix("ShieldedPoolTransfer") + // ── 6. Pool tags — ONE PER NULLIFIER, never one over the whole set ─────── + // + // `and_provides(x)` contributes exactly ONE tag: passing a `Vec>` + // encodes the entire vector into one blob. Doing that made the tag depend on + // the ORDER of the inputs and on the OTHER note in the pair, so: + // - reordering the two inputs minted a second admissible entry for the + // same spend, and + // - two transfers sharing only one note (A+B and A+C) did not collide at + // all, letting one note back an unbounded number of pool entries. + // Since the fee is only charged on execution, that was free mempool + // amplification: every variant propagates and is revalidated network-wide + // while at most one can ever execute. + // + // Calling `and_provides` once PER nullifier makes any two transactions that + // share a note mutually exclusive, in any order — which is what makes the + // pool mirror the on-chain nullifier set. + // + // Dummy nullifiers (zero) are excluded: they carry no identity, and tagging + // them would collide every padded single-input spend with every other. + let mut builder = ValidTransaction::with_tag_prefix(super::SPEND_TAG_PREFIX) .priority((*fee).saturated_into()) .longevity(TX_LONGEVITY) - .and_provides(provides) - .propagate(true) - .build() + .propagate(true); + + for nullifier in nullifiers.iter().filter(|n| n.0 != [0u8; 32]) { + builder = builder.and_provides(nullifier); + } + + builder.build() } diff --git a/frame/shielded-pool/src/validate_unsigned/unshield.rs b/frame/shielded-pool/src/validate_unsigned/unshield.rs index 2cda0cce..04516158 100644 --- a/frame/shielded-pool/src/validate_unsigned/unshield.rs +++ b/frame/shielded-pool/src/validate_unsigned/unshield.rs @@ -1,9 +1,32 @@ //! Pool admission for `unshield`. //! -//! Mirrors [`super::transfer`], plus a pool-solvency check. That check is -//! advisory only: the balance can move between admission and execution, so the -//! extrinsic re-verifies it. Rejecting early just avoids gossiping a spend the -//! pool cannot cover. +//! Mirrors [`super::transfer`] step for step, with one extra check: unshield is +//! the only call that moves value OUT of the pool, so it also verifies the pool +//! can cover it. +//! +//! ## Order of checks +//! +//! The steps below are numbered, and the order is the anti-spam property, not a +//! style choice: each step is more expensive than the last, so a junk +//! transaction is rejected as early — and as cheaply — as possible. Steps 1–4 +//! are identical to `transfer`; step 5 is unshield's own. +//! +//! | # | Check | Cost | +//! |---|----------------------|----------------------------| +//! | 1 | circuit version | in-memory lookup | +//! | 2 | fee floor | one storage read + compare | +//! | 3 | Merkle root known | one storage read | +//! | 4 | nullifier not spent | one storage read | +//! | 5 | pool can cover it | one storage read + add | +//! | 6 | build the pool tag | no reads | +//! +//! Step 5 is ADVISORY: the balance can move between admission and execution, so +//! the extrinsic re-verifies it. Rejecting here only avoids gossiping a spend +//! the pool visibly cannot cover. +//! +//! Every check here is ALSO re-done in the dispatchable. That is deliberate: a +//! check performed only at admission could be skipped by a malicious block +//! author, so admission may reject more than execution — never less. use super::{ TX_LONGEVITY, @@ -17,7 +40,6 @@ use crate::{ use frame_support::pallet_prelude::*; use pallet_relayer::RelayerInterface as _; use pallet_zk_verifier::ZkVerifierPort as _; -use parity_scale_codec::Encode; use sp_runtime::{ SaturatedConversion, transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction}, @@ -28,37 +50,55 @@ use sp_runtime::{ const CIRCUIT_UNSHIELD: u32 = 2; /// Validate an incoming `unshield` unsigned transaction. +/// +/// `_relayer` is intentionally unused — see the note in `transfer.rs`: the fee +/// recipient must not enter the pool tag, or a spoofed copy becomes a separate +/// pool entry instead of colliding with the original. pub fn validate_unshield( merkle_root: &Hash, nullifier: &Nullifier, asset_id: &u32, amount: &BalanceOf, fee: &BalanceOf, - relayer: &Option, + _relayer: &Option, circuit_version: u32, ) -> TransactionValidity { - // Anti-spam: reject an unsupported circuit version before pool admission. + // ── 1. Circuit version ─────────────────────────────────────────────────── + // Cheapest gate first: a transaction proving against a retired circuit can + // never execute, so it must not reach the pool at all. if !T::ZkVerifier::is_supported_version(CIRCUIT_UNSHIELD, circuit_version) { return reject(codes::UNSUPPORTED_CIRCUIT_VERSION).into(); } - // Anti-spam: fee must meet minimum relay fee + // ── 2. Fee floor ───────────────────────────────────────────────────────── + // The pool's price of entry. Submissions are unsigned and gasless, so this + // is what stops an attacker from filling it for nothing. let min_fee: BalanceOf = T::Relayer::min_relay_fee().saturated_into(); if *fee < min_fee { return InvalidTransaction::Payment.into(); } - // Reject unknown Merkle roots + // ── 3. Merkle root ─────────────────────────────────────────────────────── + // An unknown root cannot verify, and the retention window is sized to + // outlive `TX_LONGEVITY` so a root accepted here stays valid until the + // transaction expires. if !MerkleRepository::is_known_root::(merkle_root) { return reject(codes::UNKNOWN_ROOT).into(); } - // Reject already-spent nullifier + // ── 4. Nullifier not already spent ─────────────────────────────────────── + // Unlike `transfer`, unshield has exactly one input and no dummy padding — + // so there is no zero sentinel to skip here. if NullifierSet::::contains_key(nullifier) { return InvalidTransaction::Stale.into(); } - // Reject if pool balance is insufficient + // ── 5. Pool solvency (unshield only) ───────────────────────────────────── + // `amount + fee` both leave the pool, so both count against its balance. + // The addition is CHECKED: a wrapping sum would produce a small total that + // passes the comparison below, admitting a spend the pool cannot cover. + // Advisory — the balance can move before execution, so the extrinsic checks + // it again (see the module header). let total = amount .checked_add(fee) .ok_or(reject(codes::AMOUNT_OVERFLOW))?; @@ -66,14 +106,18 @@ pub fn validate_unshield( return reject(codes::INSUFFICIENT_POOL_BALANCE).into(); } - // Bind `relayer` into the tag alongside the nullifier: a variant differing only - // in the fee recipient is a distinct pool entry, so it cannot silently replace - // the honest tx. Same-nullifier variants stay mutually exclusive (first-seen - // wins at equal fee). - ValidTransaction::with_tag_prefix("ShieldedPoolUnshield") + // ── 6. Pool tag — the NULLIFIER ALONE: one note, one pool entry ────────── + // + // `relayer` used to be concatenated in, which made a copy differing only in + // the fee recipient a SEPARATE entry: anyone could rebroadcast someone + // else's unshield pointed at their own account and have both sit in the pool, + // racing for a fee the copy never paid for. Keyed on the nullifier the two + // are mutually exclusive, so taking the fee requires out-bidding — which + // means actually paying it. Mirrors `transfer.rs`. + ValidTransaction::with_tag_prefix(super::SPEND_TAG_PREFIX) .priority((*fee).saturated_into()) .longevity(TX_LONGEVITY) - .and_provides([nullifier.encode(), relayer.encode()]) + .and_provides(nullifier) .propagate(true) .build() } diff --git a/ts-tests/tests/test-relay-rpc.ts b/ts-tests/tests/test-relay-rpc.ts index afb92c73..ebe41d85 100644 --- a/ts-tests/tests/test-relay-rpc.ts +++ b/ts-tests/tests/test-relay-rpc.ts @@ -14,9 +14,17 @@ const MIN_RELAY_FEE = ethers.parseUnits("0.001", 18); /// EVM address derived from GENESIS_ACCOUNT_PRIVATE_KEY (lower‑case, with 0x) const RELAYER_ADDRESS = "0x6be02d1d3665660d22ff9624b7be0551ee1ac91b"; -/// Verified function selectors (keccak256 of ABI signature, first 4 bytes) -const SEL_UNSHIELD = "47fc44a2"; -const SEL_PRIVATE_TRANSFER = "8c0f5d24"; +/// Function selectors, derived below from the ABI signatures rather than +/// hardcoded. A stale copy here fails silently: the tests keep passing because +/// 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 SEL_UNSHIELD = ethers.id(SIG_UNSHIELD).slice(2, 10); +const SEL_PRIVATE_TRANSFER = ethers.id(SIG_PRIVATE_TRANSFER).slice(2, 10); // --------------------------------------------------------------------------- // Calldata builders @@ -25,15 +33,29 @@ const SEL_PRIVATE_TRANSFER = "8c0f5d24"; const abiCoder = ethers.AbiCoder.defaultAbiCoder(); /** - * Build ABI-encoded calldata for `unshield(bytes,bytes32,bytes32,uint32,uint256,bytes32,uint256)` - * with the given relay fee inserted as the 7th argument. + * Build ABI-encoded calldata for `unshield(...)` with the given relay fee. + * + * The argument list mirrors SIG_UNSHIELD exactly — the head is 10 slots, and the + * relay's `min_calldata_len` (324 = 4 + 10×32) is derived from that. Encoding a + * shorter argument list here would build calldata the relay rightly refuses. * * ABI head layout after prepending the selector: * data[196..228] = slot 6 = uint256 fee ← the value relay.rs reads */ function buildUnshieldCalldata(fee: bigint): string { const encoded = abiCoder.encode( - ["bytes", "bytes32", "bytes32", "uint32", "uint256", "bytes32", "uint256"], + [ + "bytes", + "bytes32", + "bytes32", + "uint32", + "uint256", + "bytes32", + "uint256", + "bytes32", + "bytes", + "uint32", + ], [ "0x" + "aa".repeat(32), // proof (32 dummy bytes) "0x" + "bb".repeat(32), // merkle root @@ -42,21 +64,25 @@ function buildUnshieldCalldata(fee: bigint): string { ethers.parseEther("1"), // amount "0x" + "00".repeat(32), // recipient (AccountId32 as bytes32) fee, // relay fee + "0x" + "00".repeat(32), // change commitment (total unshield → zero) + "0x", // change encrypted memo (empty for total unshield) + 1, // circuit version ] ); return "0x" + SEL_UNSHIELD + encoded.slice(2); } /** - * Build ABI-encoded calldata for - * `privateTransfer(bytes,bytes32,bytes32[],bytes32[],bytes[],uint32,uint256)` - * with the given relay fee as the 7th argument. + * Build ABI-encoded calldata for `privateTransfer(...)` with the given relay fee. + * + * Mirrors SIG_PRIVATE_TRANSFER: 8 head slots, so `min_calldata_len` is + * 260 = 4 + 8×32. * * ABI head layout: data[196..228] = slot 6 = uint256 fee */ function buildPrivateTransferCalldata(fee: bigint): string { const encoded = abiCoder.encode( - ["bytes", "bytes32", "bytes32[]", "bytes32[]", "bytes[]", "uint32", "uint256"], + ["bytes", "bytes32", "bytes32[]", "bytes32[]", "bytes[]", "uint32", "uint256", "uint32"], [ "0x" + "aa".repeat(32), // proof "0x" + "bb".repeat(32), // merkle root @@ -65,6 +91,7 @@ function buildPrivateTransferCalldata(fee: bigint): string { ["0x" + "ee".repeat(104)], // encrypted memos[] 0, // assetId fee, // relay fee + 1, // circuit version ] ); return "0x" + SEL_PRIVATE_TRANSFER + encoded.slice(2);