From 31cbea7fe2aa0a12279147b410dc748d322aa4a9 Mon Sep 17 00:00:00 2001 From: mrdanish26 Date: Thu, 16 Apr 2026 17:13:00 -0700 Subject: [PATCH 1/3] fix(sdk-core): verify recipients before signing to prevent signableHex substitution TICKET: WAL-375 --- .../test/v2/unit/internal/tssUtils/ecdsa.ts | 114 +++++++++++++++++ .../tssUtils/ecdsaMPCv2/signTxRequest.ts | 115 ++++++++++++++++++ .../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/base.ts | 53 +++++++- .../src/bitgo/utils/tss/ecdsa/ecdsa.ts | 8 +- .../src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts | 8 +- 8 files changed, 307 insertions(+), 12 deletions(-) diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts index 9786e02c2a..123430f937 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts @@ -749,6 +749,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; @@ -766,12 +767,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 5ca48252d4..510539e4a2 100644 --- a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts +++ b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts @@ -250,7 +250,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 e4dfebe65c..ab9a0bf8da 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'; @@ -538,11 +539,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 f773e370a7..4b173123b0 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts @@ -760,7 +760,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 41391d7232..614ee75f3f 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 129877a4af..eaeedd5f75 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'; @@ -737,6 +737,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. @@ -744,14 +746,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(), }); From 1f553991806987d0c2a568dcd046b40e3536df93 Mon Sep 17 00:00:00 2001 From: mrdanish26 Date: Wed, 22 Apr 2026 16:56:00 -0700 Subject: [PATCH 2/3] fix(sdk-core): verify recipients before signing (ECDSA) TICKET: WAL-375 --- .../src/abstractEthLikeNewCoins.ts | 13 +- .../tssUtils/ecdsaMPCv2/signTxRequest.ts | 165 +++++++++++++----- .../src/bitgo/utils/tss/ecdsa/base.ts | 53 +----- .../src/bitgo/utils/tss/ecdsa/ecdsa.ts | 3 +- .../src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts | 3 +- .../src/bitgo/utils/tss/recipientUtils.ts | 53 ++++++ .../unit/bitgo/utils/tss/recipientUtils.ts | 121 +++++++++++++ 7 files changed, 310 insertions(+), 101 deletions(-) create mode 100644 modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts create 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 7f830bd1a9..80c6b623b7 100644 --- a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts +++ b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts @@ -3109,9 +3109,16 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin { txParams.prebuildTx?.consolidateId || txPrebuild?.consolidateId || (txParams.type && - ['acceleration', 'fillNonce', 'transferToken', 'tokenApproval', 'consolidate', 'bridgeFunds'].includes( - txParams.type - )) + [ + 'acceleration', + 'fillNonce', + 'transferToken', + 'tokenApproval', + 'consolidate', + 'bridgeFunds', + 'enableToken', + 'customTx', + ].includes(txParams.type)) ) ) { throw new Error('missing txParams'); 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 de5f65516e..739abb3506 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts @@ -318,8 +318,6 @@ describe('signTxRequest:', function () { }); 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: { @@ -332,53 +330,120 @@ describe('signTxRequest:', function () { ], }, }; - // 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.' - ); + 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'); - // 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.' - ); + // 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' }, - }) - .should.not.be.rejectedWith( - 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' - ); + 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 when txParams.recipients takes priority over intent.recipients', async function () { + 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: { @@ -386,16 +451,26 @@ describe('signTxRequest:', function () { 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.' - ); + 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(); }); }); 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 46b9fcd6c3..d6512327fb 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/base.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/base.ts @@ -1,61 +1,12 @@ import { secp256k1 } from '@noble/curves/secp256k1'; -import { IBaseCoin, TransactionParams } from '../../../baseCoin'; +import { IBaseCoin } from '../../../baseCoin'; import baseTSSUtils from '../baseTSSUtils'; import { KeyShare } from './types'; -import { BackupGpgKey, PopulatedIntent, TxRequest } from '../baseTypes'; +import { BackupGpgKey } 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 614ee75f3f..163b97d66c 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,8 @@ import { TssEcdsaStep2ReturnMessage, TxRequestChallengeResponse, } from '../../../tss/types'; -import { BaseEcdsaUtils, resolveEffectiveTxParams } from './base'; +import { BaseEcdsaUtils } from './base'; +import { resolveEffectiveTxParams } from '../recipientUtils'; import { IRequestTracer } from '../../../../api'; const encryptNShare = ECDSAMethods.encryptNShare; 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 eaeedd5f75..e76eee3e09 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,8 @@ import { TSSParamsWithPrv, TxRequest, } from '../baseTypes'; -import { BaseEcdsaUtils, resolveEffectiveTxParams } from './base'; +import { BaseEcdsaUtils } from './base'; +import { resolveEffectiveTxParams } from '../recipientUtils'; import { EcdsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './ecdsaMPCv2KeyGenSender'; import { envRequiresBitgoPubGpgKeyConfig, isBitgoMpcPubKey } from '../../../tss/bitgoPubKeys'; diff --git a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts new file mode 100644 index 0000000000..682228c4b0 --- /dev/null +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -0,0 +1,53 @@ +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 new file mode 100644 index 0000000000..9ff4ad37c8 --- /dev/null +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts @@ -0,0 +1,121 @@ +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); + }); + }); + }); +}); From 5ccc429554300eb8632eb10709077e0bf8c25301 Mon Sep 17 00:00:00 2001 From: mrdanish26 Date: Fri, 24 Apr 2026 12:30:31 -0700 Subject: [PATCH 3/3] fix(sdk-core): verify recipients before signing (EdDSA) Add resolveEffectiveTxParams guard to EdDSA signRequestBase() mirroring the ECDSA fix from WAL-375. Expands NO_RECIPIENT_TX_TYPES with ~36 EdDSA coin types (staking, wallet init, token ops, CANTON flows) that legitimately carry no recipients. Guard also falls back to intent.intentType when txParams.type is unset, covering EdDSA coins that populate intent but not txParams. TICKET: WCN-196 --- .../test/v2/unit/internal/tssUtils/eddsa.ts | 56 +++++++++++++++-- .../src/bitgo/utils/tss/eddsa/eddsa.ts | 9 +++ .../src/bitgo/utils/tss/recipientUtils.ts | 63 ++++++++++++++++++- .../unit/bitgo/utils/tss/recipientUtils.ts | 51 +++++++++++++-- 4 files changed, 165 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 ca077e9cd2..7b6b4dd1b9 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts @@ -612,9 +612,11 @@ describe('TSS Utils:', async function () { }); it('signTxRequest should succeed with txRequest object as input', async function () { + sandbox.stub(baseCoin, 'verifyTransaction').resolves(); const signedTxRequest = await tssUtils.signTxRequest({ txRequest, prv: JSON.stringify(validUserSigningMaterial), + txParams: { recipients: [{ address: '5f8f5a1d7f', amount: '10000' }] }, reqId, }); signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); @@ -623,19 +625,57 @@ describe('TSS Utils:', async function () { }); it('signTxRequest should succeed with txRequest id as input', async function () { - const getTxRequest = sandbox.stub(tssUtils, 'getTxRequest'); - getTxRequest.resolves(txRequest); - getTxRequest.calledWith(txRequestId); + sandbox.stub(baseCoin, 'verifyTransaction').resolves(); + const getTxRequestStub = sandbox.stub(tssUtils, 'getTxRequest'); + getTxRequestStub.resolves(txRequest); + getTxRequestStub.calledWith(txRequestId); const signedTxRequest = await tssUtils.signTxRequest({ txRequest: txRequestId, prv: JSON.stringify(validUserSigningMaterial), + txParams: { recipients: [{ address: '5f8f5a1d7f', amount: '10000' }] }, reqId, }); signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); sandbox.verifyAndRestore(); }); + + it('signTxRequest should reject when txParams.recipients is missing for payment', async function () { + await tssUtils + .signTxRequest({ + txRequest, + prv: JSON.stringify(validUserSigningMaterial), + reqId, + }) + .should.be.rejectedWith( + 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' + ); + }); + + it('signTxRequest should succeed for stakingActivate without recipients', async function () { + const stakingTxRequest = { ...txRequest, intent: { intentType: 'stakingActivate' } }; + sandbox.stub(baseCoin, 'verifyTransaction').resolves(); + const signedTxRequest = await tssUtils.signTxRequest({ + txRequest: stakingTxRequest, + prv: JSON.stringify(validUserSigningMaterial), + reqId, + }); + signedTxRequest.unsignedTxs.should.deepEqual(stakingTxRequest.unsignedTxs); + sandbox.verifyAndRestore(); + }); + + it('signTxRequest should succeed for walletInitialization without recipients', async function () { + const walletInitTxRequest = { ...txRequest, intent: { intentType: 'walletInitialization' } }; + sandbox.stub(baseCoin, 'verifyTransaction').resolves(); + const signedTxRequest = await tssUtils.signTxRequest({ + txRequest: walletInitTxRequest, + prv: JSON.stringify(validUserSigningMaterial), + reqId, + }); + signedTxRequest.unsignedTxs.should.deepEqual(walletInitTxRequest.unsignedTxs); + sandbox.verifyAndRestore(); + }); }); describe('signTxRequest With Commitment:', function () { @@ -700,9 +740,11 @@ describe('TSS Utils:', async function () { }); it('signTxRequest should succeed with txRequest object as input', async function () { + sandbox.stub(baseCoin, 'verifyTransaction').resolves(); const signedTxRequest = await tssUtils.signTxRequest({ txRequest, prv: JSON.stringify(validUserSigningMaterial), + txParams: { recipients: [{ address: '5f8f5a1d7f', amount: '10000' }] }, reqId, }); signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); @@ -711,13 +753,15 @@ describe('TSS Utils:', async function () { }); it('signTxRequest should succeed with txRequest id as input', async function () { - const getTxRequest = sandbox.stub(tssUtils, 'getTxRequest'); - getTxRequest.resolves(txRequest); - getTxRequest.calledWith(txRequestId); + sandbox.stub(baseCoin, 'verifyTransaction').resolves(); + const getTxRequestStub = sandbox.stub(tssUtils, 'getTxRequest'); + getTxRequestStub.resolves(txRequest); + getTxRequestStub.calledWith(txRequestId); const signedTxRequest = await tssUtils.signTxRequest({ txRequest: txRequestId, prv: JSON.stringify(validUserSigningMaterial), + txParams: { recipients: [{ address: '5f8f5a1d7f', amount: '10000' }] }, reqId, }); signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs); 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 de4790dfd5..7c1295dda7 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts @@ -43,6 +43,7 @@ import { IRequestTracer } from '../../../../api'; import { getBitgoMpcGpgPubKey } from '../../../tss/bitgoPubKeys'; import { EnvironmentName } from '../../../environments'; import { readKey } from 'openpgp'; +import { resolveEffectiveTxParams } from '../recipientUtils'; /** * Utility functions for TSS work flows. @@ -610,6 +611,14 @@ export class EddsaUtils extends baseTSSUtils { ); unsignedTx = apiVersion === 'full' ? txRequestResolved.transactions![0].unsignedTx : txRequestResolved.unsignedTxs[0]; + + const effectiveTxParams = resolveEffectiveTxParams(txRequestResolved, params.txParams); + await this.baseCoin.verifyTransaction({ + txPrebuild: { txHex: unsignedTx.signableHex }, + txParams: effectiveTxParams, + wallet: this.wallet, + walletType: this.wallet.multisigType(), + }); } else if (requestType === RequestType.message) { assert(txRequestResolved.messages?.length, 'Unable to find messages in txRequest for message signing'); const message = txRequestResolved.messages[0]; diff --git a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts index 682228c4b0..b470eb7670 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -6,8 +6,13 @@ 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. + * + * ECDSA types: acceleration, fillNonce, transferToken, tokenApproval, consolidate, + * bridgeFunds, enableToken, customTx + * EdDSA types: staking operations, wallet/account init, token ops, CANTON 2-step flows */ export const NO_RECIPIENT_TX_TYPES = new Set([ + // ECDSA types 'acceleration', 'fillNonce', 'transferToken', @@ -15,7 +20,57 @@ export const NO_RECIPIENT_TX_TYPES = new Set([ 'consolidate', 'bridgeFunds', 'enableToken', - 'customTx', // DeFi/WalletConnect smart contract interactions have no traditional recipients + 'customTx', + + // EdDSA staking (SOL, ADA, NEAR, DOT, CSPR, SUI, APT) + 'stakingActivate', + 'stakingDeactivate', + 'stakingWithdraw', + 'stakingClaim', + 'stakingDelegate', + 'stakingUnlock', + 'stakingUnvote', + 'stakingPledge', + 'stakingAuthorize', + 'stakingAuthorizeRaw', + 'stakingLock', // CSPR + 'stakingAdd', // SUI + 'addStake', // SUI + 'withdrawStake', // SUI + + // EdDSA account / wallet initialization + 'walletInitialization', + 'addressInitialization', // DOT + 'createAccount', // SOL + CANTON + 'accountUpdate', // HBAR + + // EdDSA token operations + 'trustline', // XLM + 'closeAssociatedTokenAccount', // SOL, HBAR + 'associatedTokenAccountInitialization', // SOL, HBAR + 'tokenTransfer', // SUI + 'sendToken', // TON + 'sendNFT', // APT + + // NEAR + 'storageDeposit', + + // TON + 'singleNominatorWithdraw', + 'tonWhalesDeposit', + 'tonWhalesWithdrawal', + 'tonWhalesVestingDeposit', + 'tonWhalesVestingWithdrawal', + + // ADA + 'voteDelegation', + + // CANTON 2-step transfer flows + 'transferAccept', // already in populateIntent() exempt list + 'transferReject', // already in populateIntent() exempt list + 'transferAcknowledge', + 'transferOfferWithdrawn', // already in populateIntent() exempt list + 'oneStepPreApproval', // CANTON enableToken ]); /** @@ -43,7 +98,11 @@ export function resolveEffectiveTxParams( recipients: txParams?.recipients?.length ? txParams.recipients : intentRecipients, }; - if (!effectiveTxParams.recipients?.length && !NO_RECIPIENT_TX_TYPES.has(effectiveTxParams.type ?? '')) { + // Fall back to intent.intentType when txParams.type is not explicitly set. + // This covers EdDSA coins where the wallet SDK populates intent but not txParams.type. + const txType = effectiveTxParams.type ?? (txRequest.intent as PopulatedIntent)?.intentType ?? ''; + + if (!effectiveTxParams.recipients?.length && !NO_RECIPIENT_TX_TYPES.has(txType)) { throw new InvalidTransactionError( 'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.' ); diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts index 9ff4ad37c8..76fb7b7bb1 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts @@ -23,9 +23,9 @@ function makeTxRequest( describe('recipientUtils', function () { describe('NO_RECIPIENT_TX_TYPES', function () { - it('contains exactly the 8 expected exempted types', function () { + it('contains all ECDSA exempted types', function () { const { NO_RECIPIENT_TX_TYPES } = getModule(); - const expected = [ + const ecdsaTypes = [ 'acceleration', 'fillNonce', 'transferToken', @@ -35,8 +35,32 @@ describe('recipientUtils', function () { '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); + ecdsaTypes.forEach((t) => assert.ok(NO_RECIPIENT_TX_TYPES.has(t), `${t} should be in NO_RECIPIENT_TX_TYPES`)); + }); + + it('contains EdDSA staking types', function () { + const { NO_RECIPIENT_TX_TYPES } = getModule(); + const stakingTypes = [ + 'stakingActivate', + 'stakingDeactivate', + 'stakingWithdraw', + 'stakingClaim', + 'stakingDelegate', + 'walletInitialization', + ]; + stakingTypes.forEach((t) => assert.ok(NO_RECIPIENT_TX_TYPES.has(t), `${t} should be in NO_RECIPIENT_TX_TYPES`)); + }); + + it('contains CANTON transfer flow types', function () { + const { NO_RECIPIENT_TX_TYPES } = getModule(); + const cantonTypes = [ + 'transferAccept', + 'transferReject', + 'transferAcknowledge', + 'transferOfferWithdrawn', + 'oneStepPreApproval', + ]; + cantonTypes.forEach((t) => assert.ok(NO_RECIPIENT_TX_TYPES.has(t), `${t} should be in NO_RECIPIENT_TX_TYPES`)); }); }); @@ -98,7 +122,22 @@ describe('recipientUtils', function () { ); }); - const NO_RECIPIENT_TYPES = [ + it('allows empty recipients when txParams.type is a no-recipient type', function () { + const { resolveEffectiveTxParams } = getModule(); + const txRequest = makeTxRequest(); + const result = resolveEffectiveTxParams(txRequest, { type: 'stakingActivate' }); + result.type.should.equal('stakingActivate'); + }); + + it('allows empty recipients when intent.intentType is a no-recipient type (EdDSA fallback)', function () { + const { resolveEffectiveTxParams } = getModule(); + const txRequest = { ...makeTxRequest(), intent: { intentType: 'walletInitialization' } }; + // No txParams.type — guard must fall back to intent.intentType + const result = resolveEffectiveTxParams(txRequest, undefined); + assert.ok(!result.recipients?.length, 'No recipients expected'); + }); + + const ECDSA_NO_RECIPIENT_TYPES = [ 'acceleration', 'fillNonce', 'transferToken', @@ -109,7 +148,7 @@ describe('recipientUtils', function () { 'customTx', // DeFi/WalletConnect smart contract interactions have no traditional recipients ]; - NO_RECIPIENT_TYPES.forEach((type) => { + ECDSA_NO_RECIPIENT_TYPES.forEach((type) => { it(`allows empty recipients for no-recipient tx type: ${type}`, function () { const { resolveEffectiveTxParams } = getModule(); const txRequest = makeTxRequest();