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 and optimize eth_estimateGas API #409

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

trinhdn2
Copy link
Contributor

@trinhdn2 trinhdn2 commented Dec 9, 2023

Prerequisite:

This PR add a rpc.BlockNumber input parameters for eth_estimateGas API because most 3rd-party tools (such as MetaMask, Remix IDE, etc.) require the same parameters for this API.

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

_, _, failed, err := s.doCall(ctx, args, rpc.LatestBlockNumber, vm.Config{}, 0)

Because the pending/latest state can be changed during the execution of EstimateGas, this can potentially cause strange behaviors. EstimateGas currently conducts a binary search to determine a suitable gas limit for a tx, and continues searching with a higher gas limit whenever the transaction's execution results in revert. While this makes sense if the reason for revert is out of gas, increasing the gas limit is highly unlikely to allow a non-OOG reverting transaction to succeed (unless it is making unusual use of the GAS opcode).

This PR modifies EstimateGas to retrieve the state once and use a copy of it for every call invocation it does. This has two benefits:

  • Saves compute overhead.
  • Eliminates a race condition where a reverting transaction causes the gas limit lowerbound to increase repeatedly during gas estimation, only for a state change to allow the transaction to later succeed later in the loop, resulting in an unreasonably high estimate being returned. (We believe this is happening in practice in one of our applications)

Resolve issue:

Supersede:

Reference:

@trinhdn2 trinhdn2 mentioned this pull request Dec 9, 2023
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.

None yet

2 participants