Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(forge): Add expectEmit cheatcode #329

Merged
merged 6 commits into from
Dec 29, 2021
Merged

feat(forge): Add expectEmit cheatcode #329

merged 6 commits into from
Dec 29, 2021

Conversation

brockelmore
Copy link
Member

@brockelmore brockelmore commented Dec 29, 2021

Closes #310

This works by grabbing the next emitted event. You specify if you want to match against topics 1-3 and the data in the call. You can expect multiple events by repeatedly doing vm.expectEvent then emitting an event prior to a call. After the call, if the expected events are found, it continues execution. Otherwise, if the expected logs aren't found, it reverts.

interface Vm {
    function expectEmit(bool,bool,bool,bool) external;
}

contract T is DSTest {
    Vm vm = Vm(HEVM_ADDRESS);
    event Transfer(address indexed from,address indexed to, uint256 amount);
    function testExpectEmit() public {
        ExpectEmit emitter = new ExpectEmit();
        // check topic 1, topic 2, and data are the same as the following emitted event
        hevm.expectEmit(true,true,false,true);
        emit Transfer(address(this), address(1337), 1337);
        emitter.t();
    }
}

contract ExpectEmit {
    event Transfer(address indexed from,address indexed to, uint256 amount);
    function t() public {
        emit Transfer(msg.sender, address(1337), 1337);
    }
}

@wilsoncusack
Copy link
Contributor

Could the expectEmit take any number of bools? Event could have more than four args/topics? I find the syntax confusing. Thoughts on just expecting all to match? Too constraining?

@brockelmore
Copy link
Member Author

brockelmore commented Dec 29, 2021

Could the expectEmit take any number of bools? Event could have more than four args/topics? I find the syntax confusing. Thoughts on just expecting all to match? Too constraining?

No. There can only be 4 topics (3 user defined, topic 0 is always the event signature) max emitted by an event as defined by the available evm opcodes. Anything marked as indexed is put as a topic. All non-indexed are encoded into the data field. Thus (check_topic1, check_topic2, check_topic3, check_data) is basically the function signature

Copy link
Collaborator

@mds1 mds1 left a comment

Choose a reason for hiding this comment

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

Tested a few permutations locally and everything passed/failed as I'd expect, so functionality-wise this looks good to me.

Agreed this function signature makes sense and is the simplest approach. I can see how it's a little confusing in the case where your event doesn't have 4 topics and some inputs are therefore unused, but I think that's ok.

One nuisance of this approach is you have to re-declare your events in the test file as shown in the example here, but a workaround is to define all your contract's events in an interface and just inherit from that in your test contract

Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

LGTM.

Writing a mini code walkthrough below for anyone following along, or if you want to add it as a doc in the code:

  1. User calls the vm.expectEmit(...) cheatcode. This pushes an ExpectedEmit to the expected_emits vector on the state, with log = None and found = false. The expected depth is d+1 because it's expected to appear in the subcall and not in the current call.
  2. User calls emit Transfer(...) (or whatever event they want to use). This triggers a evm.log call on SputnikEVM which hits the if branch in L1188 and populates the FIRST of the expected_emits where log = None with the raw log that was emitted. (This is important because there may be >1 expected emits in a call)
  3. Then the user makes the actual subcall, where an event gets emitted. By that time, there are no expected_emit with log = None, which means we hit the else branch which looks for the first expected_emit with found = false, and proceeds to check that the actual emitted log topics / data match the ones in the found expected_emit.

CAVEAT / LIMITATION:

This does not support matching on the data (unindexed topics) if there's >1 params, because it checks the entire concatenated data, vs giving granularity over specific event params which were unindexed. We are merging this as-is, but if people end up using it, we may need to get more granular.

brockelmore and others added 2 commits December 29, 2021 14:17
Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>
Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>
@brockelmore brockelmore merged commit 74ccffa into master Dec 29, 2021
@brockelmore brockelmore deleted the brock/expectEmit branch December 29, 2021 21:22
@andrewhong5297
Copy link

This is awesome! I've done a bunch of work on data decoding > 1 param for events and functions calle, if that's a large need I can try to contribute that utility.

Would just need some time to get better at Rust lol.

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.

expect emit
5 participants