Skip to content

Commit 2a27243

Browse files
mohammadalfaiyazbitgomohammadalfaiyazbitgo
committed
Revert "fix(tss): verify recipients before signing"
TICKET: WAL-375
1 parent e385447 commit 2a27243

10 files changed

Lines changed: 11 additions & 515 deletions

File tree

‎modules/abstract-eth/src/abstractEthLikeNewCoins.ts‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3109,16 +3109,9 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
31093109
txParams.prebuildTx?.consolidateId ||
31103110
txPrebuild?.consolidateId ||
31113111
(txParams.type &&
3112-
[
3113-
'acceleration',
3114-
'fillNonce',
3115-
'transferToken',
3116-
'tokenApproval',
3117-
'consolidate',
3118-
'bridgeFunds',
3119-
'enableToken',
3120-
'customTx',
3121-
].includes(txParams.type))
3112+
['acceleration', 'fillNonce', 'transferToken', 'tokenApproval', 'consolidate', 'bridgeFunds'].includes(
3113+
txParams.type
3114+
))
31223115
)
31233116
) {
31243117
throw new Error('missing txParams');

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

Lines changed: 0 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,6 @@ describe('TSS Ecdsa Utils:', async function () {
749749
backupNShare: backupKeyShare.nShares[1],
750750
}),
751751
reqId,
752-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
753752
});
754753
signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs);
755754
const userGpgActual = sendShareSpy.getCalls()[0].args[10] as string;
@@ -767,125 +766,12 @@ describe('TSS Ecdsa Utils:', async function () {
767766
backupNShare: backupKeyShare.nShares[1],
768767
}),
769768
reqId,
770-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
771769
});
772770
signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs);
773771
const userGpgActual = sendShareSpy.getCalls()[0].args[10] as string;
774772
userGpgActual.should.startWith('-----BEGIN PGP PUBLIC KEY BLOCK-----');
775773
});
776774

