diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts index 6e9574b491..c898bc261a 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts @@ -747,6 +747,7 @@ 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; @@ -764,12 +765,46 @@ 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 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 9b2a43be4e..5442f4d3b7 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts @@ -193,6 +193,7 @@ 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(); @@ -215,6 +216,7 @@ 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(); @@ -236,6 +238,7 @@ 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(); @@ -257,6 +260,7 @@ 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(); @@ -277,11 +281,41 @@ 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.' + ); + }); }); 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 ebf7e898ff..9de0f72079 100644 --- a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts +++ b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts @@ -246,7 +246,9 @@ export class PendingApproval implements IPendingApproval { } const decryptedPrv = await this.wallet.getPrv({ walletPassphrase }); - const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId); + const pendingApprovalRecipients = this._pendingApproval.info?.transactionRequest?.recipients; + const txParams = pendingApprovalRecipients?.length ? { recipients: pendingApprovalRecipients } : undefined; + const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId, txParams); 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 de7fd8bfdd..ca9d96a791 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts @@ -39,6 +39,7 @@ 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'; @@ -533,11 +534,16 @@ export default class BaseTssUtils extends MpcUtils implements ITssUtil * @param {RequestTracer} reqId id tracer. * @returns {Promise} */ - async recreateTxRequest(txRequestId: string, decryptedPrv: string, reqId: IRequestTracer): Promise { + async recreateTxRequest( + txRequestId: string, + decryptedPrv: string, + reqId: IRequestTracer, + txParams?: TransactionParams + ): 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 }); + return await this.signTxRequest({ txRequest, prv: decryptedPrv, reqId, txParams }); } /** diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts index a740729846..5dfd22e728 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts @@ -748,7 +748,12 @@ 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): Promise; + recreateTxRequest( + txRequestId: string, + decryptedPrv: string, + reqId: IRequestTracer, + txParams?: TransactionParams + ): 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 32e185d3b8..1e45de5ebc 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts @@ -47,6 +47,7 @@ import { } from '../../../tss/types'; import { BaseEcdsaUtils } from './base'; import { IRequestTracer } from '../../../../api'; +import { InvalidTransactionError } from '../../../errors'; const encryptNShare = ECDSAMethods.encryptNShare; @@ -745,6 +746,12 @@ export class EcdsaUtils extends BaseEcdsaUtils { const unsignedTx = txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0]; + if (!params.txParams?.recipients?.length) { + throw new InvalidTransactionError( + 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' + ); + } + // 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. @@ -752,14 +759,14 @@ export class EcdsaUtils extends BaseEcdsaUtils { if (this.baseCoin.getConfig().family === 'icp') { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex }, - txParams: params.txParams || { recipients: [] }, + txParams: params.txParams, wallet: this.wallet, walletType: this.wallet.multisigType(), }); } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex }, - txParams: params.txParams || { recipients: [] }, + txParams: params.txParams, 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 be1dff5b51..d0984135f4 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts @@ -32,6 +32,7 @@ import { verifyBitGoMessagesAndSignaturesRoundOne, verifyBitGoMessagesAndSignaturesRoundTwo, } from '../../../tss/ecdsa/ecdsaMPCv2'; +import { InvalidTransactionError } from '../../../errors'; import { KeyCombined } from '../../../tss/ecdsa/types'; import { generateGPGKeyPair } from '../../opengpgUtils'; import { @@ -736,6 +737,12 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { const unsignedTx = txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0]; + if (!params.txParams?.recipients?.length) { + throw new InvalidTransactionError( + 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' + ); + } + // 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. @@ -743,14 +750,14 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { if (this.baseCoin.getConfig().family === 'icp') { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex }, - txParams: params.txParams || { recipients: [] }, + txParams: params.txParams, wallet: this.wallet, walletType: this.wallet.multisigType(), }); } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex }, - txParams: params.txParams || { recipients: [] }, + txParams: params.txParams, wallet: this.wallet, walletType: this.wallet.multisigType(), });