feat(wallets): teach the approval loop the nested quorumApprovals shape - #1993
Conversation
Quorum entries in approvals.pending no longer throw: the loop signs the members this client holds (each member's own message, submitted under the member locator) and skips the rest without error, leaving threshold accounting to the API. The Solana ed25519 special case applies per member, and approvals carry their message only when one signer submits several signatures in a single call. Flat approval entries are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 571aab2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
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 |
Contributor
Prompt To Fix All With AI### Issue 1
packages/wallets/src/wallets/wallet.ts:1200-1222
**Quorum path nesting at 5+ levels — extract a helper**
The quorum branch inside `#collectApprovals` reaches six levels of indentation (class → method → for → if → inner-for → inner-if → `continue`). Per the team rule, deeply nested code (5+ levels) should be refactored into a helper method with an early-exit pattern to improve readability. Extracting this block into a private `#collectQuorumSignables` (or similar) that returns the matched `SignerAdapter`/`PendingMemberApproval` pairs would flatten `#collectApprovals` and make the quorum-vs-flat branching easier to follow at a glance.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(wallets): teach the approval loop t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Flattens #collectApprovals per the nesting rule: the quorum branch moves to #collectQuorumSignables with an early-continue in the caller. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Reviews (2): Last reviewed commit: "refactor(wallets): extract quorum member..." | Re-trigger Greptile |
…accept-a-quorum' into panos/wal-11291-m4-3-sdk-teach-the-approval-loop-the-nested-quorumapprovals
Contributor
|
Reviews (3): Last reviewed commit: "Merge branch 'panos/wal-11290-m4-2-sdk-w..." | Re-trigger Greptile |
Comment on lines
+1342
to
+1344
| // For Solana device signers (secp256r1), the SWIG precompile expects a signature | ||
| // over the keccak256 hash, which is provided in pendingApproval.message. | ||
| // For other Solana signers (ed25519), the full serialized transaction is signed. |
Contributor
There was a problem hiding this comment.
Let's not mention implementation details!
alberto-crossmint
approved these changes
Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linear: WAL-11291 (M4-3)
Stacked on #1989 (M4-2).
What
Per EDD §6.4,
#collectApprovalsinwallets/wallet.tsnow understands quorum entries inapprovals.pendinginstead of throwingQuorumSignerNotSupportedError(the#requireNonQuorumApprovalsguard is gone):message, throwInvalidSignerErrorif the signer isn't held.quorumApprovals.pendingand match each member's locator against the signers this client holds (active signer,additionalSigners, local-device fallback). A match signs that member's ownmessageand submits{ signer: <memberLocator>, signature }. No match → skip with an info log, don't error — holders sign independently, and a member that already submitted is no longer inpending, so re-approving is an idempotent no-op.onChain.transactioninstead of the keccak hash inmessage, device signers excepted) applies per member.messageper the EDD §6.4 submission contract; otherwisemessageis omitted and the wire format is unchanged. Note: the backend does not readmessageyet (SubmitApprovalV2025Schemais a non-strict Zod object, so the extra key is silently stripped, never a 400), and the scenario is unreachable at launch — this is forward-compatibility only.Approvalgains the optionalmessagefield (additive, minor-safe).Locator-parsing audit
Per the ticket (backend hit this bug class in Paella-Labs/crossbit-main#27842): the only SDK code parsing
approvals[].signerlocators is this loop — quorum entries are now handled before flat resolution, so nothing ever tries to resolve aquorum:<id>locator as a concrete signer.Out of scope (M4-4)
Selecting a quorum member as the wallet's active signer (
useSigner,withRecoverySigner, device-recovery resume) still throws the honest "not yet supported" errors. The loop is exercised today viaadditionalSignersand the device fallback.Testing
New
Wallet - quorum approval loopsuite (9 tests total in the file): member signs only its own message, signature + transaction paths, Solana ed25519 per-member case, non-member/already-submitted skips without erroring, flat entry still signed alongside a skipped quorum entry, andmessagedisambiguation. Fullpackages/walletssuite passes (687 tests), tsc clean.🤖 Generated with Claude Code