Merge pull request #671 from onflow/mpeter/improve-gas-estimation-logic

Improve gas estimation logic for `eth_estimateGas`

Author: m-Peter

api/utils.go

@@ -136,7 +136,7 @@ func handleError[T any](err error, log zerolog.Logger, collector metrics.Collect
 // `EVM.dryRun` inside Cadence scripts, meaning that no state change
 // will occur.
 // This is only useful for `eth_estimateGas` and `eth_call` endpoints.
-func encodeTxFromArgs(args ethTypes.TransactionArgs) (*types.LegacyTx, error) {
+func encodeTxFromArgs(args ethTypes.TransactionArgs) (*types.DynamicFeeTx, error) {
 	var data []byte
 	if args.Data != nil {
 		data = *args.Data
@@ -156,12 +156,19 @@ func encodeTxFromArgs(args ethTypes.TransactionArgs) (*types.LegacyTx, error) {
 		value = args.Value.ToInt()
 	}
 
-	return &types.LegacyTx{
-		Nonce:    0,
-		To:       args.To,
-		Value:    value,
-		Gas:      gasLimit,
-		GasPrice: big.NewInt(0),
-		Data:     data,
+	accessList := types.AccessList{}
+	if args.AccessList != nil {
+		accessList = *args.AccessList
+	}
+
+	return &types.DynamicFeeTx{
+		Nonce:      0,
+		To:         args.To,
+		Value:      value,
+		Gas:        gasLimit,
+		Data:       data,
+		GasTipCap:  (*big.Int)(args.MaxPriorityFeePerGas),
+		GasFeeCap:  (*big.Int)(args.MaxFeePerGas),
+		AccessList: accessList,
 	}, nil
 }

services/requester/requester.go

@@ -45,6 +45,10 @@ var (
 const minFlowBalance = 2
 const blockGasLimit = 120_000_000
 
+// estimateGasErrorRatio is the amount of overestimation eth_estimateGas
+// is allowed to produce in order to speed up calculations.
+const estimateGasErrorRatio = 0.015
+
 type Requester interface {
 	// SendRawTransaction will submit signed transaction data to the network.
 	// The submitted EVM transaction hash is returned.
@@ -58,7 +62,7 @@ type Requester interface {
 	// Note, this function doesn't make and changes in the state/blockchain and is
 	// useful to execute and retrieve values.
 	Call(
-		tx *types.LegacyTx,
+		tx *types.DynamicFeeTx,
 		from common.Address,
 		height uint64,
 		stateOverrides *ethTypes.StateOverride,
@@ -68,7 +72,7 @@ type Requester interface {
 	// Note, this function doesn't make any changes in the state/blockchain and is
 	// useful to executed and retrieve the gas consumption and possible failures.
 	EstimateGas(
-		tx *types.LegacyTx,
+		tx *types.DynamicFeeTx,
 		from common.Address,
 		height uint64,
 		stateOverrides *ethTypes.StateOverride,
@@ -324,7 +328,7 @@ func (e *EVM) GetStorageAt(
 }
 
 func (e *EVM) Call(
-	tx *types.LegacyTx,
+	tx *types.DynamicFeeTx,
 	from common.Address,
 	height uint64,
 	stateOverrides *ethTypes.StateOverride,
@@ -334,42 +338,113 @@ func (e *EVM) Call(
 		return nil, err
 	}
 
-	return result.ReturnedData, err
+	resultSummary := result.ResultSummary()
+	if resultSummary.ErrorCode != 0 {
+		if resultSummary.ErrorCode == evmTypes.ExecutionErrCodeExecutionReverted {
+			return nil, errs.NewRevertError(resultSummary.ReturnedData)
+		}
+		return nil, errs.NewFailedTransactionError(resultSummary.ErrorMessage)
+	}
+
+	return result.ReturnedData, nil
 }
 
 func (e *EVM) EstimateGas(
-	tx *types.LegacyTx,
+	tx *types.DynamicFeeTx,
 	from common.Address,
 	height uint64,
 	stateOverrides *ethTypes.StateOverride,
 ) (uint64, error) {
+	// Note: The following algorithm, is largely inspired from
+	// https://github.com/onflow/go-ethereum/blob/master/eth/gasestimator/gasestimator.go#L49-L192,
+	// and adapted to fit our use-case.
+	// Binary search the gas limit, as it may need to be higher than the amount used
+	var (
+		failingGasLimit uint64 // lowest-known gas limit where tx execution fails
+		passingGasLimit uint64 // lowest-known gas limit where tx execution succeeds
+	)
+	// Determine the highest gas limit that can be used during the estimation.
+	passingGasLimit = blockGasLimit
+	if tx.Gas >= gethParams.TxGas {
+		passingGasLimit = tx.Gas
+	}
+	tx.Gas = passingGasLimit
+	// We first execute the transaction at the highest allowable gas limit,
+	// since if this fails we can return error immediately.
 	result, err := e.dryRunTx(tx, from, height, stateOverrides)
 	if err != nil {
 		return 0, err
 	}
+	resultSummary := result.ResultSummary()
+	if resultSummary.ErrorCode != 0 {
+		if resultSummary.ErrorCode == evmTypes.ExecutionErrCodeExecutionReverted {
+			return 0, errs.NewRevertError(resultSummary.ReturnedData)
+		}
+		return 0, errs.NewFailedTransactionError(resultSummary.ErrorMessage)
+	}
+
+	// For almost any transaction, the gas consumed by the unconstrained execution
+	// above lower-bounds the gas limit required for it to succeed. One exception
+	// is those that explicitly check gas remaining in order to execute within a
+	// given limit, but we probably don't want to return the lowest possible gas
+	// limit for these cases anyway.
+	failingGasLimit = result.GasConsumed - 1
+
+	// There's a fairly high chance for the transaction to execute successfully
+	// with gasLimit set to the first execution's GasConsumed + GasRefund.
+	// Explicitly check that gas amount and use as a limit for the binary search.
+	optimisticGasLimit := (result.GasConsumed + result.GasRefund + gethParams.CallStipend) * 64 / 63
+	if optimisticGasLimit < passingGasLimit {
+		tx.Gas = optimisticGasLimit
+		result, err = e.dryRunTx(tx, from, height, stateOverrides)
+		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.
+			return 0, err
+		}
+		resultSummary := result.ResultSummary()
+		if resultSummary.ErrorCode == evmTypes.ExecutionErrCodeOutOfGas {
+			failingGasLimit = optimisticGasLimit
+		} else {
+			passingGasLimit = optimisticGasLimit
+		}
+	}
 
-	if result.Successful() {
-		// As mentioned in https://github.com/ethereum/EIPs/blob/master/EIPS/eip-150.md#specification
-		// Define "all but one 64th" of N as N - floor(N / 64).
-		// If a call asks for more gas than the maximum allowed amount
-		// (i.e. the total amount of gas remaining in the parent after subtracting
-		// the gas cost of the call and memory expansion), do not return an OOG error;
-		// instead, if a call asks for more gas than all but one 64th of the maximum
-		// allowed amount, call with all but one 64th of the maximum allowed amount of
-		// gas (this is equivalent to a version of EIP-901 plus EIP-1142).
-		// CREATE only provides all but one 64th of the parent gas to the child call.
-		result.GasConsumed = AddOne64th(result.GasConsumed)
-
-		// Adding `gethParams.SstoreSentryGasEIP2200` is needed for this condition:
-		// https://github.com/onflow/go-ethereum/blob/master/core/vm/operations_acl.go#L29-L32
-		result.GasConsumed += gethParams.SstoreSentryGasEIP2200
+	// Binary search for the smallest gas limit that allows the tx to execute successfully.
+	for failingGasLimit+1 < passingGasLimit {
+		// 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(passingGasLimit-failingGasLimit)/float64(passingGasLimit) < estimateGasErrorRatio {
+			break
+		}
+		mid := (passingGasLimit + failingGasLimit) / 2
+		if mid > failingGasLimit*2 {
+			// Most txs don't need much higher gas limit than their gas used, and most txs don't
+			// require near the full block limit of gas, so the selection of where to bisect the
+			// range here is skewed to favor the low side.
+			mid = failingGasLimit * 2
+		}
+		tx.Gas = mid
+		result, err = e.dryRunTx(tx, from, height, stateOverrides)
+		if err != nil {
+			return 0, err
+		}
+		resultSummary := result.ResultSummary()
+		if resultSummary.ErrorCode == evmTypes.ExecutionErrCodeOutOfGas {
+			failingGasLimit = mid
+		} else {
+			passingGasLimit = mid
+		}
+	}
 
-		// Take into account any gas refunds, which are calculated only after
-		// transaction execution.
-		result.GasConsumed += result.GasRefund
+	if tx.AccessList != nil {
+		passingGasLimit += uint64(len(tx.AccessList)) * gethParams.TxAccessListAddressGas
+		passingGasLimit += uint64(tx.AccessList.StorageKeys()) * gethParams.TxAccessListStorageKeyGas
 	}
 
-	return result.GasConsumed, err
+	return passingGasLimit, nil
 }
 
 func (e *EVM) GetCode(
@@ -461,7 +536,7 @@ func (e *EVM) evmToCadenceHeight(height uint64) (uint64, error) {
 }
 
 func (e *EVM) dryRunTx(
-	tx *types.LegacyTx,
+	tx *types.DynamicFeeTx,
 	from common.Address,
 	height uint64,
 	stateOverrides *ethTypes.StateOverride,
@@ -521,14 +596,6 @@ func (e *EVM) dryRunTx(
 		return nil, err
 	}
 
-	resultSummary := result.ResultSummary()
-	if resultSummary.ErrorCode != 0 {
-		if resultSummary.ErrorCode == evmTypes.ExecutionErrCodeExecutionReverted {
-			return nil, errs.NewRevertError(resultSummary.ReturnedData)
-		}
-		return nil, errs.NewFailedTransactionError(resultSummary.ErrorMessage)
-	}
-
 	return result, nil
 }
 

tests/web3js/build_evm_state_test.js

@@ -156,7 +156,7 @@ it('should handle a large number of EVM interactions', async () => {
         gas: 55_000,
         gasPrice: conf.minGasPrice
     }, 82n)
-    assert.equal(estimatedGas, 23823n)
+    assert.equal(estimatedGas, 21358n)
 
     estimatedGas = await web3.eth.estimateGas({
         from: conf.eoa.address,
@@ -165,7 +165,7 @@ it('should handle a large number of EVM interactions', async () => {
         gas: 55_000,
         gasPrice: conf.minGasPrice
     }, latest)
-    assert.equal(estimatedGas, 29292n)
+    assert.equal(estimatedGas, 26811n)
 
     // Add calls to verify correctness of eth_getCode on historical heights
     let code = await web3.eth.getCode(contractAddress, 82n)

tests/web3js/debug_traces_test.js

@@ -35,7 +35,7 @@ it('should retrieve transaction traces', async () => {
     // Assert proper response for `callTracer`
     let txTrace = response.body.result
     assert.equal(txTrace.from, '0xfacf71692421039876a5bb4f10ef7a439d8ef61e')
-    assert.equal(txTrace.gas, '0x118e0c')
+    assert.equal(txTrace.gas, '0x1167ac')
     assert.equal(txTrace.gasUsed, '0x114010')
     assert.equal(txTrace.to, '0x99a64c993965f8d69f985b5171bc20065cc32fab')
     assert.lengthOf(txTrace.input, 9856n)
@@ -92,7 +92,7 @@ it('should retrieve transaction traces', async () => {
     // Assert proper response for `callTracer`
     txTrace = response.body.result
     assert.equal(txTrace.from, '0xfacf71692421039876a5bb4f10ef7a439d8ef61e')
-    assert.equal(txTrace.gas, '0x72c3')
+    assert.equal(txTrace.gas, '0x697f')
     assert.equal(txTrace.gasUsed, '0x6827')
     assert.equal(txTrace.to, '0x99a64c993965f8d69f985b5171bc20065cc32fab')
     assert.equal(
@@ -161,10 +161,10 @@ it('should retrieve transaction traces', async () => {
         txTraces,
         [
             {
-                txHash: '0x87449feedc004c75c0e8b12d01656f2e28366c7d73b1b5336beae20aaa5033dd',
+                txHash: '0xc34f49f9c6b56ebd88095054e2ad42d6854ba818a9657caf3f8500161a5e4ef7',
                 result: {
                     from: '0xfacf71692421039876a5bb4f10ef7a439d8ef61e',
-                    gas: '0x72c3',
+                    gas: '0x697f',
                     gasUsed: '0x6827',
                     to: '0x99a64c993965f8d69f985b5171bc20065cc32fab',
                     input: '0x6057361d0000000000000000000000000000000000000000000000000000000000000064',
@@ -200,10 +200,10 @@ it('should retrieve transaction traces', async () => {
         txTraces,
         [
             {
-                txHash: '0x87449feedc004c75c0e8b12d01656f2e28366c7d73b1b5336beae20aaa5033dd',
+                txHash: '0xc34f49f9c6b56ebd88095054e2ad42d6854ba818a9657caf3f8500161a5e4ef7',
                 result: {
                     from: '0xfacf71692421039876a5bb4f10ef7a439d8ef61e',
-                    gas: '0x72c3',
+                    gas: '0x697f',
                     gasUsed: '0x6827',
                     to: '0x99a64c993965f8d69f985b5171bc20065cc32fab',
                     input: '0x6057361d0000000000000000000000000000000000000000000000000000000000000064',
@@ -257,15 +257,15 @@ it('should retrieve transaction traces', async () => {
         txTrace,
         {
             from: conf.eoa.address.toLowerCase(),
-            gas: '0xc9c7',
+            gas: '0xbf57',
             gasUsed: '0x6147',
             to: contractAddress.toLowerCase(),
             input: '0xc550f90f',
             output: '0x0000000000000000000000000000000000000000000000000000000000000006',
             calls: [
                 {
                     from: contractAddress.toLowerCase(),
-                    gas: '0x6948',
+                    gas: '0x5f01',
                     gasUsed: '0x2',
                     to: '0x0000000000000000000000010000000000000001',
                     input: '0x53e87d66',

tests/web3js/eth_deploy_contract_and_interact_test.js

@@ -221,7 +221,7 @@ it('deploy contract and interact', async () => {
         },
         '0x1'
     )
-    assert.equal(gasEstimate, 23977n)
+    assert.equal(gasEstimate, 21510n)
 
     gasEstimate = await web3.eth.estimateGas(
         {
@@ -233,7 +233,7 @@ it('deploy contract and interact', async () => {
         },
         'latest'
     )
-    assert.equal(gasEstimate, 27398n)
+    assert.equal(gasEstimate, 25052n)
 
     // check that `eth_call` can handle state overrides
     let stateOverrides = {
@@ -274,7 +274,7 @@ it('deploy contract and interact', async () => {
     assert.isDefined(response.body)
 
     result = response.body.result
-    assert.equal(result, '0x72c3')
+    assert.equal(result, '0x697f')
 
     stateOverrides = {
         [contractAddress]: {
@@ -295,5 +295,5 @@ it('deploy contract and interact', async () => {
     // setting a storage slot from a zero-value, to a non-zero value has an
     // increase of about 20,000 gas. Which is quite different to `0x72c3`.
     result = response.body.result
-    assert.equal(result, '0xb69a')
+    assert.equal(result, '0xac6d')
 })