-
Notifications
You must be signed in to change notification settings - Fork 20.8k
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
Conversation
yes,why this code had not merge into master? |
I think this really problem, should use LatestBlockNumber rather than PendingBlockNumber can avoid error. |
Reopened to hear the advice of the official team :) |
if use pendingblocknumber,then in code: |
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.
LGTM!
…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>
…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". ```
…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>
…lt (ethereum#24363)" This reverts commit 4f615de.
…lt (ethereum#24363)" This reverts commit 4f615de.
…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>
…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>
…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>
We met a problem that if the
args TransactionArgs
depends on the latest committed blockN
,PendingBlockNumber
is stillN-1
, which causes the estimation to fail.LatestBlockNumber
is a better default value IMO.