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

fix: crash when waiting for a tx confirmation #1774

Merged
merged 1 commit into from
May 22, 2024

Conversation

banteg
Copy link
Collaborator

@banteg banteg commented May 10, 2024

What I did

in recent versions of anvil sometimes brownie ends up in a race condition when waiting for a tx confirmation. the bug results inKeyError: 'blockNumber'.

the root cause is anvil retuning no blockNumber field in eth_getTransactionByHash for pending transactions.

the bug was introduced here alloy-rs/alloy#523

How I did it

use tx.get('blockNumber') instead of tx['blockNumber']

How to verify it

run a bunch of transactions, it doesn't happen every time. it's easier if you run anvil alongside, so you can see its output. the bug manifests when there is a eth_getTransactionByHash before Transaction printout.

eth_sendTransaction
eth_getTransactionByHash
eth_getCode

    Transaction: 0xc94c6c69570ca19763eda0a33a9b6a67803a5fa1fca2861ed2f4c08607edf0fb
    Gas used: 21000

Checklist

  • I have confirmed that my PR passes all linting checks
  • I have included test cases
  • I have updated the documentation
  • I have added an entry to the changelog

@banteg
Copy link
Collaborator Author

banteg commented May 10, 2024

fixed in alloy-rs/alloy#719, so should be fixed in the next nightly anvil.

your call whether to merge this, technically brownie has followed the spec and anvil hasn't.

@iamdefinitelyahuman
Copy link
Member

Merging because this also fixes a related issue flagged by @michwill in an unrelated RPC client. Not sure the root cause but this solves, and it's easier to handle here than try to fix somewhere upstream.

@iamdefinitelyahuman iamdefinitelyahuman merged commit 32b5e34 into master May 22, 2024
14 of 24 checks passed
@iamdefinitelyahuman iamdefinitelyahuman deleted the fix/tx-confirmation branch May 22, 2024 13:47
@lemenkov
Copy link

Funny but this field is required and yet something weird happened recently.

@banteg
Copy link
Collaborator Author

banteg commented May 24, 2024

the root cause was this alloy-rs/alloy#719

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.

3 participants