diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts index 6e9574b491..8995acb1c4 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,125 @@ 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 9b2a43be4e..de5f65516e 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,122 @@ 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 userShare = fs.readFileSync(shareFiles[vector.party1]); + const userPrvBase64 = Buffer.from(userShare).toString('base64'); + const txRequestWithIntentRecipients = { + ...txRequest, + intent: { + intentType: 'contractCall', + recipients: [ + { + address: { address: '0xrecipient' }, + amount: { value: '1000', symbol: 'hteth' }, + }, + ], + }, + }; + // Should not throw the recipients guard error — falls back to intent.recipients + await tssUtils + .signTxRequest({ + txRequest: txRequestWithIntentRecipients, + prv: userPrvBase64, + reqId, + // no txParams.recipients + }) + .should.not.be.rejectedWith( + 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' + ); + }); + + it('accepts signTxRequest for no-recipient tx types (tokenApproval)', async function () { + const userShare = fs.readFileSync(shareFiles[vector.party1]); + const userPrvBase64 = Buffer.from(userShare).toString('base64'); + // Should not throw the recipients guard error — type exemption applies + await tssUtils + .signTxRequest({ + txRequest, + prv: userPrvBase64, + reqId, + txParams: { type: 'tokenApproval' }, + }) + .should.not.be.rejectedWith( + 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' + ); + }); + + it('accepts signTxRequest for no-recipient tx types (acceleration)', async function () { + const userShare = fs.readFileSync(shareFiles[vector.party1]); + const userPrvBase64 = Buffer.from(userShare).toString('base64'); + await tssUtils + .signTxRequest({ + txRequest, + prv: userPrvBase64, + reqId, + txParams: { type: 'acceleration' }, + }) + .should.not.be.rejectedWith( + 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' + ); + }); + + it('accepts signTxRequest when txParams.recipients takes priority over intent.recipients', async function () { + const userShare = fs.readFileSync(shareFiles[vector.party1]); + const userPrvBase64 = Buffer.from(userShare).toString('base64'); + const txRequestWithBothRecipients = { + ...txRequest, + intent: { + intentType: 'contractCall', + recipients: [{ address: { address: '0xintentRecipient' }, amount: { value: '9999', symbol: 'hteth' } }], + }, + }; + await tssUtils + .signTxRequest({ + txRequest: txRequestWithBothRecipients, + prv: userPrvBase64, + reqId, + txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] }, + }) + .should.not.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 719ffd4507..d1dfb1b3d4 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts @@ -759,7 +759,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/base.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/base.ts index d6512327fb..46b9fcd6c3 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/base.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/base.ts @@ -1,12 +1,61 @@ import { secp256k1 } from '@noble/curves/secp256k1'; -import { IBaseCoin } from '../../../baseCoin'; +import { IBaseCoin, TransactionParams } from '../../../baseCoin'; import baseTSSUtils from '../baseTSSUtils'; import { KeyShare } from './types'; -import { BackupGpgKey } from '../baseTypes'; +import { BackupGpgKey, PopulatedIntent, TxRequest } from '../baseTypes'; import { generateGPGKeyPair } from '../../opengpgUtils'; import { BitGoBase } from '../../../bitgoBase'; import { IWallet } from '../../../wallet'; +import { InvalidTransactionError } from '../../../errors'; + +/** + * 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', +]); + +/** + * 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; +} /** @inheritdoc */ export class BaseEcdsaUtils extends baseTSSUtils { 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..965e539e9e 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts @@ -45,7 +45,7 @@ import { TssEcdsaStep2ReturnMessage, TxRequestChallengeResponse, } from '../../../tss/types'; -import { BaseEcdsaUtils } from './base'; +import { BaseEcdsaUtils, resolveEffectiveTxParams } from './base'; import { IRequestTracer } from '../../../../api'; const encryptNShare = ECDSAMethods.encryptNShare; @@ -745,6 +745,8 @@ 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. @@ -752,14 +754,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: effectiveTxParams, wallet: this.wallet, walletType: this.wallet.multisigType(), }); } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex }, - txParams: params.txParams || { recipients: [] }, + txParams: effectiveTxParams, 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 55f1d9b725..e06463a888 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts @@ -46,7 +46,7 @@ import { TSSParamsWithPrv, TxRequest, } from '../baseTypes'; -import { BaseEcdsaUtils } from './base'; +import { BaseEcdsaUtils, resolveEffectiveTxParams } from './base'; import { EcdsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './ecdsaMPCv2KeyGenSender'; import { envRequiresBitgoPubGpgKeyConfig, isBitgoMpcPubKey } from '../../../tss/bitgoPubKeys'; @@ -736,6 +736,8 @@ 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. @@ -743,14 +745,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: effectiveTxParams, wallet: this.wallet, walletType: this.wallet.multisigType(), }); } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex }, - txParams: params.txParams || { recipients: [] }, + txParams: effectiveTxParams, wallet: this.wallet, walletType: this.wallet.multisigType(), });