Reduce diff between tracer and binary search for callGasLimit (stacku…

…p-wallet#222)

.github/workflows/compliance.yml

@@ -61,7 +61,7 @@ jobs:
           --dev \
           --allow-insecure-unlock \
           --rpc.allow-unprotected-txs \
-          --dev.gaslimit 20000000 &
+          --miner.gaslimit 12000000 &
 
       - name: Install bundler dependencies
         run: go mod download
@@ -87,6 +87,7 @@ jobs:
           ERC4337_BUNDLER_ETH_CLIENT_URL: http://localhost:8545/
           ERC4337_BUNDLER_PORT: 3000
           ERC4337_BUNDLER_DEBUG_MODE: true
+          ERC4337_BUNDLER_MAX_BATCH_GAS_LIMIT: 12000000
           # This key is for testing purposes only. Do not use for anything else.
           ERC4337_BUNDLER_PRIVATE_KEY: c6cbc5ffad570fdad0544d1b5358a36edeb98d163b6567912ac4754e144d4edb
 

.github/workflows/e2e.yml

@@ -82,7 +82,7 @@ jobs:
           --dev \
           --allow-insecure-unlock \
           --rpc.allow-unprotected-txs \
-          --dev.gaslimit 20000000 &
+          --miner.gaslimit 12000000 &
 
       - name: Install all dependencies
         run: |
@@ -110,6 +110,7 @@ jobs:
         env:
           ERC4337_BUNDLER_ETH_CLIENT_URL: http://localhost:8545/
           ERC4337_BUNDLER_DEBUG_MODE: true
+          ERC4337_BUNDLER_MAX_BATCH_GAS_LIMIT: 12000000
           # This key is for testing purposes only. Do not use for anything else.
           ERC4337_BUNDLER_PRIVATE_KEY: c6cbc5ffad570fdad0544d1b5358a36edeb98d163b6567912ac4754e144d4edb
 

e2e/README.md

@@ -21,18 +21,17 @@ First, we'll need to run a local instance of geth with the following command:
 
 ```bash
 geth \
-  --miner.gaslimit 12000000 \
-  --http \
-  --http.api personal,eth,net,web3,debug \
-  --allow-insecure-unlock \
-  --rpc.allow-unprotected-txs \
   --http.vhosts '*,localhost,host.docker.internal' \
+  --http \
+  --http.api eth,net,web3,debug \
   --http.corsdomain '*' \
   --http.addr "0.0.0.0" \
-  --dev \
   --nodiscover --maxpeers 0 --mine \
-  --miner.threads 1 \
-  --ignore-legacy-receipts
+  --networkid 1337 \
+  --dev \
+  --allow-insecure-unlock \
+  --rpc.allow-unprotected-txs \
+  --miner.gaslimit 12000000
 ```
 
 In a separate process, navigate to the [eth-infinitism/account-abstraction](https://github.com/eth-infinitism/account-abstraction/) directory and run the following command to deploy the required contracts:
@@ -52,6 +51,7 @@ Lastly, run the bundler with the following config:
 ```
 ERC4337_BUNDLER_ETH_CLIENT_URL=http://localhost:8545
 ERC4337_BUNDLER_PRIVATE_KEY=c6cbc5ffad570fdad0544d1b5358a36edeb98d163b6567912ac4754e144d4edb
+ERC4337_BUNDLER_MAX_BATCH_GAS_LIMIT=12000000
 ERC4337_BUNDLER_DEBUG_MODE=true
 ```
 

e2e/config.ts

@@ -13,7 +13,8 @@ const config: IConfig = {
   nodeUrl: "http://localhost:8545",
   bundlerUrl: "http://localhost:4337",
   testERC20Token: "0x3870419Ba2BBf0127060bCB37f69A1b1C090992B",
-  testGas: "0xE057Fe015a4cf6838403213E3576B73383428500",
+  // https://github.com/stackup-wallet/contracts/blob/main/contracts/test/TestGas.sol
+  testGas: "0x310de3671FCDA16eC24433e7610c2F48DEE7f8c2",
 };
 
 export default config;

e2e/src/abi.ts

@@ -160,56 +160,25 @@ export const testGasABI = [
   {
     inputs: [],
     name: "offset",
-    outputs: [
-      {
-        internalType: "uint256",
-        name: "",
-        type: "uint256",
-      },
-    ],
+    outputs: [{ internalType: "uint256", name: "", type: "uint256" }],
     stateMutability: "view",
     type: "function",
   },
   {
     inputs: [
-      {
-        internalType: "uint8",
-        name: "depth",
-        type: "uint8",
-      },
-      {
-        internalType: "uint8",
-        name: "count",
-        type: "uint8",
-      },
+      { internalType: "uint256", name: "depth", type: "uint256" },
+      { internalType: "uint256", name: "width", type: "uint256" },
+      { internalType: "uint256", name: "count", type: "uint256" },
     ],
     name: "recursiveCall",
-    outputs: [
-      {
-        internalType: "uint256",
-        name: "",
-        type: "uint256",
-      },
-    ],
+    outputs: [{ internalType: "uint256", name: "", type: "uint256" }],
     stateMutability: "payable",
     type: "function",
   },
   {
-    inputs: [
-      {
-        internalType: "uint256",
-        name: "",
-        type: "uint256",
-      },
-    ],
+    inputs: [{ internalType: "uint256", name: "", type: "uint256" }],
     name: "store",
-    outputs: [
-      {
-        internalType: "uint256",
-        name: "",
-        type: "uint256",
-      },
-    ],
+    outputs: [{ internalType: "uint256", name: "", type: "uint256" }],
     stateMutability: "view",
     type: "function",
   },

e2e/src/helpers.ts

@@ -26,10 +26,10 @@ export const fundIfRequired = async (
   if (accountBalance.eq(0)) {
     const response = await signer.sendTransaction({
       to: account,
-      value: ethers.constants.WeiPerEther,
+      value: ethers.constants.WeiPerEther.mul(2),
     });
     await response.wait();
-    console.log("Funded Account with 1 ETH...");
+    console.log("Funded Account with 2 ETH...");
   }
 
   if (accountTokenBalance.eq(0)) {

e2e/test/withoutPaymaster.test.ts

@@ -113,16 +113,16 @@ describe("Without Paymaster", () => {
         acc.execute(
           config.testGas,
           0,
-          testGas.interface.encodeFunctionData("recursiveCall", [32, 32])
+          testGas.interface.encodeFunctionData("recursiveCall", [32, 0, 32])
         )
       );
     } catch (error: any) {
-      expect(error?.error.code).toBe(errorCodes.invalidFields);
+      expect(error?.error.code).toBe(errorCodes.executionReverted);
     }
   });
 
   describe("With increasing call stack size", () => {
-    describe("With zero value calls", () => {
+    describe("With zero value", () => {
       [0, 2, 4, 8, 16].forEach((depth) => {
         test(`Sender can make contract interactions with ${depth} recursive calls`, async () => {
           const response = await client.sendUserOperation(
@@ -131,6 +131,7 @@ describe("Without Paymaster", () => {
               0,
               testGas.interface.encodeFunctionData("recursiveCall", [
                 depth,
+                0,
                 depth,
               ])
             )
@@ -142,7 +143,7 @@ describe("Without Paymaster", () => {
       });
     });
 
-    describe("With non-zero value calls", () => {
+    describe("With non-zero value", () => {
       [0, 2, 4, 8, 16].forEach((depth) => {
         test(`Sender can make contract interactions with ${depth} recursive calls`, async () => {
           const response = await client.sendUserOperation(
@@ -151,6 +152,29 @@ describe("Without Paymaster", () => {
               ethers.utils.parseEther("0.001"),
               testGas.interface.encodeFunctionData("recursiveCall", [
                 depth,
+                0,
+                depth,
+              ])
+            )
+          );
+          const event = await response.wait();
+
+          expect(event?.args.success).toBe(true);
+        });
+      });
+    });
+
+    describe("With multiple stacks per depth", () => {
+      [0, 1, 2, 3].forEach((depth) => {
+        test(`Sender can make contract interactions with ${depth} recursive calls`, async () => {
+          const width = depth;
+          const response = await client.sendUserOperation(
+            acc.execute(
+              config.testGas,
+              0,
+              testGas.interface.encodeFunctionData("recursiveCall", [
+                depth,
+                width,
                 depth,
               ])
             )

internal/start/private.go

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

internal/start/searcher.go

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

pkg/client/utils.go

@@ -48,14 +48,16 @@ func GetGasEstimateWithEthClient(
 	rpc *rpc.Client,
 	ov *gas.Overhead,
 	chain *big.Int,
+	maxGasLimit *big.Int,
 ) 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,
+			Rpc:         rpc,
+			EntryPoint:  ep,
+			Op:          op,
+			Ov:          ov,
+			ChainID:     chain,
+			MaxGasLimit: maxGasLimit,
 		})
 	}
 }

pkg/gas/estimate.go

@@ -25,11 +25,12 @@ func isExecutionOOG(err error) bool {
 }
 
 type EstimateInput struct {
-	Rpc        *rpc.Client
-	EntryPoint common.Address
-	Op         *userop.UserOperation
-	Ov         *Overhead
-	ChainID    *big.Int
+	Rpc         *rpc.Client
+	EntryPoint  common.Address
+	Op          *userop.UserOperation
+	Ov          *Overhead
+	ChainID     *big.Int
+	MaxGasLimit *big.Int
 }
 
 // EstimateGas uses the simulateHandleOp method on the EntryPoint to derive an estimate for
@@ -56,8 +57,8 @@ func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err
 	// 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
+	l := int64(0)
+	r := in.MaxGasLimit.Int64()
 	var simErr error
 	for l <= r {
 		m := (l + r) / 2
@@ -105,7 +106,7 @@ func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err
 	// 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)))
+	data["callGasLimit"] = hexutil.EncodeBig(in.MaxGasLimit)
 	simOp, err := userop.New(data)
 	if err != nil {
 		return 0, 0, err
@@ -122,9 +123,7 @@ func EstimateGas(in *EstimateInput) (verificationGas uint64, callGas uint64, err
 
 	// Calculate final values for verificationGasLimit and callGasLimit.
 	vgl := simOp.VerificationGasLimit
-	cg := big.NewInt(0).Sub(out.Event.ActualGasUsed, out.Result.PreOpGas)
-	cgb := big.NewInt(int64(out.Trace.ExecutionGasBuffer))
-	cgl := big.NewInt(0).Add(cg, cgb)
+	cgl := big.NewInt(int64(out.Trace.ExecutionGasLimit))
 	if cgl.Cmp(in.Ov.NonZeroValueCall()) < 0 {
 		cgl = in.Ov.NonZeroValueCall()
 	}

pkg/tracer/BundlerErrorTracer.js

@@ -2,9 +2,11 @@ var tracer = {
   reverts: [],
   validationOOG: false,
   executionOOG: false,
-  executionGasBuffer: 0,
+  executionGasLimit: 0,
 
   _depth: 0,
+  _executionGasStack: [],
+  _defaultGasItem: { used: 0, required: 0 },
   _marker: 0,
   _validationMarker: 1,
   _executionMarker: 3,
@@ -48,23 +50,37 @@ var tracer = {
       reverts: this.reverts,
       validationOOG: this.validationOOG,
       executionOOG: this.executionOOG,
-      executionGasBuffer: this.executionGasBuffer,
+      executionGasLimit: this.executionGasLimit,
       userOperationEvent: this.userOperationEvent,
       output: toHex(ctx.output),
     };
   },
 
-  enter: function enter(frame) {},
+  enter: function enter(frame) {
+    if (this._isExecution()) {
+      var next = this._depth + 1;
+      if (this._executionGasStack[next] === undefined)
+        this._executionGasStack[next] = Object.assign({}, this._defaultGasItem);
+    }
+  },
   exit: function exit(frame) {
     if (this._isExecution()) {
       if (frame.getError() !== undefined) {
         this.reverts.push(toHex(frame.getOutput()));
       }
 
-      if (this._depth > 2) {
-        this.executionGasBuffer = Math.ceil(
-          frame.getGasUsed() + this.executionGasBuffer / 63
-        );
+      if (this._depth >= 2) {
+        var prev = Object.assign({}, this._defaultGasItem);
+        if (this._executionGasStack[this._depth + 1] !== undefined)
+          prev = this._executionGasStack[this._depth + 1];
+
+        var used = frame.getGasUsed();
+        var required = used - prev.used + prev.required;
+        this._executionGasStack[this._depth].used += used;
+        this._executionGasStack[this._depth].required +=
+          required + Math.ceil(required / 63);
+
+        this.executionGasLimit = this._executionGasStack[this._depth].required;
       }
     }
   },

pkg/tracer/types.go

@@ -59,7 +59,7 @@ type BundlerErrorReturn struct {
 	Reverts            []string `json:"reverts"`
 	ValidationOOG      bool     `json:"validationOOG"`
 	ExecutionOOG       bool     `json:"executionOOG"`
-	ExecutionGasBuffer float64  `json:"executionGasBuffer"`
+	ExecutionGasLimit  float64  `json:"executionGasLimit"`
 	UserOperationEvent *LogInfo `json:"userOperationEvent,omitempty"`
 	Output             string   `json:"output"`
 }