Make createSpeedUpTransaction action idempotent (#15688)

app/scripts/controllers/transactions/index.js

@@ -772,7 +772,7 @@ export default class TransactionController extends EventEmitter {
       );
     }
 
-    // In transaction is found for same action id, do not create a new transaction.
+    // If a transaction is found with the same actionId, do not create a new speed-up transaction.
     if (actionId) {
       let existingTxMeta =
         this.txStateManager.getTransactionWithActionId(actionId);
@@ -1228,13 +1228,23 @@ export default class TransactionController extends EventEmitter {
    *  params instead of allowing this method to generate them
    * @param options
    * @param options.estimatedBaseFee
+   * @param options.actionId
    * @returns {txMeta}
    */
   async createSpeedUpTransaction(
     originalTxId,
     customGasSettings,
-    { estimatedBaseFee } = {},
+    { estimatedBaseFee, actionId } = {},
   ) {
+    // If transaction is found for same action id, do not create a new speed-up transaction.
+    if (actionId) {
+      const existingTxMeta =
+        this.txStateManager.getTransactionWithActionId(actionId);
+      if (existingTxMeta) {
+        return existingTxMeta;
+      }
+    }
+
     const originalTxMeta = this.txStateManager.getTransaction(originalTxId);
     const { txParams } = originalTxMeta;
 
@@ -1253,6 +1263,7 @@ export default class TransactionController extends EventEmitter {
       status: TRANSACTION_STATUSES.APPROVED,
       type: TRANSACTION_TYPES.RETRY,
       originalType: originalTxMeta.type,
+      actionId,
     });
 
     if (estimatedBaseFee) {

app/scripts/controllers/transactions/index.test.js

@@ -1171,11 +1171,35 @@ describe('Transaction Controller', function () {
     let approveTransactionSpy;
     let txParams;
     let expectedTxParams;
+    const selectedAddress = '0x1678a085c290ebd122dc42cba69373b5953b831d';
+    const recipientAddress = '0xc42edfcc21ed14dda456aa0756c153f7985d8813';
+
+    let getSelectedAddress,
+      getPermittedAccounts,
+      getDefaultGasFees,
+      getDefaultGasLimit;
 
     beforeEach(function () {
       addTransactionSpy = sinon.spy(txController, 'addTransaction');
       approveTransactionSpy = sinon.spy(txController, 'approveTransaction');
 
+      const hash =
+        '0x2a5523c6fa98b47b7d9b6c8320179785150b42a16bcff36b398c5062b65657e8';
+      providerResultStub.eth_sendRawTransaction = hash;
+
+      getSelectedAddress = sinon
+        .stub(txController, 'getSelectedAddress')
+        .returns(selectedAddress);
+      getDefaultGasFees = sinon
+        .stub(txController, '_getDefaultGasFees')
+        .returns({});
+      getDefaultGasLimit = sinon
+        .stub(txController, '_getDefaultGasLimit')
+        .returns({});
+      getPermittedAccounts = sinon
+        .stub(txController, 'getPermittedAccounts')
+        .returns([selectedAddress]);
+
       txParams = {
         nonce: '0x00',
         from: '0xB09d8505E1F4EF1CeA089D47094f5DD3464083d4',
@@ -1201,6 +1225,10 @@ describe('Transaction Controller', function () {
     afterEach(function () {
       addTransactionSpy.restore();
       approveTransactionSpy.restore();
+      getSelectedAddress.restore();
+      getPermittedAccounts.restore();
+      getDefaultGasFees.restore();
+      getDefaultGasLimit.restore();
     });
 
     it('should call this.addTransaction and this.approveTransaction with the expected args', async function () {
@@ -1242,6 +1270,52 @@ describe('Transaction Controller', function () {
         },
       );
     });
+
+    it('should add only 1 speedup transaction when called twice with same actionId', async function () {
+      const txMeta = await txController.addUnapprovedTransaction({
+        from: selectedAddress,
+        to: recipientAddress,
+      });
+      await txController.approveTransaction(txMeta.id);
+      await txController.createSpeedUpTransaction(
+        txMeta.id,
+        {},
+        { actionId: 12345 },
+      );
+      const transactionCount1 =
+        txController.txStateManager.getTransactions().length;
+      await txController.createSpeedUpTransaction(
+        txMeta.id,
+        {},
+        { actionId: 12345 },
+      );
+      const transactionCount2 =
+        txController.txStateManager.getTransactions().length;
+      assert.equal(transactionCount1, transactionCount2);
+    });
+
+    it('should add multiple transactions when called with different actionId', async function () {
+      const txMeta = await txController.addUnapprovedTransaction({
+        from: selectedAddress,
+        to: recipientAddress,
+      });
+      await txController.approveTransaction(txMeta.id);
+      await txController.createSpeedUpTransaction(
+        txMeta.id,
+        {},
+        { actionId: 12345 },
+      );
+      const transactionCount1 =
+        txController.txStateManager.getTransactions().length;
+      await txController.createSpeedUpTransaction(
+        txMeta.id,
+        {},
+        { actionId: 11111 },
+      );
+      const transactionCount2 =
+        txController.txStateManager.getTransactions().length;
+      assert.equal(transactionCount1 + 1, transactionCount2);
+    });
   });
 
   describe('#signTransaction', function () {

app/scripts/metamask-controller.js

@@ -3301,18 +3301,14 @@ export default class MetamaskController extends EventEmitter {
    *  './controllers/transactions'
    * ).CustomGasSettings} [customGasSettings] - overrides to use for gas params
    *  instead of allowing this method to generate them
-   * @param newTxMetaProps
+   * @param options
    * @returns {object} MetaMask state
    */
-  async createSpeedUpTransaction(
-    originalTxId,
-    customGasSettings,
-    newTxMetaProps,
-  ) {
+  async createSpeedUpTransaction(originalTxId, customGasSettings, options) {
     await this.txController.createSpeedUpTransaction(
       originalTxId,
       customGasSettings,
-      newTxMetaProps,
+      options,
     );
     const state = await this.getState();
     return state;

ui/store/actions.js

@@ -2004,19 +2004,16 @@ export function createCancelTransaction(txId, customGasSettings, options) {
   };
 }
 
-export function createSpeedUpTransaction(
-  txId,
-  customGasSettings,
-  newTxMetaProps,
-) {
+export function createSpeedUpTransaction(txId, customGasSettings, options) {
   log.debug('background.createSpeedUpTransaction');
   let newTx;
 
   return (dispatch) => {
+    const actionId = Date.now() + Math.random();
     return new Promise((resolve, reject) => {
       callBackgroundMethod(
         'createSpeedUpTransaction',
-        [txId, customGasSettings, newTxMetaProps],
+        [txId, customGasSettings, { ...options, actionId }],
         (err, newState) => {
           if (err) {
             dispatch(displayWarning(err.message));
@@ -2028,6 +2025,7 @@ export function createSpeedUpTransaction(
           newTx = currentNetworkTxList[currentNetworkTxList.length - 1];
           resolve(newState);
         },
+        actionId,
       );
     })
       .then((newState) => dispatch(updateMetamaskState(newState)))