You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
DKLS hard-derive round orchestrator for safe MPC child minting (WCN-2340): the SDK drives the user/backup parties of the DKLS hard-derive protocol against the server's BitGo party, runs the R1/R2/R3 ceremony
over /mpc/generatekey, registers the derived user/backup signing shares (never the VRF share), and wires the flow into Safe.createWallet's tss branch. Derive codecs come from @bitgo/public-types@6.71.0;
server rounds land via WCN-2339/2338/2330.
Changes
sdk-lib-mpc — DklsDrv.Derive wrapper (src/tss/ecdsa-dkls/derive.ts): round driver over the wasm HardDeriveSession (initDerive/handleIncomingMessages/getKeyShare/getReducedKeyShare/restoreSession), with
up-front root and VRF keyshare partyId validation in initDerive. Exported via ecdsa-dkls/index.ts; ceremony test helper generateHardDerivedKeyShares in dkls-vrf/util.ts.
Wire contract (utils/tss/ecdsa/typesMPCv2.ts, ecdsaMPCv2KeyGenSender.ts): consume the published MPCv2DeriveRound1/2/3Request/Response codecs; KeyGenSenderForSafeChild posts /mpc/generatekey with
MPCv2KeyGenStateEnum['MPCv2Derive-R1/R2/R3'], parentKeyId/derivationIndex in the R1 payload only.
Ceremony (ecdsaVrfMPCv2.ts): createSafeChildKeychains — hardened path builder hardenedDerivationPath, root-envelope validator parseVrfKeyEnvelopes, R1/R2/R3 round senders (sessionId asserted before the
BitGo R2 messages are applied), then agreement check on the child common keychain (assert.equal(commonKeychain, …) at the registration boundary) and registration via createParticipantKeychain (child
{parentKeyId, index} options, no parent/derivedFromParentWithPath on ordinary keygen).
Activation (safe/safe.ts): createWallet tss branch — peek ecdsaMpc derivation index, fetch/decrypt user+backup root blobs, split the VRF envelopes, run the ceremony, mint with the 2-key multisigType:'tss'
body (onchain path unchanged).
Tests: sdk-lib-mpc/test/unit/tss/dkls-vrf/derive.ts (protocol, determinism, restore, partyId mismatch) · modules/bitgo/test/.../ecdsaVrfMPCv2/createSafeChildKeychains.ts (ceremony vs nocked server,
index>0, bad blob, VRF mismatch) · sdk-core/test/unit/bitgo/safe/safe.ts (tss mint body/args).
⚠️ Unit tests are failing on Node 26.x (Current release line, non-blocking). This is not an LTS version yet, so it does not block merge, but it signals an incompatibility to fix before Node 26.x becomes LTS.
Review Summary:
This PR implements DKLS hard-derive round orchestrator for safe MPC child minting. After comprehensive analysis, I found the implementation to be technically sound with robust security measures, but identified several areas for improvement.
✅ Strengths
Core Implementation (derive.ts)
Security-first design: Strong party ID validation in initDerive() (lines 173-179) prevents key substitution attacks
Robust state management: State derived from wasm session bytes rather than caller-supplied enums prevents tampering
Proper error handling: Comprehensive error messages with context throughout the protocol flow
Clean API design: New derive-specific round states (MPCv2Derive-R1/R2/R3) properly separated from keygen
Proper session validation: Session ID verification across all rounds prevents replay/confusion attacks
Secure message routing: GPG encryption/authentication maintained throughout ceremony
Test Coverage
Protocol correctness: Tests verify deterministic key derivation and party agreement
Edge case handling: Tests cover invalid blobs, VRF mismatches, and restore scenarios
Integration testing: Full ceremony mock against nocked server responses
⚠️Issues & Recommendations
High Priority
1. Incomplete Safe Integration (safe.ts:260)
// Missing validation of ceremony completion before wallet creationconst{ userKeychain, backupKeychain }=awaittssUtils.createSafeChildKeychains({...});// Should verify keychain.id is populated and ceremony succeeded
2. Error Recovery Gap
The restoreSession method doesn't support Round 1 restoration (derive.ts:335), but there's no clear guidance on how to handle Round 1 failures in production.
Medium Priority
3. Hardcoded Magic Numbers
// derive.ts:158 - Should be a named constantif(this.seed&&this.seed.length!==32){
4. Inconsistent VRF Envelope Handling
The VRF key envelope parsing in parseVrfKeyEnvelopes() could benefit from more specific error messages to aid debugging.
5. Protocol State Exposure
The DeriveSessionData interface exposes internal wasm session bytes which are marked as "secret key material" but lack additional protection beyond caller encryption.
Low Priority
6. Code Duplication
Significant duplication between EcdsaMPCv2Utils.createKeychains() and EcdsaVrfMPCv2Utils.createSafeChildKeychains() - consider extracting common ceremony patterns.
7. Type Safety
Several as type assertions in the ceremony code could be replaced with proper type guards.
🔒 Security Assessment
Strong Points:
Party ID validation prevents key substitution
Session ID verification prevents ceremony confusion
GPG encryption/authentication throughout
State integrity via wasm session validation
No Critical Vulnerabilities Detected
🧪 Test Quality
Coverage: Good protocol and integration coverage
Scenarios: Tests key determinism, error cases, and restore functionality
Integration: Proper mocking of server ceremony responses
📊 Architecture Impact
Clean separation: Derive ceremony properly isolated from keygen
This is a well-implemented feature that adds important safe MPC functionality. The security model is sound and the implementation follows established patterns. Address the incomplete safe integration validation and consider the error recovery improvements for production robustness.
Node 26.x Test Failures: Non-blocking as noted in the PR description, but should be addressed before Node 26 becomes LTS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket: WCN-2340
DKLS hard-derive round orchestrator for safe MPC child minting (WCN-2340): the SDK drives the user/backup parties of the DKLS hard-derive protocol against the server's BitGo party, runs the R1/R2/R3 ceremony
over
/mpc/generatekey, registers the derived user/backup signing shares (never the VRF share), and wires the flow intoSafe.createWallet's tss branch. Derive codecs come from@bitgo/public-types@6.71.0;server rounds land via WCN-2339/2338/2330.
Changes
up-front root and VRF keyshare partyId validation in initDerive. Exported via ecdsa-dkls/index.ts; ceremony test helper generateHardDerivedKeyShares in dkls-vrf/util.ts.
MPCv2KeyGenStateEnum['MPCv2Derive-R1/R2/R3'], parentKeyId/derivationIndex in the R1 payload only.
BitGo R2 messages are applied), then agreement check on the child common keychain (assert.equal(commonKeychain, …) at the registration boundary) and registration via createParticipantKeychain (child
{parentKeyId, index} options, no parent/derivedFromParentWithPath on ordinary keygen).
body (onchain path unchanged).
index>0, bad blob, VRF mismatch) · sdk-core/test/unit/bitgo/safe/safe.ts (tss mint body/args).