Skip to content

Commit bcfd011

Browse files
Merge pull request #7537 from BitGo/BTC-2732.testnet-default-to-psbt
feat(abstract-utxo): improve transaction format selection with PSBT defaults
2 parents 7c79047 + 7f4c3b2 commit bcfd011

File tree

3 files changed

+220
-28
lines changed

3 files changed

+220
-28
lines changed

modules/abstract-utxo/src/abstractUtxoCoin.ts

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,19 @@ import { isDescriptorWalletData } from './descriptor/descriptorWallet';
8484

8585
import ScriptType2Of3 = utxolib.bitgo.outputScripts.ScriptType2Of3;
8686

87+
export type TxFormat =
88+
// This is a legacy transaction format based around the bitcoinjs-lib serialization of unsigned transactions
89+
// does not include prevOut data and is a bit painful to work with
90+
// going to be deprecated in favor of psbt
91+
// @deprecated
92+
| 'legacy'
93+
// This is the standard psbt format, including the full prevTx data for legacy transactions.
94+
// This will remain supported but is not the default, since the data sizes can become prohibitively large.
95+
| 'psbt'
96+
// This is a nonstandard psbt version where legacy inputs are serialized as if they were segwit inputs.
97+
// While this prevents us to fully verify the transaction fee, we have other checks in place to ensure the fee is within bounds.
98+
| 'psbt-lite';
99+
87100
type UtxoCustomSigningFunction<TNumber extends number | bigint> = {
88101
(params: {
89102
coin: IBaseCoin;
@@ -957,37 +970,43 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
957970
};
958971
}
959972

960-
private shouldDefaultToPsbtTxFormat(buildParams: ExtraPrebuildParamsOptions & { wallet: Wallet }) {
961-
const walletFlagMusigKp = buildParams.wallet.flag('musigKp') === 'true';
962-
const isHotWallet = buildParams.wallet.type() === 'hot';
963-
964-
// if not txFormat is already specified figure out if we should default to psbt format
965-
return (
966-
buildParams.txFormat === undefined &&
967-
(buildParams.wallet.subType() === 'distributedCustody' ||
968-
// default to testnet for all utxo coins except zcash
969-
(isTestnet(this.network) &&
970-
// FIXME(BTC-1322): fix zcash PSBT support
971-
getMainnet(this.network) !== utxolib.networks.zcash &&
972-
isHotWallet) ||
973-
// if mainnet, only default to psbt for btc hot wallets
974-
(isMainnet(this.network) && getMainnet(this.network) === utxolib.networks.bitcoin && isHotWallet) ||
975-
// default to psbt if it has the wallet flag
976-
walletFlagMusigKp)
977-
);
973+
/**
974+
* Determines the default transaction format based on wallet properties and network
975+
* @param wallet - The wallet to check
976+
* @param requestedFormat - Optional explicitly requested format
977+
* @returns The transaction format to use, or undefined if no default applies
978+
*/
979+
getDefaultTxFormat(wallet: Wallet, requestedFormat?: TxFormat): TxFormat | undefined {
980+
// If format is explicitly requested, use it
981+
if (requestedFormat !== undefined) {
982+
return requestedFormat;
983+
}
984+
985+
if (isTestnet(this.network)) {
986+
return 'psbt-lite';
987+
}
988+
989+
const walletFlagMusigKp = wallet.flag('musigKp') === 'true';
990+
const isHotWallet = wallet.type() === 'hot';
991+
992+
// Determine if we should default to psbt format
993+
const shouldDefaultToPsbt =
994+
wallet.subType() === 'distributedCustody' ||
995+
// if mainnet, only default to psbt for btc hot wallets
996+
(isMainnet(this.network) && getMainnet(this.network) === utxolib.networks.bitcoin && isHotWallet) ||
997+
// default to psbt if it has the wallet flag
998+
walletFlagMusigKp;
999+
1000+
return shouldDefaultToPsbt ? 'psbt' : undefined;
9781001
}
9791002

9801003
async getExtraPrebuildParams(buildParams: ExtraPrebuildParamsOptions & { wallet: Wallet }): Promise<{
981-
txFormat?: 'legacy' | 'psbt';
1004+
txFormat?: TxFormat;
9821005
changeAddressType?: ScriptType2Of3[] | ScriptType2Of3;
9831006
}> {
984-
let txFormat = buildParams.txFormat as 'legacy' | 'psbt' | undefined;
1007+
const txFormat = this.getDefaultTxFormat(buildParams.wallet, buildParams.txFormat as TxFormat | undefined);
9851008
let changeAddressType = buildParams.changeAddressType as ScriptType2Of3[] | ScriptType2Of3 | undefined;
9861009

987-
if (this.shouldDefaultToPsbtTxFormat(buildParams)) {
988-
txFormat = 'psbt';
989-
}
990-
9911010
// if the addressType is not specified, we need to default to p2trMusig2 for testnet hot wallets for staged rollout of p2trMusig2
9921011
if (
9931012
buildParams.addressType === undefined && // addressType is deprecated and replaced by `changeAddress`

modules/abstract-utxo/test/unit/prebuildAndSign.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,8 @@ function run(coin: AbstractUtxoCoin, inputScripts: ScriptType[], txFormat: TxFor
214214

215215
[true, false].forEach((useWebauthn) => {
216216
it(`should succeed with ${useWebauthn ? 'webauthn encryptedPrv' : 'encryptedPrv'}`, async function () {
217-
const txCoins = ['tzec', 'zec', 'ltc', 'bcha', 'doge', 'dash', 'btg', 'bch'];
217+
// Check if this wallet/coin combination defaults to psbt
218+
const defaultTxFormat = coin.getDefaultTxFormat(wallet);
218219
const nocks = createNocks({
219220
bgUrl,
220221
wallet,
@@ -223,7 +224,7 @@ function run(coin: AbstractUtxoCoin, inputScripts: ScriptType[], txFormat: TxFor
223224
recipient,
224225
addressInfo,
225226
nockOutputAddresses: txFormat !== 'psbt',
226-
txFormat: !txCoins.includes(coin.getChain()) ? 'psbt' : undefined,
227+
txFormat: defaultTxFormat,
227228
});
228229

229230
// call prebuild and sign, nocks should be consumed
@@ -261,7 +262,8 @@ function run(coin: AbstractUtxoCoin, inputScripts: ScriptType[], txFormat: TxFor
261262
it(`should be able to build, sign, & verify a replacement transaction with selfSend: ${selfSend}`, async function () {
262263
const rbfTxIds = ['tx-to-be-replaced'],
263264
feeMultiplier = 1.5;
264-
const txCoins = ['tzec', 'zec', 'ltc', 'bcha', 'doge', 'dash', 'btg', 'bch'];
265+
// Check if this wallet/coin combination defaults to psbt
266+
const defaultTxFormat = coin.getDefaultTxFormat(wallet);
265267
const nocks = createNocks({
266268
bgUrl,
267269
wallet,
@@ -273,7 +275,7 @@ function run(coin: AbstractUtxoCoin, inputScripts: ScriptType[], txFormat: TxFor
273275
feeMultiplier,
274276
selfSend,
275277
nockOutputAddresses: txFormat !== 'psbt',
276-
txFormat: !txCoins.includes(coin.getChain()) ? 'psbt' : undefined,
278+
txFormat: defaultTxFormat,
277279
});
278280

279281
// call prebuild and sign, nocks should be consumed
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
import * as assert from 'assert';
2+
3+
import * as utxolib from '@bitgo/utxo-lib';
4+
import { Wallet } from '@bitgo/sdk-core';
5+
6+
import { AbstractUtxoCoin, TxFormat } from '../../src';
7+
8+
import { utxoCoins, defaultBitGo } from './util';
9+
10+
type WalletType = 'hot' | 'cold' | 'custodial' | 'custodialPaired' | 'trading';
11+
type WalletSubType = 'distributedCustody';
12+
type WalletFlag = { name: string; value: string };
13+
14+
type WalletOptions = {
15+
type?: WalletType;
16+
subType?: WalletSubType;
17+
walletFlags?: WalletFlag[];
18+
};
19+
20+
/**
21+
* Enumerates common wallet configurations for testing
22+
*/
23+
export function getWalletConfigurations(): Array<{ name: string; options: WalletOptions }> {
24+
return [
25+
{ name: 'hot wallet', options: { type: 'hot' } },
26+
{ name: 'cold wallet', options: { type: 'cold' } },
27+
{ name: 'custodial wallet', options: { type: 'custodial' } },
28+
{ name: 'distributedCustody wallet', options: { type: 'cold', subType: 'distributedCustody' } },
29+
{ name: 'musigKp wallet', options: { type: 'cold', walletFlags: [{ name: 'musigKp', value: 'true' }] } },
30+
{ name: 'hot musigKp wallet', options: { type: 'hot', walletFlags: [{ name: 'musigKp', value: 'true' }] } },
31+
];
32+
}
33+
34+
/**
35+
* Helper function to create a mock wallet for testing
36+
*/
37+
export function createMockWallet(coin: AbstractUtxoCoin, options: WalletOptions = {}): Wallet {
38+
const walletData = {
39+
id: '5b34252f1bf349930e34020a',
40+
coin: coin.getChain(),
41+
type: options.type || 'hot',
42+
...(options.subType && { subType: options.subType }),
43+
...(options.walletFlags && { walletFlags: options.walletFlags }),
44+
};
45+
return new Wallet(defaultBitGo, coin, walletData);
46+
}
47+
48+
/**
49+
* Helper function to get the txFormat from a coin's getDefaultTxFormat method
50+
*/
51+
export function getTxFormat(coin: AbstractUtxoCoin, wallet: Wallet, requestedFormat?: TxFormat): TxFormat | undefined {
52+
return coin.getDefaultTxFormat(wallet, requestedFormat);
53+
}
54+
55+
/**
56+
* Helper function to run a txFormat test with named arguments.
57+
* By default, iterates over all wallet configurations and all coins.
58+
*/
59+
function runTest(params: {
60+
description: string;
61+
expectedTxFormat:
62+
| TxFormat
63+
| undefined
64+
| ((coin: AbstractUtxoCoin, walletConfig: WalletOptions) => TxFormat | undefined);
65+
coinFilter?: (coin: AbstractUtxoCoin) => boolean;
66+
walletFilter?: (walletConfig: { name: string; options: WalletOptions }) => boolean;
67+
requestedTxFormat?: TxFormat;
68+
}): void {
69+
it(params.description, function () {
70+
const walletConfigs = getWalletConfigurations();
71+
72+
for (const walletConfig of walletConfigs) {
73+
// Skip wallet configurations that don't match the filter
74+
if (params.walletFilter && !params.walletFilter(walletConfig)) {
75+
continue;
76+
}
77+
78+
for (const coin of utxoCoins) {
79+
// Skip coins that don't match the filter
80+
if (params.coinFilter && !params.coinFilter(coin)) {
81+
continue;
82+
}
83+
84+
const wallet = createMockWallet(coin, walletConfig.options);
85+
const txFormat = getTxFormat(coin, wallet, params.requestedTxFormat);
86+
87+
const expectedTxFormat =
88+
typeof params.expectedTxFormat === 'function'
89+
? params.expectedTxFormat(coin, walletConfig.options)
90+
: params.expectedTxFormat;
91+
92+
assert.strictEqual(
93+
txFormat,
94+
expectedTxFormat,
95+
`${params.description} - ${
96+
walletConfig.name
97+
} - ${coin.getChain()}: expected ${expectedTxFormat}, got ${txFormat}`
98+
);
99+
}
100+
}
101+
});
102+
}
103+
104+
describe('txFormat', function () {
105+
describe('getDefaultTxFormat', function () {
106+
// All testnet wallets default to PSBT-lite
107+
runTest({
108+
description: 'should always return psbt-lite for testnet',
109+
coinFilter: (coin) => utxolib.isTestnet(coin.network),
110+
expectedTxFormat: 'psbt-lite',
111+
});
112+
113+
// DistributedCustody wallets default to PSBT (mainnet only, testnet already covered)
114+
runTest({
115+
description: 'should return psbt for distributedCustody wallets on mainnet',
116+
coinFilter: (coin) => utxolib.isMainnet(coin.network),
117+
walletFilter: (w) => w.options.subType === 'distributedCustody',
118+
expectedTxFormat: 'psbt',
119+
});
120+
121+
// MuSig2 wallets default to PSBT (mainnet only, testnet already covered)
122+
runTest({
123+
description: 'should return psbt for wallets with musigKp flag on mainnet',
124+
coinFilter: (coin) => utxolib.isMainnet(coin.network),
125+
walletFilter: (w) => Boolean(w.options.walletFlags?.some((f) => f.name === 'musigKp' && f.value === 'true')),
126+
expectedTxFormat: 'psbt',
127+
});
128+
129+
// Mainnet Bitcoin hot wallets default to PSBT
130+
runTest({
131+
description: 'should return psbt for mainnet bitcoin hot wallets',
132+
coinFilter: (coin) =>
133+
utxolib.isMainnet(coin.network) && utxolib.getMainnet(coin.network) === utxolib.networks.bitcoin,
134+
walletFilter: (w) => w.options.type === 'hot',
135+
expectedTxFormat: 'psbt',
136+
});
137+
138+
// Other mainnet wallets do NOT default to PSBT
139+
runTest({
140+
description: 'should return undefined for other mainnet wallets',
141+
coinFilter: (coin) => utxolib.isMainnet(coin.network),
142+
walletFilter: (w) => {
143+
const isHotBitcoin = w.options.type === 'hot'; // This will be bitcoin hot wallets
144+
const isDistributedCustody = w.options.subType === 'distributedCustody';
145+
const hasMusigKpFlag = Boolean(w.options.walletFlags?.some((f) => f.name === 'musigKp' && f.value === 'true'));
146+
// Only test "other" wallets - exclude the special cases
147+
return !isHotBitcoin && !isDistributedCustody && !hasMusigKpFlag;
148+
},
149+
expectedTxFormat: undefined,
150+
});
151+
152+
// Test explicitly requested formats
153+
runTest({
154+
description: 'should respect explicitly requested legacy format',
155+
expectedTxFormat: 'legacy',
156+
requestedTxFormat: 'legacy',
157+
});
158+
159+
runTest({
160+
description: 'should respect explicitly requested psbt format',
161+
expectedTxFormat: 'psbt',
162+
requestedTxFormat: 'psbt',
163+
});
164+
165+
runTest({
166+
description: 'should respect explicitly requested psbt-lite format',
167+
expectedTxFormat: 'psbt-lite',
168+
requestedTxFormat: 'psbt-lite',
169+
});
170+
});
171+
});

0 commit comments

Comments
 (0)