Merge pull request #8161 from BitGo/BTC-2768.disable-legacy-txformat-for-wp-request

feat(abstract-utxo): disable legacy tx format for wallet-platform requests

Author: OttoAllmendinger

modules/abstract-utxo/src/abstractUtxoCoin.ts

@@ -79,7 +79,6 @@ import {
   getFullNameFromCoinName,
   getMainnetCoinName,
   getNetworkFromCoinName,
-  isTestnetCoin,
   isUtxoCoinNameMainnet,
   UtxoCoinName,
   UtxoCoinNameMainnet,
@@ -333,6 +332,11 @@ type UtxoBaseSignTransactionOptions<TNumber extends number | bigint = number> =
    * transaction (nonWitnessUtxo)
    */
   allowNonSegwitSigningWithoutPrevTx?: boolean;
+  /**
+   * When true, the signed transaction will be converted from PSBT to legacy format before returning.
+   * Set automatically by presignTransaction() when the caller explicitly requested txFormat: 'legacy'.
+   */
+  returnLegacyFormat?: boolean;
   wallet?: UtxoWallet;
 };
 
@@ -991,24 +995,20 @@ export abstract class AbstractUtxoCoin
    * @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) {
-      if (isTestnetCoin(this.name) && requestedFormat === 'legacy') {
-        throw new ErrorDeprecatedTxFormat(requestedFormat);
-      }
-
-      return requestedFormat;
+  getDefaultTxFormat(wallet: Wallet, requestedFormat?: TxFormat): TxFormat {
+    if (requestedFormat === 'legacy') {
+      return 'psbt-lite';
     }
-
-    return 'psbt-lite';
+    return requestedFormat ?? 'psbt-lite';
   }
 
   async getExtraPrebuildParams(buildParams: ExtraPrebuildParamsOptions & { wallet: Wallet }): Promise<{
     txFormat?: TxFormat;
     changeAddressType?: ScriptType2Of3[] | ScriptType2Of3;
+    allowedInputScriptTypes?: ScriptType2Of3[];
   }> {
-    const txFormat = this.getDefaultTxFormat(buildParams.wallet, buildParams.txFormat as TxFormat | undefined);
+    const requestedFormat = buildParams.txFormat as TxFormat | undefined;
+    const txFormat = this.getDefaultTxFormat(buildParams.wallet, requestedFormat);
     let changeAddressType = buildParams.changeAddressType as ScriptType2Of3[] | ScriptType2Of3 | undefined;
 
     // if the addressType is not specified, we need to default to p2trMusig2 for testnet hot wallets for staged rollout of p2trMusig2
@@ -1021,9 +1021,26 @@ export abstract class AbstractUtxoCoin
       changeAddressType = ['p2trMusig2', 'p2wsh', 'p2shP2wsh', 'p2sh', 'p2tr'];
     }
 
+    // getHalfSignedLegacyFormat() only supports p2ms-based types (p2sh, p2shP2wsh, p2wsh).
+    // Filter change outputs and restrict input selection to these types.
+    const legacyCompatibleTypes: ScriptType2Of3[] = ['p2sh', 'p2shP2wsh', 'p2wsh'];
+    let allowedInputScriptTypes: ScriptType2Of3[] | undefined;
+
+    if (requestedFormat === 'legacy') {
+      allowedInputScriptTypes = legacyCompatibleTypes;
+      if (Array.isArray(changeAddressType)) {
+        changeAddressType = changeAddressType.filter((t): t is ScriptType2Of3 =>
+          legacyCompatibleTypes.includes(t as ScriptType2Of3)
+        );
+      } else if (changeAddressType !== undefined && !legacyCompatibleTypes.includes(changeAddressType)) {
+        changeAddressType = legacyCompatibleTypes;
+      }
+    }
+
     return {
       txFormat,
       changeAddressType,
+      allowedInputScriptTypes,
     };
   }
 
