Default to sender balance override in gas estimation if no paymaster (#…

…350)

e2e/test/verification.test.ts

@@ -1,10 +1,18 @@
 import { ethers } from "ethers";
-import { Client, Presets } from "userop";
+import {
+  BundlerJsonRpcProvider,
+  Client,
+  Presets,
+  UserOperationBuilder,
+} from "userop";
 import { errorCodes } from "../src/errors";
 import { TestAccount } from "../src/testAccount";
 import config from "../config";
 
 describe("During the verification phase", () => {
+  const provider = new BundlerJsonRpcProvider(config.nodeUrl).setBundlerRpc(
+    config.bundlerUrl
+  );
   let client: Client;
   let acc: TestAccount;
   beforeAll(async () => {
@@ -59,49 +67,126 @@ describe("During the verification phase", () => {
     });
   });
 
-  describe("With state overrides", () => {
-    test("Sender with zero funds can successfully estimate UserOperation gas", async () => {
-      expect.assertions(4);
+  describe("With no gas fees", () => {
+    test("Sender with funds can estimate gas and send", async () => {
+      const signer = new ethers.Wallet(config.signingKey);
+      const fundedAcc = await Presets.Builder.SimpleAccount.init(
+        signer,
+        config.nodeUrl,
+        {
+          overrideBundlerRpc: config.bundlerUrl,
+        }
+      );
+      const op = await client.buildUserOperation(
+        fundedAcc.execute(
+          ethers.constants.AddressZero,
+          ethers.constants.Zero,
+          "0x"
+        )
+      );
+
+      const builderWithEstimate = new UserOperationBuilder()
+        .useDefaults({
+          ...op,
+          maxFeePerGas: 0,
+          maxPriorityFeePerGas: 0,
+        })
+        .useMiddleware(Presets.Middleware.estimateUserOperationGas(provider));
+      const opWithEstimate = await client.buildUserOperation(
+        builderWithEstimate
+      );
+      expect(ethers.BigNumber.from(opWithEstimate.maxFeePerGas).isZero()).toBe(
+        true
+      );
+      expect(
+        ethers.BigNumber.from(opWithEstimate.maxPriorityFeePerGas).isZero()
+      ).toBe(true);
+
+      const builderWithGasPrice = new UserOperationBuilder()
+        .useDefaults(opWithEstimate)
+        .useMiddleware(Presets.Middleware.getGasPrice(provider))
+        .useMiddleware(Presets.Middleware.signUserOpHash(signer));
+      const response = await client.sendUserOperation(builderWithGasPrice);
+      const event = await response.wait();
+      expect(event?.args.success).toBe(true);
+    });
+
+    test("Sender with zero funds can estimate gas but cannot send", async () => {
+      expect.assertions(3);
+      const signer = new ethers.Wallet(ethers.utils.randomBytes(32));
       const randAcc = await Presets.Builder.SimpleAccount.init(
-        new ethers.Wallet(ethers.utils.randomBytes(32)),
+        signer,
         config.nodeUrl,
         {
           overrideBundlerRpc: config.bundlerUrl,
         }
       );
-      randAcc
-        .setPreVerificationGas(0)
-        .setVerificationGasLimit(0)
-        .setCallGasLimit(0);
+      const op = await client.buildUserOperation(
+        randAcc.execute(
+          ethers.constants.AddressZero,
+          ethers.constants.Zero,
+          "0x"
+        )
+      );
+
+      const builderWithEstimate = new UserOperationBuilder()
+        .useDefaults({
+          ...op,
+          maxFeePerGas: 0,
+          maxPriorityFeePerGas: 0,
+        })
+        .useMiddleware(Presets.Middleware.estimateUserOperationGas(provider));
+      const opWithEstimate = await client.buildUserOperation(
+        builderWithEstimate
+      );
+      expect(ethers.BigNumber.from(opWithEstimate.maxFeePerGas).isZero()).toBe(
+        true
+      );
+      expect(
+        ethers.BigNumber.from(opWithEstimate.maxPriorityFeePerGas).isZero()
+      ).toBe(true);
 
       try {
-        await client.sendUserOperation(
-          randAcc.execute(randAcc.getSender(), ethers.constants.Zero, "0x")
-        );
+        const builderWithGasPrice = new UserOperationBuilder()
+          .useDefaults(opWithEstimate)
+          .useMiddleware(Presets.Middleware.getGasPrice(provider))
+          .useMiddleware(Presets.Middleware.signUserOpHash(signer));
+        await client.sendUserOperation(builderWithGasPrice);
       } catch (error: any) {
         expect(error?.error.code).toBe(errorCodes.rejectedByEpOrAccount);
       }
+    });
+  });
 
-      await client.sendUserOperation(
-        randAcc.execute(randAcc.getSender(), ethers.constants.Zero, "0x"),
+  describe("With state overrides", () => {
+    test("New sender will fail estimation if it uses its actual balance", async () => {
+      expect.assertions(1);
+      const randAcc = await Presets.Builder.SimpleAccount.init(
+        new ethers.Wallet(ethers.utils.randomBytes(32)),
+        config.nodeUrl,
         {
-          dryRun: true,
-          stateOverrides: {
-            [randAcc.getSender()]: {
-              balance: ethers.constants.MaxUint256.toHexString(),
-            },
-          },
-          onBuild(op) {
-            expect(ethers.BigNumber.from(op.preVerificationGas).gte(0)).toBe(
-              true
-            );
-            expect(ethers.BigNumber.from(op.verificationGasLimit).gte(0)).toBe(
-              true
-            );
-            expect(ethers.BigNumber.from(op.callGasLimit).gte(0)).toBe(true);
-          },
+          overrideBundlerRpc: config.bundlerUrl,
         }
       );
+
+      try {
+        await client.buildUserOperation(
+          randAcc.execute(
+            ethers.constants.AddressZero,
+            ethers.constants.Zero,
+            "0x"
+          ),
+          {
+            [randAcc.getSender()]: {
+              balance: ethers.utils.hexValue(
+                await provider.getBalance(randAcc.getSender())
+              ),
+            },
+          }
+        );
+      } catch (error: any) {
+        expect(error?.error.code).toBe(errorCodes.rejectedByEpOrAccount);
+      }
     });
   });
 });

internal/start/private.go

@@ -130,6 +130,7 @@ func PrivateMode() {
 	// Init Client
 	c := client.New(mem, ov, chain, conf.SupportedEntryPoints)
 	c.SetGetUserOpReceiptFunc(client.GetUserOpReceiptWithEthClient(eth))
+	c.SetGetGasPricesFunc(client.GetGasPricesWithEthClient(eth))
 	c.SetGetGasEstimateFunc(
 		client.GetGasEstimateWithEthClient(rpc, ov, chain, conf.MaxBatchGasLimit),
 	)

internal/start/searcher.go

@@ -122,6 +122,7 @@ func SearcherMode() {
 	// Init Client
 	c := client.New(mem, ov, chain, conf.SupportedEntryPoints)
 	c.SetGetUserOpReceiptFunc(client.GetUserOpReceiptWithEthClient(eth))
+	c.SetGetGasPricesFunc(client.GetGasPricesWithEthClient(eth))
 	c.SetGetGasEstimateFunc(
 		client.GetGasEstimateWithEthClient(rpc, ov, chain, conf.MaxBatchGasLimit),
 	)

pkg/client/client.go

@@ -28,6 +28,7 @@ type Client struct {
 	userOpHandler        modules.UserOpHandlerFunc
 	logger               logr.Logger
 	getUserOpReceipt     GetUserOpReceiptFunc
+	getGasPrices         GetGasPricesFunc
 	getGasEstimate       GetGasEstimateFunc
 	getUserOpByHash      GetUserOpByHashFunc
 }
@@ -48,6 +49,7 @@ func New(
 		userOpHandler:        noop.UserOpHandler,
 		logger:               logger.NewZeroLogr().WithName("client"),
 		getUserOpReceipt:     getUserOpReceiptNoop(),
+		getGasPrices:         getGasPricesNoop(),
 		getGasEstimate:       getGasEstimateNoop(),
 		getUserOpByHash:      getUserOpByHashNoop(),
 	}
@@ -79,6 +81,13 @@ func (i *Client) SetGetUserOpReceiptFunc(fn GetUserOpReceiptFunc) {
 	i.getUserOpReceipt = fn
 }
 
+// SetGetGasPricesFunc defines a general function for fetching values for maxFeePerGas and
+// maxPriorityFeePerGas. This function is called in *Client.EstimateUserOperationGas if given fee values are
+// 0.
+func (i *Client) SetGetGasPricesFunc(fn GetGasPricesFunc) {
+	i.getGasPrices = fn
+}
+
 // SetGetGasEstimateFunc defines a general function for fetching an estimate for verificationGasLimit and
 // callGasLimit given a userOp and EntryPoint address. This function is called in
 // *Client.EstimateUserOperationGas.
@@ -170,11 +179,30 @@ func (i *Client) EstimateUserOperationGas(
 	hash := userOp.GetUserOpHash(epAddr, i.chainID)
 	l = l.WithValues("userop_hash", hash)
 
+	// Parse state override set. If paymaster is not included and sender overrides are not set, default to
+	// overriding sender balance to max uint96. This ensures gas estimation is not blocked by insufficient
+	// funds.
 	sos, err := state.ParseOverrideData(os)
 	if err != nil {
 		l.Error(err, "eth_estimateUserOperationGas error")
 		return nil, err
 	}
+	if userOp.GetPaymaster() == common.HexToAddress("0x") {
+		sos = state.WithMaxBalanceOverride(userOp.Sender, sos)
+	}
+
+	// Override op with suggested gas prices if maxFeePerGas is 0. This allows for more reliable gas
+	// estimations upstream. The default balance override also ensures simulations won't revert on
+	// insufficient funds.
+	if userOp.MaxFeePerGas.Cmp(common.Big0) != 1 {
+		gp, err := i.getGasPrices()
+		if err != nil {
+			l.Error(err, "eth_estimateUserOperationGas error")
+			return nil, err
+		}
+		userOp.MaxFeePerGas = gp.MaxFeePerGas
+		userOp.MaxPriorityFeePerGas = gp.MaxPriorityFeePerGas
+	}
 
 	// Estimate gas limits
 	vg, cg, err := i.getGasEstimate(epAddr, userOp, sos)

pkg/client/utils.go

@@ -8,6 +8,7 @@ import (
 	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/ethereum/go-ethereum/rpc"
 	"github.com/stackup-wallet/stackup-bundler/pkg/entrypoint/filter"
+	"github.com/stackup-wallet/stackup-bundler/pkg/fees"
 	"github.com/stackup-wallet/stackup-bundler/pkg/gas"
 	"github.com/stackup-wallet/stackup-bundler/pkg/state"
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
@@ -32,6 +33,26 @@ func GetUserOpReceiptWithEthClient(eth *ethclient.Client) GetUserOpReceiptFunc {
 	}
 }
 
+// GetGasPricesFunc is a general interface for fetching values for maxFeePerGas and maxPriorityFeePerGas.
+type GetGasPricesFunc = func() (*fees.GasPrices, error)
+
+func getGasPricesNoop() GetGasPricesFunc {
+	return func() (*fees.GasPrices, error) {
+		return &fees.GasPrices{
+			MaxFeePerGas:         big.NewInt(0),
+			MaxPriorityFeePerGas: big.NewInt(0),
+		}, nil
+	}
+}
+
+// GetGasPricesWithEthClient returns an implementation of GetGasPricesFunc that relies on an eth client to
+// fetch values for maxFeePerGas and maxPriorityFeePerGas.
+func GetGasPricesWithEthClient(eth *ethclient.Client) GetGasPricesFunc {
+	return func() (*fees.GasPrices, error) {
+		return fees.NewGasPrices(eth)
+	}
+}
+
 // GetGasEstimateFunc is a general interface for fetching an estimate for verificationGasLimit and
 // callGasLimit given a userOp and EntryPoint address.
 type GetGasEstimateFunc = func(

pkg/entrypoint/execution/trace.go

@@ -100,7 +100,7 @@ func TraceSimulateHandleOp(in *TraceInput) (*TraceOutput, error) {
 	}
 	opts := utils.TraceCallOpts{
 		Tracer:         tracer.Loaded.BundlerExecutionTracer,
-		StateOverrides: state.WithZeroAddressOverride(in.Sos),
+		StateOverrides: state.WithMaxBalanceOverride(common.HexToAddress("0x"), in.Sos),
 	}
 	if err := in.Rpc.CallContext(context.Background(), &res, "debug_traceCall", &req, "latest", &opts); err != nil {
 		return nil, err

pkg/entrypoint/simulation/tracevalidation.go

@@ -63,7 +63,7 @@ func TraceSimulateValidation(in *TraceInput) (*TraceOutput, error) {
 	}
 	opts := utils.TraceCallOpts{
 		Tracer:         tracer.Loaded.BundlerCollectorTracer,
-		StateOverrides: state.WithZeroAddressOverride(nil),
+		StateOverrides: state.WithMaxBalanceOverride(common.HexToAddress("0x"), nil),
 	}
 	if err := in.Rpc.CallContext(context.Background(), &res, "debug_traceCall", &req, "latest", &opts); err != nil {
 		return nil, err

pkg/fees/gasprices.go

@@ -0,0 +1,39 @@
+package fees
+
+import (
+	"context"
+	"math/big"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/ethclient"
+)
+
+// GasPrices contains recommended gas fees for a UserOperation to be included in a timely manner.
+type GasPrices struct {
+	MaxFeePerGas         *big.Int
+	MaxPriorityFeePerGas *big.Int
+}
+
+// NewGasPrices returns an instance of GasPrices with the latest suggested fees derived from an Eth Client.
+func NewGasPrices(eth *ethclient.Client) (*GasPrices, error) {
+	gp := GasPrices{}
+	if head, err := eth.HeaderByNumber(context.Background(), nil); err != nil {
+		return nil, err
+	} else if head.BaseFee != nil {
+		tip, err := eth.SuggestGasTipCap(context.Background())
+		if err != nil {
+			return nil, err
+		}
+		gp.MaxFeePerGas = big.NewInt(0).Add(tip, big.NewInt(0).Mul(head.BaseFee, common.Big2))
+		gp.MaxPriorityFeePerGas = tip
+	} else {
+		sgp, err := eth.SuggestGasPrice(context.Background())
+		if err != nil {
+			return nil, err
+		}
+		gp.MaxFeePerGas = sgp
+		gp.MaxPriorityFeePerGas = sgp
+	}
+
+	return &gp, 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/errors"
 	"github.com/stackup-wallet/stackup-bundler/pkg/state"
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
@@ -79,15 +78,6 @@ func retryEstimateGas(err error, vgl int64, in *EstimateInput) (uint64, uint64,
 // EstimateGas uses the simulateHandleOp method on the EntryPoint to derive an estimate for
 // verificationGasLimit and callGasLimit.
 func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err error) {
-	// Skip if maxFeePerGas is zero.
-	if in.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 := in.Op.ToMap()
 	if err != nil {
@@ -97,9 +87,9 @@ func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err
 	data["verificationGasLimit"] = hexutil.EncodeBig(big.NewInt(0))
 	data["callGasLimit"] = hexutil.EncodeBig(big.NewInt(0))
 
-	// 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.
+	// Find the optimal verificationGasLimit with binary search. A gas price of 0 may result in certain
+	// upstream code paths in the EVM to not be executed which can affect the reliability of gas estimates. In
+	// this case, consider calling the EstimateGas function after setting the gas price on the UserOperation.
 	l := int64(0)
 	r := in.MaxGasLimit.Int64()
 	f := in.lastVGL