777-
it('signTxRequest should fail when txParams is missing', async function () {
778-
await tssUtils
779-
.signTxRequest({
780-
txRequest,
781-
prv: JSON.stringify({
782-
pShare: userKeyShare.pShare,
783-
bitgoNShare: bitgoKeyShare.nShares[1],
784-
backupNShare: backupKeyShare.nShares[1],
785-
}),
786-
reqId,
787-
})
788-
.should.be.rejectedWith(
789-
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
790-
);
791-
});
792-
793-
it('signTxRequest should fail when txParams has empty recipients', async function () {
794-
await tssUtils
795-
.signTxRequest({
796-
txRequest,
797-
prv: JSON.stringify({
798-
pShare: userKeyShare.pShare,
799-
bitgoNShare: bitgoKeyShare.nShares[1],
800-
backupNShare: backupKeyShare.nShares[1],
801-
}),
802-
reqId,
803-
txParams: { recipients: [] },
804-
})
805-
.should.be.rejectedWith(
806-
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
807-
);
808-
});
809-
810-
it('signTxRequest should succeed when recipients are only in intent (smart contract interaction)', async function () {
811-
await setupSignTxRequestNocks(false, userSignShare, aShare, dShare, enterpriseData);
812-
const txRequestWithIntentRecipients = {
813-
...txRequest,
814-
intent: {
815-
intentType: 'contractCall',
816-
recipients: [
817-
{
818-
address: { address: '0xrecipient' },
819-
amount: { value: '1000', symbol: 'hteth' },
820-
},
821-
],
822-
},
823-
};
824-
const signedTxRequest = await tssUtils.signTxRequest({
825-
txRequest: txRequestWithIntentRecipients,
826-
prv: JSON.stringify({
827-
pShare: userKeyShare.pShare,
828-
bitgoNShare: bitgoKeyShare.nShares[1],
829-
backupNShare: backupKeyShare.nShares[1],
830-
}),
831-
reqId,
832-
// no txParams.recipients — should fall back to intent.recipients
833-
});
834-
signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs);
835-
});
836-
837-
it('signTxRequest should succeed for no-recipient tx types (tokenApproval)', async function () {
838-
await setupSignTxRequestNocks(false, userSignShare, aShare, dShare, enterpriseData);
839-
const signedTxRequest = await tssUtils.signTxRequest({
840-
txRequest,
841-
prv: JSON.stringify({
842-
pShare: userKeyShare.pShare,
843-
bitgoNShare: bitgoKeyShare.nShares[1],
844-
backupNShare: backupKeyShare.nShares[1],
845-
}),
846-
reqId,
847-
txParams: { type: 'tokenApproval' },
848-
});
849-
signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs);
850-
});
851-
852-
it('signTxRequest should succeed for no-recipient tx types (acceleration)', async function () {
853-
await setupSignTxRequestNocks(false, userSignShare, aShare, dShare, enterpriseData);
854-
const signedTxRequest = await tssUtils.signTxRequest({
855-
txRequest,
856-
prv: JSON.stringify({
857-
pShare: userKeyShare.pShare,
858-
bitgoNShare: bitgoKeyShare.nShares[1],
859-
backupNShare: backupKeyShare.nShares[1],
860-
}),
861-
reqId,
862-
txParams: { type: 'acceleration' },
863-
});
864-
signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs);
865-
});
866-
867-
it('signTxRequest should prefer txParams.recipients over intent.recipients when both are present', async function () {
868-
await setupSignTxRequestNocks(false, userSignShare, aShare, dShare, enterpriseData);
869-
const txRequestWithBothRecipients = {
870-
...txRequest,
871-
intent: {
872-
intentType: 'contractCall',
873-
recipients: [{ address: { address: '0xintentRecipient' }, amount: { value: '9999', symbol: 'hteth' } }],
874-
},
875-
};
876-
const signedTxRequest = await tssUtils.signTxRequest({
877-
txRequest: txRequestWithBothRecipients,
878-
prv: JSON.stringify({
879-
pShare: userKeyShare.pShare,
880-
bitgoNShare: bitgoKeyShare.nShares[1],
881-
backupNShare: backupKeyShare.nShares[1],
882-
}),
883-
reqId,
884-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
885-
});
886-
signedTxRequest.unsignedTxs.should.deepEqual(txRequest.unsignedTxs);
887-
});
888-
889775
it('signTxRequest should fail with wrong recipient', async function () {
890776
// To generate these Hex values, we used the bitgo-ui to create a transaction and then
891777
// used the `signableHex` and `serializedTxHex` values from the prebuild.

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

Lines changed: 0 additions & 190 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ describe('signTxRequest:', function () {
193193
txRequest,
194194
prv: userPrvBase64,
195195
reqId,
196-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
197196
});
198197
nockPromises[0].isDone().should.be.true();
199198
nockPromises[1].isDone().should.be.true();
@@ -216,7 +215,6 @@ describe('signTxRequest:', function () {
216215
prv: backupPrvBase64,
217216
mpcv2PartyId: 1,
218217
reqId,
219-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
220218
});
221219
nockPromises[0].isDone().should.be.true();
222220
nockPromises[1].isDone().should.be.true();
@@ -238,7 +236,6 @@ describe('signTxRequest:', function () {
238236
txRequest,
239237
prv: userPrvBase64,
240238
reqId,
241-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
242239
});
243240
nockPromises[0].isDone().should.be.true();
244241
nockPromises[1].isDone().should.be.true();
@@ -260,7 +257,6 @@ describe('signTxRequest:', function () {
260257
txRequest,
261258
prv: userPrvBase64,
262259
reqId,
263-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
264260
});
265261
nockPromises[0].isDone().should.be.true();
266262
nockPromises[1].isDone().should.be.true();
@@ -281,197 +277,11 @@ describe('signTxRequest:', function () {
281277
txRequest,
282278
prv: userPrvBase64,
283279
reqId,
284-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
285280
})
286281
.should.be.rejectedWith('Too many requests, slow down!');
287282
nockPromises[0].isDone().should.be.true();
288283
nockPromises[1].isDone().should.be.false();
289284
});
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-
});
319-
320-
it('accepts signTxRequest when recipients are only in intent (smart contract interaction)', async function () {
321-
const txRequestWithIntentRecipients = {
322-
...txRequest,
323-
intent: {
324-
intentType: 'contractCall',
325-
recipients: [
326-
{
327-
address: { address: '0xrecipient' },
328-
amount: { value: '1000', symbol: 'hteth' },
329-
},
330-
],
331-
},
332-
};
333-
const nockPromises = [
334-
await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequestWithIntentRecipients, bitgoGpgKey),
335-
await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequestWithIntentRecipients, bitgoGpgKey),
336-
await nockTxRequestResponseSignatureShareRoundThree(txRequestWithIntentRecipients),
337-
await nockSendTxRequest(txRequestWithIntentRecipients),
338-
];
339-
await Promise.all(nockPromises);
340-
341-
const userShare = fs.readFileSync(shareFiles[vector.party1]);
342-
const userPrvBase64 = Buffer.from(userShare).toString('base64');
343-
// Falls back to intent.recipients — guard should pass and signing should complete
344-
await tssUtils.signTxRequest({
345-
txRequest: txRequestWithIntentRecipients,
346-
prv: userPrvBase64,
347-
reqId,
348-
// no txParams.recipients
349-
});
350-
nockPromises[0].isDone().should.be.true();
351-
nockPromises[1].isDone().should.be.true();
352-
nockPromises[2].isDone().should.be.true();
353-
});
354-
355-
it('accepts signTxRequest for no-recipient tx types (tokenApproval)', async function () {
356-
const nockPromises = [
357-
await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequest, bitgoGpgKey),
358-
await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequest, bitgoGpgKey),
359-
await nockTxRequestResponseSignatureShareRoundThree(txRequest),
360-
await nockSendTxRequest(txRequest),
361-
];
362-
await Promise.all(nockPromises);
363-
364-
const userShare = fs.readFileSync(shareFiles[vector.party1]);
365-
const userPrvBase64 = Buffer.from(userShare).toString('base64');
366-
// Type exemption applies — guard passes without recipients
367-
await tssUtils.signTxRequest({
368-
txRequest,
369-
prv: userPrvBase64,
370-
reqId,
371-
txParams: { type: 'tokenApproval' },
372-
});
373-
nockPromises[0].isDone().should.be.true();
374-
nockPromises[1].isDone().should.be.true();
375-
nockPromises[2].isDone().should.be.true();
376-
});
377-
378-
it('accepts signTxRequest for no-recipient tx types (acceleration)', async function () {
379-
const nockPromises = [
380-
await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequest, bitgoGpgKey),
381-
await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequest, bitgoGpgKey),
382-
await nockTxRequestResponseSignatureShareRoundThree(txRequest),
383-
await nockSendTxRequest(txRequest),
384-
];
385-
await Promise.all(nockPromises);
386-
387-
const userShare = fs.readFileSync(shareFiles[vector.party1]);
388-
const userPrvBase64 = Buffer.from(userShare).toString('base64');
389-
await tssUtils.signTxRequest({
390-
txRequest,
391-
prv: userPrvBase64,
392-
reqId,
393-
txParams: { type: 'acceleration' },
394-
});
395-
nockPromises[0].isDone().should.be.true();
396-
nockPromises[1].isDone().should.be.true();
397-
nockPromises[2].isDone().should.be.true();
398-
});
399-
400-
it('accepts signTxRequest for no-recipient tx types (customTx)', async function () {
401-
const nockPromises = [
402-
await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequest, bitgoGpgKey),
403-
await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequest, bitgoGpgKey),
404-
await nockTxRequestResponseSignatureShareRoundThree(txRequest),
405-
await nockSendTxRequest(txRequest),
406-
];
407-
await Promise.all(nockPromises);
408-
409-
const userShare = fs.readFileSync(shareFiles[vector.party1]);
410-
const userPrvBase64 = Buffer.from(userShare).toString('base64');
411-
// DeFi/WalletConnect smart contract interactions have no traditional recipients
412-
await tssUtils.signTxRequest({
413-
txRequest,
414-
prv: userPrvBase64,
415-
reqId,
416-
txParams: { type: 'customTx' },
417-
});
418-
nockPromises[0].isDone().should.be.true();
419-
nockPromises[1].isDone().should.be.true();
420-
nockPromises[2].isDone().should.be.true();
421-
});
422-
423-
it('accepts signTxRequest for no-recipient tx types (enableToken)', async function () {
424-
const nockPromises = [
425-
await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequest, bitgoGpgKey),
426-
await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequest, bitgoGpgKey),
427-
await nockTxRequestResponseSignatureShareRoundThree(txRequest),
428-
await nockSendTxRequest(txRequest),
429-
];
430-
await Promise.all(nockPromises);
431-
432-
const userShare = fs.readFileSync(shareFiles[vector.party1]);
433-
const userPrvBase64 = Buffer.from(userShare).toString('base64');
434-
// TSS wallets do not populate recipients for token enablement — exemption must apply
435-
await tssUtils.signTxRequest({
436-
txRequest,
437-
prv: userPrvBase64,
438-
reqId,
439-
txParams: { type: 'enableToken' },
440-
});
441-
nockPromises[0].isDone().should.be.true();
442-
nockPromises[1].isDone().should.be.true();
443-
nockPromises[2].isDone().should.be.true();
444-
});
445-
446-
it('accepts signTxRequest when txParams.recipients takes priority over intent.recipients', async function () {
447-
const txRequestWithBothRecipients = {
448-
...txRequest,
449-
intent: {
450-
intentType: 'contractCall',
451-
recipients: [{ address: { address: '0xintentRecipient' }, amount: { value: '9999', symbol: 'hteth' } }],
452-
},
453-
};
454-
const nockPromises = [
455-
await nockTxRequestResponseSignatureShareRoundOne(bitgoParty, txRequestWithBothRecipients, bitgoGpgKey),
456-
await nockTxRequestResponseSignatureShareRoundTwo(bitgoParty, txRequestWithBothRecipients, bitgoGpgKey),
457-
await nockTxRequestResponseSignatureShareRoundThree(txRequestWithBothRecipients),
458-
await nockSendTxRequest(txRequestWithBothRecipients),
459-
];
460-
await Promise.all(nockPromises);
461-
462-
const userShare = fs.readFileSync(shareFiles[vector.party1]);
463-
const userPrvBase64 = Buffer.from(userShare).toString('base64');
464-
// txParams.recipients takes priority — guard passes and signing completes
465-
await tssUtils.signTxRequest({
466-
txRequest: txRequestWithBothRecipients,
467-
prv: userPrvBase64,
468-
reqId,
469-
txParams: { recipients: [{ address: '0xrecipient', amount: '1000' }] },
470-
});
471-
nockPromises[0].isDone().should.be.true();
472-
nockPromises[1].isDone().should.be.true();
473-
nockPromises[2].isDone().should.be.true();
474-
});
475285
});
476286

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

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,7 @@ export class PendingApproval implements IPendingApproval {
250250
}
251251

252252
const decryptedPrv = await this.wallet.getPrv({ walletPassphrase });
253-
const pendingApprovalRecipients = this._pendingApproval.info?.transactionRequest?.recipients;
254-
const txParams = pendingApprovalRecipients?.length ? { recipients: pendingApprovalRecipients } : undefined;
255-
const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId, txParams);
253+
const txRequest = await this.tssUtils!.recreateTxRequest(txRequestId, decryptedPrv, reqId);
256254
if (txRequest.apiVersion === 'lite') {
257255
if (!txRequest.unsignedTxs || txRequest.unsignedTxs.length === 0) {
258256
throw new Error('Unexpected error, no transactions found in txRequest.');

0 commit comments

Comments
 (0)