-
Notifications
You must be signed in to change notification settings - Fork 8
Default eth_estimateGas block parameter to pending to match Anvil/EDR behavior #509
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -722,6 +722,9 @@ impl ApiServer { | |
| ) -> Result<sp_core::U256> { | ||
| node_info!("eth_estimateGas"); | ||
|
|
||
| // Default to pending block, same as EDR and original Anvil | ||
| // See: https://github.com/paritytech/contract-issues/issues/261 | ||
| let block = block.or(Some(BlockId::Number(BlockNumberOrTag::Pending))); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't we change that inside get_block_hash_for_tag instead when block is None?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updating get_block_hash_for_tag could impact the behavior of several other APIs, such as eth_call and eth_getTransactionCount. Looking at the original Anvil implementation, the block is defaulted to pending (from None) only for eth_estimateGas. |
||
| let hash = self.get_block_hash_for_tag(block).await?; | ||
| let runtime_api = self.eth_rpc_client.runtime_api(hash); | ||
| let dry_run = runtime_api | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anvil-polkadot does not currently have any special support for the pending tag (like building an intermediate block with the transactions in the pool), so I don't think this achieves anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing this PR, Alin!
This change enables the logic in eth-rpc and pallet-revive that is triggered when the block tag is pending.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I see. So for now we only want to have the current timestamp and the block number being increased. I thought we wanted to achieve full compatibility with what a real pending block would be in upstream anvil