From dc6915c95145a3f4f975f59653e74d44cdd4c50f Mon Sep 17 00:00:00 2001 From: David Kaplan Date: Fri, 30 Oct 2026 14:09:13 -0400 Subject: [PATCH] feat: verify addresses under the per-wallet derivation prefix Address verification and derivation read coinSpecific.derivationPrefix: absent means legacy, present must equal the canonical prefix recomputed from the three keys and the statics coin type. PSBT outputs take their prefix from the parsed derivation path so bip44 change is not reported as external. Express codecs accept derivationPrefix, the statics test pins every UTXO coin type, and the abstract-utxo README documents the two schemes. Ticket: WCN-3936 --- modules/abstract-utxo/README.md | 32 ++ modules/abstract-utxo/src/abstractUtxoCoin.ts | 8 + .../src/address/derivationPrefix.ts | 73 +++++ .../abstract-utxo/src/address/fixedScript.ts | 48 ++- modules/abstract-utxo/src/address/index.ts | 6 + .../fixedScript/explainPsbtWasm.ts | 4 + .../transaction/fixedScript/parseOutput.ts | 6 + .../abstract-utxo/src/transaction/types.ts | 5 + modules/abstract-utxo/test/unit/address.ts | 284 +++++++++++++++++- .../parseOutputDerivationPrefix.ts | 91 ++++++ .../src/typedRoutes/api/v2/deriveAddress.ts | 4 + .../src/typedRoutes/api/v2/isWalletAddress.ts | 9 + .../test/unit/typedRoutes/deriveAddress.ts | 46 +++ .../test/unit/typedRoutes/isWalletAddress.ts | 56 ++++ .../sdk-core/src/bitgo/baseCoin/iBaseCoin.ts | 6 + .../wallet/walletUtxoAddressVerification.ts | 98 ++++++ modules/statics/test/unit/bip44CoinTypes.ts | 37 +++ 17 files changed, 801 insertions(+), 12 deletions(-) create mode 100644 modules/abstract-utxo/src/address/derivationPrefix.ts create mode 100644 modules/abstract-utxo/test/unit/transaction/fixedScript/parseOutputDerivationPrefix.ts create mode 100644 modules/sdk-core/test/unit/bitgo/wallet/walletUtxoAddressVerification.ts diff --git a/modules/abstract-utxo/README.md b/modules/abstract-utxo/README.md index cf4060668e5..4965dead861 100644 --- a/modules/abstract-utxo/README.md +++ b/modules/abstract-utxo/README.md @@ -38,6 +38,38 @@ Concrete implementations are provided in the `src/impl/` directory: - Support for multiple address formats (base58, bech32, cashaddr for BCH) - Chain/index-based address generation for fixed script wallets +### Address Derivation Schemes + +Fixed script wallets derive every address from the same three root keys (user, backup, BitGo) at +`//`, applying the identical path to all three keys. Two schemes exist; the scheme of an address +is carried in the Address document as `coinSpecific.derivationPrefix`: + +| Scheme | `derivationPrefix` | Prefix used | Notes | +| ------ | ------------------ | ----------- | ----- | +| Legacy | absent | `0/0` (the wallet's existing key path) | Every wallet created so far. A missing prefix always means legacy. | +| BIP44-style | `44//

/

