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

Use next block's context for eth_estimateGas #28175

Open
s1na opened this issue Sep 21, 2023 · 10 comments · May be fixed by #30695
Open

Use next block's context for eth_estimateGas #28175

s1na opened this issue Sep 21, 2023 · 10 comments · May be fixed by #30695

Comments

@s1na
Copy link
Contributor

s1na commented Sep 21, 2023

Nethermind is using the next block's context for eth_estimateGas which seems to be a sane default. This is particularly important because block number and timestamp are exposed to EVM. They merged a PR which increases the timestamp by 12s: NethermindEth/nethermind#6037.

We could add the block overrides and have users set this explicitly even if we don't do the defaults.

@s1na s1na added the type:bug label Sep 21, 2023
@karalabe
Copy link
Member

I'm unsure if that makes much sense/difference. If you're tx relies on the timestamp, then the latest block or the next block will both be wrong, because by the time the tx executes, it will again change timestamps.

@joohhnnn
Copy link
Contributor

How about introducing a new method, like eth_estimateGasCustom, where we can manually set the block number and timestamp?

@karalabe
Copy link
Member

What's the point? If it's gonna be "incorrect" anyway?

@joohhnnn
Copy link
Contributor

What's the point? If it's gonna be "incorrect" anyway?

The point is that you can estimate how much gas is needed under the correct conditions, rather than constantly failing to get an accurate gas estimate due to issues like the timestamp or block number. For example, let's say the current block number is 100, and the contract specifies that the block number must be greater than 200. With a custom estimate, you can overwrite the block number so that you don't have to wait until block 200 to get an accurate gas estimate.

Then, I believe using the estimate of the next block as the default value for the point could be highly effective for scenarios like MEV, which have a strong demand for gas estimation for the next block, and where transactions are likely to be executed in the very next block?

@bnovil
Copy link
Contributor

bnovil commented Oct 11, 2023

It seems useful in some cases. I'd like to make a PR.

@s1na
Copy link
Contributor Author

s1na commented Oct 11, 2023

@joohhnnn what Peter is getting at here is that there's no guarantee which block a tx will be mined in. Hence it's hard to say what exactly to set timestamp and number.

let's say the current block number is 100, and the contract specifies that the block number must be greater than 200.

This is not a great example because in 100 blocks the state of a contract could change enough to make that gas estimate invalid.

I don't disagree with adding the block overrides to estimateGas just to stay consistent with our other methods but after reading Peter's point I'm less convinced about the "need" for this feature.

@joohhnnn
Copy link
Contributor

oh, got the point now. make sense!

@s1na s1na closed this as completed Oct 13, 2023
@s1na
Copy link
Contributor Author

s1na commented Oct 28, 2024

This might be useful for wallets to do some sanity checks against the contract (if contract includes has logic conditional on block context).

Edit: to be clear I am talking about adding block overrides to eth_estimateGas.

@s1na s1na reopened this Oct 28, 2024
@antonydenyer antonydenyer linked a pull request Oct 29, 2024 that will close this issue
@antonydenyer
Copy link

In some situations, a wallet may use eth_estimateGas to estimate gas and check if the transaction will revert. Ideally, you would override the block info to check it's valid for the next block.

@fjl
Copy link
Contributor

fjl commented Oct 29, 2024

I think this is a valid use case for block overrides in estimateGas. We should add them!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants