feat: implement safe createSafeKeys and finalize orchestrator

[davidkaplanbitgo]

Implement Phase 2 (root key ceremonies) and Phase 3 (finalize) of safe creation in sdk-core, plus the generateSafe orchestrator chaining all three phases.

createSafeKeys runs the 4 root ceremonies tagged with safeId and returns the 12 key ids as ordered [user, backup, bitgo] triplets:

• 2.1 multisig roots (secp256k1Multisig, ed25519Multisig): local user/backup keypairs + BitGo key via the existing keychain flows.
• 2.2 MPC roots (ecdsaMpc DKLS, eddsaMpc EdDSA): the existing MPCv2 ceremonies via createMpc.

Thread an optional safeId through the shared key-generation paths: keychains.add/createMpc, the ecdsa/eddsa MPCv2 createKeychains ceremonies (round 1 generatekey envelope + participant registration), and their key-gen senders. Legacy MPCv1 paths accept and ignore safeId for signature compatibility.

finalizeSafe posts the rootKeys to the finalize route using the existing public-types codec; generateSafe chains init -> keys -> finalize.

WCN-1192

[linear-code]

WCN-1192

[davidkaplanbitgo]

@claude

[github-actions]

Claude finished @davidkaplanbitgo's task —— View job

---

Code Review: Safe CreateSafeKeys and Finalize Orchestrator

This PR implements Phase 2 (root key ceremonies) and Phase 3 (finalize) of safe creation in sdk-core, along with the generateSafe orchestrator. Here's my detailed review:

✅ Strengths

1. Well-Structured Architecture

• Clean separation between multisig and MPC root creation paths
• Good use of Promise.allSettled() for parallel ceremony execution
• Proper error handling with cleanup (archiving failed safes)

2. Comprehensive Safe ID Threading

• safeId parameter correctly threaded through all key generation paths
• Backwards compatibility maintained for MPCv1 paths
• Interface changes are minimal and well-documented

3. Robust Error Handling

• Automatic cleanup via archiveSafe when key generation fails
• Detailed error messages that list all ceremony failures
• Proper handling of partial failures across 4 ceremonies

4. Good Test Coverage

• Tests verify correct coin routing (mainnet/testnet mappings)
• Proper assertion of safeId tagging in key creation
• Mock structure allows testing ceremony orchestration

🔍 Areas for Improvement

1. Type Safety (modules/sdk-core/src/bitgo/safe/safes.ts:139-143)

const [secp256k1Multisig, ecdsaMpc, eddsaMpc, ed25519Multisig] = results.map(
  (result) => (result as PromiseFulfilledResult<RootKeyTriplet>).value
);

Issue: Unsafe type assertion assumes all results are fulfilled, but failures.length > 0 check should prevent reaching this code with rejections.
Suggestion: Add runtime assertion or use type guards for safety.

2. Error Aggregation Logic (modules/sdk-core/src/bitgo/safe/safes.ts:113-119)
The failure detection correctly handles Promise.allSettled results, but consider extracting this pattern to a utility function since it may be reused.

3. Network Detection (modules/sdk-core/src/bitgo/safe/safes.ts:178)

const network = Environments[this.bitgo.getEnv()].network === 'bitcoin' ? 'mainnet' : 'testnet';

This hardcoded logic may not scale if new environments are added. Consider a more explicit mapping.

🔧 Technical Implementation

1. MPC Integration

• ECDSA MPCv2 and EdDSA MPCv2 implementations correctly thread safeId through createKeychains
• Key generation senders properly handle the safeId parameter
• Maintains compatibility with existing MPC flows

2. Interface Extensions

• AddKeychainOptions.safeId is properly optional and documented as experimental
• CreateMpcOptions.safeId integration is clean and backwards compatible

3. Coin Routing

• ROOT_COIN_BY_NETWORK mapping is sensible and uses stable coin choices
• Representative coins are correctly selected for each root slot type

🛡️ Security Considerations

1. Safe Archival on Failure
The automatic archival of failed safes is a good security practice that prevents orphaned partial states.

2. Ceremony Isolation
Each root ceremony runs independently - failure in one doesn't affect others until final aggregation.

3. Parameter Threading
safeId is consistently threaded through all paths, ensuring proper tagging without data leakage.

📝 Minor Suggestions

1. Documentation: Add JSDoc for the ROOT_COIN_BY_NETWORK constant explaining the coin selection criteria
2. Logging: Consider adding debug logging for ceremony progress tracking
3. Constants: Extract magic numbers like ceremony count (4) into named constants

✨ Overall Assessment

This is a well-implemented feature that successfully orchestrates multiple complex cryptographic ceremonies while maintaining proper error handling and cleanup. The code follows BitGoJS patterns and maintains backwards compatibility.

The implementation correctly handles the challenging requirement of running 4 different key generation ceremonies in parallel while ensuring atomic success/failure semantics.

Recommendation: ✅ APPROVE - This PR is ready for merge after addressing the type safety concern in the results mapping.

---

🔗 Links: Job Run | Branch: davidkaplan/wcn-1192-safe-createkeys

[davidkaplanbitgo]

Addressed the type-safety point and clarified the network logic; leaving the rest as-is with rationale below.

Addressed

• Type assertion (safes.ts) — removed the as PromiseFulfilledResult<...> cast. The settled results are now consumed in a single pass where status === 'fulfilled' narrows .value to a RootKeyTriplet, so the hot triplets are built without any assertion.
• @experimental markings — added @experimental to every new safe symbol (options types, ISafes/Safes methods, ROOT_COIN_BY_NETWORK) and the safeId threading additions, since TypeDoc doesn't inherit the tag from the class/interface to members.

Not worth changing (with reasons)

• Extract error-aggregation to a utility — there's only one caller; extracting now is speculative (YAGNI). It's a 6-line local block. Happy to extract when a second use appears.
• "Network detection may not scale" (coinForRoot) — not a bug. V1Network is exactly 'bitcoin' | 'testnet' (types.ts), and every Environments entry uses one of those two, so the ternary is exhaustive. Added a comment noting the invariant rather than restructuring.
• Extract the ceremony count (4) into a constant — the count isn't hardcoded anywhere; it falls out of failures.length / the results array. There's no magic number to name.
• Debug logging for ceremony progress — this module has no logging convention to match; adding ad-hoc logging here would be inconsistent. Can revisit if we add structured logging to the safe flows.
• JSDoc for ROOT_COIN_BY_NETWORK — it already has a doc comment explaining the curve-scoped coin selection.

[davidkaplanbitgo]

@claude re-review and look at the comments to see if everything was either changed or addressed

[github-actions]

Claude finished @davidkaplanbitgo's task —— View job

---

Re-Review: Safe CreateSafeKeys Implementation ✅

Re-reviewing the implementation and author's responses to previous feedback:

Todo List

• Read current implementation in safes.ts
• Verify type safety fixes were implemented
• Check @experimental markings were added
• Review author's rationale for unchanged items
• Provide updated assessment

✅ Author Successfully Addressed Previous Concerns

1. Type Safety Issue RESOLVED (safes.ts:117-127)
The dangerous type assertion has been completely removed. The code now uses proper type narrowing:

// Single pass over the settled results: `status === 'fulfilled'` narrows `.value` to a
// RootKeyTriplet (no cast needed), and rejections are collected per-slot for the error below.
const hot = {} as SafeKeys['rootKeys']['hot'];
const failures: string[] = [];
results.forEach((result, i) => {
  if (result.status === 'fulfilled') {
    hot[slots[i]] = result.value;  // ← TypeScript correctly narrows this to RootKeyTriplet
  } else {
    // ... handle rejections
  }
});

This is much safer than the previous (result as PromiseFulfilledResult<RootKeyTriplet>).value approach.

2. @experimental Markings ADDED
Comprehensive @experimental annotations added throughout:

• All new interfaces: CreateSafeOptions, SafeCreationHandle, SafeKeys, ListSafesOptions, GetSafeOptions, ISafes
• All new methods in Safes class: generateSafe, initializeSafe, createSafeKeys, finalizeSafe, list, get
• New constants: ROOT_COIN_BY_NETWORK
• Thread-through parameters: safeId in AddKeychainOptions, CreateBackupOptions, CreateBitGoOptions, CreateMpcOptions

✅ Author's Rationale for Unchanged Items is Sound

1. Error Aggregation Pattern - Correctly kept local (6 lines, single caller, YAGNI principle)
2. Network Detection Logic - Author added clarifying comment explaining the invariant that V1Network is exhaustively 'bitcoin' | 'testnet'
3. Magic Number "4" - Correctly noted that ceremony count emerges naturally from array operations, not hardcoded
4. Debug Logging - Good point about consistency; no existing logging convention to match
5. JSDoc for ROOT_COIN_BY_NETWORK - Already has adequate documentation explaining curve-scoped coin selection

🔍 Code Quality Assessment

Strengths Maintained:

• Robust Error Handling: Automatic safe archival on ceremony failure prevents orphaned states
• Parallel Execution: Promise.allSettled() efficiently runs 4 ceremonies concurrently
• Clean Architecture: Well-separated multisig vs MPC paths with proper abstraction
• Backwards Compatibility: safeId parameter threading maintains MPCv1 compatibility

Implementation Details:

• Safe ID Threading: Consistently applied across all key generation paths (iKeychains.ts:156,185,195,214)
• Ceremony Orchestration: Proper handling of partial failures with detailed error messages
• Coin Routing: ROOT_COIN_BY_NETWORK provides stable, representative coins for each root type

🛡️ Security & Reliability

• Atomic Operations: All-or-nothing semantics prevent partial safe creation
• Cleanup on Failure: Failed safes are properly archived to prevent state corruption
• Error Transparency: Detailed failure reporting helps with debugging and retry logic
• Parameter Isolation: safeId tagging ensures proper key association without data leakage

✨ Final Assessment

EXCELLENT - The author has successfully addressed all technical concerns from the previous review:

1. ✅ Type safety issue completely resolved with proper TypeScript narrowing
2. ✅ @experimental markings comprehensively added across the API surface
3. ✅ Solid technical rationale provided for items marked as not needing changes

This implementation demonstrates:

• Strong TypeScript practices with proper type safety
• Well-thought-out API design with appropriate experimental markings
• Robust error handling with proper cleanup mechanisms
• Good engineering judgment in technical decisions

Recommendation: ✅ STRONG APPROVAL - This PR successfully implements the safe creation orchestrator with high code quality, proper error handling, and excellent attention to the review feedback. Ready for merge.

---

🔗 Links: Job Run | Branch: davidkaplan/wcn-1192-safe-createkeys

[zahin-mohammad]

lgtm, just need to check the multisig ed representative coin