From 511f64396434d105a6bad5f787ab173b657de2c6 Mon Sep 17 00:00:00 2001 From: Artur Gontijo Date: Sat, 18 Apr 2026 12:32:42 -0300 Subject: [PATCH 1/5] Improve eth_call logic --- client/rpc/src/eth/execute.rs | 206 ++++++++++++++++++++++++++++++---- frame/ethereum/src/lib.rs | 1 + frame/evm/src/lib.rs | 1 + frame/evm/src/runner/mod.rs | 5 +- frame/evm/src/runner/stack.rs | 62 ++++++++-- frame/evm/src/tests.rs | 19 ++++ primitives/evm/src/lib.rs | 12 ++ primitives/rpc/src/lib.rs | 18 ++- template/fuzz/src/main.rs | 1 + template/runtime/src/lib.rs | 2 + 10 files changed, 295 insertions(+), 32 deletions(-) diff --git a/client/rpc/src/eth/execute.rs b/client/rpc/src/eth/execute.rs index edbe6e68e5..5e15eef3d8 100644 --- a/client/rpc/src/eth/execute.rs +++ b/client/rpc/src/eth/execute.rs @@ -38,7 +38,7 @@ use sp_runtime::{ use sp_state_machine::OverlayedChanges; // Frontier use fc_rpc_core::types::*; -use fp_evm::{ExecutionInfo, ExecutionInfoV2}; +use fp_evm::{ExecutionInfo, ExecutionInfoV2, StateOverride}; use fp_rpc::{EthereumRuntimeRPCApi, RuntimeStorageOverride}; use fp_storage::constants::{EVM_ACCOUNT_CODES, EVM_ACCOUNT_STORAGES, PALLET_EVM}; @@ -97,6 +97,18 @@ where .. } = request; + // Geth-parity: `state` and `stateDiff` are mutually exclusive for any given + // account. Reject requests that set both rather than silently merging them. + if let Some(ref overrides) = state_overrides { + for (address, ov) in overrides { + if ov.state.is_some() && ov.state_diff.is_some() { + return Err(internal_err(format!( + "both `state` and `stateDiff` specified for account {address:?}; they are mutually exclusive" + ))); + } + } + } + let (gas_price, max_fee_per_gas, max_priority_fee_per_gas) = { let details = fee_details(gas_price, max_fee_per_gas, max_priority_fee_per_gas)?; ( @@ -368,6 +380,72 @@ where error_on_execution_failure(&info.exit_reason, &info.value)?; + Ok(Bytes(info.value)) + } else if api_version == 7 { + // Pectra + efficient `state` full-storage override (runtime API v7) + let access_list = access_list + .unwrap_or_default() + .into_iter() + .map(|item| (item.address, item.storage_keys)) + .collect::)>>(); + + let state_override_payload = build_state_override_payload(&state_overrides); + + let encoded_params = Encode::encode(&( + &from.unwrap_or_default(), + &to, + &data, + &value.unwrap_or_default(), + &gas_limit, + &max_fee_per_gas, + &max_priority_fee_per_gas, + &nonce, + &false, + &Some(access_list), + &authorization_list, + &state_override_payload, + )); + let overlayed_changes = self.create_overrides_overlay( + substrate_hash, + api_version, + state_overrides, + )?; + + let recorder: sp_trie::recorder::Recorder> = Default::default(); + let ext = sp_trie::proof_size_extension::ProofSizeExt::new(recorder.clone()); + let mut exts = Extensions::new(); + exts.register(ext); + + let params = CallApiAtParams { + at: substrate_hash, + function: "EthereumRuntimeRPCApi_call", + arguments: encoded_params, + overlayed_changes: &RefCell::new(overlayed_changes), + call_context: CallContext::Offchain, + recorder: &Some(recorder), + extensions: &RefCell::new(exts), + }; + + let info = self + .client + .call_api_at(params) + .and_then(|r| { + Result::map_err( + >, DispatchError> as Decode>::decode( + &mut &r[..], + ), + |error| sp_api::ApiError::FailedToDecodeReturnValue { + function: "EthereumRuntimeRPCApi_call", + error, + raw: r, + }, + ) + }) + .map_err(|err| internal_err(format!("runtime error: {err}")))? + .map_err(|err| internal_err(format!("execution fatal: {err:?}")))?; + + error_on_execution_failure(&info.exit_reason, &info.value)?; + Ok(Bytes(info.value)) } else { Err(internal_err("failed to retrieve Runtime Api version")) @@ -479,7 +557,7 @@ where .account_code_at(substrate_hash, info.value) .map_err(|err| internal_err(format!("runtime error: {err}")))?; Ok(Bytes(code)) - } else if api_version == 6 { + } else if api_version == 6 || api_version == 7 { // Pectra EIP-7702 support let access_list = access_list.unwrap_or_default(); let authorization_list = authorization_list.unwrap_or_default(); @@ -839,6 +917,61 @@ where extensions: &RefCell::new(exts), }; + let info = self + .client + .call_api_at(params) + .and_then(|r| { + Result::map_err( + >, DispatchError> as Decode>::decode(&mut &r[..]), + |error| sp_api::ApiError::FailedToDecodeReturnValue { + function: "EthereumRuntimeRPCApi_call", + error, + raw: r + }, + ) + }) + .map_err(|err| internal_err(format!("runtime error: {err}")))? + .map_err(|err| internal_err(format!("execution fatal: {err:?}")))?; + + (info.exit_reason, info.value, info.used_gas.effective) + } else if api_version == 7 { + let access_list = access_list + .unwrap_or_default() + .into_iter() + .map(|item| (item.address, item.storage_keys)) + .collect::)>>(); + + let encoded_params = Encode::encode(&( + &from.unwrap_or_default(), + &to, + &data, + &value.unwrap_or_default(), + &gas_limit, + &max_fee_per_gas, + &max_priority_fee_per_gas, + &None::>, + &estimate_mode, + &Some(access_list), + &authorization_list, + &None::>, + &None::>, + )); + + let recorder: sp_trie::recorder::Recorder> = Default::default(); + let ext = sp_trie::proof_size_extension::ProofSizeExt::new(recorder.clone()); + let mut exts = Extensions::new(); + exts.register(ext); + + let params = CallApiAtParams { + at: substrate_hash, + function: "EthereumRuntimeRPCApi_call", + arguments: encoded_params, + overlayed_changes: &RefCell::new(Default::default()), + call_context: CallContext::Offchain, + recorder: &Some(recorder), + extensions: &RefCell::new(exts), + }; + let info = self .client .call_api_at(params) @@ -974,7 +1107,7 @@ where .map_err(|err| internal_err(format!("execution fatal: {err:?}")))?; (info.exit_reason, Vec::new(), info.used_gas.effective) - } else if api_version == 6 { + } else if api_version == 6 || api_version == 7 { // Pectra - authorization list support (EIP-7702) let access_list = access_list .unwrap_or_default() @@ -1168,6 +1301,7 @@ where api_version: u32, state_overrides: Option>, ) -> RpcResult>> { + let use_runtime_full_storage = api_version >= 7; let mut overlayed_changes = OverlayedChanges::default(); if let Some(state_overrides) = state_overrides { for (address, state_override) in state_overrides { @@ -1204,29 +1338,35 @@ where account_storage_key.extend(blake2_128(address.as_bytes())); account_storage_key.extend(address.as_bytes()); - // Use `state` first. If `stateDiff` is also present, it resolves consistently + // `state` and `stateDiff` are mutually exclusive (validated upstream). + // + // For runtime API v7+, a `state` override is applied inside the runtime via + // the `state_override` parameter (see `build_state_override_payload`) and must + // NOT be mirrored into the overlay here. For older runtimes we fall back to + // the legacy behavior, which enumerates every existing slot and clears it + // before writing the provided values — costly for large contracts. if let Some(state) = &state_override.state { - // clear all storage - if let Ok(all_keys) = self.client.storage_keys( - block_hash, - Some(&sp_storage::StorageKey(account_storage_key.clone())), - None, - ) { - for key in all_keys { - overlayed_changes.set_storage(key.0, None); + if !use_runtime_full_storage { + if let Ok(all_keys) = self.client.storage_keys( + block_hash, + Some(&sp_storage::StorageKey(account_storage_key.clone())), + None, + ) { + for key in all_keys { + overlayed_changes.set_storage(key.0, None); + } } - } - // set provided storage - for (k, v) in state { - let mut slot_key = account_storage_key.clone(); - slot_key.extend(blake2_128(k.as_bytes())); - slot_key.extend(k.as_bytes()); + for (k, v) in state { + let mut slot_key = account_storage_key.clone(); + slot_key.extend(blake2_128(k.as_bytes())); + slot_key.extend(k.as_bytes()); - overlayed_changes.set_storage(slot_key, Some(v.as_bytes().to_owned())); + overlayed_changes.set_storage(slot_key, Some(v.as_bytes().to_owned())); + } } - } - - if let Some(state_diff) = &state_override.state_diff { + } else if let Some(state_diff) = &state_override.state_diff { + // `stateDiff` always goes through the overlay: cost is bounded by the + // caller's payload on both legacy and v7+ runtimes. for (k, v) in state_diff { let mut slot_key = account_storage_key.clone(); slot_key.extend(blake2_128(k.as_bytes())); @@ -1242,6 +1382,28 @@ where } } +/// Build the runtime-API v7 state-override payload from the caller's overrides. +/// +/// Only accounts that specify `state` (full replacement) are included. Each entry is +/// `(address, slots)`; an empty `slots` vector represents a full storage wipe +/// (the `"state": {}` case). `stateDiff` is handled separately via the overlay and +/// must not be combined with `state` (rejected upstream). +/// +/// Cost is bounded by the request payload size, not on-chain storage (Geth parity). +fn build_state_override_payload( + state_overrides: &Option>, +) -> StateOverride { + let state_overrides = state_overrides.as_ref()?; + let mut payload = Vec::new(); + for (address, ov) in state_overrides { + if let Some(state) = &ov.state { + let slots = state.iter().map(|(k, v)| (*k, *v)).collect::>(); + payload.push((*address, slots)); + } + } + (!payload.is_empty()).then_some(payload) +} + pub fn error_on_execution_failure(reason: &ExitReason, data: &[u8]) -> RpcResult<()> { match reason { ExitReason::Succeed(_) => Ok(()), diff --git a/frame/ethereum/src/lib.rs b/frame/ethereum/src/lib.rs index 54cbd4f92b..92888629f4 100644 --- a/frame/ethereum/src/lib.rs +++ b/frame/ethereum/src/lib.rs @@ -911,6 +911,7 @@ impl Pallet { validate, weight_limit, proof_size_base_cost, + None, config.as_ref().unwrap_or_else(|| T::config()), ) { Ok(res) => res, diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index dac94491a4..08bd765cfc 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -348,6 +348,7 @@ pub mod pallet { validate, None, None, + None, T::config(), ) { Ok(info) => info, diff --git a/frame/evm/src/runner/mod.rs b/frame/evm/src/runner/mod.rs index e10c07c07e..f8094d2a1f 100644 --- a/frame/evm/src/runner/mod.rs +++ b/frame/evm/src/runner/mod.rs @@ -21,7 +21,7 @@ pub mod stack; use crate::{Config, Weight}; use alloc::vec::Vec; use ethereum::AuthorizationList; -use fp_evm::{CallInfo, CreateInfo}; +use fp_evm::{CallInfo, CreateInfo, StateOverride}; use sp_core::{H160, H256, U256}; #[derive(Debug)] @@ -65,6 +65,9 @@ pub trait Runner { validate: bool, weight_limit: Option, proof_size_base_cost: Option, + // Per-account full storage replacement (Geth-style `state` override). + // See [`StateOverride`] for the encoding and semantics. + state_override: StateOverride, config: &evm::Config, ) -> Result>; diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index 1c3d6aa159..6825542878 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -45,7 +45,7 @@ use sp_runtime::traits::UniqueSaturatedInto; // Frontier use fp_evm::{ AccessedStorage, CallInfo, CreateInfo, ExecutionInfoV2, IsPrecompileResult, Log, PrecompileSet, - Vicinity, WeightInfo, ACCOUNT_BASIC_PROOF_SIZE, ACCOUNT_CODES_KEY_SIZE, + StateOverride, Vicinity, WeightInfo, ACCOUNT_BASIC_PROOF_SIZE, ACCOUNT_CODES_KEY_SIZE, ACCOUNT_CODES_METADATA_PROOF_SIZE, ACCOUNT_STORAGE_PROOF_SIZE, IS_EMPTY_CHECK_PROOF_SIZE, WRITE_PROOF_SIZE, }; @@ -83,6 +83,7 @@ where weight_limit: Option, proof_size_base_cost: Option, measured_proof_size_before: u64, + state_override: StateOverride, f: F, ) -> Result, RunnerError>> where @@ -114,6 +115,7 @@ where weight_limit, proof_size_base_cost, measured_proof_size_before, + state_override, ); #[cfg(feature = "forbid-evm-reentrancy")] @@ -153,6 +155,7 @@ where weight_limit, proof_size_base_cost, measured_proof_size_before, + state_override, ) }); @@ -175,6 +178,7 @@ where weight_limit: Option, proof_size_base_cost: Option, measured_proof_size_before: u64, + state_override: StateOverride, ) -> Result, RunnerError>> where F: FnOnce( @@ -307,7 +311,13 @@ where }; let metadata = StackSubstateMetadata::new(gas_limit, config); - let state = SubstrateStackState::new(&vicinity, metadata, maybe_weight_info, storage_limit); + let state = SubstrateStackState::new( + &vicinity, + metadata, + maybe_weight_info, + storage_limit, + state_override, + ); let mut executor = StackExecutor::new_with_precompiles(state, config, precompiles); // Execute the EVM call. @@ -530,6 +540,7 @@ where validate: bool, weight_limit: Option, proof_size_base_cost: Option, + state_override: StateOverride, config: &evm::Config, ) -> Result> { let measured_proof_size_before = get_proof_size().unwrap_or_default(); @@ -578,6 +589,7 @@ where weight_limit, proof_size_base_cost, measured_proof_size_before, + state_override, |executor| { executor.transact_call( source, @@ -658,6 +670,7 @@ where weight_limit, proof_size_base_cost, measured_proof_size_before, + None, |executor| { let address = executor.create_address(evm::CreateScheme::Legacy { caller: source }); T::OnCreate::on_create(source, address); @@ -742,6 +755,7 @@ where weight_limit, proof_size_base_cost, measured_proof_size_before, + None, |executor| { let address = executor.create_address(evm::CreateScheme::Create2 { caller: source, @@ -829,6 +843,7 @@ where weight_limit, proof_size_base_cost, measured_proof_size_before, + None, |executor| { T::OnCreate::on_create(source, contract_address); let (reason, _) = executor.transact_create_force_address( @@ -973,6 +988,11 @@ pub struct SubstrateStackState<'vicinity, 'config, T> { substate: SubstrateStackSubstate<'config>, original_storage: BTreeMap<(H160, H256), H256>, transient_storage: BTreeMap<(H160, H256), H256>, + /// Per-account full storage replacement. When an address is present here, storage + /// reads are served exclusively from the per-address map (missing slots return zero) + /// instead of the on-chain trie. An entry with an empty inner map represents a full + /// wipe (Geth-style `"state": {}`). + state_override: BTreeMap>, recorded: Recorded, weight_info: Option, storage_meter: Option, @@ -986,8 +1006,17 @@ impl<'vicinity, 'config, T: Config> SubstrateStackState<'vicinity, 'config, T> { metadata: StackSubstateMetadata<'config>, weight_info: Option, storage_limit: Option, + state_override: StateOverride, ) -> Self { let storage_meter = storage_limit.map(StorageMeter::new); + let state_override = state_override + .map(|per_address| { + per_address + .into_iter() + .map(|(address, slots)| (address, slots.into_iter().collect())) + .collect::>>() + }) + .unwrap_or_default(); Self { vicinity, substate: SubstrateStackSubstate { @@ -1000,12 +1029,24 @@ impl<'vicinity, 'config, T: Config> SubstrateStackState<'vicinity, 'config, T> { _marker: PhantomData, original_storage: BTreeMap::new(), transient_storage: BTreeMap::new(), + state_override, recorded: Default::default(), weight_info, storage_meter, } } + /// Read a slot for `address`, honoring any active state override. + /// + /// If `address` is in the override set, the slot is served from the in-memory + /// map only; missing slots return zero. Otherwise the on-chain trie is read. + fn read_persisted_storage(&self, address: H160, index: H256) -> H256 { + if let Some(slots) = self.state_override.get(&address) { + return slots.get(&index).copied().unwrap_or_default(); + } + >::get(address, index) + } + pub fn weight_info(&self) -> Option { self.weight_info } @@ -1127,7 +1168,7 @@ where } fn storage(&self, address: H160, index: H256) -> H256 { - >::get(address, index) + self.read_persisted_storage(address, index) } fn transient_storage(&self, address: H160, index: H256) -> H256 { @@ -1142,7 +1183,7 @@ where self.original_storage .get(&(address, index)) .cloned() - .unwrap_or_else(|| self.storage(address, index)), + .unwrap_or_else(|| self.read_persisted_storage(address, index)), ) } } @@ -1196,9 +1237,9 @@ where fn set_storage(&mut self, address: H160, index: H256, value: H256) { // We cache the current value if this is the first time we modify it // in the transaction. - use alloc::collections::btree_map::Entry::Vacant; - if let Vacant(e) = self.original_storage.entry((address, index)) { - let original = >::get(address, index); + use alloc::collections::btree_map::Entry; + let original = self.read_persisted_storage(address, index); + if let Entry::Vacant(e) = self.original_storage.entry((address, index)) { // No need to cache if same value. if original != value { e.insert(original); @@ -1226,6 +1267,10 @@ where } fn reset_storage(&mut self, address: H160) { + // Geth-parity: once an account is destructed/recreated, the prior state + // override (if any) no longer applies — subsequent SLOADs must fall back + // to the (now-empty) persisted storage, returning zero until written. + self.state_override.remove(&address); #[allow(deprecated)] let _ = >::remove_prefix(address, None); } @@ -1596,6 +1641,7 @@ mod tests { None, None, measured_proof_size_before, + None, |_| { let measured_proof_size_before2 = get_proof_size().unwrap_or_default(); let res = Runner::::execute( @@ -1610,6 +1656,7 @@ mod tests { None, None, measured_proof_size_before2, + None, |_| (ExitReason::Succeed(ExitSucceed::Stopped), ()), ); assert_matches!( @@ -1644,6 +1691,7 @@ mod tests { None, None, measured_proof_size_before, + None, |_| (ExitReason::Succeed(ExitSucceed::Stopped), ()), ); assert!(res.is_ok()); diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs index bfc4a35f94..d4e56655e6 100644 --- a/frame/evm/src/tests.rs +++ b/frame/evm/src/tests.rs @@ -230,6 +230,7 @@ mod proof_size_test { true, // must be validated Some(weight_limit), Some(0), + None, &::config().clone(), ) .expect("call succeeds"); @@ -287,6 +288,7 @@ mod proof_size_test { true, // must be validated Some(weight_limit), Some(0), + None, &::config().clone(), ) .expect("call succeeds"); @@ -343,6 +345,7 @@ mod proof_size_test { true, // must be validated Some(weight_limit), Some(0), + None, &::config().clone(), ) .expect("call succeeds"); @@ -393,6 +396,7 @@ mod proof_size_test { true, // must be validated Some(weight_limit), Some(0), + None, &::config().clone(), ) .expect("call succeeds"); @@ -448,6 +452,7 @@ mod proof_size_test { true, // must be validated Some(weight_limit), Some(0), + None, &::config().clone(), ) .expect("call succeeds"); @@ -505,6 +510,7 @@ mod proof_size_test { true, // must be validated Some(weight_limit), Some(0), + None, &::config().clone(), ) .expect("call succeeds"); @@ -565,6 +571,7 @@ mod proof_size_test { true, // must be validated Some(weight_limit), Some(0), + None, &config, ) .expect("call succeeds"); @@ -607,6 +614,7 @@ mod proof_size_test { true, // must be validated Some(weight_limit), Some(0), + None, &config, ) .expect("call succeeds"); @@ -684,6 +692,7 @@ mod storage_growth_test { true, // must be validated None, Some(0), + None, ::config(), ) } @@ -1063,6 +1072,7 @@ fn test_inner_contract_deploy_succeeds_if_address_is_allowed() { true, // must be validated Some(weight_limit), Some(0), + None, &::config().clone(), ) .expect("call succeeds"); @@ -1102,6 +1112,7 @@ fn test_inner_contract_deploy_reverts_if_address_not_allowed() { true, // must be validated Some(weight_limit), Some(0), + None, &::config().clone(), ) .expect("call succeeds"); @@ -1502,6 +1513,7 @@ fn runner_non_transactional_calls_with_non_balance_accounts_is_ok_without_gas_pr true, // must be validated None, None, + None, &::config().clone(), ) .expect("Non transactional call succeeds"); @@ -1539,6 +1551,7 @@ fn runner_non_transactional_calls_with_non_balance_accounts_is_err_with_gas_pric true, // must be validated None, None, + None, &::config().clone(), ); assert!(res.is_err()); @@ -1564,6 +1577,7 @@ fn runner_transactional_call_with_zero_gas_price_fails() { true, // must be validated None, None, + None, &::config().clone(), ); assert!(res.is_err()); @@ -1589,6 +1603,7 @@ fn runner_max_fee_per_gas_gte_max_priority_fee_per_gas() { true, // must be validated None, None, + None, &::config().clone(), ); assert!(res.is_err()); @@ -1607,6 +1622,7 @@ fn runner_max_fee_per_gas_gte_max_priority_fee_per_gas() { true, // must be validated None, None, + None, &::config().clone(), ); assert!(res.is_err()); @@ -1633,6 +1649,7 @@ fn eip3607_transaction_from_contract() { false, // not sure be validated None, None, + None, &::config().clone(), ) { Err(RunnerError { @@ -1659,6 +1676,7 @@ fn eip3607_transaction_from_contract() { true, // must be validated None, None, + None, &::config().clone(), ) .is_ok()); @@ -1774,6 +1792,7 @@ mod eip7939_clz_test { true, // must be validated None, Some(0), + None, ::config(), ) } diff --git a/primitives/evm/src/lib.rs b/primitives/evm/src/lib.rs index 23ca081c73..3dc625aaba 100644 --- a/primitives/evm/src/lib.rs +++ b/primitives/evm/src/lib.rs @@ -69,6 +69,18 @@ pub struct Vicinity { pub origin: H160, } +/// Per-account EVM storage override payload (Geth-style `state` override). +/// +/// Each entry is `(address, slots)` where `slots` is `[(key, value), ...]`. +/// When an address is present, its persisted storage is fully replaced: reads +/// are served exclusively from the provided slot list and missing keys return +/// zero. An empty `slots` vector represents a full storage wipe (the +/// `"state": {}` case). +/// +/// `None` means no state overrides at all. The encoding cost is bounded by the +/// caller-supplied payload size (Geth parity); no on-chain storage is enumerated. +pub type StateOverride = Option)>>; + /// `System::Account` 16(hash) + 20 (key) + 72 (AccountInfo::max_encoded_len) pub const ACCOUNT_BASIC_PROOF_SIZE: u64 = 108; /// `AccountCodesMetadata` read, temtatively 16 (hash) + 20 (key) + 40 (CodeMetadata). diff --git a/primitives/rpc/src/lib.rs b/primitives/rpc/src/lib.rs index 10fb3316a2..6975ac29ab 100644 --- a/primitives/rpc/src/lib.rs +++ b/primitives/rpc/src/lib.rs @@ -94,7 +94,7 @@ impl RuntimeStorageOverride for () { sp_api::decl_runtime_apis! { /// API necessary for Ethereum-compatibility layer. - #[api_version(6)] + #[api_version(7)] pub trait EthereumRuntimeRPCApi { /// Returns runtime defined pallet_evm::ChainId. fn chain_id() -> u64; @@ -164,7 +164,20 @@ sp_api::decl_runtime_apis! { estimate: bool, access_list: Option)>>, ) -> Result>, sp_runtime::DispatchError>; - #[allow(clippy::type_complexity)] + #[changed_in(7)] + fn call( + from: Address, + to: Address, + data: Vec, + value: U256, + gas_limit: U256, + max_fee_per_gas: Option, + max_priority_fee_per_gas: Option, + nonce: Option, + estimate: bool, + access_list: Option)>>, + authorization_list: Option, + ) -> Result>, sp_runtime::DispatchError>; fn call( from: Address, to: Address, @@ -177,6 +190,7 @@ sp_api::decl_runtime_apis! { estimate: bool, access_list: Option)>>, authorization_list: Option, + state_override: fp_evm::StateOverride, ) -> Result>, sp_runtime::DispatchError>; /// Returns a frame_ethereum::create response. diff --git a/template/fuzz/src/main.rs b/template/fuzz/src/main.rs index ad7ce2bee6..28a273d346 100644 --- a/template/fuzz/src/main.rs +++ b/template/fuzz/src/main.rs @@ -55,6 +55,7 @@ fn main() { true, Some(weight_limit), Some(0u64), + None, &::config().clone(), ); let proof_size = match res { diff --git a/template/runtime/src/lib.rs b/template/runtime/src/lib.rs index c687de7ed9..d2004f2e5f 100644 --- a/template/runtime/src/lib.rs +++ b/template/runtime/src/lib.rs @@ -810,6 +810,7 @@ impl_runtime_apis! { estimate: bool, access_list: Option)>>, authorization_list: Option, + state_override: fp_evm::StateOverride, ) -> Result { use pallet_evm::GasWeightMapping as _; @@ -880,6 +881,7 @@ impl_runtime_apis! { true, weight_limit, proof_size_base_cost, + state_override, config.as_ref().unwrap_or(::config()), ).map_err(|err| err.error.into()) } From 6f7239f286743e2cd6fc6723900227bdaec40775 Mon Sep 17 00:00:00 2001 From: Artur Gontijo Date: Sat, 18 Apr 2026 17:36:54 -0300 Subject: [PATCH 2/5] Add tests --- client/rpc/src/eth/execute.rs | 230 ++++++++++++++- frame/evm/src/runner/stack.rs | 13 + frame/evm/src/tests.rs | 406 ++++++++++++++++++++++++++ ts-tests/tests/test-state-override.ts | 68 +++++ 4 files changed, 706 insertions(+), 11 deletions(-) diff --git a/client/rpc/src/eth/execute.rs b/client/rpc/src/eth/execute.rs index 5e15eef3d8..bc7c2df24a 100644 --- a/client/rpc/src/eth/execute.rs +++ b/client/rpc/src/eth/execute.rs @@ -97,17 +97,7 @@ where .. } = request; - // Geth-parity: `state` and `stateDiff` are mutually exclusive for any given - // account. Reject requests that set both rather than silently merging them. - if let Some(ref overrides) = state_overrides { - for (address, ov) in overrides { - if ov.state.is_some() && ov.state_diff.is_some() { - return Err(internal_err(format!( - "both `state` and `stateDiff` specified for account {address:?}; they are mutually exclusive" - ))); - } - } - } + validate_state_overrides(&state_overrides)?; let (gas_price, max_fee_per_gas, max_priority_fee_per_gas) = { let details = fee_details(gas_price, max_fee_per_gas, max_priority_fee_per_gas)?; @@ -1382,6 +1372,26 @@ where } } +/// Validate caller-supplied state overrides. +/// +/// Geth-parity: `state` and `stateDiff` are mutually exclusive for any given +/// account. Rather than silently merging them (which would hide a caller bug), +/// reject the whole request with a descriptive error. +fn validate_state_overrides( + state_overrides: &Option>, +) -> RpcResult<()> { + if let Some(overrides) = state_overrides { + for (address, ov) in overrides { + if ov.state.is_some() && ov.state_diff.is_some() { + return Err(internal_err(format!( + "both `state` and `stateDiff` specified for account {address:?}; they are mutually exclusive" + ))); + } + } + } + Ok(()) +} + /// Build the runtime-API v7 state-override payload from the caller's overrides. /// /// Only accounts that specify `state` (full replacement) are included. Each entry is @@ -1496,3 +1506,201 @@ fn fee_details( }), } } + +#[cfg(test)] +mod state_override_tests { + use super::*; + + fn addr(byte: u8) -> H160 { + H160::repeat_byte(byte) + } + + fn slot(byte: u8) -> H256 { + H256::repeat_byte(byte) + } + + fn override_with_state(pairs: &[(H256, H256)]) -> CallStateOverride { + CallStateOverride { + balance: None, + nonce: None, + code: None, + state: Some(pairs.iter().cloned().collect()), + state_diff: None, + } + } + + fn override_with_state_diff(pairs: &[(H256, H256)]) -> CallStateOverride { + CallStateOverride { + balance: None, + nonce: None, + code: None, + state: None, + state_diff: Some(pairs.iter().cloned().collect()), + } + } + + fn override_with_balance(balance: U256) -> CallStateOverride { + CallStateOverride { + balance: Some(balance), + nonce: None, + code: None, + state: None, + state_diff: None, + } + } + + // ---------- validate_state_overrides ---------- + + #[test] + fn validate_accepts_none() { + assert!(validate_state_overrides(&None).is_ok()); + } + + #[test] + fn validate_accepts_empty_map() { + let overrides: BTreeMap = BTreeMap::new(); + assert!(validate_state_overrides(&Some(overrides)).is_ok()); + } + + #[test] + fn validate_accepts_state_only() { + let mut overrides = BTreeMap::new(); + overrides.insert( + addr(0xaa), + override_with_state(&[(slot(0x01), H256::from_low_u64_be(0x22))]), + ); + assert!(validate_state_overrides(&Some(overrides)).is_ok()); + } + + #[test] + fn validate_accepts_state_diff_only() { + let mut overrides = BTreeMap::new(); + overrides.insert( + addr(0xaa), + override_with_state_diff(&[(slot(0x01), H256::from_low_u64_be(0x22))]), + ); + assert!(validate_state_overrides(&Some(overrides)).is_ok()); + } + + #[test] + fn validate_accepts_balance_only() { + let mut overrides = BTreeMap::new(); + overrides.insert(addr(0xaa), override_with_balance(U256::from(1_000u64))); + assert!(validate_state_overrides(&Some(overrides)).is_ok()); + } + + #[test] + fn validate_rejects_state_and_state_diff_on_same_account() { + let mut overrides = BTreeMap::new(); + overrides.insert( + addr(0xaa), + CallStateOverride { + balance: None, + nonce: None, + code: None, + state: Some( + [(slot(0x01), H256::from_low_u64_be(0x22))] + .into_iter() + .collect(), + ), + state_diff: Some( + [(slot(0x02), H256::from_low_u64_be(0x33))] + .into_iter() + .collect(), + ), + }, + ); + let err = validate_state_overrides(&Some(overrides)).unwrap_err(); + // Error message should name the offending account. + assert!(err.message().contains("mutually exclusive")); + } + + #[test] + fn validate_accepts_state_on_one_account_state_diff_on_another() { + // Mutual exclusivity is per-account, not global. + let mut overrides = BTreeMap::new(); + overrides.insert( + addr(0xaa), + override_with_state(&[(slot(0x01), H256::from_low_u64_be(0x22))]), + ); + overrides.insert( + addr(0xbb), + override_with_state_diff(&[(slot(0x01), H256::from_low_u64_be(0x33))]), + ); + assert!(validate_state_overrides(&Some(overrides)).is_ok()); + } + + // ---------- build_state_override_payload ---------- + + #[test] + fn payload_none_for_no_overrides() { + assert!(build_state_override_payload(&None).is_none()); + } + + #[test] + fn payload_none_for_empty_overrides() { + let overrides: BTreeMap = BTreeMap::new(); + assert!(build_state_override_payload(&Some(overrides)).is_none()); + } + + #[test] + fn payload_none_when_no_state_field_is_set() { + // balance/nonce/code/state_diff should NOT populate the v7 payload, + // since they're handled via the overlay path. + let mut overrides = BTreeMap::new(); + overrides.insert(addr(0xaa), override_with_balance(U256::from(100u64))); + overrides.insert( + addr(0xbb), + override_with_state_diff(&[(slot(0x01), H256::from_low_u64_be(0x22))]), + ); + assert!(build_state_override_payload(&Some(overrides)).is_none()); + } + + #[test] + fn payload_includes_only_accounts_with_state() { + let v1 = H256::from_low_u64_be(0x22); + let v2 = H256::from_low_u64_be(0x33); + + let mut overrides = BTreeMap::new(); + overrides.insert( + addr(0xaa), + override_with_state(&[(slot(0x01), v1), (slot(0x02), v2)]), + ); + overrides.insert( + addr(0xbb), + override_with_state_diff(&[(slot(0x03), H256::from_low_u64_be(0x44))]), + ); + overrides.insert(addr(0xcc), override_with_balance(U256::from(42u64))); + + let payload = build_state_override_payload(&Some(overrides)) + .expect("at least one `state` entry => Some"); + + assert_eq!(payload.len(), 1, "only 0xaa has a `state` override"); + let (address, slots) = &payload[0]; + assert_eq!(*address, addr(0xaa)); + assert_eq!(slots.len(), 2); + // BTreeMap iteration order is deterministic by key. + assert_eq!(slots[0], (slot(0x01), v1)); + assert_eq!(slots[1], (slot(0x02), v2)); + } + + #[test] + fn payload_empty_state_is_preserved_as_full_wipe() { + // Geth-parity: `"state": {}` wipes the account's storage. The payload + // must still include the address with an empty slot vec so the runtime + // can mark it as overridden with no surviving slots. + let mut overrides = BTreeMap::new(); + overrides.insert(addr(0xaa), override_with_state(&[])); + + let payload = build_state_override_payload(&Some(overrides)) + .expect("empty-state entry still yields Some payload"); + + assert_eq!(payload.len(), 1); + let (address, slots) = &payload[0]; + assert_eq!(*address, addr(0xaa)); + assert!( + slots.is_empty(), + "empty `state` must produce an empty slot vec (full-wipe marker)" + ); + } +} diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index 6825542878..98f203ae1e 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -1246,6 +1246,19 @@ where } } + // Geth-parity: if this account has an active state override, writes + // update the override map (the account's "fresh state"), not the + // underlying trie. Subsequent SLOADs will observe the new value via + // `read_persisted_storage`, while the on-chain storage stays untouched. + if let Some(slots) = self.state_override.get_mut(&address) { + if value == H256::default() { + slots.remove(&index); + } else { + slots.insert(index, value); + } + return; + } + // Then we insert or remove the entry based on the value. if value == H256::default() { log::debug!( diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs index d4e56655e6..97fbbb9a18 100644 --- a/frame/evm/src/tests.rs +++ b/frame/evm/src/tests.rs @@ -1858,3 +1858,409 @@ mod eip7939_clz_test { run_clz_test(CLZ_MSB_UNSET, 1); } } + +/// Tests for the `eth_call`-style state override feature. +/// +/// These tests exercise `Runner::call`'s `state_override` parameter, which is the +/// efficient Geth-style full-storage replacement introduced for the RPC layer +/// (runtime API v7). The semantics we verify here: +/// +/// * A non-`None` override entry for an account causes storage reads for that +/// account to be served **exclusively** from the in-memory override map, +/// mirroring Geth's "destructed and recreated" state object. Slots not present +/// in the override map read as zero, regardless of on-chain state. +/// * An empty override map for an account (`"state": {}`) performs a full wipe. +/// * Accounts not listed in the override map are unaffected and read from on-chain +/// storage normally. +/// * Writes within the overridden call (SSTORE) take precedence over the override +/// baseline for subsequent reads in that call. +/// * The override is ephemeral: on-chain `AccountStorages` is not mutated. +/// * `reset_storage` (invoked e.g. by SELFDESTRUCT) clears any active override +/// entry for the affected address. +mod state_override_test { + use super::*; + use crate::runner::stack::SubstrateStackState; + use evm::ExitSucceed; + + // Minimal runtime bytecode that returns `storage[calldata[0..32]]` as a 32-byte value. + // + // PUSH1 0x00 // 60 00 (calldata offset) + // CALLDATALOAD // 35 (slot = calldata[0..32]) + // SLOAD // 54 (value = storage[slot]) + // PUSH1 0x00 // 60 00 (memory offset) + // MSTORE // 52 (mem[0..32] = value) + // PUSH1 0x20 // 60 20 (length = 32) + // PUSH1 0x00 // 60 00 (memory offset) + // RETURN // f3 + const SLOAD_RETURN_BYTECODE: &str = "6000355460005260206000f3"; + + // Runtime bytecode that SSTOREs `calldata[32..64]` at `calldata[0..32]`, then + // SLOADs that slot and returns the value. Used to verify that writes during + // the call take precedence over the state-override baseline. + // + // PUSH1 0x20, CALLDATALOAD // value + // PUSH1 0x00, CALLDATALOAD // slot + // SSTORE // storage[slot] = value + // PUSH1 0x00, CALLDATALOAD // slot + // SLOAD // value (from executor cache after SSTORE) + // PUSH1 0x00, MSTORE + // PUSH1 0x20, PUSH1 0x00, RETURN + const SSTORE_THEN_SLOAD_BYTECODE: &str = "602035600035556000355460005260206000f3"; + + fn slot(v: u64) -> H256 { + H256::from_low_u64_be(v) + } + + fn value(v: u64) -> H256 { + H256::from_low_u64_be(v) + } + + fn slot_calldata(slot_idx: u64) -> Vec { + slot(slot_idx).as_bytes().to_vec() + } + + fn slot_value_calldata(slot_idx: u64, val: u64) -> Vec { + let mut data = Vec::with_capacity(64); + data.extend_from_slice(slot(slot_idx).as_bytes()); + data.extend_from_slice(value(val).as_bytes()); + data + } + + fn expected_return(val: u64) -> Vec { + value(val).as_bytes().to_vec() + } + + // Deploy a runtime bytecode directly at `address`, bypassing the constructor. + fn deploy_runtime_code(address: H160, bytecode_hex: &str) { + let bytecode = hex::decode(bytecode_hex).expect("valid hex bytecode"); + crate::Pallet::::create_account(address, bytecode, None) + .expect("account creation succeeds"); + } + + // Execute an eth_call-style simulation (non-transactional, no fee) against `to` + // with the given override payload. + fn eth_call_with_override( + to: H160, + calldata: Vec, + state_override: fp_evm::StateOverride, + ) -> CallInfo { + ::Runner::call( + H160::default(), + to, + calldata, + U256::zero(), + 1_000_000, + None, // max_fee_per_gas (non-transactional, no fee) + None, + None, + Vec::new(), + Vec::new(), + false, // non-transactional (eth_call semantics) + true, // must be validated + None, + None, + state_override, + ::config(), + ) + .expect("call succeeds") + } + + fn assert_succeeded(info: &CallInfo) { + assert_eq!( + info.exit_reason, + ExitReason::Succeed(ExitSucceed::Returned), + "contract call must succeed", + ); + } + + // -------------------------------------------------------------------- + // A2: state_override replaces persisted slot. + // -------------------------------------------------------------------- + #[test] + fn state_override_replaces_persisted_slot() { + new_test_ext().execute_with(|| { + let contract = H160::repeat_byte(0xA1); + deploy_runtime_code(contract, SLOAD_RETURN_BYTECODE); + + // Seed on-chain storage: slot 1 -> 0x11. + AccountStorages::::insert(contract, slot(1), value(0x11)); + + // Control: with no override, SLOAD(1) reads the on-chain value. + let baseline = eth_call_with_override(contract, slot_calldata(1), None); + assert_succeeded(&baseline); + assert_eq!(baseline.value, expected_return(0x11)); + + // With override: SLOAD(1) reads the override value, not the on-chain one. + let override_payload = Some(vec![(contract, vec![(slot(1), value(0x22))])]); + let overridden = eth_call_with_override(contract, slot_calldata(1), override_payload); + assert_succeeded(&overridden); + assert_eq!(overridden.value, expected_return(0x22)); + + // On-chain storage must be unchanged: eth_call does not commit. + assert_eq!( + AccountStorages::::get(contract, slot(1)), + value(0x11), + "persisted storage must not be mutated by an overridden eth_call", + ); + }); + } + + // -------------------------------------------------------------------- + // A3: empty state_override wipes all slots (Geth `"state": {}` case). + // -------------------------------------------------------------------- + // + // This is the exact payload shape that previously forced the RPC layer to + // enumerate every on-chain storage key for the target address (the DoS + // vector). With the new runtime-API path, a full wipe costs O(1) in the + // caller payload — the runtime simply marks the address as overridden with + // no surviving slots. + #[test] + fn state_override_empty_wipes_all_slots() { + new_test_ext().execute_with(|| { + let contract = H160::repeat_byte(0xA2); + deploy_runtime_code(contract, SLOAD_RETURN_BYTECODE); + + // Seed several on-chain slots. + AccountStorages::::insert(contract, slot(1), value(0x11)); + AccountStorages::::insert(contract, slot(2), value(0x22)); + AccountStorages::::insert(contract, slot(3), value(0x33)); + + // Full-wipe override: empty slot vec for this account. + let wipe_override = Some(vec![(contract, Vec::<(H256, H256)>::new())]); + + for idx in [1u64, 2, 3, 99] { + let info = + eth_call_with_override(contract, slot_calldata(idx), wipe_override.clone()); + assert_succeeded(&info); + assert_eq!( + info.value, + expected_return(0), + "slot {idx} must read as zero under a full-wipe override", + ); + } + + // Persisted storage must still be intact after the eth_call. + assert_eq!(AccountStorages::::get(contract, slot(1)), value(0x11)); + assert_eq!(AccountStorages::::get(contract, slot(2)), value(0x22)); + assert_eq!(AccountStorages::::get(contract, slot(3)), value(0x33)); + }); + } + + // -------------------------------------------------------------------- + // A4: state_override is per-account. + // -------------------------------------------------------------------- + // + // Overriding account A must not affect reads of account B. + #[test] + fn state_override_is_per_account() { + new_test_ext().execute_with(|| { + let contract_a = H160::repeat_byte(0xA4); + let contract_b = H160::repeat_byte(0xB4); + deploy_runtime_code(contract_a, SLOAD_RETURN_BYTECODE); + deploy_runtime_code(contract_b, SLOAD_RETURN_BYTECODE); + + AccountStorages::::insert(contract_a, slot(1), value(0x11)); + AccountStorages::::insert(contract_b, slot(1), value(0xAA)); + + // Override only A. + let override_payload = Some(vec![(contract_a, vec![(slot(1), value(0x22))])]); + + let info_a = + eth_call_with_override(contract_a, slot_calldata(1), override_payload.clone()); + assert_succeeded(&info_a); + assert_eq!( + info_a.value, + expected_return(0x22), + "A must read the overridden value" + ); + + let info_b = eth_call_with_override(contract_b, slot_calldata(1), override_payload); + assert_succeeded(&info_b); + assert_eq!( + info_b.value, + expected_return(0xAA), + "B must read on-chain storage, unaffected by A's override" + ); + }); + } + + // -------------------------------------------------------------------- + // A5: missing slot in override reads zero (not the on-chain value). + // -------------------------------------------------------------------- + // + // This is the crux of the "destructed and recreated" semantics: once an + // account is listed in the override map, *every* slot not present in that + // map must read as zero, even if it has a non-zero on-chain value. + #[test] + fn state_override_missing_slot_reads_zero() { + new_test_ext().execute_with(|| { + let contract = H160::repeat_byte(0xA5); + deploy_runtime_code(contract, SLOAD_RETURN_BYTECODE); + + // Seed two slots on chain. + AccountStorages::::insert(contract, slot(1), value(0x11)); + AccountStorages::::insert(contract, slot(2), value(0xAA)); + + // Override specifies only slot 1. + let override_payload = Some(vec![(contract, vec![(slot(1), value(0x22))])]); + + let info_1 = + eth_call_with_override(contract, slot_calldata(1), override_payload.clone()); + assert_succeeded(&info_1); + assert_eq!(info_1.value, expected_return(0x22)); + + let info_2 = eth_call_with_override(contract, slot_calldata(2), override_payload); + assert_succeeded(&info_2); + assert_eq!( + info_2.value, + expected_return(0), + "slot 2 is absent from the override; must read zero despite on-chain 0xAA", + ); + }); + } + + // -------------------------------------------------------------------- + // A6: SSTORE during the call wins over the override baseline. + // -------------------------------------------------------------------- + #[test] + fn sstore_during_call_wins_over_override() { + new_test_ext().execute_with(|| { + let contract = H160::repeat_byte(0xA6); + deploy_runtime_code(contract, SSTORE_THEN_SLOAD_BYTECODE); + + // Override baseline: slot 1 -> 0x22. + let override_payload = Some(vec![(contract, vec![(slot(1), value(0x22))])]); + + // Calldata says: SSTORE(slot=1, value=0x33); SLOAD(1); return. + let info = + eth_call_with_override(contract, slot_value_calldata(1, 0x33), override_payload); + assert_succeeded(&info); + assert_eq!( + info.value, + expected_return(0x33), + "write made during the call must take precedence over the override baseline", + ); + }); + } + + // -------------------------------------------------------------------- + // A7: state_override does not persist across calls. + // -------------------------------------------------------------------- + // + // Because eth_call is non-transactional, no state change (override or + // SSTORE) should survive the call. We verify this with a direct on-chain + // storage read and a second, override-less call. + #[test] + fn state_override_does_not_persist_across_calls() { + new_test_ext().execute_with(|| { + let contract = H160::repeat_byte(0xA7); + deploy_runtime_code(contract, SSTORE_THEN_SLOAD_BYTECODE); + + AccountStorages::::insert(contract, slot(1), value(0x11)); + + // First call: override + SSTORE a different value. + let override_payload = Some(vec![(contract, vec![(slot(1), value(0x22))])]); + let first = + eth_call_with_override(contract, slot_value_calldata(1, 0x33), override_payload); + assert_succeeded(&first); + assert_eq!(first.value, expected_return(0x33)); + + // On-chain storage must still hold the original seeded value. + assert_eq!( + AccountStorages::::get(contract, slot(1)), + value(0x11), + "non-transactional call must not mutate on-chain storage", + ); + + // Second call without an override: the original on-chain value must be visible. + // We use the SLOAD-only contract for a clean read. + let reader = H160::repeat_byte(0xA8); + deploy_runtime_code(reader, SLOAD_RETURN_BYTECODE); + AccountStorages::::insert(reader, slot(1), value(0x11)); + + let second = eth_call_with_override(reader, slot_calldata(1), None); + assert_succeeded(&second); + assert_eq!(second.value, expected_return(0x11)); + }); + } + + // -------------------------------------------------------------------- + // A8: `None` state_override uses on-chain storage (regression guard). + // -------------------------------------------------------------------- + // + // A future refactor must not break the non-override path. + #[test] + fn none_state_override_uses_on_chain_storage() { + new_test_ext().execute_with(|| { + let contract = H160::repeat_byte(0xA9); + deploy_runtime_code(contract, SLOAD_RETURN_BYTECODE); + + AccountStorages::::insert(contract, slot(7), value(0x777)); + + let info = eth_call_with_override(contract, slot_calldata(7), None); + assert_succeeded(&info); + assert_eq!(info.value, expected_return(0x777)); + }); + } + + // -------------------------------------------------------------------- + // A9: `reset_storage` clears any active override entry. + // -------------------------------------------------------------------- + // + // Mirrors the Geth-parity guarantee for SELFDESTRUCT (or contract + // re-creation): once an account is destructed, the previously overridden + // storage is gone — subsequent reads fall back to the (now-empty) persisted + // trie rather than the stale override snapshot. + // + // This is a direct backend-level test against `SubstrateStackState` so we + // don't have to hand-craft a SELFDESTRUCT bytecode, which would also have + // to navigate EIP-6780 semantics. + #[test] + fn reset_storage_clears_override_entry() { + new_test_ext().execute_with(|| { + let addr = H160::repeat_byte(0xD1); + let slot_1 = slot(1); + let val_1 = value(0x22); + + // Build a fresh SubstrateStackState with the override active. + let vicinity = Vicinity { + gas_price: U256::zero(), + origin: H160::default(), + }; + let config = ::config().clone(); + let metadata = evm::executor::stack::StackSubstateMetadata::new(1_000_000, &config); + let state_override = Some(vec![(addr, vec![(slot_1, val_1)])]); + let mut state = SubstrateStackState::<'_, '_, Test>::new( + &vicinity, + metadata, + None, + None, + state_override, + ); + + // Before reset: override is active for this slot. + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + val_1, + "override must be honored before reset_storage", + ); + + // SELFDESTRUCT-like: wipe the account's storage. + as evm::executor::stack::StackState<'_>>::reset_storage( + &mut state, addr, + ); + + // After reset: override entry is cleared; subsequent reads fall back to + // `AccountStorages`, which is empty for this address, so they return zero. + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + H256::zero(), + "post-reset read must fall back to (empty) persisted storage and return zero", + ); + }); + } +} diff --git a/ts-tests/tests/test-state-override.ts b/ts-tests/tests/test-state-override.ts index 5a07c23372..7bf3a78d86 100644 --- a/ts-tests/tests/test-state-override.ts +++ b/ts-tests/tests/test-state-override.ts @@ -242,4 +242,72 @@ describeWithFrontier("Frontier RPC (StateOverride)", (context) => { ]); expect(Web3.utils.hexToNumberString(result)).to.equal("35"); }); + + // Geth-parity: `"state": {}` marks the account as destructed-and-recreated + // with no surviving slots. Every pre-existing storage slot must read as + // zero during the call. This was previously the O(N on-chain slots) DoS + // vector; the runtime-API v7 path serves this payload in O(1). + it("empty `state: {}` wipes all slots for the overridden account", async function () { + const { result: before } = await customRequest(context.web3, "eth_call", [ + { + from: GENESIS_ACCOUNT, + to: contractAddress, + data: contract.methods.availableFunds().encodeABI(), + }, + ]); + expect(Web3.utils.hexToNumberString(before)).to.equal("100"); + + const { result: wiped } = await customRequest(context.web3, "eth_call", [ + { + from: GENESIS_ACCOUNT, + to: contractAddress, + data: contract.methods.availableFunds().encodeABI(), + }, + "latest", + { + [contractAddress]: { + state: {}, + }, + }, + ]); + expect(Web3.utils.hexToNumberString(wiped)).to.equal("0"); + + // A subsequent call without the override must still see the original value. + const { result: after } = await customRequest(context.web3, "eth_call", [ + { + from: GENESIS_ACCOUNT, + to: contractAddress, + data: contract.methods.availableFunds().encodeABI(), + }, + ]); + expect(Web3.utils.hexToNumberString(after)).to.equal("100"); + }); + + // Geth-parity: `state` and `stateDiff` are mutually exclusive per-account. + // The RPC must reject the request rather than silently merging them. + it("rejects `state` and `stateDiff` set on the same account", async function () { + const availableFundsKey = Web3.utils.padLeft(Web3.utils.numberToHex(1), 64); + const newValue = Web3.utils.padLeft(Web3.utils.numberToHex(500), 64); + + const response = await customRequest(context.web3, "eth_call", [ + { + from: GENESIS_ACCOUNT, + to: contractAddress, + data: contract.methods.availableFunds().encodeABI(), + }, + "latest", + { + [contractAddress]: { + state: { + [availableFundsKey]: newValue, + }, + stateDiff: { + [availableFundsKey]: newValue, + }, + }, + }, + ]); + expect(response.error, "expected an error payload").to.exist; + expect(response.error.message).to.match(/mutually exclusive/i); + }); }); From cadd9a29bb5042f13df10e3856eaeb25a3ae04b8 Mon Sep 17 00:00:00 2001 From: Artur Gontijo Date: Mon, 20 Apr 2026 08:24:28 -0300 Subject: [PATCH 3/5] coderabbit: nitpick --- client/rpc/src/eth/execute.rs | 3 +-- frame/evm/src/runner/stack.rs | 17 +++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/client/rpc/src/eth/execute.rs b/client/rpc/src/eth/execute.rs index bc7c2df24a..8ea043b618 100644 --- a/client/rpc/src/eth/execute.rs +++ b/client/rpc/src/eth/execute.rs @@ -943,8 +943,7 @@ where &estimate_mode, &Some(access_list), &authorization_list, - &None::>, - &None::>, + &None:: )); let recorder: sp_trie::recorder::Recorder> = Default::default(); diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index 98f203ae1e..e63112138c 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -1036,11 +1036,12 @@ impl<'vicinity, 'config, T: Config> SubstrateStackState<'vicinity, 'config, T> { } } - /// Read a slot for `address`, honoring any active state override. + /// Resolve a storage slot for `address` as the EVM should observe it. /// - /// If `address` is in the override set, the slot is served from the in-memory - /// map only; missing slots return zero. Otherwise the on-chain trie is read. - fn read_persisted_storage(&self, address: H160, index: H256) -> H256 { + /// If an active state override covers `address`, the slot is served from the + /// in-memory override map (missing slots return zero, matching Geth's + /// "fresh state object" semantics). Otherwise the on-chain trie is read. + fn read_effective_storage(&self, address: H160, index: H256) -> H256 { if let Some(slots) = self.state_override.get(&address) { return slots.get(&index).copied().unwrap_or_default(); } @@ -1168,7 +1169,7 @@ where } fn storage(&self, address: H160, index: H256) -> H256 { - self.read_persisted_storage(address, index) + self.read_effective_storage(address, index) } fn transient_storage(&self, address: H160, index: H256) -> H256 { @@ -1183,7 +1184,7 @@ where self.original_storage .get(&(address, index)) .cloned() - .unwrap_or_else(|| self.read_persisted_storage(address, index)), + .unwrap_or_else(|| self.read_effective_storage(address, index)), ) } } @@ -1238,7 +1239,7 @@ where // We cache the current value if this is the first time we modify it // in the transaction. use alloc::collections::btree_map::Entry; - let original = self.read_persisted_storage(address, index); + let original = self.read_effective_storage(address, index); if let Entry::Vacant(e) = self.original_storage.entry((address, index)) { // No need to cache if same value. if original != value { @@ -1249,7 +1250,7 @@ where // Geth-parity: if this account has an active state override, writes // update the override map (the account's "fresh state"), not the // underlying trie. Subsequent SLOADs will observe the new value via - // `read_persisted_storage`, while the on-chain storage stays untouched. + // `read_effective_storage`, while the on-chain storage stays untouched. if let Some(slots) = self.state_override.get_mut(&address) { if value == H256::default() { slots.remove(&index); From 2f13b7b063da47a58df4809896455c035203df56 Mon Sep 17 00:00:00 2001 From: Artur Gontijo Date: Mon, 20 Apr 2026 09:09:59 -0300 Subject: [PATCH 4/5] coderabbit: add SubstrateStackState::state_override_journal --- frame/evm/src/runner/stack.rs | 25 ++++ frame/evm/src/tests.rs | 210 ++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+) diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index e63112138c..f675da0519 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -993,6 +993,14 @@ pub struct SubstrateStackState<'vicinity, 'config, T> { /// instead of the on-chain trie. An entry with an empty inner map represents a full /// wipe (Geth-style `"state": {}`). state_override: BTreeMap>, + /// Per-frame snapshots of `state_override`, pushed on `enter` and popped on + /// `exit_{commit,revert,discard}`. On revert/discard the popped snapshot is + /// restored as the current `state_override`, mirroring the Substrate storage + /// transaction rollback that `SubstrateStackSubstate` performs for the trie. + /// This keeps override mutations (SSTORE, SELFDESTRUCT-driven `reset_storage`) + /// journaled along the EVM call-frame chain, matching Geth's StateDB snapshot + /// semantics. + state_override_journal: Vec>>, recorded: Recorded, weight_info: Option, storage_meter: Option, @@ -1030,6 +1038,7 @@ impl<'vicinity, 'config, T: Config> SubstrateStackState<'vicinity, 'config, T> { original_storage: BTreeMap::new(), transient_storage: BTreeMap::new(), state_override, + state_override_journal: Vec::new(), recorded: Default::default(), weight_info, storage_meter, @@ -1202,18 +1211,34 @@ where } fn enter(&mut self, gas_limit: u64, is_static: bool) { + // Journal the override alongside the Substrate storage transaction that + // `SubstrateStackSubstate::enter` opens, so an inner frame's SSTORE or + // SELFDESTRUCT-driven `reset_storage` against an overridden account can + // be rolled back on revert/discard (Geth StateDB parity). + self.state_override_journal + .push(self.state_override.clone()); self.substate.enter(gas_limit, is_static) } fn exit_commit(&mut self) -> Result<(), ExitError> { + // Keep the mutated `state_override`; just drop the snapshot. + let _ = self.state_override_journal.pop(); self.substate.exit_commit() } fn exit_revert(&mut self) -> Result<(), ExitError> { + self.state_override = self + .state_override_journal + .pop() + .expect("exit_revert paired with enter; snapshot always pushed"); self.substate.exit_revert() } fn exit_discard(&mut self) -> Result<(), ExitError> { + self.state_override = self + .state_override_journal + .pop() + .expect("exit_discard paired with enter; snapshot always pushed"); self.substate.exit_discard() } diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs index 97fbbb9a18..a73a5d0213 100644 --- a/frame/evm/src/tests.rs +++ b/frame/evm/src/tests.rs @@ -2263,4 +2263,214 @@ mod state_override_test { ); }); } + + // -------------------------------------------------------------------- + // A10: reverted inner-frame SSTORE under an override is rolled back. + // -------------------------------------------------------------------- + // + // Mirrors a realistic `eth_call` shape: outer frame runs against an account + // that has a state override; an inner CALL frame SSTOREs a slot on that + // account and REVERTs. The outer frame's subsequent SLOAD must observe the + // original override value — the reverted write must not leak across the + // frame boundary. + // + // Exercises the `state_override_journal` snapshot-on-enter / restore-on-revert + // path directly against `SubstrateStackState`, matching Geth StateDB + // snapshot semantics. Pre-journaling, this test would observe `inner_write` + // after the revert. + #[test] + fn revert_restores_sstore_under_override() { + new_test_ext().execute_with(|| { + let addr = H160::repeat_byte(0xE1); + let slot_1 = slot(1); + let initial = value(0x22); + let inner_write = value(0x33); + + let vicinity = Vicinity { + gas_price: U256::zero(), + origin: H160::default(), + }; + let config = ::config().clone(); + let metadata = evm::executor::stack::StackSubstateMetadata::new(1_000_000, &config); + let state_override = Some(vec![(addr, vec![(slot_1, initial)])]); + let mut state = SubstrateStackState::<'_, '_, Test>::new( + &vicinity, + metadata, + None, + None, + state_override, + ); + + // Top-level view: override is active. + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + initial, + ); + + // Enter an inner frame (simulating CALL into another contract that + // reaches back into `addr`'s storage). + as evm::executor::stack::StackState<'_>>::enter( + &mut state, 0, false, + ); + + // Inner-frame SSTORE: because `addr` has an active override, this + // updates the override map (the account's "fresh state"), not the trie. + as evm::executor::stack::StackState<'_>>::set_storage( + &mut state, addr, slot_1, inner_write, + ); + + // Inner frame sees the write. + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + inner_write, + "inner frame must observe its own SSTORE", + ); + + // Inner frame reverts. The journal must restore the pre-enter snapshot. + as evm::executor::stack::StackState<'_>>::exit_revert( + &mut state, + ) + .expect("exit_revert succeeds"); + + // Outer frame SLOAD must see the original override, not the reverted write. + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + initial, + "reverted inner SSTORE must not leak into outer frame's SLOAD", + ); + }); + } + + // -------------------------------------------------------------------- + // A11: reverted inner-frame SELFDESTRUCT-style reset under an override + // is rolled back. + // -------------------------------------------------------------------- + // + // When an inner frame calls `reset_storage` (the code path taken by + // SELFDESTRUCT and contract re-creation) on an overridden account and + // then REVERTs, the outer frame must still see the account as + // overridden. Pre-journaling, the override entry removal was permanent + // regardless of revert, causing outer-frame SLOADs to read zero. + #[test] + fn revert_restores_selfdestruct_override_wipe() { + new_test_ext().execute_with(|| { + let addr = H160::repeat_byte(0xE2); + let slot_1 = slot(1); + let initial = H256::from_low_u64_be(0xABC); + + let vicinity = Vicinity { + gas_price: U256::zero(), + origin: H160::default(), + }; + let config = ::config().clone(); + let metadata = evm::executor::stack::StackSubstateMetadata::new(1_000_000, &config); + let state_override = Some(vec![(addr, vec![(slot_1, initial)])]); + let mut state = SubstrateStackState::<'_, '_, Test>::new( + &vicinity, + metadata, + None, + None, + state_override, + ); + + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + initial, + ); + + as evm::executor::stack::StackState<'_>>::enter( + &mut state, 0, false, + ); + + // Inner-frame SELFDESTRUCT-style wipe: clears the override entry + // and wipes on-chain storage (empty here, so a no-op on the trie). + as evm::executor::stack::StackState<'_>>::reset_storage( + &mut state, addr, + ); + + // Inner frame sees the wipe. + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + H256::zero(), + "inner frame must observe its own reset", + ); + + // Inner frame reverts. The journal must restore the override map. + as evm::executor::stack::StackState<'_>>::exit_revert( + &mut state, + ) + .expect("exit_revert succeeds"); + + // Outer frame: override must be back in effect. + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + initial, + "reverted reset_storage must not leak into outer frame's SLOAD", + ); + }); + } + + // -------------------------------------------------------------------- + // A12: committed inner-frame mutations under an override survive. + // -------------------------------------------------------------------- + // + // Counterpart to A10: on a normal (non-revert) exit, inner-frame SSTOREs + // against an overridden account must propagate to the outer frame. This + // guards against overzealous journaling that would snapshot-restore on + // `exit_commit`. + #[test] + fn commit_preserves_sstore_under_override() { + new_test_ext().execute_with(|| { + let addr = H160::repeat_byte(0xE3); + let slot_1 = slot(1); + let initial = value(0x22); + let inner_write = value(0x33); + + let vicinity = Vicinity { + gas_price: U256::zero(), + origin: H160::default(), + }; + let config = ::config().clone(); + let metadata = evm::executor::stack::StackSubstateMetadata::new(1_000_000, &config); + let state_override = Some(vec![(addr, vec![(slot_1, initial)])]); + let mut state = SubstrateStackState::<'_, '_, Test>::new( + &vicinity, + metadata, + None, + None, + state_override, + ); + + as evm::executor::stack::StackState<'_>>::enter( + &mut state, 0, false, + ); + as evm::executor::stack::StackState<'_>>::set_storage( + &mut state, addr, slot_1, inner_write, + ); + as evm::executor::stack::StackState<'_>>::exit_commit( + &mut state, + ) + .expect("exit_commit succeeds"); + + assert_eq!( + as evm::backend::Backend>::storage( + &state, addr, slot_1, + ), + inner_write, + "committed inner SSTORE must be visible to the outer frame", + ); + }); + } } From 85310d7578f1f2ff75048787b0c8f3fc45f5553e Mon Sep 17 00:00:00 2001 From: Artur Gontijo Date: Mon, 20 Apr 2026 09:28:34 -0300 Subject: [PATCH 5/5] coderabbit: per-frame fast path --- frame/evm/src/runner/stack.rs | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index f675da0519..95345fff63 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -993,14 +993,11 @@ pub struct SubstrateStackState<'vicinity, 'config, T> { /// instead of the on-chain trie. An entry with an empty inner map represents a full /// wipe (Geth-style `"state": {}`). state_override: BTreeMap>, - /// Per-frame snapshots of `state_override`, pushed on `enter` and popped on - /// `exit_{commit,revert,discard}`. On revert/discard the popped snapshot is - /// restored as the current `state_override`, mirroring the Substrate storage - /// transaction rollback that `SubstrateStackSubstate` performs for the trie. - /// This keeps override mutations (SSTORE, SELFDESTRUCT-driven `reset_storage`) - /// journaled along the EVM call-frame chain, matching Geth's StateDB snapshot - /// semantics. - state_override_journal: Vec>>, + /// Per-frame snapshots of `state_override`, pushed on `enter` and restored + /// on `exit_revert`/`exit_discard` (Geth StateDB snapshot parity). + /// `None` entries skip the clone when the override map is empty — safe + /// because `state_override` only shrinks during execution. + state_override_journal: Vec>>>, recorded: Recorded, weight_info: Option, storage_meter: Option, @@ -1215,8 +1212,10 @@ where // `SubstrateStackSubstate::enter` opens, so an inner frame's SSTORE or // SELFDESTRUCT-driven `reset_storage` against an overridden account can // be rolled back on revert/discard (Geth StateDB parity). - self.state_override_journal - .push(self.state_override.clone()); + // `None` when empty: child frames can't mutate an empty map, so we skip + // the clone while still pairing the push/pop with the substate frame. + let snapshot = (!self.state_override.is_empty()).then(|| self.state_override.clone()); + self.state_override_journal.push(snapshot); self.substate.enter(gas_limit, is_static) } @@ -1227,18 +1226,24 @@ where } fn exit_revert(&mut self) -> Result<(), ExitError> { - self.state_override = self + if let Some(snapshot) = self .state_override_journal .pop() - .expect("exit_revert paired with enter; snapshot always pushed"); + .expect("exit_revert paired with enter; snapshot always pushed") + { + self.state_override = snapshot; + } self.substate.exit_revert() } fn exit_discard(&mut self) -> Result<(), ExitError> { - self.state_override = self + if let Some(snapshot) = self .state_override_journal .pop() - .expect("exit_discard paired with enter; snapshot always pushed"); + .expect("exit_discard paired with enter; snapshot always pushed") + { + self.state_override = snapshot; + } self.substate.exit_discard() }