Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

targeted_contracts in invariant test is getting updated unexpectedly partway through an invariant test run #5625

Closed
2 tasks done
Melvillian opened this issue Aug 14, 2023 · 6 comments · Fixed by #7595
Closed
2 tasks done
Labels
T-bug Type: bug

Comments

@Melvillian
Copy link

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 (a0a31c3 2023-08-11T18:45:55.249069000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

Background:

I am running a invariant test contract using a forked version of the blockchain pointing at Mainnet, with a single Handler, and I only want the Handler and its functions to be called throughout the entire invariant test run. Any calls to non-handler functions is undesired and results in me needing to set fail_on_revert = false in order to prevent my invariant test run from failing every time.

Expected Behavior:

I use targetContract and targetSelector to select my Handler's address and various Handler functions, respectively, in the setUp function of my InvariantTest contract. This will mean that the only function I ever see being fuzzed should be the Handler. I should run forge test -vvvv and not see any calls to other contracts

Actual Behavior:

When I run forge test -vvvv, I see that there are many calls being made to a contract that was deployed during the setUp of my InvariantTest contract. Specifically, the only other contract besides the Handler being fuzzed is the one that was setup with the code below. The purpose of the code below is to take an already-deployed (i.e. already existing contract on the mainnet fork) and overwrite its bytecode with my own supplied code in order to mock it and make testing simpler:

function _deployMockBridge() internal {
        vm.selectFork(_l1Fork);
        MockBridge mb1 = new MockBridge();
        bytes memory mb1Code = address(mb1).code;
        vm.etch(_bridge, mb1Code);

        vm.selectFork(_l2Fork);
        MockBridge mb2 = new MockBridge();
        bytes memory mb2Code = address(mb2).code;
        vm.etch(_bridge, mb2Code);
    }

Hypothesis:

I cloned foundry and added some debugging logs to a locally running version of forge in order to try to understand what is going on. From running that, I was able to that in this executor.rs function the list of targeted_contracts is being updated with the MockBridge code's address. Whys is that happening, even though we never deployed the MockBridge in our InvariantTeest? My guess is that because of this check in collect_data, because of the call to vm.etch, there is a bug where Forge thinks that we actually did deploy the contract with a sender that didn't have any code (whatever addresses is used by vm) and so it ends up adding the MockBridge contract to the listed of created_contracts, which ultimately gets sampled from to decide on the next fuzz run, and thus will probabilistically end up choosing the MockBridge code and its selectors to be fuzzed.

Solution:
when calculated created_contracts, exclude any that were touched by Cheatcode addresses, as those are not really created, as we can see here with etch. Though to be clear, I'm not sure this is the problem because I'm not super familiar with Forge's codebase; that is just what I've been able to uncover by manually debugging.

@Melvillian Melvillian added the T-bug Type: bug label Aug 14, 2023
@gakonst gakonst added this to Foundry Aug 14, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Aug 14, 2023
@Evalir
Copy link
Member

Evalir commented Aug 14, 2023

hey—could you try with the latest foundry version by using foundryup? this issue was caused by an unrelated update, and should be OK now

@Melvillian
Copy link
Author

hey—could you try with the latest foundry version by using foundryup? this issue was caused by an unrelated update, and should be OK now

Hey, just upgraded to:

forge 0.2.0 (93ebcdd 2023-08-15T00:27:50.951269000Z)

and I'm still seeing the MockBridge contract get added to the list of contracts that it uses to build the next fuzz run.

@PatrickAlphaC
Copy link

@Melvillian I'm seeing something similar. My MockERC20 keeps getting added and it's breaking my runs.

@Melvillian
Copy link
Author

@Melvillian I'm seeing something similar. My MockERC20 keeps getting added and it's breaking my runs.

We never ended up being able to figure this out, and instead we threw up our hands and commented out the invariant test.

From what I remember from debugging with a local version of Foundry that I could add println statements to, there is a part of the invariant test code which looks at every transaction trace for contracts that your test contracts interacted with, and decided whether to add those contracts to the list of contracts that should be fuzzed. I guessed the bug is that Foundry thinks it should add the MockERC20/MockBridge to that list, even though it was etch'ed in.

If I had more time, I'd write and submit a patch which exempted etch'ed contracts from this list of contracts-to-be-fuzzed when considering the excluded contracts CLI param

@77abe77
Copy link

77abe77 commented Oct 21, 2023

this is still happing on forge 0.2.0 (619f3c5 2023-10-20T00:22:09.970731000Z). Please help with this issue bc its super annoying to have to give up on invariant testing bc of features that should work as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants
@Melvillian @77abe77 @Evalir @PatrickAlphaC and others