@@ -1035,6 +1052,9 @@ export abstract class AbstractUtxoCoin
     if (params.walletData && isUtxoWalletData(params.walletData) && isDescriptorWalletData(params.walletData)) {
       return params;
     }
+
+    const returnLegacyFormat = (params as Record<string, unknown>).txFormat === 'legacy';
+
     // In the case that we have a 'psbt-lite' transaction format, we want to indicate in signing to not fail
     const txHex = (params.txHex ?? params.txPrebuild?.txHex) as string;
     if (
@@ -1043,9 +1063,9 @@ export abstract class AbstractUtxoCoin
       utxolib.bitgo.isPsbtLite(utxolib.bitgo.createPsbtFromHex(txHex, this.network)) &&
       params.allowNonSegwitSigningWithoutPrevTx === undefined
     ) {
-      return { ...params, allowNonSegwitSigningWithoutPrevTx: true };
+      return { ...params, allowNonSegwitSigningWithoutPrevTx: true, returnLegacyFormat };
     }
-    return params;
+    return { ...params, returnLegacyFormat };
   }
 
   async supplementGenerateWallet(

modules/abstract-utxo/src/transaction/signTransaction.ts

@@ -1,6 +1,6 @@
 import _ from 'lodash';
 import { BitGoBase } from '@bitgo/sdk-core';
-import { BIP32 } from '@bitgo/wasm-utxo';
+import { BIP32, fixedScriptWallet } from '@bitgo/wasm-utxo';
 import buildDebug from 'debug';
 
 import { AbstractUtxoCoin, SignTransactionOptions } from '../abstractUtxoCoin';
@@ -10,7 +10,7 @@ import { isUtxoLibPsbt, toWasmPsbt } from '../wasmUtil';
 
 import * as fixedScript from './fixedScript';
 import * as descriptor from './descriptor';
-import { encodeTransaction } from './decode';
+import { decodePsbtWith, encodeTransaction } from './decode';
 
 const debug = buildDebug('bitgo:abstract-utxo:transaction:signTransaction');
 
@@ -43,7 +43,13 @@ export async function signTransaction<TNumber extends number | bigint>(
     throw new Error('missing txPrebuild parameter');
   }
 
-  const tx = coin.decodeTransactionFromPrebuild(params.txPrebuild);
+  let tx = coin.decodeTransactionFromPrebuild(params.txPrebuild);
+
+  // When returnLegacyFormat is set, ensure we use wasm-utxo's BitGoPsbt so
+  // getHalfSignedLegacyFormat() is available after signing.
+  if (params.returnLegacyFormat && isUtxoLibPsbt(tx)) {
+    tx = decodePsbtWith(tx.toBuffer(), coin.name, 'wasm-utxo');
+  }
 
   const signerKeychain = getSignerKeychain(params.prv);
 
@@ -73,6 +79,12 @@ export async function signTransaction<TNumber extends number | bigint>(
       pubs: params.pubs,
       cosignerPub: params.cosignerPub,
     });
+
+    // Convert half-signed PSBT to legacy format when the caller explicitly requested txFormat: 'legacy'
+    if (params.returnLegacyFormat && signedTx instanceof fixedScriptWallet.BitGoPsbt) {
+      return { txHex: Buffer.from(signedTx.getHalfSignedLegacyFormat()).toString('hex') };
+    }
+
     const buffer = Buffer.isBuffer(signedTx) ? signedTx : encodeTransaction(signedTx);
     return { txHex: buffer.toString('hex') };
   }

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

