ExpectRevert not catching Revert in super()

[jordaniza]

Hi team,

Love foundry. Wish I could marry yall.

Anyway, posted on TG but here's my issue:

I have a reducer function in my application, looks like this:

    function _reducer(
        uint16 _srcChainId,
        bytes memory _srcAddress,
        Message memory message
    ) internal {
        if (message.action == DEPOSIT_ACTION) {
            _depositAction(_srcChainId, message.payload);
        } else if (message.action == REQUEST_WITHDRAW_ACTION) {
            _requestWithdrawAction(_srcChainId, message.payload);
        } else if (message.action == FINALIZE_WITHDRAW_ACTION) {
            _finalizeWithdrawAction(_srcChainId, message.payload);
        } else if (message.action == REPORT_UNDERLYING_ACTION) {
            _reportUnderlyingAction(message.payload);
        } else {
            revert("XChainHub::_reducer:UNRECOGNISED ACTION");
        }
    }

It's an internal function and I wanna test it, so I wrote a mock contract that supers the function as an external method
(which I have read cannot be mocked with mockCall at this time #432)

    /// @notice grant access to the internal reducer function
    function reducer(
        uint16 _srcChainId,
        bytes memory _srcAddress,
        Message memory message
    ) external {
        super._reducer(_srcChainId, _srcAddress, message);
    }

My test reverts if I call:

        hubMockReducer.reducer(
            1,
            abi.encodePacked(vaultAddr),
            hubMockReducer.makeMessage(245)
        );

But if I stick an expect revert the test fails:

        vm.expectRevert(bytes("XChainHub::_reducer:UNRECOGNISED ACTION"));
        hubMockReducer.reducer(
            1,
            abi.encodePacked(vaultAddr),
            hubMockReducer.makeMessage(245)
        );

FYI: Make message works fine, have tested that.

Any advice much appreciated

[onbjerg]

Does it work if you use vm.expectRevert("XChainHub::_reducer:UNRECOGNISED ACTION");? If not, does the test revert if you remove the vm.expectRevert? If not, then your function call is not reverting 😄

[jordaniza]

Tried:

vm.expectRevert("XChainHub::_reducer:UNRECOGNISED ACTION");
vm.expectRevert(bytes("XChainHub::_reducer:UNRECOGNISED ACTION"));

Same issue.

If I remove expectRevert the test fails:

[FAIL. Reason: XChainHub::_reducer:UNRECOGNISED ACTION] testReducerRevertsOnUnknownAction() (gas: 2931659)

[jordaniza]

[Edit] I was making some changes to the code at the same time

I undid the changes and the original message is still present:

• The revert works as expected, but expectRevert is not catching it for some strange reason

[onbjerg]

This is pretty strange. What version of Forge are you on, and can you try to post the trace output of forge test -vvvvv?

[jordaniza]

Version

forge 0.2.0 (f016135 2022-07-04T00:04:24.423011561Z)

forge test without expectRevert:

Running 1 test for test/XChainStargateHub.t.sol:TestXChainStargateHub
[FAIL. Reason: XChainHub::_reducer:UNRECOGNISED ACTION] testReducerRevertsOnUnknownAction() (gas: 11987)
Traces:
  [6520024] TestXChainStargateHub::setUp() 
    ├─ [3412309] → new XChainStargateHub@"0xce71…c246"
    │   ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: TestXChainStargateHub: [0xb4c79dab8f259c7aee6e5b2aa729821864227e84])
    │   └─ ← 16810 bytes of code
    ├─ [2882642] → new XChainStargateHubMockReducer@"0x185a…1aea"
    │   ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: TestXChainStargateHub: [0xb4c79dab8f259c7aee6e5b2aa729821864227e84])
    │   └─ ← 14165 bytes of code
    └─ ← ()

  [11987] TestXChainStargateHub::testReducerRevertsOnUnknownAction() 
    ├─ [932] XChainStargateHubMockReducer::makeMessage(245) [staticcall]
    │   └─ ← (245, 0x)
    ├─ [1847] XChainStargateHubMockReducer::reducer(1, 0x4a1c900ee1042dc2ba405821f0ea13cfbadcab7b, (245, 0x)) 
    │   └─ ← "XChainHub::_reducer:UNRECOGNISED ACTION"
    └─ ← "XChainHub::_reducer:UNRECOGNISED ACTION"

Test result: FAILED. 0 passed; 1 failed; finished in 1.84ms

Failed tests:
[FAIL. Reason: XChainHub::_reducer:UNRECOGNISED ACTION] testReducerRevertsOnUnknownAction() (gas: 11987)

forge test -vvvv with expectRevert

Running 1 test for test/XChainStargateHub.t.sol:TestXChainStargateHub
[FAIL. Reason: Call did not revert as expected] testReducerRevertsOnUnknownAction() (gas: 11655)
Traces:
  [6520024] TestXChainStargateHub::setUp() 
    ├─ [3412309] → new XChainStargateHub@"0xce71…c246"
    │   ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: TestXChainStargateHub: [0xb4c79dab8f259c7aee6e5b2aa729821864227e84])
    │   └─ ← 16810 bytes of code
    ├─ [2882642] → new XChainStargateHubMockReducer@"0x185a…1aea"
    │   ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: TestXChainStargateHub: [0xb4c79dab8f259c7aee6e5b2aa729821864227e84])
    │   └─ ← 14165 bytes of code
    └─ ← ()

  [11655] TestXChainStargateHub::testReducerRevertsOnUnknownAction() 
    ├─ [0] VM::expectRevert(XChainHub::_reducer:UNRECOGNISED ACTION) 
    │   └─ ← ()
    ├─ [932] XChainStargateHubMockReducer::makeMessage(245) [staticcall]
    │   └─ ← (245, 0x)
    └─ ← "Call did not revert as expected"

Test result: FAILED. 0 passed; 1 failed; finished in 1.83ms

Failed tests:
[FAIL. Reason: Call did not revert as expected] testReducerRevertsOnUnknownAction() (gas: 11655)

[jordaniza]

Should add, I'm having this in several places in my test now, same pattern - reverts as expected but can't "catch" the revert with expectRevert.

Example above, happy to provide the full test if needed.

[jordaniza]

Wondering if anyone has had a chance to look into this further @onbjerg ?

[onbjerg]

Oh, right. This is because it works for the next call - makeMessage is the next call, and it doesn't revert. I'm not sure why I didn't catch that earlier, I'm sorry. Essentially you need to call makeMessage before calling vm.expectRevert