Cleanup Passkey Tests (#340)

This PR does some small cleanups for the Passkey tests.

Notably, #301 introduces a `MockContract` dependency, so the P-256
verifier tests were rewritten to use the general purpose mock contract
instead of the `BadP256Verifier` contract. Additionally, the
`TestWebAuthnSignerFactory` contract was lying around but not used and
no longer needed.

modules/4337/package.json

@@ -15,8 +15,8 @@
   ],
   "scripts": {
     "build": "npm run build:sol && npm run build:ts",
-    "build:ts": "rimraf dist && tsc",
     "build:sol": "rimraf build typechain-types && hardhat compile",
+    "build:ts": "rimraf dist && tsc",
     "test": "hardhat test --deploy-fixture",
     "test:4337": "4337-local-bundler-test",
     "test:4337:upstream": "USE_UPSTREAM_BUNDLER=1 4337-local-bundler-test",

modules/passkey/contracts/test/4337Dependencies.sol → modules/passkey/contracts/test/TestDependencies.sol

@@ -3,4 +3,5 @@
 pragma solidity >=0.8.0;
 
 import "@account-abstraction/contracts/interfaces/IEntryPoint.sol";
+import "@safe-global/mock-contract/contracts/MockContract.sol";
 import "@safe-global/safe-4337/contracts/test/TestStakedFactory.sol";

modules/passkey/test/libraries/P256.spec.ts

@@ -51,15 +51,20 @@ describe('P256', function () {
     const { r, s } = account.sign(message)
     const { x, y } = account.publicKey
 
-    const BadP256Verifier = await ethers.getContractFactory('BadP256Verifier')
-    const WRONG_RETURNDATA_LENGTH = 0
-    const INVALID_BOOLEAN_VALUE = 1
-    const REVERT = 2
-
-    for (const behaviour of [WRONG_RETURNDATA_LENGTH, INVALID_BOOLEAN_VALUE, REVERT]) {
-      const verifier = await BadP256Verifier.deploy(behaviour)
-      expect(await p256Lib.verifySignature(verifier, message, r, s, x, y)).to.be.false
-      expect(await p256Lib.verifySignatureAllowMalleability(verifier, message, r, s, x, y)).to.be.false
+    const MockContract = await ethers.getContractFactory('MockContract')
+    const mockVerifier = await MockContract.deploy()
+
+    for (const configureMock of [
+      // wrong return data length
+      () => mockVerifier.givenAnyReturn(ethers.AbiCoder.defaultAbiCoder().encode(['bool', 'uint256'], [true, 42])),
+      // invalid boolean value
+      () => mockVerifier.givenAnyReturnUint(ethers.MaxUint256),
+      // revert
+      () => mockVerifier.givenAnyRevert(),
+    ]) {
+      await configureMock()
+      expect(await p256Lib.verifySignature(mockVerifier, message, r, s, x, y)).to.be.false
+      expect(await p256Lib.verifySignatureAllowMalleability(mockVerifier, message, r, s, x, y)).to.be.false
     }
   })
 })

modules/passkey/test/libraries/WebAuthn.spec.ts

@@ -1,13 +1,12 @@
 import { expect } from 'chai'
 import { deployments, ethers } from 'hardhat'
-import hre from 'hardhat'
 import { DUMMY_CLIENT_DATA_FIELDS, base64UrlEncode, getSignatureBytes } from '../utils/webauthn'
 
 describe('WebAuthn Library', () => {
   const setupTests = deployments.createFixture(async () => {
     const WebAuthnLibFactory = await ethers.getContractFactory('TestWebAuthnLib')
     const webAuthnLib = await WebAuthnLibFactory.deploy()
-    const mockP256Verifier = await (await hre.ethers.getContractFactory('MockContract')).deploy()
+    const mockP256Verifier = await (await ethers.getContractFactory('MockContract')).deploy()
 
     return { webAuthnLib, mockP256Verifier }
   })

modules/passkey/test/utils/webauthn.ts

@@ -8,8 +8,8 @@
  */
 
 import { p256 } from '@noble/curves/p256'
-import { ethers, BytesLike } from 'ethers'
-import type { BigNumberish } from 'ethers'
+import { ethers } from 'ethers'
+import type { BigNumberish, BytesLike } from 'ethers'
 import CBOR from 'cbor'
 
 export interface CredentialCreationOptions {
@@ -384,6 +384,10 @@ export function encodeWebAuthnSignature(response: AuthenticatorAssertionResponse
   })
 }
 
+/**
+ * Dummy client data JSON fields. This can be used for gas estimations, as it pads the fields enough
+ * to account for variations in WebAuthn implementations.
+ */
 export const DUMMY_CLIENT_DATA_FIELDS = [
   `"origin":"http://safe.global"`,
   `"padding":"This pads the clientDataJSON so that we can leave room for additional implementation specific fields for a more accurate 'preVerificationGas' estimate."`,