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

Improve gas estimation logic for eth_estimateGas #671

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions api/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
133 changes: 100 additions & 33 deletions services/requester/requester.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ const minFlowBalance = 2
const coaFundingBalance = minFlowBalance - 1
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.
Expand All @@ -62,7 +66,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,
Expand All @@ -72,7 +76,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,
Expand Down Expand Up @@ -348,7 +352,7 @@ func (e *EVM) GetStorageAt(
}

func (e *EVM) Call(
tx *types.LegacyTx,
tx *types.DynamicFeeTx,
from common.Address,
height uint64,
stateOverrides *ethTypes.StateOverride,
Expand All @@ -358,42 +362,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this algorithm copied from geth? if so, can you add a link or mention where it's from?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the algorithmic approach is copied over from Geth, with some necessary tweaks to fit our use-case.
Added the note in: affe39d .

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be interesting to see a metric of how many iterations we make on average to get the passingGasLimit maybe something we can add in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea 👍

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(
Expand Down Expand Up @@ -485,7 +560,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,
Expand Down Expand Up @@ -545,14 +620,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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/web3js/build_evm_state_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions tests/web3js/debug_traces_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
8 changes: 4 additions & 4 deletions tests/web3js/eth_deploy_contract_and_interact_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ it('deploy contract and interact', async () => {
},
'0x1'
)
assert.equal(gasEstimate, 23977n)
assert.equal(gasEstimate, 21510n)

gasEstimate = await web3.eth.estimateGas(
{
Expand All @@ -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 = {
Expand Down Expand Up @@ -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]: {
Expand All @@ -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')
})