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

forge test hangs on invariant testing #4656

Closed
2 tasks done
trojanMcAfee opened this issue Mar 27, 2023 · 50 comments
Closed
2 tasks done

forge test hangs on invariant testing #4656

trojanMcAfee opened this issue Mar 27, 2023 · 50 comments
Labels
A-testing Area: testing C-forge Command: forge Cmd-forge-test Command: forge test T-bug Type: bug

Comments

@trojanMcAfee
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 (97f070f 2023-03-17T09:30:30.350509Z)

What command(s) is the bug in?

No response

Operating System

macOS (Intel)

Describe the bug

When running any type of invariant testing, the terminal hangs after compiling the contracts:

image

This is the function I'm calling:

function invariant_getHello() public {
    assertTrue(true);
}

It hangs with either forge test --match-test $FUNC --fork-url $ARB --fork-block-number $BLOCK -vvv where $FUNC is invariant_getHello or with forge test --match-test invariant --fork-url $ARB --fork-block-number $BLOCK -vvv.

If I change the function name to test_getHello, the test runs and passes without issues, so the problem is in the invariant keyword:

image

@trojanMcAfee trojanMcAfee added the T-bug Type: bug label Mar 27, 2023
@gakonst gakonst added this to Foundry Mar 27, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Mar 27, 2023
@trojanMcAfee
Copy link
Author

The bug is when passing the --fork-url $ARB flag. If I remove it, the test runs and passes.

image

@trojanMcAfee
Copy link
Author

Selecting the fork directly in the contract with createSelectFork doesn't solve the issue.

@mds1
Copy link
Collaborator

mds1 commented Mar 28, 2023

If you run with RUST_LOG=forge=trace,foundry_evm=trace,ethers=trace forge test <rest of command> you'll see this is just because it's making a lot of RPC requests since it's a fork test. If you pin to a block and use a fuzz seed, subsequent invocations will be much fast since responses will be cached. You can read more here: https://book.getfoundry.sh/forge/fork-testing

@mds1 mds1 closed this as completed Mar 28, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Foundry Mar 28, 2023
@trojanMcAfee
Copy link
Author

It didn't work @mds1

As you can see on my screenshots, I was passing the flag --fork-block-number $BLOCK with 69254399 as BLOCK.

I also tried removing the flag and setting the block directly in the contract with vm.createSelectFork(vm.rpcUrl('arbitrum'), 69254399); while having my .toml file like:

[profile.default]
src = 'contracts'
out = 'out'
libs = ['node_modules', 'lib']
test = 'test/foundry'
cache_path = 'forge-cache'

[rpc_endpoints]
arbitrum = "${ARBITRUM}"

[fuzz]
seed = 10

As you can see there, I specified a seed also.

@trojanMcAfee
Copy link
Author

trojanMcAfee commented Mar 29, 2023

I created #4662 since the problem is unresolved and I can't re-open this issue.

Once the situation is resolved, I can close that other issue.

@trojanMcAfee
Copy link
Author

trojanMcAfee commented Mar 29, 2023

Don't think that it's a cache issue, which would explain why your solution didn't work @mds1

I removed cache_path = 'forge-cache' from my .toml file so the cache would default back to ~/.foundry/cache/rpc/arbitrum/69254399 (which it did), but the hangs still remains only for invariant tests.

EDIT:

Foundry is catching properly to ~/.foundry/cache.. (there's data there), but when I run forge cache ls, I still get an output of 0.0B

image

@mds1 mds1 reopened this Mar 29, 2023
@mds1
Copy link
Collaborator

mds1 commented Mar 29, 2023

One way you can try to verify that tests are not hanging, and are simply slow due to the RPC queries is as follows:

Modify your foundry config as follows:

[profile.default.fuzz]
seed = 1

[profile.default.invariant]
runs = 1
depth = 1

This specifies a seed for the random number generator, and makes the invariant tests only run a single run of depth 1. Now, run forge test --fork-url $ARBITRUM_RPC_URL . I ran forge init, and modified Counter.t.sol as follows:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "../src/Counter.sol";

contract CounterTest is Test {
    Counter public counter;

    function setUp() public {
        counter = new Counter();
        counter.setNumber(0);
    }

    function invariant_example() public {
        assertTrue(true);
    }
}

With this configuration, it took 2.5s to run the test. Remember that you can modify the test command to be RUST_LOG=forge=trace,foundry_evm=trace,ethers=trace forge test --fork-url $ARBITRUM_RPC_URL to see what's happening under the hood so you can see if things are hanging vs. if there's just a lot of RPC requests happening.

However, it does seem that RPC caching is not being leveraged, as it does take ~2.5s each invocation, even if I specify a fuzz seed

@trojanMcAfee
Copy link
Author

trojanMcAfee commented Mar 29, 2023

Thanks for the reply :) @mds1

