From f5d6fd36043a0c1e7f04ae9670c3ad2731d91f75 Mon Sep 17 00:00:00 2001
From: devopsbo3 <69951731+devopsbo3@users.noreply.github.com>
Date: Fri, 10 Nov 2023 12:27:53 -0600
Subject: [PATCH] Revert "internal/ethapi: optimize & clean up EstimateGas
 (#27710)"

This reverts commit 8be8e98163820c63ee915608ef2d1f3831ad9b4b.
---
 internal/ethapi/api.go | 122 +++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 65 deletions(-)

diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go
index 604c00a8fd..09f41a8361 100644
--- a/internal/ethapi/api.go
+++ b/internal/ethapi/api.go
@@ -1162,30 +1162,12 @@ func (s *BlockChainAPI) Call(ctx context.Context, args TransactionArgs, blockNrO
 	return result.Return(), result.Err
 }
 
-// executeEstimate is a helper that executes the transaction under a given gas limit and returns
-// true if the transaction fails for a reason that might be related to not enough gas. A non-nil
-// error means execution failed due to reasons unrelated to the gas limit.
-func executeEstimate(ctx context.Context, b Backend, args TransactionArgs, state *state.StateDB, header *types.Header, gasCap uint64, gasLimit uint64) (bool, *core.ExecutionResult, error) {
-	args.Gas = (*hexutil.Uint64)(&gasLimit)
-	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
-		}
-		return true, nil, err // Bail out
-	}
-	return result.Failed(), result, nil
-}
-
-// DoEstimateGas returns the lowest possible gas limit that allows the transaction to run
-// successfully at block `blockNrOrHash`. It returns error if the transaction would revert, or if
-// there are unexpected failures. The gas limit is capped by both `args.Gas` (if non-nil &
-// non-zero) and `gasCap` (if non-zero).
 func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, gasCap uint64) (hexutil.Uint64, error) {
-	// Binary search the gas limit, as it may need to be higher than the amount used
+	// Binary search the gas requirement, as it may be higher than the amount used
 	var (
-		lo uint64 // lowest-known gas limit where tx execution fails
-		hi uint64 // lowest-known gas limit where tx execution succeeds
+		lo  uint64 = params.TxGas - 1
+		hi  uint64
+		cap uint64
 	)
 	// Use zero address if sender unspecified.
 	if args.From == nil {
@@ -1216,17 +1198,16 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
 	} else {
 		feeCap = common.Big0
 	}
-
-	state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
-	if state == nil || err != nil {
-		return 0, err
-	}
-	if err := overrides.Apply(state); err != nil {
-		return 0, err
-	}
-
 	// Recap the highest gas limit with account's available balance.
 	if feeCap.BitLen() != 0 {
+		state, _, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
+		if err != nil {
+			return 0, err
+		}
+		err = overrides.Apply(state)
+		if err != nil {
+			return 0, err
+		}
 		balance := state.GetBalance(*args.From) // from can't be nil
 		available := new(big.Int).Set(balance)
 		if args.Value != nil {
@@ -1253,42 +1234,39 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
 		log.Warn("Caller gas above allowance, capping", "requested", hi, "cap", gasCap)
 		hi = gasCap
 	}
+	cap = hi
 
-	// We first execute the transaction at the highest allowable gas limit, since if this fails we
-	// can return error immediately.
-	failed, result, err := executeEstimate(ctx, b, args, state.Copy(), header, gasCap, hi)
+	// Create a helper to check if a gas allowance results in an executable transaction
+	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, state, header, nil, nil, 0, gasCap)
+		if err != nil {
+			if errors.Is(err, core.ErrIntrinsicGas) {
+				return true, nil, nil // Special case, raise gas limit
+			}
+			return true, nil, err // Bail out
+		}
+		return result.Failed(), result, nil
+	}
+	state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
+	if state == nil || err != nil {
+		return 0, err
+	}
+	err = overrides.Apply(state)
 	if err != nil {
 		return 0, err
 	}
-	if failed {
-		if result != nil && result.Err != vm.ErrOutOfGas {
-			if len(result.Revert()) > 0 {
-				return 0, newRevertError(result)
-			}
-			return 0, result.Err
-		}
-		return 0, fmt.Errorf("gas required exceeds allowance (%d)", hi)
-	}
-	// 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 txs that
-	// explicitly check gas remaining in order to successfully execute within a given limit, but we
-	// probably don't want to return a lowest possible gas limit for these cases anyway.
-	lo = result.UsedGas - 1
-
-	// Binary search for the smallest gas limit that allows the tx to execute successfully.
+	// Execute the binary search and hone in on an executable gas limit
 	for lo+1 < hi {
+		s := state.Copy()
 		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
-			// 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 = lo * 2
-		}
-		failed, _, err = executeEstimate(ctx, b, args, state.Copy(), header, gasCap, 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
+		// assigned. Return the error directly, don't struggle any more.
 		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, err
 		}
 		if failed {
@@ -1297,14 +1275,28 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr
 			hi = mid
 		}
 	}
+	// Reject the transaction as invalid if it still fails at the highest allowance
+	if hi == cap {
+		failed, result, err := executable(hi, state, header)
+		if err != nil {
+			return 0, err
+		}
+		if failed {
+			if result != nil && result.Err != vm.ErrOutOfGas {
+				if len(result.Revert()) > 0 {
+					return 0, newRevertError(result)
+				}
+				return 0, result.Err
+			}
+			// Otherwise, the specified gas cap is too low
+			return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap)
+		}
+	}
 	return hexutil.Uint64(hi), nil
 }
 
-// EstimateGas returns the lowest possible gas limit that allows the transaction to run
-// successfully at block `blockNrOrHash`, or the latest block if `blockNrOrHash` is unspecified. It
-// returns error if the transaction would revert or if there are unexpected failures. The returned
-// value is capped by both `args.Gas` (if non-nil & non-zero) and the backend's RPCGasCap
-// configuration (if non-zero).
+// EstimateGas returns an estimate of the amount of gas needed to execute the
+// given transaction against the current pending block.
 func (s *BlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, blockNrOrHash *rpc.BlockNumberOrHash, overrides *StateOverride) (hexutil.Uint64, error) {
 	bNrOrHash := rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber)
 	if blockNrOrHash != nil {