From 2a27243474731d5a8d43c41b56fa3cf970b78e22 Mon Sep 17 00:00:00 2001 From: Mohammad Faiyaz <101133667+mohammadalfaiyazbitgo@users.noreply.github.com> Date: Thu, 30 Apr 2026 12:47:25 -0400 Subject: [PATCH] Revert "fix(tss): verify recipients before signing" TICKET: WAL-375 --- .../src/abstractEthLikeNewCoins.ts | 13 +- .../test/v2/unit/internal/tssUtils/ecdsa.ts | 114 ----------- .../tssUtils/ecdsaMPCv2/signTxRequest.ts | 190 ------------------ .../bitgo/pendingApproval/pendingApproval.ts | 4 +- .../src/bitgo/utils/tss/baseTSSUtils.ts | 10 +- .../sdk-core/src/bitgo/utils/tss/baseTypes.ts | 7 +- .../src/bitgo/utils/tss/ecdsa/ecdsa.ts | 7 +- .../src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts | 7 +- .../src/bitgo/utils/tss/recipientUtils.ts | 53 ----- .../unit/bitgo/utils/tss/recipientUtils.ts | 121 ----------- 10 files changed, 11 insertions(+), 515 deletions(-) delete mode 100644 modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts delete mode 100644 modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts diff --git a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts index 80c6b623b7..7f830bd1a9 100644 --- a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts +++ b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts @@ -3109,16 +3109,9 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { txParams.prebuildTx?.consolidateId || txPrebuild?.consolidateId || (txParams.type && - [ - 'acceleration', - 'fillNonce', - 'transferToken', - 'tokenApproval', - 'consolidate', - 'bridgeFunds', - 'enableToken', - 'customTx', - ].includes(txParams.type)) + ['acceleration', 'fillNonce', 'transferToken', 'tokenApproval', 'consolidate', 'bridgeFunds'].includes( + txParams.type + )) ) ) { throw new Error('missing txParams'); diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts index 123430f937..9786e02c2a 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts @@ -749,7 +749,6 @@ describe('TSS Ecdsa Utils:', async function () { backupNShare: backupKeyShare.nShares[1], }), reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, }); signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); const userGpgActual = sendShareSpy.getCalls()[0].args[10] as string; @@ -767,125 +766,12 @@ describe('TSS Ecdsa Utils:', async function () { backupNShare: backupKeyShare.nShares[1], }), reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, }); signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); const userGpgActual = sendShareSpy.getCalls()[0].args[10] as string; userGpgActual.should.startWith('-----BEGIN PGP PUBLIC KEY BLOCK-----'); }); - it('signTxRequest should fail when txParams is missing', async function () { - await tssUtils - .signTxRequest({ - txRequest, - prv: JSON.stringify({ - pShare: userKeyShare.pShare, - bitgoNShare: bitgoKeyShare.nShares[1], - backupNShare: backupKeyShare.nShares[1], - }), - reqId, - }) - .should.be.rejectedWith( - 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' - ); - }); - - it('signTxRequest should fail when txParams has empty recipients', async function () { - await tssUtils - .signTxRequest({ - txRequest, - prv: JSON.stringify({ - pShare: userKeyShare.pShare, - bitgoNShare: bitgoKeyShare.nShares[1], - backupNShare: backupKeyShare.nShares[1], - }), - reqId, - txParams: { recipients: [] }, - }) - .should.be.rejectedWith( - 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' - ); - }); - - it('signTxRequest should succeed when recipients are only in intent (smart contract interaction)', async function () { - await setupSignTxRequestNocks(false, userSignShare, aShare, dShare, enterpriseData); - const txRequestWithIntentRecipients = { - ...txRequest, - intent: { - intentType: 'contractCall', - recipients: [ - { - address: { address: '0xrecipient' }, - amount: { value: '1000', symbol: 'hteth' }, - }, - ], - }, - }; - const signedTxRequest = await tssUtils.signTxRequest({ - txRequest: txRequestWithIntentRecipients, - prv: JSON.stringify({ - pShare: userKeyShare.pShare, - bitgoNShare: bitgoKeyShare.nShares[1], - backupNShare: backupKeyShare.nShares[1], - }), - reqId, - // no txParams.recipients — should fall back to intent.recipients - }); - signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); - }); - - it('signTxRequest should succeed for no-recipient tx types (tokenApproval)', async function () { - await setupSignTxRequestNocks(false, userSignShare, aShare, dShare, enterpriseData); - const signedTxRequest = await tssUtils.signTxRequest({ - txRequest, - prv: JSON.stringify({ - pShare: userKeyShare.pShare, - bitgoNShare: bitgoKeyShare.nShares[1], - backupNShare: backupKeyShare.nShares[1], - }), - reqId, - txParams: { type: 'tokenApproval' }, - }); - signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); - }); - - it('signTxRequest should succeed for no-recipient tx types (acceleration)', async function () { - await setupSignTxRequestNocks(false, userSignShare, aShare, dShare, enterpriseData); - const signedTxRequest = await tssUtils.signTxRequest({ - txRequest, - prv: JSON.stringify({ - pShare: userKeyShare.pShare, - bitgoNShare: bitgoKeyShare.nShares[1], - backupNShare: backupKeyShare.nShares[1], - }), - reqId, - txParams: { type: 'acceleration' }, - }); - signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); - }); - - it('signTxRequest should prefer txParams.recipients over intent.recipients when both are present', async function () { - await setupSignTxRequestNocks(false, userSignShare, aShare, dShare, enterpriseData); - const txRequestWithBothRecipients = { - ...txRequest, - intent: { - intentType: 'contractCall', - recipients: [{ address: { address: '0xintentRecipient' }, amount: { value: '9999', symbol: 'hteth' } }], - }, - }; - const signedTxRequest = await tssUtils.signTxRequest({ - txRequest: txRequestWithBothRecipients, - prv: JSON.stringify({ - pShare: userKeyShare.pShare, - bitgoNShare: bitgoKeyShare.nShares[1], - backupNShare: backupKeyShare.nShares[1], - }), - reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, - }); - signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); - }); - it('signTxRequest should fail with wrong recipient', async function () { // To generate these Hex values, we used the bitgo-ui to create a transaction and then // used the `signableHex` and `serializedTxHex` values from the prebuild. diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts index 739abb3506..9b2a43be4e 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts @@ -193,7 +193,6 @@ describe('signTxRequest:', function () { txRequest, prv: userPrvBase64, reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, }); nockPromises[0].isDone().should.be.true(); nockPromises[1].isDone().should.be.true(); @@ -216,7 +215,6 @@ describe('signTxRequest:', function () { prv: backupPrvBase64, mpcv2PartyId: 1, reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, }); nockPromises[0].isDone().should.be.true(); nockPromises[1].isDone().should.be.true(); @@ -238,7 +236,6 @@ describe('signTxRequest:', function () { txRequest, prv: userPrvBase64, reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, }); nockPromises[0].isDone().should.be.true(); nockPromises[1].isDone().should.be.true(); @@ -260,7 +257,6 @@ describe('signTxRequest:', function () { txRequest, prv: userPrvBase64, reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, }); nockPromises[0].isDone().should.be.true(); nockPromises[1].isDone().should.be.true(); @@ -281,197 +277,11 @@ describe('signTxRequest:', function () { txRequest, prv: userPrvBase64, reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, }) .should.be.rejectedWith('Too many requests, slow down!'); nockPromises[0].isDone().should.be.true(); nockPromises[1].isDone().should.be.false(); }); - - it('rejects signTxRequest when txParams is missing', async function () { - const userShare = fs.readFileSync(shareFiles[vector.party1]); - const userPrvBase64 = Buffer.from(userShare).toString('base64'); - await tssUtils - .signTxRequest({ - txRequest, - prv: userPrvBase64, - reqId, - }) - .should.be.rejectedWith( - 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' - ); - }); - - it('rejects signTxRequest when txParams has empty recipients', async function () { - const userShare = fs.readFileSync(shareFiles[vector.party1]); - const userPrvBase64 = Buffer.from(userShare).toString('base64'); - await tssUtils - .signTxRequest({ - txRequest, - prv: userPrvBase64, - reqId, - txParams: { recipients: [] }, - }) - .should.be.rejectedWith( - 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' - ); - }); - - it('accepts signTxRequest when recipients are only in intent (smart contract interaction)', async function () { - const txRequestWithIntentRecipients = { - ...txRequest, - intent: { - intentType: 'contractCall', - recipients: [ - { - address: { address: '0xrecipient' }, - amount: { value: '1000', symbol: 'hteth' }, - }, - ], - }, - }; - const nockPromises = [ - await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequestWithIntentRecipients, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequestWithIntentRecipients, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundThree(txRequestWithIntentRecipients), - await nockSendTxRequest(txRequestWithIntentRecipients), - ]; - await Promise.all(nockPromises); - - const userShare = fs.readFileSync(shareFiles[vector.party1]); - const userPrvBase64 = Buffer.from(userShare).toString('base64'); - // Falls back to intent.recipients — guard should pass and signing should complete - await tssUtils.signTxRequest({ - txRequest: txRequestWithIntentRecipients, - prv: userPrvBase64, - reqId, - // no txParams.recipients - }); - nockPromises[0].isDone().should.be.true(); - nockPromises[1].isDone().should.be.true(); - nockPromises[2].isDone().should.be.true(); - }); - - it('accepts signTxRequest for no-recipient tx types (tokenApproval)', async function () { - const nockPromises = [ - await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequest, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequest, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundThree(txRequest), - await nockSendTxRequest(txRequest), - ]; - await Promise.all(nockPromises); - - const userShare = fs.readFileSync(shareFiles[vector.party1]); - const userPrvBase64 = Buffer.from(userShare).toString('base64'); - // Type exemption applies — guard passes without recipients - await tssUtils.signTxRequest({ - txRequest, - prv: userPrvBase64, - reqId, - txParams: { type: 'tokenApproval' }, - }); - nockPromises[0].isDone().should.be.true(); - nockPromises[1].isDone().should.be.true(); - nockPromises[2].isDone().should.be.true(); - }); - - it('accepts signTxRequest for no-recipient tx types (acceleration)', async function () { - const nockPromises = [ - await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequest, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequest, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundThree(txRequest), - await nockSendTxRequest(txRequest), - ]; - await Promise.all(nockPromises); - - const userShare = fs.readFileSync(shareFiles[vector.party1]); - const userPrvBase64 = Buffer.from(userShare).toString('base64'); - await tssUtils.signTxRequest({ - txRequest, - prv: userPrvBase64, - reqId, - txParams: { type: 'acceleration' }, - }); - nockPromises[0].isDone().should.be.true(); - nockPromises[1].isDone().should.be.true(); - nockPromises[2].isDone().should.be.true(); - }); - - it('accepts signTxRequest for no-recipient tx types (customTx)', async function () { - const nockPromises = [ - await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequest, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequest, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundThree(txRequest), - await nockSendTxRequest(txRequest), - ]; - await Promise.all(nockPromises); - - const userShare = fs.readFileSync(shareFiles[vector.party1]); - const userPrvBase64 = Buffer.from(userShare).toString('base64'); - // DeFi/WalletConnect smart contract interactions have no traditional recipients - await tssUtils.signTxRequest({ - txRequest, - prv: userPrvBase64, - reqId, - txParams: { type: 'customTx' }, - }); - nockPromises[0].isDone().should.be.true(); - nockPromises[1].isDone().should.be.true(); - nockPromises[2].isDone().should.be.true(); - }); - - it('accepts signTxRequest for no-recipient tx types (enableToken)', async function () { - const nockPromises = [ - await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequest, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequest, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundThree(txRequest), - await nockSendTxRequest(txRequest), - ]; - await Promise.all(nockPromises); - - const userShare = fs.readFileSync(shareFiles[vector.party1]); - const userPrvBase64 = Buffer.from(userShare).toString('base64'); - // TSS wallets do not populate recipients for token enablement — exemption must apply - await tssUtils.signTxRequest({ - txRequest, - prv: userPrvBase64, - reqId, - txParams: { type: 'enableToken' }, - }); - nockPromises[0].isDone().should.be.true(); - nockPromises[1].isDone().should.be.true(); - nockPromises[2].isDone().should.be.true(); - }); - - it('accepts signTxRequest when txParams.recipients takes priority over intent.recipients', async function () { - const txRequestWithBothRecipients = { - ...txRequest, - intent: { - intentType: 'contractCall', - recipients: [{ address: { address: '0xintentRecipient' }, amount: { value: '9999', symbol: 'hteth' } }], - }, - }; - const nockPromises = [ - await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequestWithBothRecipients, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequestWithBothRecipients, bitgoGpgKey), - await nockTxRequestResponseSignatureShareRoundThree(txRequestWithBothRecipients), - await nockSendTxRequest(txRequestWithBothRecipients), - ]; - await Promise.all(nockPromises); - - const userShare = fs.readFileSync(shareFiles[vector.party1]); - const userPrvBase64 = Buffer.from(userShare).toString('base64'); - // txParams.recipients takes priority — guard passes and signing completes - await tssUtils.signTxRequest({ - txRequest: txRequestWithBothRecipients, - prv: userPrvBase64, - reqId, - txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, - }); - nockPromises[0].isDone().should.be.true(); - nockPromises[1].isDone().should.be.true(); - nockPromises[2].isDone().should.be.true(); - }); }); export function getBitGoPartyGpgKeyPrv(key: openpgp.SerializedKeyPair): DklsTypes.PartyGpgKey { diff --git a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts index 510539e4a2..5ca48252d4 100644 --- a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts +++ b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts @@ -250,9 +250,7 @@ export class PendingApproval implements IPendingApproval { } const decryptedPrv = await this.wallet.getPrv({ walletPassphrase }); - const pendingApprovalRecipients = this._pendingApproval.info?.transactionRequest?.recipients; - const txParams = pendingApprovalRecipients?.length ? { recipients: pendingApprovalRecipients } : undefined; - const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId, txParams); + const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId); if (txRequest.apiVersion === 'lite') { if (!txRequest.unsignedTxs || txRequest.unsignedTxs.length === 0) { throw new Error('Unexpected error, no transactions found in txRequest.'); diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts index ab9a0bf8da..e4dfebe65c 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts @@ -39,7 +39,6 @@ import { TxRequest, TxRequestVersion, } from './baseTypes'; -import { TransactionParams } from '../../baseCoin/iBaseCoin'; import { GShare, SignShare } from '../../../account-lib/mpc/tss'; import { RequestTracer } from '../util'; import { envRequiresBitgoPubGpgKeyConfig, getBitgoMpcGpgPubKey } from '../../tss/bitgoPubKeys'; @@ -539,16 +538,11 @@ export default class BaseTssUtils extends MpcUtils implements ITssUtil * @param {RequestTracer} reqId id tracer. * @returns {Promise} */ - async recreateTxRequest( - txRequestId: string, - decryptedPrv: string, - reqId: IRequestTracer, - txParams?: TransactionParams - ): Promise { + async recreateTxRequest(txRequestId: string, decryptedPrv: string, reqId: IRequestTracer): Promise { await this.deleteSignatureShares(txRequestId, reqId); // after delete signatures shares get the tx without them const txRequest = await getTxRequest(this.bitgo, this.wallet.id(), txRequestId, reqId); - return await this.signTxRequest({ txRequest, prv: decryptedPrv, reqId, txParams }); + return await this.signTxRequest({ txRequest, prv: decryptedPrv, reqId }); } /** diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts index 4b173123b0..f773e370a7 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts @@ -760,12 +760,7 @@ export interface ITssUtils { deleteSignatureShares(txRequestId: string): Promise; // eslint-disable-next-line @typescript-eslint/no-explicit-any sendTxRequest(txRequestId: string): Promise; - recreateTxRequest( - txRequestId: string, - decryptedPrv: string, - reqId: IRequestTracer, - txParams?: TransactionParams - ): Promise; + recreateTxRequest(txRequestId: string, decryptedPrv: string, reqId: IRequestTracer): Promise; getTxRequest(txRequestId: string): Promise; supportedTxRequestVersions(): TxRequestVersion[]; } diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts index 163b97d66c..41391d7232 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts @@ -46,7 +46,6 @@ import { TxRequestChallengeResponse, } from '../../../tss/types'; import { BaseEcdsaUtils } from './base'; -import { resolveEffectiveTxParams } from '../recipientUtils'; import { IRequestTracer } from '../../../../api'; const encryptNShare = ECDSAMethods.encryptNShare; @@ -746,8 +745,6 @@ export class EcdsaUtils extends BaseEcdsaUtils { const unsignedTx = txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0]; - const effectiveTxParams = resolveEffectiveTxParams(txRequest, params.txParams); - // For ICP transactions, the HSM signs the serializedTxHex, while the user signs the signableHex separately. // Verification cannot be performed directly on the signableHex alone. However, we can parse the serializedTxHex // to regenerate the signableHex and compare it against the provided value for verification. @@ -755,14 +752,14 @@ export class EcdsaUtils extends BaseEcdsaUtils { if (this.baseCoin.getConfig().family === 'icp') { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex }, - txParams: effectiveTxParams, + txParams: params.txParams || { recipients: [] }, wallet: this.wallet, walletType: this.wallet.multisigType(), }); } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex }, - txParams: effectiveTxParams, + txParams: params.txParams || { recipients: [] }, wallet: this.wallet, walletType: this.wallet.multisigType(), }); diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts index f5dbe75eeb..6c36191979 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts @@ -47,7 +47,6 @@ import { TxRequest, } from '../baseTypes'; import { BaseEcdsaUtils } from './base'; -import { resolveEffectiveTxParams } from '../recipientUtils'; import { EcdsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './ecdsaMPCv2KeyGenSender'; import { envRequiresBitgoPubGpgKeyConfig, isBitgoMpcPubKey } from '../../../tss/bitgoPubKeys'; @@ -742,8 +741,6 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { const unsignedTx = txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0]; - const effectiveTxParams = resolveEffectiveTxParams(txRequest, params.txParams); - // For ICP transactions, the HSM signs the serializedTxHex, while the user signs the signableHex separately. // Verification cannot be performed directly on the signableHex alone. However, we can parse the serializedTxHex // to regenerate the signableHex and compare it against the provided value for verification. @@ -751,14 +748,14 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { if (this.baseCoin.getConfig().family === 'icp') { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex }, - txParams: effectiveTxParams, + txParams: params.txParams || { recipients: [] }, wallet: this.wallet, walletType: this.wallet.multisigType(), }); } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex }, - txParams: effectiveTxParams, + txParams: params.txParams || { recipients: [] }, wallet: this.wallet, walletType: this.wallet.multisigType(), }); diff --git a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts deleted file mode 100644 index 682228c4b0..0000000000 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { TransactionParams } from '../../baseCoin'; -import { InvalidTransactionError } from '../../errors'; -import { PopulatedIntent, TxRequest } from './baseTypes'; - -/** - * Transaction types that legitimately carry no explicit recipients. - * verifyTransaction handles no-recipient validation for these internally. - * Mirrors the bypass list in abstractEthLikeNewCoins.ts verifyTssTransaction. - */ -export const NO_RECIPIENT_TX_TYPES = new Set([ - 'acceleration', - 'fillNonce', - 'transferToken', - 'tokenApproval', - 'consolidate', - 'bridgeFunds', - 'enableToken', - 'customTx', // DeFi/WalletConnect smart contract interactions have no traditional recipients -]); - -/** - * Resolves the effective txParams for TSS signing recipient verification. - * - * For smart contract interactions, recipients live in txRequest.intent.recipients - * (native amount = 0, so buildParams is empty). Falls back to intent recipients - * mapped to ITransactionRecipient shape when txParams.recipients is absent. - * - * Throws InvalidTransactionError if no recipients can be resolved and the - * transaction type is not a known no-recipient type. - */ -export function resolveEffectiveTxParams( - txRequest: TxRequest, - txParams: TransactionParams | undefined -): TransactionParams { - const intentRecipients = (txRequest.intent as PopulatedIntent)?.recipients?.map((intentRecipient) => ({ - address: intentRecipient.address.address, - amount: intentRecipient.amount.value, - data: intentRecipient.data, - })); - - const effectiveTxParams: TransactionParams = { - ...txParams, - recipients: txParams?.recipients?.length ? txParams.recipients : intentRecipients, - }; - - if (!effectiveTxParams.recipients?.length && !NO_RECIPIENT_TX_TYPES.has(effectiveTxParams.type ?? '')) { - throw new InvalidTransactionError( - 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' - ); - } - - return effectiveTxParams; -} diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts deleted file mode 100644 index 9ff4ad37c8..0000000000 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts +++ /dev/null @@ -1,121 +0,0 @@ -import 'should'; -import * as assert from 'assert'; - -// Import directly from source so no top-level export change is needed -const getModule = () => require('../../../../../src/bitgo/utils/tss/recipientUtils'); - -function makeTxRequest( - intentRecipients?: { address: { address: string }; amount: { value: string }; data?: string }[] -): any { - return { - txRequestId: 'test-req-id', - intent: intentRecipients ? { intentType: 'contractCall', recipients: intentRecipients } : { intentType: 'payment' }, - transactions: [], - unsignedTxs: [], - state: 'pendingUserSignature', - walletType: 'hot', - walletId: 'walletId', - policiesChecked: true, - version: 1, - userId: 'userId', - }; -} - -describe('recipientUtils', function () { - describe('NO_RECIPIENT_TX_TYPES', function () { - it('contains exactly the 8 expected exempted types', function () { - const { NO_RECIPIENT_TX_TYPES } = getModule(); - const expected = [ - 'acceleration', - 'fillNonce', - 'transferToken', - 'tokenApproval', - 'consolidate', - 'bridgeFunds', - 'enableToken', - 'customTx', - ]; - expected.forEach((t) => assert.ok(NO_RECIPIENT_TX_TYPES.has(t), `${t} should be in NO_RECIPIENT_TX_TYPES`)); - assert.strictEqual(NO_RECIPIENT_TX_TYPES.size, expected.length); - }); - }); - - describe('resolveEffectiveTxParams', function () { - it('uses txParams.recipients when provided', function () { - const { resolveEffectiveTxParams } = getModule(); - const txRequest = makeTxRequest(); - const result = resolveEffectiveTxParams(txRequest, { recipients: [{ address: '0xabc', amount: '100' }] }); - result.recipients[0].address.should.equal('0xabc'); - result.recipients[0].amount.should.equal('100'); - }); - - it('falls back to intent.recipients when txParams has no recipients', function () { - const { resolveEffectiveTxParams } = getModule(); - const txRequest = makeTxRequest([{ address: { address: '0xintent' }, amount: { value: '500' } }]); - const result = resolveEffectiveTxParams(txRequest, {}); - result.recipients[0].address.should.equal('0xintent'); - result.recipients[0].amount.should.equal('500'); - }); - - it('falls back to intent.recipients when txParams is undefined', function () { - const { resolveEffectiveTxParams } = getModule(); - const txRequest = makeTxRequest([{ address: { address: '0xintent' }, amount: { value: '999' } }]); - const result = resolveEffectiveTxParams(txRequest, undefined); - result.recipients[0].address.should.equal('0xintent'); - }); - - it('prefers txParams.recipients over intent.recipients when both present', function () { - const { resolveEffectiveTxParams } = getModule(); - const txRequest = makeTxRequest([{ address: { address: '0xintent' }, amount: { value: '9999' } }]); - const result = resolveEffectiveTxParams(txRequest, { recipients: [{ address: '0xexplicit', amount: '1' }] }); - result.recipients[0].address.should.equal('0xexplicit'); - }); - - it('maps intent.data field through to the recipient', function () { - const { resolveEffectiveTxParams } = getModule(); - const txRequest = makeTxRequest([ - { address: { address: '0xcontract' }, amount: { value: '0' }, data: '0xabcdef' }, - ]); - const result = resolveEffectiveTxParams(txRequest, undefined); - result.recipients[0].data.should.equal('0xabcdef'); - }); - - it('throws InvalidTransactionError when no recipients and type is not exempted', function () { - const { resolveEffectiveTxParams } = getModule(); - const txRequest = makeTxRequest(); - assert.throws( - () => resolveEffectiveTxParams(txRequest, {}), - /Recipient details are required to verify this transaction before signing/ - ); - }); - - it('throws when txParams is undefined and intent has no recipients', function () { - const { resolveEffectiveTxParams } = getModule(); - const txRequest = makeTxRequest(); - assert.throws( - () => resolveEffectiveTxParams(txRequest, undefined), - /Recipient details are required to verify this transaction before signing/ - ); - }); - - const NO_RECIPIENT_TYPES = [ - 'acceleration', - 'fillNonce', - 'transferToken', - 'tokenApproval', - 'consolidate', - 'bridgeFunds', - 'enableToken', // TSS wallets do not populate recipients for token enablement - 'customTx', // DeFi/WalletConnect smart contract interactions have no traditional recipients - ]; - - NO_RECIPIENT_TYPES.forEach((type) => { - it(`allows empty recipients for no-recipient tx type: ${type}`, function () { - const { resolveEffectiveTxParams } = getModule(); - const txRequest = makeTxRequest(); - const result = resolveEffectiveTxParams(txRequest, { type }); - result.type.should.equal(type); - }); - }); - }); -});