` | the value itself | Per-wallet: two wallets sharing the same root keys get distinct addresses. | + +For the BIP44-style scheme: + +- `coinType` is the coin's `bip44CoinType` from the `@bitgo/statics` registry (btc 0, ltc 2, doge 3, dash 5, zec 133, + bch 145, btg 156, bsv 236, bcha 899; testnets inherit their mainnet's value). It is read from the coin instance + (`coin.getConfig().bip44CoinType`), never from the wasm coin name. +- `h1`/`h2` (the "account") are derived from a SHA-256 hash of the three neutered root keys + (`fixedScriptWallet.deriveWalletAccount`), and the whole prefix is produced by `fixedScriptWallet.deriveWalletPrefix`. + All components are non-hardened, so addresses can be derived from xpubs alone. +- The prefix is **never trusted**. `isWalletAddress` and `deriveAddress` recompute the canonical prefix from the three + keys and the coin type and reject any other value (strict in both directions: a legacy address presented with a + prefix fails, and a bip44 address presented without one fails). A coin without a BIP44 coin type throws when it is + given a prefix; `allowSkipVerifyAddress` cannot skip this check. +- PSBT outputs are classified from the full derivation path recorded in the PSBT (all components except the last two + are the prefix), under the same canonical-equality check, so bip44 change outputs are reported as change. +- SDK versions that predate this check throw `UnexpectedAddressError` for a bip44 address. + +**This is not the Safes scheme.** Safes (`@bitgo/sdk-lib-safes`) derive a child key per wallet from the enterprise key +with hardened levels and a server-side counter, so each Safe has its own keys. Here the wallet keeps the same root +keys as any other wallet that uses them; only the non-hardened prefix differs, and the account comes from a hash of +the three xpubs rather than a counter. The two share only the coin-type registry in `@bitgo/statics`. + ### Recovery Operations - **Backup Key Recovery**: Recover funds using backup key when BitGo is unavailable - **Cross-Chain Recovery**: Recover coins sent to wrong chain (e.g., BTC sent to LTC address) diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index ff6953ae420..9aea210860b 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -710,12 +710,17 @@ export abstract class AbstractUtxoCoin extends BaseCoin implements Musig2Partici throw new Error('missing required param keychains'); } + // A missing derivationPrefix means the legacy scheme. A present one is only accepted if it equals the canonical + // prefix recomputed from the three keys and this coin's bip44 coin type; this is enforced in generateAddress + // and cannot be skipped (an undefined coin type throws). assertFixedScriptWalletAddress(this.wasmName, { address, keychains, format: params.format ?? 'base58', chain, index, + derivationPrefix: params.coinSpecific?.derivationPrefix, + bip44CoinType: this.getConfig().bip44CoinType, }); return true; @@ -745,6 +750,9 @@ export abstract class AbstractUtxoCoin extends BaseCoin implements Musig2Partici chain, index, format, + // coin type is resolved from the coin instance: `this.wasmName` is not a statics name (e.g. `tbtcreg`) + derivationPrefix: params.coinSpecific?.derivationPrefix, + bip44CoinType: this.getConfig().bip44CoinType, }); return { address, chain, index }; diff --git a/modules/abstract-utxo/src/address/derivationPrefix.ts b/modules/abstract-utxo/src/address/derivationPrefix.ts new file mode 100644 index 00000000000..1c9d402ebbf --- /dev/null +++ b/modules/abstract-utxo/src/address/derivationPrefix.ts @@ -0,0 +1,73 @@ +import { Triple, UnexpectedAddressError } from '@bitgo/sdk-core'; +import { fixedScriptWallet } from '@bitgo/wasm-utxo'; + +/** + * Prefix of the legacy scheme: every wallet key is derived at `m/0/0//`. + * An address without a `derivationPrefix` always uses this prefix. + */ +export const LEGACY_DERIVATION_PREFIX = '0/0'; + +/** + * Strip a single leading `m/` or `/` so prefixes recorded in PSBTs (`m/44/...`, `/44/...`) + * compare equal to the relative form used on addresses (`44/...`). + */ +export function normalizeDerivationPrefix(prefix: string): string { + if (prefix.startsWith('m/')) { + return prefix.slice(2); + } + if (prefix.startsWith('/')) { + return prefix.slice(1); + } + return prefix; +} + +/** + * Extract the non-legacy derivation prefix from the full path recorded for a PSBT output: + * all components except the last two (`chain`, `index`), without a leading `m/` or `/`. + * + * Returns `undefined` when the path carries no prefix or the legacy `0/0` prefix, so callers + * treat the output exactly as before. Any other prefix is returned unvalidated: it must be + * checked with {@link assertCanonicalDerivationPrefix} before it is used for derivation. + */ +export function getDerivationPrefixFromPath(path: string | null | undefined): string | undefined { + if (!path) { + return undefined; + } + const components = normalizeDerivationPrefix(path).split('/'); + if (components.length < 3) { + return undefined; + } + const prefix = components.slice(0, -2).join('/'); + return prefix === LEGACY_DERIVATION_PREFIX ? undefined : prefix; +} + +/** + * Assert that `derivationPrefix` is the canonical prefix of the wallet formed by `keys`: + * the one recomputed from the three root keys and the coin's BIP44 coin type. A server (or PSBT) + * supplied prefix is never trusted, so a different prefix is rejected as an unexpected address. + * + * @param keys the wallet's root xpubs `[user, backup, bitgo]` + * @param derivationPrefix the prefix to check; leading `m/` or `/` is ignored + * @param bip44CoinType the coin's BIP44 coin type (`coin.getConfig().bip44CoinType`) + * @returns the canonical prefix (relative, no leading `/`) + * @throws {Error} if the coin has no BIP44 coin type + * @throws {UnexpectedAddressError} if the prefix is not the canonical one + */ +export function assertCanonicalDerivationPrefix( + keys: Triple, + derivationPrefix: string, + bip44CoinType: number | undefined +): string { + if (bip44CoinType === undefined) { + throw new Error( + `cannot verify derivationPrefix "${derivationPrefix}": coin has no bip44 coin type and does not support per-wallet derivation prefixes` + ); + } + const expected = normalizeDerivationPrefix(fixedScriptWallet.deriveWalletPrefix(keys, bip44CoinType)); + if (normalizeDerivationPrefix(derivationPrefix) !== expected) { + throw new UnexpectedAddressError( + `address validation failure: derivationPrefix "${derivationPrefix}" is not the canonical prefix "${expected}" of the wallet keys` + ); + } + return expected; +} diff --git a/modules/abstract-utxo/src/address/fixedScript.ts b/modules/abstract-utxo/src/address/fixedScript.ts index 8f889b665c9..8805ce81044 100644 --- a/modules/abstract-utxo/src/address/fixedScript.ts +++ b/modules/abstract-utxo/src/address/fixedScript.ts @@ -16,11 +16,14 @@ import { fixedScriptWallet } from '@bitgo/wasm-utxo'; import { toWasmUtxoCoinName, UtxoCoinName, WasmUtxoCoinName } from '../names'; +import { assertCanonicalDerivationPrefix } from './derivationPrefix'; + type ScriptType2Of3 = fixedScriptWallet.OutputScriptType; type ChainCode = fixedScriptWallet.ChainCode; export interface FixedScriptAddressCoinSpecific { outputScript?: string; + derivationPrefix?: string; } export interface GenerateAddressOptions { @@ -33,6 +36,13 @@ export interface GenerateAddressOptions { interface GenerateFixedScriptAddressOptions extends GenerateAddressOptions { format?: CreateAddressFormat; keychains: { pub: string }[]; + /** + * Absent = legacy scheme (`0/0`). Present = the bip44 prefix `44//

/

