expectRevert bug with internal call

[MathisGD]

Component

Forge

Have you ensured that all of these are up to date?

• Foundry
• Foundryup

What version of Foundry are you on?

1.3.2-stable

What version of Foundryup are you on?

No response

What command(s) is the bug in?

forge t

Operating System

macOS (Apple Silicon)

Describe the bug

Both

/// forge-config: default.allow_internal_expect_revert = true
function testA() {
    IERC20(address(0)).totalSupply();
}

and

/// forge-config: default.allow_internal_expect_revert = true
function testB() {
    vm.expectRevert();
    IERC20(address(0)).totalSupply();
}

fail (the first one for "revert" and the second one for "call didn't revert as expected"). I guess that this is not the expected behavior (the second one should pass, considering this call really fails as address zero has no code).

edit: note that also both

function testC() public {
    IERC20(address(0)).totalSupply();
}

function testD() public {
    vm.expectRevert();
    IERC20(address(0)).totalSupply();
}

fail but that's expected

[grandizzy]

hey @MathisGD /// forge-config: default.allow_internal_expect_revert = true and vm.expectRevert(); means it expects revert on the next internal call, however IERC20(address(0)).totalSupply(); is not internal, hence call didn't revert as expected

[MathisGD]

hey! but calling an address with no code is actually an internal revert. So testB should pass, normally. Anyway it's weird that all these 4 tests fail (it is impossible that this call is neither successful, neither an internal revert, neither a revert deeper).

[grandizzy]

allow_internal_expect_revert only works for internal calls happening at the same depth as test, please see https://getfoundry.sh/reference/cheatcodes/expect-revert#description hence testB fails. However testD you pointed out should pass

function testD() public {
    vm.expectRevert();
    IERC20(address(0)).totalSupply();
}

looks like an issue in expectRevert, will check why this happens

l.e.

function testD() public {
    Counter counter = new Counter();
    vm.expectRevert();
    IERC20(address(counter)).totalSupply();
}

works properly, the first one fails because is a call to non-contract address 0x and seems this scenario is not properly handled

[grandizzy]

thing here is that IERC20(address(0)).totalSupply() doesn't revert but rather stop hence is not treated as revert, if you run with verbosity

    vm.expectRevert();
    IERC20(address(0)).totalSupply();

returns

[FAIL: call didn't revert at a lower depth than cheatcode call depth] testD() (gas: 3302)
Traces:
  [3302] CounterTest::testD()
    ├─ [0] VM::expectRevert(custom error 0xf4844814)
    │   └─ ← [Return]
    ├─ [0] 0x0000000000000000000000000000000000000000::totalSupply() [staticcall]
    │   └─ ← [Stop]
    └─ ← [Revert] call didn't revert at a lower depth than cheatcode call depth

while

    Counter counter = new Counter();
    vm.expectRevert();
    IERC20(address(counter)).totalSupply();

returns

[PASS] testD() (gas: 132309)
Traces:
  [132309] CounterTest::testD()
    ├─ [96345] → new Counter@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
    │   └─ ← [Return] 481 bytes of code
    ├─ [0] VM::expectRevert(custom error 0xf4844814)
    │   └─ ← [Return]
    ├─ [145] Counter::totalSupply() [staticcall]
    │   └─ ← [Revert] unrecognized function selector 0x18160ddd for contract 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f, which has no fallback function.
    └─ ← [Stop]

[MathisGD]

[FAIL: call didn't revert at a lower depth than cheatcode call depth] testD() (gas: 3302)
Traces:
  [3302] CounterTest::testD()
    ├─ [0] VM::expectRevert(custom error 0xf4844814)
    │   └─ ← [Return]
    ├─ [0] 0x0000000000000000000000000000000000000000::totalSupply() [staticcall]
    │   └─ ← [Stop]
    └─ ← [Revert] call didn't revert at a lower depth than cheatcode call depth

but you see that here CounterTest reverts, so it should be considered as an internal revert right? (thus my initial guess that testB would pass)

[grandizzy]

yeah, well that's a revert of vm.expectRevert cheatcode that doesn't match any revert, the totalSupply call itself doesn't revert

    ├─ [0] 0x0000000000000000000000000000000000000000::totalSupply() [staticcall]
    │   └─ ← [Stop]

[MathisGD]

I don't think so. See the trace of testC for example

[FAIL: call to non-contract address 0x0000000000000000000000000000000000000000] testC() (gas: 532)
Traces:
  [532] CounterTest::testC()
    ├─ [0] 0x0000000000000000000000000000000000000000::totalSupply() [staticcall]
    │   └─ ← [Stop]
    └─ ← [Revert] call to non-contract address 0x0000000000000000000000000000000000000000

[grandizzy]

hm, @MathisGD that's not a revert but a stop hence it is not caught by vm.expectRevert, do I get that wrong?

> cast call 0x0000000000000000000000000000000000000000 "totalSupply()" --rpc-url https://eth.llamarpc.com
Warning: Contract code is empty
0x

> cast call 0x0000000000000000000000000000000000000000 "totalSupply()" --trace --rpc-url https://eth.llamarpc.com
Traces:
  [0] 0x0000000000000000000000000000000000000000::totalSupply()
    └─ ← [Stop]

Confusion could come from same reason as in #11344 where we revert on verbosity with [Revert] call to non-contract address 0x0000000000000000000000000000000000000000 message if called address is not a contract (we might reconsider this to warn only instead changing the outcome)