And yeah, setting runs = 1 makes the test run, and like you mentioned, with RUST_LOC=...., it shows the multiple RPC calls being made under the hood, but in the end, the test just sits there (aka it hangs), which is what matters since I'm trying to run an invariant test, and having only one run defeats the purpose.

image

I'm not sure I fully understood your last sentence, but it seems like you were able to reproduce my issue (?).

In the end, what I'm trying to do is to run a invariant fuzzing campaign on an Arbitrum fork with standard parameters. Is this a bug or do I have to make a change in my config in order to achieve this outcome?

@mds1
Copy link
Collaborator

mds1 commented Mar 29, 2023

but in the end, the test just sits there (aka it hangs)

Can you clarify what you mean here? From what I can see it is running and making RPC requests, it's not stuck/hanging

In the end, what I'm trying to do is to run a invariant fuzzing campaign on an Arbitrum fork with standard parameters. Is this a bug or do I have to make a change in my config in order to achieve this outcome?

Are you running your own node or using a service like e.g. Alchecmy/Infura? If the latter, it's very likely you'll get rate limited due to the number of RPC requests, which will make the tests execute very slowly. Unfortunately there's not much we can do here on the forge side. One solution is to allow forge to be initialized with a list of provider URLs and have it rotate providers with each call, but that might be a nontrivial change. If this is something you need I'd suggest opening a separate issue for this

I'm not sure I fully understood your last sentence, but it seems like you were able to reproduce my issue (?).

I just meant that it seems the cache is not being used for invariant tests, because every run took ~2.5s, but I'd have expected subsequent runs to be only a few milliseconds thanks to using the cache. However, I had forgotten to pin a block, and with pinning + seed, RPC caching is working as expected (~2.5s for first run, a few millseconds for subsequent runs)

@trojanMcAfee
Copy link
Author

Can you clarify what you mean here? From what I can see it is running and making RPC requests, it's not stuck/hanging

I can see that the RPC requests are being made when I run forge test with RUST_LOG=...., but if I run it without it, so I don't see under hood, which is what matters for me, the completion of the test per se (not the behaviour under the hood), it just sits in this screen forever. It doesn't complete the test. That's what I mean by "it hangs".

image

Do you manage to actually complete a test with standard invariant-fuzzing parameters on a fork?

Are you running your own node or using a service like e.g. Alchecmy/Infura? If the latter, it's very likely you'll get rate limited due to the number of RPC requests, which will make the tests execute very slowly

I'm using RPC services (both Alchemy and Infura), but with other fuzzers like Echidna, running tests with forks work without issue, so it should work with Foundry also, no? Especially if you're mentioning that the RPC catching is not being leveraged, that I can't successfully run an invariant test on a fork, and have the evidence of the issue. @mds1

Isn't this enough to qualify the issue as a bug?

@trojanMcAfee
Copy link
Author

I understand what you mean now. @mds1

The "hang" that I'm experiencing is just forge doing the RPC requests. If I have set runs to like 100, it's going to feel like forever.

But still, there's a catching issue that foundry is not doing. That's the bug I'm mentioning should be considered no?

@mds1
Copy link
Collaborator

mds1 commented Mar 29, 2023

it just sits in this screen forever. It doesn't complete the test. That's what I mean by "it hangs".

Got it. So it will complete eventually, just slowly, due to RPC queries. Something like #585 would help make it clear that things are progressing and not actually hanging

Do you manage to actually complete a test with standard invariant-fuzzing parameters on a fork?

If I run using alchemy on arbitrum, I have not waited long enough for it to complete. However if I use my local mainnet erigon node I get through an invariant run with the default settings (256 runs, depth of 15) in ~20 seconds

