internal/ethapi: cherry pick EstimateGas improvement from go-ethereum #392
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
minh-bq
force-pushed
the
feat/estimate-gas-improvement
branch
3 times, most recently
from
09a2e80 to
a2edf0b
Compare
DNK90
approved these changes
Jan 16, 2024
* EstimateGas should use LatestBlockNumber by default * graphql: default to use latest for gas estimation --------- Co-authored-by: Martin Holst Swende <martin@swende.se>
This fixes a bug where contract code would be overridden to empty code ("0x")
when the Code field of OverrideAccount was left nil. The change also cleans up
the encoding of overrides to only send necessary fields, and improves documentation.
Fixes #25615
Co-authored-by: Felix Lange <fjl@twurst.com>
Co-authored-by: Martin Holst Swende <martin@swende.se>
Adds an optional config parameter to eth_call which allows users to override block context fields (same functionality that was added to traceCall in #24871) --------- Co-authored-by: Martin Holst Swende <martin@swende.se>
…s (#27505) EstimateGas repeatedly executes a transaction, performing a binary search with multiple gas prices to determine proper pricing. Each call retrieves a new copy of the state (https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1017) . Because the pending/latest state can change during the execution of EstimateGas, this can potentially cause strange behavior (as noted here: ethereum/go-ethereum#27502 (comment)). This PR modifies EstimateGas to retrieve the state once and use a copy of it for every call invocation it does.
Optimizations: - Previously, if a transaction was reverting, EstimateGas would exhibit worst-case behavior and binary search up to the max gas limit (~40 state-clone + tx executions). This change allows EstimateGas to return after only a single unconstrained execution in this scenario. - Uses the gas used from the unconstrained execution to bias the remaining binary search towards the likely solution in a simple way that doesn't impact the worst case. For a typical contract-invoking transaction, this reduces the median number of state-clone+executions from 25 to 18 (28% reduction). Cleanup: - added & improved function + code comments - correct the EstimateGas documentation to clarify the gas limit determination is at latest block, not pending, if the blockNr is unspecified.
…ations (#28618) * eth/gasestimator: early exit for plain transfer and error allowance * core, eth/gasestimator: hard guess at a possible required gas * internal/ethapi: update estimation tests with the error ratio * eth/gasestimator: I hate you linter * graphql: fix gas estimation test --------- Co-authored-by: Oren <orenyomtov@users.noreply.github.com>
This commit only copies the TestEstimateGas and TestCall from the original file in go-ethereum.
minh-bq
force-pushed
the
feat/estimate-gas-improvement
branch
from
a2edf0b to
cee8362
Compare
huyngopt1994
approved these changes
Feb 15, 2024
Francesco4203
pushed a commit
to Francesco4203/ronin
that referenced
this pull request
Jun 18, 2024
…axieinfinity#392) * internal/ethapi: make EstimateGas use `latest` block by default (#24363) * EstimateGas should use LatestBlockNumber by default * graphql: default to use latest for gas estimation --------- Co-authored-by: Martin Holst Swende <martin@swende.se> * ethclient/gethclient: fix bugs in override object encoding (#25616) This fixes a bug where contract code would be overridden to empty code ("0x") when the Code field of OverrideAccount was left nil. The change also cleans up the encoding of overrides to only send necessary fields, and improves documentation. Fixes #25615 Co-authored-by: Felix Lange <fjl@twurst.com> Co-authored-by: Martin Holst Swende <martin@swende.se> * internal/ethapi: add block overrides to eth_call (#26414) Adds an optional config parameter to eth_call which allows users to override block context fields (same functionality that was added to traceCall in #24871) --------- Co-authored-by: Martin Holst Swende <martin@swende.se> * internal/ethapi: use same state for each invocation within EstimateGas (#27505) EstimateGas repeatedly executes a transaction, performing a binary search with multiple gas prices to determine proper pricing. Each call retrieves a new copy of the state (https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1017) . Because the pending/latest state can change during the execution of EstimateGas, this can potentially cause strange behavior (as noted here: ethereum/go-ethereum#27502 (comment)). This PR modifies EstimateGas to retrieve the state once and use a copy of it for every call invocation it does. * internal/ethapi: add state override to estimateGas (#27845) * internal/ethapi: optimize & clean up EstimateGas (#27710) Optimizations: - Previously, if a transaction was reverting, EstimateGas would exhibit worst-case behavior and binary search up to the max gas limit (~40 state-clone + tx executions). This change allows EstimateGas to return after only a single unconstrained execution in this scenario. - Uses the gas used from the unconstrained execution to bias the remaining binary search towards the likely solution in a simple way that doesn't impact the worst case. For a typical contract-invoking transaction, this reduces the median number of state-clone+executions from 25 to 18 (28% reduction). Cleanup: - added & improved function + code comments - correct the EstimateGas documentation to clarify the gas limit determination is at latest block, not pending, if the blockNr is unspecified. * eth/gasestimator, internal/ethapi: move gas estimator out of rpc (#28600) * eth/gasestimator: allow slight estimation error in favor of less iterations (#28618) * eth/gasestimator: early exit for plain transfer and error allowance * core, eth/gasestimator: hard guess at a possible required gas * internal/ethapi: update estimation tests with the error ratio * eth/gasestimator: I hate you linter * graphql: fix gas estimation test --------- Co-authored-by: Oren <orenyomtov@users.noreply.github.com> * internal/ethapi: partially copy the api_test from go-ethereum This commit only copies the TestEstimateGas and TestCall from the original file in go-ethereum. --------- Co-authored-by: zhiqiangxu <652732310@qq.com> Co-authored-by: Martin Holst Swende <martin@swende.se> Co-authored-by: Janko Simonovic <simonovic86@gmail.com> Co-authored-by: Felix Lange <fjl@twurst.com> Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com> Co-authored-by: jwasinger <j-wasinger@hotmail.com> Co-authored-by: Amin Talebi <talebi242@gmail.com> Co-authored-by: Roberto Bayardo <roberto.bayardo@coinbase.com> Co-authored-by: Péter Szilágyi <peterke@gmail.com> Co-authored-by: Oren <orenyomtov@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some notable changes:
gas = (result.UsedGas + result.RefundedGas + params.CallStipend) * 64 / 63to reduce the binary search range