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

feat: ethclient gas estimation blocknumber param #25009

Closed

Conversation

tynes
Copy link
Contributor

@tynes tynes commented Jun 1, 2022

This commit adds the blocknumber param to the ethclient's
gas estimation function. This lets users choose the block number
to estimate gas against. If nil is passed, then pending will
be used to preserve existing behavior.

Technically, a block tag can be used here but a blocknumber
is used for simplicity. Gas estimation takes similar params
as eth_call, and there are two different functions on the
ethclient for doing eth_call - one by block number and
one by block hash.

The simulated backend is not updated to take into account
the block number parameter, it will still use the pending
state. That functionality can be added in the future if
desired.

Closes #25001

This commit adds the blocknumber param to the `ethclient`'s
gas estimation function. This lets users choose the block number
to estimate gas against. If `nil` is passed, then `pending` will
be used to preserve existing behavior.

Technically, a block tag can be used here but a blocknumber
is used for simplicity. Gas estimation takes similar params
as `eth_call`, and there are two different functions on the
`ethclient` for doing `eth_call` - one by block number and
one by block hash.

The simulated backend is not updated to take into account
the block number parameter, it will still use the pending
state. That functionality can be added in the future if
desired.

Closes ethereum#25001
@tynes
Copy link
Contributor Author

tynes commented Jun 6, 2022

It looks like the tests failed on a flake

not ok 1 TestSnapGetStorageRanges
# peering failed: handshake failed: bad handshake: ðtest.Error{err:(*errors.errorString)(0x210a66c0)}

@fjl
Copy link
Contributor

fjl commented Jun 15, 2022

The test failure you observed there is not related.

@fjl
Copy link
Contributor

fjl commented Jun 15, 2022

We cannot make this change because it modifies the Go API in a backwards-incompatible way.

The only way to achieve this is with new API surface. For example, we could add EstimateGasAtBlock as a new method.

However, I also wonder, what's the point of estimating the gas usage of a transaction at any block other than "latest" or "pending".

@holiman
Copy link
Contributor

holiman commented Nov 4, 2022

@fjl made some good points, and this PR has been inactive since June. Closing.

Thanks for making an effort!

@holiman holiman closed this Nov 4, 2022
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.

ethclient.EstimateGas blocktag param
3 participants