Skip to content

feat(sdk-core): wire eddsaMPCv2Callbacks into generateWalletWithExternalSigner#9200

Draft
bitgo-ai-agent-dev[bot] wants to merge 2 commits into
wci-916-add-eddsampcv2utils-createkeychainswithexternalsignerfrom
wci-917-wire-eddsampcv2callbacks-into-generate-wallet-with-external-signer
Draft

feat(sdk-core): wire eddsaMPCv2Callbacks into generateWalletWithExternalSigner#9200
bitgo-ai-agent-dev[bot] wants to merge 2 commits into
wci-916-add-eddsampcv2utils-createkeychainswithexternalsignerfrom
wci-917-wire-eddsampcv2callbacks-into-generate-wallet-with-external-signer

Conversation

@bitgo-ai-agent-dev

Copy link
Copy Markdown

What

  • Add eddsaMPCv2Callbacks?: EddsaMPCv2KeyGenCallbacks to GenerateWalletWithExternalSignerOptions in iWallets.ts
  • Update hasMpcCallbacks in wallets.ts to include eddsaMPCv2Callbacks so createKeychainCallback + MPCv2 callbacks is rejected
  • Add EdDSA MPCv2 routing branch in generateMpcWalletWithExternalSigner: dispatches to EddsaMPCv2Utils.createKeychainsWithExternalSigner when eddsaMPCv2Callbacks is set; rejects if both eddsaMPCv2Callbacks and eddsaCallbacks are provided; falls through to the existing MPCv1 path otherwise
  • Add tests covering: routing to EddsaMPCv2Utils, mutual-exclusion rejection, no-callbacks rejection, MPCv1 path unchanged

Why

  • EdDSA MPCv2 wallet generation (Silence Labs DKG) requires its own key generation callbacks distinct from the existing MPCv1 path
  • This wires the newly-defined EddsaMPCv2KeyGenCallbacks type (WCI-894) into the external signer flow so consumers can opt into MPCv2 by passing eddsaMPCv2Callbacks
  • Part of the EdDSA upgrade from Zengo to Silence Labs implementation (WCI-888)

Dependencies

  • Base branch wci-894-define-eddsa-mpcv2-keygen-callback-types contains the EddsaMPCv2KeyGenCallbacks type definitions (WCI-894)
  • EddsaMPCv2Utils.createKeychainsWithExternalSigner is added by WCI-916; the implementation uses an as any cast until that lands and the type annotation will resolve when these branches are merged together

Test plan

  • All 474 tests in walletsExternalSigner.ts pass
  • New generateWalletWithExternalSigner - EdDSA MPCv2 TSS describe block (7 tests) all pass
  • eddsaMPCv2Callbacks routes to EddsaMPCv2Utils.createKeychainsWithExternalSigner with correct params
  • eddsaMPCv2Callbacks + eddsaCallbacks together is rejected
  • No callbacks for EdDSA coin is rejected with the existing MPCv1 error
  • Providing only eddsaCallbacks still routes to the MPCv1 EddsaUtils path (not MPCv2)
  • createKeychainCallback + eddsaMPCv2Callbacks together is rejected

Ticket: WCI-917

@linear-code

linear-code Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

WCI-917

@bitgo-ai-agent-dev bitgo-ai-agent-dev Bot force-pushed the wci-917-wire-eddsampcv2callbacks-into-generate-wallet-with-external-signer branch from 7997d9a to 5a10bf9 Compare July 7, 2026 18:18
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-917-wire-eddsampcv2callbacks-into-generate-wallet-with-external-signer branch from 5a10bf9 to 8cd823e Compare July 8, 2026 07:31
@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 for EdDSA MPCv2 Callbacks Integration

Todo List:

  • Read and analyze the interface changes in iWallets.ts
  • Review the implementation logic in wallets.ts
  • Examine test coverage in walletsExternalSigner.ts
  • Check for potential issues and provide feedback
  • Verify alignment with PR requirements

Review Summary

This PR successfully implements EdDSA MPCv2 callback support in the external signer flow. The implementation is well-structured, includes comprehensive test coverage, and maintains backward compatibility while adding the new MPCv2 functionality.

