-
Notifications
You must be signed in to change notification settings - Fork 10
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
Improve gas estimation logic for eth_estimateGas
#671
Improve gas estimation logic for eth_estimateGas
#671
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
b2c86aa
to
3d94243
Compare
from common.Address, | ||
height uint64, | ||
stateOverrides *ethTypes.StateOverride, | ||
) (uint64, error) { | ||
// Binary search the gas limit, as it may need to be higher than the amount used |
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.
is this algorithm copied from geth? if so, can you add a link or mention where it's from?
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.
Yes, the algorithmic approach is copied over from Geth, with some necessary tweaks to fit our use-case.
Added the note in: affe39d .
3d94243
to
affe39d
Compare
92ad02c
to
227f57e
Compare
mid = failingGasLimit * 2 | ||
} | ||
tx.Gas = mid | ||
result, err = e.dryRunTx(tx, from, height, stateOverrides) |
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.
It would be interesting to see a metric of how many iterations we make on average to get the passingGasLimit
maybe something we can add in the future.
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.
That's a good idea 👍
Closes: #663
Description
Instead of simply adding some buffer gas to the gas consumed returned by dry-running an EVM transaction, we now use that value as a starting point, and perform a binary search for the minimum required gas for successfully running an EVM transaction. In the context of gas estimation, successful means to not fail with a
out of gas
execution error.The binary search approach, is largely inspired from https://github.com/onflow/go-ethereum/blob/master/eth/gasestimator/gasestimator.go#L49-L192, and adapter to our use-case.
For contributor use:
master
branchFiles changed
in the Github PR explorer