Skip to content

Commit

Permalink
Merge pull request #3830 from BitGo/BTC-376-psbt-prevTx
Browse files Browse the repository at this point in the history 
fix(utxo-lib): remove prev tx from psbt
  • Loading branch information
@saravanan7mani
saravanan7mani authored Aug 22, 2023
2 parents 1409adc + 69bb9a1 commit 100b79e
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 28 deletions.
13 changes: 13 additions & 0 deletions modules/utxo-lib/src/bitgo/PsbtUtil.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { decodeProprietaryKey, ProprietaryKey } from 'bip174/src/lib/proprietaryKeyVal';
import { PsbtInput } from 'bip174/src/lib/interfaces';
import { Psbt } from 'bitcoinjs-lib/src/psbt';

/**
* bitgo proprietary key identifier
Expand Down Expand Up @@ -104,3 +105,15 @@ export function isPsbt(data: Buffer | string): boolean {
}
return 5 <= data.length && data.readUInt32BE(0) === 0x70736274 && data.readUInt8(4) === 0xff;
}

/**
* This function allows signing or validating a psbt with non-segwit inputs those do not contain nonWitnessUtxo.
*/
export function withUnsafeNonSegwit<T>(psbt: Psbt, fn: () => T, unsafe = true): T {
(psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT = unsafe;
try {
return fn();
} finally {
(psbt as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT = false;
}
}
34 changes: 34 additions & 0 deletions modules/utxo-lib/src/bitgo/wallet/Psbt.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,3 +550,37 @@ export function extractP2msOnlyHalfSignedTx(psbt: UtxoPsbt): UtxoTransaction<big

return tx;
}

/**
* Clones the psbt without nonWitnessUtxo for non-segwit inputs and witnessUtxo is added instead.
* It is not BIP-174 compliant, so use it carefully.
*/
export function clonePsbtWithoutNonWitnessUtxo(psbt: UtxoPsbt): UtxoPsbt {
const newPsbt = psbt.clone();
const txInputs = psbt.txInputs;

psbt.data.inputs.forEach((input, i) => {
if (input.nonWitnessUtxo && !input.witnessUtxo) {
const tx = UtxoTransaction.fromBuffer<bigint>(input.nonWitnessUtxo, false, 'bigint', psbt.network);
if (!txInputs[i].hash.equals(tx.getHash())) {
throw new Error(`Non-witness UTXO hash for input #${i} doesn't match the hash specified in the prevout`);
}
newPsbt.data.inputs[i].witnessUtxo = tx.outs[txInputs[i].index];
}
delete newPsbt.data.inputs[i].nonWitnessUtxo;
});

return newPsbt;
}

/**
* Deletes witnessUtxo for non-segwit inputs to make the PSBT BIP-174 compliant.
*/
export function deleteWitnessUtxoForNonSegwitInputs(psbt: UtxoPsbt): void {
psbt.data.inputs.forEach((input, i) => {
const scriptType = getPsbtInputScriptType(input);
if (scriptType === 'p2sh' || scriptType === 'p2shP2pk') {
delete input.witnessUtxo;
}
});
}
15 changes: 3 additions & 12 deletions modules/utxo-lib/src/bitgo/zcash/ZcashPsbt.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Network, PsbtTransaction, Signer } from '../../';
import { Psbt as PsbtBase } from 'bip174';
import * as types from 'bitcoinjs-lib/src/types';
import { ValidateSigFunction } from 'bitcoinjs-lib/src/psbt';
import { ProprietaryKeySubtype, PSBT_PROPRIETARY_IDENTIFIER } from '../PsbtUtil';
import { ProprietaryKeySubtype, PSBT_PROPRIETARY_IDENTIFIER, withUnsafeNonSegwit } from '../PsbtUtil';
const typeforce = require('typeforce');

