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

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

Closed
2 tasks done
evan-gray opened this issue Aug 2, 2024 · 4 comments · Fixed by #8594
Closed
2 tasks done

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

evan-gray opened this issue Aug 2, 2024 · 4 comments · Fixed by #8594
Labels
T-bug Type: bug

Comments

@evan-gray
Copy link
Contributor

Component

Anvil

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

  • Foundry
  • Foundryup

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
Copy link
Contributor Author

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
Copy link
Member

mattsse commented Aug 2, 2024

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?

async fn do_mine_block(
&self,
pool_transactions: Vec<Arc<PoolTransaction>>,
) -> MinedBlockOutcome {

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

or perhaps add it here

self.do_mine_block(pool_transactions).await

@evan-gray
Copy link
Contributor Author

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

@evan-gray
Copy link
Contributor Author

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. 🤝

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

Successfully merging a pull request may close this issue.

2 participants