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

When testing against a fork, vm.roll does not change RPC query block #939

Closed
1 of 2 tasks
mds1 opened this issue Mar 14, 2022 · 6 comments · Fixed by #1715
Closed
1 of 2 tasks

When testing against a fork, vm.roll does not change RPC query block #939

mds1 opened this issue Mar 14, 2022 · 6 comments · Fixed by #1715
Assignees
Labels
A-cheatcodes Area: cheatcodes A-evm Area: EVM C-forge Command: forge Cmd-forge-test Command: forge test D-hard Difficulty: hard P-normal Priority: normal T-bug Type: bug
Milestone

Comments

@mds1
Copy link
Collaborator

mds1 commented Mar 14, 2022

Component

Forge

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

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.1.0 (0f58c52 2022-03-14T00:11:43.939930+00:00)

What command(s) is the bug in?

forge test --fork-url --fork-block-number <fork_block_number>

Operating System

macOS (amd)

Describe the bug

Steps to reproduce:

  1. Run forge test --fork-url <url> --fork-block-number <fork_block_number>
  2. block.number will return the current fork block, block.timestamp will return the corresponding timestamp, and all state read from the RPC's state at that block
  3. Call vm.roll(fork_block_number + x), where fork_block_number + x is a block that has already been mined on the specified RPC
  4. block.number returns the new block number, but block.timestamp and all state read from the RPC's state still give values for fork_block_number, and not fork_block_number + x

A sample use case here is testing a Uniswap TWAP oracle: scaffolding everything locally is a hassle, and it would be much easier to test prices/behavior against historical mainnet data. However, it seems this is not current possible with forge

When rolling to a block number that has not yet been mined on the provided RPC, block.timestamp can be either the same as the most recent blocks timestamp, or extrapolated by assuming a 12 second block time. Similarly, RPC queries for state should use the most recent block at the time of the roll

@mds1 mds1 added the T-bug Type: bug label Mar 14, 2022
@onbjerg onbjerg added A-evm Area: EVM Cmd-forge-test Command: forge test C-forge Command: forge A-cheatcodes Area: cheatcodes P-normal Priority: normal labels Mar 14, 2022
@onbjerg
Copy link
Member

onbjerg commented Mar 14, 2022

Can we get a link from #876? 🙏

@mds1
Copy link
Collaborator Author

mds1 commented Mar 15, 2022

Ah yes good call, done!

@onbjerg onbjerg self-assigned this Mar 21, 2022
@onbjerg
Copy link
Member

onbjerg commented Mar 22, 2022

Hmm, thinking about this more, I wonder if this is expected behavior or not. I can see use cases for both behaviors, so it might make sense to gate one of them behind a flag?

@mds1
Copy link
Collaborator Author

mds1 commented Mar 22, 2022

Yea I was thinking about that too, I agree a flag is the right approach. Keeping the default of not changing RPC queries on roll and adding this functionality behind a flag seems reasonable to me

@onbjerg
Copy link
Member

onbjerg commented Mar 25, 2022

Relaying here from #876: a better way is to have a flag in cheats.roll, e.g. two variants:

  • cheats.roll(number): Rolls our test backend to number
  • cheats.roll(number, remote): Also fetches state for number for this test if remote is true

@onbjerg onbjerg added the D-hard Difficulty: hard label Apr 6, 2022
@mds1 mds1 mentioned this issue Apr 12, 2022
23 tasks
@onbjerg onbjerg added this to Foundry Apr 17, 2022
@onbjerg onbjerg moved this to Todo in Foundry Apr 17, 2022
@onbjerg onbjerg removed their assignment Apr 18, 2022
@onbjerg
Copy link
Member

onbjerg commented Jul 1, 2022

Marking this as fixed by #1715 although with a few changes (I think it doesn't use vm.roll but instead a new cheatcode?)

@onbjerg onbjerg moved this from Todo to In Progress in Foundry Jul 1, 2022
@onbjerg onbjerg linked a pull request Jul 1, 2022 that will close this issue
3 tasks
@onbjerg onbjerg added this to the v1.0.0 milestone Jul 1, 2022
Repository owner moved this from In Progress to Done in Foundry Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes A-evm Area: EVM C-forge Command: forge Cmd-forge-test Command: forge test D-hard Difficulty: hard P-normal Priority: normal T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants