Skip to content

Commit c447baf

Browse files
Merge pull request #8824 from BitGo/otto/t1-3400-fix-descriptor-sign
refactor(abstract-utxo): decode descriptor PSBTs directly to wasm Psbt
2 parents 235af5f + 6766023 commit c447baf

8 files changed

Lines changed: 196 additions & 43 deletions

File tree

modules/abstract-utxo/src/abstractUtxoCoin.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,12 @@ import {
8686
} from './names';
8787
import { assertFixedScriptWalletAddress } from './address/fixedScript';
8888
import { isSdkBackend, ParsedTransaction, SdkBackend } from './transaction/types';
89-
import { decodePsbtWith, encodeTransaction, stringToBufferTryFormats } from './transaction/decode';
89+
import {
90+
decodeDescriptorPsbt,
91+
decodePsbtWith,
92+
encodeTransaction,
93+
stringToBufferTryFormats,
94+
} from './transaction/decode';
9095
import { fetchKeychains, toBip32Triple, UtxoKeychain } from './keychains';
9196
import { verifyKeySignature, verifyUserPublicKey } from './verifyKey';
9297
import { getPolicyForEnv } from './descriptor/validatePolicy';
@@ -963,7 +968,12 @@ export abstract class AbstractUtxoCoin
963968
params.pubs = toBip32Triple(keychains).map((k) => k.neutered().toBase58()) as Triple<string>;
964969
}
965970
}
966-
return explainTx(this.decodeTransactionFromPrebuild(params), params, this.name);
971+
if (wallet && isDescriptorWallet(wallet)) {
972+
// Descriptor wallets decode prebuild bytes straight into the wasm-utxo
973+
// descriptor Psbt, skipping the fixedScriptWallet.BitGoPsbt intermediate.
974+
return explainTx(decodeDescriptorPsbt(params), { ...params, wallet }, this.name);
975+
}
976+
return explainTx(this.decodeTransactionFromPrebuild(params), { ...params, wallet }, this.name);
967977
}
968978