`, which must equal the + * canonical prefix recomputed from `keychains` and `bip44CoinType`. + */ + derivationPrefix?: string; + /** BIP44 coin type of the coin (`coin.getConfig().bip44CoinType`); required when `derivationPrefix` is present. */ + bip44CoinType?: number; } function supportsAddressType(coinName: UtxoCoinName | WasmUtxoCoinName, addressType: ScriptType2Of3): boolean { @@ -68,7 +78,9 @@ export function generateAddressWithChainAndIndex( * @param params.index {number} Derivation index * @param params.segwit {boolean} Deprecated * @param params.bech32 {boolean} Deprecated - * @returns {string} The generated address + * @param params.derivationPrefix {string} Absent = legacy `0/0` scheme. Present = must equal the canonical bip44 + * prefix recomputed from `keychains` and `bip44CoinType`, else this throws + * @param params.bip44CoinType {number} Coin's BIP44 coin type; required when `derivationPrefix` is present */ export function generateAddress( coinName: UtxoCoinName | WasmUtxoCoinName, @@ -79,8 +91,14 @@ export function generateAddress( derivationIndex = params.index as number; } - const { keychains, chain, segwit = false, bech32 = false } = params as GenerateFixedScriptAddressOptions; - + const { + keychains, + chain, + segwit = false, + bech32 = false, + derivationPrefix, + bip44CoinType, + } = params as GenerateFixedScriptAddressOptions; if (_.isNumber(chain) && _.isInteger(chain) && !fixedScriptWallet.ChainCode.is(chain)) { throw new InvalidAddressDerivationPropertyError(`address validation failure: unrecognised chain code (${chain})`); } @@ -131,13 +149,15 @@ export function generateAddress( throw new Error('keychains must be a triple'); } - return generateAddressWithChainAndIndex( - coinName, - keychains.map((k) => k.pub) as Triple, - derivationChain, - derivationIndex, - params.format - ); + const keys = keychains.map((k) => k.pub) as Triple; + let walletKeys: fixedScriptWallet.WalletKeysArg = keys; + if (derivationPrefix !== undefined) { + // never derive under a prefix that was merely supplied: it must be the canonical one for these keys + const canonicalPrefix = assertCanonicalDerivationPrefix(keys, derivationPrefix, bip44CoinType); + walletKeys = { triple: keys, derivationPrefixes: [canonicalPrefix, canonicalPrefix, canonicalPrefix] }; + } + + return generateAddressWithChainAndIndex(coinName, walletKeys, derivationChain, derivationIndex, params.format); } type Keychain = { @@ -152,12 +172,18 @@ export function assertFixedScriptWalletAddress( keychains, format, address, + derivationPrefix, + bip44CoinType, }: { chain: number | undefined; index: number; keychains: Keychain[]; format: CreateAddressFormat; address: string; + /** Absent = legacy scheme. Present = must be the canonical bip44 prefix of `keychains`. */ + derivationPrefix?: string; + /** BIP44 coin type of the coin; required when `derivationPrefix` is present. */ + bip44CoinType?: number; } ): void { if ((_.isUndefined(chain) && _.isUndefined(index)) || !(_.isFinite(chain) && _.isFinite(index))) { @@ -175,6 +201,8 @@ export function assertFixedScriptWalletAddress( keychains, chain, index, + derivationPrefix, + bip44CoinType, }); if (expectedAddress !== address) { diff --git a/modules/abstract-utxo/src/address/index.ts b/modules/abstract-utxo/src/address/index.ts index b4466837b6a..4f60a224124 100644 --- a/modules/abstract-utxo/src/address/index.ts +++ b/modules/abstract-utxo/src/address/index.ts @@ -4,3 +4,9 @@ export { assertFixedScriptWalletAddress, FixedScriptAddressCoinSpecific, } from './fixedScript'; +export { + LEGACY_DERIVATION_PREFIX, + assertCanonicalDerivationPrefix, + getDerivationPrefixFromPath, + normalizeDerivationPrefix, +} from './derivationPrefix'; diff --git a/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts b/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts index 03100ac1bd5..b021ef638ff 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts @@ -4,6 +4,7 @@ import { Triple } from '@bitgo/sdk-core'; import type { FixedScriptWalletOutput, Output, BitGoPsbt } from '../types'; import type { Bip322Message } from '../../abstractUtxoCoin'; import { AddressCodec } from '../recipient'; +import { getDerivationPrefixFromPath } from '../../address/derivationPrefix'; import type { TransactionExplanationWasm } from './explainTransaction'; function scriptToAddress(script: Uint8Array): string { @@ -22,12 +23,15 @@ function isParsedExternalOutput(output: ParsedWalletOutput | ParsedExternalOutpu } function toChangeOutputBigInt(output: ParsedWalletOutput): FixedScriptWalletOutput { + // `derivationPath` is the full path recorded in the PSBT; everything but chain/index is the prefix (absent = legacy) + const derivationPrefix = getDerivationPrefixFromPath(output.derivationPath); return { address: output.address ?? scriptToAddress(output.script), amount: output.value, chain: output.scriptId.chain, index: output.scriptId.index, external: false, + ...(derivationPrefix === undefined ? {} : { derivationPrefix }), }; } diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts index 5a079a42dea..1804264dded 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts @@ -253,11 +253,17 @@ export async function parseOutput({ // derivation information we have. Otherwise, if the address is external, which is the only remaining case, we // can just return the current output as is without contacting the server. if (isWalletOutput(currentOutput)) { + // The prefix comes from the derivation path recorded in the PSBT (absent = legacy `0/0`). It is never trusted: + // isWalletAddress only accepts it if it equals the canonical prefix recomputed from the wallet keys, so a + // bip44 change output is recognised as internal and a forged prefix is classified as external. const res = await coin.isWalletAddress({ keychains: keychainArray, address: currentAddress, chain: currentOutput.chain, index: currentOutput.index, + ...(currentOutput.derivationPrefix === undefined + ? {} + : { coinSpecific: { derivationPrefix: currentOutput.derivationPrefix } }), }); if (!res) { throw new UnexpectedAddressError(); diff --git a/modules/abstract-utxo/src/transaction/types.ts b/modules/abstract-utxo/src/transaction/types.ts index 2ed4bdd86ba..a5d648a3a06 100644 --- a/modules/abstract-utxo/src/transaction/types.ts +++ b/modules/abstract-utxo/src/transaction/types.ts @@ -21,6 +21,11 @@ export interface FixedScriptWalletOutput extends Base needsCustomChangeKeySignatureVerification?: boolean; chain: number; index: number; + /** + * Derivation prefix (all path components except `chain/index`) recorded in the PSBT for this output. Absent for + * the legacy `0/0` prefix. Unvalidated: consumers must check it against the canonical prefix of the wallet keys. + */ + derivationPrefix?: string; } export type Output = BaseOutput | FixedScriptWalletOutput; diff --git a/modules/abstract-utxo/test/unit/address.ts b/modules/abstract-utxo/test/unit/address.ts index 362d16d3ad2..a1c7584eb4f 100644 --- a/modules/abstract-utxo/test/unit/address.ts +++ b/modules/abstract-utxo/test/unit/address.ts @@ -1,7 +1,8 @@ import * as assert from 'assert'; -import { InvalidAddressDerivationPropertyError, UnexpectedAddressError } from '@bitgo/sdk-core'; -import { fixedScriptWallet } from '@bitgo/wasm-utxo'; +import * as sinon from 'sinon'; +import { InvalidAddressDerivationPropertyError, Triple, UnexpectedAddressError } from '@bitgo/sdk-core'; +import { BIP32 as WasmBIP32, fixedScriptWallet } from '@bitgo/wasm-utxo'; import { assertFixedScriptWalletAddress, generateAddress } from '../../src'; @@ -229,3 +230,282 @@ describe('AbstractUtxoCoin.deriveAddress', function () { await assert.rejects(async () => coin.deriveAddress({ chain: 0, index: 0 }), /keychains/); }); }); + +describe('bip44 derivation prefix', function () { + // Vectors from the wallet-prefix definition: sha256("BitGo/utxo/wallet-prefix/v1" || K_user || K_backup || K_bitgo) + // with K_x = compressed pubkey || chain code of the neutered root key. + const user = WasmBIP32.fromSeedSha256('wcn3907-u').neutered().toBase58(); + const backup = WasmBIP32.fromSeedSha256('wcn3907-b').neutered().toBase58(); + const bitgo1 = WasmBIP32.fromSeedSha256('wcn3907-g1').neutered().toBase58(); + const bitgo2 = WasmBIP32.fromSeedSha256('wcn3907-g2').neutered().toBase58(); + const wallet1 = [user, backup, bitgo1].map((pub) => ({ pub })); + const wallet2 = [user, backup, bitgo2].map((pub) => ({ pub })); + const prefix1 = (coinType: number) => `44/${coinType}/1528449227/506574854`; + const prefix2 = (coinType: number) => `44/${coinType}/1401193521/285195054`; + + function derive( + coinName: 'btc' | 'ltc' | 'tbtcreg', + wallet: { pub: string }[], + prefix: string | undefined, + chain: number, + index: number + ): string { + const triple = wallet.map((k) => k.pub) as Triple; + return fixedScriptWallet.address( + prefix === undefined ? triple : { triple, derivationPrefixes: [prefix, prefix, prefix] }, + chain as fixedScriptWallet.ChainCode, + index, + coinName + ); + } + + const chains = [0, 10, 20, 40]; + + describe('assertFixedScriptWalletAddress', function () { + for (const chain of chains) { + it(`verifies a bip44 address (chain ${chain})`, function () { + const address = derive('btc', wallet1, prefix1(0), chain, 7); + assertFixedScriptWalletAddress('btc', { + chain, + index: 7, + keychains: wallet1, + format: 'base58', + address, + derivationPrefix: prefix1(0), + bip44CoinType: 0, + }); + }); + + it(`still verifies a legacy address without prefix (chain ${chain})`, function () { + const address = derive('btc', wallet1, undefined, chain, 7); + assertFixedScriptWalletAddress('btc', { + chain, + index: 7, + keychains: wallet1, + format: 'base58', + address, + bip44CoinType: 0, + }); + }); + + it(`rejects a bip44 address presented without prefix (chain ${chain})`, function () { + const address = derive('btc', wallet1, prefix1(0), chain, 7); + assert.throws( + () => + assertFixedScriptWalletAddress('btc', { + chain, + index: 7, + keychains: wallet1, + format: 'base58', + address, + bip44CoinType: 0, + }), + UnexpectedAddressError + ); + }); + + it(`rejects a legacy address presented with the canonical prefix (chain ${chain})`, function () { + const address = derive('btc', wallet1, undefined, chain, 7); + assert.throws( + () => + assertFixedScriptWalletAddress('btc', { + chain, + index: 7, + keychains: wallet1, + format: 'base58', + address, + derivationPrefix: prefix1(0), + bip44CoinType: 0, + }), + UnexpectedAddressError + ); + }); + } + + it('does not resolve the coin type from the wasm coin name (tbtcreg is not a statics coin)', function () { + const address = derive('tbtcreg', wallet1, prefix1(0), 0, 1); + assertFixedScriptWalletAddress('tbtcreg', { + chain: 0, + index: 1, + keychains: wallet1, + format: 'base58', + address, + derivationPrefix: prefix1(0), + bip44CoinType: 0, + }); + }); + + it('uses the coin type of the coin: ltc verifies under 44/2/..', function () { + const address = derive('ltc', wallet1, prefix1(2), 0, 1); + assertFixedScriptWalletAddress('ltc', { + chain: 0, + index: 1, + keychains: wallet1, + format: 'base58', + address, + derivationPrefix: prefix1(2), + bip44CoinType: 2, + }); + }); + + it('rejects a prefix computed for a different coin type', function () { + const address = derive('ltc', wallet1, prefix1(0), 0, 1); + assert.throws( + () => + assertFixedScriptWalletAddress('ltc', { + chain: 0, + index: 1, + keychains: wallet1, + format: 'base58', + address, + derivationPrefix: prefix1(0), + bip44CoinType: 2, + }), + UnexpectedAddressError + ); + }); + + it('rejects a prefix that belongs to another wallet (tag/keys mismatch), even if the address matches the prefix', function () { + // the address really is derived under wallet2's canonical prefix, but verified against wallet1's keys + const address = derive('btc', wallet1, prefix2(0), 0, 1); + assert.throws( + () => + assertFixedScriptWalletAddress('btc', { + chain: 0, + index: 1, + keychains: wallet1, + format: 'base58', + address, + derivationPrefix: prefix2(0), + bip44CoinType: 0, + }), + UnexpectedAddressError + ); + }); + + it('rejects a non-bip44 prefix such as the legacy 0/0', function () { + const address = derive('btc', wallet1, undefined, 0, 1); + assert.throws( + () => + assertFixedScriptWalletAddress('btc', { + chain: 0, + index: 1, + keychains: wallet1, + format: 'base58', + address, + derivationPrefix: '0/0', + bip44CoinType: 0, + }), + UnexpectedAddressError + ); + }); + + it('throws (not skips) when a prefix is given but the coin has no bip44 coin type', function () { + const address = derive('btc', wallet1, prefix1(0), 0, 1); + assert.throws( + () => + assertFixedScriptWalletAddress('btc', { + chain: 0, + index: 1, + keychains: wallet1, + format: 'base58', + address, + derivationPrefix: prefix1(0), + bip44CoinType: undefined, + }), + /no bip44 coin type/ + ); + }); + }); + + describe('AbstractUtxoCoin', function () { + for (const coinName of ['btc', 'tbtc', 'tbtcsig', 'tbtcbgsig']) { + it(`${coinName}: deriveAddress honours coinSpecific.derivationPrefix and isWalletAddress verifies it`, async function () { + const coin = getUtxoCoin(coinName); + const coinSpecific = { derivationPrefix: prefix1(0) }; + const derived = await coin.deriveAddress({ keychains: wallet1, chain: 20, index: 4, coinSpecific }); + const legacy = await coin.deriveAddress({ keychains: wallet1, chain: 20, index: 4 }); + assert.notStrictEqual(derived.address, legacy.address); + + assert.strictEqual( + await coin.isWalletAddress({ + address: derived.address, + keychains: wallet1, + chain: 20, + index: 4, + coinSpecific, + }), + true + ); + assert.strictEqual( + await coin.isWalletAddress({ address: legacy.address, keychains: wallet1, chain: 20, index: 4 }), + true + ); + // strict in both directions + await assert.rejects( + coin.isWalletAddress({ address: derived.address, keychains: wallet1, chain: 20, index: 4 }), + UnexpectedAddressError + ); + await assert.rejects( + coin.isWalletAddress({ address: legacy.address, keychains: wallet1, chain: 20, index: 4, coinSpecific }), + UnexpectedAddressError + ); + }); + } + + it('ltc derives under 44/2/..', async function () { + const coin = getUtxoCoin('ltc'); + const derived = await coin.deriveAddress({ + keychains: wallet1, + chain: 0, + index: 0, + coinSpecific: { derivationPrefix: prefix1(2) }, + }); + derived.address.should.equal(derive('ltc', wallet1, prefix1(2), 0, 0)); + }); + + it('isWalletAddress and deriveAddress reject a prefix of another wallet', async function () { + const coin = getUtxoCoin('btc'); + const coinSpecific = { derivationPrefix: prefix2(0) }; + await assert.rejects( + coin.deriveAddress({ keychains: wallet1, chain: 0, index: 0, coinSpecific }), + UnexpectedAddressError + ); + await assert.rejects( + coin.isWalletAddress({ + address: derive('btc', wallet1, prefix2(0), 0, 0), + keychains: wallet1, + chain: 0, + index: 0, + coinSpecific, + }), + UnexpectedAddressError + ); + }); + + it('throws when the coin has no bip44 coin type; allowSkipVerifyAddress cannot skip it', async function () { + const coin = getUtxoCoin('btc'); + const config = coin.getConfig(); + const stub = sinon.stub(coin, 'getConfig').returns({ ...config, bip44CoinType: undefined } as typeof config); + try { + const coinSpecific = { derivationPrefix: prefix1(0) }; + await assert.rejects( + coin.isWalletAddress({ + address: derive('btc', wallet1, prefix1(0), 0, 0), + keychains: wallet1, + chain: 0, + index: 0, + coinSpecific, + }), + /no bip44 coin type/ + ); + await assert.rejects( + coin.deriveAddress({ keychains: wallet1, chain: 0, index: 0, coinSpecific }), + /no bip44 coin type/ + ); + } finally { + stub.restore(); + } + }); + }); +}); diff --git a/modules/abstract-utxo/test/unit/transaction/fixedScript/parseOutputDerivationPrefix.ts b/modules/abstract-utxo/test/unit/transaction/fixedScript/parseOutputDerivationPrefix.ts new file mode 100644 index 00000000000..e7d259928e7 --- /dev/null +++ b/modules/abstract-utxo/test/unit/transaction/fixedScript/parseOutputDerivationPrefix.ts @@ -0,0 +1,91 @@ +import assert from 'node:assert/strict'; + +import * as sinon from 'sinon'; +import { IWallet, Triple, UnexpectedAddressError } from '@bitgo/sdk-core'; + +import { getDerivationPrefixFromPath, LEGACY_DERIVATION_PREFIX } from '../../../../src/address/derivationPrefix'; +import { parseOutput } from '../../../../src/transaction/fixedScript/parseOutput'; +import type { FixedScriptWalletOutput } from '../../../../src/transaction/types'; +import { getUtxoCoin, keychainsBase58 } from '../../util'; + +describe('getDerivationPrefixFromPath', function () { + it('returns undefined for chain/index only and for the legacy prefix', function () { + assert.equal(getDerivationPrefixFromPath(undefined), undefined); + assert.equal(getDerivationPrefixFromPath(null), undefined); + assert.equal(getDerivationPrefixFromPath(''), undefined); + assert.equal(getDerivationPrefixFromPath('0/1'), undefined); + assert.equal(getDerivationPrefixFromPath(`${LEGACY_DERIVATION_PREFIX}/1/5`), undefined); + assert.equal(getDerivationPrefixFromPath('m/0/0/20/5'), undefined); + assert.equal(getDerivationPrefixFromPath('/0/0/20/5'), undefined); + }); + + it('returns every component except the last two, without a leading m/ or /', function () { + assert.equal(getDerivationPrefixFromPath('44/0/1528449227/506574854/1/9'), '44/0/1528449227/506574854'); + assert.equal(getDerivationPrefixFromPath('m/44/0/1528449227/506574854/1/9'), '44/0/1528449227/506574854'); + assert.equal(getDerivationPrefixFromPath('/44/0/1528449227/506574854/1/9'), '44/0/1528449227/506574854'); + }); + + it('returns other (non-legacy) prefixes unvalidated so that the canonical check can reject them', function () { + assert.equal(getDerivationPrefixFromPath('m/0/1/20/5'), '0/1'); + }); +}); + +describe('parseOutput derivation prefix', function () { + const coin = getUtxoCoin('btc'); + const keychainArray = keychainsBase58.map((k) => ({ pub: k.pub })) as Triple<{ pub: string }>; + const wallet = { migratedFrom: () => undefined } as unknown as IWallet; + const prefix = '44/0/1528449227/506574854'; + + async function parse(output: FixedScriptWalletOutput) { + return parseOutput({ + currentOutput: output, + coin, + txPrebuild: {}, + verification: {}, + keychainArray, + wallet, + addressCodec: coin.addressCodec, + txParams: { recipients: [] }, + }); + } + + afterEach(function () { + sinon.restore(); + }); + + it('passes the prefix recorded in the PSBT to isWalletAddress as coinSpecific.derivationPrefix', async function () { + const isWalletAddress = sinon.stub(coin, 'isWalletAddress').resolves(true); + const output = await parse({ + address: 'bc1qexample', + amount: '1000', + external: false, + chain: 21, + index: 3, + derivationPrefix: prefix, + }); + assert.equal(output.external, false); + const args = isWalletAddress.getCall(0).args[0]; + assert.equal(args.chain, 21); + assert.equal(args.index, 3); + assert.deepEqual(args.coinSpecific, { derivationPrefix: prefix }); + }); + + it('does not pass a coinSpecific for legacy outputs', async function () { + const isWalletAddress = sinon.stub(coin, 'isWalletAddress').resolves(true); + await parse({ address: 'bc1qexample', amount: '1000', external: false, chain: 21, index: 3 }); + assert.equal(isWalletAddress.getCall(0).args[0].coinSpecific, undefined); + }); + + it('classifies an output whose prefix is not canonical as external', async function () { + sinon.stub(coin, 'isWalletAddress').rejects(new UnexpectedAddressError('not canonical')); + const output = await parse({ + address: 'bc1qexample', + amount: '1000', + external: false, + chain: 21, + index: 3, + derivationPrefix: prefix, + }); + assert.equal(output.external, true); + }); +}); diff --git a/modules/express/src/typedRoutes/api/v2/deriveAddress.ts b/modules/express/src/typedRoutes/api/v2/deriveAddress.ts index 9bcc8813682..d33c0d796cd 100644 --- a/modules/express/src/typedRoutes/api/v2/deriveAddress.ts +++ b/modules/express/src/typedRoutes/api/v2/deriveAddress.ts @@ -52,11 +52,15 @@ export const DeriveAddressBody = { /** * Coin-specific derivation inputs. For EVM legacy forwarder wallets: * `forwarderVersion` (1/2/4) selects the forwarder factory, and `feeAddress` is required for v4. + * For UTXO multisig wallets: `derivationPrefix` (address.coinSpecific.derivationPrefix) selects the BIP44-style + * per-wallet scheme (`44//

