Fix runOp Script (#435)

I was running the 4337 `runOp.ts` script this morning and noticed it was subtly broken. This PR fixes it. I also refactored the scripts a bit - specifically we no longer `--fix` on `lint:ts`. This was because it would cause CI to silently pass in cases where the linter could autofix the code (when we want it to fail instead).
safe-global · Jun 8, 2024 · fca988e · fca988e
1 parent 8a8f501
commit fca988e
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 18 deletions.
diff --git a/modules/4337/package.json b/modules/4337/package.json
@@ -26,8 +26,9 @@
     "deploy": "hardhat deploy --network",
     "lint": "pnpm run lint:sol && npm run lint:ts",
     "lint:sol": "solhint 'contracts/**/*.sol'",
-    "lint:ts": "eslint ./src --fix && eslint ./test --fix",
-    "fmt": "prettier --write ./contracts/**/*.sol",
+    "lint:ts": "eslint ./src && eslint ./test",
+    "lint:fix": "eslint ./src --fix && eslint ./test --fix",
+    "fmt": "prettier --write .",
     "fmt:check": "prettier --check ./**/*.sol",
     "prepare": "pnpm run build"
   },

diff --git a/modules/4337/scripts/runOp.ts b/modules/4337/scripts/runOp.ts
@@ -48,11 +48,18 @@ const runOp = async () => {
   // All other methods return an error
   const accountAbstractionProvider = new MultiProvider4337(BUNDLER_URL!, ethers.provider)
   const entryPoints = await getSupportedEntryPoints(accountAbstractionProvider)
-  const entryPoint = entryPoints[0]
   const moduleAddress = MODULE_ADDRESS ?? (await getSafe4337Module().then((module) => module.getAddress()))
-  const moduleSupportedEntrypoint = await user1.call({ to: moduleAddress, data: INTERFACES.encodeFunctionData('SUPPORTED_ENTRYPOINT') })
+  const [moduleSupportedEntrypoint] = ethers.AbiCoder.defaultAbiCoder().decode(
+    ['address'],
+    await user1.call({ to: moduleAddress, data: INTERFACES.encodeFunctionData('SUPPORTED_ENTRYPOINT') }),
+  )
   console.log({ moduleAddress, moduleSupportedEntrypoint })
 
+  const entryPoint = entryPoints.find((entry) => entry === moduleSupportedEntrypoint)
+  if (entryPoint === undefined) {
+    throw new Error('Module does not support any of the available entry points')
+  }
+
   const proxyCreationCode = (await callInterface(PROXY_FACTORY_ADDRESS, 'proxyCreationCode'))[0]
 
   const globalConfig: GlobalConfig = {
@@ -68,10 +75,10 @@ const runOp = async () => {
 
   safe.connect(accountAbstractionProvider)
 
-  console.log(safe.address)
+  console.log({ safe: safe.address })
   const safeBalance = await ethers.provider.getBalance(safe.address)
   const minBalance = ethers.parseEther('0.01')
-  console.log(safeBalance)
+  console.log({ safeBalance })
   if (safeBalance < minBalance) {
     await (await user1.sendTransaction({ to: safe.address, value: ethers.parseEther('0.01') })).wait()
   }

diff --git a/modules/4337/src/utils/safe.ts b/modules/4337/src/utils/safe.ts
@@ -3,7 +3,14 @@ import { Provider, Signer, ethers } from 'ethers'
 
 // Import from Safe contracts repo once it is upgraded to ethers v6 and can be installed via npm
 import { MetaTransaction, SafeSignature, SignedSafeTransaction, buildSignatureBytes } from './execution'
-import { PackedUserOperation, UserOperation, EIP712_SAFE_OPERATION_TYPE, packGasParameters, unpackUserOperation } from './userOp'
+import {
+  PackedUserOperation,
+  UserOperation,
+  EIP712_SAFE_OPERATION_TYPE,
+  packGasParameters,
+  unpackUserOperation,
+  unpackInitCode,
+} from './userOp'
 
 export { MultiProvider4337 }
 
@@ -86,7 +93,6 @@ const callInterface = async (provider: Provider, contract: string, method: strin
     to: contract,
     data: INTERFACES.encodeFunctionData(method, params),
   })
-  console.log(result)
   return INTERFACES.decodeFunctionResult(method, result)
 }
 
@@ -115,6 +121,7 @@ export class Safe4337Operation {
       {
         safe: this.safe.address,
         callData: actionCalldata(this.action),
+        paymasterAndData: '0x',
         entryPoint: this.globalConfig.entryPoint,
         ...this.params,
       },
@@ -160,12 +167,13 @@ export class Safe4337Operation {
         {
           safe: this.safe.address,
           callData: actionCalldata(this.action),
+          paymasterAndData: '0x',
           entryPoint: this.globalConfig.entryPoint,
           ...this.params,
         },
       ),
     })
-    console.log(this.signatures)
+    console.log({ signatures: this.signatures })
   }
 
   static async build(
@@ -179,30 +187,36 @@ export class Safe4337Operation {
     const estimateOperation = {
       sender: safe.address,
       callData: actionCalldata(action),
-      paymasterAndData: '0x',
       nonce: ethers.toBeHex(nonce),
-      initCode,
-      signature: '0x'.padEnd(130, 'a'),
+      ...unpackInitCode({ initCode }),
       // For some providers we need to set some really high values to allow estimation
-      preVerificationGas: ethers.toBeHex(1000000),
-      verificationGasLimit: ethers.toBeHex(1000000),
       callGasLimit: ethers.toBeHex(10000000),
-      // To keep the required funds low, the gas fee is set close to the minimum
+      verificationGasLimit: ethers.toBeHex(1000000),
+      preVerificationGas: ethers.toBeHex(1000000),
+      // User arbitrary gas fee values - note that we use lower values in order to reduce the amount
+      // of gas fees used in tests; when estimating with a real bundler, they will choose these for
+      // for us anyway.
       maxFeePerGas: '0x10',
       maxPriorityFeePerGas: '0x10',
+      // Use dummy signature that makes ECRECOVER get called in order to have slightly more accurate
+      // estimates for single signer operations.
+      signature: `0x${'aa'.repeat(32)}${'bb'.repeat(32)}1b`,
     }
     const estimates = await provider.send('eth_estimateUserOperationGas', [
       {
         ...estimateOperation,
       },
       globalConfig.entryPoint,
     ])
-    console.log(estimates)
-
-    const feeData = await provider.getFeeData()
+    console.log({ estimates })
 
+    const feeData = { ...(await provider.getFeeData()) }
     if (!feeData.maxFeePerGas || !feeData.maxPriorityFeePerGas) throw Error('Missing fee data')
 
+    // Some bundlers require higher priority fees and use non-standard APIs for this. Instead, just
+    // bump the priority fee by 20% to ensure that the operation is accepted by the bundler.
+    feeData.maxPriorityFeePerGas += (feeData.maxPriorityFeePerGas * 20n) / 100n
+
     const params: OperationParams = {
       nonce,
       initCode,