@@ -0,0 +1,91 @@
+import * as assert from 'assert';
+
+import * as utxolib from '@bitgo/utxo-lib';
+import { hasPsbtMagic } from '@bitgo/wasm-utxo';
+import nock = require('nock');
+import { common, HalfSignedUtxoTransaction } from '@bitgo/sdk-core';
+import { getSeed } from '@bitgo/sdk-test';
+
+import {
+  defaultBitGo,
+  encryptKeychain,
+  getDefaultWalletKeys,
+  getMinUtxoCoins,
+  getUtxoWallet,
+  keychainsBase58,
+  getScriptTypes,
+} from './util';
+
+const walletPassphrase = 'gabagool';
+
+const rootWalletKeys = getDefaultWalletKeys();
+const keyDocumentObjects = rootWalletKeys.triple.map((bip32, keyIdx) => ({
+  id: getSeed(keychainsBase58[keyIdx].pub).toString('hex'),
+  pub: bip32.neutered().toBase58(),
+  source: ['user', 'backup', 'bitgo'][keyIdx],
+  encryptedPrv: encryptKeychain(walletPassphrase, keychainsBase58[keyIdx]),
+  coinSpecific: {},
+}));
+
+// Test that txFormat: 'legacy' converts the signed PSBT back to legacy (non-PSBT) format.
+// Uses BTC with legacy-compatible script types (no taproot).
+describe('prebuildAndSign-returnLegacyFormat', function () {
+  const coin = getMinUtxoCoins().find((c) => c.getChain() === 'btc')!;
+  const inputScripts = getScriptTypes(coin, 'legacy');
+  const wallet = getUtxoWallet(coin, {
+    coinSpecific: { addressVersion: 'base58' },
+    keys: keyDocumentObjects.map((k) => k.id),
+    id: 'walletId',
+  });
+  const bgUrl = common.Environments[defaultBitGo.getEnv()].uri;
+  let prebuild: utxolib.bitgo.UtxoPsbt;
+  let recipient: { address: string; amount: string };
+  const fee = BigInt(10000);
+
+  before(function () {
+    const outputAmount = BigInt(inputScripts.length) * BigInt(1e8) - fee;
+    const outputScriptType: utxolib.bitgo.outputScripts.ScriptType = 'p2sh';
+    const outputChain = utxolib.bitgo.getExternalChainCode(outputScriptType);
+    const outputAddress = utxolib.bitgo.getWalletAddress(rootWalletKeys, outputChain, 0, coin.network);
+    recipient = { address: outputAddress, amount: outputAmount.toString() };
+    prebuild = utxolib.testutil.constructPsbt(
+      inputScripts.map((s) => ({ scriptType: s, value: BigInt(1e8) })),
+      [{ scriptType: outputScriptType, value: outputAmount }],
+      coin.network,
+      rootWalletKeys,
+      'unsigned'
+    );
+    utxolib.bitgo.addXpubsToPsbt(prebuild, rootWalletKeys);
+  });
+
+  afterEach(nock.cleanAll);
+
+  it('should build with PSBT internally but return legacy format to the caller', async function () {
+    // WP receives a PSBT build request (getExtraPrebuildParams maps 'legacy' -> 'psbt-lite')
+    const nocks: nock.Scope[] = [];
+    nocks.push(
+      nock(bgUrl)
+        .post(`/api/v2/${coin.getChain()}/wallet/${wallet.id()}/tx/build`)
+        .reply(200, { txHex: prebuild.toHex(), txInfo: {} })
+    );
+    nocks.push(nock(bgUrl).get(`/api/v2/${coin.getChain()}/public/block/latest`).reply(200, { height: 1000 }));
+    keyDocumentObjects.forEach((keyDocument) => {
+      nocks.push(nock(bgUrl).get(`/api/v2/${coin.getChain()}/key/${keyDocument.id}`).times(3).reply(200, keyDocument));
+    });
+
+    // The prebuild from WP is a PSBT
+    assert.strictEqual(hasPsbtMagic(Buffer.from(prebuild.toHex(), 'hex')), true);
+
+    // The caller requests txFormat: 'legacy'
+    const res = (await wallet.prebuildAndSignTransaction({
+      recipients: [recipient],
+      walletPassphrase,
+      txFormat: 'legacy',
+    })) as HalfSignedUtxoTransaction;
+
+    nocks.forEach((n) => assert.ok(n.isDone()));
+
+    // The signed result is converted back to legacy (non-PSBT) format
+    assert.strictEqual(hasPsbtMagic(Buffer.from(res.txHex, 'hex')), false);
+  });
+});

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

