evm: rewrite eth_estimateGas for panic safety and performance

.github/workflows/changelog-check.yml

@@ -1,4 +1,4 @@
-name: "Check changelog update"
+name: "Changelog"
 on:
   pull_request:
     # The specific activity types are listed here to include "labeled" and "unlabeled"

x/evm/const.go

@@ -36,6 +36,13 @@ const (
 	Erc20GasLimitExecute uint64 = 200_000
 )
 
+type contextKey string
+
+const (
+	SimulationCtxKey               contextKey = "evm_simulation"
+	GasEstimateZeroToleranceCtxKey contextKey = "gas_estimate_zero_tolerance"
+)
+
 // BASE_FEE_MICRONIBI is the global base fee value for the network. It has a
 // constant value of 1 unibi (micronibi) == 10^12 wei.
 var (
@@ -123,15 +130,6 @@ const (
 	updateParamsName = "evm/MsgUpdateParams"
 )
 
-type CallType int
-
-const (
-	// CallTypeRPC call type is used on requests to eth_estimateGas rpc API endpoint
-	CallTypeRPC CallType = iota + 1
-	// CallTypeSmart call type is used in case of smart contract methods calls
-	CallTypeSmart
-)
-
 var (
 	EVM_MODULE_ADDRESS        gethcommon.Address
 	EVM_MODULE_ADDRESS_NIBI   sdk.AccAddress

x/evm/evm.go

@@ -114,36 +114,24 @@ func ValidateFunTokenBankMetadata(
 	return out, nil
 }
 
-// HandleOutOfGasPanic captures an sdk.ErrorOutOfGas panic and folds it into
-// *errp, an error pointer.
-// - If *errp is nil: sets *errp = vm.ErrOutOfGas
-// - If *errp is non-nil: preserves it (do not overwrite)
-// - Always applies `format` wrapping if *errp is non-nil after recovery
-// - Re-panics for any non-OutOfGas panic
-func HandleOutOfGasPanic(errp *error, format string) func() {
-	return func() {
-		if perr := recover(); perr != nil {
-			_, isOutOfGasPanic := perr.(sdk.ErrorOutOfGas)
-			switch {
-			case isOutOfGasPanic:
-				if errp != nil && *errp == nil {
-					*errp = vm.ErrOutOfGas
-				}
-				// else: preserve existing detailed error
-			case strings.Contains(fmt.Sprint(perr), vm.ErrOutOfGas.Error()):
-				if errp == nil {
-					errp = new(error)
-				}
-				*errp = fmt.Errorf("%s: %w", perr, vm.ErrOutOfGas)
-			default:
-				// Non-OOG panics are not handled here
-				panic(perr)
-			}
+// RecoverOutOfGasPanic captures an "out of gas" panic and returns an error
+// instead, adding some safety. If the recovered panic is not gas related, we
+// propagate the error info.
+//
+// Rationale: In "eth_estimateGas", OOG is a VM-level execution failure and should
+// not abort the search; unexpected panics should.
+func RecoverOutOfGasPanic(context string) (isOog bool, perr error) {
+	if panicInfo := recover(); panicInfo != nil {
+		if _, isOutOfGasPanic := panicInfo.(sdk.ErrorOutOfGas); isOutOfGasPanic {
+			return true, vm.ErrOutOfGas
 		}
-		if errp != nil && *errp != nil && format != "" {
-			*errp = fmt.Errorf("%s: %w", format, *errp)
+		if strings.Contains(fmt.Sprint(panicInfo), "out of gas") {
+			return true, vm.ErrOutOfGas
 		}
+		// Non-OOG panics are not handled here
+		return false, fmt.Errorf("unexpected panic in %s: %v", context, panicInfo)
 	}
+	return false, nil
 }
 
 // Gracefully handles "out of gas"

x/evm/evmstate/grpc_query.go

@@ -264,7 +264,7 @@ func (k *Keeper) EthCall(
 	}
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
-	ctx = ctx.WithValue(SimulationContextKey, true)
+	ctx = ctx.WithValue(evm.SimulationCtxKey, true)
 
 	var args evm.JsonTxArgs
 	err := json.Unmarshal(req.Args, &args)
@@ -295,15 +295,9 @@ func (k *Keeper) EthCall(
 }
 
 // EstimateGas: Implements the gRPC query for "/eth.evm.v1.Query/EstimateGas".
-// EstimateGas implements eth_estimateGas rpc api.
-func (k Keeper) EstimateGas(
-	goCtx context.Context, req *evm.EthCallRequest,
-) (*evm.EstimateGasResponse, error) {
-	return k.EstimateGasForEvmCallType(goCtx, req, evm.CallTypeRPC)
-}
-
-// EstimateGasForEvmCallType estimates the gas cost of a transaction. This can be
-// called with the "eth_estimateGas" JSON-RPC method or smart contract query.
+// This estimates the lowest possible gas limit that allows a transaction to run
+// successfully with the provided context options. This can be called with the
+// "eth_estimateGas" JSON-RPC method.
 //
 // When [EstimateGas] is called from the JSON-RPC client, we need to reset the
 // gas meter before simulating the transaction (tx) to have an accurate gas
@@ -316,16 +310,16 @@ func (k Keeper) EstimateGas(
 // Returns:
 //   - A response containing the estimated gas cost.
 //   - An error if the gas estimation process encounters any issues.
-func (k Keeper) EstimateGasForEvmCallType(
-	goCtx context.Context, req *evm.EthCallRequest, fromType evm.CallType,
+func (k Keeper) EstimateGas(
+	goCtx context.Context, req *evm.EthCallRequest,
 ) (*evm.EstimateGasResponse, error) {
 	if err := req.Validate(); err != nil {
 		return nil, err
 	}
 
-	ctx := sdk.UnwrapSDKContext(goCtx)
-	ctx = ctx.WithValue(SimulationContextKey, true)
-	evmCfg := k.GetEVMConfig(ctx)
+	rootCtx := sdk.UnwrapSDKContext(goCtx).
+		WithValue(evm.SimulationCtxKey, true)
+	evmCfg := k.GetEVMConfig(rootCtx)
 
 	if req.GasCap < gethparams.TxGas {
 		return nil, grpcstatus.Errorf(grpccodes.InvalidArgument, "gas cap cannot be lower than %d", gethparams.TxGas)
@@ -338,54 +332,74 @@ func (k Keeper) EstimateGasForEvmCallType(
 	}
 
 	// ApplyMessageWithConfig expect correct nonce set in msg
-	nonce := k.GetAccNonce(ctx, args.GetFrom())
+	nonce := k.GetAccNonce(rootCtx, args.GetFrom())
 	args.Nonce = (*hexutil.Uint64)(&nonce)
 
 	// Binary search the gas requirement, as it may be higher than the amount used
 	var (
-		lo     = gethparams.TxGas - 1
-		hi     uint64
-		gasCap uint64
+		// Set smart lower bound based on the gas used in the first execution
+		// (base case).
+		lo uint64
+		hi uint64
+
+		// executable runs one probe at a specific gas limit.
+		//   - Rewrites evmMsg.GasLimit to the probed value.
+		//   - Constructs a fresh SDB on a context with an infinite gas meter and zero
+		//     KV/transient KV gas costs, isolating the probe from store-gas panics.
+		//   - Defers a panic classifier, where SDK/go-ethereum "out of gas"
+		//     panics result in  { vmError=true, err=nil }. Any other panic gets
+		//     bubbled up through the call stack.
+		//   - Returns (vmError, resp, err) where vmError signals VM-level failure
+		//     (incl. OOG/revert), and err signals consensus/unexpected failure.
+		executable func(gas uint64) (vmError bool, rsp *evm.MsgEthereumTxResponse, err error)
 	)
 
 	// Determine the highest gas limit can be used during the estimation.
+	// Start with block gas limit
+	params := rootCtx.ConsensusParams()
+	if params != nil && params.Block != nil && params.Block.MaxGas > 0 {
+		hi = uint64(params.Block.MaxGas)
+	} else {
+		// Fallback to gasCap if block params not available
+		hi = req.GasCap
+	}
+
+	// Override with user-provided gas limit if it's valid
 	if args.Gas != nil && uint64(*args.Gas) >= gethparams.TxGas {
 		hi = uint64(*args.Gas)
-	} else {
-		// Query block gas limit
-		params := ctx.ConsensusParams()
-		if params != nil && params.Block != nil && params.Block.MaxGas > 0 {
-			hi = uint64(params.Block.MaxGas)
-		} else {
-			hi = req.GasCap
-		}
 	}
 
-	// TODO: Recap the highest gas limit with account's available balance.
 	// Recap the highest gas allowance with specified gascap.
 	if req.GasCap != 0 && hi > req.GasCap {
 		hi = req.GasCap
 	}
 
-	gasCap = hi
-
 	// convert the tx args to an ethereum message
 	evmMsg, err := args.ToMessage(req.GasCap, evmCfg.BaseFeeWei)
 	if err != nil {
 		return nil, grpcstatus.Error(grpccodes.Internal, err.Error())
 	}
 
-	// NOTE: the errors from the executable below should be consistent with
-	// go-ethereum, so we don't wrap them with the gRPC status code Create a
-	// helper to check if a gas allowance results in an executable transaction.
-	executable := func(gas uint64) (vmError bool, rsp *evm.MsgEthereumTxResponse, err error) {
-		// update the message with the new gas value
-		evmMsg = core.Message{
+	executable = func(gas uint64) (vmError bool, rsp *evm.MsgEthereumTxResponse, err error) {
+		defer func() {
+			// Recover OOG panics as a normal VM failure so the binary search can
+			// increase gas. Any non-OOG panic aborts the search with a
+			// contextual error for diagnostics.
+			oog, perr := evm.RecoverOutOfGasPanic(fmt.Sprintf(`eth_estimateGas { gas: %d }`, gas))
+			if oog {
+				vmError, rsp, err = true, nil, nil
+				return
+			} else if perr != nil {
+				err = perr // Unexpected panic -> Abort the search
+				return
+			}
+		}()
+		evmMsg = core.Message{ // update the message with the new gas value
 			To:               evmMsg.To,
 			From:             evmMsg.From,
 			Nonce:            evmMsg.Nonce,
 			Value:            evmMsg.Value,
-			GasLimit:         gas, // <---- This one changed
+			GasLimit:         gas, // <---- This one changes
 			GasPrice:         evmMsg.GasPrice,
 			GasFeeCap:        evmMsg.GasFeeCap,
 			GasTipCap:        evmMsg.GasTipCap,
@@ -397,37 +411,35 @@ func (k Keeper) EstimateGasForEvmCallType(
 			SkipFromEOACheck: evmMsg.SkipFromEOACheck,
 		}
 
-		tmpCtx := ctx
-		if fromType == evm.CallTypeRPC {
-			tmpCtx, _ = ctx.CacheContext()
+		// Initialize SDB
+		sdb := k.NewSDB(
+			rootCtx,
+			k.TxConfig(rootCtx, rootCtx.EvmTxHash()),
+		)
+		sdb.SetCtx(
+			sdb.Ctx().
+				WithGasMeter(eth.NewInfiniteGasMeterWithLimit(evmMsg.GasLimit)).
+				WithKVGasConfig(storetypes.GasConfig{}).
+				WithTransientKVGasConfig(storetypes.GasConfig{}),
+		)
 
-			acct := k.GetAccount(tmpCtx, evmMsg.From)
+		acct := k.GetAccount(sdb.Ctx(), evmMsg.From)
 
-			from := evmMsg.From
-			if acct == nil {
-				acc := k.accountKeeper.NewAccountWithAddress(tmpCtx, from[:])
-				k.accountKeeper.SetAccount(tmpCtx, acc)
-				acct = NewEmptyAccount()
-			}
-			// When submitting a transaction, the `EthIncrementSenderSequence` ante handler increases the account nonce
-			acct.Nonce = nonce + 1
-			err = k.SetAccount(tmpCtx, from, *acct)
-			if err != nil {
-				return true, nil, err
-			}
-			// resetting the gasMeter after increasing the sequence to have an accurate gas estimation on EVM extensions transactions
-			gasMeter := eth.NewInfiniteGasMeterWithLimit(evmMsg.GasLimit)
-			tmpCtx = tmpCtx.WithGasMeter(gasMeter).
-				WithKVGasConfig(storetypes.GasConfig{}).
-				WithTransientKVGasConfig(storetypes.GasConfig{})
+		from := evmMsg.From
+		if acct == nil {
+			acc := k.accountKeeper.NewAccountWithAddress(sdb.Ctx(), from[:])
+			k.accountKeeper.SetAccount(sdb.Ctx(), acc)
+			acct = NewEmptyAccount()
+		}
+		// When submitting a transaction, the `EthIncrementSenderSequence` ante handler increases the account nonce
+		acct.Nonce = nonce + 1
+		err = k.SetAccount(sdb.Ctx(), from, *acct)
+		if err != nil {
+			return true, nil, err
 		}
+
 		// pass false to not commit StateDB
-		sdb := NewSDB(
-			ctx,
-			&k,
-			NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash().Bytes())),
-		)
-		evmObj := k.NewEVM(tmpCtx, evmMsg, evmCfg, nil /*tracer*/, sdb)
+		evmObj := k.NewEVM(sdb.Ctx(), evmMsg, evmCfg, nil /*tracer*/, sdb)
 		rsp, err = k.ApplyEvmMsg(evmMsg, evmObj, false /*commit*/)
 		if err != nil {
 			if strings.Contains(err.Error(), core.ErrIntrinsicGas.Error()) {
@@ -438,31 +450,35 @@ func (k Keeper) EstimateGasForEvmCallType(
 		return len(rsp.VmError) > 0, rsp, nil
 	}
 
-	// Execute the binary search and hone in on an executable gas limit
-	hi, err = evm.BinSearch(lo, hi, executable)
+	// BASE CASE:  Jumping straight into binary search is extermely inefficient.
+	// Instead, execute at the highest allowable gas limit first to validate and
+	// set a smarter lower bound.
+	failed, result, err := executable(hi)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("eth call exec error: %w", err)
 	}
-
-	// The gas limit is now the highest gas limit that results in an executable transaction
-	// Reject the transaction as invalid if it still fails at the highest allowance
-	if hi == gasCap {
-		failed, result, err := executable(hi)
-		if err != nil {
-			return nil, fmt.Errorf("eth call exec error: %w", err)
-		}
-
-		if failed && result != nil {
+	// If the base case fails for non-gas reasons, return the error immediately
+	if failed {
+		if result != nil && result.VmError != "" && result.VmError != vm.ErrOutOfGas.Error() {
 			if result.VmError == vm.ErrExecutionReverted.Error() {
 				return nil, fmt.Errorf("estimate gas VMError: %w", evm.NewRevertError(result.Ret))
 			}
-
-			if result.VmError == vm.ErrOutOfGas.Error() {
-				return nil, fmt.Errorf("gas required gas limit (%d)", gasCap)
-			}
-
 			return nil, fmt.Errorf("estimate gas VMError: %s", result.VmError)
 		}
+		return nil, fmt.Errorf("gas required exceeds allowance (%d)", hi)
+	}
+
+	// Set smart lower bound based on actual gas used
+	lo = result.GasUsed - 1
+
+	// Execute the binary search and hone in on an executable gas limit
+	estimateTolerance := evm.GasEstimateErrorRatioTolerance
+	if rootCtx.Value(evm.GasEstimateZeroToleranceCtxKey) == true {
+		estimateTolerance = 0.00
+	}
+	hi, err = evm.BinSearch(lo, hi, executable, estimateTolerance)
+	if err != nil {
+		return nil, err
 	}
 
 	return &evm.EstimateGasResponse{Gas: hi}, nil
@@ -483,7 +499,7 @@ func (k Keeper) TraceTx(
 	contextHeight := max(req.BlockNumber, 1)
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
-	ctx = ctx.WithValue(SimulationContextKey, true)
+	ctx = ctx.WithValue(evm.SimulationCtxKey, true)
 	ctx = ctx.WithBlockHeight(contextHeight)
 	ctx = ctx.WithBlockTime(req.BlockTime)
 	ctx = ctx.WithHeaderHash(gethcommon.Hex2Bytes(req.BlockHash))
@@ -580,7 +596,7 @@ func (k Keeper) TraceCall(
 	contextHeight := max(req.BlockNumber, 1)
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
-	ctx = ctx.WithValue(SimulationContextKey, true)
+	ctx = ctx.WithValue(evm.SimulationCtxKey, true)
 	ctx = ctx.WithBlockHeight(contextHeight)
 	ctx = ctx.WithBlockTime(req.BlockTime)
 	ctx = ctx.WithHeaderHash(gethcommon.Hex2Bytes(req.BlockHash))
@@ -670,7 +686,7 @@ func (k Keeper) TraceBlock(
 		WithConsensusParams(&cmtproto.ConsensusParams{
 			Block: &cmtproto.BlockParams{MaxGas: req.BlockMaxGas},
 		})
-	ctx = ctx.WithValue(SimulationContextKey, true)
+	ctx = ctx.WithValue(evm.SimulationCtxKey, true)
 
 	evmCfg := k.GetEVMConfig(ctx)
 

x/evm/evmstate/keeper.go

@@ -24,10 +24,6 @@ import (
 	"github.com/NibiruChain/nibiru/v2/x/evm"
 )
 
-type contextKey string
-
-const SimulationContextKey contextKey = "evm_simulation"
-
 type Keeper struct {
 	cdc codec.BinaryCodec
 	// storeKey: For persistent storage of EVM state.
@@ -132,7 +128,7 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 
 // IsSimulation checks if the context is a simulation context.
 func IsSimulation(ctx sdk.Context) bool {
-	if val := ctx.Value(SimulationContextKey); val != nil {
+	if val := ctx.Value(evm.SimulationCtxKey); val != nil {
 		if simulation, ok := val.(bool); ok && simulation {
 			return true
 		}