vm.canRevert(): allow tests to revert in expected cases

[0xPhaze]

Component

Forge

Describe the feature you would like

Often it's desirable to check properties of results, such as a new price of an AMM. However, these functions can contain error-checks and revert on certain inputs. And sometimes I want to make sure a property holds only if I actually get a result from a call and ignore the reverting cases.

An example of what this could look like is:

    function test_getPrice_invariant(uint256 x, uint256 dx) public {
        dx = bound(dx, 0, type(uint256).max - x);

        vm.canRevert(Overflow.selector);
        uint256 y1 = amm.getPrice(x);

        vm.canRevert(Overflow.selector);
        uint256 y2 = amm.getPrice(x + dx);
        
        assertGte(y2, y1);
    }

A current workaround is to use low-level calls or try ... catch ..., but these have to be set up for each function manually.

    function getPrice_canRevert(uint256 x, bytes4 selector) internal returns (uint256) {
        try amm.getPrice(x) returns (uint256 price) {
            return price;
        } catch (bytes memory reason) {
            require(bytes4(reason) == selector, "unexpected revert");
        }
    }

An implementation of vm.canRevert/vm.allowRevert could add to the counter of vm.assume checks to make sure that one isn't rejecting all calls.

Additional context

No response

[mds1]

Hmm, how would this work in practice—does the value of y1 get set to zero if the call reverts? These feels like a footgun, because you now can't distinguish reverts from actual zero responses

I think you're suggesting to reject the test case though? That seems ok, but I'd suggest vm.assumeNoRevert() which adds a way to add an assumption that the next call won't revert

[0xPhaze]

Yes, I was thinking it would work like an assume. And not sure how you’re thinking about vm.assumeNoRevert, because that means “the next call should not revert”, which is the default behavior already.

[mds1]

A reverting call currently leads to a test failure, unless you have vm.expectRevert in which case execution continues. My understanding was that vm.assumeNoRevert would reject the test case if there's a revert, and generate new fuzz input values, just like vm.assume(false)

[0xPhaze]

Yeah you’re right. Just found the name vm.assumeNoRevert a bit confusing, but if you think about it, it does make sense to name it that way.