diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 200cee24ac..058079f62d 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -84,6 +84,19 @@ import { isDescriptorWalletData } from './descriptor/descriptorWallet'; import ScriptType2Of3 = utxolib.bitgo.outputScripts.ScriptType2Of3; +export type TxFormat = + // This is a legacy transaction format based around the bitcoinjs-lib serialization of unsigned transactions + // does not include prevOut data and is a bit painful to work with + // going to be deprecated in favor of psbt + // @deprecated + | 'legacy' + // This is the standard psbt format, including the full prevTx data for legacy transactions. + // This will remain supported but is not the default, since the data sizes can become prohibitively large. + | 'psbt' + // This is a nonstandard psbt version where legacy inputs are serialized as if they were segwit inputs. + // While this prevents us to fully verify the transaction fee, we have other checks in place to ensure the fee is within bounds. + | 'psbt-lite'; + type UtxoCustomSigningFunction = { (params: { coin: IBaseCoin; @@ -957,37 +970,43 @@ export abstract class AbstractUtxoCoin extends BaseCoin { }; } - private shouldDefaultToPsbtTxFormat(buildParams: ExtraPrebuildParamsOptions & { wallet: Wallet }) { - const walletFlagMusigKp = buildParams.wallet.flag('musigKp') === 'true'; - const isHotWallet = buildParams.wallet.type() === 'hot'; - - // if not txFormat is already specified figure out if we should default to psbt format - return ( - buildParams.txFormat === undefined && - (buildParams.wallet.subType() === 'distributedCustody' || - // default to testnet for all utxo coins except zcash - (isTestnet(this.network) && - // FIXME(BTC-1322): fix zcash PSBT support - getMainnet(this.network) !== utxolib.networks.zcash && - isHotWallet) || - // if mainnet, only default to psbt for btc hot wallets - (isMainnet(this.network) && getMainnet(this.network) === utxolib.networks.bitcoin && isHotWallet) || - // default to psbt if it has the wallet flag - walletFlagMusigKp) - ); + /** + * Determines the default transaction format based on wallet properties and network + * @param wallet - The wallet to check + * @param requestedFormat - Optional explicitly requested format + * @returns The transaction format to use, or undefined if no default applies + */ + getDefaultTxFormat(wallet: Wallet, requestedFormat?: TxFormat): TxFormat | undefined { + // If format is explicitly requested, use it + if (requestedFormat !== undefined) { + return requestedFormat; + } + + if (isTestnet(this.network)) { + return 'psbt-lite'; + } + + const walletFlagMusigKp = wallet.flag('musigKp') === 'true'; + const isHotWallet = wallet.type() === 'hot'; + + // Determine if we should default to psbt format + const shouldDefaultToPsbt = + wallet.subType() === 'distributedCustody' || + // if mainnet, only default to psbt for btc hot wallets + (isMainnet(this.network) && getMainnet(this.network) === utxolib.networks.bitcoin && isHotWallet) || + // default to psbt if it has the wallet flag + walletFlagMusigKp; + + return shouldDefaultToPsbt ? 'psbt' : undefined; } async getExtraPrebuildParams(buildParams: ExtraPrebuildParamsOptions & { wallet: Wallet }): Promise<{ - txFormat?: 'legacy' | 'psbt'; + txFormat?: TxFormat; changeAddressType?: ScriptType2Of3[] | ScriptType2Of3; }> { - let txFormat = buildParams.txFormat as 'legacy' | 'psbt' | undefined; + const txFormat = this.getDefaultTxFormat(buildParams.wallet, buildParams.txFormat as TxFormat | undefined); let changeAddressType = buildParams.changeAddressType as ScriptType2Of3[] | ScriptType2Of3 | undefined; - if (this.shouldDefaultToPsbtTxFormat(buildParams)) { - txFormat = 'psbt'; - } - // if the addressType is not specified, we need to default to p2trMusig2 for testnet hot wallets for staged rollout of p2trMusig2 if ( buildParams.addressType === undefined && // addressType is deprecated and replaced by `changeAddress` diff --git a/modules/abstract-utxo/test/unit/prebuildAndSign.ts b/modules/abstract-utxo/test/unit/prebuildAndSign.ts index ab021a6f15..6fe553c03f 100644 --- a/modules/abstract-utxo/test/unit/prebuildAndSign.ts +++ b/modules/abstract-utxo/test/unit/prebuildAndSign.ts @@ -214,7 +214,8 @@ function run(coin: AbstractUtxoCoin, inputScripts: ScriptType[], txFormat: TxFor [true, false].forEach((useWebauthn) => { it(`should succeed with ${useWebauthn ? 'webauthn encryptedPrv' : 'encryptedPrv'}`, async function () { - const txCoins = ['tzec', 'zec', 'ltc', 'bcha', 'doge', 'dash', 'btg', 'bch']; + // Check if this wallet/coin combination defaults to psbt + const defaultTxFormat = coin.getDefaultTxFormat(wallet); const nocks = createNocks({ bgUrl, wallet, @@ -223,7 +224,7 @@ function run(coin: AbstractUtxoCoin, inputScripts: ScriptType[], txFormat: TxFor recipient, addressInfo, nockOutputAddresses: txFormat !== 'psbt', - txFormat: !txCoins.includes(coin.getChain()) ? 'psbt' : undefined, + txFormat: defaultTxFormat, }); // call prebuild and sign, nocks should be consumed @@ -261,7 +262,8 @@ function run(coin: AbstractUtxoCoin, inputScripts: ScriptType[], txFormat: TxFor it(`should be able to build, sign, & verify a replacement transaction with selfSend: ${selfSend}`, async function () { const rbfTxIds = ['tx-to-be-replaced'], feeMultiplier = 1.5; - const txCoins = ['tzec', 'zec', 'ltc', 'bcha', 'doge', 'dash', 'btg', 'bch']; + // Check if this wallet/coin combination defaults to psbt + const defaultTxFormat = coin.getDefaultTxFormat(wallet); const nocks = createNocks({ bgUrl, wallet, @@ -273,7 +275,7 @@ function run(coin: AbstractUtxoCoin, inputScripts: ScriptType[], txFormat: TxFor feeMultiplier, selfSend, nockOutputAddresses: txFormat !== 'psbt', - txFormat: !txCoins.includes(coin.getChain()) ? 'psbt' : undefined, + txFormat: defaultTxFormat, }); // call prebuild and sign, nocks should be consumed diff --git a/modules/abstract-utxo/test/unit/txFormat.ts b/modules/abstract-utxo/test/unit/txFormat.ts new file mode 100644 index 0000000000..0911d0e93a --- /dev/null +++ b/modules/abstract-utxo/test/unit/txFormat.ts @@ -0,0 +1,171 @@ +import * as assert from 'assert'; + +import * as utxolib from '@bitgo/utxo-lib'; +import { Wallet } from '@bitgo/sdk-core'; + +import { AbstractUtxoCoin, TxFormat } from '../../src'; + +import { utxoCoins, defaultBitGo } from './util'; + +type WalletType = 'hot' | 'cold' | 'custodial' | 'custodialPaired' | 'trading'; +type WalletSubType = 'distributedCustody'; +type WalletFlag = { name: string; value: string }; + +type WalletOptions = { + type?: WalletType; + subType?: WalletSubType; + walletFlags?: WalletFlag[]; +}; + +/** + * Enumerates common wallet configurations for testing + */ +export function getWalletConfigurations(): Array<{ name: string; options: WalletOptions }> { + return [ + { name: 'hot wallet', options: { type: 'hot' } }, + { name: 'cold wallet', options: { type: 'cold' } }, + { name: 'custodial wallet', options: { type: 'custodial' } }, + { name: 'distributedCustody wallet', options: { type: 'cold', subType: 'distributedCustody' } }, + { name: 'musigKp wallet', options: { type: 'cold', walletFlags: [{ name: 'musigKp', value: 'true' }] } }, + { name: 'hot musigKp wallet', options: { type: 'hot', walletFlags: [{ name: 'musigKp', value: 'true' }] } }, + ]; +} + +/** + * Helper function to create a mock wallet for testing + */ +export function createMockWallet(coin: AbstractUtxoCoin, options: WalletOptions = {}): Wallet { + const walletData = { + id: '5b34252f1bf349930e34020a', + coin: coin.getChain(), + type: options.type || 'hot', + ...(options.subType && { subType: options.subType }), + ...(options.walletFlags && { walletFlags: options.walletFlags }), + }; + return new Wallet(defaultBitGo, coin, walletData); +} + +/** + * Helper function to get the txFormat from a coin's getDefaultTxFormat method + */ +export function getTxFormat(coin: AbstractUtxoCoin, wallet: Wallet, requestedFormat?: TxFormat): TxFormat | undefined { + return coin.getDefaultTxFormat(wallet, requestedFormat); +} + +/** + * Helper function to run a txFormat test with named arguments. + * By default, iterates over all wallet configurations and all coins. + */ +function runTest(params: { + description: string; + expectedTxFormat: + | TxFormat + | undefined + | ((coin: AbstractUtxoCoin, walletConfig: WalletOptions) => TxFormat | undefined); + coinFilter?: (coin: AbstractUtxoCoin) => boolean; + walletFilter?: (walletConfig: { name: string; options: WalletOptions }) => boolean; + requestedTxFormat?: TxFormat; +}): void { + it(params.description, function () { + const walletConfigs = getWalletConfigurations(); + + for (const walletConfig of walletConfigs) { + // Skip wallet configurations that don't match the filter + if (params.walletFilter && !params.walletFilter(walletConfig)) { + continue; + } + + for (const coin of utxoCoins) { + // Skip coins that don't match the filter + if (params.coinFilter && !params.coinFilter(coin)) { + continue; + } + + const wallet = createMockWallet(coin, walletConfig.options); + const txFormat = getTxFormat(coin, wallet, params.requestedTxFormat); + + const expectedTxFormat = + typeof params.expectedTxFormat === 'function' + ? params.expectedTxFormat(coin, walletConfig.options) + : params.expectedTxFormat; + + assert.strictEqual( + txFormat, + expectedTxFormat, + `${params.description} - ${ + walletConfig.name + } - ${coin.getChain()}: expected ${expectedTxFormat}, got ${txFormat}` + ); + } + } + }); +} + +describe('txFormat', function () { + describe('getDefaultTxFormat', function () { + // All testnet wallets default to PSBT-lite + runTest({ + description: 'should always return psbt-lite for testnet', + coinFilter: (coin) => utxolib.isTestnet(coin.network), + expectedTxFormat: 'psbt-lite', + }); + + // DistributedCustody wallets default to PSBT (mainnet only, testnet already covered) + runTest({ + description: 'should return psbt for distributedCustody wallets on mainnet', + coinFilter: (coin) => utxolib.isMainnet(coin.network), + walletFilter: (w) => w.options.subType === 'distributedCustody', + expectedTxFormat: 'psbt', + }); + + // MuSig2 wallets default to PSBT (mainnet only, testnet already covered) + runTest({ + description: 'should return psbt for wallets with musigKp flag on mainnet', + coinFilter: (coin) => utxolib.isMainnet(coin.network), + walletFilter: (w) => Boolean(w.options.walletFlags?.some((f) => f.name === 'musigKp' && f.value === 'true')), + expectedTxFormat: 'psbt', + }); + + // Mainnet Bitcoin hot wallets default to PSBT + runTest({ + description: 'should return psbt for mainnet bitcoin hot wallets', + coinFilter: (coin) => + utxolib.isMainnet(coin.network) && utxolib.getMainnet(coin.network) === utxolib.networks.bitcoin, + walletFilter: (w) => w.options.type === 'hot', + expectedTxFormat: 'psbt', + }); + + // Other mainnet wallets do NOT default to PSBT + runTest({ + description: 'should return undefined for other mainnet wallets', + coinFilter: (coin) => utxolib.isMainnet(coin.network), + walletFilter: (w) => { + const isHotBitcoin = w.options.type === 'hot'; // This will be bitcoin hot wallets + const isDistributedCustody = w.options.subType === 'distributedCustody'; + const hasMusigKpFlag = Boolean(w.options.walletFlags?.some((f) => f.name === 'musigKp' && f.value === 'true')); + // Only test "other" wallets - exclude the special cases + return !isHotBitcoin && !isDistributedCustody && !hasMusigKpFlag; + }, + expectedTxFormat: undefined, + }); + + // Test explicitly requested formats + runTest({ + description: 'should respect explicitly requested legacy format', + expectedTxFormat: 'legacy', + requestedTxFormat: 'legacy', + }); + + runTest({ + description: 'should respect explicitly requested psbt format', + expectedTxFormat: 'psbt', + requestedTxFormat: 'psbt', + }); + + runTest({ + description: 'should respect explicitly requested psbt-lite format', + expectedTxFormat: 'psbt-lite', + requestedTxFormat: 'psbt-lite', + }); + }); +});