Skip to content

feat: derive safe children at BIP44 paths - #9974

Merged
davidkaplanbitgo merged 1 commit into
masterfrom
WCN-2964
Oct 14, 2026
Merged

davidkaplanbitgo merged 1 commit into
masterfrom
WCN-2964

Conversation

@davidkaplanbitgo

Copy link
Copy Markdown
Contributor

Description

Moves safe child derivation from the single-level m/<n>' scheme to BIP44 paths:

  • user child (every slot) and every MPC child: m/44'/<coinType>'/<slot>'/<account>'
  • multisig co-signers (backup / BitGo): the soft twin m/44/<coinType>/<slot>/<account>

coinType is statics bip44CoinType and slot is statics SAFE_ROOT_SLOT_ORDINALS (1–4). account is the server's per-(slot, coinType) counter. Nothing is in production, so this is a clean cutover. The SDK keeps no single-level support.

This PR also covers the SDK side of SD2 (WCN-2965) and SD3 (WCN-2966), plus the rest of SD4 (WCN-2967). Those tickets are closed as duplicates.

@bitgo/sdk-lib-safes

  • The path types (DerivedFromParentWithHardenedPath / DerivedFromParentWithSoftPath) now accept only exact BIP44 paths: 4 levels, purpose 44, slot in the statics ordinals, canonical decimal, every index below 2^31. m/0', wrong depth, wrong purpose, slot 0/5, mixed hardening and leading zeros are all rejected.
  • New: getSafeBip44CoinType(coin), buildSafeBip44Path(coinType, slot, account) (same signature as the WP builder in BitGo/bitgo-microservices#63403), toSoftTwinPath, parseSafeBip44Path, deriveSafeChildTriplet.
  • The three derive functions take a path instead of an index. They check the path at runtime too, so a cast soft path never gets hardened-derived.
  • Removed parseSafeDerivationIndex, getSafeHardenedDerivationPath, parseDerivedFromParentWithHardenedPath and DERIVED_FROM_PARENT_WITH_HARDENED_PATH (all experimental).

@bitgo/sdk-core

  • Safe.createWallet calls GET derivation-index?slot=&coin= and expects {slot, coinType, account} back. It rejects the response if the server's coinType differs from the SDK's statics (version skew), before any key is registered. It builds the path and mints both multisig and TSS children at it.
  • TSS: ECDSA and EdDSA createSafeChildKeychains take the path instead of derivationIndex.
    • ECDSA: the single-u32 hardenedDerivationPath is gone. The DKLS session uses the same 4-level encoding as recovery (DklsVrfUtils.buildSafeMpcDerivePath).
    • All three children are registered at the path; the hand-built m/${index}' strings are removed.
  • safeKeychain re-derives at the stored BIP44 path. A child stored at a single-level path now fails to re-derive.
  • recovery.ts uses the lib builder and triplet helper instead of its own path builders. Output is unchanged; the existing tests pass without edits.

Not in this PR

  • Round 1 of the MPC derive still sends derivationIndex (set to the account) because the @bitgo/public-types request has no path field yet. Two call sites carry TODO(WCN-2969); MPC1 swaps them for path.
  • Regular (non-safe) wallet creation, signing and address derivation are untouched. The shared TSS keychain helpers only receive the child argument from createSafeChildKeychains.

Rollout

  • Multisig mint needs WP W1/W3/W4/W5: the ?coin= peek, the new response shape, and BIP44 path gates.
  • TSS mint additionally needs MPC1/MPC2.

Until those land, safe minting against staging fails. That's expected for the cutover. Draft until then.

Issue Number

WCN-2964

Type of change

  • Breaking change (experimental safe APIs only: removed index-based derive helpers, createSafeChildKeychains takes path)

How Has This Been Tested?

  • sdk-lib-safes (81 passing):
    • Table of accepted and rejected paths.
    • Builder and coinType lookup: testnets and tokens inherit their chain's coinType; OFC coins are rejected.
    • Pinned keys at BIP44 paths, checked independently. secp256k1 matches chained deriveHardened. The ed25519 deriver reproduces the published 5-level SLIP-0010 vector.
    • Triplet: backup and BitGo co-signer keys match soft derivation from the xpubs alone.
  • sdk-core safe, wallet, TSS and keychain tests (133 passing):
    • createWallet registers BIP44 paths on every branch, including ed25519 coins at their own coinType.
    • A coinType mismatch is rejected before any key is registered.
    • A stored single-level child is rejected.
    • EdDSA: non-BIP44 paths are rejected with no request sent.
  • bitgo ECDSA createSafeChildKeychains (10 passing):
    • The child keychain created at mint equals DklsVrfUtils.deriveSafeEcdsaMpcChild at the same path. Mint and offline recovery land on the same key.
    • Non-BIP44 paths, including a soft one, are rejected before any request.
  • express unit suite (1976 passing): the mint route works against the new derivation-index response.
  • Pre-existing, unrelated: bitgo/test/v2/unit/keychains.ts has 8 failing tests (kaspa/starknet seed, OFC key rotation, EdDSA TSS settings mock). The same 8 fail on a clean master checkout.

Safe child keys move from the single-level m/<n>' scheme to
m/44'/<coinType>'/<slot>'/<account>' for the user child and every MPC
child, with multisig co-signers at the soft twin m/44/<coinType>/<slot>/
<account>. coinType comes from statics bip44CoinType and slot from
SAFE_ROOT_SLOT_ORDINALS. Clean cutover, no single-level support left.

sdk-lib-safes:
- strict BIP44 path brands (exactly 4 levels, purpose 44, slot in the
  statics ordinals, canonical decimal, < 2^31)
- getSafeBip44CoinType, buildSafeBip44Path, toSoftTwinPath,
  parseSafeBip44Path, deriveSafeChildTriplet
- derive functions take a branded path instead of an index
- drop parseSafeDerivationIndex, getSafeHardenedDerivationPath,
  parseDerivedFromParentWithHardenedPath and the single-level regex

sdk-core:
- createWallet peeks derivation-index with ?coin=, expects
  {slot, coinType, account}, rejects a coinType that differs from
  local statics, and mints multisig and TSS children at the BIP44 path
- ECDSA and EdDSA createSafeChildKeychains take the path; the DKLS
  session uses the same 4-level encoding as recovery and every child
  is registered at the path. Round 1 still sends the account as
  derivationIndex until public-types carries a path (MPC1 follow-up)
- safeKeychain re-derives at the stored BIP44 path, so a leftover
  single-level child fails closed
- recovery uses the lib builder and triplet helper

TICKET: WCN-2964
@linear-code

linear-code Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

WCN-2964

@github-actions

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

@davidkaplanbitgo

Copy link
Copy Markdown
Contributor Author

@claude

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @davidkaplanbitgo's task —— View job


PR Review: BIP44 Path Migration for Safe Child Derivation

This PR successfully migrates safe child derivation from single-level m/<n>' paths to proper BIP44 paths m/44'/<coinType>'/<slot>'/<account>'. The implementation is well-designed with strong security considerations and comprehensive validation.

Code Quality Analysis

✅ Excellent Path Validation:

  • Strict regex-based BIP44 format enforcement (/^m\/44'\/(\d+)'\/(\d+)'\/(\d+)'$/)
  • Canonical decimal parsing prevents path aliasing (rejects 044 vs 44)
  • Slot ordinal validation against SAFE_ROOT_SLOT_ORDINALS (1-4 only)
  • Type-safe branded strings prevent mixing hardened/soft paths

✅ Strong Security Design:

  • Runtime path assertions prevent hardened derivation at soft paths (assertHardenedPath)
  • Self-check derivation ensures deterministic results
  • Clear separation: user children use hardened paths, co-signers use soft twins
  • Version skew protection by validating server coinType against SDK statics

✅ Clean Architecture:

  • Proper separation of concerns in @bitgo/sdk-lib-safes
  • Recovery module simplified using new utility functions
  • TSS integration cleanly updated to use paths instead of indices
  • Comprehensive test coverage across multiple modules (81 + 133 + 10 + 1976 tests)

Security Assessment

No Security Issues Found:

  • Path validation prevents injection attacks through proper parsing
  • Type safety prevents accidental path mixing at compile time
  • Runtime assertions catch invalid paths before any cryptographic operations
  • No private key exposure risks - proper handling of hardened vs soft derivation

Breaking Changes

Appropriately Scoped: Breaking changes are limited to experimental safe APIs only:

  • Removed deprecated single-level helpers (parseSafeDerivationIndex, etc.)
  • createSafeChildKeychains now takes path parameter instead of derivationIndex
  • Path validation is stricter and rejects previously accepted patterns

Technical Correctness

✅ BIP44 Compliance:

  • Correct 4-level depth: m/44'/<coinType>'/<slot>'/<account>'
  • Purpose field fixed at 44 (BIP44 standard)
  • Proper use of hardened derivation for user paths, soft for co-signers
  • Account counter managed per (slot, coinType) pair

✅ Implementation Quality:

  • Ed25519 derivation follows SLIP-0010 standard correctly
  • Cross-checked against published test vectors
  • Proper error handling with descriptive messages
  • Recovery functionality maintains output compatibility

Minor Observations

  1. Future MPC Enhancement: TODO comments indicate Round 1 will eventually send path instead of derivationIndex (WCN-2969)
  2. Test Coverage: Comprehensive test suite validates edge cases and security boundaries
  3. Migration Strategy: Clean cutover approach - no backward compatibility needed since nothing is in production

Recommendation

✅ APPROVE - This is a well-implemented security enhancement that properly migrates to standard BIP44 paths while maintaining all security properties. The validation is thorough, the breaking changes are appropriately scoped, and the test coverage is comprehensive.


Branch: WCN-2964 | Job: View run

@davidkaplanbitgo
davidkaplanbitgo marked this pull request as ready for review October 14, 2026 16:29
@davidkaplanbitgo
davidkaplanbitgo requested review from a team as code owners October 14, 2026 16:29

@s84krish s84krish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@davidkaplanbitgo
davidkaplanbitgo merged commit fa7b85f into master Oct 14, 2026
26 checks passed
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.

2 participants