From cbcb46f7041099105cb592bb479580634774aaa4 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Wed, 3 May 2023 13:07:03 +0200 Subject: [PATCH] fix: prevent cancel and speedup transactions to call accept approval (#18846) --- app/scripts/controllers/transactions/index.js | 16 ++++++++++++---- .../controllers/transactions/index.test.js | 3 +++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index be61a789a..5b393963c 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -1248,7 +1248,9 @@ export default class TransactionController extends EventEmitter { } this.addTransaction(newTxMeta); - await this.approveTransaction(newTxMeta.id, actionId); + await this.approveTransaction(newTxMeta.id, actionId, { + hasApprovalRequest: false, + }); return newTxMeta; } @@ -1306,7 +1308,9 @@ export default class TransactionController extends EventEmitter { } this.addTransaction(newTxMeta); - await this.approveTransaction(newTxMeta.id, actionId); + await this.approveTransaction(newTxMeta.id, actionId, { + hasApprovalRequest: false, + }); return newTxMeta; } @@ -1345,8 +1349,10 @@ export default class TransactionController extends EventEmitter { * * @param {number} txId - the tx's Id * @param {string} actionId - actionId passed from UI + * @param opts - options object + * @param opts.hasApprovalRequest - whether the transaction has an approval request */ - async approveTransaction(txId, actionId) { + async approveTransaction(txId, actionId, { hasApprovalRequest = true } = {}) { // TODO: Move this safety out of this function. // Since this transaction is async, // we need to keep track of what is currently being signed, @@ -1361,7 +1367,9 @@ export default class TransactionController extends EventEmitter { try { // approve this.txStateManager.setTxStatusApproved(txId); - this._acceptApproval(txMeta); + if (hasApprovalRequest) { + this._acceptApproval(txMeta); + } // get next nonce const fromAddress = txMeta.txParams.from; // wait for a nonce diff --git a/app/scripts/controllers/transactions/index.test.js b/app/scripts/controllers/transactions/index.test.js index 601b1fb88..3767572b7 100644 --- a/app/scripts/controllers/transactions/index.test.js +++ b/app/scripts/controllers/transactions/index.test.js @@ -608,6 +608,8 @@ describe('Transaction Controller', function () { cancelTxMeta.id, ); assert.deepEqual(cancelTxMeta, memTxMeta); + // One for the initial addUnapprovedTransaction, one for the approval + assert.equal(messengerMock.call.callCount, 2); }); it('should add only 1 cancel transaction when called twice with same actionId', async function () { @@ -1385,6 +1387,7 @@ describe('Transaction Controller', function () { type: TransactionType.retry, }, ); + assert.equal(messengerMock.call.callCount, 0); }); it('should call this.approveTransaction with the id of the returned tx', async function () {