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) => {