diff --git a/Cargo.lock b/Cargo.lock index ad49c864..6d38f78e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11179,7 +11179,7 @@ dependencies = [ [[package]] name = "pallet-evm-precompile-balances" -version = "0.1.0" +version = "0.2.0" dependencies = [ "fp-evm", "frame-support", @@ -11188,6 +11188,7 @@ dependencies = [ "pallet-evm", "pallet-timestamp", "parity-scale-codec", + "precompile-utils", "scale-info", "sp-core", "sp-io", diff --git a/frame/evm/precompile/balances/CHANGELOG.md b/frame/evm/precompile/balances/CHANGELOG.md new file mode 100644 index 00000000..eda905fc --- /dev/null +++ b/frame/evm/precompile/balances/CHANGELOG.md @@ -0,0 +1,45 @@ +# Changelog + +All notable changes to `pallet-evm-precompile-balances` will be documented in this file. + +## [Unreleased] + +## [0.2.0] - 2026-10-16 + +**Breaking** — gas is charged by dispatch weight and every failure reverts with +a reason. Requires the next runtime `spec_version`. + +### Security + +- Static calls, delegated calls (DELEGATECALL, CALLCODE) and calls carrying + value are refused. A transfer debits `context.caller`, which under + DELEGATECALL is the delegating contract's caller: any contract a user called + could move that user's ORB through `0x…0802`. Value sent to the precompile + was stranded in its account. + +### Changed + +- **Gas follows the dispatch weight.** A call is charged the weight of + `transfer_allow_death` or `transfer_keep_alive`, its proof size and the + storage a new account takes, before the transfer runs, so a failing transfer + costs the same as a successful one. The flat 5,000 gas, which metered neither + proof size nor storage, is gone. On Orbinum the proof size dominates: a + direct transfer transaction uses about 60,000 gas. +- **Failures revert with a reason.** Every failure reverts with + `Error(string)` and returns the unused gas, instead of an exceptional halt + that consumed all of it. A failed transfer reverts with + `transfer failed: `. +- **Calldata length is exact.** Input must be the selector plus one `bytes32` + and one `uint256` (68 bytes); shorter or longer input reverts. +- The precompile is split into `abi` (decoding), `guard` (caller rules) and + `dispatch` (charging and the transfer). + +## [0.1.0] - 2026-05-29 + +### Added + +- Native `pallet_balances` transfers to any `AccountId32` from EVM contracts and + wallets, at `0x…0802`: `transfer(bytes32,uint256)` (`0x6a467394`) and + `transferKeepAlive(bytes32,uint256)` (`0xac6ac4c4`). The `bytes32` is the + recipient's raw `AccountId32`; a `uint256` above `u128::MAX` is rejected. +- A flat 5,000 gas per call. diff --git a/frame/evm/precompile/balances/Cargo.toml b/frame/evm/precompile/balances/Cargo.toml index 933a26ca..1b600f8c 100644 --- a/frame/evm/precompile/balances/Cargo.toml +++ b/frame/evm/precompile/balances/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-evm-precompile-balances" -version = "0.1.0" +version = "0.2.0" authors = { workspace = true } edition = "2021" description = "EVM Precompile for native Balances transfers to arbitrary AccountId32 destinations." @@ -20,6 +20,7 @@ pallet-balances = { workspace = true } # Frontier EVM fp-evm = { workspace = true } pallet-evm = { workspace = true } +precompile-utils = { workspace = true } [dev-dependencies] frame-system = { workspace = true, features = ["default"] } @@ -42,4 +43,5 @@ std = [ "fp-evm/std", "pallet-evm/std", "pallet-balances/std", + "precompile-utils/std", ] diff --git a/frame/evm/precompile/balances/src/abi.rs b/frame/evm/precompile/balances/src/abi.rs new file mode 100644 index 00000000..44a03f81 --- /dev/null +++ b/frame/evm/precompile/balances/src/abi.rs @@ -0,0 +1,57 @@ +//! Calldata decoding: the two selectors and their fixed-size arguments. + +use crate::revert; +use fp_evm::PrecompileFailure; + +// Verify with: cast sig "transfer(bytes32,uint256)" +// cast sig "transferKeepAlive(bytes32,uint256)" + +/// `transfer(bytes32,uint256)` → 0x6a467394 +pub const SEL_TRANSFER: [u8; 4] = [0x6a, 0x46, 0x73, 0x94]; +/// `transferKeepAlive(bytes32,uint256)` → 0xac6ac4c4 +pub const SEL_TRANSFER_KEEP_ALIVE: [u8; 4] = [0xac, 0x6a, 0xc4, 0xc4]; + +/// Selector + `bytes32` + `uint256`. Both functions take exactly this; longer +/// calldata is refused rather than silently ignored. +const INPUT_LEN: usize = 4 + 32 + 32; + +/// A decoded transfer: who receives, how much, and whether the sender must stay +/// alive. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct TransferCall { + pub dest: [u8; 32], + pub value: u128, + pub keep_alive: bool, +} + +pub(crate) fn decode(input: &[u8]) -> Result { + let selector: [u8; 4] = input + .get(..4) + .and_then(|s| s.try_into().ok()) + .ok_or_else(|| revert("input too short: missing selector"))?; + let keep_alive = match selector { + SEL_TRANSFER => false, + SEL_TRANSFER_KEEP_ALIVE => true, + _ => return Err(revert("unknown selector")), + }; + if input.len() != INPUT_LEN { + return Err(revert("calldata must be selector + bytes32 + uint256")); + } + + let mut dest = [0u8; 32]; + dest.copy_from_slice(&input[4..36]); + + // uint256 big-endian; the high 16 bytes must be zero for it to fit in u128. + let word = &input[36..68]; + if word[..16].iter().any(|&b| b != 0) { + return Err(revert("value overflows u128")); + } + let mut low = [0u8; 16]; + low.copy_from_slice(&word[16..]); + + Ok(TransferCall { + dest, + value: u128::from_be_bytes(low), + keep_alive, + }) +} diff --git a/frame/evm/precompile/balances/src/dispatch.rs b/frame/evm/precompile/balances/src/dispatch.rs new file mode 100644 index 00000000..da41d4f5 --- /dev/null +++ b/frame/evm/precompile/balances/src/dispatch.rs @@ -0,0 +1,63 @@ +//! Runs a decoded transfer as a `pallet_balances` call from the EVM caller. + +use crate::{abi::TransferCall, revert}; +use alloc::format; +use fp_evm::{ExitSucceed, PrecompileHandle, PrecompileOutput, PrecompileResult}; +use frame_support::dispatch::{GetDispatchInfo, PostDispatchInfo}; +use pallet_evm::{AddressMapping, GasWeightMapping}; +use sp_runtime::traits::{Dispatchable, StaticLookup}; + +/// Storage a transfer may add: a new `System::Account` entry (key and value), +/// the same figure Frontier's balance precompiles charge. +pub const NEW_ACCOUNT_STORAGE_GROWTH: u64 = 148; + +/// Dispatches `transfer_allow_death` / `transfer_keep_alive` signed by the caller. +/// +/// The call's weight (as gas), proof size and storage growth are recorded BEFORE +/// it runs: charging after a successful dispatch only would make every failing +/// transfer free, an unmetered loop for a spammer. +pub(crate) fn transfer( + handle: &mut impl PrecompileHandle, + call: TransferCall, +) -> PrecompileResult +where + T: pallet_evm::Config + pallet_balances::Config, + T::AccountId: From<[u8; 32]>, + ::Balance: TryFrom, + pallet_evm::AccountIdOf: Into<::AccountId>, + ::RuntimeCall: Dispatchable + + GetDispatchInfo + + From>, + <::RuntimeCall as Dispatchable>::RuntimeOrigin: + From::AccountId>>, +{ + let value: ::Balance = call + .value + .try_into() + .map_err(|_| revert("value does not fit the balance type"))?; + let dest = ::Lookup::unlookup(T::AccountId::from(call.dest)); + let balances_call = if call.keep_alive { + pallet_balances::Call::::transfer_keep_alive { dest, value } + } else { + pallet_balances::Call::::transfer_allow_death { dest, value } + }; + let runtime_call = ::RuntimeCall::from(balances_call); + + let weight = runtime_call.get_dispatch_info().total_weight(); + handle.record_cost(T::GasWeightMapping::weight_to_gas(weight))?; + handle.record_external_cost( + None, + Some(weight.proof_size()), + Some(NEW_ACCOUNT_STORAGE_GROWTH), + )?; + + let caller: ::AccountId = + T::AddressMapping::into_account_id(handle.context().caller).into(); + match runtime_call.dispatch(Some(caller).into()) { + Ok(_) => Ok(PrecompileOutput { + exit_status: ExitSucceed::Returned, + output: Default::default(), + }), + Err(e) => Err(revert(&format!("transfer failed: {:?}", e.error))), + } +} diff --git a/frame/evm/precompile/balances/src/guard.rs b/frame/evm/precompile/balances/src/guard.rs new file mode 100644 index 00000000..fb99abc9 --- /dev/null +++ b/frame/evm/precompile/balances/src/guard.rs @@ -0,0 +1,21 @@ +//! The only way to reach this precompile: a direct, non-static, valueless call. + +use crate::revert; +use fp_evm::{PrecompileFailure, PrecompileHandle}; + +/// A transfer debits `context.caller`. Under DELEGATECALL or CALLCODE that is the +/// delegating contract's caller, so any contract a user called could spend the +/// user's ORB; under STATICCALL nothing may change state; value sent here would +/// be stranded in this precompile's account. Each is refused. +pub(crate) fn ensure_direct_call(handle: &impl PrecompileHandle) -> Result<(), PrecompileFailure> { + if handle.is_static() { + return Err(revert("static calls are not allowed")); + } + if handle.code_address() != handle.context().address { + return Err(revert("delegated calls are not allowed")); + } + if !handle.context().apparent_value.is_zero() { + return Err(revert("not payable")); + } + Ok(()) +} diff --git a/frame/evm/precompile/balances/src/lib.rs b/frame/evm/precompile/balances/src/lib.rs index e5f74f2a..4534502a 100644 --- a/frame/evm/precompile/balances/src/lib.rs +++ b/frame/evm/precompile/balances/src/lib.rs @@ -3,24 +3,32 @@ //! Exposes native `pallet_balances` transfers to arbitrary `AccountId32` //! destinations from EVM contracts and wallets. //! -//! ## ABI Selectors +//! ## ABI selectors //! -//! | Function | Selector | -//! |-----------------------------------|------------| -//! | `transfer(bytes32,uint256)` | `0x6a467394` | +//! | Function | Selector | +//! |--------------------------------------|--------------| +//! | `transfer(bytes32,uint256)` | `0x6a467394` | //! | `transferKeepAlive(bytes32,uint256)` | `0xac6ac4c4` | //! -//! The `bytes32` parameter is the raw 32-byte `AccountId32` of the recipient -//! (e.g. a Sr25519 public key or an EVM-suffix account `[H160 | 12×0x00]`). -//! The `uint256` parameter is the amount to transfer; values exceeding -//! `u128::MAX` are rejected. +//! The `bytes32` is the recipient's raw `AccountId32` (an Sr25519 public key, or +//! an EVM-suffix account `[H160 | 12×0x00]`); the `uint256` is the amount and +//! must fit in `u128`. The calldata is exactly selector + the two words. //! -//! ## Gas +//! ## Caller //! -//! A flat base cost of `5_000` gas is charged per call to cover the storage -//! reads and writes performed by `pallet_balances`. +//! The transfer debits `msg.sender`. Static and delegated calls (DELEGATECALL, +//! CALLCODE) are refused, so a contract moves only its own balance, never its +//! caller's; calls carrying value are refused too (`guard`). //! -//! ## Deployment Address +//! ## Gas and failures +//! +//! The call dispatches `Balances::transfer_allow_death` / `transfer_keep_alive` +//! as the caller and is charged that call's weight, its proof size and the +//! storage a new account takes, before it runs — a failing transfer is not free +//! (`dispatch`). Every failure reverts with a Solidity `Error(string)`, so the +//! caller keeps its unused gas and tools decode the reason. +//! +//! ## Deployment address //! //! `hash(2050)` — precompile address `0x…0802` on Orbinum networks. @@ -28,6 +36,10 @@ extern crate alloc; +mod abi; +mod dispatch; +mod guard; + #[cfg(test)] mod mock; #[cfg(test)] @@ -35,33 +47,12 @@ mod tests; use core::marker::PhantomData; -use fp_evm::{ - ExitError, ExitSucceed, Precompile, PrecompileFailure, PrecompileHandle, PrecompileOutput, - PrecompileResult, -}; -use frame_support::traits::{fungible::Mutate, tokens::Preservation}; -use pallet_evm::AddressMapping; -use sp_core::H160; - -// ───────────────────────────────────────────────────────────────────────────── -// ABI Function Selectors -// -// Verify with: cast sig "transfer(bytes32,uint256)" -// cast sig "transferKeepAlive(bytes32,uint256)" -// ───────────────────────────────────────────────────────────────────────────── - -/// `transfer(bytes32,uint256)` → 0x6a467394 -const SEL_TRANSFER: [u8; 4] = [0x6a, 0x46, 0x73, 0x94]; -/// `transferKeepAlive(bytes32,uint256)` → 0xac6ac4c4 -const SEL_TRANSFER_KEEP_ALIVE: [u8; 4] = [0xac, 0x6a, 0xc4, 0xc4]; - -/// Flat base gas cost per call. -const BASE_GAS: u64 = 5_000; - -/// Minimum input length: 4 bytes selector + 32 bytes dest + 32 bytes value. -const MIN_INPUT_LEN: usize = 4 + 32 + 32; +use fp_evm::{Precompile, PrecompileFailure, PrecompileHandle, PrecompileResult}; +use frame_support::dispatch::{GetDispatchInfo, PostDispatchInfo}; +use sp_runtime::traits::Dispatchable; -// ───────────────────────────────────────────────────────────────────────────── +pub use abi::{SEL_TRANSFER, SEL_TRANSFER_KEEP_ALIVE}; +pub use dispatch::NEW_ACCOUNT_STORAGE_GROWTH; pub struct BalancesPrecompile(PhantomData); @@ -71,106 +62,21 @@ where T::AccountId: From<[u8; 32]>, ::Balance: TryFrom, pallet_evm::AccountIdOf: Into<::AccountId>, - pallet_balances::Pallet: - Mutate::Balance>, + ::RuntimeCall: Dispatchable + + GetDispatchInfo + + From>, + <::RuntimeCall as Dispatchable>::RuntimeOrigin: + From::AccountId>>, { fn execute(handle: &mut impl PrecompileHandle) -> PrecompileResult { - // Charge base gas first so DoS attempts are not free. - handle - .record_cost(BASE_GAS) - .map_err(|e| PrecompileFailure::Error { exit_status: e })?; - - let input = handle.input(); - - if input.len() < 4 { - return Err(PrecompileFailure::Error { - exit_status: ExitError::Other("input too short: missing selector".into()), - }); - } - - let selector: [u8; 4] = input[0..4].try_into().unwrap(); - - match selector { - SEL_TRANSFER => Self::do_transfer(handle, false), - SEL_TRANSFER_KEEP_ALIVE => Self::do_transfer(handle, true), - _ => Err(PrecompileFailure::Error { - exit_status: ExitError::Other("unknown selector".into()), - }), - } + guard::ensure_direct_call(handle)?; + let call = abi::decode(handle.input())?; + dispatch::transfer::(handle, call) } } -impl BalancesPrecompile -where - T: pallet_evm::Config + pallet_balances::Config, - T::AccountId: From<[u8; 32]>, - ::Balance: TryFrom, - pallet_evm::AccountIdOf: Into<::AccountId>, - pallet_balances::Pallet: - Mutate::Balance>, -{ - /// Decode input, resolve caller, and perform the balance transfer. - /// - /// Input layout after the 4-byte selector: - /// ```text - /// [ 0.. 32] bytes32 dest — recipient AccountId32 - /// [ 32.. 64] uint256 value — amount (must fit in u128) - /// ``` - fn do_transfer(handle: &mut impl PrecompileHandle, keep_alive: bool) -> PrecompileResult { - let input = handle.input(); - - if input.len() < MIN_INPUT_LEN { - return Err(PrecompileFailure::Error { - exit_status: ExitError::Other("input too short: expected bytes32 + uint256".into()), - }); - } - - // Decode dest (bytes32 → AccountId32). - let dest_bytes: [u8; 32] = input[4..36].try_into().unwrap(); - let dest: T::AccountId = dest_bytes.into(); - - // Decode value (uint256 big-endian, reject anything > u128::MAX). - let value_bytes: [u8; 32] = input[36..68].try_into().unwrap(); - - // The high 16 bytes must all be zero for the value to fit in u128. - if value_bytes[..16].iter().any(|&b| b != 0) { - return Err(PrecompileFailure::Error { - exit_status: ExitError::Other("value overflows u128".into()), - }); - } - let value_u128 = u128::from_be_bytes(value_bytes[16..32].try_into().unwrap()); - - let value: ::Balance = - value_u128 - .try_into() - .map_err(|_| PrecompileFailure::Error { - exit_status: ExitError::Other("value conversion failed".into()), - })?; - - // Resolve the EVM caller to a Substrate AccountId. - let caller_h160: H160 = handle.context().caller; - let caller: T::AccountId = - ::AddressMapping::into_account_id(caller_h160).into(); - - let preservation = if keep_alive { - Preservation::Preserve - } else { - Preservation::Expendable - }; - - as Mutate>::transfer( - &caller, - &dest, - value, - preservation, - ) - .map_err(|e| PrecompileFailure::Error { - exit_status: ExitError::Other(alloc::format!("transfer failed: {e:?}").into()), - })?; - - Ok(PrecompileOutput { - exit_status: ExitSucceed::Returned, - output: alloc::vec![], - }) - } +/// A revert carrying `msg` as a Solidity `Error(string)`: the caller gets its +/// unused gas back and tools decode the reason. +pub(crate) fn revert(msg: &str) -> PrecompileFailure { + precompile_utils::prelude::revert(msg) } diff --git a/frame/evm/precompile/balances/src/mock.rs b/frame/evm/precompile/balances/src/mock.rs index 9bc36ad4..7520ed61 100644 --- a/frame/evm/precompile/balances/src/mock.rs +++ b/frame/evm/precompile/balances/src/mock.rs @@ -170,6 +170,16 @@ pub fn new_test_ext() -> sp_io::TestExternalities { pub(crate) struct MockHandle { pub input: Vec, pub context: Context, + /// The precompile's own address. Equal to `context.address` on a direct call; + /// differs under DELEGATECALL / CALLCODE. + pub code_address: H160, + pub is_static: bool, + /// Gas recorded with `record_cost`. + pub gas_used: u64, + /// Proof size recorded with `record_external_cost`. + pub proof_size: u64, + /// Storage growth recorded with `record_external_cost`. + pub storage_growth: u64, } impl MockHandle { @@ -181,6 +191,11 @@ impl MockHandle { caller: caller(), apparent_value: U256::zero(), }, + code_address: H160::zero(), + is_static: false, + gas_used: 0, + proof_size: 0, + storage_growth: 0, } } } @@ -198,16 +213,19 @@ impl PrecompileHandle for MockHandle { unimplemented!() } - fn record_cost(&mut self, _: u64) -> Result<(), ExitError> { + fn record_cost(&mut self, cost: u64) -> Result<(), ExitError> { + self.gas_used += cost; Ok(()) } fn record_external_cost( &mut self, _ref_time: Option, - _proof_size: Option, - _storage_growth: Option, + proof_size: Option, + storage_growth: Option, ) -> Result<(), ExitError> { + self.proof_size += proof_size.unwrap_or(0); + self.storage_growth += storage_growth.unwrap_or(0); Ok(()) } @@ -222,7 +240,7 @@ impl PrecompileHandle for MockHandle { } fn code_address(&self) -> H160 { - H160::zero() + self.code_address } fn input(&self) -> &[u8] { @@ -238,7 +256,7 @@ impl PrecompileHandle for MockHandle { } fn is_static(&self) -> bool { - false + self.is_static } fn gas_limit(&self) -> Option { diff --git a/frame/evm/precompile/balances/src/tests.rs b/frame/evm/precompile/balances/src/tests.rs deleted file mode 100644 index 74b43636..00000000 --- a/frame/evm/precompile/balances/src/tests.rs +++ /dev/null @@ -1,312 +0,0 @@ -//! Tests for `pallet-evm-precompile-balances`. -//! -//! # Coverage -//! -//! | # | Test | What it verifies | -//! |---|-----------------------------------------------|------------------------------------------------------| -//! | 1 | `selector_too_short_returns_error` | Input < 4 bytes → selector error | -//! | 2 | `unknown_selector_returns_error` | Unrecognised 4-byte selector → error | -//! | 3 | `transfer_input_too_short_returns_error` | Selector OK but params missing → error | -//! | 4 | `transfer_allow_death_updates_balances` | Happy path: balances change correctly | -//! | 5 | `transfer_keep_alive_updates_balances` | Happy path with keep-alive flag | -//! | 6 | `transfer_keep_alive_fails_if_would_kill_sender` | keep-alive prevents draining below ED | -//! | 7 | `transfer_allow_death_drains_full_balance` | allow-death can drain sender to 0 (account reaped) | -//! | 8 | `transfer_to_evm_suffix_account` | Dest is `[H160 | 12×0x00]` — EVM-mapped style | -//! | 9 | `transfer_to_arbitrary_accountid32` | Dest is pure arbitrary 32-byte account (Sr25519) | -//! |10 | `transfer_value_overflow_returns_error` | uint256 > u128::MAX → error | -//! |11 | `transfer_insufficient_balance_returns_error` | Caller has less than requested → error | - -use fp_evm::{ExitSucceed, Precompile}; -use pallet_balances::Pallet as Balances; - -use crate::{ - mock::{bob_account, caller_account, eve_evm_account, new_test_ext, MockHandle, Test}, - BalancesPrecompile, SEL_TRANSFER, SEL_TRANSFER_KEEP_ALIVE, -}; - -// ───────────────────────────────────────────────────────────────────────────── -// Encoding helpers -// ───────────────────────────────────────────────────────────────────────────── - -/// Encode `dest` (32 bytes) + `value` (u128 as uint256 big-endian) after `selector`. -fn encode_transfer(selector: [u8; 4], dest: [u8; 32], value: u128) -> Vec { - let mut input = selector.to_vec(); - input.extend_from_slice(&dest); - // uint256: 16 zero bytes (high) + 16 bytes (u128 big-endian value, low) - input.extend_from_slice(&[0u8; 16]); - input.extend_from_slice(&value.to_be_bytes()); - input -} - -// ───────────────────────────────────────────────────────────────────────────── -// 1. Selector validation -// ───────────────────────────────────────────────────────────────────────────── - -#[test] -fn selector_too_short_returns_error() { - new_test_ext().execute_with(|| { - for len in 0..4 { - let mut handle = MockHandle::new(vec![0xAAu8; len]); - let result = BalancesPrecompile::::execute(&mut handle); - assert!(result.is_err(), "expected error for input of length {len}"); - } - }); -} - -#[test] -fn unknown_selector_returns_error() { - new_test_ext().execute_with(|| { - // 0xDEADBEEF is not a known selector. - let mut handle = MockHandle::new(vec![0xDE, 0xAD, 0xBE, 0xEF]); - let result = BalancesPrecompile::::execute(&mut handle); - assert!(result.is_err(), "expected error for unknown selector"); - }); -} - -// ───────────────────────────────────────────────────────────────────────────── -// 2. Parameter decoding -// ───────────────────────────────────────────────────────────────────────────── - -#[test] -fn transfer_input_too_short_returns_error() { - new_test_ext().execute_with(|| { - // Selector only — no dest or value. - let mut handle = MockHandle::new(SEL_TRANSFER.to_vec()); - let result = BalancesPrecompile::::execute(&mut handle); - assert!(result.is_err(), "expected error for truncated input"); - - // Selector + partial dest (only 20 bytes instead of 32+32). - let partial: Vec = SEL_TRANSFER.iter().chain(&[0xFFu8; 20]).copied().collect(); - let mut handle2 = MockHandle::new(partial); - let result2 = BalancesPrecompile::::execute(&mut handle2); - assert!( - result2.is_err(), - "expected error for partially present dest" - ); - }); -} - -#[test] -fn transfer_value_overflow_returns_error() { - new_test_ext().execute_with(|| { - let mut input = SEL_TRANSFER.to_vec(); - input.extend_from_slice(bob_account().as_ref()); - // Encode 2^128 as uint256: first byte of the high half is 0x01, rest 0x00. - let mut value_bytes = [0u8; 32]; - value_bytes[15] = 0x01; // bit 128 set → overflows u128 - input.extend_from_slice(&value_bytes); - - let mut handle = MockHandle::new(input); - let result = BalancesPrecompile::::execute(&mut handle); - assert!(result.is_err(), "expected overflow error"); - }); -} - -// ───────────────────────────────────────────────────────────────────────────── -// 3. Happy-path transfers -// ───────────────────────────────────────────────────────────────────────────── - -#[test] -fn transfer_allow_death_updates_balances() { - new_test_ext().execute_with(|| { - let initial = Balances::::free_balance(caller_account()); - let amount = 1_000u128; - - let input = encode_transfer(SEL_TRANSFER, bob_account().into(), amount); - let mut handle = MockHandle::new(input); - let result = BalancesPrecompile::::execute(&mut handle); - - assert!( - matches!(result, Ok(ref out) if out.exit_status == ExitSucceed::Returned), - "expected Ok(Returned), got {result:?}" - ); - - assert_eq!( - Balances::::free_balance(caller_account()), - initial - amount, - "caller balance not reduced by amount" - ); - assert_eq!( - Balances::::free_balance(bob_account()), - amount, - "bob balance not increased by amount" - ); - }); -} - -#[test] -fn transfer_keep_alive_updates_balances() { - new_test_ext().execute_with(|| { - let initial = Balances::::free_balance(caller_account()); - // Transfer all-but-one so the account stays alive (ED = 1). - let amount = initial - 1; - - let input = encode_transfer(SEL_TRANSFER_KEEP_ALIVE, bob_account().into(), amount); - let mut handle = MockHandle::new(input); - let result = BalancesPrecompile::::execute(&mut handle); - - assert!( - matches!(result, Ok(ref out) if out.exit_status == ExitSucceed::Returned), - "expected Ok(Returned), got {result:?}" - ); - - assert_eq!( - Balances::::free_balance(caller_account()), - 1, - "caller should retain exactly the existential deposit" - ); - assert_eq!( - Balances::::free_balance(bob_account()), - amount, - "bob should receive the transferred amount" - ); - }); -} - -#[test] -fn transfer_keep_alive_fails_if_would_kill_sender() { - new_test_ext().execute_with(|| { - let initial = Balances::::free_balance(caller_account()); - - // Attempting to transfer the full balance with keep-alive must fail - // because it would leave the sender at 0, below ED = 1. - let input = encode_transfer(SEL_TRANSFER_KEEP_ALIVE, bob_account().into(), initial); - let mut handle = MockHandle::new(input); - let result = BalancesPrecompile::::execute(&mut handle); - - assert!( - result.is_err(), - "keep-alive transfer of full balance must fail" - ); - - // Balances must be unchanged. - assert_eq!( - Balances::::free_balance(caller_account()), - initial, - "caller balance must be unchanged after failed transfer" - ); - assert_eq!( - Balances::::free_balance(bob_account()), - 0, - "bob balance must be unchanged after failed transfer" - ); - }); -} - -#[test] -fn transfer_allow_death_drains_full_balance() { - new_test_ext().execute_with(|| { - let initial = Balances::::free_balance(caller_account()); - - // `transfer` (allow-death) of the full balance must succeed and reap the - // sender's account. - let input = encode_transfer(SEL_TRANSFER, bob_account().into(), initial); - let mut handle = MockHandle::new(input); - let result = BalancesPrecompile::::execute(&mut handle); - - assert!( - matches!(result, Ok(ref out) if out.exit_status == ExitSucceed::Returned), - "expected Ok(Returned), got {result:?}" - ); - - assert_eq!( - Balances::::free_balance(caller_account()), - 0, - "caller balance must be zero after full drain" - ); - assert_eq!( - Balances::::free_balance(bob_account()), - initial, - "bob must receive the full initial balance" - ); - }); -} - -// ───────────────────────────────────────────────────────────────────────────── -// 4. Destination type variants -// ───────────────────────────────────────────────────────────────────────────── - -/// Destination is an EVM-suffix AccountId32: `[H160 | 0x00 × 12]`. -/// This mirrors how an EVM wallet is represented in the Substrate storage. -#[test] -fn transfer_to_evm_suffix_account() { - new_test_ext().execute_with(|| { - let amount = 500u128; - let dest: [u8; 32] = eve_evm_account().into(); - - let input = encode_transfer(SEL_TRANSFER, dest, amount); - let mut handle = MockHandle::new(input); - let result = BalancesPrecompile::::execute(&mut handle); - - assert!( - matches!(result, Ok(ref out) if out.exit_status == ExitSucceed::Returned), - "transfer to EVM-suffix account failed: {result:?}" - ); - - assert_eq!( - Balances::::free_balance(eve_evm_account()), - amount, - "EVM-suffix recipient did not receive the expected balance" - ); - }); -} - -/// Destination is an arbitrary 32-byte AccountId32 (e.g. a Sr25519 public key). -#[test] -fn transfer_to_arbitrary_accountid32() { - new_test_ext().execute_with(|| { - let amount = 250u128; - // A deterministic but arbitrary 32-byte account (distinct from bob). - let dest_bytes = [0xCAu8; 32]; - let dest_account = sp_runtime::AccountId32::from(dest_bytes); - - let input = encode_transfer(SEL_TRANSFER, dest_bytes, amount); - let mut handle = MockHandle::new(input); - let result = BalancesPrecompile::::execute(&mut handle); - - assert!( - matches!(result, Ok(ref out) if out.exit_status == ExitSucceed::Returned), - "transfer to arbitrary AccountId32 failed: {result:?}" - ); - - assert_eq!( - Balances::::free_balance(&dest_account), - amount, - "arbitrary AccountId32 recipient did not receive the expected balance" - ); - }); -} - -// ───────────────────────────────────────────────────────────────────────────── -// 5. Insufficient balance -// ───────────────────────────────────────────────────────────────────────────── - -#[test] -fn transfer_insufficient_balance_returns_error() { - new_test_ext().execute_with(|| { - let initial = Balances::::free_balance(caller_account()); - // Request more than the caller has. - let amount = initial + 1; - - let input = encode_transfer(SEL_TRANSFER, bob_account().into(), amount); - let mut handle = MockHandle::new(input); - let result = BalancesPrecompile::::execute(&mut handle); - - assert!( - result.is_err(), - "expected error when caller has insufficient balance" - ); - - // Balances must be unchanged. - assert_eq!( - Balances::::free_balance(caller_account()), - initial, - "caller balance must be unchanged after failed transfer" - ); - assert_eq!( - Balances::::free_balance(bob_account()), - 0, - "bob balance must be unchanged after failed transfer" - ); - }); -} diff --git a/frame/evm/precompile/balances/src/tests/abi.rs b/frame/evm/precompile/balances/src/tests/abi.rs new file mode 100644 index 00000000..8de44002 --- /dev/null +++ b/frame/evm/precompile/balances/src/tests/abi.rs @@ -0,0 +1,62 @@ +use super::{encode_transfer, revert_reason, run}; +use crate::{ + mock::{bob_account, new_test_ext, MockHandle}, + SEL_TRANSFER, SEL_TRANSFER_KEEP_ALIVE, +}; + +#[test] +fn selector_too_short_reverts() { + new_test_ext().execute_with(|| { + for len in 0..4 { + let reason = revert_reason(run(&mut MockHandle::new(vec![0xAA; len]))); + assert_eq!(reason, "input too short: missing selector"); + } + }); +} + +#[test] +fn unknown_selector_reverts() { + new_test_ext().execute_with(|| { + let reason = revert_reason(run(&mut MockHandle::new(vec![0xDE, 0xAD, 0xBE, 0xEF]))); + assert_eq!(reason, "unknown selector"); + }); +} + +#[test] +fn calldata_must_be_exactly_two_words() { + new_test_ext().execute_with(|| { + let full = encode_transfer(SEL_TRANSFER, bob_account().into(), 1); + for input in [ + full[..4].to_vec(), + full[..67].to_vec(), + [full.clone(), vec![0]].concat(), + ] { + let reason = revert_reason(run(&mut MockHandle::new(input))); + assert_eq!(reason, "calldata must be selector + bytes32 + uint256"); + } + }); +} + +#[test] +fn value_above_u128_reverts() { + new_test_ext().execute_with(|| { + for selector in [SEL_TRANSFER, SEL_TRANSFER_KEEP_ALIVE] { + let mut input = encode_transfer(selector, bob_account().into(), 0); + input[36] = 0x01; // a bit in the high 16 bytes of the uint256 + assert_eq!( + revert_reason(run(&mut MockHandle::new(input))), + "value overflows u128" + ); + } + }); +} + +#[test] +fn selectors_are_the_keccak_of_their_signatures() { + use sp_io::hashing::keccak_256; + assert_eq!(SEL_TRANSFER, keccak_256(b"transfer(bytes32,uint256)")[..4]); + assert_eq!( + SEL_TRANSFER_KEEP_ALIVE, + keccak_256(b"transferKeepAlive(bytes32,uint256)")[..4] + ); +} diff --git a/frame/evm/precompile/balances/src/tests/dispatch.rs b/frame/evm/precompile/balances/src/tests/dispatch.rs new file mode 100644 index 00000000..3f877c9f --- /dev/null +++ b/frame/evm/precompile/balances/src/tests/dispatch.rs @@ -0,0 +1,114 @@ +use super::{encode_transfer, revert_reason, run, transfer_handle}; +use crate::{ + mock::{bob_account, caller_account, eve_evm_account, new_test_ext, MockHandle, Test}, + NEW_ACCOUNT_STORAGE_GROWTH, SEL_TRANSFER, SEL_TRANSFER_KEEP_ALIVE, +}; +use frame_support::dispatch::GetDispatchInfo; +use pallet_balances::Pallet as Balances; +use pallet_evm::GasWeightMapping; + +const START: u128 = 1_000_000; + +fn free(who: &sp_runtime::AccountId32) -> u128 { + Balances::::free_balance(who) +} + +/// Gas a transfer of this kind is charged: its declared dispatch weight. +fn expected_gas(keep_alive: bool) -> u64 { + let dest = bob_account(); + let call: crate::mock::RuntimeCall = if keep_alive { + pallet_balances::Call::::transfer_keep_alive { dest, value: 1 }.into() + } else { + pallet_balances::Call::::transfer_allow_death { dest, value: 1 }.into() + }; + ::GasWeightMapping::weight_to_gas( + call.get_dispatch_info().total_weight(), + ) +} + +#[test] +fn transfer_moves_the_amount() { + new_test_ext().execute_with(|| { + let mut handle = transfer_handle(bob_account().into(), 1_000); + assert!(run(&mut handle).is_ok()); + assert_eq!(free(&caller_account()), START - 1_000); + assert_eq!(free(&bob_account()), 1_000); + }); +} + +#[test] +fn transfer_keep_alive_moves_the_amount() { + new_test_ext().execute_with(|| { + let input = encode_transfer(SEL_TRANSFER_KEEP_ALIVE, bob_account().into(), 500); + assert!(run(&mut MockHandle::new(input)).is_ok()); + assert_eq!(free(&caller_account()), START - 500); + assert_eq!(free(&bob_account()), 500); + }); +} + +#[test] +fn transfer_keep_alive_never_kills_the_sender() { + new_test_ext().execute_with(|| { + let input = encode_transfer(SEL_TRANSFER_KEEP_ALIVE, bob_account().into(), START); + let reason = revert_reason(run(&mut MockHandle::new(input))); + assert!(reason.starts_with("transfer failed:"), "{reason}"); + assert_eq!(free(&caller_account()), START); + assert_eq!(free(&bob_account()), 0); + }); +} + +#[test] +fn transfer_can_drain_the_sender() { + new_test_ext().execute_with(|| { + assert!(run(&mut transfer_handle(bob_account().into(), START)).is_ok()); + assert_eq!(free(&caller_account()), 0); + assert_eq!(free(&bob_account()), START); + }); +} + +#[test] +fn any_account_id32_can_receive() { + new_test_ext().execute_with(|| { + for dest in [ + eve_evm_account(), + sp_runtime::AccountId32::from([0x77u8; 32]), + ] { + assert!(run(&mut transfer_handle(dest.clone().into(), 100)).is_ok()); + assert_eq!(free(&dest), 100); + } + }); +} + +#[test] +fn insufficient_balance_reverts_with_the_pallet_error() { + new_test_ext().execute_with(|| { + let reason = revert_reason(run(&mut transfer_handle(bob_account().into(), START + 1))); + assert!(reason.starts_with("transfer failed:"), "{reason}"); + assert_eq!(free(&caller_account()), START); + assert_eq!(free(&bob_account()), 0); + }); +} + +#[test] +fn the_dispatch_weight_is_charged_up_front() { + new_test_ext().execute_with(|| { + for (selector, keep_alive) in [(SEL_TRANSFER, false), (SEL_TRANSFER_KEEP_ALIVE, true)] { + let mut handle = MockHandle::new(encode_transfer(selector, bob_account().into(), 10)); + assert!(run(&mut handle).is_ok()); + assert!(expected_gas(keep_alive) > 0); + assert_eq!(handle.gas_used, expected_gas(keep_alive)); + assert!(handle.proof_size > 0, "proof size is metered"); + assert_eq!(handle.storage_growth, NEW_ACCOUNT_STORAGE_GROWTH); + } + }); +} + +#[test] +fn a_failing_transfer_is_still_charged() { + new_test_ext().execute_with(|| { + // Charging only after a successful dispatch would make failures free. + let mut handle = transfer_handle(bob_account().into(), START + 1); + assert!(run(&mut handle).is_err()); + assert_eq!(handle.gas_used, expected_gas(false)); + }); +} diff --git a/frame/evm/precompile/balances/src/tests/guard.rs b/frame/evm/precompile/balances/src/tests/guard.rs new file mode 100644 index 00000000..07f57e0d --- /dev/null +++ b/frame/evm/precompile/balances/src/tests/guard.rs @@ -0,0 +1,52 @@ +use super::{revert_reason, run, transfer_handle}; +use crate::mock::{bob_account, caller_account, new_test_ext, Test}; +use pallet_balances::Pallet as Balances; +use sp_core::{H160, U256}; + +/// Runs `handle` and checks it was refused with `reason` and moved nothing. +fn assert_refused(mut handle: crate::mock::MockHandle, reason: &str) { + let before = Balances::::free_balance(caller_account()); + assert_eq!(revert_reason(run(&mut handle)), reason); + assert_eq!( + Balances::::free_balance(caller_account()), + before, + "caller debited" + ); + assert_eq!( + Balances::::free_balance(bob_account()), + 0, + "bob credited" + ); + assert_eq!(handle.gas_used, 0, "a refused call dispatches nothing"); +} + +#[test] +fn static_call_is_refused() { + new_test_ext().execute_with(|| { + let mut handle = transfer_handle(bob_account().into(), 1_000); + handle.is_static = true; + assert_refused(handle, "static calls are not allowed"); + }); +} + +#[test] +fn delegated_call_is_refused() { + new_test_ext().execute_with(|| { + // A contract delegating to the precompile: the code runs at the + // precompile's address but in the contract's context, whose caller is + // the user who called the contract. + let mut handle = transfer_handle(bob_account().into(), 1_000); + handle.code_address = H160::from_low_u64_be(2050); + handle.context.address = H160::repeat_byte(0xC0); + assert_refused(handle, "delegated calls are not allowed"); + }); +} + +#[test] +fn call_with_value_is_refused() { + new_test_ext().execute_with(|| { + let mut handle = transfer_handle(bob_account().into(), 1_000); + handle.context.apparent_value = U256::from(1u8); + assert_refused(handle, "not payable"); + }); +} diff --git a/frame/evm/precompile/balances/src/tests/mod.rs b/frame/evm/precompile/balances/src/tests/mod.rs new file mode 100644 index 00000000..7407005b --- /dev/null +++ b/frame/evm/precompile/balances/src/tests/mod.rs @@ -0,0 +1,53 @@ +//! Tests for `pallet-evm-precompile-balances`, one module per concern: +//! +//! | Module | What it verifies | +//! |------------|-------------------------------------------------------------------| +//! | `abi` | Selector and calldata decoding, exact length, u128 bound | +//! | `guard` | Static, delegated and value-carrying calls are refused | +//! | `dispatch` | Transfers move balances; weight, proof size and storage are paid | +//! | | up front, failing transfers included; failures revert with reason | + +mod abi; +mod dispatch; +mod guard; + +use crate::{mock::MockHandle, BalancesPrecompile, SEL_TRANSFER}; +use fp_evm::{Precompile, PrecompileFailure, PrecompileResult}; + +pub(super) type Precompiled = BalancesPrecompile; + +/// `selector ‖ dest ‖ uint256(value)`. +pub(super) fn encode_transfer(selector: [u8; 4], dest: [u8; 32], value: u128) -> Vec { + let mut input = selector.to_vec(); + input.extend_from_slice(&dest); + input.extend_from_slice(&[0u8; 16]); + input.extend_from_slice(&value.to_be_bytes()); + input +} + +pub(super) fn run(handle: &mut MockHandle) -> PrecompileResult { + Precompiled::execute(handle) +} + +pub(super) fn transfer_handle(dest: [u8; 32], value: u128) -> MockHandle { + MockHandle::new(encode_transfer(SEL_TRANSFER, dest, value)) +} + +/// The `Error(string)` reason of a revert; panics on any other outcome, so a +/// test also proves the failure is a revert (unused gas returned), not an +/// `Error` exit that burns the caller's whole gas limit. +pub(super) fn revert_reason(result: PrecompileResult) -> String { + match result { + Err(PrecompileFailure::Revert { output, .. }) => { + // Error(string): selector ‖ offset ‖ length ‖ bytes. + assert_eq!( + &output[..4], + &[0x08, 0xc3, 0x79, 0xa0], + "not an Error(string)" + ); + let len = u32::from_be_bytes(output[64..68].try_into().unwrap()) as usize; + String::from_utf8(output[68..68 + len].to_vec()).unwrap() + } + other => panic!("expected a revert, got {other:?}"), + } +} diff --git a/template/runtime/RUNTIME_VERSIONS.md b/template/runtime/RUNTIME_VERSIONS.md index 81a66796..97c8874a 100644 --- a/template/runtime/RUNTIME_VERSIONS.md +++ b/template/runtime/RUNTIME_VERSIONS.md @@ -20,6 +20,18 @@ to `spec_version` / `transaction_version` must add a row here in the same PR. The genesis reset (`69d1b837`) set `spec_version` back to 1 and `transaction_version` to 1 for the public testnet launch. +### Next spec — pending + +Not yet versioned: needs a `spec_version` bump (transaction version unchanged) +before `setCode`. + +- **Balances precompile (`0x…0802`)** refuses static calls, delegated calls + (DELEGATECALL, CALLCODE) and calls carrying value. Under DELEGATECALL the + debited `context.caller` was the user who called the delegating contract, so + any contract could move its caller's ORB. Calls are charged the transfer's + dispatch weight up front, failing ones included, instead of a flat 5,000 gas, + and every failure reverts with an `Error(string)` reason. + ### spec 19 — tx 7 — [Unreleased] A validator is its account and session keys, nothing else: the EVM relayer layer diff --git a/template/runtime/src/precompiles.rs b/template/runtime/src/precompiles.rs index 261f73a8..ca91e1f7 100644 --- a/template/runtime/src/precompiles.rs +++ b/template/runtime/src/precompiles.rs @@ -44,7 +44,8 @@ where + pallet_balances::Config, ::RuntimeCall: sp_runtime::traits::Dispatchable + frame_support::dispatch::GetDispatchInfo - + From>, + + From> + + From>, <::RuntimeCall as sp_runtime::traits::Dispatchable>::RuntimeOrigin: From::AccountId>>, <::RuntimeCall as sp_runtime::traits::Dispatchable>::PostInfo: @@ -53,10 +54,6 @@ where pallet_evm::AccountIdOf: Into<::AccountId>, pallet_shielded_pool::BalanceOf: TryFrom, ::Balance: TryFrom, - pallet_balances::Pallet: frame_support::traits::fungible::Mutate< - ::AccountId, - Balance = ::Balance, - >, { fn execute(&self, handle: &mut impl PrecompileHandle) -> Option { match handle.code_address() {