From a086c692781019f92171b670970212730038c683 Mon Sep 17 00:00:00 2001 From: Nicholas Rodrigues Lordello Date: Tue, 12 Mar 2024 17:00:36 +0100 Subject: [PATCH] Add Gas Benchmarking Tests for WebAuthn Signer --- .../contracts/WebAuthnSignerFactory.sol | 12 +-- .../passkey/contracts/test/Benchmarker.sol | 19 ++++ .../contracts/test/DummyP256Verifier.sol | 12 +++ modules/passkey/test/GasBenchmarking.spec.ts | 94 +++++++++++++++++++ .../passkey/test/MultipleVerifiers.spec.ts | 69 -------------- .../test/webauthn/WebAuthnShim.spec.ts | 4 +- 6 files changed, 128 insertions(+), 82 deletions(-) create mode 100644 modules/passkey/contracts/test/Benchmarker.sol create mode 100644 modules/passkey/contracts/test/DummyP256Verifier.sol create mode 100644 modules/passkey/test/GasBenchmarking.spec.ts delete mode 100644 modules/passkey/test/MultipleVerifiers.spec.ts diff --git a/modules/passkey/contracts/WebAuthnSignerFactory.sol b/modules/passkey/contracts/WebAuthnSignerFactory.sol index 0f2c72911..c74a77081 100644 --- a/modules/passkey/contracts/WebAuthnSignerFactory.sol +++ b/modules/passkey/contracts/WebAuthnSignerFactory.sol @@ -26,7 +26,7 @@ contract WebAuthnSignerFactory is ICustomECDSASignerFactory { function createSigner(uint256 x, uint256 y, address verifier) external returns (address signer) { signer = getSigner(x, y, verifier); - if (_hasNoCode(signer) && _validVerifier(verifier)) { + if (_hasNoCode(signer)) { WebAuthnSigner created = new WebAuthnSigner{salt: bytes32(0)}(x, y, verifier); require(address(created) == signer); } @@ -47,16 +47,6 @@ contract WebAuthnSignerFactory is ICustomECDSASignerFactory { } } - /** - * @dev Checks if the given verifier address contains code. - * @param verifier The address of the verifier to check. - * @return A boolean indicating whether the verifier contains code or not. - */ - function _validVerifier(address verifier) internal view returns (bool) { - // The verifier should contain code (The only way to implement a webauthn verifier is with a smart contract) - return !_hasNoCode(verifier); - } - /** * @dev Checks if the provided account has no code. * @param account The address of the account to check. diff --git a/modules/passkey/contracts/test/Benchmarker.sol b/modules/passkey/contracts/test/Benchmarker.sol new file mode 100644 index 000000000..15c504547 --- /dev/null +++ b/modules/passkey/contracts/test/Benchmarker.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity ^0.8.0; + +contract Benchmarker { + function call(address to, bytes memory data) external returns (uint256 gas, bytes memory returnData) { + gas = gasleft(); + + bool success; + (success, returnData) = to.call(data); + if (!success) { + // solhint-disable-next-line no-inline-assembly + assembly ("memory-safe") { + revert(add(returnData, 32), mload(returnData)) + } + } + + gas = gas - gasleft(); + } +} diff --git a/modules/passkey/contracts/test/DummyP256Verifier.sol b/modules/passkey/contracts/test/DummyP256Verifier.sol new file mode 100644 index 000000000..ef66fae13 --- /dev/null +++ b/modules/passkey/contracts/test/DummyP256Verifier.sol @@ -0,0 +1,12 @@ +// 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 DummyP256Verifier is IP256Verifier { + fallback(bytes calldata) external returns (bytes memory output) { + output = abi.encode(true); + } +} diff --git a/modules/passkey/test/GasBenchmarking.spec.ts b/modules/passkey/test/GasBenchmarking.spec.ts new file mode 100644 index 000000000..009c913c8 --- /dev/null +++ b/modules/passkey/test/GasBenchmarking.spec.ts @@ -0,0 +1,94 @@ +import { expect } from 'chai' +import { deployments, ethers } from 'hardhat' + +import { WebAuthnCredentials, decodePublicKey, encodeWebAuthnSignature } from './utils/webauthn' +import { IP256Verifier } from '../typechain-types' + +describe('Gas Benchmarking', function () { + const navigator = { + credentials: new WebAuthnCredentials(), + } + const credential = navigator.credentials.create({ + publicKey: { + rp: { + name: 'Safe', + id: 'safe.global', + }, + user: { + id: ethers.getBytes(ethers.id('chucknorris')), + name: 'chucknorris', + displayName: 'Chuck Norris', + }, + challenge: ethers.toBeArray(Date.now()), + pubKeyCredParams: [{ type: 'public-key', alg: -7 }], + }, + }) + + const setupTests = deployments.createFixture(async ({ deployments }) => { + const { DaimoP256Verifier, FCLP256Verifier, WebAuthnSignerFactory } = await deployments.fixture() + + const Benchmarker = await ethers.getContractFactory('Benchmarker') + const benchmarker = await Benchmarker.deploy() + + const factory = await ethers.getContractAt('WebAuthnSignerFactory', WebAuthnSignerFactory.address) + + const DummyP256Verifier = await ethers.getContractFactory('DummyP256Verifier') + const verifiers = { + fcl: await ethers.getContractAt('IP256Verifier', FCLP256Verifier.address), + daimo: await ethers.getContractAt('IP256Verifier', DaimoP256Verifier.address), + dummy: await DummyP256Verifier.deploy(), + } as Record + + return { benchmarker, factory, verifiers } + }) + + describe('WebAuthnSigner', () => { + it(`Benchmark signer deployment cost`, async function () { + const { benchmarker, factory } = await setupTests() + + const { x, y } = decodePublicKey(credential.response) + const verifier = `0x${'ee'.repeat(20)}` + + const [gas] = await benchmarker.call.staticCall(factory, factory.interface.encodeFunctionData('createSigner', [x, y, verifier])) + + console.log(` ⛽ deployment: ${gas}`) + }) + + for (const [name, key] of [ + ['FreshCryptoLib', 'fcl'], + ['daimo-eth', 'daimo'], + ['Dummy', 'dummy'], + ]) { + it(`Benchmark signer verification cost with ${name} verifier`, async function () { + const { benchmarker, verifiers, factory } = await setupTests() + + const challenge = ethers.id('hello world') + const assertion = navigator.credentials.get({ + publicKey: { + challenge: ethers.getBytes(challenge), + rpId: 'safe.global', + allowCredentials: [{ type: 'public-key', id: new Uint8Array(credential.rawId) }], + userVerification: 'required', + }, + }) + + const { x, y } = decodePublicKey(credential.response) + const verifier = verifiers[key] + + await factory.createSigner(x, y, verifier) + const signer = await ethers.getContractAt('WebAuthnSigner', await factory.getSigner(x, y, verifier)) + const signature = encodeWebAuthnSignature(assertion.response) + + const [gas, returnData] = await benchmarker.call.staticCall( + signer, + signer.interface.encodeFunctionData('isValidSignature(bytes32,bytes)', [challenge, signature]), + ) + + const [magicValue] = ethers.AbiCoder.defaultAbiCoder().decode(['bytes4'], returnData) + expect(magicValue).to.equal('0x1626ba7e') + + console.log(` ⛽ verification (${name}): ${gas}`) + }) + } + }) +}) diff --git a/modules/passkey/test/MultipleVerifiers.spec.ts b/modules/passkey/test/MultipleVerifiers.spec.ts deleted file mode 100644 index 6d0c0e7ff..000000000 --- a/modules/passkey/test/MultipleVerifiers.spec.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { expect } from 'chai' -import { deployments, ethers } from 'hardhat' - -import { WebAuthnCredentials, decodePublicKey, encodeWebAuthnSignature } from './utils/webauthn' -import { IP256Verifier } from '../typechain-types' - -describe('Multiple Verifiers', function () { - const navigator = { - credentials: new WebAuthnCredentials(), - } - const credential = navigator.credentials.create({ - publicKey: { - rp: { - name: 'Safe', - id: 'safe.global', - }, - user: { - id: ethers.getBytes(ethers.id('chucknorris')), - name: 'chucknorris', - displayName: 'Chuck Norris', - }, - challenge: ethers.toBeArray(Date.now()), - pubKeyCredParams: [{ type: 'public-key', alg: -7 }], - }, - }) - - const setupTests = deployments.createFixture(async ({ deployments }) => { - const { DaimoP256Verifier, FCLP256Verifier } = await deployments.fixture() - - const verifiers = { - fcl: await ethers.getContractAt('IP256Verifier', FCLP256Verifier.address), - daimo: await ethers.getContractAt('IP256Verifier', DaimoP256Verifier.address), - } as Record - - // TODO: Right now, we are using a test factory. However, once the canonical factory is - // introduced, we should port this test to use it instead. - const TestWebAuthnSignerFactory = await ethers.getContractFactory('TestWebAuthnSignerFactory') - const factory = await TestWebAuthnSignerFactory.deploy() - - return { verifiers, credential, factory } - }) - - for (const [name, key] of [ - ['FreshCryptoLib', 'fcl'], - ['daimo-eth', 'daimo'], - ]) { - it(`Should support the ${name} P-256 Verifier`, async function () { - const { verifiers, credential, factory } = await setupTests() - - const challenge = ethers.id('hello world') - const assertion = navigator.credentials.get({ - publicKey: { - challenge: ethers.getBytes(challenge), - rpId: 'safe.global', - allowCredentials: [{ type: 'public-key', id: new Uint8Array(credential.rawId) }], - }, - }) - - const verifier = verifiers[key] - const { x, y } = decodePublicKey(credential.response) - - const signer = await ethers.getContractAt('TestWebAuthnSigner', await factory.createSigner.staticCall(verifier, x, y)) - const signature = encodeWebAuthnSignature(assertion.response) - - await factory.createSigner(verifier, x, y) - expect(await signer.isValidSignature(challenge, signature)).to.be.equal('0x1626ba7e') - }) - } -}) diff --git a/modules/passkey/test/webauthn/WebAuthnShim.spec.ts b/modules/passkey/test/webauthn/WebAuthnShim.spec.ts index 1ac405dd2..d5f5ebfc9 100644 --- a/modules/passkey/test/webauthn/WebAuthnShim.spec.ts +++ b/modules/passkey/test/webauthn/WebAuthnShim.spec.ts @@ -31,7 +31,7 @@ describe('WebAuthn Shim', () => { } describe('navigator.credentials.create()', () => { - it('creates and verifies a new credential', async () => { + it('Should create and verify a new credential', async () => { const options = await generateRegistrationOptions({ rpName: rp.name, rpID: rp.id, @@ -79,7 +79,7 @@ describe('WebAuthn Shim', () => { }) describe('navigator.credentials.get()', () => { - it('authorises and verifies an existing credential', async () => { + it('Should authorise and verify an existing credential', async () => { const credential = navigator.credentials.create({ publicKey: { rp,