feat(statics): add bip44CoinType per coin family - #9882
danielpeng1 wants to merge 1 commit into
Conversation
|
|
mohd-kashif
left a comment
mohd-kashif
left a comment
There was a problem hiding this comment.
Non-blocking suggestions from a code review — all optional, none block merge.
| * change one. A new coin family takes the next free value above the highest (0x70000000 and up). | ||
| * OFC and fiat have no family entry and are not BIP44-derivable. | ||
| */ | ||
| export const BIP44_COIN_TYPES: Partial<Record<CoinFamilyName, number>> = { |
There was a problem hiding this comment.
nit (non-blocking): Since ofc and fiat are the only families intentionally omitted, consider typing this as Record<Exclude<CoinFamilyName, 'ofc' | 'fiat'>, number> instead of Partial<Record<...>>. That would make a missing derivable family a compile error rather than something only the runtime invariant test catches — and it would surface today that dydx and eth2 have no entry (fine while no derivable coins use them, but the compiler would force a conscious decision when one is added).
| * coins carry no `bip44CoinType` and cannot mint a safe child key. | ||
| * @experimental | ||
| */ | ||
| export function isBip44Derivable(coin: Readonly<BaseCoin>): boolean { |
There was a problem hiding this comment.
nit (non-blocking): isBip44Derivable is exported but has no consumer in this PR (only tests call it). Fine as experimental groundwork, but if no caller lands soon it's dead surface area in a package that's widely re-exported — consider landing it together with its first consumer or deferring it.
| * (`m/44/<bip44CoinType>/<slot>/<account>`). | ||
| * @experimental | ||
| */ | ||
| export type SafeRootSlot = 'secp256k1Multisig' | 'ed25519Multisig' | 'ecdsaMpc' | 'eddsaMpc'; |
There was a problem hiding this comment.
nit (non-blocking): This adds a third name for the same 4-value union — public-types has RootKeyType, sdk-lib-safes has SafeRootKeyType (see #9881), and now SafeRootSlot here. The SAFE_ROOT_SLOTS: RootKeyType[] = STATICS_SAFE_ROOT_SLOTS assignment in sdk-core's rootCoin.ts does act as a compile-time drift check, which helps — but a short comment here noting RootKeyType (in @bitgo/public-types) as the canonical source of these slot names would help future readers keep them in sync.
| } | ||
|
|
||
| // the bip44 coin type is the <coinType>' segment of safe child derivation paths | ||
| if (options.bip44CoinType !== undefined) { |
There was a problem hiding this comment.
nit (non-blocking): Validation asymmetry — explicitly-passed bip44CoinType values are range-checked here, but family-derived values (the ?? getBip44CoinType(...) fallback in the constructor) are only guarded by the BIP44_COIN_TYPES table test. Consistent today since the table is tested, just worth being aware the two paths carry different guarantees.
| otherSupportedKeyCurves?: KeyCurve[]; | ||
| /** | ||
| * BIP44 coin type used by the safe child derivation scheme | ||
| * `m/44'/<bip44CoinType>'/<slot>'/<account>'` (see SAFE_ROOT_SLOT_ORDINALS for `<slot>`). |
There was a problem hiding this comment.
nit (non-blocking): This reference to SAFE_ROOT_SLOT_ORDINALS is cross-package (it lives in safe.ts in this package), so a plain-text mention won't resolve as a JSDoc link from consumer packages. Minor — either keep as prose or note the defining module explicitly.
This branch has not been deployed
3 participants
Adds an optional
bip44CoinTypeto statics coins, resolved per coin family, and moves the root slot list into statics.bip44CoinTypetoBaseCoin, validated as an integer in[0, 0x7fffffff]BIP44_COIN_TYPES, a table keyed by coin family, so testnets and tokens inherit their parent chain's value (OFC and fiat have none)sdk-coreimports it instead of keeping its own copyTests
Ticket: WCN-2955