Native assertions are not skipped for expectRevert

[jinxinglim]

Issue

According to the foundry book, after calling expectRevert, calls to other cheatcodes before the reverting call are ignored.

However, when running a test like

    function test_assertTrue_string() public {
        string memory err = "throw test";
        vm.expectRevert(bytes(err));
        assertTrue(false, err);
    }

the vm.assertTrue call is executed and the test passed.

Question

Are the native assertions such as assertTrue, assertFalse, assertEq, etc not skipped for expectRevert? Are there any other exceptions as well? Thank you.

[mds1]

I was able to reproduce this on the latest foundry, cc @klkvr

[klkvr]

This is happening because the revert is being catched on the level of the test contract itself.

After calling expectRevert, calls to other cheatcodes before the reverting call are ignored.

this means that when expectRevert is active we do not expect the next cheatcode call to revert, but if it does, there is not much we can do

[mds1]

Hmm, ideally the test would fail here though, this feels like a footgun. Would this be solved by the proposed v1 changes for expectRevert? I'm not sure what the latest on those is

[jinxinglim]

Hi @mds1 and @klkvr. Thank you for looking into this issue. Can I check if there is any update on this? Thank you.

Anyway, I am Jin Xing, part of the Kontrol team, where Kontrol combines KEVM and Foundry to grant developers the ability to perform formal verification without learning a new language or tool. So we want to make sure that the behaviour of this issue (expecting revert for assertions) in Foundry is done similarly in Kontrol.

[klkvr]

hi @jinxinglim what exactly is your usecase for this?

expectRevert does not catch reverts for cheatcode calls (which native assertions are as well), and changing this would be a breaking change

[jinxinglim]

Hi @klkvr. We are fine with either direction you guys will be taking as we just want Kontrol to behave similarly when it comes to expertRevert. At the current state, we have made Kontrol to not catch reverts for cheatcode calls as stated in the Foundry book.

expectRevert does not catch reverts for cheatcode calls (which native assertions are as well), and changing this would be a breaking change

But the issue is that when we run the following test, it does catch the revert from assertTrue(false, err), a cheatcode in Foundry and making it passed:

    function test_assertTrue_string() public {
        string memory err = "throw test";
        vm.expectRevert(bytes(err));
        assertTrue(false, err);
    }

Thus, we would like to clarify if expectRevert catches reverts for native assertions.

[klkvr]

expectRevert catches reverts on its depth or above. So, in your example it does not expect assertTrue call on depth 1 to revert, however, the revert of assertTrue is being propagated by Solidity to revert entire test contract call, thus causing a revert on depth 0 which expectRevert catches.

I see how this is confusing though, and this indeed will be fixed by v1 expectRevert changes

[jinxinglim]

expectRevert catches reverts on its depth or above. So, in your example it does not expect assertTrue call on depth 1 to revert, however, the revert of assertTrue is being propagated by Solidity to revert entire test contract call, thus causing a revert on depth 0 which expectRevert catches.

Oh. Thank you for the explanation!

this indeed will be fixed by v1 expectRevert changes

If that is the case, we will continue to make expectRevert to not catch not catch reverts for cheatcode calls (which native assertions are as well) in Kontrol so to be in sync with the v1 expectRevert changes as you mentioned. Just to check if you guys have an estimate on when this v1 change will be released? Thank you.