From f45a0dd6141fb465cb20275f8f60e9dd873c4938 Mon Sep 17 00:00:00 2001 From: mrdanish26 Date: Fri, 24 Apr 2026 12:30:31 -0700 Subject: [PATCH] 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 6100856062..f67a7ea1b6 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();