diff --git a/modules/4337/package.json b/modules/4337/package.json index cec30279d..f9ac66cfe 100644 --- a/modules/4337/package.json +++ b/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", diff --git a/modules/passkey/contracts/test/BadP256Verfier.sol b/modules/passkey/contracts/test/BadP256Verfier.sol deleted file mode 100644 index 0c304c2be..000000000 --- a/modules/passkey/contracts/test/BadP256Verfier.sol +++ /dev/null @@ -1,34 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable no-complex-fallback */ -/* solhint-disable payable-fallback */ -pragma solidity ^0.8.0; - -import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; - -contract BadP256Verifier is IP256Verifier { - enum Behaviour { - WRONG_RETURNDATA_LENGTH, - INVALID_BOOLEAN_VALUE, - REVERT - } - - Behaviour public immutable BEHAVIOUR; - - constructor(Behaviour behaviour) { - BEHAVIOUR = behaviour; - } - - fallback(bytes calldata) external returns (bytes memory output) { - if (BEHAVIOUR == Behaviour.WRONG_RETURNDATA_LENGTH) { - output = abi.encode(true, 0); - } else if (BEHAVIOUR == Behaviour.INVALID_BOOLEAN_VALUE) { - output = abi.encode(-1); - } else if (BEHAVIOUR == Behaviour.REVERT) { - // solhint-disable-next-line no-inline-assembly - assembly ("memory-safe") { - mstore(0, 1) - revert(0, 32) - } - } - } -} diff --git a/modules/passkey/contracts/test/MockContractImport.sol b/modules/passkey/contracts/test/MockContractImport.sol deleted file mode 100644 index e010a132d..000000000 --- a/modules/passkey/contracts/test/MockContractImport.sol +++ /dev/null @@ -1,5 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable no-global-import */ -pragma solidity >=0.8.0; - -import "@safe-global/mock-contract/contracts/MockContract.sol"; diff --git a/modules/passkey/contracts/test/4337Dependencies.sol b/modules/passkey/contracts/test/TestDependencies.sol similarity index 79% rename from modules/passkey/contracts/test/4337Dependencies.sol rename to modules/passkey/contracts/test/TestDependencies.sol index 43668bcec..6dcb5c812 100644 --- a/modules/passkey/contracts/test/4337Dependencies.sol +++ b/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"; diff --git a/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol b/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol deleted file mode 100644 index 7fb6d96af..000000000 --- a/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol +++ /dev/null @@ -1,32 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable one-contract-per-file */ -pragma solidity ^0.8.0; - -import {IP256Verifier, P256} from "../libraries/P256.sol"; -import {WebAuthn} from "../libraries/WebAuthn.sol"; - -contract TestWebAuthnSignerFactory { - function createSigner(address verifier, uint256 x, uint256 y) external returns (TestWebAuthnSigner signer) { - signer = new TestWebAuthnSigner{salt: 0}(verifier, x, y); - } -} - -contract TestWebAuthnSigner { - using P256 for IP256Verifier; - - address private immutable _VERIFIER; - uint256 private immutable _X; - uint256 private immutable _Y; - - constructor(address verifier, uint256 x, uint256 y) { - _VERIFIER = verifier; - _X = x; - _Y = y; - } - - function isValidSignature(bytes32 hash, bytes calldata signature) external view returns (bytes4 magicValue) { - if (WebAuthn.verifySignature(hash, signature, WebAuthn.USER_VERIFICATION, _X, _Y, IP256Verifier(_VERIFIER))) { - magicValue = this.isValidSignature.selector; - } - } -} diff --git a/modules/passkey/test/libraries/P256.spec.ts b/modules/passkey/test/libraries/P256.spec.ts index 229dd324d..5b4e3a00b 100644 --- a/modules/passkey/test/libraries/P256.spec.ts +++ b/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 } }) }) diff --git a/modules/passkey/test/libraries/WebAuthn.spec.ts b/modules/passkey/test/libraries/WebAuthn.spec.ts index 7eb8c3e41..bb69b3933 100644 --- a/modules/passkey/test/libraries/WebAuthn.spec.ts +++ b/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 } }) diff --git a/modules/passkey/test/utils/webauthn.ts b/modules/passkey/test/utils/webauthn.ts index 2acdcbdc0..2da7a12ff 100644 --- a/modules/passkey/test/utils/webauthn.ts +++ b/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."`,