fix: avoid creating extra journal entries (#7493)

* fix: avoid creating extra journal entries

* add test

* fmt

* graceful error handling

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<I: Inspector<Backend>>(
     };
     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<DB: Database>(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<revm::primitives::Address, Account>,
     journaled_state: &mut JournaledState,
     fork: &mut Fork,
-) {
-    let changed_accounts = state.keys().copied().collect::<Vec<_>>();
+) -> 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(())
 }

crates/evm/evm/src/inspectors/stack.rs

@@ -3,15 +3,18 @@ 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::{
     interpreter::{
         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<DB: DatabaseExt>(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<Log>,
@@ -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 {

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);

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");
+    }
+}