feat(sdk-core): wire eddsaMPCv2Callbacks into generateWalletWithExternalSigner

[bitgo-ai-agent-dev]

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]

WCI-917

VL-6590

[Marzooqa]

@claude

[github-actions]

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.