test adjustments wip

safe-global · Jun 24, 2024 · 10f95ec · 10f95ec
1 parent bec3b5c
commit 10f95ec
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 69 deletions.
diff --git a/modules/4337/contracts/test/SafeMock.sol b/modules/4337/contracts/test/SafeMock.sol
@@ -216,8 +216,15 @@ contract Safe4337Mock is SafeMock, IAccount {
      */
     function _validateSignatures(PackedUserOperation calldata userOp) internal view returns (uint256 validationData) {
         (bytes memory operationData, uint48 validAfter, uint48 validUntil, bytes calldata signatures) = _getSafeOp(userOp);
-        checkSignatures(keccak256(operationData), operationData, signatures);
-        validationData = _packValidationData(false, validUntil, validAfter);
+
+        bytes32 dataHash = keccak256(operationData);
+        uint8 v;
+        bytes32 r;
+        bytes32 s;
+        (v, r, s) = _signatureSplit(signatures);
+        bool validSignature = owner == ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
+
+        validationData = _packValidationData(!validSignature, validUntil, validAfter);
     }
 
     /**

diff --git a/modules/4337/src/utils/execution.ts b/modules/4337/src/utils/execution.ts
@@ -146,27 +146,48 @@ export const logGas = async (message: string, tx: Promise<TransactionResponse>,
   })
 }
 
+type UserOperationGasLog = {
+  actualGasUsed: bigint
+  actualGasCost: bigint
+  transactionResponse: TransactionResponse
+}
+
+/**
+ * Logs the gas used by a user operation and returns the gas log.
+ *
+ * @param message - The message associated with the user operation.
+ * @param entryPoint - The entry point object.
+ * @param tx - The transaction promise.
+ * @param skip - Optional flag to skip logging.
+ * @returns {UserOperationGasLog} A promise that resolves to the user operation gas log.
+ * @throws An error if the receipt is not available, gas used is not available in the receipt,
+ * gas used or gas cost is not available in the UserOperationEvent, or UserOperationEvent is not emitted.
+ */
 export const logUserOperationGas = async (
   message: string,
   entryPoint: IEntryPoint,
   tx: Promise<TransactionResponse>,
   skip?: boolean,
-): Promise<TransactionResponse> => {
-  return tx.then(async (result) => {
-    const receipt = await result.wait()
+): Promise<UserOperationGasLog> => {
+  return tx.then(async (transactionResponse) => {
+    const receipt = await transactionResponse.wait()
     if (!receipt) throw new Error('No receipt')
 
     const userOperationEvent = await entryPoint.queryFilter(entryPoint.filters.UserOperationEvent(), receipt.blockNumber)
     const parsedUserOperationEvent = entryPoint.interface.parseLog(userOperationEvent[0])
 
     if (!receipt?.gasUsed) throw new Error('No gas used in receipt')
-    if (!parsedUserOperationEvent?.args.actualGasUsed)
-      throw new Error('No gas used in UserOperationEvent or UserOperationEvent not emitted')
+    if (!parsedUserOperationEvent?.args.actualGasUsed || !parsedUserOperationEvent?.args.actualGasCost)
+      throw new Error('No gas used or gas cost in UserOperationEvent or UserOperationEvent not emitted')
 
     if (!skip) {
       console.log(`           Used ${parsedUserOperationEvent.args.actualGasUsed} gas (Account or Paymaster) for >${message}<`)
       console.log(`           Used ${receipt.gasUsed} gas (Transaction) for >${message}<`)
     }
-    return result
+    return {
+      actualGasUsed: parsedUserOperationEvent.args.actualGasUsed,
+      actualGasCost: parsedUserOperationEvent.args.actualGasCost,
+      transactionResponse,
+    }
   })
 }
diff --git a/modules/4337/src/utils/userOp.ts b/modules/4337/src/utils/userOp.ts
@@ -175,11 +175,10 @@ export const getRequiredGas = (userOp: PackedUserOperation): string => {
   return (BigInt(callGasLimit) + BigInt(verificationGasLimit) * multiplier + BigInt(userOp.preVerificationGas)).toString()
 }
 
-export const getRequiredPrefund = (userOp: PackedUserOperation): string => {
+export const getRequiredPrefund = (userOp: PackedUserOperation): bigint => {
   const requiredGas = getRequiredGas(userOp)
   const { maxFeePerGas } = unpackGasParameters(userOp)
-  const requiredPrefund = (BigInt(requiredGas) * BigInt(maxFeePerGas)).toString()
-  console.log({ requiredGas, requiredPrefund })
+  const requiredPrefund = BigInt(requiredGas) * BigInt(maxFeePerGas)
 
   return requiredPrefund
 }

diff --git a/modules/4337/test/erc4337/ERC4337ModuleExisting.spec.ts b/modules/4337/test/erc4337/ERC4337ModuleExisting.spec.ts
@@ -6,6 +6,7 @@ import {
   calculateSafeOperationHash,
   buildPackedUserOperationFromSafeUserOperation,
   buildSafeUserOpTransaction,
+  getRequiredPrefund,
 } from '../../src/utils/userOp'
 import { chainId } from '../utils/encoding'
 import { estimateUserOperationGas } from '../utils/simulations'
@@ -73,6 +74,8 @@ describe('Safe4337Module - Existing Safe', () => {
       safeOp.callGasLimit = gasEstimation.callGasLimit
       safeOp.preVerificationGas = gasEstimation.preVerificationGas
       safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
+      safeOp.maxFeePerGas = gasEstimation.maxFeePerGas
+      safeOp.maxPriorityFeePerGas = gasEstimation.maxPriorityFeePerGas
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildPackedUserOperationFromSafeUserOperation({ safeOp, signature })
@@ -175,14 +178,13 @@ describe('Safe4337Module - Existing Safe', () => {
         await entryPoint.getAddress(),
         false,
         true,
-        {
-          maxFeePerGas: '0',
-        },
       )
       const gasEstimation = await estimateUserOperationGas(ethers.provider, entryPointSimulations, safeOp, entryPointAddress)
       safeOp.callGasLimit = gasEstimation.callGasLimit
       safeOp.preVerificationGas = gasEstimation.preVerificationGas
       safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
+      safeOp.maxFeePerGas = gasEstimation.maxFeePerGas
+      safeOp.maxPriorityFeePerGas = gasEstimation.maxPriorityFeePerGas
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildPackedUserOperationFromSafeUserOperation({ safeOp, signature })
@@ -191,7 +193,8 @@ describe('Safe4337Module - Existing Safe', () => {
         entryPoint,
         entryPoint.handleOps([userOp], relayer),
       )
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.5'))
+      const paidPrefund = getRequiredPrefund(userOp)
+      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.5') - paidPrefund)
     })
 
     it('executeUserOpWithErrorString reverts on failure and bubbles up the revert reason', async () => {

diff --git a/modules/4337/test/erc4337/ERC4337ModuleNew.spec.ts b/modules/4337/test/erc4337/ERC4337ModuleNew.spec.ts
@@ -1,8 +1,20 @@
 import { expect } from 'chai'
 import { deployments, ethers } from 'hardhat'
-import { getSafe4337Module, getEntryPoint, getFactory, getSafeModuleSetup, getSafeL2Singleton, getEntryPointSimulations } from '../utils/setup'
+import {
+  getSafe4337Module,
+  getEntryPoint,
+  getFactory,
+  getSafeModuleSetup,
+  getSafeL2Singleton,
+  getEntryPointSimulations,
+} from '../utils/setup'
 import { buildSignatureBytes, logUserOperationGas } from '../../src/utils/execution'
-import { buildPackedUserOperationFromSafeUserOperation, buildSafeUserOpTransaction, signSafeOp } from '../../src/utils/userOp'
+import {
+  buildPackedUserOperationFromSafeUserOperation,
+  buildSafeUserOpTransaction,
+  getRequiredPrefund,
+  signSafeOp,
+} from '../../src/utils/userOp'
 import { chainId } from '../utils/encoding'
 import { Safe4337 } from '../../src/utils/safe'
 import { estimateUserOperationGas } from '../utils/simulations'
@@ -96,7 +108,8 @@ describe('Safe4337Module - Newly deployed safe', () => {
       safeOp.callGasLimit = gasEstimation.callGasLimit
       safeOp.preVerificationGas = gasEstimation.preVerificationGas
       safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
-
+      safeOp.maxFeePerGas = gasEstimation.maxFeePerGas
+      safeOp.maxPriorityFeePerGas = gasEstimation.maxPriorityFeePerGas
       const signature = buildSignatureBytes([await signSafeOp(user1, await validator.getAddress(), safeOp, await chainId())])
       const userOp = buildPackedUserOperationFromSafeUserOperation({
         safeOp,
@@ -187,6 +200,8 @@ describe('Safe4337Module - Newly deployed safe', () => {
       safeOp.callGasLimit = gasEstimation.callGasLimit
       safeOp.preVerificationGas = gasEstimation.preVerificationGas
       safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
+      safeOp.maxFeePerGas = gasEstimation.maxFeePerGas
+      safeOp.maxPriorityFeePerGas = gasEstimation.maxPriorityFeePerGas
       const signature = buildSignatureBytes([await signSafeOp(user1, await validator.getAddress(), safeOp, await chainId())])
       const userOp = buildPackedUserOperationFromSafeUserOperation({
         safeOp,
@@ -217,13 +232,14 @@ describe('Safe4337Module - Newly deployed safe', () => {
         true,
         {
           initCode: safe.getInitCode(),
-          maxFeePerGas: 0,
         },
       )
       const gasEstimation = await estimateUserOperationGas(ethers.provider, entryPointSimulations, safeOp, entryPointAddress)
       safeOp.callGasLimit = gasEstimation.callGasLimit
       safeOp.preVerificationGas = gasEstimation.preVerificationGas
       safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
+      safeOp.maxFeePerGas = gasEstimation.maxFeePerGas
+      safeOp.maxPriorityFeePerGas = gasEstimation.maxPriorityFeePerGas
       const signature = buildSignatureBytes([await signSafeOp(user1, await validator.getAddress(), safeOp, await chainId())])
       const userOp = buildPackedUserOperationFromSafeUserOperation({
         safeOp,
@@ -234,7 +250,8 @@ describe('Safe4337Module - Newly deployed safe', () => {
         entryPoint,
         entryPoint.handleOps([userOp], user1.address),
       )
-      expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.parseEther('0.5'))
+      const paidPrefund = getRequiredPrefund(userOp)
+      expect(await ethers.provider.getBalance(safe.address)).to.be.eq(ethers.parseEther('0.5') - paidPrefund)
     })
 
     it('executeUserOpWithErrorString reverts on failure and bubbles up the revert reason', async () => {

diff --git a/modules/4337/test/erc4337/ReferenceEntryPoint.spec.ts b/modules/4337/test/erc4337/ReferenceEntryPoint.spec.ts
@@ -2,7 +2,7 @@ import { expect } from 'chai'
 import { deployments, ethers } from 'hardhat'
 import { time } from '@nomicfoundation/hardhat-network-helpers'
 import { EventLog, Log } from 'ethers'
-import { getEntryPoint, getEntryPointSimulations, getFactory, getSafeModuleSetup } from '../utils/setup'
+import { getEntryPoint, getFactory, getSafeModuleSetup } from '../utils/setup'
 import { buildSignatureBytes, logUserOperationGas } from '../../src/utils/execution'
 import {
   buildSafeUserOpTransaction,
@@ -12,15 +12,13 @@ import {
 } from '../../src/utils/userOp'
 import { chainId } from '../utils/encoding'
 import { Safe4337 } from '../../src/utils/safe'
-import { estimateUserOperationGas } from '../utils/simulations'
 
 describe('Safe4337Module - Reference EntryPoint', () => {
   const setupTests = async () => {
     await deployments.fixture()
     const [user, deployer, relayer] = await ethers.getSigners()
 
     const entryPoint = await getEntryPoint()
-    const entryPointSimulations = await getEntryPointSimulations()
     const moduleFactory = await ethers.getContractFactory('Safe4337Module')
     const module = await moduleFactory.deploy(await entryPoint.getAddress())
     const proxyFactory = await getFactory()
@@ -46,14 +44,12 @@ describe('Safe4337Module - Reference EntryPoint', () => {
       safe,
       validator: module,
       entryPoint,
-      entryPointSimulations,
       safeGlobalConfig,
     }
   }
 
   it('should deploy a Safe and execute transactions', async () => {
-    const { user, relayer, safe, validator, entryPoint, entryPointSimulations } = await setupTests()
-    const entryPointAddress = await entryPoint.getAddress()
+    const { user, relayer, safe, validator, entryPoint } = await setupTests()
 
     const accountBalance = ethers.parseEther('1.0')
     await user.sendTransaction({ to: safe.address, value: accountBalance })
@@ -74,10 +70,7 @@ describe('Safe4337Module - Reference EntryPoint', () => {
             initCode: nonce === 0 ? safe.getInitCode() : '0x',
           },
         )
-        const gasEstimation = await estimateUserOperationGas(ethers.provider, entryPointSimulations, safeOp, entryPointAddress)
-        safeOp.callGasLimit = gasEstimation.callGasLimit
-        safeOp.preVerificationGas = gasEstimation.preVerificationGas
-        safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
+
         const signature = buildSignatureBytes([await signSafeOp(user, await validator.getAddress(), safeOp, await chainId())])
         return buildPackedUserOperationFromSafeUserOperation({
           safeOp,
@@ -86,12 +79,12 @@ describe('Safe4337Module - Reference EntryPoint', () => {
       }),
     )
 
-    const transaction = await logUserOperationGas(
+    const { transactionResponse } = await logUserOperationGas(
       'Execute UserOps with reference EntryPoint',
       entryPoint,
       entryPoint.handleOps(userOps, await relayer.getAddress()),
     )
-    const receipt = await transaction.wait()
+    const receipt = await transactionResponse.wait()
 
     const transfers = ethers.parseEther('0.1') * BigInt(userOps.length)
     const deposits = receipt.logs
@@ -102,8 +95,7 @@ describe('Safe4337Module - Reference EntryPoint', () => {
   })
 
   it('should correctly bubble up the signature timestamps to the entrypoint', async () => {
-    const { user, relayer, safe, validator, entryPoint, entryPointSimulations } = await setupTests()
-    const entryPointAddress = await entryPoint.getAddress()
+    const { user, relayer, safe, validator, entryPoint } = await setupTests()
 
     const accountBalance = ethers.parseEther('1.0')
     const now = await time.latest()
@@ -132,10 +124,6 @@ describe('Safe4337Module - Reference EntryPoint', () => {
             validUntil,
           },
         )
-        const gasEstimation = await estimateUserOperationGas(ethers.provider, entryPointSimulations, safeOp, entryPointAddress)
-        safeOp.callGasLimit = gasEstimation.callGasLimit
-        safeOp.preVerificationGas = gasEstimation.preVerificationGas
-        safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
         const signature = buildSignatureBytes([await signSafeOp(user, await validator.getAddress(), safeOp, await chainId())])
         return buildPackedUserOperationFromSafeUserOperation({
           safeOp,
@@ -149,12 +137,12 @@ describe('Safe4337Module - Reference EntryPoint', () => {
       .withArgs(0, 'AA22 expired or not due')
     await time.increaseTo(validAfter + 1)
 
-    const transaction = await logUserOperationGas(
+    const { transactionResponse } = await logUserOperationGas(
       'Execute UserOps with reference EntryPoint',
       entryPoint,
       entryPoint.handleOps(userOps, await relayer.getAddress()),
     )
-    const receipt = await transaction.wait()
+    const receipt = await transactionResponse.wait()
 
     const transfers = ethers.parseEther('0.1') * BigInt(userOps.length)
     const deposits = receipt.logs
@@ -165,8 +153,7 @@ describe('Safe4337Module - Reference EntryPoint', () => {
   })
 
   it('should support a Safe signer (NOTE: would require a staked paymaster for ERC-4337)', async () => {
-    const { user, relayer, safe: parentSafe, validator, entryPoint, entryPointSimulations, safeGlobalConfig } = await setupTests()
-    const entryPointAddress = await entryPoint.getAddress()
+    const { user, relayer, safe: parentSafe, validator, entryPoint, safeGlobalConfig } = await setupTests()
 
     await parentSafe.deploy(user)
     const daughterSafe = await Safe4337.withSigner(parentSafe.address, safeGlobalConfig)
@@ -189,10 +176,7 @@ describe('Safe4337Module - Reference EntryPoint', () => {
         initCode: daughterSafe.getInitCode(),
       },
     )
-    const gasEstimation = await estimateUserOperationGas(ethers.provider, entryPointSimulations, safeOp, entryPointAddress)
-    safeOp.callGasLimit = gasEstimation.callGasLimit
-    safeOp.preVerificationGas = gasEstimation.preVerificationGas
-    safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
+
     const opData = calculateSafeOperationData(await validator.getAddress(), safeOp, await chainId())
     const signature = buildSignatureBytes([
       {
@@ -217,12 +201,12 @@ describe('Safe4337Module - Reference EntryPoint', () => {
       signature,
     })
 
-    const transaction = await logUserOperationGas(
+    const { transactionResponse } = await logUserOperationGas(
       'Execute UserOps with reference EntryPoint',
       entryPoint,
       entryPoint.handleOps([userOp], await relayer.getAddress()),
     )
-    const receipt = await transaction.wait()
+    const receipt = await transactionResponse.wait()
 
     const deposits = receipt.logs
       .filter(isEventLog)

diff --git a/modules/4337/test/erc4337/Safe4337Mock.spec.ts b/modules/4337/test/erc4337/Safe4337Mock.spec.ts
@@ -7,6 +7,7 @@ import {
   buildSafeUserOpTransaction,
   buildPackedUserOperationFromSafeUserOperation,
   calculateSafeOperationHash,
+  getRequiredPrefund,
 } from '../../src/utils/userOp'
 import { chainId, timestamp } from '../utils/encoding'
 import { estimateUserOperationGas } from '../utils/simulations'
@@ -54,11 +55,14 @@ describe('Safe4337Mock', () => {
       safeOp.callGasLimit = gasEstimation.callGasLimit
       safeOp.preVerificationGas = gasEstimation.preVerificationGas
       safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
+      safeOp.maxFeePerGas = gasEstimation.maxFeePerGas
+      safeOp.maxPriorityFeePerGas = gasEstimation.maxPriorityFeePerGas
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildPackedUserOperationFromSafeUserOperation({ safeOp, signature })
       await logUserOperationGas('Execute UserOp without fee payment', entryPoint, entryPoint.handleOps([userOp], user1.address))
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.5'))
+      const paidPrefund = getRequiredPrefund(userOp)
+      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.5') - paidPrefund)
     })
 
     it('should execute contract calls with fee', async () => {
@@ -82,6 +86,8 @@ describe('Safe4337Mock', () => {
       safeOp.callGasLimit = gasEstimation.callGasLimit
       safeOp.preVerificationGas = gasEstimation.preVerificationGas
       safeOp.verificationGasLimit = gasEstimation.verificationGasLimit
+      safeOp.maxFeePerGas = gasEstimation.maxFeePerGas
+      safeOp.maxPriorityFeePerGas = gasEstimation.maxPriorityFeePerGas
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildPackedUserOperationFromSafeUserOperation({ safeOp, signature })