Skip to content
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

fix eth_estimateGas: remove ErrorRatio #29729

Closed
wants to merge 1 commit into from

Conversation

wjmelements
Copy link
Contributor

Fixes #29726 by removing ErrorRatio from the gas estimator.

I prefer this fix to #29727 because eth_estimateGas has defined behavior that is important for users such as myself. This defined behavior is found in several places, listed below. The "estimate" meaning pertains to how the execution can change if the contingent state changes, and does not excuse inaccuracy. False results are very bad in my line of work, and can cause huge losses.

Defined in geth codebase:

DoEstimateGas returns the lowest possible gas limit that allows the transaction to run

EstimateGas returns the lowest possible gas limit that allows the transaction to run

Defined in ethjsonrpc

Generates and returns an estimate of how much gas is necessary to allow the transaction to complete.

@karalabe
Copy link
Member

karalabe commented May 8, 2024

Could you detail a bit why it's important for you to have a 100% accurate estimate?

Reasons against it:

  • Allowing 1.5% error rate gives us a 50% runtime performance gain.
  • The transaction only pays for used gas, so anything above that is not relevant from a fee percpective.
  • An exact number is only correct if the context stays the same during inclusion time. If the dependent state changes, an exact estimate could result in a failed transaction vs. an overestimate.

To top it off, wallets usually add 25+% on top, making out 1.5% a drop in the bucket.

@wjmelements
Copy link
Contributor Author

wjmelements commented May 8, 2024 via email

@karalabe
Copy link
Member

karalabe commented May 8, 2024

Please see my comment at #29726 (comment)

@karalabe karalabe closed this May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eth_estimateGas returns wrong gas, off by 1.5%
2 participants