Skip to content

feat(wallets): widen create-side recovery to accept a quorum object#1989

Open
panosinthezone wants to merge 2 commits into
panos/wal-11289-m4-1-sdk-regenerate-openapi-types-with-quorum-boundaryfrom
panos/wal-11290-m4-2-sdk-widen-create-side-recovery-to-accept-a-quorum
Open

feat(wallets): widen create-side recovery to accept a quorum object#1989
panosinthezone wants to merge 2 commits into
panos/wal-11289-m4-1-sdk-regenerate-openapi-types-with-quorum-boundaryfrom
panos/wal-11290-m4-2-sdk-widen-create-side-recovery-to-accept-a-quorum

Conversation

@panosinthezone

Copy link
Copy Markdown
Contributor

Summary

Implements WAL-11290 (M4-2, per EDD §6.2/§6.3 option B). Stacked on the M4-1 OpenAPI regen branch (WAL-11289).

  • Types: WalletCreateArgs.recovery now also accepts a quorum config { type: "quorum", threshold?, methods }; new QuorumMemberConfigForChain (members exclude api-key and device), QuorumRecoveryConfig, RecoveryConfigForChain, and isQuorumRecovery guard, all exported. threshold defaults to 1.
  • Creation: each methods entry runs through the existing per-type prep (passkey credential creation, server-signer address derivation) and the quorum is forwarded on the config.recovery wire property (member array mapped to the wire name signers). A 1-element methods collapses to a plain single admin; empty methods and invalid thresholds (non-integer, < 1, > methods.length) are rejected before any API call.
  • Existing-wallet validation: quorum arm compares threshold (?? 1 on both sides) and the member set order-insensitively with consume-once matching — server members via derived address (primary + legacy), emails Gmail-normalized, passkeys by id then name, compareSignerConfigs as the per-pair field backstop. Quorum-vs-single mismatches in either direction throw.
  • Crash guard: initDefaultSigner skips a quorum recovery — previously getSignerDescriptor("quorum") would throw after the wallet was created on-chain. The approval loop is untouched (separate ticket; still throws QuorumSignerNotSupportedError).
  • react-base: one param-type widening (initializeWebViewIfNeeded) to keep compilation; runtime quorum handling (WebView init, email population) is audited in its own ticket.

Everything is additive: single-signer create payloads are byte-for-byte unchanged (covered by a regression assertion).

Notes for review

  • The quorum config carries a required type: "quorum" discriminant (the EDD surface is the bare { threshold, methods }); if preferred, we can split into an external type without type and an internal one with it.
  • The response DTO doesn't model a quorum config.adminSigner yet, so the validation arm reads it through a runtime-tolerant cast per EDD §5.1.

Testing

  • 13 new unit tests in wallet-factory.test.ts: wire shape (prepared server/passkey members, threshold omission), single-method collapse, empty-methods and threshold rejections, and existing-wallet validation (out-of-order match, undefined1 threshold, threshold/member/count mismatches, quorum↔single mismatches, Gmail-dot normalization).
  • Full @crossmint/wallets-sdk suite: 673 passed.
  • tsc --noEmit clean; wallets-sdk, react-base, react-ui, and react-native all build including DTS.

🤖 Generated with Claude Code

Widen WalletCreateArgs.recovery to also accept a quorum config
({ type: "quorum", threshold?, methods }) whose members exclude api-key
and device signers. createWallet preps each member through the existing
per-type prep (passkey credential creation, server-signer address
derivation) and forwards the quorum on the `recovery` wire property;
a single-method quorum collapses to a plain admin signer and empty
methods are rejected. Existing-wallet validation compares threshold and
member set order-insensitively. Single-signer payloads are unchanged.

WAL-11290

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8154d93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@crossmint/wallets-sdk Minor
@crossmint/client-sdk-react-base Patch
@crossmint/wallets-quickstart-devkit Patch
@crossmint/wallets-playground-react Patch
@crossmint/client-sdk-react-native-ui Patch
@crossmint/client-sdk-react-ui Patch
@crossmint/wallets-playground-expo Patch
@crossmint/auth-ssr-nextjs-demo Patch
@crossmint/client-sdk-nextjs-starter Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/client/react-base/src/providers/CrossmintWalletBaseProvider.tsx:255-262
**WebView init skipped for quorum email/phone members**

`initializeWebViewIfNeeded` only checks the top-level `signer.type`. When the recovery is `{ type: "quorum", methods: [{ type: "email", ... }] }`, the outer type is `"quorum"` — neither `"email"` nor `"phone"` — so `initializeWebView` is never called. Any email/phone OTP flow inside a quorum recovery will then try to use the WebView frame before it has been initialized, likely throwing at runtime. The PR description acknowledges this is deferred to a follow-up ticket, but worth tracking explicitly so the ticket includes walking `methods` for quorum inputs.

### Issue 2 of 2
packages/wallets/src/wallets/wallet-factory.ts:498-506
**Passkey fallback returns `true` for any candidate when neither `id` nor `name` is set**

When a passkey member has neither `id` nor `name`, `isMatchingQuorumMember` returns `true` for the first passkey found in `unmatched`. With multiple passkey members all lacking identifiers, the consume-once greedy loop will pair them with existing passkeys in list order rather than by content, so `compareSignerConfigs` may be called on a mis-matched pair. In practice `compareSignerConfigs` is then the sole safety net; the pre-selection step provides no additional guarantee. Consider throwing (or at least logging a warning) when a quorum contains ≥ 2 passkey members without identifiers, similar to how `isMatchingPasskeySigner` already throws for the delegated-signer case.

Reviews (1): Last reviewed commit: "feat(wallets): widen create-side recover..." | Re-trigger Greptile

Comment thread packages/wallets/src/wallets/wallet-factory.ts
@panosinthezone panosinthezone self-assigned this Jul 24, 2026
@greptile-apps

greptile-apps Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/wallets/src/wallets/wallet-factory.ts:252-255
Collapsed single-member quorum loses runtime data for `server`/`external-wallet` members.

`createArgs.recovery.type` is always `"quorum"` here when the user passes a quorum config, so the `"server" || "external-wallet"` guard never fires even after `normalizeRecovery` already collapsed the quorum to its sole member. The wallet instance is therefore built from `apiRecovery` — a server signer without the secret, or an external-wallet signer without the `onSign` callback — causing all subsequent recovery operations to fail with "no secret available" / no signing function.

```suggestion
        // Normalize before the type check so that a single-member quorum that was collapsed
        // to a server or external-wallet signer still preserves its runtime data (secret / onSign).
        const normalizedRecovery =
            createArgs.recovery != null ? this.normalizeRecovery(createArgs.recovery) : undefined;
        const recovery =
            normalizedRecovery?.type === "server" || normalizedRecovery?.type === "external-wallet"
                ? normalizedRecovery
                : apiRecovery;
```

Reviews (2): Last reviewed commit: "type fixes" | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant