diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 7d6da1fea5..200cee24ac 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -17,7 +17,6 @@ import { InvalidAddressError, IRequestTracer, isTriple, - ITransactionExplanation as BaseTransactionExplanation, IWallet, KeychainsTriplet, KeyIndices, @@ -66,6 +65,7 @@ import { parseTransaction, verifyTransaction, } from './transaction'; +import type { TransactionExplanation } from './transaction/fixedScript/explainTransaction'; import { AggregateValidationError, ErrorMissingOutputs, @@ -200,29 +200,6 @@ export function isWalletOutput(output: Output): output is FixedScriptWalletOutpu ); } -export interface TransactionExplanation extends BaseTransactionExplanation { - locktime?: number; - /** NOTE: this actually only captures external outputs */ - outputs: Output[]; - changeOutputs: Output[]; - - /** - * Number of input signatures per input. - */ - inputSignatures: number[]; - - /** - * Highest input signature count for the transaction - */ - signatures: number; - - /** - * BIP322 messages extracted from the transaction inputs. - * These messages are used for verifying the transaction against the BIP322 standard. - */ - messages?: Bip322Message[]; -} - export interface TransactionInfo { /** Maps txid to txhex. Required for offline signing. */ txHexes?: Record; @@ -870,9 +847,9 @@ export abstract class AbstractUtxoCoin extends BaseCoin { * change amounts, and transaction outputs. * @param params */ - async explainTransaction( + override async explainTransaction( params: ExplainTransactionOptions - ): Promise> { + ): Promise { return explainTx(this.decodeTransactionFromPrebuild(params), params, this.network); } diff --git a/modules/abstract-utxo/src/impl/doge/doge.ts b/modules/abstract-utxo/src/impl/doge/doge.ts index 3a7d1e394b..937e08cbf3 100644 --- a/modules/abstract-utxo/src/impl/doge/doge.ts +++ b/modules/abstract-utxo/src/impl/doge/doge.ts @@ -6,7 +6,6 @@ import { UtxoNetwork, SignTransactionOptions, ExplainTransactionOptions, - TransactionExplanation, ParseTransactionOptions, ParsedTransaction, VerifyTransactionOptions, @@ -14,6 +13,7 @@ import { TransactionInfo, TransactionPrebuild, } from '../../abstractUtxoCoin'; +import type { TransactionExplanation } from '../../transaction/fixedScript/explainTransaction'; import type { CrossChainRecoverySigned, CrossChainRecoveryUnsigned } from '../../recovery/crossChainRecovery'; type UnspentJSON = bitgo.Unspent & { valueString: string }; @@ -114,7 +114,7 @@ export class Doge extends AbstractUtxoCoin { async explainTransaction( params: ExplainTransactionOptions | (ExplainTransactionOptions & { txInfo: TransactionInfoJSON }) - ): Promise> { + ): Promise { return super.explainTransaction({ ...params, txInfo: params.txInfo ? parseTransactionInfo(params.txInfo as TransactionInfoJSON) : undefined, diff --git a/modules/abstract-utxo/src/transaction/descriptor/explainPsbt.ts b/modules/abstract-utxo/src/transaction/descriptor/explainPsbt.ts index 58e2257c34..1ad3537058 100644 --- a/modules/abstract-utxo/src/transaction/descriptor/explainPsbt.ts +++ b/modules/abstract-utxo/src/transaction/descriptor/explainPsbt.ts @@ -3,7 +3,7 @@ import { ITransactionRecipient } from '@bitgo/sdk-core'; import * as coreDescriptors from '@bitgo/utxo-core/descriptor'; import { toExtendedAddressFormat } from '../recipient'; -import { TransactionExplanation } from '../../abstractUtxoCoin'; +import type { TransactionExplanationUtxolibPsbt } from '../fixedScript/explainTransaction'; function toRecipient(output: coreDescriptors.ParsedOutput, network: utxolib.Network): ITransactionRecipient { return { @@ -34,7 +34,7 @@ function getInputSignatures(psbt: utxolib.bitgo.UtxoPsbt): number[] { export function explainPsbt( psbt: utxolib.bitgo.UtxoPsbt, descriptors: coreDescriptors.DescriptorMap -): TransactionExplanation { +): TransactionExplanationUtxolibPsbt { const parsedTransaction = coreDescriptors.parse(psbt, descriptors, psbt.network); const { inputs, outputs } = parsedTransaction; const externalOutputs = outputs.filter((o) => o.scriptId === undefined); diff --git a/modules/abstract-utxo/src/transaction/explainTransaction.ts b/modules/abstract-utxo/src/transaction/explainTransaction.ts index bf7663cc44..0e2a93d73e 100644 --- a/modules/abstract-utxo/src/transaction/explainTransaction.ts +++ b/modules/abstract-utxo/src/transaction/explainTransaction.ts @@ -1,11 +1,14 @@ import * as utxolib from '@bitgo/utxo-lib'; import { isTriple, IWallet } from '@bitgo/sdk-core'; -import { TransactionExplanation } from '../abstractUtxoCoin'; import { getDescriptorMapFromWallet, isDescriptorWallet } from '../descriptor'; import { toBip32Triple } from '../keychains'; import { getPolicyForEnv } from '../descriptor/validatePolicy'; +import type { + TransactionExplanationUtxolibLegacy, + TransactionExplanationUtxolibPsbt, +} from './fixedScript/explainTransaction'; import * as fixedScript from './fixedScript'; import * as descriptor from './descriptor'; @@ -22,7 +25,7 @@ export function explainTx( changeInfo?: fixedScript.ChangeAddressInfo[]; }, network: utxolib.Network -): TransactionExplanation { +): TransactionExplanationUtxolibLegacy | TransactionExplanationUtxolibPsbt { if (params.wallet && isDescriptorWallet(params.wallet)) { if (tx instanceof utxolib.bitgo.UtxoPsbt) { if (!params.pubs || !isTriple(params.pubs)) { diff --git a/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts index d78d7627cb..e136810db1 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts @@ -2,13 +2,53 @@ import * as utxolib from '@bitgo/utxo-lib'; import { bip322 } from '@bitgo/utxo-core'; import { BIP32Interface, bip32 } from '@bitgo/secp256k1'; import { bitgo } from '@bitgo/utxo-lib'; -import { Triple } from '@bitgo/sdk-core'; +import { ITransactionExplanation as BaseTransactionExplanation, Triple } from '@bitgo/sdk-core'; import * as utxocore from '@bitgo/utxo-core'; -import { Output, TransactionExplanation, Bip322Message, FixedScriptWalletOutput } from '../../abstractUtxoCoin'; +import type { Output, Bip322Message, FixedScriptWalletOutput } from '../../abstractUtxoCoin'; import { toExtendedAddressFormat } from '../recipient'; import { getPayGoVerificationPubkey } from '../getPayGoVerificationPubkey'; +// ===== Transaction Explanation Type Definitions ===== + +export interface AbstractUtxoTransactionExplanation extends BaseTransactionExplanation { + /** NOTE: this actually only captures external outputs */ + outputs: Output[]; + changeOutputs: Output[]; + + /** + * BIP322 messages extracted from the transaction inputs. + * These messages are used for verifying the transaction against the BIP322 standard. + */ + messages?: Bip322Message[]; +} + +/** @deprecated - the signature fields are not very useful */ +interface TransactionExplanationWithSignatures extends AbstractUtxoTransactionExplanation { + /** @deprecated - unused outside of tests */ + locktime?: number; + + /** + * Number of input signatures per input. + * @deprecated - this is not very useful without knowing who signed each input. + */ + inputSignatures: number[]; + + /** + * Highest input signature count for the transaction + * @deprecated - this is not very useful without knowing who signed each input. + */ + signatures: number; +} + +/** When parsing the legacy transaction format, we cannot always infer the fee so we set it to string | undefined */ +export type TransactionExplanationUtxolibLegacy = TransactionExplanationWithSignatures; + +/** When parsing a PSBT, we can infer the fee so we set TFee to string. */ +export type TransactionExplanationUtxolibPsbt = TransactionExplanationWithSignatures; + +export type TransactionExplanation = TransactionExplanationUtxolibLegacy | TransactionExplanationUtxolibPsbt; + export type ChangeAddressInfo = { address: string; chain: number; @@ -302,7 +342,7 @@ export function explainPsbt( }, network: utxolib.Network, { strict = false }: { strict?: boolean } = {} -): TransactionExplanation { +): TransactionExplanationUtxolibPsbt { const payGoVerificationInfo = getPayGoVerificationInfo(psbt, network); if (payGoVerificationInfo) { try { @@ -356,7 +396,7 @@ export function explainLegacyTx( changeInfo?: { address: string; chain: number; index: number }[]; }, network: utxolib.Network -): TransactionExplanation { +): TransactionExplanationUtxolibLegacy { const common = explainCommon(tx, params, network); const inputSignaturesCount = getTxInputSignaturesCount(tx, params, network); return { diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index 024ed86fa1..28879a88f2 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -4,11 +4,10 @@ import _ from 'lodash'; import { Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core'; import * as utxolib from '@bitgo/utxo-lib'; -import { +import type { AbstractUtxoCoin, FixedScriptWalletOutput, Output, - TransactionExplanation, ParsedTransaction, ParseTransactionOptions, } from '../../abstractUtxoCoin'; @@ -16,6 +15,7 @@ import { fetchKeychains, getKeySignatures, toKeychainTriple, UtxoKeychain, UtxoN import { ComparableOutput, outputDifference } from '../outputDifference'; import { fromExtendedAddressFormatToScript, toExtendedAddressFormat } from '../recipient'; +import type { TransactionExplanation } from './explainTransaction'; import { CustomChangeOptions, parseOutput } from './parseOutput'; export type ComparableOutputWithExternal = ComparableOutput & { @@ -53,7 +53,7 @@ export async function parseTransaction( } // obtain all outputs - const explanation: TransactionExplanation = await coin.explainTransaction({ + const explanation: TransactionExplanation = await coin.explainTransaction({ txHex: txPrebuild.txHex, txInfo: txPrebuild.txInfo, pubs: keychainArray.map((k) => k.pub) as Triple, diff --git a/modules/abstract-utxo/test/unit/explainTransaction.ts b/modules/abstract-utxo/test/unit/explainTransaction.ts index ad4684ad7b..aa5d368534 100644 --- a/modules/abstract-utxo/test/unit/explainTransaction.ts +++ b/modules/abstract-utxo/test/unit/explainTransaction.ts @@ -28,6 +28,7 @@ describe('Explain Transaction', function () { assert.strictEqual(result.outputs.length, 1); assert.strictEqual(result.outputs[0].address, 'scriptPubKey:6a'); assert.strictEqual(result.fee, '0'); + assert.ok('signatures' in result); assert.strictEqual(result.signatures, 0); assert.ok(result.messages); result.messages?.forEach((obj) => { @@ -45,6 +46,7 @@ describe('Explain Transaction', function () { assert.strictEqual(result.outputs.length, 1); assert.strictEqual(result.outputs[0].address, 'scriptPubKey:6a'); assert.strictEqual(result.fee, '0'); + assert.ok('signatures' in result); assert.strictEqual(result.signatures, 1); assert.ok(result.messages); result.messages?.forEach((obj) => { @@ -62,6 +64,7 @@ describe('Explain Transaction', function () { assert.strictEqual(result.outputs.length, 1); assert.strictEqual(result.outputs[0].address, 'scriptPubKey:6a'); assert.strictEqual(result.fee, '0'); + assert.ok('signatures' in result); assert.strictEqual(result.signatures, 2); assert.ok(result.messages); result.messages?.forEach((obj) => { diff --git a/modules/abstract-utxo/test/unit/parseTransaction.ts b/modules/abstract-utxo/test/unit/parseTransaction.ts index ccc7b88b19..f746c9794b 100644 --- a/modules/abstract-utxo/test/unit/parseTransaction.ts +++ b/modules/abstract-utxo/test/unit/parseTransaction.ts @@ -3,7 +3,8 @@ import assert from 'assert'; import * as sinon from 'sinon'; import { Wallet, UnexpectedAddressError, VerificationOptions } from '@bitgo/sdk-core'; -import { UtxoWallet, Output, TransactionExplanation, TransactionParams } from '../../src'; +import { UtxoWallet, Output, TransactionParams } from '../../src'; +import type { TransactionExplanation } from '../../src/transaction/fixedScript/explainTransaction'; import { getUtxoCoin } from './util'; diff --git a/modules/abstract-utxo/test/unit/transaction.ts b/modules/abstract-utxo/test/unit/transaction.ts index 1623b62fef..b5ee481aa0 100644 --- a/modules/abstract-utxo/test/unit/transaction.ts +++ b/modules/abstract-utxo/test/unit/transaction.ts @@ -564,12 +564,16 @@ function run( ? 2 : undefined; - explanation.inputSignatures.should.eql( - // FIXME(BG-35154): implement signature verification for replay protection inputs - inputScripts.map((type) => (type === 'p2shP2pk' ? 0 : expectedSignatureCount)) - ); - explanation.signatures.should.eql(expectedSignatureCount); - explanation.changeAmount.should.eql('0'); // no change addresses given + if ('inputSignatures' in explanation) { + explanation.inputSignatures.should.eql( + // FIXME(BG-35154): implement signature verification for replay protection inputs + inputScripts.map((type) => (type === 'p2shP2pk' ? 0 : expectedSignatureCount)) + ); + } + if ('signatures' in explanation) { + explanation.signatures.should.eql(expectedSignatureCount); + explanation.changeAmount.should.eql('0'); // no change addresses given + } let expectedOutputAmount = BigInt((txFormat === 'psbt' ? getUnspentsForPsbt() : getUnspents()).length) * BigInt(value); inputScripts.forEach((type) => { diff --git a/modules/abstract-utxo/test/unit/transaction/descriptor/explainPsbt.ts b/modules/abstract-utxo/test/unit/transaction/descriptor/explainPsbt.ts index 0175db23cb..29dfd4e490 100644 --- a/modules/abstract-utxo/test/unit/transaction/descriptor/explainPsbt.ts +++ b/modules/abstract-utxo/test/unit/transaction/descriptor/explainPsbt.ts @@ -3,7 +3,7 @@ import assert from 'assert'; import { getKeyTriple } from '@bitgo/utxo-core/testutil'; import { getDescriptorMap, mockPsbtDefaultWithDescriptorTemplate } from '@bitgo/utxo-core/testutil/descriptor'; -import { TransactionExplanation } from '../../../../src'; +import type { TransactionExplanation } from '../../../../src/transaction/fixedScript/explainTransaction'; import { explainPsbt } from '../../../../src/transaction/descriptor'; import { getFixtureRoot } from './fixtures.utils'; @@ -11,6 +11,8 @@ import { getFixtureRoot } from './fixtures.utils'; const { assertEqualFixture } = getFixtureRoot(__dirname + '/fixtures'); function assertSignatureCount(expl: TransactionExplanation, signatures: number, inputSignatures: number[]) { + assert.ok('signatures' in expl); + assert.ok('inputSignatures' in expl); assert.deepStrictEqual(expl.signatures, signatures); assert.deepStrictEqual(expl.inputSignatures, inputSignatures); } diff --git a/modules/abstract-utxo/test/unit/transaction/fixedScript/explainPsbt.ts b/modules/abstract-utxo/test/unit/transaction/fixedScript/explainPsbt.ts index 4b9697a4bb..9b0e1fe237 100644 --- a/modules/abstract-utxo/test/unit/transaction/fixedScript/explainPsbt.ts +++ b/modules/abstract-utxo/test/unit/transaction/fixedScript/explainPsbt.ts @@ -2,35 +2,33 @@ import assert from 'node:assert/strict'; import { testutil } from '@bitgo/utxo-lib'; +import type { TransactionExplanation } from '../../../../src/transaction/fixedScript/explainTransaction'; import { explainPsbt } from '../../../../src/transaction/fixedScript'; function describeTransactionWith(acidTest: testutil.AcidTest) { - describe(`explainPsbt ${acidTest.name}`, function () { - it('should explain the transaction', function () { + describe(`${acidTest.name}`, function () { + let refExplanation: TransactionExplanation; + before('prepare', function () { const psbt = acidTest.createPsbt(); - const explanation = explainPsbt(psbt, { pubs: acidTest.rootWalletKeys }, acidTest.network, { strict: true }); - assert.strictEqual(explanation.outputs.length, 3); - assert.strictEqual(explanation.outputAmount, '2700'); - assert.strictEqual(explanation.changeOutputs.length, acidTest.outputs.length - 3); - explanation.changeOutputs.forEach((change) => { + refExplanation = explainPsbt(psbt, { pubs: acidTest.rootWalletKeys }, acidTest.network, { + strict: true, + }); + }); + + it('should match the expected values for explainPsbt', function () { + // note: `outputs` means external outputs here + assert.strictEqual(refExplanation.outputs.length, 3); + assert.strictEqual(refExplanation.changeOutputs.length, acidTest.outputs.length - 3); + assert.strictEqual(refExplanation.outputAmount, '2700'); + assert.strictEqual(refExplanation.changeOutputs.length, acidTest.outputs.length - 3); + refExplanation.changeOutputs.forEach((change) => { assert.strictEqual(change.amount, '900'); assert.strictEqual(typeof change.address, 'string'); }); - assert.strictEqual(explanation.inputSignatures.length, acidTest.inputs.length); - explanation.inputSignatures.forEach((signature, i) => { - if (acidTest.inputs[i].scriptType === 'p2shP2pk') { - return; - } - if (acidTest.signStage === 'unsigned') { - assert.strictEqual(signature, 0); - } else if (acidTest.signStage === 'halfsigned') { - assert.strictEqual(signature, 1); - } else if (acidTest.signStage === 'fullsigned') { - assert.strictEqual(signature, 2); - } - }); }); }); } -testutil.AcidTest.suite().forEach((test) => describeTransactionWith(test)); +describe('explainPsbt', function () { + testutil.AcidTest.suite().forEach((test) => describeTransactionWith(test)); +});