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

eth_estimateGas uses global gas cap as the high watermwark #29695

Closed
karalabe opened this issue May 2, 2024 · 4 comments
Closed

eth_estimateGas uses global gas cap as the high watermwark #29695

karalabe opened this issue May 2, 2024 · 4 comments
Labels

Comments

@karalabe
Copy link
Member

karalabe commented May 2, 2024

Tracker for #29509.

Gas estimation should cap itself to the block gas limit if none is given, currently it caps itself to the configured call gas cap. This is bad because it wastes estimation cycles.

Interestingly, the gas estimator itself works correctly, but is called bad (the gas is set before estimation to the global gas cap, so the poor estimator works with what it's given). The cause is a long stream of refactors and abstractions which gradually bubbles setting the gas value of a transaction in ToMessage, which sounds counterintuitive.

Fixing it is not super obvious, because changing ToMessage to not set the gas might have other implications. We should look through the call sites and see how to best fix without adding workarounds.

@quminghaonanya
Copy link

quminghaonanya commented May 7, 2024

can we get the block gas cap in the DoEstimateGas function and pass it to the gas estimator?

(just bring the below code back and pass the block gas cap to gasestimator.Estimate.
https://github.com/ethereum/go-ethereum/pull/28600/files#diff-c426ecd2f7d247753b9ea8c1cc003f21fa412661c1f967d203d4edf8163da344L1208-L1216 )

so that we can make the higher boundary capped at the block gas cap rather than the call gas cap.

@quminghaonanya
Copy link

Here's the MR to demonstrate my idea: #29732

@SangIlMo
Copy link
Contributor

SangIlMo commented May 8, 2024

I think we can just recap gas.args with header gasLimit when gas.args is greater than block gasLimit.
This enable us to check if gas.args == nil in CallDefaults(). And also, we can prevent gas.args from being greater than block gasLimit.

Here's the suggested PR: #29738

I think it could be a simple change which meets the purpose of this issue.

@arajasek
Copy link
Contributor

arajasek commented Jul 10, 2024

@karalabe Can this be closed since #29738 landed?

@fjl fjl closed this as completed Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants