foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.34k stars 1.77k forks source link

bug(`anvil`): simultaneous mining corrupts block number results #8590

Closed evan-gray closed 3 months ago

evan-gray commented 3 months ago

Component

Anvil

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

What version of Foundry are you on?

forge 0.2.0 (626221f 2024-08-02T00:19:31.215591698Z)

What command(s) is the bug in?

anvil

Operating System

Linux

Describe the bug

If two block mining requests overlap (either via -b <SECONDS> and anvil_mine or two parallel anvil_mine requests), eth_getBlockByNumber returns the incorrect results for blocks following the overlap.

To reproduce:

Terminal 1:

anvil

Terminal 2:

cast rpc anvil_mine 0x4000

Terminal 3 (while the previous command is still running):

cast rpc anvil_mine 0x4000

Afterwards:

$ cast rpc eth_getBlockByNumber "0x1" false | jq .number
"0x1" <-- correct
$ cast rpc eth_getBlockByNumber latest false | jq .number
"0x6871" <-- inconsistent with anvil output
$ cast rpc eth_getBlockByNumber 0x6871 false | jq .number
"0x50e2" <-- incorrect, should match the input

The anvil output lists 32768 as the last block mined, which is 0x8000.

I had intermittently hit this error in integration tests that run in parallel and relied on setting -b 1 but also used anvil_mine 0x40 to quickly reach finalized depth. I'm not sure whether parallel anvil_mine requests should be cumulative or simply ensure that at least the specified number of blocks are queued to be mined from the latest block at the time of the request, but I could find either acceptable for our use case.

evan-gray commented 3 months ago

Able to reproduce with this test in crates/anvil/tests/it/api.rs

#[tokio::test(flavor = "multi_thread")]
async fn can_mine_while_mining() {
    let (api, _) = spawn(NodeConfig::test()).await;

    let total_blocks = 200;

    let block_number = api
        .block_by_number(BlockNumberOrTag::Latest)
        .await
        .unwrap()
        .unwrap()
        .header
        .number
        .unwrap();
    assert_eq!(block_number, 0);

    let block = api.block_by_number(BlockNumberOrTag::Number(block_number)).await.unwrap().unwrap();
    assert_eq!(block.header.number.unwrap(), 0);

    let result = join!(
        api.anvil_mine(Some(U256::from(total_blocks / 2)), None),
        api.anvil_mine(Some(U256::from(total_blocks / 2)), None)
    );
    result.0.unwrap();
    result.1.unwrap();
    tokio::time::sleep(Duration::from_millis(100)).await;

    let block_number = api
        .block_by_number(BlockNumberOrTag::Latest)
        .await
        .unwrap()
        .unwrap()
        .header
        .number
        .unwrap();
    assert_eq!(block_number, total_blocks);

    let block = api.block_by_number(BlockNumberOrTag::Number(block_number)).await.unwrap().unwrap();
    assert_eq!(block.header.number.unwrap(), total_blocks);
}
test api::can_mine_while_mining ... FAILED

failures:

---- api::can_mine_while_mining stdout ----
thread 'api::can_mine_while_mining' panicked at crates/anvil/tests/it/api.rs:415:5:
assertion `left == right` failed
  left: 162
 right: 200
mattsse commented 3 months ago

I see, there are actually a few race conditions in the do_mine_block impl

the best fix here imo would be to add an additional tokio mutex that is kept for the duration of the function

do you want to try this?

https://github.com/foundry-rs/foundry/blob/cce36f85c80946bd4a1868411aa99eda879a0e43/crates/anvil/src/eth/backend/mem/mod.rs#L926-L929

mining: Arc<tokio::Mutex<()>>

or perhaps add it here https://github.com/foundry-rs/foundry/blob/cce36f85c80946bd4a1868411aa99eda879a0e43/crates/anvil/src/eth/backend/mem/mod.rs#L923-L923

evan-gray commented 3 months ago

Was just looking into this! Will give it a shot 🙂

evan-gray commented 3 months ago

Thanks for the support @mattsse ! I put up a PR with a test and also tested that it fixes the issue detailed above. Let me know if there's any changes you'd want to see to it. 🤝