Especially if you're mentioning that the RPC catching is not being leveraged,

Caching is being leveraged if you use a seed and pin to a block, I just forgot to pin to a block my initial test test

I'm using RPC services (both Alchemy and Infura), but with other fuzzers like Echidna, running tests with forks work without issue, so it should work with Foundry also, no?

Maybe they just make fewer RPC requests somehow? They would have the same issue about being rate-limited, so I'm not sure what they'd do different. Maybe @mattsse has some insight here

@trojanMcAfee
Copy link
Author

trojanMcAfee commented Mar 29, 2023

Caching is being leveraged if you use a seed and pin to a block, I just forgot to pin to a block my initial test test

I have a pinned block and seed also, but the behaviour is the same.

vm.createSelectFork(vm.rpcUrl('arbitrum'), 69254399)

But don't you think that the fact that I can't successfully complete one invariant test on a fork can be considered a bug?

I have runs = 100 (which is not that much in what fuzzing concerns), and I still have the same test that I started 10 minutes ago running without producing any output. @mds1

@mattsse
Copy link
Member

mattsse commented Mar 29, 2023

this has to do with how the sender fuzzing in combination with forking mode works

because we're using random callers which all need to be fetched via rpc first if the test is started in forking mode.

But I'm not sure if this is really necessary.
manually entering forking mode should get around this though, so

vm.createSelectFork(vm.rpcUrl('arbitrum'), 69254399)

should only fetch everything after entering the fork, if the test is not launched in forking mode

@trojanMcAfee
Copy link
Author

trojanMcAfee commented Mar 29, 2023

What does that mean exactly if I may ask? @mattsse

I tried both ways, with vm.createSelectFork(vm.rpcUrl('arbitrum'), 69254399) in the code directly, and with --fork-url $ARB --fork-block-number $BLOCK as flags on the CLI, but with both the issue is still present (both as separate attempts and not mixed with one another)

Do I have to do something extra in order to get invariant tests to work?

@mattsse
Copy link
Member

mattsse commented Mar 29, 2023

if launched in forking mode (--fork-url) then the executor that is used to run a test is already in forking mode. So every time the test is called the sender account is fetched via rpc, because it is already in forking mode.

entering manually will use a random, empty account for the sender (not in forking mode yet)

    function invariant_example() public {
       vm.createSelectFork("https://eth-mainnet.alchemyapi.io/v2/<>");
       assert(true);
    }
forge t
[PASS] invariant_example() (runs: 256, calls: 3840, reverts: 108)
Test result: ok. 1 passed; 0 failed; finished in 1.82s

I guess technically it is correct when running in forking mode that the sender account should be fetched via rpc, but I'm not sure if this is useful here or the "right" behaviour

@mds1
Copy link
Collaborator

mds1 commented Mar 29, 2023

I guess technically it is correct when running in forking mode that the sender account should be fetched via rpc, but I'm not sure if this is useful here.

What data are we actually fetching from the sender up front? vs. just fetching e.g. nonce or something lazily as needed

@mattsse
Copy link
Member

mattsse commented Mar 29, 2023

the account consists of:

  • code
  • balance
  • nonce

that's what we need to create the Account object the evm needs.

this is actually loaded during evm execution, evm needs to check balance, nonce

@trojanMcAfee
Copy link
Author

This example doesn't work for me unfortunately @mattsse

I need to use the fork in setUp() since I'm doing something with arbitrum's state there.

I tried creating several forks in the setup, end that function up with a different fork, and then select arbitrum fork in my invariant test, but the test still hangs

@mds1
Copy link
Collaborator

mds1 commented Mar 29, 2023

Hmm interesting. It does feel unnecessary, as in practice I'd suspect most bugs surfaced by invariant testing are not due to differences in the sender. But I'm also not sure what the best fix is here.

Current behavior:

/// Strategy to select a sender address:
/// * If `senders` is empty, then it's either a random address (10%) or from the dictionary (90%).
/// * If `senders` is not empty, a random address is chosen from the list of senders.

Perhaps instead, if senders is empty, we change it to 5-10 default random senders to use for invariant testing?

  • That way we know, code, balance, and nonce will always be zero
  • Can derive the hardcoded addresses as address(uint160(uint256(keccak256("invariant sender i")))) for i in the range of 1..=10 or something
  • Can have a flag to switch back to the current behavior of generating senders if we feel current behavior is useful

