diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 7eed62252e..e108fddb15 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -59,6 +59,7 @@ import { isReplayProtectionUnspent } from './transaction/fixedScript/replayProte import { supportedCrossChainRecoveries } from './config'; import { assertValidTransactionRecipient, + DecodedTransaction, explainTx, fromExtendedAddressFormat, isScriptRecipient, @@ -178,9 +179,7 @@ function convertValidationErrorToTxIntentMismatch( return txIntentError; } -export type DecodedTransaction = - | utxolib.bitgo.UtxoTransaction - | utxolib.bitgo.UtxoPsbt; +export type { DecodedTransaction } from './transaction/types'; export type RootWalletKeys = bitgo.RootWalletKeys; @@ -548,6 +547,14 @@ export abstract class AbstractUtxoCoin extends BaseCoin { } } + decodeTransactionAsPsbt(input: Buffer | string): utxolib.bitgo.UtxoPsbt { + const decoded = this.decodeTransaction(input); + if (!(decoded instanceof utxolib.bitgo.UtxoPsbt)) { + throw new Error('expected psbt but got transaction'); + } + return decoded; + } + decodeTransactionFromPrebuild(prebuild: { txHex?: string; txBase64?: string; @@ -712,12 +719,23 @@ export abstract class AbstractUtxoCoin extends BaseCoin { * @param psbtHex all MuSig2 inputs should contain user MuSig2 nonce * @param walletId */ - async signPsbt(psbtHex: string, walletId: string): Promise { - const params: SignPsbtRequest = { psbt: psbtHex }; - return await this.bitgo + async getMusig2Nonces(psbt: utxolib.bitgo.UtxoPsbt, walletId: string): Promise { + const params: SignPsbtRequest = { psbt: psbt.toHex() }; + const response = await this.bitgo .post(this.url('/wallet/' + walletId + '/tx/signpsbt')) .send(params) .result(); + return this.decodeTransactionAsPsbt(response.psbt); + } + + /** + * @deprecated Use getMusig2Nonces instead + * @returns input psbt added with deterministic MuSig2 nonce for bitgo key for each MuSig2 inputs. + * @param psbtHex all MuSig2 inputs should contain user MuSig2 nonce + * @param walletId + */ + async signPsbt(psbtHex: string, walletId: string): Promise { + return { psbt: (await this.getMusig2Nonces(this.decodeTransactionAsPsbt(psbtHex), walletId)).toHex() }; } /** @@ -727,9 +745,11 @@ export abstract class AbstractUtxoCoin extends BaseCoin { async signPsbtFromOVC(ovcJson: Record): Promise> { assert(ovcJson['psbtHex'], 'ovcJson must contain psbtHex'); assert(ovcJson['walletId'], 'ovcJson must contain walletId'); - const psbt = (await this.signPsbt(ovcJson['psbtHex'] as string, ovcJson['walletId'] as string)).psbt; - assert(psbt, 'psbt not found'); - return _.extend(ovcJson, { txHex: psbt }); + const psbt = await this.getMusig2Nonces( + this.decodeTransactionAsPsbt(ovcJson['psbtHex'] as string), + ovcJson['walletId'] as string + ); + return _.extend(ovcJson, { txHex: psbt.toHex() }); } /** diff --git a/modules/abstract-utxo/src/transaction/fixedScript/index.ts b/modules/abstract-utxo/src/transaction/fixedScript/index.ts index a4a5348e7f..12521bfe3e 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/index.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/index.ts @@ -3,6 +3,6 @@ export { explainPsbtWasm } from './explainPsbtWasm'; export { parseTransaction } from './parseTransaction'; export { CustomChangeOptions } from './parseOutput'; export { verifyTransaction } from './verifyTransaction'; -export { signTransaction } from './signTransaction'; +export { signTransaction, Musig2Participant } from './signTransaction'; export * from './sign'; export * from './replayProtection'; diff --git a/modules/abstract-utxo/src/transaction/fixedScript/sign.ts b/modules/abstract-utxo/src/transaction/fixedScript/sign.ts index ddba7bc73e..ec73016469 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/sign.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/sign.ts @@ -11,7 +11,7 @@ type Unspent = utxolib.bitgo.Unspent extends Error { static expectedWalletUnspent( inputIndex: number, + inputType: PsbtParsedScriptType | null, // null for legacy transaction format unspent: Unspent | { id: string } ): InputSigningError { - return new InputSigningError(inputIndex, unspent, `not a wallet unspent, not a replay protection unspent`); + return new InputSigningError( + inputIndex, + inputType, + unspent, + `not a wallet unspent, not a replay protection unspent` + ); } constructor( public inputIndex: number, + public inputType: PsbtParsedScriptType | null, // null for legacy transaction format public unspent: Unspent | { id: string }, public reason: Error | string ) { - super(`signing error at input ${inputIndex}: unspentId=${unspent.id}: ${reason}`); + super(`signing error at input ${inputIndex}: type=${inputType} unspentId=${unspent.id}: ${reason}`); } } @@ -70,7 +77,7 @@ export function signAndVerifyPsbt( ): utxolib.bitgo.UtxoPsbt | utxolib.bitgo.UtxoTransaction { const txInputs = psbt.txInputs; const outputIds: string[] = []; - const scriptTypes: PsbtParsedScriptTypes[] = []; + const scriptTypes: PsbtParsedScriptType[] = []; const signErrors: InputSigningError[] = psbt.data.inputs .map((input, inputIndex: number) => { @@ -89,7 +96,7 @@ export function signAndVerifyPsbt( psbt.signInputHD(inputIndex, signerKeychain); debug('Successfully signed input %d of %d', inputIndex + 1, psbt.data.inputs.length); } catch (e) { - return new InputSigningError(inputIndex, { id: outputId }, e); + return new InputSigningError(inputIndex, scriptType, { id: outputId }, e); } }) .filter((e): e is InputSigningError => e !== undefined); @@ -109,11 +116,11 @@ export function signAndVerifyPsbt( const outputId = outputIds[inputIndex]; try { if (!psbt.validateSignaturesOfInputHD(inputIndex, signerKeychain)) { - return new InputSigningError(inputIndex, { id: outputId }, new Error(`invalid signature`)); + return new InputSigningError(inputIndex, scriptType, { id: outputId }, new Error(`invalid signature`)); } } catch (e) { debug('Invalid signature'); - return new InputSigningError(inputIndex, { id: outputId }, e); + return new InputSigningError(inputIndex, scriptType, { id: outputId }, e); } }) .filter((e): e is InputSigningError => e !== undefined); @@ -178,13 +185,13 @@ export function signAndVerifyWalletTransaction( return; } if (!isWalletUnspent(unspent)) { - return InputSigningError.expectedWalletUnspent(inputIndex, unspent); + return InputSigningError.expectedWalletUnspent(inputIndex, null, unspent); } try { signInputWithUnspent(txBuilder, inputIndex, unspent, walletSigner); debug('Successfully signed input %d of %d', inputIndex + 1, unspents.length); } catch (e) { - return new InputSigningError(inputIndex, unspent, e); + return new InputSigningError(inputIndex, null, unspent, e); } }) .filter((e): e is InputSigningError => e !== undefined); @@ -203,18 +210,18 @@ export function signAndVerifyWalletTransaction( return; } if (!isWalletUnspent(unspent)) { - return InputSigningError.expectedWalletUnspent(inputIndex, unspent); + return InputSigningError.expectedWalletUnspent(inputIndex, null, unspent); } try { const publicKey = walletSigner.deriveForChainAndIndex(unspent.chain, unspent.index).signer.publicKey; if ( !utxolib.bitgo.verifySignatureWithPublicKey(signedTransaction, inputIndex, prevOutputs, publicKey) ) { - return new InputSigningError(inputIndex, unspent, new Error(`invalid signature`)); + return new InputSigningError(inputIndex, null, unspent, new Error(`invalid signature`)); } } catch (e) { debug('Invalid signature'); - return new InputSigningError(inputIndex, unspent, e); + return new InputSigningError(inputIndex, null, unspent, e); } }) .filter((e): e is InputSigningError => e !== undefined); diff --git a/modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts index 27c6854a21..d521ade6f0 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts @@ -6,10 +6,16 @@ import { bitgo } from '@bitgo/utxo-lib'; import * as utxolib from '@bitgo/utxo-lib'; import { isTriple, Triple } from '@bitgo/sdk-core'; -import { AbstractUtxoCoin, DecodedTransaction, RootWalletKeys } from '../../abstractUtxoCoin'; +import { DecodedTransaction } from '../types'; import { signAndVerifyPsbt, signAndVerifyWalletTransaction } from './sign'; +type RootWalletKeys = bitgo.RootWalletKeys; + +export interface Musig2Participant { + getMusig2Nonces(psbt: utxolib.bitgo.UtxoPsbt, walletId: string): Promise; +} + /** * Key Value: Unsigned tx id => PSBT * It is used to cache PSBTs with taproot key path (MuSig2) inputs during external express signer is activated. @@ -21,9 +27,10 @@ import { signAndVerifyPsbt, signAndVerifyWalletTransaction } from './sign'; const PSBT_CACHE = new Map(); export async function signTransaction( - coin: AbstractUtxoCoin, + coin: Musig2Participant, tx: DecodedTransaction, signerKeychain: BIP32Interface | undefined, + network: utxolib.Network, params: { walletId: string | undefined; txInfo: { unspents?: utxolib.bitgo.Unspent[] } | undefined; @@ -59,7 +66,7 @@ export async function signTransaction( return { txHex: tx.toHex() }; case 'cosignerNonce': assert(params.walletId, 'walletId is required for MuSig2 bitgo nonce'); - return { txHex: (await coin.signPsbt(tx.toHex(), params.walletId)).psbt }; + return { txHex: (await coin.getMusig2Nonces(tx, params.walletId)).toHex() }; case 'signerSignature': const txId = tx.getUnsignedTx().getId(); const psbt = PSBT_CACHE.get(txId); @@ -76,8 +83,8 @@ export async function signTransaction( assert(params.walletId, 'walletId is required for MuSig2 bitgo nonce'); assert(signerKeychain); tx.setAllInputsMusig2NonceHD(signerKeychain); - const response = await coin.signPsbt(tx.toHex(), params.walletId); - tx.combine(bitgo.createPsbtFromHex(response.psbt, coin.network)); + const response = await coin.getMusig2Nonces(tx, params.walletId); + tx = tx.combine(response); break; } } else { diff --git a/modules/abstract-utxo/src/transaction/signTransaction.ts b/modules/abstract-utxo/src/transaction/signTransaction.ts index ab8009c01e..9a4d45ee19 100644 --- a/modules/abstract-utxo/src/transaction/signTransaction.ts +++ b/modules/abstract-utxo/src/transaction/signTransaction.ts @@ -63,7 +63,7 @@ export async function signTransaction( throw new Error('expected a UtxoPsbt object'); } } else { - return fixedScript.signTransaction(coin, tx, getSignerKeychain(params.prv), { + return fixedScript.signTransaction(coin, tx, getSignerKeychain(params.prv), coin.network, { walletId: params.txPrebuild.walletId, txInfo: params.txPrebuild.txInfo, isLastSignature: params.isLastSignature ?? false, diff --git a/modules/abstract-utxo/src/transaction/types.ts b/modules/abstract-utxo/src/transaction/types.ts index f3a514594e..1f5f2c443e 100644 --- a/modules/abstract-utxo/src/transaction/types.ts +++ b/modules/abstract-utxo/src/transaction/types.ts @@ -1,7 +1,13 @@ +import * as utxolib from '@bitgo/utxo-lib'; + import type { UtxoNamedKeychains } from '../keychains'; import type { CustomChangeOptions } from './fixedScript'; +export type DecodedTransaction = + | utxolib.bitgo.UtxoTransaction + | utxolib.bitgo.UtxoPsbt; + export interface BaseOutput { address: string; amount: TAmount; diff --git a/modules/abstract-utxo/test/unit/transaction/fixedScript/signPsbt.ts b/modules/abstract-utxo/test/unit/transaction/fixedScript/signPsbt.ts new file mode 100644 index 0000000000..63880d4cdc --- /dev/null +++ b/modules/abstract-utxo/test/unit/transaction/fixedScript/signPsbt.ts @@ -0,0 +1,52 @@ +import assert from 'node:assert/strict'; + +import * as utxolib from '@bitgo/utxo-lib'; + +import { signAndVerifyPsbt } from '../../../../src/transaction/fixedScript/sign'; + +function describeSignAndVerifyPsbt(acidTest: utxolib.testutil.AcidTest) { + describe(`${acidTest.name}`, function () { + it('should sign unsigned psbt to halfsigned', function () { + // Create unsigned PSBT + const psbt = acidTest.createPsbt(); + + // Set musig2 nonces for taproot inputs before signing + const sessionId = Buffer.alloc(32); + psbt.setAllInputsMusig2NonceHD(acidTest.rootWalletKeys.user, { sessionId }); + psbt.setAllInputsMusig2NonceHD(acidTest.rootWalletKeys.bitgo, { deterministic: true }); + + // Sign with user key + const result = signAndVerifyPsbt(psbt, acidTest.rootWalletKeys.user, { + isLastSignature: false, + }); + + // Result should be a PSBT (not finalized) + assert(result instanceof utxolib.bitgo.UtxoPsbt, 'should return UtxoPsbt when not last signature'); + + // Verify that all wallet inputs have been signed by user key + result.data.inputs.forEach((input, inputIndex) => { + const { scriptType } = utxolib.bitgo.parsePsbtInput(input); + + // Skip replay protection inputs (p2shP2pk) + if (scriptType === 'p2shP2pk') { + return; + } + + // Verify user signature is present + const isValid = result.validateSignaturesOfInputHD(inputIndex, acidTest.rootWalletKeys.user); + assert(isValid, `input ${inputIndex} should have valid user signature`); + }); + }); + }); +} + +describe('signAndVerifyPsbt', function () { + // Create test suite with includeP2trMusig2ScriptPath set to false + // p2trMusig2 script path inputs are signed by user and backup keys, + // which is not the typical signing pattern and makes testing more complex + utxolib.testutil.AcidTest.suite({ includeP2trMusig2ScriptPath: false }) + .filter((test) => test.signStage === 'unsigned') + .forEach((test) => { + describeSignAndVerifyPsbt(test); + }); +}); diff --git a/modules/utxo-lib/src/testutil/psbt.ts b/modules/utxo-lib/src/testutil/psbt.ts index 2a4b0cdcdd..7e4bbb185a 100644 --- a/modules/utxo-lib/src/testutil/psbt.ts +++ b/modules/utxo-lib/src/testutil/psbt.ts @@ -258,6 +258,17 @@ export function constructPsbt( export const txFormats = ['psbt', 'psbt-lite'] as const; export type TxFormat = (typeof txFormats)[number]; +type SuiteConfig = { + /** + * By default, we include p2trMusig2 script path in the inputs. + * This input is a bit of a weirdo because it is signed by the user and the + * backup key, which usually is not mixed with other inputs and outputs. + * + * This option allows to exclude this input from the inputs. + */ + includeP2trMusig2ScriptPath?: boolean; +}; + /** * Creates a valid PSBT with as many features as possible. * @@ -297,7 +308,7 @@ export class AcidTest { this.outputs = outputs; } - static withDefaults(network: Network, signStage: SignStage, txFormat: TxFormat): AcidTest { + static withConfig(network: Network, signStage: SignStage, txFormat: TxFormat, suiteConfig: SuiteConfig): AcidTest { const rootWalletKeys = getDefaultWalletKeys(); const otherWalletKeys = getWalletKeysForSeed('too many secrets'); @@ -307,6 +318,7 @@ export class AcidTest { ? isSupportedScriptType(network, 'p2trMusig2') : isSupportedScriptType(network, scriptType) ) + .filter((scriptType) => (suiteConfig.includeP2trMusig2ScriptPath ?? true) || scriptType !== 'p2trMusig2') .map((scriptType) => ({ scriptType, value: BigInt(2000) })); const outputs: Output[] = outputScriptTypes @@ -345,12 +357,12 @@ export class AcidTest { return psbt; } - static suite(): AcidTest[] { + static suite(suiteConfig: SuiteConfig = {}): AcidTest[] { return getNetworkList() .filter((network) => isMainnet(network) && network !== networks.bitcoinsv) .flatMap((network) => signStages.flatMap((signStage) => - txFormats.flatMap((txFormat) => AcidTest.withDefaults(network, signStage, txFormat)) + txFormats.flatMap((txFormat) => AcidTest.withConfig(network, signStage, txFormat, suiteConfig)) ) ); }