969979
/**

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as utxolib from '@bitgo/utxo-lib';
2-
import { fixedScriptWallet, utxolibCompat } from '@bitgo/wasm-utxo';
2+
import { fixedScriptWallet, hasPsbtMagic, Psbt as WasmPsbt, utxolibCompat } from '@bitgo/wasm-utxo';
33

44
import { getNetworkFromCoinName, UtxoCoinName } from '../names';
55

@@ -66,6 +66,30 @@ export function decodePsbt(psbt: string | Buffer, coinName: UtxoCoinName): BitGo
6666
return decodePsbtWith(psbt, coinName, 'wasm-utxo');
6767
}
6868

69+
export type PrebuildLike = {
70+
txHex?: string;
71+
txBase64?: string;
72+
txHexPsbt?: string;
73+
};
74+
75+
/**
76+
* Decode a prebuild's PSBT bytes directly into a wasm-utxo descriptor `Psbt`.
77+
*
78+
* Skips the `fixedScriptWallet.BitGoPsbt` intermediate that `decodeTransactionFromPrebuild`
79+
* + `toWasmPsbt` would otherwise round-trip through for descriptor flows.
80+
*/
81+
export function decodeDescriptorPsbt(prebuild: PrebuildLike): WasmPsbt {
82+
const s = prebuild.txHexPsbt ?? prebuild.txHex ?? prebuild.txBase64;
83+
if (!s) {
84+
throw new Error('missing required txHex or txBase64 property');
85+
}
86+
const bytes = stringToBufferTryFormats(s, ['hex', 'base64']);
87+
if (!hasPsbtMagic(bytes)) {
88+
throw new Error('descriptor wallets require PSBT format transactions');
89+
}
90+
return WasmPsbt.deserialize(bytes);
91+
}
92+
6993
export function encodeTransaction(
7094
transaction: utxolib.bitgo.UtxoTransaction<bigint | number> | utxolib.bitgo.UtxoPsbt | fixedScriptWallet.BitGoPsbt
7195
): Buffer {

modules/abstract-utxo/src/transaction/descriptor/parse.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { fromExtendedAddressFormatToScript, toExtendedAddressFormat } from '../r
1010
import { outputDifferencesWithExpected, OutputDifferenceWithExpected } from '../outputDifference';
1111
import { UtxoCoinName } from '../../names';
1212
import { sumValues, toWasmPsbt, UtxoLibPsbt } from '../../wasmUtil';
13+
import { decodeDescriptorPsbt } from '../decode';
1314

1415
type ParsedOutput = Omit<descriptorWallet.ParsedOutput, 'script'> & { script: Buffer };
1516

@@ -128,13 +129,7 @@ export function parse(
128129
if (!recipients) {
129130
throw new Error('recipients is required');
130131
}
131-
const psbt = coin.decodeTransactionFromPrebuild(params.txPrebuild);
132-
let wasmPsbt: Psbt;
133-
try {
134-
wasmPsbt = toWasmPsbt(psbt as Psbt | UtxoLibPsbt | Uint8Array);
135-
} catch (e) {
136-
throw new Error(`expected psbt to be a wasm-utxo or utxo-lib PSBT: ${e instanceof Error ? e.message : e}`);
137-
}
132+
const wasmPsbt = decodeDescriptorPsbt(params.txPrebuild);
138133
const walletKeys = toBip32Triple(keychains);
139134
const descriptorMap = getDescriptorMapFromWallet(wallet, walletKeys, getPolicyForEnv(params.wallet.bitgo.env));
140135
return {

modules/abstract-utxo/src/transaction/descriptor/verifyTransaction.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import type { Psbt, descriptorWallet } from '@bitgo/wasm-utxo';
44
import { AbstractUtxoCoin, VerifyTransactionOptions } from '../../abstractUtxoCoin';
55
import { BaseOutput, BaseParsedTransactionOutputs } from '../types';
66
import { UtxoCoinName } from '../../names';
7-
import { toWasmPsbt, UtxoLibPsbt } from '../../wasmUtil';
7+
import { UtxoLibPsbt } from '../../wasmUtil';
8+
import { decodeDescriptorPsbt } from '../decode';
89

910
import { toBaseParsedTransactionOutputsFromPsbt } from './parse';
1011

@@ -76,10 +77,9 @@ export async function verifyTransaction<TNumber extends number | bigint>(
7677
params: VerifyTransactionOptions<TNumber>,
7778
descriptorMap: descriptorWallet.DescriptorMap
7879
): Promise<boolean> {
79-
const tx = coin.decodeTransactionFromPrebuild(params.txPrebuild);
8080
let psbt: Psbt;
8181
try {
82-
psbt = toWasmPsbt(tx as Psbt | UtxoLibPsbt | Uint8Array);
82+
psbt = decodeDescriptorPsbt(params.txPrebuild);
8383
} catch (e) {
8484
const txExplanation = await TxIntentMismatchError.tryGetTxExplanation(
8585
coin as unknown as IBaseCoin,

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

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
import * as utxolib from '@bitgo/utxo-lib';
2-
import { fixedScriptWallet } from '@bitgo/wasm-utxo';
2+
import { fixedScriptWallet, Psbt as WasmPsbt } from '@bitgo/wasm-utxo';
33
import { isTriple, IWallet, Triple } from '@bitgo/sdk-core';
44

55
import { getDescriptorMapFromWallet, isDescriptorWallet } from '../descriptor';
66
import { toBip32Triple } from '../keychains';
77
import { getPolicyForEnv } from '../descriptor/validatePolicy';
88
import { UtxoCoinName } from '../names';
99
import type { Unspent } from '../unspent';
10-
import { toWasmPsbt } from '../wasmUtil';
1110

1211
import { getReplayProtectionPubkeys } from './fixedScript/replayProtection';
1312
import type {
@@ -23,7 +22,7 @@ import * as descriptor from './descriptor';
2322
* change amounts, and transaction outputs.
2423
*/
2524
export function explainTx<TNumber extends number | bigint>(
26-
tx: utxolib.bitgo.UtxoTransaction<TNumber> | utxolib.bitgo.UtxoPsbt | fixedScriptWallet.BitGoPsbt,
25+
tx: utxolib.bitgo.UtxoTransaction<TNumber> | utxolib.bitgo.UtxoPsbt | fixedScriptWallet.BitGoPsbt | WasmPsbt,
2726
params: {
2827
wallet?: IWallet;
2928
pubs?: string[];
@@ -34,20 +33,15 @@ export function explainTx<TNumber extends number | bigint>(
3433
coinName: UtxoCoinName
3534
): TransactionExplanationUtxolibLegacy | TransactionExplanationUtxolibPsbt | TransactionExplanationWasm {
3635
if (params.wallet && isDescriptorWallet(params.wallet)) {
37-
if (tx instanceof utxolib.bitgo.UtxoPsbt) {
38-
if (!params.pubs || !isTriple(params.pubs)) {
39-
throw new Error('pub triple is required for descriptor wallets');
40-
}
41-
const walletKeys = toBip32Triple(params.pubs);
42-
const descriptors = getDescriptorMapFromWallet(
43-
params.wallet,
44-
walletKeys,
45-
getPolicyForEnv(params.wallet.bitgo.env)
46-
);
47-
return descriptor.explainPsbt(toWasmPsbt(tx), descriptors, coinName);
36+
if (!(tx instanceof WasmPsbt)) {
37+
throw new Error('descriptor wallets require PSBT format transactions');
4838
}
49-
50-
throw new Error('legacy transactions are not supported for descriptor wallets');
39+
if (!params.pubs || !isTriple(params.pubs)) {
40+
throw new Error('pub triple is required for descriptor wallets');
41+
}
42+
const walletKeys = toBip32Triple(params.pubs);
43+
const descriptors = getDescriptorMapFromWallet(params.wallet, walletKeys, getPolicyForEnv(params.wallet.bitgo.env));
44+
return descriptor.explainPsbt(tx, descriptors, coinName);
5145
}
5246
if (tx instanceof utxolib.bitgo.UtxoPsbt) {
5347
return fixedScript.explainPsbt(tx, { ...params, customChangePubs: params.customChangeXpubs }, coinName);
@@ -71,6 +65,8 @@ export function explainTx<TNumber extends number | bigint>(
7165
},
7266
customChangeWalletXpubs: params.customChangeXpubs,
7367
});
68+
} else if (tx instanceof WasmPsbt) {
69+
throw new Error('descriptor Psbt is only supported for descriptor wallets');
7470
} else {
7571
return fixedScript.explainLegacyTx(tx, params, coinName);
7672
}

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

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import buildDebug from 'debug';
66
import { AbstractUtxoCoin, SignTransactionOptions } from '../abstractUtxoCoin';
77
import { getDescriptorMapFromWallet, getPolicyForEnv, isDescriptorWallet } from '../descriptor';
88
import { fetchKeychains, toBip32Triple } from '../keychains';
9-
import { isUtxoLibPsbt, toWasmPsbt } from '../wasmUtil';
9+
import { isUtxoLibPsbt } from '../wasmUtil';
1010

1111
import * as fixedScript from './fixedScript';
1212
import * as descriptor from './descriptor';
13-
import { decodePsbtWith, encodeTransaction } from './decode';
13+
import { decodeDescriptorPsbt, decodePsbtWith, encodeTransaction } from './decode';
1414

1515
const debug = buildDebug('bitgo:abstract-utxo:transaction:signTransaction');
1616

@@ -43,14 +43,6 @@ export async function signTransaction<TNumber extends number | bigint>(
4343
throw new Error('missing txPrebuild parameter');
4444
}
4545

46-
let tx = coin.decodeTransactionFromPrebuild(params.txPrebuild);
47-
48-
// When returnLegacyFormat is set, ensure we use wasm-utxo's BitGoPsbt so
49-
// getHalfSignedLegacyFormat() is available after signing.
50-
if (params.returnLegacyFormat && isUtxoLibPsbt(tx)) {
51-
tx = decodePsbtWith(tx.toBuffer(), coin.name, 'wasm-utxo');
52-
}
53-
5446
const signerKeychain = getSignerKeychain(params.prv);
5547

5648
const { wallet } = params;
@@ -59,17 +51,22 @@ export async function signTransaction<TNumber extends number | bigint>(
5951
if (!signerKeychain) {
6052
throw new Error('missing signer');
6153
}
62-
if (!isUtxoLibPsbt(tx) && !(tx instanceof Uint8Array)) {
63-
throw new Error('descriptor wallets require PSBT format transactions');
64-
}
54+
const psbt = decodeDescriptorPsbt(params.txPrebuild);
6555
const walletKeys = toBip32Triple(await fetchKeychains(coin, wallet));
6656
const descriptorMap = getDescriptorMapFromWallet(wallet, walletKeys, getPolicyForEnv(bitgo.env));
67-
const psbt = toWasmPsbt(tx);
6857
descriptor.signPsbt(psbt, descriptorMap, signerKeychain, {
6958
onUnknownInput: 'throw',
7059
});
7160
return { txHex: Buffer.from(psbt.serialize()).toString('hex') };
7261
} else {
62+
let tx = coin.decodeTransactionFromPrebuild(params.txPrebuild);
63+
64+
// When returnLegacyFormat is set, ensure we use wasm-utxo's BitGoPsbt so
65+
// getHalfSignedLegacyFormat() is available after signing.
66+
if (params.returnLegacyFormat && isUtxoLibPsbt(tx)) {
67+
tx = decodePsbtWith(tx.toBuffer(), coin.name, 'wasm-utxo');
68+
}
69+
7370
const signedTx = await fixedScript.signTransaction(coin, tx, getSignerKeychain(params.prv), coin.name, {
7471
walletId: params.txPrebuild.walletId,
7572
txInfo: params.txPrebuild.txInfo,
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import 'mocha';
2+
import assert from 'assert';
3+
4+
import { Psbt } from '@bitgo/wasm-utxo';
5+
import * as testutils from '@bitgo/wasm-utxo/testutils';
6+
7+
import type { UtxoWallet } from '../../../../src/wallet';
8+
import { getUtxoCoin } from '../../util/utxoCoins';
9+
import { nockBitGo } from '../../util/nockBitGo';
10+
11+
const { getDescriptorMap, mockPsbtDefaultWithDescriptorTemplate } = testutils.descriptor;
12+
const { getKeyTriple } = testutils;
13+
14+
// End-to-end coverage for descriptor wallet signing through the
15+
// top-level coin.signTransaction entry point (T1-3401). Locks in the
16+
// wasm-utxo decode path that T1-3400 broke.
17+
describe('signTransaction E2E: descriptor wallet (wasm-utxo backend)', function () {
18+
it('produces a signed PSBT with valid user signatures on every input', async function () {
19+
const coin = getUtxoCoin('btc');
20+
// mockPsbtDefaultWithDescriptorTemplate uses getDefaultXPubs('a') —
21+
// i.e. getKeyTriple('a') — so we sign with the same triple.
22+
const keychain = getKeyTriple('a');
23+
const userKey = keychain[0];
24+
const descriptorMap = getDescriptorMap('Wsh2Of3', keychain);
25+
26+
const unsignedPsbt = mockPsbtDefaultWithDescriptorTemplate('Wsh2Of3');
27+
const psbtHex = Buffer.from(unsignedPsbt.serialize()).toString('hex');
28+
29+
const keyIds = ['kU', 'kB', 'kG'];
30+
const wallet = {
31+
coinSpecific: () => ({
32+
descriptors: [...descriptorMap.entries()].map(([name, descriptor]) => ({
33+
name,
34+
value: descriptor.toString(),
35+
})),
36+
}),
37+
keyIds: () => keyIds,
38+
} as unknown as UtxoWallet;
39+
40+
// Mock the keychain fetch — fetchKeychains pulls each key by id.
41+
keyIds.forEach((id, i) => {
42+
nockBitGo().get(`/api/v2/${coin.getChain()}/key/${id}`).reply(200, { pub: keychain[i].neutered().toBase58() });
43+
});
44+
45+
// decodeWith: 'wasm-utxo' is explicit to lock in the BitGoPsbt
46+
// decode path that T1-3400 broke; this is also the production
47+
// default after 1702a08009.
48+
const result = await coin.signTransaction({
49+
txPrebuild: { txHex: psbtHex, decodeWith: 'wasm-utxo' },
50+
prv: userKey.toBase58(),
51+
wallet,
52+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
53+
} as any);
54+
55+
assert.ok('txHex' in result, 'expected signTransaction to return { txHex }');
56+
const signedPsbt = Psbt.deserialize(Buffer.from(result.txHex, 'hex'));
57+
58+
const inputs = signedPsbt.getInputs();
59+
assert.ok(inputs.length > 0, 'expected at least one input');
60+
inputs.forEach((_input, vin) => {
61+
assert.ok(signedPsbt.hasPartialSignatures(vin), `input ${vin} has no partial signatures`);
62+
const sigs = signedPsbt.getPartialSignatures(vin);
63+
assert.ok(sigs.length > 0, `input ${vin} returned empty partial signatures`);
64+
// Pubkeys in partial sigs are the descriptor-derived child keys, not
65+
// the user master pubkey; assert that each sig validates at its claimed
66+
// pubkey, which is the strongest "signing actually worked" check.
67+
for (const sig of sigs) {
68+
assert.ok(
69+
signedPsbt.validateSignatureAtInput(vin, sig.pubkey),
70+
`input ${vin} has an invalid signature for pubkey ${Buffer.from(sig.pubkey).toString('hex')}`
71+
);
72+
}
73+
});
74+
});
75+
});
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import 'mocha';
2+
import assert from 'assert';
3+
4+
import * as testutils from '@bitgo/wasm-utxo/testutils';
5+
6+
import { signTransaction } from '../../../../src/transaction/signTransaction';
7+
import type { UtxoWallet } from '../../../../src/wallet';
8+
import { defaultBitGo, getUtxoCoin } from '../../util/utxoCoins';
9+
import { getDefaultWalletKeys } from '../../util/keychains';
10+
11+
const { getDescriptorMap, mockPsbtDefaultWithDescriptorTemplate } = testutils.descriptor;
12+
13+
// Regression test for T1-3400. The descriptor sign guard previously
14+
// only accepted utxo-lib PSBTs (UtxoLibPsbt) and Uint8Array, throwing
15+
// `descriptor wallets require PSBT format transactions` when the
16+
// decoded prebuild was a wasm-utxo BitGoPsbt — the default after
17+
// defaultSdkBackend flipped to 'wasm-utxo' for all coins.
18+
describe('signTransaction: descriptor wallet with BitGoPsbt prebuild (T1-3400)', function () {
19+
it('does not reject BitGoPsbt with the descriptor PSBT guard', async function () {
20+
const coin = getUtxoCoin('btc');
21+
const descriptorMap = getDescriptorMap('Wsh2Of3');
22+
const psbt = mockPsbtDefaultWithDescriptorTemplate('Wsh2Of3');
23+
const psbtHex = Buffer.from(psbt.serialize()).toString('hex');
24+
25+
const wallet = {
26+
coinSpecific: () => ({
27+
descriptors: [...descriptorMap.entries()].map(([name, descriptor]) => ({
28+
name,
29+
value: descriptor.toString(),
30+
})),
31+
}),
32+
keyIds: () => ['k0', 'k1', 'k2'],
33+
} as unknown as UtxoWallet;
34+
35+
const userPrv = getDefaultWalletKeys().user.toBase58();
36+
37+
let caught: Error | undefined;
38+
try {
39+
// decodeWith: 'wasm-utxo' forces decodeTransactionFromPrebuild to
40+
// return fixedScriptWallet.BitGoPsbt (the path that used to trip
41+
// the guard).
42+
await signTransaction(coin, defaultBitGo, {
43+
txPrebuild: { txHex: psbtHex, decodeWith: 'wasm-utxo' },
44+
prv: userPrv,
45+
wallet,
46+
} as any);
47+
} catch (e) {
48+
caught = e as Error;
49+
}
50+
51+
assert.ok(
52+
!caught || !/descriptor wallets require PSBT format transactions/.test(caught.message),
53+
`descriptor sign guard incorrectly rejected BitGoPsbt: ${caught?.message}`
54+
);
55+
});
56+
});

0 commit comments

Comments
 (0)