cc @joshieDo @lucas-manuel curious to hear thoughts

@trojanMcAfee
Copy link
Author

trojanMcAfee commented Mar 29, 2023

Any other possible solution that I could try?

I tried also creating a fork in the setup and one in the invariant test...same result

@mattsse
Copy link
Member

mattsse commented Mar 29, 2023

yeh if entered in setUp then it is also already in forking mode when the random sender for the test function is fetched.

this does not seem very useful imo especially since we don't enforce balance/nonce/code constraints for the sender.

so I suggest we insert an empty sender account into the database first?

@trojanMcAfee
Copy link
Author

How do I do that? Sorry for the ignorant question.

@mds1
Copy link
Collaborator

mds1 commented Mar 29, 2023

@cdgmachado0 If you want a workaround in the meantime just use targetSenders() to hardcode a list of 1 or more senders, that way random senders are not generated. You can read more in the docs: https://book.getfoundry.sh/forge/invariant-testing#invariant-test-helper-functions

@mds1
Copy link
Collaborator

mds1 commented Mar 29, 2023

@mds1 I think both fixes would be pretty simple

Sorry just to clarify, this is just a single solution that would behave as follows:

  • Define 5-10 default random senders to use for invariant testing?
  • Derive those hardcoded addresses as address(uint160(uint256(keccak256("invariant sender i")))) for i in the range of 1..=10 (or however many senders we generate)
  • Since no one will have the private key for these addresses, we know, code, balance, and nonce will always be zero
  • We can consider also adding a flag to switch back to the current behavior of "If senders is empty, then it's either a random address (10%) or from the dictionary (90%)." IMO we can leave this out for now and only add it back in if requested, or if @joshieDo has a reason for the current approach that we're not thinking of. I know @lucas-manuel usually uses targetSenders so I think he'd be onboard with this change and not need a flag to bring back old behavior

@trojanMcAfee
Copy link
Author

Just to let you know guys, targetSender didn't work as a workaround. It still hangs.

Or were you able to get a test going? If you did, let me know and I'll check if the problem is me.

@lucas-manuel
Copy link

Hi @cdgmachado0!

Just caught up on all of this, for the example outlined it seems like having a constant targetSender should solve the issue. What is in your setUp function?

@trojanMcAfee
Copy link
Author

trojanMcAfee commented Mar 31, 2023

Hi @lucas-manuel

It's something like this:

address bob = makeAddr('bob');
address alice = makeAddr('alice');
address ray = makeAddr('ray');

function setUp() public {
   vm.createSelectFork(vm.rpcUrl('arbitrum'), 69254399);

   //random init code that creates some contracts 

   vm.prank(deployer);
   OZL.diamondCut(cuts, address(initUpgrade), data); //modifies state in the fork 

   deal(address(USDC), bob, 5000 * 10 ** 6);

   targetSender(alice);
   targetSender(ray);

   _setLabels(); //sets the labels of the contracts for the traces
}

@lucas-manuel
Copy link

Not sure, @mds1 @mattsse any ideas?

@mds1
Copy link
Collaborator

mds1 commented Apr 4, 2023

@cdgmachado0 Can you share a minimal repo that we can clone along with the command you're using to run the tests? It would make it much easier to reproduce

@trojanMcAfee
Copy link
Author

There you go guys: https://github.com/cdgmachado0/forReview

The command I'm using is forge test -m invariant_myTest, targeting this function in the file EnergyETHFacet.t.sol:

function invariant_myTest() public {
    assertTrue(true);
}

Running forge test -m test_getPrice, which targets this function, runs without issues:

function test_getPrice() public {
     uint price = energyFacet.getPrice();
     assertTrue(price > 0);
}

Let me know if you anything else @mds1 @lucas-manuel @mattsse

@trojanMcAfee
Copy link
Author

trojanMcAfee commented Apr 5, 2023

There's an env file there already with an Arbitrum RPC URL so you just need to run the forge command.

And it's a hardhat repo with a foundry integration.

@mds1
Copy link
Collaborator

mds1 commented Apr 5, 2023

It does seem like we're making too many RPC requests. You'll notice that the alice and ray addresses are:

