From 67cd5fc42f95a5be1f1e8c2cb3ae8421928e20a3 Mon Sep 17 00:00:00 2001 From: Panos Halios Date: Wed, 29 Jul 2026 10:54:55 -0400 Subject: [PATCH] refactor(wallets): extract shared quorum member matching into utils/quorum-members Introduces getQuorumMemberLocator/matchesQuorumMember and the ResolvedQuorumMember type, and rewires WalletFactory.isMatchingQuorumMember onto the shared helper (now also wiping derived key bytes after address comparison). No behavior change. Co-Authored-By: Claude Fable 5 --- packages/wallets/src/signers/types.ts | 18 ++- .../wallets/src/utils/quorum-members.test.ts | 118 ++++++++++++++++++ packages/wallets/src/utils/quorum-members.ts | 66 ++++++++++ .../wallets/src/wallets/wallet-factory.ts | 41 ++---- 4 files changed, 212 insertions(+), 31 deletions(-) create mode 100644 packages/wallets/src/utils/quorum-members.test.ts create mode 100644 packages/wallets/src/utils/quorum-members.ts diff --git a/packages/wallets/src/signers/types.ts b/packages/wallets/src/signers/types.ts index a6bb3e9d6..c0dca6463 100644 --- a/packages/wallets/src/signers/types.ts +++ b/packages/wallets/src/signers/types.ts @@ -121,6 +121,22 @@ export function isApiSourcedServerSignerConfig(config: { type: string }): config export type RecoverySignerConfigForChain = SignerConfigForChain | ApiSourcedServerSignerConfig; +/** + * A member of a resolved quorum admin signer. API-sourced members carry the identity fields + * (per-member `locator`, passkey `id`, server/external-wallet `address`); the index signature + * additionally admits runtime fields grafted from the caller's config (server `secret`, + * external-wallet `onSign`, passkey `onSignWithPasskey`), which the API cannot store. + */ +export type ResolvedQuorumMember = Record & { + type: string; + locator?: string; + address?: string; + email?: string; + phone?: string; + id?: string; + name?: string; +}; + /** * A quorum admin signer as **resolved by the API** (read side): members live under `signers` * and the derived `quorum:` locator is present. The create-side input counterpart is @@ -130,7 +146,7 @@ export type ResolvedQuorumRecoveryConfig = { type: "quorum"; threshold?: number; locator?: string; - signers: Array & { type: string }>; + signers: ResolvedQuorumMember[]; }; /** diff --git a/packages/wallets/src/utils/quorum-members.test.ts b/packages/wallets/src/utils/quorum-members.test.ts new file mode 100644 index 000000000..78a8d55cb --- /dev/null +++ b/packages/wallets/src/utils/quorum-members.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, it, vi } from "vitest"; +import type { ResolvedQuorumMember } from "../signers/types"; +import { getQuorumMemberLocator, matchesQuorumMember } from "./quorum-members"; + +const NO_SERVER_ADDRESSES = () => [] as string[]; + +describe("getQuorumMemberLocator", () => { + it.each([ + [ + "an API-sourced locator", + { type: "email", email: "other@x.com", locator: "email:alice@gmail.com" }, + "email:alice@gmail.com", + ], + ["a server address fallback", { type: "server", address: "0xServer" }, "server:0xServer"], + [ + "an email fallback with normalization", + { type: "email", email: "A.l.i.c.e@GMAIL.com" }, + "email:alice@gmail.com", + ], + ["an external-wallet fallback", { type: "external-wallet", address: "0xExt" }, "external-wallet:0xExt"], + ])("uses %s", (_name, member, expected) => { + expect(getQuorumMemberLocator(member as ResolvedQuorumMember)).toBe(expected); + }); +}); + +describe("matchesQuorumMember", () => { + it("rejects a candidate of a different type", () => { + expect( + matchesQuorumMember( + { type: "email", email: "alice@gmail.com" }, + { type: "phone", phone: "+15551234567" }, + NO_SERVER_ADDRESSES + ) + ).toBe(false); + }); + + describe("when the member is an email", () => { + const member: ResolvedQuorumMember = { type: "email", email: "alice@gmail.com" }; + + it.each([ + ["an exact address", "alice@gmail.com", true], + ["a denormalized Gmail address", "A.l.i.c.e@GMAIL.com", true], + ["a different address", "mallory@gmail.com", false], + ])("compares normalized emails: %s", (_name, email, expected) => { + expect(matchesQuorumMember({ type: "email", email }, member, NO_SERVER_ADDRESSES)).toBe(expected); + }); + }); + + describe("when the member is a phone number", () => { + it.each([ + ["the same number", "+15551234567", true], + ["a different number", "+15550000000", false], + ])("compares exactly: %s", (_name, phone, expected) => { + expect( + matchesQuorumMember( + { type: "phone", phone }, + { type: "phone", phone: "+15551234567" }, + NO_SERVER_ADDRESSES + ) + ).toBe(expected); + }); + }); + + describe("when the member is an external wallet", () => { + it.each([ + ["the same address", "0xAbC", true], + ["a different address", "0xDef", false], + ])("compares addresses exactly: %s", (_name, address, expected) => { + expect( + matchesQuorumMember( + { type: "external-wallet", address }, + { type: "external-wallet", address: "0xAbC" }, + NO_SERVER_ADDRESSES + ) + ).toBe(expected); + }); + }); + + describe("when the member is a passkey", () => { + const member: ResolvedQuorumMember = { type: "passkey", id: "pk-1", name: "primary" }; + + it.each([ + ["a matching id", { id: "pk-1" }, true], + ["a mismatching id even when the name matches", { id: "pk-9", name: "primary" }, false], + ["a matching name when no id is given", { name: "primary" }, true], + ["a mismatching name", { name: "backup" }, false], + ["no id and no name (permissive)", {}, true], + ])("matches by id, then name, then permissively: %s", (_name, fields, expected) => { + expect(matchesQuorumMember({ type: "passkey", ...fields }, member, NO_SERVER_ADDRESSES)).toBe(expected); + }); + }); + + describe("when the member is a server signer", () => { + const member: ResolvedQuorumMember = { type: "server", address: "0xDerived" }; + + it("matches when any derivable candidate address equals the member address", () => { + const candidates = vi.fn(() => ["0xPrimary", "0xDerived"]); + expect(matchesQuorumMember({ type: "server", secret: "s3cret" }, member, candidates)).toBe(true); + expect(candidates).toHaveBeenCalledWith({ type: "server", secret: "s3cret" }); + }); + + it("rejects when no derivable candidate address matches", () => { + expect(matchesQuorumMember({ type: "server", secret: "s3cret" }, member, () => ["0xOther"])).toBe(false); + }); + + it("rejects a member without an API address", () => { + expect( + matchesQuorumMember({ type: "server", secret: "s3cret" }, { type: "server" }, () => ["0xDerived"]) + ).toBe(false); + }); + + it("rejects a candidate without a secret", () => { + const candidates = vi.fn(() => ["0xDerived"]); + expect(matchesQuorumMember({ type: "server", address: "0xDerived" }, member, candidates)).toBe(false); + expect(candidates).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/wallets/src/utils/quorum-members.ts b/packages/wallets/src/utils/quorum-members.ts new file mode 100644 index 000000000..1fc95259c --- /dev/null +++ b/packages/wallets/src/utils/quorum-members.ts @@ -0,0 +1,66 @@ +import type { Chain } from "../chains/chains"; +import type { ResolvedQuorumMember, ServerSignerConfig, SignerConfigForChain } from "../signers/types"; +import { getSignerLocator } from "./signer-locator"; +import { normalizeEmail } from "./signer-validation"; + +/** + * Locator of a resolved quorum member. API-sourced members carry their locator; for members + * that don't (e.g. caller-supplied configs on the no-API-config fallback path), fall back to + * deriving one from the config fields. + */ +export function getQuorumMemberLocator(member: ResolvedQuorumMember): string { + if (typeof member.locator === "string") { + return member.locator; + } + if (member.type === "server" && member.address != null) { + return `server:${member.address}`; + } + return getSignerLocator(member as SignerConfigForChain) as string; +} + +/** + * Whether a caller-supplied signer config identifies the given quorum member. + * + * Matching is per-type, mirroring the existing-wallet quorum validation: + * - server: the candidate's derivable addresses (primary and legacy) must include the member's + * API address — secrets are never compared. Derivation is injected so callers can plug their + * own provider (factory: fresh derivation; wallet: `ServerSignerResolver.candidateAddresses`). + * - passkey: by `id` when given, else by `name`, else permissive (callers layer count-based + * disambiguation on top). + * - email: normalized comparison; phone: exact; external-wallet: exact address. + * Null-tolerant on candidate fields to preserve the factory's permissive semantics. + */ +export function matchesQuorumMember( + candidate: { type: string } & Record, + member: ResolvedQuorumMember, + serverCandidateAddresses: (config: ServerSignerConfig) => string[] +): boolean { + if (candidate.type !== member.type) { + return false; + } + if (candidate.type === "server") { + if (member.address == null || typeof (candidate as { secret?: unknown }).secret !== "string") { + return false; + } + return serverCandidateAddresses(candidate as unknown as ServerSignerConfig).includes(member.address); + } + if (candidate.type === "passkey") { + if (candidate.id != null) { + return member.id === candidate.id; + } + if (candidate.name != null) { + return member.name === candidate.name; + } + return true; + } + if (candidate.type === "email") { + return ( + candidate.email == null || + normalizeEmail(String(candidate.email)) === normalizeEmail(String(member.email ?? "")) + ); + } + if (candidate.type === "phone") { + return candidate.phone == null || candidate.phone === member.phone; + } + return candidate.address === member.address; +} diff --git a/packages/wallets/src/wallets/wallet-factory.ts b/packages/wallets/src/wallets/wallet-factory.ts index a9363c5b3..d153ec72d 100644 --- a/packages/wallets/src/wallets/wallet-factory.ts +++ b/packages/wallets/src/wallets/wallet-factory.ts @@ -20,13 +20,16 @@ import type { QuorumMemberConfigForChain, QuorumRecoveryConfig, RecoveryConfigForChain, + ServerSignerConfig, SignerConfigForChain, ResolvedRecoveryConfigForChain, } from "../signers/types"; import { isQuorumRecovery } from "../signers/types"; +import { matchesQuorumMember } from "../utils/quorum-members"; +import { secureWipe } from "../utils/secure-wipe"; import { Wallet } from "./wallet"; import type { WalletArgsFor, WalletCreateArgs } from "./types"; -import { compareSignerConfigs, normalizeEmail, normalizeValueForComparison } from "../utils/signer-validation"; +import { compareSignerConfigs, normalizeValueForComparison } from "../utils/signer-validation"; import { getSignerLocator } from "../utils/signer-locator"; import { deriveServerSignerDetails, deriveServerSignerCandidates } from "../signers/server"; import type { DeviceSignerKeyStorage } from "@/utils/device-signers/DeviceSignerKeyStorage"; @@ -516,40 +519,18 @@ export class WalletFactory { candidate: Record & { type: string }, chain: C ): boolean { - if (method.type !== candidate.type) { - return false; - } - if (method.type === "server") { - // User-supplied server members carry a secret; the API returns the derived address. + // User-supplied server members carry a secret; the API returns the derived address. + return matchesQuorumMember(method, candidate, (config: ServerSignerConfig) => { const { primary, legacy } = deriveServerSignerCandidates( - method, + config, chain, this.apiClient.projectId, this.apiClient.environment ); - return ( - candidate.address === primary.derivedAddress || - (legacy != null && candidate.address === legacy.derivedAddress) - ); - } - if (method.type === "passkey") { - if (method.id != null) { - return candidate.id === method.id; - } - if (method.name != null) { - return candidate.name === method.name; - } - return true; // field-level checks follow via compareSignerConfigs - } - if (method.type === "email") { - return ( - method.email == null || normalizeEmail(method.email) === normalizeEmail(String(candidate.email ?? "")) - ); - } - if (method.type === "phone") { - return method.phone == null || method.phone === candidate.phone; - } - return method.address === candidate.address; // external-wallet + const addresses = [primary.derivedAddress, ...(legacy != null ? [legacy.derivedAddress] : [])]; + secureWipe(primary.derivedKeyBytes, legacy?.derivedKeyBytes); + return addresses; + }); } private validateSigners(