Use production entrypoint in integration tests (#262)

This PR: - Implements #226 by deploying the EntryPoint contract and using it through the `IEntryPoint` interface - Removed the test entrypoint contract - Had to adjust quite some test because of the difference in entrypoint implementations (for example the test entrypoint didn't care about the pre-fund) --------- Co-authored-by: Nicholas Rodrigues Lordello <[email protected]>
safe-global · Feb 13, 2024 · b8568dd · b8568dd
1 parent c2bdf00
commit b8568dd
Show file tree

Hide file tree

Showing 10 changed files with 173 additions and 315 deletions.
diff --git a/modules/4337/contracts/test/TestEntryPoint.sol b/modules/4337/contracts/test/TestEntryPoint.sol
diff --git a/modules/4337/src/deploy/entrypoint.ts b/modules/4337/src/deploy/entrypoint.ts
@@ -7,15 +7,7 @@ const deploy: DeployFunction = async ({ deployments, getNamedAccounts, network }
   const { deployer } = await getNamedAccounts()
   const { deploy } = deployments
 
-  if (network.tags.test) {
-    await deploy('EntryPoint', {
-      from: deployer,
-      contract: 'TestEntryPoint',
-      args: [],
-      log: true,
-      deterministicDeployment: true,
-    })
-  } else if (network.tags.dev) {
+  if (network.tags.dev || network.tags.test) {
     await deploy('EntryPoint', {
       from: deployer,
       contract: EntryPoint,

diff --git a/modules/4337/src/utils/userOp.ts b/modules/4337/src/utils/userOp.ts
@@ -80,19 +80,19 @@ export const buildSafeUserOp = (template: OptionalExceptFor<SafeUserOperation, '
   return {
     safe: template.safe,
     nonce: template.nonce,
-    initCode: template.initCode || '0x',
-    callData: template.callData || '0x',
-    callGasLimit: template.callGasLimit || 2000000,
-    verificationGasLimit: template.verificationGasLimit || 500000,
-    preVerificationGas: template.preVerificationGas || 60000,
+    initCode: template.initCode ?? '0x',
+    callData: template.callData ?? '0x',
+    callGasLimit: template.callGasLimit ?? 2000000,
+    verificationGasLimit: template.verificationGasLimit ?? 500000,
+    preVerificationGas: template.preVerificationGas ?? 60000,
     // use same maxFeePerGas and maxPriorityFeePerGas to ease testing prefund validation
     // otherwise it's tricky to calculate the prefund because of dynamic parameters like block.basefee
     // check UserOperation.sol#gasPrice()
-    maxFeePerGas: template.maxFeePerGas || 10000000000,
-    maxPriorityFeePerGas: template.maxPriorityFeePerGas || 10000000000,
-    paymasterAndData: template.paymasterAndData || '0x',
-    validAfter: template.validAfter || 0,
-    validUntil: template.validUntil || 0,
+    maxFeePerGas: template.maxFeePerGas ?? 10000000000,
+    maxPriorityFeePerGas: template.maxPriorityFeePerGas ?? 10000000000,
+    paymasterAndData: template.paymasterAndData ?? '0x',
+    validAfter: template.validAfter ?? 0,
+    validUntil: template.validUntil ?? 0,
     entryPoint: template.entryPoint,
   }
 }

diff --git a/modules/4337/test/erc4337/ERC4337ModuleExisting.spec.ts b/modules/4337/test/erc4337/ERC4337ModuleExisting.spec.ts
@@ -9,25 +9,26 @@ describe('Safe4337Module - Existing Safe', () => {
   const setupTests = deployments.createFixture(async ({ deployments }) => {
     await deployments.fixture()
 
-    const [user1] = await ethers.getSigners()
-    const entryPoint = await getEntryPoint()
+    const [user1, relayer] = await ethers.getSigners()
+    let entryPoint = await getEntryPoint()
+    entryPoint = entryPoint.connect(relayer)
     const module = await getSafe4337Module()
     const safe = await getTestSafe(user1, await module.getAddress(), await module.getAddress())
 
     return {
       user1,
       safe,
+      relayer,
       validator: module,
       entryPoint,
     }
   })
 
-  describe('executeUserOp - existing account', () => {
+  describe('handleOps - existing account', () => {
     it('should revert with invalid signature', async () => {
       const { user1, safe, entryPoint } = await setupTests()
 
       await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('1.0') })
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('1.0'))
       const safeOp = buildSafeUserOpTransaction(
         await safe.getAddress(),
         user1.address,
@@ -38,38 +39,45 @@ describe('Safe4337Module - Existing Safe', () => {
       )
       const signature = buildSignatureBytes([await signHash(user1, ethers.keccak256('0xbaddad42'))])
       const userOp = buildUserOperationFromSafeUserOperation({ safeOp, signature })
-      await expect(entryPoint.executeUserOp(userOp, 0)).to.be.revertedWith('Signature validation failed')
+      await expect(entryPoint.handleOps([userOp], user1.address))
+        .to.be.revertedWithCustomError(entryPoint, 'FailedOp')
+        .withArgs(0, 'AA24 signature error')
+
       expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('1.0'))
     })
 
-    it('should execute contract calls without fee', async () => {
-      const { user1, safe, validator, entryPoint } = await setupTests()
+    it('should execute contract calls without a prefund required', async () => {
+      const { user1, safe, validator, entryPoint, relayer } = await setupTests()
 
-      await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('1.0') })
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('1.0'))
+      await entryPoint.depositTo(await safe.getAddress(), { value: ethers.parseEther('1.0') })
+
+      await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('0.5') })
       const safeOp = buildSafeUserOpTransaction(
         await safe.getAddress(),
         user1.address,
         ethers.parseEther('0.5'),
         '0x',
         '0',
         await entryPoint.getAddress(),
+        false,
+        false,
       )
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildUserOperationFromSafeUserOperation({ safeOp, signature })
-      await logGas('Execute UserOp without fee payment', entryPoint.executeUserOp(userOp, 0))
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.5'))
+      await logGas('Execute UserOp without a prefund payment', entryPoint.handleOps([userOp], relayer))
+      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0'))
     })
 
     it('should not be able to execute contract calls twice', async () => {
       const { user1, safe, validator, entryPoint } = await setupTests()
+      const receiver = ethers.Wallet.createRandom().address
 
       await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('1.0') })
       expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('1.0'))
       const safeOp = buildSafeUserOpTransaction(
         await safe.getAddress(),
-        user1.address,
+        receiver,
         ethers.parseEther('0.5'),
         '0x',
         '0',
@@ -78,19 +86,24 @@ describe('Safe4337Module - Existing Safe', () => {
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildUserOperationFromSafeUserOperation({ safeOp, signature })
-      await entryPoint.executeUserOp(userOp, 0)
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.5'))
-      await expect(entryPoint.executeUserOp(userOp, 0)).to.be.revertedWithCustomError(entryPoint, 'InvalidNonce').withArgs(0)
+      await entryPoint.handleOps([userOp], user1.address)
+      expect(await ethers.provider.getBalance(receiver)).to.be.eq(ethers.parseEther('0.5'))
+      await expect(entryPoint.handleOps([userOp], user1.address))
+        .to.be.revertedWithCustomError(entryPoint, 'FailedOp')
+        .withArgs(0, 'AA25 invalid account nonce')
     })
 
     it('should execute contract calls with fee', async () => {
       const { user1, safe, validator, entryPoint } = await setupTests()
+      const feeBeneficiary = ethers.Wallet.createRandom().address
+      const randomAddress = ethers.Wallet.createRandom().address
 
+      expect(await ethers.provider.getBalance(feeBeneficiary)).to.be.eq(0)
       await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('1.0') })
       expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('1.0'))
       const safeOp = buildSafeUserOpTransaction(
         await safe.getAddress(),
-        user1.address,
+        randomAddress,
         ethers.parseEther('0.5'),
         '0x',
         '0',
@@ -99,15 +112,19 @@ describe('Safe4337Module - Existing Safe', () => {
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildUserOperationFromSafeUserOperation({ safeOp, signature })
-      await logGas('Execute UserOp with fee payment', entryPoint.executeUserOp(userOp, ethers.parseEther('0.000001')))
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.499999'))
+      await logGas('Execute UserOp with fee payment', entryPoint.handleOps([userOp], feeBeneficiary))
+
+      // checking that the fee was paid
+      expect(await ethers.provider.getBalance(feeBeneficiary)).to.be.gt(0)
+      // check that the call was executed
+      expect(await ethers.provider.getBalance(randomAddress)).to.be.eq(ethers.parseEther('0.5'))
     })
 
     it('reverts on failure', async () => {
       const { user1, safe, validator, entryPoint } = await setupTests()
 
-      await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('0.000001') })
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.000001'))
+      // Make sure to send enough ETH for the pre-fund but less than the transaction
+      await entryPoint.depositTo(await safe.getAddress(), { value: ethers.parseEther('1.0') })
       const safeOp = buildSafeUserOpTransaction(
         await safe.getAddress(),
         user1.address,
@@ -119,16 +136,16 @@ describe('Safe4337Module - Existing Safe', () => {
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildUserOperationFromSafeUserOperation({ safeOp, signature })
+      const userOpHash = await entryPoint.getUserOpHash(userOp)
+      const expectedReturnData = validator.interface.encodeErrorResult('Error(string)', ['Execution failed'])
 
-      const transaction = await entryPoint.executeUserOp(userOp, ethers.parseEther('0.000001')).then((tx) => tx.wait())
-      const logs = transaction.logs.map((log) => entryPoint.interface.parseLog(log))
-      const emittedRevert = logs.some((l) => l?.name === 'UserOpReverted')
-
-      expect(emittedRevert).to.be.true
+      await expect(entryPoint.handleOps([userOp], user1.address))
+        .to.emit(entryPoint, 'UserOperationRevertReason')
+        .withArgs(userOpHash, userOp.sender, 0, expectedReturnData)
     })
 
     it('executeUserOpWithErrorString should execute contract calls', async () => {
-      const { user1, safe, validator, entryPoint } = await setupTests()
+      const { user1, safe, validator, entryPoint, relayer } = await setupTests()
 
       await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('1.0') })
       expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('1.0'))
@@ -141,11 +158,14 @@ describe('Safe4337Module - Existing Safe', () => {
         await entryPoint.getAddress(),
         false,
         true,
+        {
+          maxFeePerGas: '0',
+        },
       )
       const safeOpHash = calculateSafeOperationHash(await validator.getAddress(), safeOp, await chainId())
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildUserOperationFromSafeUserOperation({ safeOp, signature })
-      await logGas('Execute UserOp without fee payment and bubble up error string', entryPoint.executeUserOp(userOp, 0))
+      await logGas('Execute UserOp without fee payment and bubble up error string', entryPoint.handleOps([userOp], relayer))
       expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.5'))
     })
 
@@ -154,8 +174,8 @@ describe('Safe4337Module - Existing Safe', () => {
       const reverterContract = await ethers.getContractFactory('TestReverter').then((factory) => factory.deploy())
       const callData = reverterContract.interface.encodeFunctionData('alwaysReverting')
 
-      await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('0.000001') })
-      expect(await ethers.provider.getBalance(await safe.getAddress())).to.be.eq(ethers.parseEther('0.000001'))
+      await user1.sendTransaction({ to: await safe.getAddress(), value: ethers.parseEther('0.5') })
+
       const safeOp = buildSafeUserOpTransaction(
         await safe.getAddress(),
         await reverterContract.getAddress(),
@@ -170,12 +190,11 @@ describe('Safe4337Module - Existing Safe', () => {
       const signature = buildSignatureBytes([await signHash(user1, safeOpHash)])
       const userOp = buildUserOperationFromSafeUserOperation({ safeOp, signature })
 
-      const transaction = await entryPoint.executeUserOp(userOp, ethers.parseEther('0.000001')).then((tx) => tx.wait())
-      const logs = transaction.logs.map((log) => entryPoint.interface.parseLog(log)) ?? []
-      const emittedRevert = logs.find((l) => l?.name === 'UserOpReverted')
-      expect(emittedRevert?.args.reason).to.equal(
-        reverterContract.interface.encodeErrorResult('Error', ['You called a function that always reverts']),
-      )
+      const expectedRevertReason = validator.interface.encodeErrorResult('Error(string)', ['You called a function that always reverts'])
+
+      await expect(entryPoint.handleOps([userOp], user1.address))
+        .to.emit(entryPoint, 'UserOperationRevertReason')
+        .withArgs(await entryPoint.getUserOpHash(userOp), safeOp.safe, 0, expectedRevertReason)
     })
   })
 })