From bd8a18d43c40b6a15c861f631244d3f9aa479440 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Wed, 12 Feb 2025 03:29:41 -0800 Subject: [PATCH 1/7] chore: cast to uint256 before zero compare (#286) --- src/validators/WebAuthValidator.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/validators/WebAuthValidator.sol b/src/validators/WebAuthValidator.sol index 4ad5dbb2..804cf5e5 100644 --- a/src/validators/WebAuthValidator.sol +++ b/src/validators/WebAuthValidator.sol @@ -62,7 +62,7 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { upperKeyHalf[originDomain][msg.sender] = key32[1]; // we're returning true if this was a new key, false for update - bool keyExists = initialLowerHalf == 0 && initialUpperHalf == 0; + bool keyExists = uint256(initialLowerHalf) == 0 && uint256(initialUpperHalf) == 0; emit PasskeyCreated(msg.sender, originDomain); @@ -133,7 +133,7 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { pubkey[0] = lowerKeyHalf[origin][msg.sender]; pubkey[1] = upperKeyHalf[origin][msg.sender]; // This really only validates the origin is set - if (pubkey[0] == 0 || pubkey[1] == 0) { + if (uint256(pubkey[0]) == 0 || uint256(pubkey[1]) == 0) { return false; } From 3798afd103d187945ddf6898b24236b93aa403a2 Mon Sep 17 00:00:00 2001 From: Lyova Potyomkin Date: Wed, 12 Feb 2025 16:28:43 +0200 Subject: [PATCH 2/7] test: fix flaky passkey tests (#282) --- test/PasskeyModule.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/PasskeyModule.ts b/test/PasskeyModule.ts index ef5263bf..61e0263d 100644 --- a/test/PasskeyModule.ts +++ b/test/PasskeyModule.ts @@ -11,7 +11,7 @@ import { SmartAccount, Wallet } from "zksync-ethers"; import { SsoAccount__factory, WebAuthValidator, WebAuthValidator__factory } from "../typechain-types"; import { ContractFixtures, getProvider, getWallet, LOCAL_RICH_WALLETS, logInfo, RecordedResponse } from "./utils"; import { base64UrlToUint8Array } from "zksync-sso/utils"; -import { encodeAbiParameters, Hex, hexToBytes, toHex } from "viem"; +import { encodeAbiParameters, Hex, hexToBytes, toHex, pad } from "viem"; import { randomBytes } from "crypto"; import { parseEther, ZeroAddress } from "ethers"; @@ -428,7 +428,14 @@ async function validateSignatureTest( { name: "clientDataJson", type: "string" }, { name: "rs", type: "bytes32[2]" }, ], - [toHex(authData), sampleClientString, [toHex(rNormalization(generatedSignature.r)), toHex(sNormalization(generatedSignature.s))]] + [ + toHex(authData), + sampleClientString, + [ + pad(toHex(rNormalization(generatedSignature.r))), + pad(toHex(sNormalization(generatedSignature.s))) + ] + ] ) return await passkeyValidator.validateSignature(transactionHash, fatSignature); } From ee178e3bb7c64d34a6f610af966c8b0ca8f2e0a6 Mon Sep 17 00:00:00 2001 From: Lyova Potyomkin Date: Wed, 12 Feb 2025 17:12:56 +0200 Subject: [PATCH 3/7] docs: minor comment improvements (#290) --- src/AAFactory.sol | 1 + src/validators/SessionKeyValidator.sol | 3 ++- src/validators/WebAuthValidator.sol | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/AAFactory.sol b/src/AAFactory.sol index 17e537e7..5b946878 100644 --- a/src/AAFactory.sol +++ b/src/AAFactory.sol @@ -19,6 +19,7 @@ contract AAFactory { /// @dev The bytecode hash of the beacon proxy, used for deploying proxy accounts. bytes32 public immutable beaconProxyBytecodeHash; + /// @dev The address of the SsoBeacon contract used for the SSO accounts' beacon proxies. address public immutable beacon; /// @notice A mapping from unique account IDs to their corresponding deployed account addresses. diff --git a/src/validators/SessionKeyValidator.sol b/src/validators/SessionKeyValidator.sol index e2ecfa94..0365f82f 100644 --- a/src/validators/SessionKeyValidator.sol +++ b/src/validators/SessionKeyValidator.sol @@ -68,7 +68,8 @@ contract SessionKeyValidator is IModuleValidator { require(sessionCounter[msg.sender] == 0, "Revoke all keys first"); } - /// @notice This module should not be used to validate signatures + /// @notice This module should not be used to validate signatures (including EIP-1271), + /// as a signature by itself does not have enough information to validate it against a session. /// @return false function validateSignature(bytes32, bytes memory) external pure returns (bool) { return false; diff --git a/src/validators/WebAuthValidator.sol b/src/validators/WebAuthValidator.sol index 804cf5e5..d24f3bc8 100644 --- a/src/validators/WebAuthValidator.sol +++ b/src/validators/WebAuthValidator.sol @@ -19,6 +19,8 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { using JSONParserLib for JSONParserLib.Item; using JSONParserLib for string; + /// @dev P256Verify precompile implementation, as defined in RIP-7212, is found at + /// https://github.com/matter-labs/era-contracts/blob/main/system-contracts/contracts/precompiles/P256Verify.yul address private constant P256_VERIFIER = address(0x100); bytes1 private constant AUTH_DATA_MASK = 0x05; bytes32 private constant LOW_S_MAX = 0x7fffffff800000007fffffffffffffffde737d56d38bcf4279dce5617e3192a8; From 42ea74e127abae5922826dab906d92653463ead1 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Wed, 12 Feb 2025 07:16:29 -0800 Subject: [PATCH 4/7] chore: add more comments to webauthn verify (#285) Also move the test function to a test contract Co-authored-by: Lyova Potyomkin --- src/test/TestWebAuthValidator.sol | 20 +++++++ src/validators/WebAuthValidator.sol | 17 +++++- test/PasskeyModule.ts | 87 +++++++++++++++-------------- 3 files changed, 79 insertions(+), 45 deletions(-) create mode 100644 src/test/TestWebAuthValidator.sol diff --git a/src/test/TestWebAuthValidator.sol b/src/test/TestWebAuthValidator.sol new file mode 100644 index 00000000..21e1bbb1 --- /dev/null +++ b/src/test/TestWebAuthValidator.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import { WebAuthValidator } from "../validators/WebAuthValidator.sol"; + +contract WebAuthValidatorTest is WebAuthValidator { + /// @notice Verifies a message using the P256 curve. + /// @dev Useful for testing the P256 precompile + /// @param message The sha256 hash of the authenticator hash and hashed client data + /// @param rs The signature to validate (r, s) from the signed message + /// @param pubKey The public key to validate the signature against (x, y) + /// @return valid true if the signature is valid + function p256Verify( + bytes32 message, + bytes32[2] calldata rs, + bytes32[2] calldata pubKey + ) external view returns (bool valid) { + valid = rawVerify(message, rs, pubKey); + } +} diff --git a/src/validators/WebAuthValidator.sol b/src/validators/WebAuthValidator.sol index d24f3bc8..d7eb0e9d 100644 --- a/src/validators/WebAuthValidator.sol +++ b/src/validators/WebAuthValidator.sol @@ -22,6 +22,8 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { /// @dev P256Verify precompile implementation, as defined in RIP-7212, is found at /// https://github.com/matter-labs/era-contracts/blob/main/system-contracts/contracts/precompiles/P256Verify.yul address private constant P256_VERIFIER = address(0x100); + + // check for secure validation: bit 0 = 1 (user present), bit 2 = 1 (user verified) bytes1 private constant AUTH_DATA_MASK = 0x05; bytes32 private constant LOW_S_MAX = 0x7fffffff800000007fffffffffffffffde737d56d38bcf4279dce5617e3192a8; bytes32 private constant HIGH_R_MAX = 0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551; @@ -105,17 +107,19 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { fatSignature ); + // prevent signature replay https://yondon.blog/2019/01/01/how-not-to-use-ecdsa/ if (rs[0] <= 0 || rs[0] > HIGH_R_MAX || rs[1] <= 0 || rs[1] > LOW_S_MAX) { return false; } - // check if the flags are set + // https://developer.mozilla.org/en-US/docs/Web/API/Web_Authentication_API/Authenticator_data#attestedcredentialdata if (authenticatorData[32] & AUTH_DATA_MASK != AUTH_DATA_MASK) { return false; } - // parse out the important fields (type, challenge, origin, crossOrigin): https://goo.gl/yabPex + // parse out the required fields (type, challenge, crossOrigin): https://goo.gl/yabPex JSONParserLib.Item memory root = JSONParserLib.parse(clientDataJSON); + // challenge should contain the transaction hash, ensuring that the transaction is signed string memory challenge = root.at('"challenge"').value().decodeString(); bytes memory challengeData = Base64.decode(challenge); if (challengeData.length != 32) { @@ -125,11 +129,14 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { return false; } + // type ensures the signature was created from a validation request string memory type_ = root.at('"type"').value().decodeString(); if (!Strings.equal("webauthn.get", type_)) { return false; } + // the origin determines which key to validate against + // as passkeys are linked to domains, so the storage mapping reflects that string memory origin = root.at('"origin"').value().decodeString(); bytes32[2] memory pubkey; pubkey[0] = lowerKeyHalf[origin][msg.sender]; @@ -139,6 +146,10 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { return false; } + // cross-origin validation is optional, but explicitly not supported. + // cross-origin requests would be from embedding the auth request + // from another domain. The current SSO setup uses a pop-up instead of + // an i-frame, so we're rejecting these until the implemention supports it JSONParserLib.Item memory crossOriginItem = root.at('"crossOrigin"'); if (!crossOriginItem.isUndefined()) { string memory crossOrigin = crossOriginItem.value(); @@ -183,7 +194,7 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { bytes32 message, bytes32[2] calldata rs, bytes32[2] calldata pubKey - ) external view returns (bool valid) { + ) internal view returns (bool valid) { valid = callVerifier(P256_VERIFIER, message, rs, pubKey); } } diff --git a/test/PasskeyModule.ts b/test/PasskeyModule.ts index 61e0263d..d4708c7b 100644 --- a/test/PasskeyModule.ts +++ b/test/PasskeyModule.ts @@ -5,15 +5,15 @@ import { ECDSASigValue } from "@peculiar/asn1-ecc"; import { AsnParser } from "@peculiar/asn1-schema"; import { bigintToBuf, bufToBigint } from "bigint-conversion"; import { assert, expect } from "chai"; +import { randomBytes } from "crypto"; +import { parseEther, ZeroAddress } from "ethers"; import * as hre from "hardhat"; +import { encodeAbiParameters, Hex, hexToBytes, toHex, pad } from "viem"; import { SmartAccount, Wallet } from "zksync-ethers"; +import { base64UrlToUint8Array } from "zksync-sso/utils"; -import { SsoAccount__factory, WebAuthValidator, WebAuthValidator__factory } from "../typechain-types"; +import { SsoAccount__factory, WebAuthValidator, WebAuthValidator__factory, WebAuthValidatorTest, WebAuthValidatorTest__factory } from "../typechain-types"; import { ContractFixtures, getProvider, getWallet, LOCAL_RICH_WALLETS, logInfo, RecordedResponse } from "./utils"; -import { base64UrlToUint8Array } from "zksync-sso/utils"; -import { encodeAbiParameters, Hex, hexToBytes, toHex, pad } from "viem"; -import { randomBytes } from "crypto"; -import { parseEther, ZeroAddress } from "ethers"; /** * Decode from a Base64URL-encoded string to an ArrayBuffer. Best used when converting a @@ -36,6 +36,14 @@ async function deployValidator(wallet: Wallet): Promise { return WebAuthValidator__factory.connect(await validator.getAddress(), wallet); } +async function deployP256Tester(wallet: Wallet): Promise { + const deployer: Deployer = new Deployer(hre, wallet); + const passkeyValidatorArtifact = await deployer.loadArtifact("WebAuthValidatorTest"); + + const validator = await deployer.deploy(passkeyValidatorArtifact, []); + return WebAuthValidatorTest__factory.connect(await validator.getAddress(), wallet); +} + /** * COSE Keys * @@ -338,9 +346,6 @@ function encodeFatSignature( clientDataJSON: string; signature: string; }, - contracts: { - passkey: string; - }, ) { const signature = unwrapEC2Signature(base64UrlToUint8Array(passkeyResponse.signature)); return encodeAbiParameters( @@ -358,7 +363,7 @@ function encodeFatSignature( } async function rawVerify( - passkeyValidator: WebAuthValidator, + passkeyValidator: WebAuthValidatorTest, authenticatorData: string, clientData: string, b64SignedChallange: string, @@ -370,7 +375,7 @@ async function rawVerify( const rs = unwrapEC2Signature(toBuffer(b64SignedChallange)); const publicKeys = await getPublicKey(publicKeyEs256Bytes); - return await passkeyValidator.rawVerify(hashedData, rs, publicKeys); + return await passkeyValidator.p256Verify(hashedData, rs, publicKeys); } async function verifyKeyStorage( @@ -390,11 +395,11 @@ function encodeKeyFromHex(hexStrings: [Hex, Hex], domain: string) { // the same as the ethers: new AbiCoder().encode(["bytes32[2]", "string"], [bytes, domain]); return encodeAbiParameters( [ - { name: 'publicKeys', type: 'bytes32[2]' }, - { name: 'domain', type: 'string' }, + { name: "publicKeys", type: "bytes32[2]" }, + { name: "domain", type: "string" }, ], - [hexStrings, domain] - ) + [hexStrings, domain], + ); } function encodeKeyFromBytes(bytes: [Uint8Array, Uint8Array], domain: string) { @@ -428,15 +433,14 @@ async function validateSignatureTest( { name: "clientDataJson", type: "string" }, { name: "rs", type: "bytes32[2]" }, ], + [ + toHex(authData), + sampleClientString, [ - toHex(authData), - sampleClientString, - [ - pad(toHex(rNormalization(generatedSignature.r))), - pad(toHex(sNormalization(generatedSignature.s))) - ] + pad(toHex(rNormalization(generatedSignature.r))), + pad(toHex(sNormalization(generatedSignature.s))) ] - ) + ]); return await passkeyValidator.validateSignature(transactionHash, fatSignature); } @@ -487,7 +491,7 @@ describe("Passkey validation", function () { const receipt = await fundTx.wait(); expect(receipt.status).to.eq(1, "send funds to proxy account"); - return { passKeyModuleContract, sampleDomain, proxyAccountAddress, generatedR1Key, passKeyModuleAddress } + return { passKeyModuleContract, sampleDomain, proxyAccountAddress, generatedR1Key, passKeyModuleAddress }; } it("should deploy proxy account via factory", async () => { @@ -530,8 +534,8 @@ describe("Passkey validation", function () { ], [ toHex(authData), sampleClientString, - [toHex(normalizeR(generatedSignature.r)), toHex(normalizeS(generatedSignature.s))] - ]) + [toHex(normalizeR(generatedSignature.r)), toHex(normalizeS(generatedSignature.s))], + ]); const moduleSignature = encodeAbiParameters( [{ name: "signature", type: "bytes" }, { name: "moduleAddress", type: "address" }, { name: "validatorData", type: "bytes" }], @@ -539,7 +543,7 @@ describe("Passkey validation", function () { return moduleSignature; }, address: proxyAccountAddress, - secret: wallet.privateKey, //generatedR1Key.privateKey, + secret: wallet.privateKey, // generatedR1Key.privateKey, }, provider); const aaTransaction = { @@ -660,7 +664,6 @@ describe("Passkey validation", function () { clientDataJSON: ethersResponse.clientData, signature: ethersResponse.b64SignedChallenge, }, - { passkey: publicKeys[0] }, ); const initData = encodeKeyFromHex(publicKeys, "http://localhost:5173"); @@ -678,14 +681,14 @@ describe("Passkey validation", function () { // fully expand the raw validation to compare step by step describe("P256 precompile comparison", () => { it("should verify passkey", async function () { - const passkeyValidator = await deployValidator(wallet); + const passkeyValidator = await deployP256Tester(wallet); // 37 bytes const authenticatorData = "SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MFAAAABQ"; - const clientData = - "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiZFhPM3ctdWdycS00SkdkZUJLNDFsZFk1V2lNd0ZORDkiLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjUxNzMiLCJjcm9zc09yaWdpbiI6ZmFsc2UsIm90aGVyX2tleXNfY2FuX2JlX2FkZGVkX2hlcmUiOiJkbyBub3QgY29tcGFyZSBjbGllbnREYXRhSlNPTiBhZ2FpbnN0IGEgdGVtcGxhdGUuIFNlZSBodHRwczovL2dvby5nbC95YWJQZXgifQ"; - const b64SignedChallenge = - "MEUCIQCYrSUCR_QUPAhvRNUVfYiJC2JlOKuqf4gx7i129n9QxgIgaY19A9vAAObuTQNs5_V9kZFizwRpUFpiRVW_dglpR2A"; + const clientData + = "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiZFhPM3ctdWdycS00SkdkZUJLNDFsZFk1V2lNd0ZORDkiLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjUxNzMiLCJjcm9zc09yaWdpbiI6ZmFsc2UsIm90aGVyX2tleXNfY2FuX2JlX2FkZGVkX2hlcmUiOiJkbyBub3QgY29tcGFyZSBjbGllbnREYXRhSlNPTiBhZ2FpbnN0IGEgdGVtcGxhdGUuIFNlZSBodHRwczovL2dvby5nbC95YWJQZXgifQ"; + const b64SignedChallenge + = "MEUCIQCYrSUCR_QUPAhvRNUVfYiJC2JlOKuqf4gx7i129n9QxgIgaY19A9vAAObuTQNs5_V9kZFizwRpUFpiRVW_dglpR2A"; const verifyMessage = await rawVerify( passkeyValidator, @@ -698,7 +701,7 @@ describe("Passkey validation", function () { assert(verifyMessage == true, "valid sig"); }); it("should sign with new data", async function () { - const passkeyValidator = await deployValidator(wallet); + const passkeyValidator = await deployP256Tester(wallet); // The precompile expects the fully hashed data const preHashedData = await toHash( concat([toBuffer(ethersResponse.authenticatorData), await toHash(toBuffer(ethersResponse.clientData))]), @@ -725,7 +728,7 @@ describe("Passkey validation", function () { [generatedSignature.r, generatedSignature.s], [generatedX, generatedY], ); - const onChainGeneratedVerified = await passkeyValidator.rawVerify( + const onChainGeneratedVerified = await passkeyValidator.p256Verify( preHashedData, [generatedSignature.r, generatedSignature.s], [generatedX, generatedY], @@ -735,7 +738,7 @@ describe("Passkey validation", function () { [recordedR, recordedS], [recordedX, recordedY], ); - const onChainRecordedVerified = await passkeyValidator.rawVerify( + const onChainRecordedVerified = await passkeyValidator.p256Verify( preHashedData, [recordedR, recordedS], [recordedX, recordedY], @@ -748,7 +751,7 @@ describe("Passkey validation", function () { }); it("should verify other test passkey data", async function () { - const passkeyValidator = await deployValidator(wallet); + const passkeyValidator = await deployP256Tester(wallet); const verifyMessage = await rawVerify( passkeyValidator, @@ -762,10 +765,10 @@ describe("Passkey validation", function () { }); it("should fail when signature is bad", async function () { - const passkeyValidator = await deployValidator(wallet); + const passkeyValidator = await deployP256Tester(wallet); - const b64SignedChallenge = - "MEUCIQCYrSUCR_QUPAhvRNUVfYiJC2JlOKuqf4gx7i129n9QxgIgaY19A9vAAObuTQNs5_V9kZFizwRpUFpiRVW_dglpR2A"; + const b64SignedChallenge + = "MEUCIQCYrSUCR_QUPAhvRNUVfYiJC2JlOKuqf4gx7i129n9QxgIgaY19A9vAAObuTQNs5_V9kZFizwRpUFpiRVW_dglpR2A"; const verifyMessage = await rawVerify( passkeyValidator, ethersResponse.authenticatorData, @@ -906,10 +909,10 @@ describe("Passkey validation", function () { const partialClientObject = { challenge: "jBBiiOGt1aSBy1WAuRGxqU7YzRM5oWpMA9g8MKydjPI", }; - const duplicatedClientString = - JSON.stringify(sampleClientObject).slice(0, -1) + - "," + - JSON.stringify(partialClientObject).slice(1); + const duplicatedClientString + = JSON.stringify(sampleClientObject).slice(0, -1) + + "," + + JSON.stringify(partialClientObject).slice(1); const authData = toBuffer(ethersResponse.authenticatorData); const transactionHash = Buffer.from(sampleClientObject.challenge, "base64url"); const isValidSignature = await validateSignatureTest( From 8e3068450dc08cad4e6b35996846956e33e7f25d Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Wed, 12 Feb 2025 07:45:46 -0800 Subject: [PATCH 5/7] chore: update authors on contracts (#288) --- src/interfaces/IHook.sol | 4 ++-- src/managers/HookManager.sol | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/interfaces/IHook.sol b/src/interfaces/IHook.sol index 94172b9f..16519706 100644 --- a/src/interfaces/IHook.sol +++ b/src/interfaces/IHook.sol @@ -6,7 +6,7 @@ import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol import { IModule } from "./IModule.sol"; /// @title Validation hook interface for native AA -/// @author getclave.io +/// @author Initially getclave.io, updated by Matter Labs /// @notice Validation hooks trigger before each transaction, /// can be used to enforce additional restrictions on the account and/or transaction during the validation phase. interface IValidationHook is IModule, IERC165 { @@ -18,7 +18,7 @@ interface IValidationHook is IModule, IERC165 { } /// @title Execution hook interface for native AA -/// @author getclave.io +/// @author Initially getclave.io, updated by Matter Labs /// @notice Execution hooks trigger before and after each transaction, during the execution phase. interface IExecutionHook is IModule, IERC165 { /// @notice Hook that triggers before each transaction during the execution phase. diff --git a/src/managers/HookManager.sol b/src/managers/HookManager.sol index 4559843d..a3eb8e76 100644 --- a/src/managers/HookManager.sol +++ b/src/managers/HookManager.sol @@ -17,7 +17,7 @@ import { IModule } from "../interfaces/IModule.sol"; * @title Manager contract for hooks * @notice Abstract contract for managing the enabled hooks of the account * @dev Hook addresses are stored in a linked list - * @author https://getclave.io + * @author Initially https://getclave.io, then updated by Matter Labs */ abstract contract HookManager is IHookManager, SelfAuth { using EnumerableSet for EnumerableSet.AddressSet; From 344053a85e197bdee9ae4b46700879f39162595f Mon Sep 17 00:00:00 2001 From: Lyova Potyomkin Date: Wed, 12 Feb 2025 19:24:42 +0200 Subject: [PATCH 6/7] fix: rename error variable (#292) --- src/SsoAccount.sol | 4 ++-- src/handlers/ERC1271Handler.sol | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SsoAccount.sol b/src/SsoAccount.sol index 7ad8231b..17a0596f 100644 --- a/src/SsoAccount.sol +++ b/src/SsoAccount.sol @@ -200,9 +200,9 @@ contract SsoAccount is } if (_transaction.signature.length == 65) { - (address signer, ECDSA.RecoverError error) = ECDSA.tryRecover(_signedHash, _transaction.signature); + (address signer, ECDSA.RecoverError err) = ECDSA.tryRecover(_signedHash, _transaction.signature); return - signer == address(0) || error != ECDSA.RecoverError.NoError || !_k1IsOwner(signer) + signer == address(0) || err != ECDSA.RecoverError.NoError || !_k1IsOwner(signer) ? bytes4(0) : ACCOUNT_VALIDATION_SUCCESS_MAGIC; } diff --git a/src/handlers/ERC1271Handler.sol b/src/handlers/ERC1271Handler.sol index 9b45684f..2d12853d 100644 --- a/src/handlers/ERC1271Handler.sol +++ b/src/handlers/ERC1271Handler.sol @@ -31,9 +31,9 @@ abstract contract ERC1271Handler is IERC1271Upgradeable, EIP712("Sso1271", "1.0. */ function isValidSignature(bytes32 hash, bytes memory signature) external view override returns (bytes4 magicValue) { if (signature.length == 65) { - (address signer, ECDSA.RecoverError error) = ECDSA.tryRecover(hash, signature); + (address signer, ECDSA.RecoverError err) = ECDSA.tryRecover(hash, signature); return - signer == address(0) || error != ECDSA.RecoverError.NoError || !_k1IsOwner(signer) ? bytes4(0) : _ERC1271_MAGIC; + signer == address(0) || err != ECDSA.RecoverError.NoError || !_k1IsOwner(signer) ? bytes4(0) : _ERC1271_MAGIC; } (bytes memory decodedSignature, address validator) = SignatureDecoder.decodeSignatureNoHookData(signature); From faa4f88be8f6624acdb7538145ff155a44ed9738 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Thu, 13 Feb 2025 02:48:34 -0800 Subject: [PATCH 7/7] chore: make comments more exact when referring to internal data (#287) --- src/AAFactory.sol | 2 +- src/interfaces/IOwnerManager.sol | 4 ++-- src/managers/HookManager.sol | 2 +- src/managers/OwnerManager.sol | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/AAFactory.sol b/src/AAFactory.sol index 5b946878..22c8cae3 100644 --- a/src/AAFactory.sol +++ b/src/AAFactory.sol @@ -66,7 +66,7 @@ contract AAFactory { accountMappings[_uniqueAccountId] = accountAddress; - // Initialize the newly deployed account with validators, hooks and K1 owners. + // Initialize the newly deployed account with validators and K1 owners. ISsoAccount(accountAddress).initialize(_initialValidators, _initialK1Owners); emit AccountCreated(accountAddress, _uniqueAccountId); diff --git a/src/interfaces/IOwnerManager.sol b/src/interfaces/IOwnerManager.sol index 9f701141..f431a887 100644 --- a/src/interfaces/IOwnerManager.sol +++ b/src/interfaces/IOwnerManager.sol @@ -20,7 +20,7 @@ interface IOwnerManager { /** * @notice Adds a k1 owner to the list of k1 owners - * @dev Can only be called by self or a whitelisted module + * @dev Can only be called by self * @dev Address can not be the zero address * @param addr address - Address to add to the list of k1 owners */ @@ -28,7 +28,7 @@ interface IOwnerManager { /** * @notice Removes a k1 owner from the list of k1 owners - * @dev Can only be called by self or a whitelisted module + * @dev Can only be called by self * @param addr address - Address to remove from the list of k1 owners */ function k1RemoveOwner(address addr) external; diff --git a/src/managers/HookManager.sol b/src/managers/HookManager.sol index a3eb8e76..bdc35f06 100644 --- a/src/managers/HookManager.sol +++ b/src/managers/HookManager.sol @@ -16,7 +16,7 @@ import { IModule } from "../interfaces/IModule.sol"; /** * @title Manager contract for hooks * @notice Abstract contract for managing the enabled hooks of the account - * @dev Hook addresses are stored in a linked list + * @dev Hook addresses are stored in an enumerable set * @author Initially https://getclave.io, then updated by Matter Labs */ abstract contract HookManager is IHookManager, SelfAuth { diff --git a/src/managers/OwnerManager.sol b/src/managers/OwnerManager.sol index 8116dd81..a156f0e8 100644 --- a/src/managers/OwnerManager.sol +++ b/src/managers/OwnerManager.sol @@ -11,7 +11,7 @@ import { IOwnerManager } from "../interfaces/IOwnerManager.sol"; * @title Manager contract for owners * @notice Abstract contract for managing the owners of the account * @dev K1 Owners are secp256k1 addresses - * @dev Owners are stored in a linked list + * @dev Owners are stored in an enumerable set * @author https://getclave.io */ abstract contract OwnerManager is IOwnerManager, SelfAuth {