Skip to content

internal/ethapi: EstimateGas should use LatestBlockNumber by default #24363

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

Merged
merged 3 commits into from
May 11, 2023

Conversation

zhiqiangxu
Copy link
Contributor

We met a problem that if the args TransactionArgs depends on the latest committed block N, PendingBlockNumber is still N-1, which causes the estimation to fail.

LatestBlockNumber is a better default value IMO.

@zhiqiangxu zhiqiangxu closed this Feb 9, 2022
@csquan
Copy link

csquan commented Dec 8, 2022

yes,why this code had not merge into master?

@csquan
Copy link

csquan commented Dec 8, 2022

I think this really problem, should use LatestBlockNumber rather than PendingBlockNumber can avoid error.

@zhiqiangxu zhiqiangxu reopened this Dec 8, 2022
@zhiqiangxu
Copy link
Contributor Author

I think this really problem, should use LatestBlockNumber rather than PendingBlockNumber can avoid error.

Reopened to hear the advice of the official team :)

@csquan
Copy link

csquan commented Dec 8, 2022

if use pendingblocknumber,then in code:
block, err := b.BlockByNumberOrHash(ctx, blockNrOrHash)
you may get error or "block not found".

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@holiman holiman added this to the 1.11.7 milestone May 11, 2023
@holiman holiman requested a review from s1na as a code owner May 11, 2023 09:05
@holiman holiman merged commit 0b66d47 into ethereum:master May 11, 2023
shekhirin pushed a commit to shekhirin/go-ethereum that referenced this pull request Jun 6, 2023
…reum#24363)

* EstimateGas should use LatestBlockNumber by default

* graphql: default to use latest for gas estimation

---------

Co-authored-by: Martin Holst Swende <martin@swende.se>
DarianShawn pushed a commit to dogechain-lab/dbsc that referenced this pull request Sep 3, 2023
…20)

### Description

upstream PR:
[go-ethereum#24363](ethereum/go-ethereum#24363)

We met a problem that if the `args TransactionArgs` depends on the
latest committed block N, `PendingBlockNumber` is still N-1, which
causes the estimation to fail.

`LatestBlockNumber` is a better default value IMO.


ethereum/go-ethereum#24363 (comment)
:

```
if use pendingblocknumber,then in code:
block, err := b.BlockByNumberOrHash(ctx, blockNrOrHash)
you may get error or "block not found".
```
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
…reum#24363)

* EstimateGas should use LatestBlockNumber by default

* graphql: default to use latest for gas estimation

---------

Co-authored-by: Martin Holst Swende <martin@swende.se>
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
MariusVanDerWijden pushed a commit that referenced this pull request Mar 18, 2025
…c block (#27508)

The main use case I see of this is that it allows users to estimate gas
against the same state that they query for their nonce, and the same
state they base the data of their transaction against. This helps ensure
that gas estimation won't fail and the transaction won't revert on-chain
because of a mismatch between the state used for gas estimation and the
state used to generate the inputs to gas estimation or the transaction's
nonce when submitted to the mempool.

This PR also updates the EstimateGas comment based on the new geth
`eth_estimateGas` default of using latest state as of v1.12.0:
#24363

---------

Co-authored-by: Felix Lange <fjl@twurst.com>
sivaratrisrinivas pushed a commit to sivaratrisrinivas/go-ethereum that referenced this pull request Apr 21, 2025
…c block (ethereum#27508)

The main use case I see of this is that it allows users to estimate gas
against the same state that they query for their nonce, and the same
state they base the data of their transaction against. This helps ensure
that gas estimation won't fail and the transaction won't revert on-chain
because of a mismatch between the state used for gas estimation and the
state used to generate the inputs to gas estimation or the transaction's
nonce when submitted to the mempool.

This PR also updates the EstimateGas comment based on the new geth
`eth_estimateGas` default of using latest state as of v1.12.0:
ethereum#24363

---------

Co-authored-by: Felix Lange <fjl@twurst.com>
buddh0 pushed a commit to buddh0/bsc that referenced this pull request Apr 24, 2025
…c block (#27508)

The main use case I see of this is that it allows users to estimate gas
against the same state that they query for their nonce, and the same
state they base the data of their transaction against. This helps ensure
that gas estimation won't fail and the transaction won't revert on-chain
because of a mismatch between the state used for gas estimation and the
state used to generate the inputs to gas estimation or the transaction's
nonce when submitted to the mempool.

This PR also updates the EstimateGas comment based on the new geth
`eth_estimateGas` default of using latest state as of v1.12.0:
ethereum/go-ethereum#24363

---------

Co-authored-by: Felix Lange <fjl@twurst.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants