From a1155ab78056eca730343d4268b851750e453166 Mon Sep 17 00:00:00 2001 From: Karrq Date: Fri, 30 Aug 2024 07:57:16 +0200 Subject: [PATCH] fix(zk): avoid observing log from staticcalls (#547) * fix(zk): avoid observing log from staticcalls * test(zk): check not obversing staticcall logs * fix(zk): passthru console.logs (but don't record) * test(zk): check console.log passthru * chore: remove unnecessary log statement --- crates/cheatcodes/src/inspector.rs | 27 +++++++++++--------- crates/forge/tests/it/zk/cheats.rs | 2 +- testdata/zk/Cheatcodes.t.sol | 40 ++++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index f8954a882..88d09bfba 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -1395,14 +1395,6 @@ impl Cheatcodes { persisted_factory_deps: Some(&mut self.persisted_factory_deps), }; if let Ok(result) = foundry_zksync_core::vm::call::<_, DatabaseError>(call, ecx, ccx) { - if let Some(recorded_logs) = &mut self.recorded_logs { - recorded_logs.extend(result.logs.clone().into_iter().map(|log| Vm::Log { - topics: log.data.topics().to_vec(), - data: log.data.data.clone(), - emitter: log.address, - })); - } - // append console logs from zkEVM to the current executor's LogTracer result.logs.iter().filter_map(decode_console_log).for_each(|decoded_log| { executor.console_log( @@ -1417,10 +1409,21 @@ impl Cheatcodes { ); }); - // for each log in cloned logs call handle_expect_emit - if !self.expected_emits.is_empty() { - for log in result.logs { - expect::handle_expect_emit(self, &log); + // skip log processing for static calls + if !call.is_static { + if let Some(recorded_logs) = &mut self.recorded_logs { + recorded_logs.extend(result.logs.clone().into_iter().map(|log| Vm::Log { + topics: log.data.topics().to_vec(), + data: log.data.data.clone(), + emitter: log.address, + })); + } + + // for each log in cloned logs call handle_expect_emit + if !self.expected_emits.is_empty() { + for log in result.logs { + expect::handle_expect_emit(self, &log); + } } } diff --git a/crates/forge/tests/it/zk/cheats.rs b/crates/forge/tests/it/zk/cheats.rs index 98a3f066c..135e3c333 100644 --- a/crates/forge/tests/it/zk/cheats.rs +++ b/crates/forge/tests/it/zk/cheats.rs @@ -58,7 +58,7 @@ async fn test_zk_cheat_record_works() { #[tokio::test(flavor = "multi_thread")] async fn test_zk_cheat_expect_emit_works() { let runner = TEST_DATA_DEFAULT.runner_zksync(); - let filter = Filter::new("testExpectEmit|testExpectEmitOnCreate", "ZkCheatcodesTest", ".*"); + let filter = Filter::new("testExpectEmit", "ZkCheatcodesTest", ".*"); TestConfig::with_filter(runner, filter).evm_spec(SpecId::SHANGHAI).run().await; } diff --git a/testdata/zk/Cheatcodes.t.sol b/testdata/zk/Cheatcodes.t.sol index 6d66913da..f8a3e5fa9 100644 --- a/testdata/zk/Cheatcodes.t.sol +++ b/testdata/zk/Cheatcodes.t.sol @@ -53,12 +53,19 @@ contract Emitter { event EventConstructor(string message); event EventFunction(string message); + string public constant CONSTRUCTOR_MESSAGE = "constructor"; + string public constant FUNCTION_MESSAGE = "function"; + constructor() { - emit EventConstructor("constructor"); + emit EventConstructor(CONSTRUCTOR_MESSAGE); } function functionEmit() public { - emit EventFunction("function"); + emit EventFunction(FUNCTION_MESSAGE); + } + + function emitConsole(string memory message) public view { + console.log(message); } } @@ -157,6 +164,14 @@ contract ZkCheatcodesTest is DSTest { new Emitter(); } + function testExpectEmitIgnoresStaticCalls() public { + Emitter emitter = new Emitter(); + + vm.expectEmit(true, true, true, true); + emit EventFunction(emitter.FUNCTION_MESSAGE()); + emitter.functionEmit(); + } + function testZkCheatcodesValueFunctionMockReturn() public { InnerMock inner = new InnerMock(); // Send some funds to so it can pay for the inner call @@ -224,6 +239,27 @@ contract ZkCheatcodesTest is DSTest { assertEq(entries[10].data, abi.encode("function")); // 11: EthToken } + + function testRecordConsoleLogsLikeEVM() public { + Emitter emitter = new Emitter(); + vm.makePersistent(address(emitter)); + + // ensure we are in zkvm + (bool _success, bytes memory _ret) = address(vm).call(abi.encodeWithSignature("zkVm(bool)", true)); + + vm.recordLogs(); + emitter.emitConsole("zkvm"); + Vm.Log[] memory zkvmEntries = vm.getRecordedLogs(); + + // ensure we are NOT in zkvm + (_success, _ret) = address(vm).call(abi.encodeWithSignature("zkVm(bool)", false)); + + vm.recordLogs(); + emitter.emitConsole("evm"); + Vm.Log[] memory evmEntries = vm.getRecordedLogs(); + + assertEq(zkvmEntries.length, evmEntries.length); + } } contract UsesCheatcodes {