diff --git a/crates/evm/core/src/backend/mod.rs b/crates/evm/core/src/backend/mod.rs index f55259f4d1e0..ca6cbb1ef8bd 100644 --- a/crates/evm/core/src/backend/mod.rs +++ b/crates/evm/core/src/backend/mod.rs @@ -17,7 +17,7 @@ use revm::{ precompile::{PrecompileSpecId, Precompiles}, primitives::{ Account, AccountInfo, Bytecode, CreateScheme, Env, EnvWithHandlerCfg, HashMap as Map, Log, - ResultAndState, SpecId, StorageSlot, TransactTo, KECCAK_EMPTY, + ResultAndState, SpecId, State, StorageSlot, TransactTo, KECCAK_EMPTY, }, Database, DatabaseCommit, Inspector, JournaledState, }; @@ -1888,7 +1888,19 @@ fn commit_transaction>( }; trace!(elapsed = ?now.elapsed(), "transacted transaction"); - apply_state_changeset(res.state, journaled_state, fork); + apply_state_changeset(res.state, journaled_state, fork)?; + Ok(()) +} + +/// Helper method which updates data in the state with the data from the database. +pub fn update_state(state: &mut State, db: &mut DB) -> Result<(), DB::Error> { + for (addr, acc) in state.iter_mut() { + acc.info = db.basic(*addr)?.unwrap_or_default(); + for (key, val) in acc.storage.iter_mut() { + val.present_value = db.storage(*addr, *key)?; + } + } + Ok(()) } @@ -1898,19 +1910,12 @@ fn apply_state_changeset( state: Map, journaled_state: &mut JournaledState, fork: &mut Fork, -) { - let changed_accounts = state.keys().copied().collect::>(); +) -> Result<(), DatabaseError> { // commit the state and update the loaded accounts fork.db.commit(state); - for addr in changed_accounts { - // reload all changed accounts by removing them from the journaled state and reloading them - // from the now updated database - if journaled_state.state.remove(&addr).is_some() { - let _ = journaled_state.load_account(addr, &mut fork.db); - } - if fork.journaled_state.state.remove(&addr).is_some() { - let _ = fork.journaled_state.load_account(addr, &mut fork.db); - } - } + update_state(&mut journaled_state.state, &mut fork.db)?; + update_state(&mut fork.journaled_state.state, &mut fork.db)?; + + Ok(()) } diff --git a/crates/evm/evm/src/inspectors/stack.rs b/crates/evm/evm/src/inspectors/stack.rs index ea0af6bbc873..6f83bf43f867 100644 --- a/crates/evm/evm/src/inspectors/stack.rs +++ b/crates/evm/evm/src/inspectors/stack.rs @@ -3,7 +3,10 @@ use super::{ StackSnapshotType, TracingInspector, TracingInspectorConfig, }; use alloy_primitives::{Address, Bytes, Log, U256}; -use foundry_evm_core::{backend::DatabaseExt, debug::DebugArena}; +use foundry_evm_core::{ + backend::{update_state, DatabaseExt}, + debug::DebugArena, +}; use foundry_evm_coverage::HitMaps; use foundry_evm_traces::CallTraceArena; use revm::{ @@ -11,7 +14,7 @@ use revm::{ CallInputs, CallOutcome, CallScheme, CreateInputs, CreateOutcome, Gas, InstructionResult, Interpreter, InterpreterResult, }, - primitives::{BlockEnv, Env, EnvWithHandlerCfg, ExecutionResult, Output, State, TransactTo}, + primitives::{BlockEnv, Env, EnvWithHandlerCfg, ExecutionResult, Output, TransactTo}, DatabaseCommit, EvmContext, Inspector, }; use std::{collections::HashMap, sync::Arc}; @@ -228,16 +231,6 @@ macro_rules! call_inspectors_adjust_depth { }; } -/// Helper method which updates data in the state with the data from the database. -fn update_state(state: &mut State, db: &mut DB) { - for (addr, acc) in state.iter_mut() { - acc.info = db.basic(*addr).unwrap().unwrap_or_default(); - for (key, val) in acc.storage.iter_mut() { - val.present_value = db.storage(*addr, *key).unwrap(); - } - } -} - /// The collected results of [`InspectorStack`]. pub struct InspectorData { pub logs: Vec, @@ -500,8 +493,22 @@ impl InspectorStack { ecx.db.commit(res.state.clone()); // Update both states with new DB data after commit. - update_state(&mut ecx.journaled_state.state, &mut ecx.db); - update_state(&mut res.state, &mut ecx.db); + if let Err(e) = update_state(&mut ecx.journaled_state.state, &mut ecx.db) { + let res = InterpreterResult { + result: InstructionResult::Revert, + output: Bytes::from(e.to_string()), + gas, + }; + return (res, None) + } + if let Err(e) = update_state(&mut res.state, &mut ecx.db) { + let res = InterpreterResult { + result: InstructionResult::Revert, + output: Bytes::from(e.to_string()), + gas, + }; + return (res, None) + } // Merge transaction journal into the active journal. for (addr, acc) in res.state { diff --git a/crates/forge/tests/it/repros.rs b/crates/forge/tests/it/repros.rs index 38cf76331619..02f1aa15eea2 100644 --- a/crates/forge/tests/it/repros.rs +++ b/crates/forge/tests/it/repros.rs @@ -328,3 +328,5 @@ test_repro!(6634; |config| { cheats_config.always_use_create_2_factory = true; config.runner.cheats_config = std::sync::Arc::new(cheats_config); }); + +test_repro!(7481); diff --git a/testdata/default/repros/Issue7481.t.sol b/testdata/default/repros/Issue7481.t.sol new file mode 100644 index 000000000000..eb568dc94666 --- /dev/null +++ b/testdata/default/repros/Issue7481.t.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity 0.8.18; + +import "ds-test/test.sol"; +import "cheats/Vm.sol"; + +// https://github.com/foundry-rs/foundry/issues/7481 +// This test ensures that we don't panic +contract Issue7481Test is DSTest { + Vm constant vm = Vm(HEVM_ADDRESS); + + function testFailTransact() public { + vm.createSelectFork("rpcAlias", 19514903); + + // Transfer some funds to sender of tx being transacted to ensure that it appears in journaled state + payable(address(0x5C60cD7a3D50877Bfebd484750FBeb245D936dAD)).call{value: 1}(""); + vm.transact(0xccfd66fc409a633a99b5b75b0e9a2040fcf562d03d9bee3fefc1a5c0eb49c999); + + // Revert the current call to ensure that revm can revert state journal + revert("HERE"); + } +}