From 3d69e29003ceb9258822222a57c8fa6c80690177 Mon Sep 17 00:00:00 2001 From: Abhishek Agrawal Date: Tue, 22 Apr 2025 16:45:51 +0530 Subject: [PATCH] fix: call verifytransaction coin method after fetching txrequest from server for eddsa coins TICKET: WIN-5204 --- .../test/v2/unit/internal/tssUtils/eddsa.ts | 17 ++++---- modules/sdk-coin-algo/src/algo.ts | 39 +++++++++++++++++++ modules/sdk-coin-dot/src/dot.ts | 37 +++++++++++++++++- modules/sdk-coin-stx/src/stx.ts | 38 +++++++++++++++++- modules/sdk-coin-xtz/src/xtz.ts | 37 +++++++++++++++++- .../sdk-core/src/bitgo/baseCoin/iBaseCoin.ts | 1 + .../src/bitgo/utils/tss/eddsa/eddsa.ts | 34 +++++++++++++++- 7 files changed, 189 insertions(+), 14 deletions(-) diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts index 3d238b3988..a634109c0d 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts @@ -122,9 +122,12 @@ describe('TSS Utils:', async function () { }, }; + const validTxHex = + '020104096f306776291ac74cd6da1787f7b5de1f103b92d21c240d30f608f974cadb33a8051a34bc8acd438763976f96876115050f73828553566d111d7ac8bffebf587c40a0c8fc599f0defd59b130ea61bf75f441683124b8f74ec9bd3ab44f5b0c2df4f5f5987bfe26aa66013efd96d36360f2b4336c91f993259fb56051305614d429b3d6e69f97bc63846ba524986cc3a04d006898dee0c8e72a0dbbb95edaf9ca800000000000000000000000000000000000000000000000000000000000000008001d74c358d532e3cb5a42cbcb4b6d15c7d93165375427b5ef1a092234b74ff06a7d517192c568ee08a845f73d29788cf035c3145b21ab344d8062ea940000006ddf6e1d765a193d9cbe146ceeb79ac1cb485ed5f5b37913a8cf5857eff00a90549d7ae1e24cd8bb16e36e0519c66fd8b7c5223d660a46a6e0fed7a5ab262be02050303070104040000000804040602000a0c009435770000000009'; + const txRequest = { txRequestId: 'randomId', - unsignedTxs: [{ signableHex: 'MPC on a Friday night', serializedTxHex: 'MPC on a Friday night' }], + unsignedTxs: [{ signableHex: validTxHex, serializedTxHex: validTxHex }], signatureShares: [ { from: 'bitgo', @@ -554,9 +557,9 @@ describe('TSS Utils:', async function () { transactions: [], unsignedTxs: [ { - serializedTxHex: 'MPC on a Friday night', - signableHex: 'MPC on a Friday night', - derivationPath: 'm/0', + serializedTxHex: validTxHex, + signableHex: validTxHex, + derivationPath: 'm/4', }, ], date: new Date().toISOString(), @@ -643,9 +646,9 @@ describe('TSS Utils:', async function () { transactions: [], unsignedTxs: [ { - serializedTxHex: 'MPC on a Friday night', - signableHex: 'MPC on a Friday night', - derivationPath: 'm/0', + serializedTxHex: validTxHex, + signableHex: validTxHex, + derivationPath: 'm/4', }, ], date: new Date().toISOString(), diff --git a/modules/sdk-coin-algo/src/algo.ts b/modules/sdk-coin-algo/src/algo.ts index 5e1e514279..2ded874f89 100644 --- a/modules/sdk-coin-algo/src/algo.ts +++ b/modules/sdk-coin-algo/src/algo.ts @@ -579,6 +579,45 @@ export class Algo extends BaseCoin { } async verifyTransaction(params: VerifyTransactionOptions): Promise { + const { txPrebuild, txParams } = params; + + // Get raw transaction from either txHex or txRequest + const rawTx = + txPrebuild.txHex || + (txPrebuild.txRequest?.apiVersion === 'full' + ? txPrebuild.txRequest.transactions?.[0]?.unsignedTx?.signableHex + : txPrebuild.txRequest?.unsignedTxs?.[0]?.signableHex); + + if (!rawTx) { + throw new Error('missing required transaction hex'); + } + + const decodedTx = this.decodeTx(Buffer.from(rawTx, 'hex')); + + // Verify recipients if provided + if (txParams?.recipients) { + const recipients = txParams.recipients; + const txAmount = (decodedTx as any).amount; + const txToAddress = (decodedTx as any).to; + + // For Algorand, we currently only support single recipient transactions + if (recipients.length !== 1) { + throw new Error('Algorand only supports single recipient transactions'); + } + + const recipient = recipients[0]; + + // Verify recipient address matches + if (recipient.address !== txToAddress) { + throw new Error('transaction recipient address does not match expected address'); + } + + // Verify amount matches + if (new BigNumber(recipient.amount).isEqualTo(txAmount)) { + throw new Error('transaction amount does not match expected amount'); + } + } + return true; } diff --git a/modules/sdk-coin-dot/src/dot.ts b/modules/sdk-coin-dot/src/dot.ts index 3434a7a87d..ba03c1ecc2 100644 --- a/modules/sdk-coin-dot/src/dot.ts +++ b/modules/sdk-coin-dot/src/dot.ts @@ -646,12 +646,45 @@ export class Dot extends BaseCoin { } async verifyTransaction(params: VerifyTransactionOptions): Promise { - const { txParams } = params; - if (Array.isArray(txParams.recipients) && txParams.recipients.length > 1) { + const { txPrebuild, txParams } = params; + + // Verify single recipient constraint + if (Array.isArray(txParams?.recipients) && txParams.recipients.length > 1) { throw new Error( `${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.` ); } + + // Get raw transaction from either txHex or txRequest + const rawTx = + txPrebuild.txHex || + (txPrebuild.txRequest?.apiVersion === 'full' + ? txPrebuild.txRequest.transactions?.[0]?.unsignedTx?.signableHex + : txPrebuild.txRequest?.unsignedTxs?.[0]?.signableHex); + + if (!rawTx) { + throw new Error('missing required transaction hex'); + } + + // Verify recipient and amount if provided + if (txParams?.recipients?.[0]) { + const recipient = txParams.recipients[0]; + const txBuilder = this.getBuilder(); + const transaction = txBuilder.from(rawTx); + const decodedTx = await transaction.build(); + const explainedTx = decodedTx.explainTransaction(); + + // Verify recipient address matches + if (recipient.address !== explainedTx.to) { + throw new Error('transaction recipient address does not match expected address'); + } + + // Verify amount matches + if (!new BigNumber(recipient.amount).isEqualTo(explainedTx.amount)) { + throw new Error('transaction amount does not match expected amount'); + } + } + return true; } diff --git a/modules/sdk-coin-stx/src/stx.ts b/modules/sdk-coin-stx/src/stx.ts index 2d35b83bc9..53446b5304 100644 --- a/modules/sdk-coin-stx/src/stx.ts +++ b/modules/sdk-coin-stx/src/stx.ts @@ -101,12 +101,46 @@ export class Stx extends BaseCoin { } async verifyTransaction(params: VerifyTransactionOptions): Promise { - const { txParams } = params; - if (Array.isArray(txParams.recipients) && txParams.recipients.length > 1) { + const { txPrebuild, txParams } = params; + + // Verify single recipient constraint + if (Array.isArray(txParams?.recipients) && txParams.recipients.length > 1) { throw new Error( `${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.` ); } + + // Get raw transaction from either txHex or txRequest + const rawTx = + txPrebuild.txHex || + (txPrebuild.txRequest?.apiVersion === 'full' + ? txPrebuild.txRequest.transactions?.[0]?.unsignedTx?.signableHex + : txPrebuild.txRequest?.unsignedTxs?.[0]?.signableHex); + + if (!rawTx) { + throw new Error('missing required transaction hex'); + } + + // Verify recipient and amount if provided + if (txParams?.recipients?.[0]) { + const recipient = txParams.recipients[0]; + const coinConfig = coins.get(this.getChain()); + const txBuilder = new TransactionBuilderFactory(coinConfig).getTransferBuilder(); + txBuilder.from(rawTx); + const transaction = await txBuilder.build(); + const explainedTx = transaction.explainTransaction(); + + // Verify recipient address matches + if (recipient.address !== explainedTx.to) { + throw new Error('transaction recipient address does not match expected address'); + } + + // Verify amount matches + if (!new BigNumber(recipient.amount).isEqualTo(explainedTx.amount)) { + throw new Error('transaction amount does not match expected amount'); + } + } + return true; } diff --git a/modules/sdk-coin-xtz/src/xtz.ts b/modules/sdk-coin-xtz/src/xtz.ts index 67e54368a7..75d0ea109f 100644 --- a/modules/sdk-coin-xtz/src/xtz.ts +++ b/modules/sdk-coin-xtz/src/xtz.ts @@ -116,12 +116,45 @@ export class Xtz extends BaseCoin { } async verifyTransaction(params: VerifyTransactionOptions): Promise { - const { txParams } = params; - if (Array.isArray(txParams.recipients) && txParams.recipients.length > 1) { + const { txPrebuild, txParams } = params; + + // Verify single recipient constraint + if (Array.isArray(txParams?.recipients) && txParams.recipients.length > 1) { throw new Error( `${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.` ); } + + // Get raw transaction from either txHex or txRequest + const rawTx = + txPrebuild.txHex || + (txPrebuild.txRequest?.apiVersion === 'full' + ? txPrebuild.txRequest.transactions?.[0]?.unsignedTx?.signableHex + : txPrebuild.txRequest?.unsignedTxs?.[0]?.signableHex); + + if (!rawTx) { + throw new Error('missing required transaction hex'); + } + + // Verify recipient and amount if provided + if (txParams?.recipients?.[0]) { + const recipient = txParams.recipients[0]; + const txBuilder = new TransactionBuilder(coins.get(this.getChain())); + txBuilder.from(rawTx); + const transaction = await txBuilder.build(); + const explainedTx = transaction.explainTransaction(); + + // Verify recipient address matches + if (recipient.address !== explainedTx.to) { + throw new Error('transaction recipient address does not match expected address'); + } + + // Verify amount matches + if (!new BigNumber(recipient.amount).isEqualTo(explainedTx.amount)) { + throw new Error('transaction amount does not match expected amount'); + } + } + return true; } diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index 7f0a7a4c06..0b0f36af7e 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -279,6 +279,7 @@ export interface TransactionPrebuild extends BaseSignable { txBase64?: string; txHex?: string; txInfo?: unknown; + txRequest?: TxRequest; } export interface Message extends BaseSignable { diff --git a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts index 97cde95429..0dfd942b39 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts @@ -26,6 +26,7 @@ import { CustomRShareGeneratingFunction, EncryptedSignerShareRecord, EncryptedSignerShareType, + PopulatedIntent, SignatureShareRecord, SignatureShareType, TSSParamsWithPrv, @@ -37,6 +38,7 @@ import { KeychainsTriplet } from '../../../baseCoin'; import { exchangeEddsaCommitments } from '../../../tss/common'; import { Ed25519Bip32HdTree } from '@bitgo/sdk-lib-mpc'; import { IRequestTracer } from '../../../../api'; +import { Wallet } from '../../../wallet'; /** * Utility functions for TSS work flows. @@ -574,6 +576,31 @@ export class EddsaUtils extends baseTSSUtils { txRequestId = txRequest.txRequestId; } + // Verify the transaction before signing + const isVerified = await this.baseCoin.verifyTransaction({ + txParams: { + recipients: (txRequestResolved.intent as PopulatedIntent)?.recipients?.map((r) => ({ + address: r.address.address, + amount: r.amount.value.toString(), + })), + }, + txPrebuild: { + txHex: + txRequestResolved.apiVersion === 'full' + ? txRequestResolved.transactions?.[0]?.unsignedTx?.signableHex + : txRequestResolved.unsignedTxs[0]?.signableHex, + txInfo: + txRequestResolved.apiVersion === 'full' + ? txRequestResolved.transactions?.[0]?.unsignedTx + : txRequestResolved.unsignedTxs[0], + }, + wallet: this.wallet, + }); + + if (!isVerified) { + throw new Error('Transaction verification failed'); + } + const hdTree = await Ed25519Bip32HdTree.initialize(); const MPC = await Eddsa.initialize(hdTree); @@ -586,7 +613,12 @@ export class EddsaUtils extends baseTSSUtils { assert(txRequestResolved.transactions || txRequestResolved.unsignedTxs, 'Unable to find transactions in txRequest'); const unsignedTx = apiVersion === 'full' ? txRequestResolved.transactions![0].unsignedTx : txRequestResolved.unsignedTxs[0]; - + await this.baseCoin.verifyTransaction({ + txPrebuild: { txHex: unsignedTx.signableHex }, + wallet: this.wallet as unknown as Wallet, + txParams: (params as any).txParams || { recipients: [] }, + walletType: this.wallet.multisigType(), + }); const signingKey = MPC.keyDerive( userSigningMaterial.uShare, [userSigningMaterial.bitgoYShare, userSigningMaterial.backupYShare],