const CONSENSUS_BRANCH_ID_KEY = Buffer.concat([
Expand Down Expand Up @@ -126,20 +126,11 @@ export class ZcashPsbt extends UtxoPsbt<ZcashTransaction<bigint>> {
// transactions because zcash hashes the value directly. Thus, it is unnecessary to have
// the previous transaction hash on the unspent.
signInput(inputIndex: number, keyPair: Signer, sighashTypes?: number[]): this {
return this.withUnsafeSignNonSegwitTrue(super.signInput.bind(this, inputIndex, keyPair, sighashTypes));
return withUnsafeNonSegwit(this, super.signInput.bind(this, inputIndex, keyPair, sighashTypes));
}

validateSignaturesOfInput(inputIndex: number, validator: ValidateSigFunction, pubkey?: Buffer): boolean {
return this.withUnsafeSignNonSegwitTrue(super.validateSignaturesOfInput.bind(this, inputIndex, validator, pubkey));
}

private withUnsafeSignNonSegwitTrue<T>(fn: () => T): T {
(this as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT = true;
try {
return fn();
} finally {
(this as any).__CACHE.__UNSAFE_SIGN_NONSEGWIT = false;
}
return withUnsafeNonSegwit(this, super.validateSignaturesOfInput.bind(this, inputIndex, validator, pubkey));
}

private setPropertyCheckSignatures(propName: keyof ZcashTransaction<bigint>, value: unknown) {
Expand Down
32 changes: 22 additions & 10 deletions modules/utxo-lib/src/testutil/psbt.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,12 @@ export function signPsbtInput(
inputIndex: number,
rootWalletKeys: RootWalletKeys,
sign: 'halfsigned' | 'fullsigned',
signers?: { signerName: KeyName; cosignerName?: KeyName }
params?: {
signers?: { signerName: KeyName; cosignerName?: KeyName };
deterministic?: boolean;
}
): void {
const { signers, deterministic } = params ?? {};
const { signerName, cosignerName } = signers ? signers : getSigners(input.scriptType);
if (sign === 'halfsigned') {
if (input.scriptType === 'p2shP2pk') {
Expand All @@ -124,8 +128,8 @@ export function signPsbtInput(
psbt.signInputHD(inputIndex, rootWalletKeys[signerName]);
}
}
if (sign === 'fullsigned' && cosignerName) {
psbt.signInputHD(inputIndex, rootWalletKeys[cosignerName]);
if (sign === 'fullsigned' && cosignerName && input.scriptType !== 'p2shP2pk') {
psbt.signInputHD(inputIndex, rootWalletKeys[cosignerName], { deterministic });
}
}

Expand All @@ -138,10 +142,14 @@ export function signAllPsbtInputs(
inputs: Input[],
rootWalletKeys: RootWalletKeys,
sign: 'halfsigned' | 'fullsigned',
signers?: { signerName: KeyName; cosignerName?: KeyName }
params?: {
signers?: { signerName: KeyName; cosignerName?: KeyName };
deterministic?: boolean;
}
): void {
inputs.forEach((input, index) => {
signPsbtInput(psbt, input, index, rootWalletKeys, sign, signers);
const { signers, deterministic } = params ?? {};
inputs.forEach((input, inputIndex) => {
signPsbtInput(psbt, input, inputIndex, rootWalletKeys, sign, { signers, deterministic });
});
}

Expand All @@ -154,8 +162,12 @@ export function constructPsbt(
network: Network,
rootWalletKeys: RootWalletKeys,
sign: 'unsigned' | 'halfsigned' | 'fullsigned',
signers?: { signerName: KeyName; cosignerName?: KeyName }
params?: {
signers?: { signerName: KeyName; cosignerName?: KeyName };
deterministic?: boolean;
}
): UtxoPsbt {
const { signers, deterministic } = params ?? {};
const totalInputAmount = inputs.reduce((sum, input) => sum + input.value, BigInt(0));
const outputInputAmount = outputs.reduce((sum, output) => sum + output.value, BigInt(0));
assert(totalInputAmount >= outputInputAmount, 'total output can not exceed total input');
Expand Down Expand Up @@ -198,12 +210,12 @@ export function constructPsbt(
}

psbt.setAllInputsMusig2NonceHD(rootWalletKeys['user']);
psbt.setAllInputsMusig2NonceHD(rootWalletKeys['bitgo']);
psbt.setAllInputsMusig2NonceHD(rootWalletKeys['bitgo'], { deterministic });

signAllPsbtInputs(psbt, inputs, rootWalletKeys, 'halfsigned');
signAllPsbtInputs(psbt, inputs, rootWalletKeys, 'halfsigned', { signers });

if (sign === 'fullsigned') {
signAllPsbtInputs(psbt, inputs, rootWalletKeys, sign, signers);
signAllPsbtInputs(psbt, inputs, rootWalletKeys, sign, { signers, deterministic });
}

return psbt;
Expand Down
98 changes: 92 additions & 6 deletions modules/utxo-lib/test/bitgo/psbt/Psbt.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ import {
extractP2msOnlyHalfSignedTx,
toOutput,
createTransactionBuilderFromTransaction,
addXpubsToPsbt,
clonePsbtWithoutNonWitnessUtxo,
deleteWitnessUtxoForNonSegwitInputs,
getPsbtInputScriptType,
withUnsafeNonSegwit,
} from '../../../src/bitgo';
import {
createOutputScript2of3,
Expand All @@ -41,6 +46,7 @@ import {
import {
getDefaultWalletKeys,
Input,
inputScriptTypes,
mockReplayProtectionUnspent,
Output,
outputScriptTypes,
Expand Down Expand Up @@ -82,6 +88,9 @@ const halfSignedInputs = (['p2sh', 'p2wsh', 'p2shP2wsh'] as const).map((scriptTy
}));
const halfSignedOutputs = outputScriptTypes.map((scriptType) => ({ scriptType, value: BigInt(500) }));

const psbtInputs = inputScriptTypes.map((scriptType) => ({ scriptType, value: BigInt(1000) }));
const psbtOutputs = outputScriptTypes.map((scriptType) => ({ scriptType, value: BigInt(900) }));

describe('extractP2msOnlyHalfSignedTx failure', function () {
it('invalid signature count', function () {
const psbt = testutil.constructPsbt(halfSignedInputs, halfSignedOutputs, network, rootWalletKeys, 'unsigned');
Expand Down Expand Up @@ -132,7 +141,7 @@ function runExtractP2msOnlyHalfSignedTxTest(network: Network, inputs: Input[], o
)
.filter((v) => !!v) as testutil.TxnInput<bigint>[];

const psbt = testutil.constructPsbt(inputs, outputs, network, rootWalletKeys, 'halfsigned', signers);
const psbt = testutil.constructPsbt(inputs, outputs, network, rootWalletKeys, 'halfsigned', { signers });
const halfSignedPsbtTx = extractP2msOnlyHalfSignedTx(psbt);

let txb = testutil.constructTxnBuilder(txnInputs, txnOutputs, network, rootWalletKeys, 'halfsigned', signers);
Expand All @@ -144,7 +153,7 @@ function runExtractP2msOnlyHalfSignedTxTest(network: Network, inputs: Input[], o
validatePsbtParsing(halfSignedPsbtTx, psbt, unspents, 'halfsigned');
validatePsbtParsing(halfSignedTxbTx, psbt, unspents, 'halfsigned');

testutil.signAllPsbtInputs(psbt, inputs, rootWalletKeys, 'fullsigned', signers);
testutil.signAllPsbtInputs(psbt, inputs, rootWalletKeys, 'fullsigned', { signers });
const fullySignedPsbt = psbt.clone();
const psbtTx = psbt.finalizeAllInputs().extractTransaction();

Expand All @@ -161,14 +170,91 @@ function runExtractP2msOnlyHalfSignedTxTest(network: Network, inputs: Input[], o
});
}

function runBuildSignSendFlowTest(network: Network, inputs: Input[], outputs: Output[]) {
const coin = getNetworkName(network);

function assertValidate(psbt: UtxoPsbt) {
psbt.data.inputs.forEach((input, i) => {
assert.ok(psbt.validateSignaturesOfInputHD(i, rootWalletKeys['user']));
if (getPsbtInputScriptType(input) !== 'p2shP2pk') {
assert.ok(psbt.validateSignaturesOfInputHD(i, rootWalletKeys['bitgo']));
}
});
assert.ok(psbt.validateSignaturesOfAllInputs());
}

describe(`Build, sign & send flow for ${coin}`, function () {
it(`success for ${coin}`, function () {
const psbt = testutil.constructPsbt(inputs, outputs, network, rootWalletKeys, 'unsigned', {
signers: {
signerName: 'user',
cosignerName: 'bitgo',
},
});

addXpubsToPsbt(psbt, rootWalletKeys);
psbt.setAllInputsMusig2NonceHD(rootWalletKeys['user']);

let psbtWithoutPrevTx = clonePsbtWithoutNonWitnessUtxo(psbt);
let hex = psbtWithoutPrevTx.toHex();

let psbtAtHsm = createPsbtFromHex(hex, network);
psbtAtHsm.setAllInputsMusig2NonceHD(rootWalletKeys['bitgo'], { deterministic: true });
let hexAtHsm = psbtAtHsm.toHex();

let psbtFromHsm = createPsbtFromHex(hexAtHsm, network);
deleteWitnessUtxoForNonSegwitInputs(psbtFromHsm);
psbt.combine(psbtFromHsm);

testutil.signAllPsbtInputs(psbt, inputs, rootWalletKeys, 'halfsigned', {
signers: {
signerName: 'user',
cosignerName: 'bitgo',
},
});

psbtWithoutPrevTx = clonePsbtWithoutNonWitnessUtxo(psbt);
hex = psbtWithoutPrevTx.toHex();

psbtAtHsm = createPsbtFromHex(hex, network);
withUnsafeNonSegwit(psbtAtHsm, () => {
testutil.signAllPsbtInputs(psbtAtHsm, inputs, rootWalletKeys, 'fullsigned', {
signers: {
signerName: 'user',
cosignerName: 'bitgo',
},
deterministic: true,
});
});
withUnsafeNonSegwit(psbtAtHsm, () => {
assertValidate(psbtAtHsm);
});
hexAtHsm = psbtAtHsm.toHex();

psbtFromHsm = createPsbtFromHex(hexAtHsm, network);
deleteWitnessUtxoForNonSegwitInputs(psbtFromHsm);
psbt.combine(psbtFromHsm);

assertValidate(psbt);
assert.doesNotThrow(() => psbt.finalizeAllInputs().extractTransaction());
});
});
}

getNetworkList()
.filter((v) => isMainnet(v) && v !== networks.bitcoinsv)
.forEach((network) => {
const supportedPsbtInputs = halfSignedInputs.filter((input) => isSupportedScriptType(network, input.scriptType));
const supportedPsbtOutputs = halfSignedOutputs.filter((output) =>
isSupportedScriptType(network, output.scriptType)
runExtractP2msOnlyHalfSignedTxTest(
network,
halfSignedInputs.filter((input) => isSupportedScriptType(network, input.scriptType)),
halfSignedOutputs.filter((output) => isSupportedScriptType(network, output.scriptType))
);

const supportedPsbtInputs = psbtInputs.filter((input) =>
isSupportedScriptType(network, input.scriptType === 'taprootKeyPathSpend' ? 'p2trMusig2' : input.scriptType)
);
runExtractP2msOnlyHalfSignedTxTest(network, supportedPsbtInputs, supportedPsbtOutputs);
const supportedPsbtOutputs = psbtOutputs.filter((output) => isSupportedScriptType(network, output.scriptType));
runBuildSignSendFlowTest(network, supportedPsbtInputs, supportedPsbtOutputs);
});

describe('isTransactionWithKeyPathSpendInput', function () {
Expand Down

0 comments on commit 100b79e

Please sign in to comment.