Skip to content

feat(sdk-core): DKLS derive round orchestrator - #9717

Draft
s84krish wants to merge 1 commit into
masterfrom
WCN-2340
Draft

feat(sdk-core): DKLS derive round orchestrator#9717
s84krish wants to merge 1 commit into
masterfrom
WCN-2340

Conversation

@s84krish

@s84krish s84krish commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

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 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).

@linear-code

linear-code Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

WCN-2340

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

⚠️ 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.

View run

@s84krish s84krish changed the title feat: DKLS derive round orchestrator feat(sdk-core): DKLS derive round orchestrator Sep 10, 2026
@s84krish

Copy link
Copy Markdown
Contributor Author

@claude review this PR focusing on correctness, code quality, and implementation. Flag any regressions.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @s84krish's task —— View job


PR Review: DKLS Derive Round Orchestrator

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
  • Memory safety: Careful WASM object lifecycle management with explicit .free() calls

Wire Protocol & Integration

  • 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 creation
const { userKeychain, backupKeychain } = await tssUtils.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 constant
if (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
  • API consistency: Follows existing MPCv2 patterns
  • Safe integration: TSS wallet creation path cleanly integrated

Recommendation: APPROVE with Minor Fixes

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.


View Job Run | Branch: WCN-2340

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