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

test: set nonce explicitly #1354

Merged
merged 1 commit into from Jun 7, 2022

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Jun 7, 2022

Motivation

gets rid of a flaky test as seen here https://github.com/gakonst/ethers-rs/runs/6740032643?check_suite_focus=true
by explicitly setting the nonce

there's a possible race condition rn on anvil if multiple tx without nonce are sent via eth_sendTransaction because the next nonce is determined by onchain +num(pool)
however if multiple tx are sent at the same time they might get the same nonce if the nextNonce for an additional tx is determined before the previous was added to the pool.

https://github.com/foundry-rs/foundry/blob/25241a63db896c88b1717c5522ac284724003eb8/anvil/src/eth/api.rs#L547

https://github.com/foundry-rs/foundry/blob/25241a63db896c88b1717c5522ac284724003eb8/anvil/src/eth/api.rs#L569

this could be prevented via an import lock or some additional check against the Already Imported error but not sure if that's worth it @gakonst ?

the former would have some performance impact because we need to lock for the entire sendTransaction function, for the latter we can try to implement an additional attempt: if tx.nonce was None and we received an Already Imported error try again with new nonce until not Already Imported error.

Solution

  • also ran cargo clippy fix

failing watch_events test fixed in anvil: foundry-rs/foundry#1861

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@mattsse mattsse force-pushed the matt/set-nonce-explicitly branch from 1309bd3 to ce501f9 Compare June 7, 2022 13:21
@mattsse mattsse force-pushed the matt/set-nonce-explicitly branch from ce501f9 to 2432eb2 Compare June 7, 2022 14:30
@gakonst gakonst merged commit 0197fe3 into gakonst:master Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants