foundry icon indicating copy to clipboard operation
foundry copied to clipboard

createSelectFork does not fork latest block number if you sleep

Open maa105 opened this issue 1 year ago • 7 comments

Component

Forge

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

  • [X] Foundry
  • [ ] Foundryup

What version of Foundry are you on?

forge 0.2.0 (68124b5 2023-11-24T16:01:13.759990613Z)

What command(s) is the bug in?

createSelectFork

Operating System

Linux

Describe the bug

createSelectFork doesnt seem to check if a new block is mined check this:

pragma solidity ^0.8.21;

import {Test, console} from "forge-std/Test.sol";
import {Vm} from "forge-std/Vm.sol";

contract EntryTest is Test {
    function testStart() public {        
        vm.createSelectFork('rpc_url');
        console.log(block.number);
        for(uint i; i < 3; i++) {
            vm.sleep(15000);
            vm.createSelectFork('rpc_url');
            console.log(block.number);
        }
    }
}

if you ran this on sepolia test eth you will get the same block number printed out several times. while I expect that after 15 seconds a new block arrives and the createSelectFork must fork from that latest block

P.S. this takes ~45 seconds to finish so be patient

maa105 avatar Dec 18 '23 10:12 maa105

catching up here

the issue here is that we're mapping missing block number to latest and then check if there's already an existing fork that we can reuse:

https://github.com/foundry-rs/foundry/blob/ce5d2b5b280a7d79b972d6ebfe6bf3925447e833/crates/evm/core/src/fork/multi.rs#L238-L244

this means that we don't create a new fork in this example, but I think we want to, as you described. we should instead check the block number for Latest first

mattsse avatar Jan 09 '24 17:01 mattsse

Looked into this issue, should it still be open? I would like to work on it, if it's still open.

The code has been updated already and now looks like this,

        let fork_id = ForkId::new(&fork.url, fork.evm_opts.fork_block_number);
        trace!(?fork_id, "created new forkId");


        if let Some(fork) = self.forks.get(&fork_id).cloned() {
            // assign a new unique fork id but reuse the existing backend
            let unique_fork_id: ForkId =
                format!("{}-{}", fork_id.as_str(), fork.num_senders()).into();
            trace!(?fork_id, ?unique_fork_id, "created new unique forkId");
            fork.inc_senders();
            let backend = fork.backend.clone();
            let env = fork.opts.env.clone();
            self.forks.insert(unique_fork_id.clone(), fork);
            let _ = sender.send(Ok((unique_fork_id, backend, env)));
        } else {
            // there could already be a task for the requested fork in progress

Does this change resolve the issue or further dev work is still needed?

bolajahmad avatar Jan 23 '24 13:01 bolajahmad

Does this change resolve the issue or further dev work is still needed?

I had a look at this and it's still reproducible but if I am not missing something the fix is more complex than suggested. Thing is that create_fork is used for roll fork as well, where block to roll is known and passed by cheatcode user

https://github.com/foundry-rs/foundry/blob/master/crates/evm/core/src/fork/multi.rs#L295-L296

    opts.evm_opts.fork_block_number = Some(block);
    self.create_fork(opts, sender)

In case of creating a new fork the fork_block_number is always None https://github.com/foundry-rs/foundry/blob/master/crates/evm/core/src/fork/multi.rs#L286

    Request::CreateFork(fork, sender) => self.create_fork(*fork, sender),

therefore the ForkId will always match the initial created fork https://github.com/foundry-rs/foundry/blob/master/crates/evm/core/src/fork/multi.rs#L257-L261

    fn create_fork(&mut self, fork: CreateFork, sender: CreateSender) {
        let fork_id = ForkId::new(&fork.url, fork.evm_opts.fork_block_number);
        trace!(?fork_id, "created new forkId");

        if let Some(fork) = self.forks.get(&fork_id).cloned() {

I think the best way would be to be able to set latest block as fork.evm_opts.fork_block_number when creating a new fork but not sure what's the best way to accomplish this as the fork environment / provider is not yet initialized @mattsse wdyt? thanks

grandizzy avatar Feb 09 '24 06:02 grandizzy

yeah I think the best solution is to always assume the fork needs to be created if the block_number is none or latest and then we check afterwards if the url@number pair already exists?

so I guess need to shift around a few self.forks.get(&fork_id) checks

mattsse avatar Feb 09 '24 12:02 mattsse

yeah I think the best solution is to always assume the fork needs to be created if the block_number is none or latest and then we check afterwards if the url@number pair already exists?

so I guess need to shift around a few self.forks.get(&fork_id) checks

makes sense, @bolajahmad lmk if you still want to pick this up, otherwise I can craft a PR for it

grandizzy avatar Feb 09 '24 14:02 grandizzy

@grandizzy feel free to take this :)

ty!

mattsse avatar Feb 09 '24 15:02 mattsse

@grandizzy feel free to take this :)

ty!

thanks, I made a draft PR with a proposed solution https://github.com/foundry-rs/foundry/pull/7087 please have a look when you have time

grandizzy avatar Feb 12 '24 16:02 grandizzy