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

ethclient.EstimateGas blocktag param #25001

Open
tynes opened this issue May 31, 2022 · 1 comment
Open

ethclient.EstimateGas blocktag param #25001

tynes opened this issue May 31, 2022 · 1 comment

Comments

@tynes
Copy link
Contributor

tynes commented May 31, 2022

Rationale

The ability to pass a block tag with the RPC call eth_estimateGas was merged in #21545. This was done to make the RPC spec compliant.

The ethclient does not have an extra argument to be able to pass through a block tag, so "pending" is always used.

func (ec *Client) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) {

func (s *PublicBlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, blockNrOrHash *rpc.BlockNumberOrHash) (hexutil.Uint64, error) {
bNrOrHash := rpc.BlockNumberOrHashWithNumber(rpc.PendingBlockNumber)

For some users of ethclient, the difference between "pending" and "latest" can result in the gas estimation failing or succeeding. Ideally ethclient.EstimateGas can be extended to accept an additional block tag argument.

This is an API breaking change but fairly simple to fix, passing nil should default to "pending" so that the same behavior is maintained.

Implementation

diff --git a/ethclient/ethclient.go b/ethclient/ethclient.go
index 24edd8648..7704c0673 100644
--- a/ethclient/ethclient.go
+++ b/ethclient/ethclient.go
@@ -509,9 +509,12 @@ func (ec *Client) SuggestGasTipCap(ctx context.Context) (*big.Int, error) {
 // the current pending state of the backend blockchain. There is no guarantee that this is
 // the true gas limit requirement as other transactions may be added or removed by miners,
 // but it should provide a basis for setting a reasonable default.
-func (ec *Client) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) {
+func (ec *Client) EstimateGas(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) (uint64, error) {
 	var hex hexutil.Uint64
-	err := ec.c.CallContext(ctx, &hex, "eth_estimateGas", toCallArg(msg))
+	if blockNumber == nil {
+		blockNumber = big.NewInt(-1)
+	}
+	err := ec.c.CallContext(ctx, &hex, "eth_estimateGas", toCallArg(msg), toBlockNumArg(blockNumber))
 	if err != nil {
 		return 0, err
 	}

The bind.ContractBackend interface will need to be updated as well for it to build.

I am willing to finish the implementation if this is considered an acceptable change.

@s1na
Copy link
Contributor

s1na commented Jun 1, 2022

Makes sense to me. Please submit a PR.

tynes added a commit to tynes/go-ethereum that referenced this issue 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 ethereum#25001
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.

3 participants
@s1na @tynes and others