From 61b844f2b2cf6b9233a5bcd43c68d9be6f7392b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 28 Nov 2023 22:31:47 +0200 Subject: [PATCH] eth/gasestimator: allow slight estimation error in favor of less iterations (#28618) * eth/gasestimator: early exit for plain transfer and error allowance * core, eth/gasestimator: hard guess at a possible required gas * internal/ethapi: update estimation tests with the error ratio * eth/gasestimator: I hate you linter * graphql: fix gas estimation test --------- Co-authored-by: Oren --- core/state_transition.go | 23 +++++++++------- eth/gasestimator/gasestimator.go | 45 ++++++++++++++++++++++++++++++-- graphql/graphql_test.go | 2 +- internal/ethapi/api.go | 13 ++++++--- internal/ethapi/api_test.go | 2 +- 5 files changed, 68 insertions(+), 17 deletions(-) diff --git a/core/state_transition.go b/core/state_transition.go index 612fdd7813..540f63fda7 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -32,9 +32,10 @@ import ( // ExecutionResult includes all output after executing given evm // message no matter the execution itself is successful or not. type ExecutionResult struct { - UsedGas uint64 // Total used gas but include the refunded gas - Err error // Any error encountered during the execution(listed in core/vm/errors.go) - ReturnData []byte // Returned data from evm(function result or data supplied with revert opcode) + UsedGas uint64 // Total used gas, not including the refunded gas + RefundedGas uint64 // Total gas refunded after execution + Err error // Any error encountered during the execution(listed in core/vm/errors.go) + ReturnData []byte // Returned data from evm(function result or data supplied with revert opcode) } // Unwrap returns the internal evm error which allows us for further @@ -419,12 +420,13 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) { ret, st.gasRemaining, vmerr = st.evm.Call(sender, st.to(), msg.Data, st.gasRemaining, msg.Value) } + var gasRefund uint64 if !rules.IsLondon { // Before EIP-3529: refunds were capped to gasUsed / 2 - st.refundGas(params.RefundQuotient) + gasRefund = st.refundGas(params.RefundQuotient) } else { // After EIP-3529: refunds are capped to gasUsed / 5 - st.refundGas(params.RefundQuotientEIP3529) + gasRefund = st.refundGas(params.RefundQuotientEIP3529) } effectiveTip := msg.GasPrice if rules.IsLondon { @@ -442,13 +444,14 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) { } return &ExecutionResult{ - UsedGas: st.gasUsed(), - Err: vmerr, - ReturnData: ret, + UsedGas: st.gasUsed(), + RefundedGas: gasRefund, + Err: vmerr, + ReturnData: ret, }, nil } -func (st *StateTransition) refundGas(refundQuotient uint64) { +func (st *StateTransition) refundGas(refundQuotient uint64) uint64 { // Apply refund counter, capped to a refund quotient refund := st.gasUsed() / refundQuotient if refund > st.state.GetRefund() { @@ -463,6 +466,8 @@ func (st *StateTransition) refundGas(refundQuotient uint64) { // Also return remaining gas to the block gas counter so it is // available for the next transaction. st.gp.AddGas(st.gasRemaining) + + return refund } // gasUsed returns the amount of gas used up by the state transition. diff --git a/eth/gasestimator/gasestimator.go b/eth/gasestimator/gasestimator.go index 3e74b5b08b..4a8e20dfed 100644 --- a/eth/gasestimator/gasestimator.go +++ b/eth/gasestimator/gasestimator.go @@ -42,6 +42,8 @@ type Options struct { Chain core.ChainContext // Chain context to access past block hashes Header *types.Header // Header defining the block context to execute in State *state.StateDB // Pre-state on top of which to estimate the gas + + ErrorRatio float64 // Allowed overestimation ratio for faster estimation termination } // Estimate returns the lowest possible gas limit that allows the transaction to @@ -86,16 +88,28 @@ func Estimate(ctx context.Context, call *core.Message, opts *Options, gasCap uin if transfer == nil { transfer = new(big.Int) } - log.Warn("Gas estimation capped by limited funds", "original", hi, "balance", balance, + log.Debug("Gas estimation capped by limited funds", "original", hi, "balance", balance, "sent", transfer, "maxFeePerGas", feeCap, "fundable", allowance) hi = allowance.Uint64() } } // Recap the highest gas allowance with specified gascap. if gasCap != 0 && hi > gasCap { - log.Warn("Caller gas above allowance, capping", "requested", hi, "cap", gasCap) + log.Debug("Caller gas above allowance, capping", "requested", hi, "cap", gasCap) hi = gasCap } + // If the transaction is a plain value transfer, short circuit estimation and + // directly try 21000. Returning 21000 without any execution is dangerous as + // some tx field combos might bump the price up even for plain transfers (e.g. + // unused access list items). Ever so slightly wasteful, but safer overall. + if len(call.Data) == 0 { + if call.To != nil && opts.State.GetCodeSize(*call.To) == 0 { + failed, _, err := execute(ctx, call, opts, params.TxGas) + if !failed && err == nil { + return params.TxGas, nil, nil + } + } + } // We first execute the transaction at the highest allowable gas limit, since if this fails we // can return error immediately. failed, result, err := execute(ctx, call, opts, hi) @@ -115,8 +129,35 @@ func Estimate(ctx context.Context, call *core.Message, opts *Options, gasCap uin // limit for these cases anyway. lo = result.UsedGas - 1 + // There's a fairly high chance for the transaction to execute successfully + // with gasLimit set to the first execution's usedGas + gasRefund. Explicitly + // check that gas amount and use as a limit for the binary search. + optimisticGasLimit := (result.UsedGas + result.RefundedGas + params.CallStipend) * 64 / 63 + if optimisticGasLimit < hi { + failed, _, err = execute(ctx, call, opts, optimisticGasLimit) + if err != nil { + // This should not happen under normal conditions since if we make it this far the + // transaction had run without error at least once before. + log.Error("Execution error in estimate gas", "err", err) + return 0, nil, err + } + if failed { + lo = optimisticGasLimit + } else { + hi = optimisticGasLimit + } + } // Binary search for the smallest gas limit that allows the tx to execute successfully. for lo+1 < hi { + if opts.ErrorRatio > 0 { + // It is a bit pointless to return a perfect estimation, as changing + // network conditions require the caller to bump it up anyway. Since + // wallets tend to use 20-25% bump, allowing a small approximation + // error is fine (as long as it's upwards). + if float64(hi-lo)/float64(hi) < opts.ErrorRatio { + break + } + } mid := (hi + lo) / 2 if mid > lo*2 { // Most txs don't need much higher gas limit than their gas used, and most txs don't diff --git a/graphql/graphql_test.go b/graphql/graphql_test.go index a83d6bbd46..f91229d015 100644 --- a/graphql/graphql_test.go +++ b/graphql/graphql_test.go @@ -139,7 +139,7 @@ func TestGraphQLBlockSerialization(t *testing.T) { // should return `estimateGas` as decimal { body: `{"query": "{block{ estimateGas(data:{}) }}"}`, - want: `{"data":{"block":{"estimateGas":"0xcf08"}}}`, + want: `{"data":{"block":{"estimateGas":"0xd221"}}}`, code: 200, }, // should return `status` as decimal diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index f322132769..c0b28e4b69 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -51,6 +51,10 @@ import ( "github.com/tyler-smith/go-bip39" ) +// estimateGasErrorRatio is the amount of overestimation eth_estimateGas is +// allowed to produce in order to speed up calculations. +const estimateGasErrorRatio = 0.015 + // EthereumAPI provides an API to access Ethereum related information. type EthereumAPI struct { b Backend @@ -1189,10 +1193,11 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr } // Construct the gas estimator option from the user input opts := &gasestimator.Options{ - Config: b.ChainConfig(), - Chain: NewChainContext(ctx, b), - Header: header, - State: state, + Config: b.ChainConfig(), + Chain: NewChainContext(ctx, b), + Header: header, + State: state, + ErrorRatio: estimateGasErrorRatio, } // Run the gas estimation andwrap any revertals into a custom return call, err := args.ToMessage(gasCap, header.BaseFee) diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index 9b08fd8d42..c2490ac703 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -735,7 +735,7 @@ func TestEstimateGas(t *testing.T) { t.Errorf("test %d: want no error, have %v", i, err) continue } - if uint64(result) != tc.want { + if float64(result) > float64(tc.want)*(1+estimateGasErrorRatio) { t.Errorf("test %d, result mismatch, have\n%v\n, want\n%v\n", i, uint64(result), tc.want) } }