Skip to content

feat(sdk-core): integrate DKLS VRF DKG into createKeychains for ecdsaMPC safe root - #9661

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

feat(sdk-core): integrate DKLS VRF DKG into createKeychains for ecdsaMPC safe root#9661
s84krish wants to merge 1 commit into
masterfrom
WCN-2447

Conversation

@s84krish

@s84krish s84krish commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Ticket: WCN-2447

Runs the VRF DKG alongside the signing DKG during safe MPC root creation
(WCN-2447), and stores both keyshares in a versioned CBOR envelope inside
encryptedPrv. Gated on safeId — ordinary (non-safe) creation is
byte-for-byte untouched. Depends on the VrfDkg wrapper from #9650 (WCN-2446).

Type of change

  • New feature (non-breaking change which adds functionality)

Changes

  • Routingmodules/sdk-core/src/bitgo/keychain/keychains.ts (createMpc):
    safeId present + MPCv2 + ECDSA → EcdsaVrfMPCv2Utils; otherwise the
    original class.
  • Orchestratormodules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaVrfMPCv2.ts
    (new): EcdsaVrfMPCv2Utils extends EcdsaMPCv2Utils, overrides createKeychains
    to run 2 VRF sessions alongside the 2 signing sessions; VRF messages ride the
    existing MPCv2-R1/R2 keygen round payloads as opaque base64 blobs
    (serializeVrfMessages/deserializeVrfMessages, one blob per party);
    VRF finalizes locally before round 3 (no new rounds, no wire-format changes).
  • EnvelopebuildVrfKeyEnvelopes (same file): packs signing + VRF
    keyshares as cborEncode({version: 1, prvKeyShare, vrf}) into both
    encryptedPrv and reducedEncryptedPrv (keycard copy) via the inherited
    createParticipantKeychain.
  • Wire fieldstypesMPCv2.ts: optional userVrfMsg1/bitgoVrfMsg1/…
    on the R1/R2 request/response types; unset for ordinary ceremonies.
  • Depcbor-x added to sdk-core (already a sdk-lib-mpc dep).
  • Testmodules/bitgo/test/v2/unit/internal/tssUtils/ecdsaVrfMPCv2/createKeychains.ts:
    full ceremony against nocks with a real bitgo-side DKG+VRF player; asserts
    envelopes decrypt to both shares, all parties agree on common keychain and
    VRF public key, reduced share carries VRF material, keys tagged with safeId.

@linear-code

linear-code Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

WCN-2447

@s84krish

s84krish commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

@claude review this PR focusing on correctness, regressions, and code quality. Give an approval rating.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @s84krish's task —— View job


PR Review: VRF DKG Integration for ecdsaMPC Safe Root

Reviewing PR #9661 for correctness, regressions, and code quality.

Todo List:

  • Read and analyze all changed files
  • Review routing logic in keychains.ts
  • Analyze new EcdsaVrfMPCv2Utils orchestrator
  • Check CBOR envelope implementation
  • Review wire field changes in typesMPCv2.ts
  • Examine test coverage and quality
  • Check for potential security issues
  • Look for regression risks
  • Evaluate code quality and best practices
  • Provide final approval rating

Analysis Complete

I've thoroughly reviewed this PR and analyzed all changes. Here's my detailed assessment:

Correctness

Routing Logic: The changes in keychains.ts:421-424 correctly gate VRF functionality on safeId presence, using EcdsaVrfMPCv2Utils only for safe root creation while preserving existing behavior for ordinary TSS wallet creation.

VRF Integration: The EcdsaVrfMPCv2Utils class properly extends the base EcdsaMPCv2Utils and runs VRF DKG alongside signing DKG without changing the wire protocol - VRF messages ride as opaque base64 blobs on existing MPCv2-R1/R2 payloads.

CBOR Envelope: The buildVrfKeyEnvelopes function correctly combines signing and VRF keyshares using versioned CBOR encoding. Both full and reduced envelopes are properly handled.

Protocol Flow: VRF DKG correctly completes before MPCv2-R3, maintaining protocol timing and not requiring additional rounds.

Security

Gating: Strong requirement that safeId must be present (assert(params.safeId, 'safeId is required for VRF MPC root creation')) prevents accidental VRF activation.

Key Material: VRF keyshares are properly encrypted within the same CBOR envelope as signing keys and never exposed in plaintext.

GPG Validation: Proper BitGo GPG key validation is maintained (isBitgoMpcPubKey check).

Regression Prevention

Backward Compatibility: Ordinary (non-safe) creation paths are completely untouched - the safeId check ensures existing behavior is preserved byte-for-byte.

Wire Format: No changes to existing MPCv2 message structures; VRF fields are optional and only set when needed.

Error Handling: Comprehensive assertions throughout the flow prevent invalid states.

Code Quality

Architecture: Clean extension pattern using inheritance rather than modifying the base class.

Testing: Comprehensive test coverage with full ceremony simulation including BitGo-side DKG player, envelope validation, and cross-party agreement verification.

Dependencies: Proper addition of cbor-x dependency (already used in sdk-lib-mpc).

Type Safety: Strong typing with proper TypeScript interfaces and io-ts validation.

🔍 Minor Observations

  1. Documentation: The code is well-documented with clear comments explaining the VRF integration approach.

  2. Error Messages: Clear, descriptive error messages that help with debugging.

  3. Serialization: The VRF message serialization/deserialization functions are correctly implemented and tested.

  4. Chain Code: Minor fix in dkg.ts lines 61-62 appears to be a dependency update.

Test Coverage

The test file provides excellent coverage:

  • Full ceremony execution with real DKG+VRF players
  • CBOR envelope validation and decryption
  • Cross-party agreement verification on VRF public keys
  • Proper handling of reduced keyshares
  • SafeId tagging verification

Final Assessment

Approval Rating: ✅ APPROVED

This is a well-architected implementation that successfully integrates VRF DKG functionality while maintaining complete backward compatibility. The gating mechanism ensures no regressions for existing functionality, and the comprehensive test coverage validates the entire flow. The code quality is high with proper error handling, type safety, and clear documentation.

Key Strengths:

  • Zero impact on existing TSS wallet creation
  • Clean architecture using inheritance
  • Comprehensive security measures
  • Thorough test coverage
  • Proper CBOR envelope handling
  • Clear separation of concerns

Recommendation: This PR is ready for merge. The implementation correctly addresses the requirements while maintaining the high quality and security standards of the codebase.


Job Run: https://github.com/BitGo/BitGoJS/actions/runs/12276365427

@github-actions

github-actions Bot commented Sep 4, 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

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