Skip to content

feat(cheatcodes): add event expecter which supports anonymous events#8429

Merged
mattsse merged 1 commit intofoundry-rs:masterfrom
lightclient:vm-expect-emit-anonymous
Jul 15, 2024
Merged

feat(cheatcodes): add event expecter which supports anonymous events#8429
mattsse merged 1 commit intofoundry-rs:masterfrom
lightclient:vm-expect-emit-anonymous

Conversation

@lightclient
Copy link
Contributor

@lightclient lightclient commented Jul 12, 2024

closes #7457, closes #7545

--

From #7545:

Add support for a new cheatcode: vm.expectEmitAnonymous(bool checkTopic0, bool checkTopic1, bool checkTopic2, bool checkTopic3, bool checkData) (and corresponding equivalent overloads).

The rationale is that vm.expectEmit always checks topic0, which is the event signature, and checking everything else is optional. But with an anonymous event, you don't have an event signature, so it doesn't make sense to still force the user to check topic0.

This PR introduces the vm.expectEmitAnonymous cheatcode and corresponding overloads. Additionally, it disallows matching anonymous events with no indexed topics via vm.expectEmit. If the anonymous has at least one indexed event, it's impossible to differentiate from a non-anonymous event.

The restriction is necessary, because the way vm.expectEmit works is surprising (see #8416). The value checkTopic1 is basically disregarded. Since Solidity uses it to encode the event signature, it is the best way to match the signature between the expected event and actual.

The ideal solution would be to break this interface and only allow checking of 3 topics via expectEmit, since that is essentially what's possible now.

@lightclient lightclient force-pushed the vm-expect-emit-anonymous branch 6 times, most recently from 0730b5f to d31ec52 Compare July 12, 2024 14:57
@klkvr
Copy link
Member

klkvr commented Jul 12, 2024

The ideal solution would be to break this interface and only allow checking of 3 topics via expectEmit, since that is essentially what's possible now.

Sorry, don't think I am following. I believe that expectEmit already only allows checking only 3 topics (topic1, topic2 and topic3, topic0 being reserved for event signature).

However, I'd expect expectEmitAnonymous to allow matching all 4 topics with signature vm.expectEmitAnonymous(bool checkTopic0, bool checkTopic1, bool checkTopic2, bool checkTopic3, bool checkData)

@lightclient
Copy link
Contributor Author

lightclient commented Jul 12, 2024

Sorry you're right - I got confused by the 4th element in checks but that's actually the data. Let me fix.

@lightclient lightclient force-pushed the vm-expect-emit-anonymous branch 2 times, most recently from 8b4108c to e1f9f1b Compare July 12, 2024 17:45
@lightclient
Copy link
Contributor Author

Okay I updated. Let's see what CI thinks.

@lightclient lightclient force-pushed the vm-expect-emit-anonymous branch from e1f9f1b to 16f2d24 Compare July 12, 2024 17:49
Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, pending @mattsse @klkvr

event_to_fill_or_check.log = Some(log.data.clone());
// Unless the caller is trying to match an anonymous event, the first topic must be
// filled.
// TODO: failing this check should probably cause a warning
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd want to revert here ideally, should be possible after bluealloy/revm#1610

Copy link
Member

@klkvr klkvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add vm.expectEmitAnonymous cheatcode to support anonymous events bug: vm.expectEmit doesn't work with anonymous events

4 participants