Skip to content

Commit ab5753e

Browse files
mrdanish26mrdanish26
committed
fix: require txParams with recipients for TSS tx signing
Ticket: WAL-375
1 parent e3b914b commit ab5753e

File tree

7 files changed

+104
-8
lines changed

7 files changed

+104
-8
lines changed

‎modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,7 @@ describe('TSS Ecdsa Utils:', async function () {
747747
backupNShare: backupKeyShare.nShares[1],
748748
}),
749749
reqId,
750+
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
750751
});
751752
signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs);
752753
const userGpgActual = sendShareSpy.getCalls()[0].args[10] as string;
@@ -764,12 +765,46 @@ describe('TSS Ecdsa Utils:', async function () {
764765
backupNShare: backupKeyShare.nShares[1],
765766
}),
766767
reqId,
768+
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
767769
});
768770
signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs);
769771
const userGpgActual = sendShareSpy.getCalls()[0].args[10] as string;
770772
userGpgActual.should.startWith('-----BEGIN PGP PUBLIC KEY BLOCK-----');
771773
});
772774

775+
it('signTxRequest should fail when txParams is missing', async function () {
776+
await tssUtils
777+
.signTxRequest({
778+
txRequest,
779+
prv: JSON.stringify({
780+
pShare: userKeyShare.pShare,
781+
bitgoNShare: bitgoKeyShare.nShares[1],
782+
backupNShare: backupKeyShare.nShares[1],
783+
}),
784+
reqId,
785+
})
786+
.should.be.rejectedWith(
787+
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
788+
);
789+
});
790+
791+
it('signTxRequest should fail when txParams has empty recipients', async function () {
792+
await tssUtils
793+
.signTxRequest({
794+
txRequest,
795+
prv: JSON.stringify({
796+
pShare: userKeyShare.pShare,
797+
bitgoNShare: bitgoKeyShare.nShares[1],
798+
backupNShare: backupKeyShare.nShares[1],
799+
}),
800+
reqId,
801+
txParams: { recipients: [] },
802+
})
803+
.should.be.rejectedWith(
804+
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
805+
);
806+
});
807+
773808
it('signTxRequest should fail with wrong recipient', async function () {
774809
// To generate these Hex values, we used the bitgo-ui to create a transaction and then
775810
// used the `signableHex` and `serializedTxHex` values from the prebuild.

‎modules/bitgo/test/v2/unit/internal/tssUtils/ecdsaMPCv2/signTxRequest.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ describe('signTxRequest:', function () {
193193
txRequest,
194194
prv: userPrvBase64,
195195
reqId,
196+
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
196197
});
197198
nockPromises[0].isDone().should.be.true();
198199
nockPromises[1].isDone().should.be.true();
@@ -215,6 +216,7 @@ describe('signTxRequest:', function () {
215216
prv: backupPrvBase64,
216217
mpcv2PartyId: 1,
217218
reqId,
219+
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
218220
});
219221
nockPromises[0].isDone().should.be.true();
220222
nockPromises[1].isDone().should.be.true();
@@ -236,6 +238,7 @@ describe('signTxRequest:', function () {
236238
txRequest,
237239
prv: userPrvBase64,
238240
reqId,
241+
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
239242
});
240243
nockPromises[0].isDone().should.be.true();
241244
nockPromises[1].isDone().should.be.true();
@@ -257,6 +260,7 @@ describe('signTxRequest:', function () {
257260
txRequest,
258261
prv: userPrvBase64,
259262
reqId,
263+
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
260264
});
261265
nockPromises[0].isDone().should.be.true();
262266
nockPromises[1].isDone().should.be.true();
@@ -277,11 +281,41 @@ describe('signTxRequest:', function () {
277281
txRequest,
278282
prv: userPrvBase64,
279283
reqId,
284+
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
280285
})
281286
.should.be.rejectedWith('Too many requests, slow down!');
282287
nockPromises[0].isDone().should.be.true();
283288
nockPromises[1].isDone().should.be.false();
284289
});
290+
291+
it('rejects signTxRequest when txParams is missing', async function () {
292+
const userShare = fs.readFileSync(shareFiles[vector.party1]);
293+
const userPrvBase64 = Buffer.from(userShare).toString('base64');
294+
await tssUtils
295+
.signTxRequest({
296+
txRequest,
297+
prv: userPrvBase64,
298+
reqId,
299+
})
300+
.should.be.rejectedWith(
301+
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
302+
);
303+
});
304+
305+
it('rejects signTxRequest when txParams has empty recipients', async function () {
306+
const userShare = fs.readFileSync(shareFiles[vector.party1]);
307+
const userPrvBase64 = Buffer.from(userShare).toString('base64');
308+
await tssUtils
309+
.signTxRequest({
310+
txRequest,
311+
prv: userPrvBase64,
312+
reqId,
313+
txParams: { recipients: [] },
314+
})
315+
.should.be.rejectedWith(
316+
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
317+
);
318+
});
285319
});
286320