@@ -2,7 +2,7 @@ import * as assert from 'assert';
 
 import { Wallet } from '@bitgo/sdk-core';
 
-import { AbstractUtxoCoin, ErrorDeprecatedTxFormat, TxFormat } from '../../src';
+import { AbstractUtxoCoin, TxFormat } from '../../src';
 import { isMainnetCoin, isTestnetCoin } from '../../src/names';
 
 import { utxoCoins, defaultBitGo } from './util';
@@ -119,9 +119,8 @@ describe('txFormat', function () {
 
     // Test explicitly requested formats
     runTest({
-      description: 'should respect explicitly requested legacy format on mainnet',
-      coinFilter: (coin) => isMainnetCoin(coin.name),
-      expectedTxFormat: 'legacy',
+      description: 'should map explicitly requested legacy format to psbt-lite',
+      expectedTxFormat: 'psbt-lite',
       requestedTxFormat: 'legacy',
     });
 
@@ -136,19 +135,45 @@ describe('txFormat', function () {
       expectedTxFormat: 'psbt-lite',
       requestedTxFormat: 'psbt-lite',
     });
+  });
+
+  describe('getExtraPrebuildParams with legacy format', function () {
+    const legacyCompatibleTypes = ['p2sh', 'p2shP2wsh', 'p2wsh'];
 
-    // Test that legacy format is prohibited on testnet
-    it('should throw ErrorDeprecatedTxFormat when legacy format is requested on testnet', function () {
+    it('should filter changeAddressType to legacy-compatible types for hot wallets', async function () {
       for (const coin of utxoCoins) {
-        if (!isTestnetCoin(coin.name)) {
-          continue;
+        const wallet = createMockWallet(coin, { type: 'hot' });
+        const result = await coin.getExtraPrebuildParams({ txFormat: 'legacy', wallet } as any);
+        assert.ok(Array.isArray(result.changeAddressType), `${coin.getChain()}: changeAddressType should be an array`);
+        for (const t of result.changeAddressType as string[]) {
+          assert.ok(
+            legacyCompatibleTypes.includes(t),
+            `${coin.getChain()}: changeAddressType contains ${t} which is not legacy-compatible`
+          );
         }
+      }
+    });
 
+    it('should set allowedInputScriptTypes to legacy-compatible types', async function () {
+      for (const coin of utxoCoins) {
+        const wallet = createMockWallet(coin, { type: 'hot' });
+        const result = await coin.getExtraPrebuildParams({ txFormat: 'legacy', wallet } as any);
+        assert.deepStrictEqual(
+          result.allowedInputScriptTypes,
+          legacyCompatibleTypes,
+          `${coin.getChain()}: allowedInputScriptTypes should be legacy-compatible`
+        );
+      }
+    });
+
+    it('should not set allowedInputScriptTypes when txFormat is not legacy', async function () {
+      for (const coin of utxoCoins) {
         const wallet = createMockWallet(coin, { type: 'hot' });
-        assert.throws(
-          () => getTxFormat(coin, wallet, 'legacy'),
-          ErrorDeprecatedTxFormat,
-          `Expected ErrorDeprecatedTxFormat for ${coin.getChain()}`
+        const result = await coin.getExtraPrebuildParams({ wallet } as any);
+        assert.strictEqual(
+          result.allowedInputScriptTypes,
+          undefined,
+          `${coin.getChain()}: allowedInputScriptTypes should be undefined for default format`
         );
       }
     });

modules/sdk-core/src/bitgo/wallet/BuildParams.ts

@@ -31,6 +31,8 @@ export const BuildParamsUTXO = t.partial({
   enforceMinConfirmsForChange: t.unknown,
   /* legacy or psbt */
   txFormat: t.unknown,
+  /* restrict which input script types WP may select (e.g. for legacy format compatibility) */
+  allowedInputScriptTypes: t.unknown,
   maxChangeOutputs: t.unknown,
   /* rbf */
   rbfTxIds: t.array(t.string),