matter-labs / foundry-zksync

Fork of Foundry tailored for zkSync environment
Apache License 2.0
299 stars 130 forks source link

`vm.expectEmit` cheatcode seems to not work when there is a contract call in the expected event #546

Closed brotherlymite closed 2 months ago

brotherlymite commented 2 months ago

Component

Forge

Have you ensured that all of these are up to date?

What version of Foundry are you on?

forge 0.0.2 (543f39e 2024-08-26T00:32:28.186201000Z)

What command(s) is the bug in?

forge test --zksync

Operating System

macOS (Apple Silicon)

Describe the bug

vm.expectEmit cheatcode does not work and throws [FAIL. Reason: log != expected log] if there is a contract call in the expected events like the following:

vm.expectEmit(address(ghost));
emit Message(ghost.MESSAGE());

If the contract call ghost.MESSAGE() is replaced by the message directly like, it seems to work fine:

vm.expectEmit(address(ghost));
emit Message('BOO');

To reproduce the issue, run FOUNDRY_PROFILE=zksync forge test --zksync --match-contract GhostTest -vv on this repo. The same behaviour seems to work on the normal foundry which can be tested by running forge test on the same repo above.

Karrq commented 2 months ago

Hi! Thanks for the report.

After looking into this, we determined that the behavior is because expectEmit is observing some system logs emitted by the zkVM during transaction processing. This is because, unlike in the succesfull test, there exist a call being executed in zkVM before your emit statement. This call (let's pretend it wasn't a static call) then produces some system logs, which are observed by expectEmit, making the first of these the "target", instead of the one emitted in the test.

These system logs are produced everytime a call (or create) is executed in the zkVM.

In pseudo-solidity:

vm.expectEmit();
zkVm(ghost.MESSAGE()); // produces system logs, observed by expectEmit
emit Message(data); // would normally be the target, but is now ignored
zkVm(ghost.boo()); // also produces some system logs, but not the same ones as before

In your particular case, we actually identified a bug, where we are observing all logs emitted, including those by STATICCALLs (which ghost.MESSAGE() would be), which will be fixed once #547 is merged.

The solution to the general case is making sure any calls to the zkVM are executed before expectEmit would avoid observing the wrong logs. Hope this helps!

tomimor commented 2 months ago

Hi @brotherlymite, did this help? Please let us know if you need further assistance. Thanks!

dutterbutter commented 2 months ago

Closing due staleness. Reopen if need be.

brotherlymite commented 2 months ago

Hey, the issue was fixed by #547. Thank you.