/

`); absent means the legacy scheme. The prefix is never used + * as given: it must equal the canonical prefix recomputed from the keychains, otherwise the request fails. */ coinSpecific: optional( t.partial({ forwarderVersion: t.number, feeAddress: t.string, + derivationPrefix: t.string, }) ), /** diff --git a/modules/express/src/typedRoutes/api/v2/isWalletAddress.ts b/modules/express/src/typedRoutes/api/v2/isWalletAddress.ts index 7fd9d4fc211..202d6dd11cd 100644 --- a/modules/express/src/typedRoutes/api/v2/isWalletAddress.ts +++ b/modules/express/src/typedRoutes/api/v2/isWalletAddress.ts @@ -55,6 +55,8 @@ export const IsWalletAddressBody = { * BaseAddress: 0 */ index: optional(t.union([t.number, t.string])), + /** Derivation chain code of the address (address.chain); required to verify UTXO multisig addresses */ + chain: optional(t.number), /** Coin-specific address data */ coinSpecific: optional( t.partial({ @@ -73,6 +75,13 @@ export const IsWalletAddressBody = { /** Base address (wallet.coinSpecific.baseAddress) */ baseAddress: t.string, + + /** + * UTXO multisig wallets: derivation prefix of the address (address.coinSpecific.derivationPrefix, + * `44//

