diff --git a/crates/evm/evm/src/inspectors/cheatcodes/mod.rs b/crates/evm/evm/src/inspectors/cheatcodes/mod.rs index e9cc1736506a..2911f0c0ae34 100644 --- a/crates/evm/evm/src/inspectors/cheatcodes/mod.rs +++ b/crates/evm/evm/src/inspectors/cheatcodes/mod.rs @@ -831,9 +831,47 @@ impl Inspector for Cheatcodes { } } + // if there's a revert and a previous call was diagnosed as fork related revert then we can + // return a better error here + if status == InstructionResult::Revert { + if let Some(err) = self.fork_revert_diagnostic.take() { + return ( + status, + remaining_gas, + DynSolValue::String(err.to_error_msg(&self.labels)).abi_encode().into(), + ) + } + } + + // this will ensure we don't have false positives when trying to diagnose reverts in fork + // mode + let _ = self.fork_revert_diagnostic.take(); + + // try to diagnose reverts in multi-fork mode where a call is made to an address that does + // not exist + if let TransactTo::Call(test_contract) = data.env.tx.transact_to { + // if a call to a different contract than the original test contract returned with + // `Stop` we check if the contract actually exists on the active fork + if data.db.is_forked_mode() && + status == InstructionResult::Stop && + call.contract != test_contract + { + self.fork_revert_diagnostic = + data.db.diagnose_revert(call.contract, &data.journaled_state); + } + } + // If the depth is 0, then this is the root call terminating if data.journaled_state.depth() == 0 { - // Match expected calls + // If we already have a revert, we shouldn't run the below logic as it can obfuscate an + // earlier error that happened first with unrelated information about + // another error when using cheatcodes. + if status == InstructionResult::Revert { + return (status, remaining_gas, retdata) + } + + // If there's not a revert, we can continue on to run the last logic for expect* + // cheatcodes. Match expected calls for (address, calldatas) in &self.expected_calls { // Loop over each address, and for each address, loop over each calldata it expects. for (calldata, (expected, actual_count)) in calldatas { @@ -918,36 +956,6 @@ impl Inspector for Cheatcodes { } } - // if there's a revert and a previous call was diagnosed as fork related revert then we can - // return a better error here - if status == InstructionResult::Revert { - if let Some(err) = self.fork_revert_diagnostic.take() { - return ( - status, - remaining_gas, - DynSolValue::String(err.to_error_msg(&self.labels)).abi_encode().into(), - ) - } - } - - // this will ensure we don't have false positives when trying to diagnose reverts in fork - // mode - let _ = self.fork_revert_diagnostic.take(); - - // try to diagnose reverts in multi-fork mode where a call is made to an address that does - // not exist - if let TransactTo::Call(test_contract) = data.env.tx.transact_to { - // if a call to a different contract than the original test contract returned with - // `Stop` we check if the contract actually exists on the active fork - if data.db.is_forked_mode() && - status == InstructionResult::Stop && - call.contract != test_contract - { - self.fork_revert_diagnostic = - data.db.diagnose_revert(call.contract, &data.journaled_state); - } - } - (status, remaining_gas, retdata) } diff --git a/crates/forge/tests/it/repros.rs b/crates/forge/tests/it/repros.rs index b040645c19fe..45f6aeb15c89 100644 --- a/crates/forge/tests/it/repros.rs +++ b/crates/forge/tests/it/repros.rs @@ -6,6 +6,7 @@ use crate::{ }; use alloy_primitives::Address; use ethers::abi::{Event, EventParam, Log, LogParam, ParamType, RawLog, Token}; +use forge::result::TestStatus; use foundry_config::{fs_permissions::PathPermission, Config, FsPermissions}; use std::str::FromStr; @@ -308,3 +309,13 @@ async fn test_issue_5808() { async fn test_issue_6115() { test_repro!("Issue6115"); } + +// +#[tokio::test(flavor = "multi_thread")] +async fn test_issue_6170() { + let mut res = run_test_repro!("Issue6170"); + let mut res = res.remove("repros/Issue6170.t.sol:Issue6170Test").unwrap(); + let test = res.test_results.remove("test()").unwrap(); + assert_eq!(test.status, TestStatus::Failure); + assert_eq!(test.reason, Some("Log != expected log".to_string())); +} diff --git a/testdata/repros/Issue6170.t.sol b/testdata/repros/Issue6170.t.sol new file mode 100644 index 000000000000..e5070b2edb5d --- /dev/null +++ b/testdata/repros/Issue6170.t.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.18; + +import "ds-test/test.sol"; +import "../cheats/Vm.sol"; + +contract Emitter { + event Values(uint256 indexed a, uint256 indexed b); + + function plsEmit(uint256 a, uint256 b) external { + emit Values(a, b); + } +} + +// https://github.com/foundry-rs/foundry/issues/6170 +contract Issue6170Test is DSTest { + Vm constant vm = Vm(HEVM_ADDRESS); + + event Values(uint256 indexed a, uint256 b); + + Emitter e = new Emitter(); + + function test() public { + vm.expectEmit(true, true, false, true); + emit Values(69, 420); + e.plsEmit(69, 420); + } +}