Skip to content

feat(sdk-core): add EddsaMPCv2Utils.createKeychainsWithExternalSigner#9199

Draft
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
masterfrom
wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner
Draft

feat(sdk-core): add EddsaMPCv2Utils.createKeychainsWithExternalSigner#9199
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
masterfrom
wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner

Conversation

@bitgo-ai-agent-dev

Copy link
Copy Markdown

What

  • Add EddsaMPCv2Utils.createKeychainsWithExternalSigner to modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts
  • The method orchestrates EdDSA MPCv2 DKG via callbacks (initialize → round1 → WP round1 → round2 → WP round2 → finalize) and registers the resulting user, backup, and BitGo keychains with isMPCv2: true
  • Add 7 unit tests covering callback invocation order, argument threading, keychain registration, and error paths (mismatched keychain, session-ID mismatch, missing BitGo GPG key)

Why

  • Part of the EdDSA MPCv2 external-signer key generation flow (WCI-888). The external signer pattern allows AWS Managed Wallets to own all WASM/GPG/DKG logic via offline callbacks while the SDK orchestrates the WP round-trips — matching the already-shipped ECDSA MPCv2 equivalent (EcdsaMPCv2Utils.createKeychainsWithExternalSigner)
  • This orchestrator is the binding layer that connects the EddsaMPCv2KeyGenCallbacks type 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

  • Uses eddsaMpcv2PublicKey (the Ed25519 GPG key) not mpcv2PublicKey (the secp256k1 key) — using the wrong key causes a WASM "Invalid Input" error in the DKG WASM
  • WP round-2 response field is commonPublicKeychain (not commonKeychain) — matches the existing createKeychains usage at line 137
  • Messages are MPSTypes.MPSSignedMessage passed directly — no formatBitgoBroadcastMessage / formatP2PMessage wrapping (EdDSA MPS uses GPG detached signatures, not DKLs auth-enc)
  • Asserts that commonKeychain from finalizeCallback matches commonPublicKeychain from WP before registering keychains

Test plan

  • Unit tests pass: 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 passing
  • Full eddsaMPCv2 test suite: 55 passing
  • TypeScript type check: no errors (tsc --noEmit)

Ticket: WCI-916

@linear-code

linear-code Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

WCI-916

@bitgo-ai-agent-dev bitgo-ai-agent-dev Bot force-pushed the wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner branch from 162bc79 to 35a5084 Compare July 7, 2026 17:40
Base automatically changed from wci-894-define-eddsa-mpcv2-keygen-callback-types to master July 8, 2026 06:39
@Marzooqa Marzooqa force-pushed the wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner branch from 35a5084 to 8348006 Compare July 8, 2026 08:48
@Marzooqa Marzooqa assigned Marzooqa and unassigned Vignesh-285 Jul 8, 2026
@Marzooqa

Marzooqa commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@claude

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Marzooqa's task —— View job


Code Review Analysis

Since no specific instructions were provided, I'll conduct a comprehensive code review of the PR changes.

Todo List:

  • Read repository guidelines (CLAUDE.md)
  • Review implementation changes in eddsaMPCv2.ts
  • Review test changes in eddsaMPCv2.ts test file
  • Analyze code quality and security
  • Provide comprehensive feedback

Summary

This PR adds EddsaMPCv2Utils.createKeychainsWithExternalSigner method with comprehensive test coverage. The implementation follows the existing ECDSA MPCv2 pattern and enables external signers to manage EdDSA MPCv2 key generation through callbacks.

Code Review Findings

Strengths

  1. Excellent Documentation: Implementation notes in PR clearly explain key design decisions (Ed25519 vs secp256k1 keys, field naming conventions)

  2. Comprehensive Test Coverage: 7 new unit tests covering happy path, error cases, and edge conditions:

    • Callback invocation order and argument threading
    • Keychain registration with isMPCv2: true
    • Session ID mismatch detection
    • Missing BitGo GPG key handling
    • Common keychain validation
  3. Security Best Practices:

    • Proper assertion of GPG key availability before use
    • Session ID validation between rounds
    • Common keychain validation in finalize step
    • Correct use of Ed25519 GPG keys (not secp256k1)
  4. Consistent Architecture: Mirrors existing EcdsaMPCv2Utils.createKeychainsWithExternalSigner pattern

🔍 Areas for Consideration

Critical Issues: None found

Minor Suggestions:

  1. Line 280: Consider using assert.strictEqual() instead of assert.equal() for session ID comparison to match the test pattern (line 289 uses strictEqual)

  2. Error Handling: The method propagates errors from callbacks naturally, but consider if more specific error messages would help with debugging callback implementation issues

  3. Type Safety: Good use of NonEmptyString.is() validation for GPG public keys

🧪 Test Quality Assessment

Excellent test coverage with realistic scenarios:

  • Integration-style tests: Tests use real MPS WASM operations and GPG operations
  • Callback verification: Tests verify exact arguments passed to each callback
  • Error path coverage: Comprehensive error scenarios including mismatched keychains and missing GPG keys
  • State validation: Tests verify keychain registration parameters and isMPCv2 flags

Test patterns follow established conventions from existing EdDSA MPCv2 tests.

🔒 Security Analysis

No security issues identified:

  • Proper GPG key validation before operations
  • Session ID integrity checks prevent round confusion
  • Common keychain validation prevents registration of mismatched keys
  • Uses Ed25519 GPG keys correctly (avoiding secp256k1 WASM errors)

📋 Code Quality

High quality implementation:

  • Clear variable naming and code structure
  • Appropriate error messages with context
  • Consistent with existing codebase patterns
  • Good separation of concerns (orchestration vs callback logic)

Recommendation

✅ APPROVE - This is a well-implemented feature that:

  • Follows established patterns and conventions
  • Has comprehensive test coverage
  • Implements proper security validations
  • Is well-documented and maintainable

The minor suggestions above are optional improvements and don't block approval.


Job run | Branch: wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner

@Marzooqa Marzooqa force-pushed the wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner branch from 8348006 to a6d8642 Compare July 8, 2026 10:06
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
@Marzooqa Marzooqa force-pushed the wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner branch from a6d8642 to 78ad370 Compare July 8, 2026 10:14
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.

2 participants