/

`). Absent means the legacy scheme. The prefix must equal the canonical + * prefix recomputed from the keychains and the coin's BIP44 coin type; any other value fails verification. + */ + derivationPrefix: t.string, }) ), /** Implied forwarder version */ diff --git a/modules/express/test/unit/typedRoutes/deriveAddress.ts b/modules/express/test/unit/typedRoutes/deriveAddress.ts index 72fa03ad467..578824349c6 100644 --- a/modules/express/test/unit/typedRoutes/deriveAddress.ts +++ b/modules/express/test/unit/typedRoutes/deriveAddress.ts @@ -12,6 +12,7 @@ import 'should'; import 'should-http'; import 'should-sinon'; import * as sinon from 'sinon'; +import { BaseCoin } from '@bitgo/sdk-core'; import { BitGo } from 'bitgo'; import { setupAgent } from '../../lib/testutil'; @@ -108,6 +109,28 @@ describe('DeriveAddress codec tests', function () { assert.strictEqual(decoded.coinSpecific?.feeAddress, validBody.coinSpecific.feeAddress); }); + it('should validate a UTXO body with coinSpecific.derivationPrefix', function () { + const validBody = { + keychains: [{ pub: 'xpub1...' }, { pub: 'xpub2...' }, { pub: 'xpub3...' }], + index: 3, + chain: 0, + coinSpecific: { derivationPrefix: '44/0/1528449227/506574854' }, + }; + + const decoded = assertDecode(t.type(DeriveAddressBody), validBody); + assert.strictEqual(decoded.coinSpecific?.derivationPrefix, '44/0/1528449227/506574854'); + }); + + it('should reject a non-string coinSpecific.derivationPrefix', function () { + assert.throws(() => { + assertDecode(t.type(DeriveAddressBody), { + keychains: [{ pub: 'xpub1...' }], + index: 3, + coinSpecific: { derivationPrefix: 44 }, + }); + }); + }); + it('should reject body with missing index', function () { assert.throws(() => { assertDecode(t.type(DeriveAddressBody), { keychains: [{ pub: 'xpub1...' }] }); @@ -218,6 +241,29 @@ describe('DeriveAddress codec tests', function () { assert.strictEqual(callArgs.chain, 20); }); + it('should pass coinSpecific.derivationPrefix through to coin.deriveAddress', async function () { + const derivationPrefix = '44/0/1528449227/506574854'; + const requestBody = { + keychains: [{ pub: 'xpub1...' }, { pub: 'xpub2...' }, { pub: 'xpub3...' }], + chain: 20, + index: 7, + coinSpecific: { derivationPrefix }, + }; + + const deriveAddressStub = sinon.stub().resolves({ address: 'bc1qbip44address', chain: 20, index: 7 }); + sinon.stub(BitGo.prototype, 'coin').returns({ deriveAddress: deriveAddressStub } as unknown as BaseCoin); + + const result = await agent + .post('/api/v2/btc/address/derive') + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send(requestBody); + + assert.strictEqual(result.status, 200); + sinon.assert.calledOnce(deriveAddressStub); + assert.strictEqual(deriveAddressStub.firstCall.args[0].coinSpecific.derivationPrefix, derivationPrefix); + }); + it('should derive a TSS/MPC address and pass derivedFromParentWithSeed through', async function () { const requestBody = { keychains: [{ commonKeychain }, { commonKeychain }, { commonKeychain }], diff --git a/modules/express/test/unit/typedRoutes/isWalletAddress.ts b/modules/express/test/unit/typedRoutes/isWalletAddress.ts index 17a588280b2..feb63b61833 100644 --- a/modules/express/test/unit/typedRoutes/isWalletAddress.ts +++ b/modules/express/test/unit/typedRoutes/isWalletAddress.ts @@ -12,6 +12,7 @@ import 'should'; import 'should-http'; import 'should-sinon'; import * as sinon from 'sinon'; +import { BaseCoin } from '@bitgo/sdk-core'; import { BitGo } from 'bitgo'; import { setupAgent } from '../../lib/testutil'; @@ -292,6 +293,28 @@ describe('IsWalletAddress codec tests', function () { assert.strictEqual(decodedString.index, '7'); }); + it('should accept chain and coinSpecific.derivationPrefix for UTXO addresses', function () { + const decoded = assertDecode(t.type(IsWalletAddressBody), { + address: 'bc1qbip44address', + keychains: [{ pub: 'xpub1...' }, { pub: 'xpub2...' }, { pub: 'xpub3...' }], + chain: 20, + index: 7, + coinSpecific: { derivationPrefix: '44/0/1528449227/506574854' }, + }); + assert.strictEqual(decoded.chain, 20); + assert.strictEqual(decoded.coinSpecific?.derivationPrefix, '44/0/1528449227/506574854'); + }); + + it('should reject a non-numeric chain', function () { + assert.throws(() => { + assertDecode(t.type(IsWalletAddressBody), { + address: 'bc1qbip44address', + keychains: [{ pub: 'xpub1...' }], + chain: '20', + }); + }); + }); + it('should reject body with invalid derivedFromParentWithSeed type', function () { const invalidBody = { address: '0x6069a4baf2360bf67a6d02a7fc43d8f3910016ae', @@ -410,6 +433,39 @@ describe('IsWalletAddress codec tests', function () { sinon.assert.calledOnce(isWalletAddressStub); }); + it('should pass chain and coinSpecific.derivationPrefix through for a UTXO address', async function () { + const derivationPrefix = '44/0/1528449227/506574854'; + const requestBody = { + address: 'bc1qbip44address', + keychains: [{ pub: 'xpub1...' }, { pub: 'xpub2...' }, { pub: 'xpub3...' }], + chain: 20, + index: 7, + coinSpecific: { derivationPrefix }, + }; + + const isWalletAddressStub = sinon.stub().resolves(true); + const mockCoin = { + wallets: sinon + .stub() + .returns({ get: sinon.stub().resolves({ baseCoin: { isWalletAddress: isWalletAddressStub } }) }), + }; + sinon.stub(BitGo.prototype, 'coin').returns(mockCoin as unknown as BaseCoin); + + const result = await agent + .post('/api/v2/btc/wallet/test-wallet-id/iswalletaddress') + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send(requestBody); + + assert.strictEqual(result.status, 200); + assert.strictEqual(result.body, true); + sinon.assert.calledOnce(isWalletAddressStub); + const callArgs = isWalletAddressStub.firstCall.args[0]; + assert.strictEqual(callArgs.chain, 20); + assert.strictEqual(callArgs.index, 7); + assert.strictEqual(callArgs.coinSpecific.derivationPrefix, derivationPrefix); + }); + it('should verify forwarder address for wallet version 2', async function () { const requestBody = { address: '0xf636ceddffe41d106586875c0e56dc8feb6268f7', diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index cb4ec7f61c7..9379e7a9925 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -533,6 +533,12 @@ export interface AddressCoinSpecific { forwarderVersion?: number; /** Fee address for v4 EVM forwarders (used when deriving/verifying v4 forwarder addresses). */ feeAddress?: string; + /** + * UTXO fixed-script wallets: relative, non-hardened derivation prefix shared by all three root keys + * (`44//

