[davidkaplanbitgo]

Description

Part of the BIP-0044 style wallet derivation project (spike WCN-3907): wallets that share root xpubs get a second, per-wallet derivation scheme 44/<coin_type>/<h1>/<h2>/<chain>/<index>; h1/h2 are recomputed from the three wallet keys.

• AddressCoinSpecific.derivationPrefix (sdk-core). Address verification and deriveAddress read it: absent means legacy; present must equal the canonical prefix recomputed with fixedScriptWallet.deriveWalletPrefix from the three keys and the statics bip44CoinType. A server-supplied non-canonical prefix is never trusted (recovery only scans the legacy and canonical subtrees). An undefined coin type with a prefix throws; allowSkipVerifyAddress does not skip prefixed addresses.
• PSBT wallet outputs take their prefix from the parsed derivationPath (all components except the last two) so bip44 change is not reported as external.
• Express deriveAddress/isWalletAddress codecs accept coinSpecific.derivationPrefix (and chain on isWalletAddress).
• Statics test pins bip44CoinType for every UTXO coin; README section on the two schemes and how they differ from Safes.

Already-released SDKs throw UnexpectedAddressError for a bip44 address and nothing here can change that; the rollout gates enabling per enterprise.

Issue Number

WCN-3936

Type of change

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

How Has This Been Tested?

Not yet. Nothing was built or run: this needs the unreleased @bitgo/wasm-utxo (deriveWalletPrefix, full derivationPath on parsed outputs) and a version bump in abstract-utxo, utxo-core, utxo-bin, utxo-descriptors, utxo-ord and utxo-staking. Unit tests were written for address verification, output prefix forwarding, sdk-core forwarding and the Express codecs, but have not run.

[linear-code]

WCN-3936

[github-actions]

⚠️ Compatibility checks are failing on Node.js 27-nightly (experimental, non-blocking). This does not block merge, but it signals an incompatibility to fix before this release becomes supported.

View run

[davidkaplanbitgo]

@claude

[github-actions]

Review of #10107: verify addresses under the per-wallet derivation prefix

Overall this is a solid, security-conscious change. The prefix is never trusted: it is recomputed from the three keys and the coin type, and the check is strict in both directions. The "undefined coin type throws" path is covered, and the tbtcreg coin-type pitfall is called out. I found no critical issues.

Internal information leakage (please fix before merge)

• modules/abstract-utxo/test/unit/address.ts (describe('bip44 derivation prefix')): the fixture seeds 'wcn3907-u', 'wcn3907-b', 'wcn3907-g1' and 'wcn3907-g2' embed an internal spike ticket ID. Use neutral seeds, e.g. 'bip44-prefix-user', 'bip44-prefix-backup', 'bip44-prefix-bitgo1' and 'bip44-prefix-bitgo2'. The hard-coded prefix1/prefix2 constants depend on the seeds, so recompute them.
• The same file has the comment "Vectors from the wallet-prefix definition". That points at an internal spec. Describe the algorithm instead: "Prefix = 44/<coinType>/<h1>/<h2>, where h1/h2 come from sha256 over a domain tag and the three neutered root keys."
• Keep rollout and spike references (such as the per-enterprise gating) in the PR body only, not in code comments, test names or the README.

Suggestions

1. generateAddress in fixedScript.ts: the destructuring block is now followed directly by if (_.isNumber(chain) .... The blank line was dropped. Please restore it.
2. getDerivationPrefixFromPath returns undefined for paths with fewer than three components, and returns any other prefix (e.g. 5 from 5/1/2) unvalidated. This is safe today because isWalletAddress rejects non-canonical prefixes, and the new tests cover it. The JSDoc could state that a malformed short path is treated as legacy. Confirm this is intended.
3. FixedScriptAddressCoinSpecific and GenerateFixedScriptAddressOptions both gain derivationPrefix. Consider a shared type so they can't drift.
4. The express isWalletAddress codec adds chain: t.number while index is number | string. Check that a string chain fails with a clear 400.
5. Test gap: nothing goes end to end from a real PSBT with a bip44 derivationPath through toChangeOutputBigInt to an internal classification. parseOutput is tested only with a stubbed isWalletAddress. A PSBT-level explainPsbtWasm test would lock in the "all components except the last two" behavior.

Questions

• Does the statics test pin bip44CoinType for every tbtc* variant, so a new variant can't silently get undefined? That would throw, which is safe but disruptive.
• Older SDKs throw UnexpectedAddressError on bip44 addresses and rollout is gated. Is there a minimum-SDK-version signal for the server side to rely on?

Good practices

• Strict two-way verification: legacy presented with a prefix is rejected, and a prefix presented without one is rejected.
• Coin type is read from getConfig() and not from the wasm name.
• Thorough tests cover cross-coin, cross-wallet, 0/0 and missing-coin-type cases across chains.
• allowSkipVerifyAddress cannot bypass prefixed addresses.