From 9ab55927d332122ff8353cd15c77025486308575 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Wed, 20 May 2026 11:06:01 +0200 Subject: [PATCH 1/2] test(abstract-utxo): document silent P2SH fallback on unknown chain code Adds two tests to assertFixedScriptWalletAddress that reproduce the class of failure described in T1-3386 / T1-3385: - generateAddress silently returns a P2SH address for any unrecognised chain code (e.g. a future chain=99) instead of throwing - assertFixedScriptWalletAddress then throws a misleading UnexpectedAddressError that mentions a P2SH address rather than the unrecognised chain code These tests document current (broken) behaviour and should be updated when T1-3386 is fixed. Refs: T1-3386 --- modules/abstract-utxo/test/unit/address.ts | 57 ++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/modules/abstract-utxo/test/unit/address.ts b/modules/abstract-utxo/test/unit/address.ts index 3e64975ba0..3a2854298f 100644 --- a/modules/abstract-utxo/test/unit/address.ts +++ b/modules/abstract-utxo/test/unit/address.ts @@ -1,6 +1,7 @@ import * as assert from 'assert'; import { InvalidAddressDerivationPropertyError, UnexpectedAddressError } from '@bitgo/sdk-core'; +import { fixedScriptWallet } from '@bitgo/wasm-utxo'; import { assertFixedScriptWalletAddress, generateAddress } from '../../src'; @@ -8,6 +9,10 @@ import { keychainsBase58 } from './util'; const keychains = keychainsBase58.map((k) => ({ pub: k.pub })); +// A chain code that no released SDK version has ever defined — simulates a future +// server-side address type unknown to an older client. +const unknownChainCode = 99; + describe('assertFixedScriptWalletAddress', function () { describe('input validation', function () { it('throws InvalidAddressDerivationPropertyError when both chain and index are undefined', function () { @@ -121,6 +126,58 @@ describe('assertFixedScriptWalletAddress', function () { ); }); + // Regression: an unknown chain code must never silently fall back to P2SH. + // If the server returns a new chain code that this SDK version does not + // recognise, the validation should fail loudly rather than derive a P2SH + // address and produce a confusing mismatch error. + // + // Historical instance: bitgo@18.0.1 did not know chain=40 (p2trMusig2). + // When the server returned { chain: 40, address: "tb1p..." }, the client + // fell back to chain=0, derived a P2SH address, and threw + // UnexpectedAddressError("expected 2NEU8... but got tb1p...") — hiding the + // real cause entirely. + it('throws UnexpectedAddressError for an unknown chain code, revealing the P2SH fallback', function () { + assert.ok(!fixedScriptWallet.ChainCode.is(unknownChainCode), 'test prerequisite: chain must be unknown'); + + // The server-side address for chain=40 (p2trMusig2) — what a newer server + // would return when this older client does not know about chain=40 yet. + const serverAddress = generateAddress('btc', { keychains, chain: 40 }); + const p2shFallbackAddress = generateAddress('btc', { keychains, chain: 0 }); + + let err: unknown; + try { + assertFixedScriptWalletAddress('btc', { + chain: unknownChainCode, + index: 0, + keychains, + format: 'base58', + address: serverAddress, + }); + } catch (e) { + err = e; + } + + assert.ok(err instanceof UnexpectedAddressError); + + // The error message reveals the fallback: it mentions the P2SH address + // rather than anything about the unrecognised chain code. + assert.ok( + err.message.includes(p2shFallbackAddress), + `expected error to mention the P2SH fallback address (${p2shFallbackAddress}), got: ${err.message}` + ); + assert.ok( + err.message.includes(serverAddress), + `expected error to mention the server address (${serverAddress}), got: ${err.message}` + ); + }); + + it('generateAddress silently produces a P2SH address for an unknown chain code', function () { + assert.ok(!fixedScriptWallet.ChainCode.is(unknownChainCode), 'test prerequisite: chain must be unknown'); + const fallback = generateAddress('btc', { keychains, chain: unknownChainCode }); + const p2sh = generateAddress('btc', { keychains, chain: 0 }); + assert.strictEqual(fallback, p2sh); + }); + it('succeeds for bch cashaddr (chain 0)', function () { const address = generateAddress('bch', { keychains, chain: 0, format: 'cashaddr' }); assert.doesNotThrow(() => From edbb6beced44f6db64963632ed619fc6516a534c Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Wed, 20 May 2026 11:18:29 +0200 Subject: [PATCH 2/2] fix(abstract-utxo): throw on unrecognised chain code generateAddress previously fell back to chain=0 (P2SH) when given an integer chain code that ChainCode.is() did not recognise. This produced a misleading UnexpectedAddressError mentioning a P2SH address rather than anything about the unknown chain, as seen in T1-3385 where bitgo@18.0.1 did not know chain=40 (p2trMusig2). Now throws InvalidAddressDerivationPropertyError immediately, naming the unrecognised chain code so the caller knows to upgrade their SDK. Refs: T1-3386 --- .../abstract-utxo/src/address/fixedScript.ts | 4 ++ modules/abstract-utxo/test/unit/address.ts | 70 +++++++------------ 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/modules/abstract-utxo/src/address/fixedScript.ts b/modules/abstract-utxo/src/address/fixedScript.ts index 6c77abbf9e..fd3002124a 100644 --- a/modules/abstract-utxo/src/address/fixedScript.ts +++ b/modules/abstract-utxo/src/address/fixedScript.ts @@ -78,6 +78,10 @@ export function generateAddress(coinName: UtxoCoinName, params: GenerateFixedScr const { keychains, chain, segwit = false, bech32 = false } = params as GenerateFixedScriptAddressOptions; + if (_.isNumber(chain) && _.isInteger(chain) && !fixedScriptWallet.ChainCode.is(chain)) { + throw new InvalidAddressDerivationPropertyError(`address validation failure: unrecognised chain code (${chain})`); + } + let derivationChain: ChainCode = fixedScriptWallet.ChainCode.value('p2sh', 'external'); if (_.isNumber(chain) && _.isInteger(chain) && fixedScriptWallet.ChainCode.is(chain)) { derivationChain = chain; diff --git a/modules/abstract-utxo/test/unit/address.ts b/modules/abstract-utxo/test/unit/address.ts index 3a2854298f..8a66840f64 100644 --- a/modules/abstract-utxo/test/unit/address.ts +++ b/modules/abstract-utxo/test/unit/address.ts @@ -126,56 +126,38 @@ describe('assertFixedScriptWalletAddress', function () { ); }); - // Regression: an unknown chain code must never silently fall back to P2SH. - // If the server returns a new chain code that this SDK version does not - // recognise, the validation should fail loudly rather than derive a P2SH - // address and produce a confusing mismatch error. - // - // Historical instance: bitgo@18.0.1 did not know chain=40 (p2trMusig2). - // When the server returned { chain: 40, address: "tb1p..." }, the client - // fell back to chain=0, derived a P2SH address, and threw - // UnexpectedAddressError("expected 2NEU8... but got tb1p...") — hiding the - // real cause entirely. - it('throws UnexpectedAddressError for an unknown chain code, revealing the P2SH fallback', function () { + // Regression guard for T1-3386 / T1-3385: an unknown chain code must throw + // immediately with a clear message rather than silently falling back to P2SH + // and producing a confusing "expected but got " error. + it('throws InvalidAddressDerivationPropertyError for an unknown chain code', function () { assert.ok(!fixedScriptWallet.ChainCode.is(unknownChainCode), 'test prerequisite: chain must be unknown'); - - // The server-side address for chain=40 (p2trMusig2) — what a newer server - // would return when this older client does not know about chain=40 yet. const serverAddress = generateAddress('btc', { keychains, chain: 40 }); - const p2shFallbackAddress = generateAddress('btc', { keychains, chain: 0 }); - - let err: unknown; - try { - assertFixedScriptWalletAddress('btc', { - chain: unknownChainCode, - index: 0, - keychains, - format: 'base58', - address: serverAddress, - }); - } catch (e) { - err = e; - } - - assert.ok(err instanceof UnexpectedAddressError); - - // The error message reveals the fallback: it mentions the P2SH address - // rather than anything about the unrecognised chain code. - assert.ok( - err.message.includes(p2shFallbackAddress), - `expected error to mention the P2SH fallback address (${p2shFallbackAddress}), got: ${err.message}` - ); - assert.ok( - err.message.includes(serverAddress), - `expected error to mention the server address (${serverAddress}), got: ${err.message}` + assert.throws( + () => + assertFixedScriptWalletAddress('btc', { + chain: unknownChainCode, + index: 0, + keychains, + format: 'base58', + address: serverAddress, + }), + (err: unknown) => { + assert.ok(err instanceof InvalidAddressDerivationPropertyError); + assert.ok( + err.message.includes(String(unknownChainCode)), + `expected error to name the unrecognised chain code, got: ${err.message}` + ); + return true; + } ); }); - it('generateAddress silently produces a P2SH address for an unknown chain code', function () { + it('generateAddress throws InvalidAddressDerivationPropertyError for an unknown chain code', function () { assert.ok(!fixedScriptWallet.ChainCode.is(unknownChainCode), 'test prerequisite: chain must be unknown'); - const fallback = generateAddress('btc', { keychains, chain: unknownChainCode }); - const p2sh = generateAddress('btc', { keychains, chain: 0 }); - assert.strictEqual(fallback, p2sh); + assert.throws( + () => generateAddress('btc', { keychains, chain: unknownChainCode }), + InvalidAddressDerivationPropertyError + ); }); it('succeeds for bch cashaddr (chain 0)', function () {