feat(sdk-core): add EddsaMPCv2Utils.createKeychainsWithExternalSigner#9199
Conversation
162bc79 to
35a5084
Compare
35a5084 to
8348006
Compare
|
Claude finished @Marzooqa's task —— View job Code Review AnalysisSince no specific instructions were provided, I'll conduct a comprehensive code review of the PR changes. Todo List:
SummaryThis PR adds Code Review Findings✅ Strengths
🔍 Areas for ConsiderationCritical Issues: None foundMinor Suggestions:
🧪 Test Quality AssessmentExcellent test coverage with realistic scenarios:
Test patterns follow established conventions from existing EdDSA MPCv2 tests. 🔒 Security AnalysisNo security issues identified:
📋 Code QualityHigh quality implementation:
Recommendation✅ APPROVE - This is a well-implemented feature that:
The minor suggestions above are optional improvements and don't block approval. Job run | Branch: wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner |
8348006 to
a6d8642
Compare
Add createKeychainsWithExternalSigner to EddsaMPCv2Utils to orchestrate EdDSA MPCv2 DKG via callbacks and WP round-trips, following the same external-signer pattern as EcdsaMPCv2Utils (ecdsaMPCv2.ts:655-735). The method: 1. Resolves the BitGo EdDSA MPCv2 GPG key (eddsaMpcv2PublicKey, not the ECDSA mpcv2PublicKey) and guards against a missing key. 2. Calls initializeCallback to let the external signer generate GPG keys. 3. Calls round1Callback for both parties' DKG round-1 messages. 4. Sends round-1 messages to WP via sendKeyGenerationRound1. 5. Calls round2Callback with BitGo's round-1 reply. 6. Sends round-2 messages to WP via sendKeyGenerationRound2, which returns commonPublicKeychain. 7. Calls finalizeCallback with BitGo's round-2 reply and asserts that the keychain returned by the callback matches commonPublicKeychain. 8. Registers user, backup, and BitGo keychains with isMPCv2: true. Messages are MPSTypes.MPSSignedMessage passed directly — no formatBitgoBroadcastMessage / formatP2PMessage wrapping needed (EdDSA MPS uses GPG detached signatures instead of DKLs auth-enc). Tests cover: callback invocation order, argument threading between rounds, isMPCv2 keychain registration, mismatched commonKeychain rejection, session-ID mismatch rejection, and missing BitGo GPG key rejection. Ticket: WCI-916 Session-Id: 52cc0308-5664-4de7-adc0-e1ddeebb9011 Task-Id: 489a58ea-0bff-4b3a-bd2e-8d32cda52a7b
a6d8642 to
78ad370
Compare
What
EddsaMPCv2Utils.createKeychainsWithExternalSignertomodules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.tsisMPCv2: trueWhy
EcdsaMPCv2Utils.createKeychainsWithExternalSigner)EddsaMPCv2KeyGenCallbackstype definitions (WCI-894, base branch) to actual WP API calls, enabling ME to supply a callback factory (WCI-895) and BitGoJS to route wallet generation through it (WCI-917)Implementation notes
eddsaMpcv2PublicKey(the Ed25519 GPG key) notmpcv2PublicKey(the secp256k1 key) — using the wrong key causes a WASM "Invalid Input" error in the DKG WASMcommonPublicKeychain(notcommonKeychain) — matches the existingcreateKeychainsusage at line 137MPSTypes.MPSSignedMessagepassed directly — noformatBitgoBroadcastMessage/formatP2PMessagewrapping (EdDSA MPS uses GPG detached signatures, not DKLs auth-enc)commonKeychainfromfinalizeCallbackmatchescommonPublicKeychainfrom WP before registering keychainsTest plan
nix develop /workspace/BitGoJS --command bash -c "cd /workspace/BitGoJS/modules/sdk-core && ../../node_modules/.bin/mocha --require tsx --timeout 20000 test/unit/bitgo/utils/tss/eddsa/eddsaMPCv2.ts --grep 'createKeychainsWithExternalSigner'"→ 7 passingtsc --noEmit)Ticket: WCI-916