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

Add eth_necessaryGas to restore eth_estimateGas behavior #29727

Closed

Conversation

wjmelements
Copy link
Contributor

Fixes #29726 by keeping eth_estimateGas broken in favor of a new method that forces precision.

Fixes documentation for eth_estimateGas to signal its current behavior.

IMO eth_estimateGas should be correct and the faster version should be the non-standard method. eth_estimateGas has defined behavior:

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

However if geth can truly single-handledly change the specification of ethjsonrpc, then I am powerless to stop them. Instead I create a new method, eth_necessaryGas to implement the defined behavior of eth_estimateGas.

But a more ethical fix would be to revert the PR that introduced ErrorRatio.

@wjmelements wjmelements requested a review from s1na as a code owner May 8, 2024 00:36
@@ -1214,7 +1214,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
return hexutil.Uint64(estimate), nil
}

// EstimateGas returns the lowest possible gas limit that allows the transaction to run
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where I fixed your documentation.

@wjmelements
Copy link
Contributor Author

I have created #29729 as an alternative but in case that solution is not accepted I hope to have some method that can do the prior behavior.

@karalabe
Copy link
Member

karalabe commented May 8, 2024

Please see my comment at #29726 (comment)

@karalabe karalabe closed this May 8, 2024
@fjl
Copy link
Contributor

fjl commented May 8, 2024

@wjmelements just a note, you may be over-interpreting what the documentation says. Right after the sentence you quoted, it also has this:

Note that the estimate may be significantly more than the amount of gas actually used by the transaction, for a variety of reasons including EVM mechanics and node performance.

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%
3 participants