Skip to content

feat(sdk-coin-iota): support MPCv2 consolidation recovery - #9545

Merged
Marzooqa merged 1 commit into
masterfrom
WCI-1235-iota-mpcv2-consolidations
Aug 25, 2026
Merged

feat(sdk-coin-iota): support MPCv2 consolidation recovery#9545
Marzooqa merged 1 commit into
masterfrom
WCI-1235-iota-mpcv2-consolidations

Conversation

@bitgo-ai-agent-dev

Copy link
Copy Markdown
Contributor

What

  • Detect MPCv2 signing material once before IOTA consolidation scans.
  • Derive the base destination with MPCv2-compatible EdDSA derivation.
  • Thread the resolved MPCv2 route into each recovery call.

Why

  • MPCv2 signed consolidations must sweep to the correct base address without
    decrypting the keycard once per scanned receive index.

Test plan

  • git diff --check
  • Run the IOTA unit tests (Nix dev shell dependency bootstrap failed in this environment).

Ticket: WCI-1235

@linear-code

linear-code Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

WCI-1235

@bitgo-ai-agent-dev
bitgo-ai-agent-dev Bot force-pushed the WCI-1235-iota-mpcv2-consolidations branch from a4a45de to 3b699ed Compare August 24, 2026 12:12
@ralph-bitgo
ralph-bitgo Bot force-pushed the WCI-1235-iota-mpcv2-consolidations branch from 3b699ed to c5a4ba0 Compare August 24, 2026 12:13
@bitgo-ai-agent-dev
bitgo-ai-agent-dev Bot force-pushed the WCI-1235-iota-mpcv2-consolidations branch from 03e4c37 to e16c962 Compare August 25, 2026 06:43
@ralph-bitgo
ralph-bitgo Bot force-pushed the WCI-1235-iota-mpcv2-consolidations branch from e16c962 to 29b4b1d Compare August 25, 2026 06:43
Add MPCv2 signed hot recovery support to recoverConsolidations(), consistent
with the DOT/SUI pattern:

- Add protected getEddsaSigningMaterial()/signIotaMpcV2Recovery() wrapper
  methods (matching DOT/SUI) so keycard detection and MPS signing can be
  independently stubbed in tests
- recover() accepts an optional precomputedMaterial param and threads it
  through recoverIotaToken()/signRecoveryTransaction() instead of a boolean
- signRecoveryTransaction() detects the keycard version exactly once (or
  reuses precomputedMaterial), branches on .version, and passes
  signingMaterial.encryptedUserKey into the MPCv2 signing call
- recoverConsolidations() detects signing material once before the scan
  loop and passes it into each recover() call, avoiding re-decryption per
  index, with no ad hoc IotaRecoveryOptions type extension
- Add consolidation MPCv2 test coverage with two funded receive indexes

Ticket: WCI-1235
@Marzooqa
Marzooqa force-pushed the WCI-1235-iota-mpcv2-consolidations branch from 29b4b1d to 15655e0 Compare August 25, 2026 08:34
@Marzooqa

Copy link
Copy Markdown
Contributor

@claude

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Marzooqa's task —— View job


PR Review: MPCv2 Consolidation Recovery for IOTA

Todo List

  • Read and analyze the main implementation file (iota.ts)
  • Review the test changes
  • Check for potential security issues
  • Verify code quality and best practices
  • Provide comprehensive review feedback

Code Review Feedback

Implementation Overview

This PR successfully adds MPCv2 consolidation recovery support for IOTA by:

  • Detecting MPCv2 signing material once before scanning addresses (line 484-487 in recoverConsolidations)
  • Deriving the base destination address using MPCv2-compatible EdDSA derivation (line 490-492)
  • Threading the resolved MPCv2 signing material into each recovery call (line 512)

Key Improvements

1. Performance Optimization (Critical)

  • Before: The keycard was decrypted on every loop iteration during consolidation scans
  • After: MPCv2 signing material is detected once and reused (precomputedMaterial parameter)
  • Impact: Significant performance improvement for consolidation operations

2. MPCv2 Integration

  • Proper integration with the existing MPCv2 infrastructure (signEddsaMpcV2RecoveryTx)
  • Maintains backward compatibility with MPCv1 signing paths
  • Clean abstraction through protected methods (getEddsaSigningMaterial, signIotaMpcV2Recovery)

Code Quality Assessment

Well-structured changes:

  • Clean separation of concerns with helper methods
  • Consistent parameter passing pattern
  • Good error handling preservation
  • Minimal surface area changes

Good practices:

  • Maintains existing function signatures and behavior
  • Proper type safety with EddsaSigningMaterial interface
  • Clear documentation in method signatures

Security Considerations

Secure keycard handling:

  • The precomputedMaterial parameter properly threads signing material without re-decryption
  • Maintains the same security model as existing recovery flows
  • No credential exposure or logging issues detected

Validation preserved:

  • All existing validation logic remains intact
  • Error paths properly maintained
  • No bypassing of security checks

Test Coverage Analysis

Comprehensive test additions (88 new lines):

  • MPCv2 recovery path validation (lines 734-775, 844-907)
  • MPCv1 regression testing (lines 909-933) - Excellent safety measure
  • MPCv2 consolidation tests (lines 1112-1176)
  • Error case coverage (missing keys, mismatched keys)
  • Signature validation using nacl.sign.detached.verify - Great verification

⚠️ Minor Observations

1. Parameter Threading Pattern
The precomputedMaterial parameter is added to multiple methods, which increases complexity but is necessary for the optimization. This is well-handled.

2. Method Signatures
The recover method signature change (adding optional precomputedMaterial) maintains backward compatibility.

3. Code Consistency
The implementation follows the same patterns used in other coins (DOT, SUI) for MPCv2 support, which is good for consistency.

Testing Strategy Validation

Excellent test coverage:

  • Both MPCv1 and MPCv2 code paths tested
  • Regression tests ensure MPCv1 still works
  • Error scenarios properly covered
  • Real signature verification in tests adds confidence

Test quality:

  • Uses proper mocking with sinon
  • Validates actual cryptographic signatures
  • Covers both signed and unsigned transaction paths

🎯 Recommendation: APPROVE

This is a well-implemented feature that:

  1. Solves the stated problem: Avoids re-decrypting keycards during consolidation scans
  2. Maintains security: No security regressions or credential exposure
  3. Preserves compatibility: MPCv1 flows remain unchanged
  4. Has comprehensive tests: Both positive and negative test cases
  5. Follows established patterns: Consistent with other coin implementations

The performance improvement for MPCv2 consolidation operations will be significant, and the implementation is clean and safe.


Job run: View workflow run
Branch: WCI-1235-iota-mpcv2-consolidations

@Marzooqa
Marzooqa marked this pull request as ready for review August 25, 2026 10:02
@Marzooqa
Marzooqa requested a review from a team as a code owner August 25, 2026 10:02
@Marzooqa
Marzooqa requested a review from vibhavgo August 25, 2026 10:02

@vibhavgo vibhavgo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Marzooqa
Marzooqa merged commit 79c7e93 into master Aug 25, 2026
25 checks passed
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.

3 participants