✅ Strengths

  1. Clean Interface Extension: The addition of eddsaMPCv2Callbacks?: EddsaMPCv2KeyGenCallbacks to GenerateWalletWithExternalSignerOptions in iWallets.ts:275 is properly typed and follows the existing pattern.

  2. Robust Validation Logic: The implementation in wallets.ts:739 correctly updates hasMpcCallbacks to include the new callbacks, ensuring proper mutual exclusion validation with createKeychainCallback.

  3. Comprehensive Error Handling: The code includes appropriate error cases:

    • Mutual exclusion between eddsaMPCv2Callbacks and eddsaCallbacks (wallets.ts:1805-1809)
    • Validation that at least one EdDSA callback type is provided for EdDSA coins
    • Prevention of mixing onchain and MPC callback types
  4. Excellent Test Coverage: The test suite includes 7 comprehensive tests covering:

    • Routing to EddsaMPCv2Utils.createKeychainsWithExternalSigner
    • Mutual exclusion validation
    • Fallback to MPCv1 when only eddsaCallbacks provided
    • Integration with hasMpcCallbacks validation
  5. Forward-Compatible Implementation: The use of as any cast with a clear comment explaining dependency on WCI-916 is a reasonable approach for staged development.

⚠️ Minor Considerations

  1. Type Safety: The as any cast on line wallets.ts:1813 is documented as temporary but creates a type hole. Consider adding a more specific type assertion or interface to maintain type safety until WCI-916 merges.

  2. Code Organization: The nested if-else structure in generateMpcWalletWithExternalSigner (lines 1805-1825) could benefit from early returns or guard clauses for improved readability:

    if (params.eddsaMPCv2Callbacks && params.eddsaCallbacks) {
      throw new Error('...');
    }
    
    if (params.eddsaMPCv2Callbacks) {
      // EdDSA MPCv2 path
      return await new EDDSAUtils.EddsaMPCv2Utils(...)...;
    }
    
    if (!params.eddsaCallbacks) {
      throw new Error('...');
    }
    
    // EdDSA MPCv1 path
    return await new EDDSAUtils.default(...)...;

🔍 Technical Analysis

  • Dependency Management: The PR correctly handles the temporary dependency on WCI-916 through conditional logic and clear documentation.
  • Backward Compatibility: Existing MPCv1 flows remain unchanged, ensuring no breaking changes.
  • API Design: The optional callback parameters provide a clean upgrade path for consumers.
  • Error Messages: Clear, actionable error messages guide developers toward correct usage.

✅ Requirements Alignment

All stated requirements are properly implemented:

  • ✅ Added eddsaMPCv2Callbacks to interface
  • ✅ Updated hasMpcCallbacks validation
  • ✅ Added EdDSA MPCv2 routing branch with mutual exclusion
  • ✅ Comprehensive test coverage (474 existing + 7 new tests)

Recommendation: APPROVE

The implementation is solid, well-tested, and ready for merge. The code follows established patterns, maintains backward compatibility, and includes excellent test coverage. The temporary type assertion is properly documented and acceptable given the staged development approach.

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 changed the base branch from master to wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner July 8, 2026 08:54
…nalSigner

Add eddsaMPCv2Callbacks field to GenerateWalletWithExternalSignerOptions
and add routing in generateMpcWalletWithExternalSigner to dispatch to
EddsaMPCv2Utils.createKeychainsWithExternalSigner when the field is set.

Changes:
- iWallets.ts: add eddsaMPCv2Callbacks?: EddsaMPCv2KeyGenCallbacks to
  GenerateWalletWithExternalSignerOptions
- wallets.ts: update hasMpcCallbacks to include eddsaMPCv2Callbacks; add
  an EdDSA MPCv2 branch that calls EddsaMPCv2Utils; guard against passing
  both eddsaMPCv2Callbacks and eddsaCallbacks simultaneously
- walletsExternalSigner.ts: new test block covering routing to
  EddsaMPCv2Utils, mutual-exclusion rejection, no-callbacks rejection,
  and MPCv1 path unchanged verification

Ticket: WCI-917
Session-Id: 23a5c77a-c909-407f-bf3e-a820c9e88aea
Task-Id: 38b68db1-6599-4911-9635-d408663b0d82
@Marzooqa Marzooqa force-pushed the wci-917-wire-eddsampcv2callbacks-into-generate-wallet-with-external-signer branch from 8cd823e to 417f154 Compare July 8, 2026 08:57
@Marzooqa Marzooqa force-pushed the wci-916-add-eddsampcv2utils-createkeychainswithexternalsigner branch 2 times, most recently 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.

1 participant