safe-global / safe-modules / 7828420711

100%

master: 85%

push

github

Move P-256 signature verification to a separate contract, Refactor WebAuthn Signer to Allow Different Signature Verification Implementations (#223)

This PR:
- Implements https://github.com/safe-global/safe-modules/issues/219 and
https://github.com/safe-global/safe-modules/issues/218

Https://github.com/daimo-eth/p256-verifier heavily inspired the
implementation. However, I tried to reuse as much of the FCL code as
possible to make future upgrades easier.

This is the final contract hierarchy:

![Untitled-2024-02-05-1850](https://github.com/safe-global/safe-modules/assets/16622558/4ba44643-3399-4d56-aa00-929a65b7493f)

1. `P256Verifier.sol` is a `secp256r1` curve signature verifier; it
follows the interface defined in
https://eips.ethereum.org/EIPS/eip-7212.
2. `P256Wrapper.sol` is a wrapper that points to a verifier address (can
be later replaced with the precompile's address). It implements two
functions advised by the EIP: `verifySignatureWithoutMalleabilityCheck`
and `verifySignature`
3. `WebAuthnVerifier.sol` inherits from `P256Wrapper.sol` and adds a few
WebAuthn-specific things, like WebAuthn message generation and signature
verification with authenticator flags check.
4. `WebAuthnSigner.sol` and `WebAuthnSignerFactory.sol` call into the
`WebAuthnVerifier` to verify a signature. I hardcoded the authenticator
flag to be `0x04` (require user verification), compared to our
previously set `0x01` (user presence). More on that here:
https://developers.yubico.com/WebAuthn/WebAuthn_Developer_Guide/User_Presence_vs_User_Verification.html.
I believe UV makes more sense for the type of application we're
building.

**Benchmarks**:
- WebAuthnSigner codesize went from 8143 to 1610 bytes
- Used 1013491 gas for WebAuthn signer Safe deployment. Before: 2339762
(Numbers are non-deterministic)
- Used 378029 gas for WebAuthn signer Safe operation. Before: 375184
(Numbers are non-deterministic)

**Notes**:
I was torn between tw... (continued)

41 of 70 branches covered (0.0%)

Branch coverage included in aggregate %.

124 of 144 relevant lines covered (86.11%)

9.3 hits per line