Skip to content

Commit

Permalink
internal/ethapi: use same state for each invocation within EstimateGas (
Browse files Browse the repository at this point in the history
ethereum#27505)

EstimateGas repeatedly executes a transaction, performing a binary search with multiple gas prices to determine proper pricing. Each call retrieves a new copy of the state (https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1017) . Because the pending/latest state can change during the execution of EstimateGas, this can potentially cause strange behavior (as noted here: ethereum#27502 (comment)).

This PR modifies EstimateGas to retrieve the state once and use a copy of it for every call invocation it does.
  • Loading branch information
jwasinger authored and MoonShiesty committed Aug 30, 2023
1 parent dec7334 commit 1913b68
Showing 1 changed file with 21 additions and 11 deletions.
32 changes: 21 additions & 11 deletions internal/ethapi/api.go
Expand Up @@ -1011,13 +1011,7 @@ func (context *ChainContext) GetHeader(hash common.Hash, number uint64) *types.H
return header
}

func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64) (*core.ExecutionResult, error) {
defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now())

state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
if state == nil || err != nil {
return nil, err
}
func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.StateDB, header *types.Header, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64) (*core.ExecutionResult, error) {
if err := overrides.Apply(state); err != nil {
return nil, err
}
Expand Down Expand Up @@ -1068,6 +1062,17 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash
return result, nil
}

func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64) (*core.ExecutionResult, error) {
defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now())

state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
if state == nil || err != nil {
return nil, err
}

return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap)
}

func newRevertError(result *core.ExecutionResult) *revertError {
reason, errUnpack := abi.UnpackRevert(result.Revert())
err := errors.New("execution reverted")
Expand Down Expand Up @@ -1187,10 +1192,10 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
cap = hi

// Create a helper to check if a gas allowance results in an executable transaction
executable := func(gas uint64) (bool, *core.ExecutionResult, error) {
executable := func(gas uint64, state *state.StateDB, header *types.Header) (bool, *core.ExecutionResult, error) {
args.Gas = (*hexutil.Uint64)(&gas)

result, err := DoCall(ctx, b, args, blockNrOrHash, nil, nil, 0, gasCap)
result, err := doCall(ctx, b, args, state, header, nil, nil, 0, gasCap)
if err != nil {
if errors.Is(err, core.ErrIntrinsicGas) {
return true, nil, nil // Special case, raise gas limit
Expand All @@ -1199,10 +1204,15 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
}
return result.Failed(), result, nil
}
state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
if state == nil || err != nil {
return 0, err
}
// Execute the binary search and hone in on an executable gas limit
for lo+1 < hi {
s := state.Copy()
mid := (hi + lo) / 2
failed, _, err := executable(mid)
failed, _, err := executable(mid, s, header)

// If the error is not nil(consensus error), it means the provided message
// call or transaction will never be accepted no matter how much gas it is
Expand All @@ -1218,7 +1228,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
}
// Reject the transaction as invalid if it still fails at the highest allowance
if hi == cap {
failed, result, err := executable(hi)
failed, result, err := executable(hi, state, header)
if err != nil {
return 0, err
}
Expand Down

0 comments on commit 1913b68

Please sign in to comment.