/

`). Absent means the legacy scheme (the wallet's existing `0/0` key path). + * Verifiers MUST recompute the canonical prefix from the wallet keys and reject any other value. + */ + derivationPrefix?: string; /** * Zcash shielded (Orchard/Ironwood) receiver data reported by wallet-platform. Present * only for shielded addresses; see `assertShieldedWalletAddress`. diff --git a/modules/sdk-core/test/unit/bitgo/wallet/walletUtxoAddressVerification.ts b/modules/sdk-core/test/unit/bitgo/wallet/walletUtxoAddressVerification.ts new file mode 100644 index 00000000000..954eac71f76 --- /dev/null +++ b/modules/sdk-core/test/unit/bitgo/wallet/walletUtxoAddressVerification.ts @@ -0,0 +1,98 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import 'should'; +import { Wallet } from '../../../../src/bitgo/wallet/wallet'; +import { BitGoBase } from '../../../../src/bitgo/bitgoBase'; +import { IBaseCoin } from '../../../../src/bitgo/baseCoin'; + +describe('Wallet - UTXO address verification with coinSpecific.derivationPrefix', function () { + const derivationPrefix = '44/0/1528449227/506574854'; + let wallet: Wallet; + let mockBitGo: { post: sinon.SinonStub; setRequestTracer: sinon.SinonStub }; + let mockBaseCoin: Record; + + function mockCreatedAddress(address: unknown) { + mockBitGo.post.returns({ + send: sinon.stub().returns({ + result: sinon.stub().resolves(address), + }), + }); + } + + beforeEach(function () { + mockBitGo = { + post: sinon.stub(), + setRequestTracer: sinon.stub(), + }; + + mockBaseCoin = { + getFamily: sinon.stub().returns('btc'), + isEVM: sinon.stub().returns(false), + isWalletAddress: sinon.stub(), + keychains: sinon.stub().returns({ + get: sinon.stub().callsFake(({ id }: { id: string }) => Promise.resolve({ id, pub: `xpub-${id}` })), + }), + url: sinon.stub().returns('/api/v2/btc/wallet/test-wallet-id/address'), + }; + + wallet = new Wallet(mockBitGo as unknown as BitGoBase, mockBaseCoin as unknown as IBaseCoin, { + id: 'test-wallet-id', + keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], + coinSpecific: {}, + }); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('forwards the whole address coinSpecific, including derivationPrefix, to isWalletAddress', async function () { + mockCreatedAddress({ + id: 'address-id', + address: 'bc1qexample', + chain: 20, + index: 4, + coinSpecific: { witnessScript: 'aa', outputScript: 'bb', derivationPrefix }, + }); + mockBaseCoin.isWalletAddress.resolves(true); + + await wallet.createAddress({ chain: 20 }); + + const verificationData = mockBaseCoin.isWalletAddress.getCall(0).args[0]; + verificationData.coinSpecific.derivationPrefix.should.equal(derivationPrefix); + verificationData.chain.should.equal(20); + verificationData.index.should.equal(4); + verificationData.keychains + .map((k: { pub: string }) => k.pub) + .should.deepEqual(['xpub-user-key-id', 'xpub-backup-key-id', 'xpub-bitgo-key-id']); + }); + + it('does not add a derivationPrefix for legacy addresses', async function () { + mockCreatedAddress({ + id: 'address-id', + address: 'bc1qexample', + chain: 20, + index: 4, + coinSpecific: { witnessScript: 'aa', outputScript: 'bb' }, + }); + mockBaseCoin.isWalletAddress.resolves(true); + + await wallet.createAddress({ chain: 20 }); + + const verificationData = mockBaseCoin.isWalletAddress.getCall(0).args[0]; + assert.strictEqual(verificationData.coinSpecific.derivationPrefix, undefined); + }); + + it('propagates a verification failure for a prefixed address instead of skipping it', async function () { + mockCreatedAddress({ + id: 'address-id', + address: 'bc1qexample', + chain: 20, + index: 4, + coinSpecific: { derivationPrefix }, + }); + mockBaseCoin.isWalletAddress.rejects(new Error('cannot verify derivationPrefix')); + + await wallet.createAddress({ chain: 20, allowSkipVerifyAddress: true }).should.be.rejectedWith(/derivationPrefix/); + }); +}); diff --git a/modules/statics/test/unit/bip44CoinTypes.ts b/modules/statics/test/unit/bip44CoinTypes.ts index 33caa2da446..3a59fff5ada 100644 --- a/modules/statics/test/unit/bip44CoinTypes.ts +++ b/modules/statics/test/unit/bip44CoinTypes.ts @@ -28,6 +28,43 @@ describe('bip44 coin types', function () { should(coins.get('usdc').bip44CoinType).equal(coins.get('eth').bip44CoinType); }); + // Pinned values: the UTXO per-wallet derivation prefix (`44//

/

`) embeds these, and + // recovery scans only the canonical prefix, so a change here would make existing bip44 addresses unrecoverable. + describe('UTXO coin types are pinned', function () { + const pinned: Record = { + btc: 0, + tbtc: 0, + tbtc4: 0, + tbtcsig: 0, + tbtcbgsig: 0, + tbtcstx: 0, + tbtcstxprivate1: 0, + tbtcstxsignet: 0, + ltc: 2, + tltc: 2, + doge: 3, + tdoge: 3, + dash: 5, + tdash: 5, + zec: 133, + tzec: 133, + bch: 145, + tbch: 145, + btg: 156, + tbtg: 156, + bsv: 236, + tbsv: 236, + bcha: 899, + tbcha: 899, + }; + + Object.entries(pinned).forEach(([name, coinType]) => { + it(`${name} should be ${coinType}`, function () { + should(coins.get(name).bip44CoinType).equal(coinType); + }); + }); + }); + it('every BIP44-derivable coin should have a bip44CoinType', function () { const missing = new Set(); coins.forEach((coin) => {