Fix VGL estimates that have a dependency on execution (#343)

Author: hazim-j

e2e/config.ts

@@ -16,8 +16,8 @@ const config: IConfig = {
 
   // https://github.com/stackup-wallet/contracts/blob/main/contracts/test
   testERC20Token: "0x3870419Ba2BBf0127060bCB37f69A1b1C090992B",
-  testGas: "0xc2e76Ee793a194Dd930C18c4cDeC93E7C75d567C",
-  testAccount: "0x3dFD39F2c17625b301ae0EF72B411D1de5211325",
+  testGas: "0x450d8479B0ceF1e6933DED809e12845aF413A50D",
+  testAccount: "0x6D7d359cE9e60dDa36EE712cE9B5947B4C72F862",
 };
 
 export default config;

e2e/src/abi.ts

@@ -177,12 +177,19 @@ export const testGasABI = [
     type: "function",
   },
   {
-    inputs: [{ internalType: "uint256", name: "", type: "uint256" }],
+    inputs: [{ internalType: "uint256", name: "key", type: "uint256" }],
     name: "store",
-    outputs: [{ internalType: "uint256", name: "", type: "uint256" }],
+    outputs: [{ internalType: "uint256", name: "value", type: "uint256" }],
     stateMutability: "view",
     type: "function",
   },
+  {
+    inputs: [{ internalType: "uint256", name: "times", type: "uint256" }],
+    name: "wasteGas",
+    outputs: [],
+    stateMutability: "nonpayable",
+    type: "function",
+  },
 ];
 
 export const testAccountABI = [
@@ -241,15 +248,15 @@ export const testAccountABI = [
     inputs: [
       {
         internalType: "uint256",
-        name: "",
+        name: "key",
         type: "uint256",
       },
     ],
     name: "store",
     outputs: [
       {
         internalType: "uint256",
-        name: "",
+        name: "value",
         type: "uint256",
       },
     ],
@@ -342,6 +349,19 @@ export const testAccountABI = [
     stateMutability: "nonpayable",
     type: "function",
   },
+  {
+    inputs: [
+      {
+        internalType: "uint256",
+        name: "times",
+        type: "uint256",
+      },
+    ],
+    name: "wasteGas",
+    outputs: [],
+    stateMutability: "nonpayable",
+    type: "function",
+  },
   {
     stateMutability: "payable",
     type: "receive",

e2e/src/testAccount.ts

@@ -11,6 +11,7 @@ import { EntryPoint, EntryPoint__factory } from "userop/dist/typechain";
 import { testAccountABI } from "./abi";
 
 const RECURSIVE_CALL_MODE = "0x0001";
+const FORCE_VALIDATION_OOG_MODE = "0x0002";
 
 export class TestAccount extends UserOperationBuilder {
   private provider: ethers.providers.JsonRpcProvider;
@@ -64,4 +65,10 @@ export class TestAccount extends UserOperationBuilder {
       )
     ).setSignature(RECURSIVE_CALL_MODE);
   }
+
+  forceValidationOOG(wasteGasMultiplier: number) {
+    return this.setCallData(
+      ethers.utils.defaultAbiCoder.encode(["uint256"], [wasteGasMultiplier])
+    ).setSignature(FORCE_VALIDATION_OOG_MODE);
+  }
 }

e2e/test/verification.test.ts

@@ -30,4 +30,17 @@ describe("During the verification phase", () => {
       });
     });
   });
+
+  describe("With dependency on callGasLimit", () => {
+    [0, 1, 2, 3, 4, 5].forEach((times) => {
+      test(`Sender can run validation with non-simulated code that uses ${times} storage writes`, async () => {
+        const response = await client.sendUserOperation(
+          acc.forceValidationOOG(times)
+        );
+        const event = await response.wait();
+
+        expect(event?.args.success).toBe(true);
+      });
+    });
+  });
 });

internal/config/values.go

