From 5ec3759fe46c71d43a3aae10f10d61a2a3d6e1bb Mon Sep 17 00:00:00 2001 From: David Kaplan Date: Wed, 8 Nov 2023 16:33:06 -0500 Subject: [PATCH] feat(utxo-lib): build transaction without nonWitnessUtxo when specified TICKET: BTC-606 --- modules/utxo-lib/src/bitgo/wallet/Psbt.ts | 4 +- modules/utxo-lib/src/bitgo/wallet/Unspent.ts | 38 +++++++++++--- modules/utxo-lib/src/testutil/psbt.ts | 38 ++++++++++---- modules/utxo-lib/test/bitgo/psbt/Psbt.ts | 54 ++++++++++++++++++-- 4 files changed, 109 insertions(+), 25 deletions(-) diff --git a/modules/utxo-lib/src/bitgo/wallet/Psbt.ts b/modules/utxo-lib/src/bitgo/wallet/Psbt.ts index 0993f3e12e..e998cc7622 100644 --- a/modules/utxo-lib/src/bitgo/wallet/Psbt.ts +++ b/modules/utxo-lib/src/bitgo/wallet/Psbt.ts @@ -9,7 +9,7 @@ import { UtxoTransaction } from '../UtxoTransaction'; import { createOutputScript2of3, getLeafHash, scriptTypeForChain, toXOnlyPublicKey } from '../outputScripts'; import { DerivedWalletKeys, RootWalletKeys } from './WalletKeys'; import { toPrevOutputWithPrevTx } from '../Unspent'; -import { createPsbtFromTransaction } from '../transaction'; +import { createPsbtFromHex, createPsbtFromTransaction } from '../transaction'; import { isWalletUnspent, WalletUnspent } from './Unspent'; import { @@ -556,7 +556,7 @@ export function extractP2msOnlyHalfSignedTx(psbt: UtxoPsbt): UtxoTransaction { diff --git a/modules/utxo-lib/src/bitgo/wallet/Unspent.ts b/modules/utxo-lib/src/bitgo/wallet/Unspent.ts index 3b86c8a8c6..ea2eb3bba8 100644 --- a/modules/utxo-lib/src/bitgo/wallet/Unspent.ts +++ b/modules/utxo-lib/src/bitgo/wallet/Unspent.ts @@ -163,7 +163,8 @@ export function updateReplayProtectionUnspentToPsbt( psbt: UtxoPsbt, inputIndex: number, u: Unspent, - redeemScript?: Buffer + redeemScript?: Buffer, + customParams?: { skipNonWitnessUtxo?: boolean } ): void { if (!psbtIncludesUnspentAtIndex(psbt, inputIndex, u.id)) { throw new Error(`unspent does not correspond to psbt input`); @@ -178,12 +179,15 @@ export function updateReplayProtectionUnspentToPsbt( // with the previous transaction. Therefore, we can treat Zcash non-segwit transactions as Bitcoin // segwit transactions const isZcash = getMainnet(psbt.network) === networks.zcash; - if (!isUnspentWithPrevTx(u) && !isZcash) { + if (!isUnspentWithPrevTx(u) && !isZcash && !customParams?.skipNonWitnessUtxo) { throw new Error('Error, require previous tx to add to PSBT'); } if (isZcash && !input.witnessUtxo) { const { script, value } = toPrevOutput(u, psbt.network); psbt.updateInput(inputIndex, { witnessUtxo: { script, value } }); + } else if (customParams?.skipNonWitnessUtxo) { + const { script, value } = toPrevOutput(u, psbt.network); + psbt.updateInput(inputIndex, { witnessUtxo: { script, value } }); } else if (!isZcash && !input.nonWitnessUtxo) { psbt.updateInput(inputIndex, { nonWitnessUtxo: (u as UnspentWithPrevTx).prevTx }); } @@ -202,20 +206,29 @@ function addUnspentToPsbt( }); } -export function addReplayProtectionUnspentToPsbt(psbt: UtxoPsbt, u: Unspent, redeemScript: Buffer): void { +export function addReplayProtectionUnspentToPsbt( + psbt: UtxoPsbt, + u: Unspent, + redeemScript: Buffer, + customParams?: { skipNonWitnessUtxo?: boolean } +): void { addUnspentToPsbt(psbt, u.id); - updateReplayProtectionUnspentToPsbt(psbt, psbt.inputCount - 1, u, redeemScript); + updateReplayProtectionUnspentToPsbt(psbt, psbt.inputCount - 1, u, redeemScript, customParams); } /** * Update the PSBT with the unspent data for the input at the given index if the data is not there already. * + * If skipNonWitnessUtxo is true, then the nonWitnessUtxo will not be added for an input that requires it (e.g. non-segwit) + * and instead the witnessUtxo will be added + * * @param psbt * @param inputIndex * @param u * @param rootWalletKeys * @param signer * @param cosigner + * @param customParams */ export function updateWalletUnspentForPsbt( psbt: UtxoPsbt, @@ -223,7 +236,8 @@ export function updateWalletUnspentForPsbt( u: WalletUnspent, rootWalletKeys: RootWalletKeys, signer: KeyName, - cosigner: KeyName + cosigner: KeyName, + customParams?: { skipNonWitnessUtxo?: boolean } ): void { if (!psbtIncludesUnspentAtIndex(psbt, inputIndex, u.id)) { throw new Error(`unspent does not correspond to psbt input`); @@ -234,7 +248,7 @@ export function updateWalletUnspentForPsbt( // with the previous transaction. Therefore, we can treat Zcash non-segwit transactions as Bitcoin // segwit transactions const isZcashOrSegwit = isSegwit(u.chain) || getMainnet(psbt.network) === networks.zcash; - if (isZcashOrSegwit && !input.witnessUtxo) { + if ((isZcashOrSegwit && !input.witnessUtxo) || customParams?.skipNonWitnessUtxo) { const { script, value } = toPrevOutput(u, psbt.network); psbt.updateInput(inputIndex, { witnessUtxo: { script, value } }); } else if (!isZcashOrSegwit) { @@ -354,10 +368,18 @@ export function addWalletUnspentToPsbt( rootWalletKeys: RootWalletKeys, signer: KeyName, cosigner: KeyName, - customParams?: { sequenceNumber?: number } + customParams?: { sequenceNumber?: number; skipNonWitnessUtxo?: boolean } ): void { addUnspentToPsbt(psbt, u.id, { sequenceNumber: customParams ? customParams.sequenceNumber : TX_INPUT_SEQUENCE_NUMBER_FINAL, }); - updateWalletUnspentForPsbt(psbt, psbt.inputCount - 1, u, rootWalletKeys, signer, cosigner); + updateWalletUnspentForPsbt( + psbt, + psbt.inputCount - 1, + u, + rootWalletKeys, + signer, + cosigner, + customParams ? { skipNonWitnessUtxo: customParams.skipNonWitnessUtxo } : {} + ); } diff --git a/modules/utxo-lib/src/testutil/psbt.ts b/modules/utxo-lib/src/testutil/psbt.ts index 1726ead66d..fbe2de4b36 100644 --- a/modules/utxo-lib/src/testutil/psbt.ts +++ b/modules/utxo-lib/src/testutil/psbt.ts @@ -24,6 +24,7 @@ import { UtxoPsbt, UtxoTransaction, verifySignatureWithUnspent, + withUnsafeNonSegwit, } from '../bitgo'; import { Network } from '../networks'; import { mockReplayProtectionUnspent, mockWalletUnspent } from './mock'; @@ -116,19 +117,32 @@ export function signPsbtInput( params?: { signers?: { signerName: KeyName; cosignerName?: KeyName }; deterministic?: boolean; + skipNonWitnessUtxo?: boolean; } ): void { - const { signers, deterministic } = params ?? {}; + function signPsbt(psbt: UtxoPsbt, signFunc: () => void, skipNonWitnessUtxo?: boolean) { + if (skipNonWitnessUtxo) { + withUnsafeNonSegwit(psbt, signFunc); + } else { + signFunc(); + } + } + + const { signers, deterministic, skipNonWitnessUtxo } = params ?? {}; const { signerName, cosignerName } = signers ? signers : getSigners(input.scriptType); if (sign === 'halfsigned') { if (input.scriptType === 'p2shP2pk') { - psbt.signInput(inputIndex, rootWalletKeys[signerName]); + signPsbt(psbt, () => psbt.signInput(inputIndex, rootWalletKeys[signerName]), skipNonWitnessUtxo); } else { - psbt.signInputHD(inputIndex, rootWalletKeys[signerName]); + signPsbt(psbt, () => psbt.signInputHD(inputIndex, rootWalletKeys[signerName]), skipNonWitnessUtxo); } } if (sign === 'fullsigned' && cosignerName && input.scriptType !== 'p2shP2pk') { - psbt.signInputHD(inputIndex, rootWalletKeys[cosignerName], { deterministic }); + signPsbt( + psbt, + () => psbt.signInputHD(inputIndex, rootWalletKeys[cosignerName], { deterministic }), + skipNonWitnessUtxo + ); } } @@ -144,11 +158,12 @@ export function signAllPsbtInputs( params?: { signers?: { signerName: KeyName; cosignerName?: KeyName }; deterministic?: boolean; + skipNonWitnessUtxo?: boolean; } ): void { - const { signers, deterministic } = params ?? {}; + const { signers, deterministic, skipNonWitnessUtxo } = params ?? {}; inputs.forEach((input, inputIndex) => { - signPsbtInput(psbt, input, inputIndex, rootWalletKeys, sign, { signers, deterministic }); + signPsbtInput(psbt, input, inputIndex, rootWalletKeys, sign, { signers, deterministic, skipNonWitnessUtxo }); }); } @@ -164,9 +179,10 @@ export function constructPsbt( params?: { signers?: { signerName: KeyName; cosignerName?: KeyName }; deterministic?: boolean; + skipNonWitnessUtxo?: boolean; } ): UtxoPsbt { - const { signers, deterministic } = params ?? {}; + const { signers, deterministic, skipNonWitnessUtxo } = 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'); @@ -181,11 +197,11 @@ export function constructPsbt( unspents.forEach((u, i) => { const { signerName, cosignerName } = signers ? signers : getSigners(inputs[i].scriptType); if (isWalletUnspent(u) && cosignerName) { - addWalletUnspentToPsbt(psbt, u, rootWalletKeys, signerName, cosignerName); + addWalletUnspentToPsbt(psbt, u, rootWalletKeys, signerName, cosignerName, { skipNonWitnessUtxo }); } else { const { redeemScript } = createOutputScriptP2shP2pk(rootWalletKeys[signerName].publicKey); assert(redeemScript); - addReplayProtectionUnspentToPsbt(psbt, u, redeemScript); + addReplayProtectionUnspentToPsbt(psbt, u, redeemScript, { skipNonWitnessUtxo }); } }); @@ -211,10 +227,10 @@ export function constructPsbt( psbt.setAllInputsMusig2NonceHD(rootWalletKeys['user']); psbt.setAllInputsMusig2NonceHD(rootWalletKeys['bitgo'], { deterministic }); - signAllPsbtInputs(psbt, inputs, rootWalletKeys, 'halfsigned', { signers }); + signAllPsbtInputs(psbt, inputs, rootWalletKeys, 'halfsigned', { signers, skipNonWitnessUtxo }); if (sign === 'fullsigned') { - signAllPsbtInputs(psbt, inputs, rootWalletKeys, sign, { signers, deterministic }); + signAllPsbtInputs(psbt, inputs, rootWalletKeys, sign, { signers, deterministic, skipNonWitnessUtxo }); } return psbt; diff --git a/modules/utxo-lib/test/bitgo/psbt/Psbt.ts b/modules/utxo-lib/test/bitgo/psbt/Psbt.ts index 8cad0a4cf7..645f145f9e 100644 --- a/modules/utxo-lib/test/bitgo/psbt/Psbt.ts +++ b/modules/utxo-lib/test/bitgo/psbt/Psbt.ts @@ -190,7 +190,12 @@ function runExtractP2msOnlyHalfSignedTxTest(network: Network, inputs: Input[], o }); } -function runBuildSignSendFlowTest(network: Network, inputs: Input[], outputs: Output[]) { +function runBuildSignSendFlowTest( + network: Network, + inputs: Input[], + outputs: Output[], + { skipNonWitnessUtxo = false } = {} +) { const coin = getNetworkName(network); function assertValidate(psbt: UtxoPsbt) { @@ -204,14 +209,24 @@ function runBuildSignSendFlowTest(network: Network, inputs: Input[], outputs: Ou } describe(`Build, sign & send flow for ${coin}`, function () { - it(`success for ${coin}`, function () { - const psbt = testutil.constructPsbt(inputs, outputs, network, rootWalletKeys, 'unsigned', { + /** + * Skip adding nonWitnessUtxos to psbts + * ------------------------------------ + * In the instance that we want to doing a bulk sweep, for network and client performance reasons we are substituting + * the nonWitnessUtxo for p2sh and p2shP2pk inputs with a witnessUtxo. We need the witnessUtxo so that we can half + * sign the transaction locally with the user key. When we send the half signed to BitGo, the PSBT will be properly + * populated such that the non-segwit inputs have the nonWitnessUtxo. This means when we send it to BitGo we should + * remove the witnessUtxo so that it just has the partialSig and redeemScript. + */ + it(`success for ${coin}${skipNonWitnessUtxo ? ' without nonWitnessUtxo for p2sh' : ''}`, function () { + const parentPsbt = testutil.constructPsbt(inputs, outputs, network, rootWalletKeys, 'unsigned', { signers: { signerName: 'user', cosignerName: 'bitgo', }, }); + let psbt = skipNonWitnessUtxo ? clonePsbtWithoutNonWitnessUtxo(parentPsbt) : parentPsbt; addXpubsToPsbt(psbt, rootWalletKeys); psbt.setAllInputsMusig2NonceHD(rootWalletKeys['user']); @@ -231,6 +246,7 @@ function runBuildSignSendFlowTest(network: Network, inputs: Input[], outputs: Ou signerName: 'user', cosignerName: 'bitgo', }, + skipNonWitnessUtxo, }); psbtWithoutPrevTx = clonePsbtWithoutNonWitnessUtxo(psbt); @@ -253,6 +269,10 @@ function runBuildSignSendFlowTest(network: Network, inputs: Input[], outputs: Ou psbtFromHsm = createPsbtFromHex(hexAtHsm, network); deleteWitnessUtxoForNonSegwitInputs(psbtFromHsm); + + if (skipNonWitnessUtxo) { + psbt = parentPsbt; + } psbt.combine(psbtFromHsm); assertValidate(psbt); @@ -261,6 +281,28 @@ function runBuildSignSendFlowTest(network: Network, inputs: Input[], outputs: Ou }); } +function runBuildPsbtWithSDK(network: Network, inputs: Input[], outputs: Output[]) { + const coin = getNetworkName(network); + it(`check that building a PSBT while skipping nonWitnessUtxo works - ${coin}`, async function () { + const psbtWithNonWitness = testutil.constructPsbt(inputs, outputs, network, rootWalletKeys, 'unsigned', { + signers: { + signerName: 'user', + cosignerName: 'bitgo', + }, + }); + const psbtWithoutNonWitness = testutil.constructPsbt(inputs, outputs, network, rootWalletKeys, 'unsigned', { + signers: { + signerName: 'user', + cosignerName: 'bitgo', + }, + skipNonWitnessUtxo: true, + }); + + const clonedPsbt = clonePsbtWithoutNonWitnessUtxo(psbtWithNonWitness); + assert.deepStrictEqual(psbtWithoutNonWitness.toHex(), clonedPsbt.toHex()); + }); +} + getNetworkList() .filter((v) => isMainnet(v) && v !== networks.bitcoinsv) .forEach((network) => { @@ -274,7 +316,11 @@ getNetworkList() isSupportedScriptType(network, input.scriptType === 'taprootKeyPathSpend' ? 'p2trMusig2' : input.scriptType) ); const supportedPsbtOutputs = psbtOutputs.filter((output) => isSupportedScriptType(network, output.scriptType)); - runBuildSignSendFlowTest(network, supportedPsbtInputs, supportedPsbtOutputs); + [false, true].forEach((skipNonWitnessUtxo) => + runBuildSignSendFlowTest(network, supportedPsbtInputs, supportedPsbtOutputs, { skipNonWitnessUtxo }) + ); + + runBuildPsbtWithSDK(network, supportedPsbtInputs, supportedPsbtOutputs); }); describe('isTransactionWithKeyPathSpendInput', function () {