Fix estimate callGasLimit to account for value transfers (#204)

Author: hazim-j

pkg/entrypoint/execution/trace.go

@@ -9,6 +9,7 @@ import (
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/ethclient"
 	ethRpc "github.com/ethereum/go-ethereum/rpc"
 	"github.com/stackup-wallet/stackup-bundler/pkg/entrypoint"
@@ -19,6 +20,36 @@ import (
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
 
+func parseUserOperationEvent(
+	entryPoint common.Address,
+	ep *entrypoint.Entrypoint,
+	log *tracer.LogInfo,
+) (*entrypoint.EntrypointUserOperationEvent, error) {
+	if log == nil {
+		return nil, nil
+	}
+
+	topics := []common.Hash{}
+	for _, topic := range log.Topics {
+		topics = append(topics, common.HexToHash(topic))
+	}
+	data, err := hexutil.Decode(log.Data)
+	if err != nil {
+		return nil, err
+	}
+
+	ev, err := ep.ParseUserOperationEvent(types.Log{
+		Address: entryPoint,
+		Topics:  topics,
+		Data:    data,
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return ev, nil
+}
+
 func TraceSimulateHandleOp(
 	rpc *ethRpc.Client,
 	entryPoint common.Address,
@@ -27,20 +58,20 @@ func TraceSimulateHandleOp(
 	customTracer string,
 	target common.Address,
 	data []byte,
-) (*reverts.ExecutionResultRevert, error) {
+) (*reverts.ExecutionResultRevert, *entrypoint.EntrypointUserOperationEvent, error) {
 	ep, err := entrypoint.NewEntrypoint(entryPoint, ethclient.NewClient(rpc))
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	auth, err := bind.NewKeyedTransactorWithChainID(utils.DummyPk, chainID)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	auth.GasLimit = math.MaxUint64
 	auth.NoSend = true
 	tx, err := ep.SimulateHandleOp(auth, entrypoint.UserOperation(*op), target, data)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	var res tracer.BundlerErrorReturn
@@ -53,43 +84,48 @@ func TraceSimulateHandleOp(
 		Tracer: customTracer,
 	}
 	if err := rpc.CallContext(context.Background(), &res, "debug_traceCall", &req, "latest", &opts); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	outErr, err := errors.ParseHexToRpcDataError(res.Output)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	if res.ValidationOOG {
-		return nil, errors.NewRPCError(errors.EXECUTION_REVERTED, "validation OOG", nil)
+		return nil, nil, errors.NewRPCError(errors.EXECUTION_REVERTED, "validation OOG", nil)
 	}
 
 	sim, simErr := reverts.NewExecutionResult(outErr)
 	if simErr != nil {
 		fo, foErr := reverts.NewFailedOp(outErr)
 		if foErr != nil {
-			return nil, fmt.Errorf("%s, %s", simErr, foErr)
+			return nil, nil, fmt.Errorf("%s, %s", simErr, foErr)
 		}
-		return nil, errors.NewRPCError(errors.REJECTED_BY_EP_OR_ACCOUNT, fo.Reason, fo)
+		return nil, nil, errors.NewRPCError(errors.REJECTED_BY_EP_OR_ACCOUNT, fo.Reason, fo)
 	}
 
 	if len(res.Reverts) != 0 {
 		data, err := hexutil.Decode(res.Reverts[len(res.Reverts)-1])
 		if err != nil {
-			return sim, err
+			return sim, nil, err
 		}
 
 		if len(data) == 0 {
 			if res.ExecutionOOG {
-				return sim, errors.NewRPCError(errors.EXECUTION_REVERTED, "execution OOG", nil)
+				return sim, nil, errors.NewRPCError(errors.EXECUTION_REVERTED, "execution OOG", nil)
 			}
-			return sim, errors.NewRPCError(errors.EXECUTION_REVERTED, "execution reverted", nil)
+			return sim, nil, errors.NewRPCError(errors.EXECUTION_REVERTED, "execution reverted", nil)
 		}
 
 		reason, err := errors.DecodeRevert(data)
 		if err != nil {
-			return sim, err
+			return sim, nil, err
 		}
-		return sim, errors.NewRPCError(errors.EXECUTION_REVERTED, reason, reason)
+		return sim, nil, errors.NewRPCError(errors.EXECUTION_REVERTED, reason, reason)
+	}
+
+	ev, err := parseUserOperationEvent(entryPoint, ep, res.UserOperationEvent)
+	if err != nil {
+		return nil, nil, err
 	}
-	return sim, nil
+	return sim, ev, nil
 }

pkg/gas/estimate.go

@@ -8,7 +8,6 @@ import (
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/rpc"
 	"github.com/stackup-wallet/stackup-bundler/pkg/entrypoint/execution"
-	"github.com/stackup-wallet/stackup-bundler/pkg/entrypoint/reverts"
 	"github.com/stackup-wallet/stackup-bundler/pkg/errors"
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
@@ -25,38 +24,49 @@ func isExecutionOOG(err error) bool {
 	return strings.Contains(err.Error(), "execution OOG")
 }
 
-func runSimulations(rpc *rpc.Client,
+// EstimateGas uses the simulateHandleOp method on the EntryPoint to derive an estimate for
+// verificationGasLimit and callGasLimit.
+func EstimateGas(
+	rpc *rpc.Client,
 	from common.Address,
 	op *userop.UserOperation,
+	ov *Overhead,
 	chainID *big.Int,
-	tracer string) (*reverts.ExecutionResultRevert, error) {
+	tracer string,
+) (verificationGas uint64, callGas uint64, err error) {
+	// Skip if maxFeePerGas is zero.
+	if op.MaxFeePerGas.Cmp(big.NewInt(0)) != 1 {
+		return 0, 0, errors.NewRPCError(
+			errors.INVALID_FIELDS,
+			"maxFeePerGas must be more than 0",
+			nil,
+		)
+	}
+
+	// Set the initial conditions.
 	data, err := op.ToMap()
 	if err != nil {
-		return nil, err
+		return 0, 0, err
 	}
-
-	// Set MaxPriorityFeePerGas = MaxFeePerGas to simplify downstream calculations.
 	data["maxPriorityFeePerGas"] = hexutil.EncodeBig(op.MaxFeePerGas)
-
-	// Setting default values for gas limits.
 	data["verificationGasLimit"] = hexutil.EncodeBig(big.NewInt(0))
 	data["callGasLimit"] = hexutil.EncodeBig(big.NewInt(0))
 
-	// Maintain an outer reference to the latest simulation error.
-	var simErr error
-
-	// Find the maximal verificationGasLimit to simulate from.
+	// Find the optimal verificationGasLimit with binary search. Setting gas price to 0 and maxing out the gas
+	// limit here would result in certain code paths not being executed which results in an inaccurate gas
+	// estimate.
 	l := 0
 	r := MaxGasLimit
+	var simErr error
 	for l <= r {
 		m := (l + r) / 2
 
 		data["verificationGasLimit"] = hexutil.EncodeBig(big.NewInt(int64(m)))
 		simOp, err := userop.New(data)
 		if err != nil {
-			return nil, err
+			return 0, 0, err
 		}
-		sim, err := execution.TraceSimulateHandleOp(
+		sim, _, err := execution.TraceSimulateHandleOp(
 			rpc,
 			from,
 			simOp,
@@ -79,94 +89,72 @@ func runSimulations(rpc *rpc.Client,
 			}
 			// CGL is set to 0 and execution will always be OOG. Ignore it.
 			if !isExecutionOOG(err) {
-				return nil, err
+				return 0, 0, err
 			}
 		}
 
-		data["verificationGasLimit"] = hexutil.EncodeBig(sim.PreOpGas)
+		// Optimal VGL found.
+		data["verificationGasLimit"] = hexutil.EncodeBig(
+			big.NewInt(0).Sub(sim.PreOpGas, op.PreVerificationGas),
+		)
 		break
 	}
 	if simErr != nil && !isExecutionOOG(simErr) {
-		return nil, simErr
+		return 0, 0, simErr
 	}
 
-	// Find the maximal callGasLimit to simulate from.
-	l = 0
-	r = MaxGasLimit
-	var res *reverts.ExecutionResultRevert
-	for l <= r {
-		m := (l + r) / 2
-
-		data["callGasLimit"] = hexutil.EncodeBig(big.NewInt(int64(m)))
-		simOp, err := userop.New(data)
-		if err != nil {
-			return nil, err
-		}
-		sim, err := execution.TraceSimulateHandleOp(
-			rpc,
-			from,
-			simOp,
-			chainID,
-			tracer,
-			common.Address{},
-			[]byte{},
-		)
-		simErr = err
-		if err != nil {
-			if isPrefundNotPaid(err) {
-				// CGL too high, go lower.
-				r = m - 1
-				continue
-			}
-			if isExecutionOOG(err) {
-				// CGL too low, go higher.
-				l = m + 1
-				continue
-			}
-			return nil, err
-		}
-
-		res = sim
-		break
+	// Find the optimal callGasLimit by setting the gas price to 0 and maxing out the gas limit. We don't run
+	// into the same restrictions here as we do with verificationGasLimit.
+	data["maxFeePerGas"] = hexutil.EncodeBig(big.NewInt(0))
+	data["maxPriorityFeePerGas"] = hexutil.EncodeBig(big.NewInt(0))
+	data["callGasLimit"] = hexutil.EncodeBig(big.NewInt(int64(MaxGasLimit)))
+	simOp, err := userop.New(data)
+	if err != nil {
+		return 0, 0, err
 	}
-	if simErr != nil {
-		return nil, simErr
+	sim, ev, err := execution.TraceSimulateHandleOp(
+		rpc,
+		from,
+		simOp,
+		chainID,
+		tracer,
+		common.Address{},
+		[]byte{},
+	)
+	if err != nil {
+		return 0, 0, err
 	}
 
-	return res, nil
-}
-
-// EstimateGas uses the simulateHandleOp method on the EntryPoint to derive an estimate for
-// verificationGasLimit and callGasLimit.
-func EstimateGas(
-	rpc *rpc.Client,
-	from common.Address,
-	op *userop.UserOperation,
-	ov *Overhead,
-	chainID *big.Int,
-	tracer string,
-) (verificationGas uint64, callGas uint64, err error) {
-	// Skip if maxFeePerGas is zero.
-	if op.MaxFeePerGas.Cmp(big.NewInt(0)) != 1 {
-		return 0, 0, errors.NewRPCError(
-			errors.INVALID_FIELDS,
-			"maxFeePerGas must be more than 0",
-			nil,
-		)
+	// Calculate final values for verificationGasLimit and callGasLimit.
+	vgl := simOp.VerificationGasLimit
+	cgl := big.NewInt(0).
+		Add(big.NewInt(0).Sub(ev.ActualGasUsed, sim.PreOpGas), big.NewInt(int64(ov.intrinsicFixed)))
+	min := ov.NonZeroValueCall()
+	if min.Cmp(cgl) >= 1 {
+		cgl = min
 	}
 
-	// Estimate gas limits using a binary search approach.
-	sim, err := runSimulations(rpc, from, op, chainID, tracer)
+	// Run a final simulation to check wether or not value transfers are still okay when factoring in the
+	// expected gas cost.
+	data["maxFeePerGas"] = hexutil.EncodeBig(op.MaxFeePerGas)
+	data["maxPriorityFeePerGas"] = hexutil.EncodeBig(op.MaxFeePerGas)
+	data["verificationGasLimit"] = hexutil.EncodeBig(vgl)
+	data["callGasLimit"] = hexutil.EncodeBig(cgl)
+	simOp, err = userop.New(data)
 	if err != nil {
 		return 0, 0, err
 	}
-
-	// Return verificationGasLimit and callGasLimit.
-	tg := big.NewInt(0).Div(sim.Paid, op.MaxFeePerGas)
-	cgl := big.NewInt(0).Add(big.NewInt(0).Sub(tg, sim.PreOpGas), big.NewInt(int64(ov.intrinsicFixed)))
-	min := ov.NonZeroValueCall()
-	if cgl.Cmp(min) >= 1 {
-		return sim.PreOpGas.Uint64(), cgl.Uint64(), nil
+	_, _, err = execution.TraceSimulateHandleOp(
+		rpc,
+		from,
+		simOp,
+		chainID,
+		tracer,
+		common.Address{},
+		[]byte{},
+	)
+	if err != nil {
+		return 0, 0, err
 	}
-	return sim.PreOpGas.Uint64(), min.Uint64(), nil
+	return vgl.Uint64(), cgl.Uint64(), nil
 }