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
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
Routing — modules/sdk-core/src/bitgo/keychain/keychains.ts (createMpc): safeId present + MPCv2 + ECDSA → EcdsaVrfMPCv2Utils; otherwise the
original class.
Orchestrator — modules/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).
Envelope — buildVrfKeyEnvelopes (same file): packs signing + VRF
keyshares as cborEncode({version: 1, prvKeyShare, vrf}) into both encryptedPrv and reducedEncryptedPrv (keycard copy) via the inherited createParticipantKeychain.
Wire fields — typesMPCv2.ts: optional userVrfMsg1/bitgoVrfMsg1/…
on the R1/R2 request/response types; unset for ordinary ceremonies.
Dep — cbor-x added to sdk-core (already a sdk-lib-mpc dep).
Test — modules/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.
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.
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
Documentation: The code is well-documented with clear comments explaining the VRF integration approach.
Error Messages: Clear, descriptive error messages that help with debugging.
Serialization: The VRF message serialization/deserialization functions are correctly implemented and tested.
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.
⚠️ 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.
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-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 onsafeId— ordinary (non-safe) creation isbyte-for-byte untouched. Depends on the
VrfDkgwrapper from #9650 (WCN-2446).Type of change
Changes
modules/sdk-core/src/bitgo/keychain/keychains.ts(createMpc):safeIdpresent + MPCv2 + ECDSA →EcdsaVrfMPCv2Utils; otherwise theoriginal class.
modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaVrfMPCv2.ts(new):
EcdsaVrfMPCv2Utils extends EcdsaMPCv2Utils, overridescreateKeychainsto 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).
buildVrfKeyEnvelopes(same file): packs signing + VRFkeyshares as
cborEncode({version: 1, prvKeyShare, vrf})into bothencryptedPrvandreducedEncryptedPrv(keycard copy) via the inheritedcreateParticipantKeychain.typesMPCv2.ts: optionaluserVrfMsg1/bitgoVrfMsg1/…on the R1/R2 request/response types; unset for ordinary ceremonies.
cbor-xadded tosdk-core(already asdk-lib-mpcdep).modules/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.