287321
export function getBitGoPartyGpgKeyPrv(key: openpgp.SerializedKeyPair<string>): DklsTypes.PartyGpgKey {

‎modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,9 @@ export class PendingApproval implements IPendingApproval {
246246
}
247247

248248
const decryptedPrv = await this.wallet.getPrv({ walletPassphrase });
249-
const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId);
249+
const pendingApprovalRecipients = this._pendingApproval.info?.transactionRequest?.recipients;
250+
const txParams = pendingApprovalRecipients?.length ? { recipients: pendingApprovalRecipients } : undefined;
251+
const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId, txParams);
250252
if (txRequest.apiVersion === 'lite') {
251253
if (!txRequest.unsignedTxs || txRequest.unsignedTxs.length === 0) {
252254
throw new Error('Unexpected error, no transactions found in txRequest.');

‎modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
TxRequest,
4040
TxRequestVersion,
4141
} from './baseTypes';
42+
import { TransactionParams } from '../../baseCoin/iBaseCoin';
4243
import { GShare, SignShare } from '../../../account-lib/mpc/tss';
4344
import { RequestTracer } from '../util';
4445
import { envRequiresBitgoPubGpgKeyConfig, getBitgoMpcGpgPubKey } from '../../tss/bitgoPubKeys';
@@ -533,11 +534,16 @@ export default class BaseTssUtils<KeyShare> extends MpcUtils implements ITssUtil
533534
* @param {RequestTracer} reqId id tracer.
534535
* @returns {Promise<any>}
535536
*/
536-
async recreateTxRequest(txRequestId: string, decryptedPrv: string, reqId: IRequestTracer): Promise<TxRequest> {
537+
async recreateTxRequest(
538+
txRequestId: string,
539+
decryptedPrv: string,
540+
reqId: IRequestTracer,
541+
txParams?: TransactionParams
542+
): Promise<TxRequest> {
537543
await this.deleteSignatureShares(txRequestId, reqId);
538544
// after delete signatures shares get the tx without them
539545
const txRequest = await getTxRequest(this.bitgo, this.wallet.id(), txRequestId, reqId);
540-
return await this.signTxRequest({ txRequest, prv: decryptedPrv, reqId });
546+
return await this.signTxRequest({ txRequest, prv: decryptedPrv, reqId, txParams });
541547
}
542548

543549
/**

‎modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,12 @@ export interface ITssUtils<KeyShare = EDDSA.KeyShare> {
748748
deleteSignatureShares(txRequestId: string): Promise<SignatureShareRecord[]>;
749749
// eslint-disable-next-line @typescript-eslint/no-explicit-any
750750
sendTxRequest(txRequestId: string): Promise<any>;
751-
recreateTxRequest(txRequestId: string, decryptedPrv: string, reqId: IRequestTracer): Promise<TxRequest>;
751+
recreateTxRequest(
752+
txRequestId: string,
753+
decryptedPrv: string,
754+
reqId: IRequestTracer,
755+
txParams?: TransactionParams
756+
): Promise<TxRequest>;
752757
getTxRequest(txRequestId: string): Promise<TxRequest>;
753758
supportedTxRequestVersions(): TxRequestVersion[];
754759
}

‎modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
} from '../../../tss/types';
4848
import { BaseEcdsaUtils } from './base';
4949
import { IRequestTracer } from '../../../../api';
50+
import { InvalidTransactionError } from '../../../errors';
5051

5152
const encryptNShare = ECDSAMethods.encryptNShare;
5253

@@ -745,21 +746,27 @@ export class EcdsaUtils extends BaseEcdsaUtils {
745746
const unsignedTx =
746747
txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
747748

749+
if (!params.txParams?.recipients?.length) {
750+
throw new InvalidTransactionError(
751+
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
752+
);
753+
}
754+
748755
// For ICP transactions, the HSM signs the serializedTxHex, while the user signs the signableHex separately.
749756
// Verification cannot be performed directly on the signableHex alone. However, we can parse the serializedTxHex
750757
// to regenerate the signableHex and compare it against the provided value for verification.
751758
// In contrast, for other coin families, verification is typically done using just the signableHex.
752759
if (this.baseCoin.getConfig().family === 'icp') {
753760
await this.baseCoin.verifyTransaction({
754761
txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex },
755-
txParams: params.txParams || { recipients: [] },
762+
txParams: params.txParams,
756763
wallet: this.wallet,
757764
walletType: this.wallet.multisigType(),
758765
});
759766
} else {
760767
await this.baseCoin.verifyTransaction({
761768
txPrebuild: { txHex: unsignedTx.signableHex },
762-
txParams: params.txParams || { recipients: [] },
769+
txParams: params.txParams,
763770
wallet: this.wallet,
764771
walletType: this.wallet.multisigType(),
765772
});

‎modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
verifyBitGoMessagesAndSignaturesRoundOne,
3333
verifyBitGoMessagesAndSignaturesRoundTwo,
3434
} from '../../../tss/ecdsa/ecdsaMPCv2';
35+
import { InvalidTransactionError } from '../../../errors';
3536
import { KeyCombined } from '../../../tss/ecdsa/types';
3637
import { generateGPGKeyPair } from '../../opengpgUtils';
3738
import {
@@ -736,21 +737,27 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
736737
const unsignedTx =
737738
txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
738739

740+
if (!params.txParams?.recipients?.length) {
741+
throw new InvalidTransactionError(
742+
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
743+
);
744+
}
745+
739746
// For ICP transactions, the HSM signs the serializedTxHex, while the user signs the signableHex separately.
740747
// Verification cannot be performed directly on the signableHex alone. However, we can parse the serializedTxHex
741748
// to regenerate the signableHex and compare it against the provided value for verification.
742749
// In contrast, for other coin families, verification is typically done using just the signableHex.
743750
if (this.baseCoin.getConfig().family === 'icp') {
744751
await this.baseCoin.verifyTransaction({
745752
txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex },
746-
txParams: params.txParams || { recipients: [] },
753+
txParams: params.txParams,
747754
wallet: this.wallet,
748755
walletType: this.wallet.multisigType(),
749756
});
750757
} else {
751758
await this.baseCoin.verifyTransaction({
752759
txPrebuild: { txHex: unsignedTx.signableHex },
753-
txParams: params.txParams || { recipients: [] },
760+
txParams: params.txParams,
754761
wallet: this.wallet,
755762
walletType: this.wallet.multisigType(),
756763
});

0 commit comments

Comments
 (0)