@@ -21,7 +21,6 @@ type Values struct {
 	SupportedEntryPoints    []common.Address
 	MaxVerificationGas      *big.Int
 	MaxBatchGasLimit        *big.Int
-	PMGasEstBuffer          int64
 	MaxOpTTL                time.Duration
 	MaxOpsForUnstakedSender int
 	Beneficiary             string
@@ -87,7 +86,6 @@ func GetValues() *Values {
 	viper.SetDefault("erc4337_bundler_supported_entry_points", "0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789")
 	viper.SetDefault("erc4337_bundler_max_verification_gas", 3000000)
 	viper.SetDefault("erc4337_bundler_max_batch_gas_limit", 25000000)
-	viper.SetDefault("erc4337_bundler_paymaster_gas_estimate_buffer", 10)
 	viper.SetDefault("erc4337_bundler_max_op_ttl_seconds", 180)
 	viper.SetDefault("erc4337_bundler_max_ops_for_unstaked_sender", 4)
 	viper.SetDefault("erc4337_bundler_blocks_in_the_future", 25)
@@ -117,7 +115,6 @@ func GetValues() *Values {
 	_ = viper.BindEnv("erc4337_bundler_beneficiary")
 	_ = viper.BindEnv("erc4337_bundler_max_verification_gas")
 	_ = viper.BindEnv("erc4337_bundler_max_batch_gas_limit")
-	_ = viper.BindEnv("erc4337_bundler_paymaster_gas_estimate_buffer")
 	_ = viper.BindEnv("erc4337_bundler_max_op_ttl_seconds")
 	_ = viper.BindEnv("erc4337_bundler_max_ops_for_unstaked_sender")
 	_ = viper.BindEnv("erc4337_bundler_eth_builder_url")
@@ -176,7 +173,6 @@ func GetValues() *Values {
 	beneficiary := viper.GetString("erc4337_bundler_beneficiary")
 	maxVerificationGas := big.NewInt(int64(viper.GetInt("erc4337_bundler_max_verification_gas")))
 	maxBatchGasLimit := big.NewInt(int64(viper.GetInt("erc4337_bundler_max_batch_gas_limit")))
-	pmGasEstBuffer := int64(viper.GetInt("erc4337_bundler_paymaster_gas_estimate_buffer"))
 	maxOpTTL := time.Second * viper.GetDuration("erc4337_bundler_max_op_ttl_seconds")
 	maxOpsForUnstakedSender := viper.GetInt("erc4337_bundler_max_ops_for_unstaked_sender")
 	ethBuilderUrl := viper.GetString("erc4337_bundler_eth_builder_url")
@@ -198,7 +194,6 @@ func GetValues() *Values {
 		Beneficiary:             beneficiary,
 		MaxVerificationGas:      maxVerificationGas,
 		MaxBatchGasLimit:        maxBatchGasLimit,
-		PMGasEstBuffer:          pmGasEstBuffer,
 		MaxOpTTL:                maxOpTTL,
 		MaxOpsForUnstakedSender: maxOpsForUnstakedSender,
 		EthBuilderUrl:           ethBuilderUrl,

internal/start/private.go

@@ -131,7 +131,7 @@ func PrivateMode() {
 	c := client.New(mem, ov, chain, conf.SupportedEntryPoints)
 	c.SetGetUserOpReceiptFunc(client.GetUserOpReceiptWithEthClient(eth))
 	c.SetGetGasEstimateFunc(
-		client.GetGasEstimateWithEthClient(rpc, ov, chain, conf.MaxBatchGasLimit, conf.PMGasEstBuffer),
+		client.GetGasEstimateWithEthClient(rpc, ov, chain, conf.MaxBatchGasLimit),
 	)
 	c.SetGetUserOpByHashFunc(client.GetUserOpByHashWithEthClient(eth))
 	c.UseLogger(logr)

internal/start/searcher.go

@@ -123,7 +123,7 @@ func SearcherMode() {
 	c := client.New(mem, ov, chain, conf.SupportedEntryPoints)
 	c.SetGetUserOpReceiptFunc(client.GetUserOpReceiptWithEthClient(eth))
 	c.SetGetGasEstimateFunc(
-		client.GetGasEstimateWithEthClient(rpc, ov, chain, conf.MaxBatchGasLimit, conf.PMGasEstBuffer),
+		client.GetGasEstimateWithEthClient(rpc, ov, chain, conf.MaxBatchGasLimit),
 	)
 	c.SetGetUserOpByHashFunc(client.GetUserOpByHashWithEthClient(eth))
 	c.UseLogger(logr)

pkg/client/utils.go

@@ -49,17 +49,15 @@ func GetGasEstimateWithEthClient(
 	ov *gas.Overhead,
 	chain *big.Int,
 	maxGasLimit *big.Int,
-	paymasterBuffer int64,
 ) GetGasEstimateFunc {
 	return func(ep common.Address, op *userop.UserOperation) (verificationGas uint64, callGas uint64, err error) {
 		return gas.EstimateGas(&gas.EstimateInput{
-			Rpc:             rpc,
-			EntryPoint:      ep,
-			Op:              op,
-			Ov:              ov,
-			ChainID:         chain,
-			MaxGasLimit:     maxGasLimit,
-			PaymasterBuffer: paymasterBuffer,
+			Rpc:         rpc,
+			EntryPoint:  ep,
+			Op:          op,
+			Ov:          ov,
+			ChainID:     chain,
+			MaxGasLimit: maxGasLimit,
 		})
 	}
 }

pkg/gas/estimate.go

@@ -14,6 +14,8 @@ import (
 
 var (
 	fallBackBinarySearchCutoff = int64(30000)
+	maxRetries                 = int64(7)
+	baseVGLBuffer              = int64(25)
 )
 
 func isPrefundNotPaid(err error) bool {
@@ -41,13 +43,34 @@ func isExecutionReverted(err error) bool {
 }
 
 type EstimateInput struct {
-	Rpc             *rpc.Client
-	EntryPoint      common.Address
-	Op              *userop.UserOperation
-	Ov              *Overhead
-	ChainID         *big.Int
-	MaxGasLimit     *big.Int
-	PaymasterBuffer int64
+	Rpc         *rpc.Client
+	EntryPoint  common.Address
+	Op          *userop.UserOperation
+	Ov          *Overhead
+	ChainID     *big.Int
+	MaxGasLimit *big.Int
+
+	attempts int64
+	lastVGL  int64
+}
+
+// retryEstimateGas will recursively call estimateGas if execution has caused VGL to be under estimated. This
+// can occur for edge cases where a paymaster's postOp > gas required during verification or if verification
+// has a dependency on CGL. Reset the estimate with a higher buffer on VGL.
+func retryEstimateGas(err error, vgl int64, in *EstimateInput) (uint64, uint64, error) {
+	if isValidationOOG(err) && in.attempts < maxRetries {
+		return EstimateGas(&EstimateInput{
+			Rpc:         in.Rpc,
+			EntryPoint:  in.EntryPoint,
+			Op:          in.Op,
+			Ov:          in.Ov,
+			ChainID:     in.ChainID,
+			MaxGasLimit: in.MaxGasLimit,
+			attempts:    in.attempts + 1,
+			lastVGL:     vgl,
+		})
+	}
+	return 0, 0, err
 }
 
 // EstimateGas uses the simulateHandleOp method on the EntryPoint to derive an estimate for
@@ -76,9 +99,9 @@ func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err
 	// estimate.
 	l := int64(0)
 	r := in.MaxGasLimit.Int64()
-	f := int64(0)
+	f := in.lastVGL
 	var simErr error
-	for r-l >= fallBackBinarySearchCutoff {
+	for in.lastVGL == 0 && r-l >= fallBackBinarySearchCutoff {
 		m := (l + r) / 2
 
 		data["verificationGasLimit"] = hexutil.EncodeBig(big.NewInt(int64(m)))
@@ -113,12 +136,7 @@ func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err
 	if f == 0 {
 		return 0, 0, simErr
 	}
-	// TODO: Find a more reliable approach for the edge case where the gas required during a paymaster's
-	// postOp > gas required during verification. As a workaround we add a configurable percentage buffer if a
-	// paymaster is included.
-	if in.Op.GetPaymaster() != common.HexToAddress("0x") {
-		f = (f * (100 + in.PaymasterBuffer)) / 100
-	}
+	f = (f * (100 + baseVGLBuffer)) / 100
 	data["verificationGasLimit"] = hexutil.EncodeBig(big.NewInt(int64(f)))
 
 	// Find the optimal callGasLimit by setting the gas price to 0 and maxing out the gas limit. We don't run
@@ -138,7 +156,7 @@ func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err
 		TraceFeeCap: in.Op.MaxFeePerGas,
 	})
 	if err != nil {
-		return 0, 0, err
+		return retryEstimateGas(err, f, in)
 	}
 
 	// Calculate final values for verificationGasLimit and callGasLimit.
@@ -211,7 +229,7 @@ func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err
 			}
 			return simOp.VerificationGasLimit.Uint64(), big.NewInt(f).Uint64(), nil
 		}
-		return 0, 0, err
+		return retryEstimateGas(err, simOp.VerificationGasLimit.Int64(), in)
 	}
 	return simOp.VerificationGasLimit.Uint64(), simOp.CallGasLimit.Uint64(), nil
 }