alice 0x328809Bc894f92807417D2dAD6b7C998c1aFdac6
ray 0x0Bf6534a8459BC34CF6e86A3902F68333cF0991D

And if I run RUST_LOG=trace forge test -m invariant_myTest there are a lot of RPC requests to get transaction count and storage at those addresses. So my guess is that after each call, since we're forking, we don't know if some forked contract changed the state of those addresses, and therefore we have to query for updated state each time? @mattsse

But we know the transaction count couldn't have changed, so we should be able to drop those requests. Getting storage seems reasonable though and I'm not sure of a way around that. @mattsse lmk what you think

@mattsse
Copy link
Member

mattsse commented Apr 6, 2023

It does seem like we're making too many RPC requests.

yes, because we're in forking mode context at this point which happens if launched in forking mode or entered during setup

this is a bit horrible, I guess the best solution would be to not fetch the caller from remote and instead use an empty account (same as non forking mode) (so it bypasses the rpc requests for the caller account)
since the caller accounts are chosen at random I guess this is fine?

@trojanMcAfee
Copy link
Author

I think I'm using an empty account there. I'm initializing alice with address alice = makeAddr('alice'); , and then targeting her with targetSender(alice);.

No further actions are being done. Or by "empty account", do you mean other thing? @mattsse

@mattsse
Copy link
Member

mattsse commented Apr 6, 2023

ah there's targetSender.
i'm not familiar with targetSender works,
will check

@trojanMcAfee
Copy link
Author

Any updates or ideas that I could do to get this to work? @mattsse

I appreciate a lot the effort that you guys are doing btw. Much respect!

@trojanMcAfee
Copy link
Author

Found another section where this bug happens (in stdstore).

Opened issue #4735

@mds1 mds1 added Cmd-forge-test Command: forge test C-forge Command: forge A-testing Area: testing labels Apr 13, 2023
@thedavidmeister
Copy link

https://github.com/rainprotocol/rain.orderbook/actions/runs/5454746019/jobs/9925276209

I found a single invariant with no fork times out CI, it hangs locally too.

This happens as long as i have -vvv specified. Setting -vv or -v or nothing allows the tests to complete.

However, it _also _ seems to cause the invariant test to not be run at all, I don't see anything in my test output related to invariant tests.

@0xMySt1c
Copy link

0xMySt1c commented Aug 4, 2023

I'm having similar difficulties when running invariants in CI only.
Local seems to work.
The runs at 500 will fail, but pass at 200.

running CI on docker. local is mac

@thedavidmeister
Copy link

thedavidmeister commented Aug 7, 2023

not sure if related but i'm seeing forge test hang for 6 hours before being killed on CI

hangs locally too https://github.com/rainprotocol/rain.interpreter/actions/runs/5778610955/job/15660019345

nm, i think i actually have an infinite loop in my contract, not sure why it doesn't run out of gas before 6 hours, but i think it's unrelated

@stobiewan
Copy link

Have a similar issue. Not using a forked network. Sometimes invariant tests never terminate, it may be the same thing. You can check by writing lines to a file in the test and see it keeps calling functions and testing invariant forever as if depth = infinity, or depth counter fails to increment after some point

@beeb
Copy link
Contributor

beeb commented Nov 27, 2023

Not sure if related but I'm also experiencing hanging during invariant testing, and I narrowed the issue to the call_override option. If set to true, then the tests stop almost immediately and the logs do not show anything weird in the traces, just no more output after a few milliseconds.

@Nikhil8400
Copy link

I was also facing the same problem, but I used a VPN, and my problem was solved

@grandizzy
Copy link
Collaborator

#7756 should help, can you guys pls recheck? tested also on a project that had call_override true and was hanging and is no longer reproducible

@grandizzy
Copy link
Collaborator

I retried with project from #4656 (comment) and wasn't able to reproduce, test completed properly

image

also traces show that fork properly initialized and test preformed OK

image

I am going to close this ticket for now, please reopen if you hit it again. thank you

@jenpaff jenpaff moved this from Done to Completed in Foundry Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing Area: testing C-forge Command: forge Cmd-forge-test Command: forge test T-bug Type: bug
Projects
Archived in project
Development

No branches or pull requests

10 participants