feat(sdk-coin-sol): add MPCv2 support to recoverConsolidations

[bitgo-ai-agent-dev]

What

• Detect MPCv2 once at the top of recoverConsolidations via EDDSAUtils.isEddsaMpcV1SigningMaterial instead of per-iteration keycard decryption in recover()
• For MPCv2 wallets, derive the base destination address (index 0) using deriveUnhardenedMps instead of MPC.deriveUnhardened, preventing funds from being swept to an inaccessible address
• Thread multisigTypeVersion: 'MPCv2' as const into each recover() call so recover() short-circuits isMpcv2SigningMaterial and skips redundant detection on every loop iteration
• Add multisigTypeVersion?: 'MPCv2' to SolRecoveryOptions interface to support pre-resolved route threading
• Add unit tests: MPCv2 signed native SOL consolidation, MPCv2 signed SPL token consolidation, single-detection assertion (isMpcv2SigningMaterial not called from recover()), unsigned cold path regression, MPCv1 regression

Why

• Without this change, an MPCv2 wallet calling recoverConsolidations would have its funds swept to an address derived via legacy MPC.deriveUnhardened — the wrong address for MPCv2, making the funds unrecoverable without a separate recovery operation
• The per-iteration keycard decryption in recover() (decrypting the same keycard N times across the scan loop) is wasteful and slow for wallets scanning many indexes

Test plan

• yarn run unit-test --scope @bitgo/sdk-coin-sol — all 606 tests pass
• eslint src/sol.ts test/unit/sol.ts — 0 errors
• TypeScript type check — no errors

Ticket: WCI-496

[linear-code]

WCI-496

[vibhavgo]

@claude

[github-actions]

Claude finished @vibhavgo's task —— View job

---

Code Review: MPCv2 Support for recoverConsolidations

• Read repository guidelines (CLAUDE.md)
• Examine main implementation changes in sol.ts
• Review test additions and coverage
• Analyze security and correctness
• Check for performance improvements
• Provide comprehensive review feedback

Summary

This PR successfully implements MPCv2 support for recoverConsolidations with significant performance optimizations. The implementation is well-structured and includes comprehensive test coverage.

Positive Aspects

✅ Excellent Performance Optimization

The core optimization in recoverConsolidations (sol.ts:1798-1800) detects MPCv2 once at the function start using EDDSAUtils.isEddsaMpcV1SigningMaterial instead of calling isMpcv2SigningMaterial on every loop iteration. This eliminates redundant keycard decryption operations.

✅ Correct MPCv2 Address Derivation

Lines 1806-1808 properly use deriveUnhardenedMps for MPCv2 instead of the legacy MPC.deriveUnhardened, preventing funds from being swept to inaccessible addresses.

✅ Smart Threading Pattern

The multisigTypeVersion: 'MPCv2' as const parameter (sol.ts:1822) threads the pre-resolved detection result to recover(), which then short-circuits further detection (sol.ts:1234-1237).

✅ Comprehensive Test Coverage

The 336 lines of new tests cover all critical scenarios:

• MPCv2 signed native SOL consolidation
• MPCv2 signed SPL token consolidation
• Assertion that isMpcv2SigningMaterial is never called when pre-resolved
• Unsigned cold path regression testing
• MPCv1 backward compatibility

✅ Type Safety

The addition of multisigTypeVersion?: 'MPCv2' to SolRecoveryOptions (sol.ts:191) provides proper type safety for the threading mechanism.

Technical Implementation Review

Address Derivation Logic (sol.ts:1806-1808)

const baseAccountId = isMpcV2
  ? deriveUnhardenedMps(bitgoKey, baseAddressPath).slice(0, 64)
  : (await EDDSAMethods.getInitializedMpcInstance()).deriveUnhardened(bitgoKey, baseAddressPath).slice(0, 64);

✅ Correct: This ensures MPCv2 wallets derive addresses using the proper cryptographic method.

Performance Optimization (sol.ts:1234-1237)

const isMpcV2 =
  params.multisigTypeVersion === 'MPCv2'
    ? true
    : await this.isMpcv2SigningMaterial(params.userKey, params.backupKey, params.walletPassphrase);

✅ Excellent: The conditional check prevents expensive keycard decryption when the result is already known.

Edge Case Handling

✅ The unsigned sweep scenario (!params.walletPassphrase) correctly defaults isMpcV2 to false since cold MPCv2 is out of scope.

Minor Observations

Code Style

• The comments are clear and explain the rationale well
• Variable naming is consistent (isMpcV2, baseAccountId)
• Proper use of TypeScript const assertions

Error Handling

• Existing error handling patterns are preserved
• No new error conditions introduced

Backward Compatibility

• MPCv1 wallets continue to work unchanged
• Unsigned sweeps maintain legacy behavior
• No breaking changes to existing interfaces

Test Quality Assessment

The test suite demonstrates excellent engineering practices:

1. Isolation: Uses proper sinon sandboxes and stubs
2. Coverage: Tests both positive paths and edge cases
3. Assertions: Specifically verifies performance optimization (spy on isMpcv2SigningMaterial)
4. Regression Protection: Ensures unsigned consolidations still work

Particularly well done: The test that asserts isMpcv2SigningMaterial is never called when multisigTypeVersion is pre-resolved - this directly validates the performance optimization.

Security Analysis

✅ No security concerns identified:

• Uses existing, trusted cryptographic functions
• No new attack vectors introduced
• Proper key material handling maintained
• MPCv2 detection logic is secure

Overall Assessment

This is a high-quality implementation that successfully addresses the stated goals:

1. ✅ Prevents fund loss by using correct MPCv2 address derivation
2. ✅ Improves performance by eliminating redundant keycard decryption
3. ✅ Maintains compatibility with existing MPCv1 and unsigned workflows
4. ✅ Includes thorough testing with good coverage and performance validation

The code is production-ready and follows BitGoJS architectural patterns. The performance optimization is particularly valuable for wallets scanning many addresses.

Recommendation: Approve 🚀

---

Job run: https://github.com/BitGo/BitGoJS/actions/runs/