Foundry crashes with arithmetic operation overflow

[benesjan]

Component

Forge

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

• Foundry
• Foundryup

What version of Foundry are you on?

forge 0.2.0 (70f4fb5 2022-11-02T00:05:06.592613Z)

What command(s) is the bug in?

forge test

Operating System

Linux

Describe the bug

Hello, after updating forge-std dependency foundry started crashing with arithmetic operation overflow error. The issue is occuring locally on my M1 Macbook as well as in a linux docker container in CI. Here is the code and here is the log of a failing CI pipeline.

Update

The bug is not related to forge-std (the forge-std update simply made it occur more frequently). The bug is caused by overflow in smart contract execution.

[mattsse]

I think this is a U256 overflow, @mds1 were there any changes re balance or similar?

[mattsse]

@benesjan I believe it's the deal cheatcode that overflows here https://github.com/AztecProtocol/aztec-connect-bridges/blob/f8ef3f8dc5a3c21046d20fa235de6271586cb1a1/src/test/bridges/erc4626/ERC4626.t.sol#L108

or rather somewhere in that test

[mds1]

There's no changes in forge-std that I can think of that would cause. Though it seems to me like this is a forge error, not a forge-std error. From lines 87-89 of the linked CI job:

The application panicked (crashed).
Message:  arithmetic operation overflow
Location: /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/primitive-types-0.11.1/src/lib.rs:38

If this was a solidity/EVM overflow it wouldn't panic

[mattsse]

okay it's the bound here

https://github.com/AztecProtocol/aztec-connect-bridges/blob/f8ef3f8dc5a3c21046d20fa235de6271586cb1a1/src/test/bridges/erc4626/ERC4626.t.sol#L105

if you change the max to type(uint96).max it succeeds, unclear why there's a U256 overflow,
there's no cheatcode involved perhaps this is an issue inside the evm

nvm it's somewhere else

I'm almost certain this is the call that's causing this

https://github.com/AztecProtocol/aztec-connect-bridges/blob/f8ef3f8dc5a3c21046d20fa235de6271586cb1a1/src/test/bridges/erc4626/ERC4626.t.sol#L56

couldn't find the actual implementation of this, could you please provide the link to the function impl?

[benesjan]

@mattsse Here is the function implementation. And this is the contract deployment on mainnet.

[benesjan]

It seems that the ERC4626Test is failing with overflow error and weirdly it causes forge to crash for me most of the time. Tried to run it a few times and only once from ~8 runs forge didn't crash.

[mattsse]

maybe related to the uint96 value fuzzing
@joshieDo any ideas?

[mds1]

@benesjan have you narrowed down the cause of this, i.e. are you sure it's the forge-std update as mentioned in the original description vs. a certain forge release that introduced this?

[mattsse]

I've narrowed this down to these lines

https://github.com/AztecProtocol/aztec-connect-bridges/blob/f8ef3f8dc5a3c21046d20fa235de6271586cb1a1/src/test/bridges/erc4626/ERC4626.t.sol#L56-L57

at least if I comment out the entire testFullFlow test and those two lines then the panic goes away.

 forge test --match-test "testFullFlow" --match-contract "ERC4626"

[benesjan]

@mds1 it's not caused by the update but it seems the update makes it more frequent. I think that the tests are simply failing with the overflow error and for some reason it causes forge to crash. Sometimes forge doesn't crash and I see that the tests fail with the overflow error. I tried compiling an older forge version (from October 17) and the issue occurs there as well.

[mattsse]

the randomness of this is likely related to the fuzz test function's inputs.

Me guess at this point is, that the state changes inside the prank are picked up by the fuzzer (which looks at state) and perhaps the uint96 strategy has a problem with those values.

[benesjan]

Hello, this bug is still occurring. I have a new example. I introduced a bug in test in this commit which caused overflow in the tested smart contract. This overflow caused forge to crash. Log can be seen in our CI pipeline.

[mattsse]

is it still the same test?

[benesjan]

No, it's a different test. Previously it was an ERC4626 bridge test and now it is a TroveBridge test. They are unrelated and don't share basically any code.

[mattsse]

sorry, I meant test function

[benesjan]

It's a different test function. I run locally the following command and it fails:
forge test --fork-block-number 14970000 -m 'testRedistributionSuccessfulSwap' --fork-url https://mainnet.infura.io/v3/9928b52099854248b3a096be07a6b23c

It is on this PR's branch.

Thanks

[mattsse]

thanks,

no fuzz test this time, makes it easier to debug, will have a closer look.

[mattsse]

@benesjan found it after I spent some time debugging the test.

there was an edge with deal+transfer that was triggered on revert. fixed in #3767

[benesjan]

@mattsse Thanks a ton! This was haunting us for a while

[benesjan]

@mattsse Unfortunately it seems that the issue was not eliminated. The bug occured in our CI pipeline again and there we always install the newest release.

[mattsse]

pushed two related fixes today,

If you could narrow this down to the failing test, I'd be happy to give it another try.

[benesjan]

Hello @mattsse, while trying to reproduce the bug I've stumbled upon another one:

After dealing 5 ETH to an address owner's balance suddenly drops to 1 wei.

This is how you should be able to reproduce it:

Checkout the janb/foundry-bug branch and run:
forge test -m testLiquidationFlowWithCollateralToClaim --fork-url https://mainnet.infura.io/v3/9928b52099854248b3a096be07a6b23c --fork-block-number 15977385 -vv

I just executed foundryup so I am using the newest version.

[mattsse]

@mattsse

I believe I maybe suffering from the same issue. I have it narrowed down to a much smaller, very simple example. Would it be useful to post that information here? I don't want to clog up the thread unnecessarily.

yes please, ty!

[mattsse]

@benesjan it appears either #3688 or #3687 has fixed that, couldn't reproduce on master, but present on nightly

will land in nightly in a couple hours, sorry about these hiccups

[mattsse]

@sprw121 does your code make use of deal cheatcode? of not then this is unrelated, could you please open a separate issue for this, and ideally create a repro repo so I can reproduce this more easily?

[sprw121]

@sprw121 does your code make use of deal cheatcode? of not then this is unrelated, could you please open a separate issue for this, and ideally create a repro repo so I can reproduce this more easily?

No cheat code. Yes I can try to put up a repro repo. Let me know if you want me to delete / clean up the comments in this thread.

[benesjan]

Hello @mattsse, our CI is now passing without any crashes so you can probably close this issue. Thank you for fixing this 👍

[chrismin]

Hi @mattsse — wanted to share that I'm unfortunately running into a similar issue. As far as I can tell, if a test combines the following, it leads to intermittent crashing of foundry:

• using a fuzzed uint256 value with the deal cheatcode
• expectRevert

When I get a chance, I will try to create a small repro example!

[chrismin]

@benesjan, I'm curious if you had to do anything specific to resolve the issue in your code?