Skip to content

feat(cheatcodes): V1 expectRevert behavior #7238

@Evalir

Description

@Evalir

Motivation

expect* cheatcodes are ALL intended to function only for the next call. That means, if the next call does not fulfill the expect* set up before making it, tests should fail. However, this is not the case for expectRevert works right now.

See: #3723, #3437, #4832 .

expectRevert, since inception, works differently from intended: it matches reverts at the test level, not at the next call level. This means that reverting cheatcodes were being incorrectly catched by expectRevert (we're supposed to bypass checks for cheatcode calls) and also led users to use cheatcode incorrectly as illustrated in #3723, sometimes becoming actual accepted foundry patterns. It also meant that, if used to catch any revert without matching revert data, any revert at any point in the test after the expectRevert cheatcode was used could make the test pass. This is especially dangerous as it can hide actual code failures from intended reverts.

New behavior

expectRevert now only works on the next CALL. Cheatcode calls are now ignored properly, even if those cheatcode calls revert. To illustrate, after introducing this new behavior, these are examples of now both passing and failing tests:

// OK
function testExpectRevertString() public {
    Reverter reverter = new Reverter();
    cheats.expectRevert("revert");
    reverter.revertWithMessage("revert");
}

// FAIL
function testFailRevertNotOnImmediateNextCall() public {
    Reverter reverter = new Reverter();
    // expectRevert should only work for the next call. However,
    // we do not inmediately revert, so,
    // we fail.
    cheats.expectRevert("revert");
    reverter.doNotRevert();
    reverter.revertWithMessage("revert");
}

// FAIL
function testFailCheatcodeRevert() public {
  // This expectRevert is hanging, as the next cheatcode call is ignored.
  vm.expectRevert();
  vm.fsMetadata(fakePath) // try to go to some non-existent path to cause a revert
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-cheatcodesArea: cheatcodesT-likely-breakingType: requires changes that can be breakingT-to-discussType: requires discussion

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions