Make updateTransactionSendFlowHistory background method idempotent (#15585)

app/scripts/controllers/transactions/index.js

@@ -673,27 +673,36 @@ export default class TransactionController extends EventEmitter {
    * state is unapproved. Returns the updated transaction.
    *
    * @param {string} txId - transaction id
+   * @param {number} currentSendFlowHistoryLength - sendFlowHistory entries currently
    * @param {Array<{ entry: string, timestamp: number }>} sendFlowHistory -
    *  history to add to the sendFlowHistory property of txMeta.
    * @returns {TransactionMeta} the txMeta of the updated transaction
    */
-  updateTransactionSendFlowHistory(txId, sendFlowHistory) {
+  updateTransactionSendFlowHistory(
+    txId,
+    currentSendFlowHistoryLength,
+    sendFlowHistory,
+  ) {
     this._throwErrorIfNotUnapprovedTx(txId, 'updateTransactionSendFlowHistory');
     const txMeta = this._getTransaction(txId);
 
-    // only update what is defined
-    const note = `Update sendFlowHistory for ${txId}`;
+    if (
+      currentSendFlowHistoryLength === (txMeta?.sendFlowHistory?.length || 0)
+    ) {
+      // only update what is defined
+      const note = `Update sendFlowHistory for ${txId}`;
 
-    this.txStateManager.updateTransaction(
-      {
-        ...txMeta,
-        sendFlowHistory: [
-          ...(txMeta?.sendFlowHistory ?? []),
-          ...sendFlowHistory,
-        ],
-      },
-      note,
-    );
+      this.txStateManager.updateTransaction(
+        {
+          ...txMeta,
+          sendFlowHistory: [
+            ...(txMeta?.sendFlowHistory ?? []),
+            ...sendFlowHistory,
+          ],
+        },
+        note,
+      );
+    }
     return this._getTransaction(txId);
   }
 

app/scripts/metamask-controller.actions.test.js

@@ -1,6 +1,7 @@
 import { strict as assert } from 'assert';
 import sinon from 'sinon';
 import proxyquire from 'proxyquire';
+import { ORIGIN_METAMASK } from '../../shared/constants/app';
 
 const Ganache = require('../../test/e2e/ganache');
 
@@ -209,4 +210,32 @@ describe('MetaMaskController', function () {
       assert.equal(rpcList1Length, rpcList2Length);
     });
   });
+
+  describe('#updateTransactionSendFlowHistory', function () {
+    it('two sequential calls with same history give same result', async function () {
+      const recipientAddress = '0xc42edfcc21ed14dda456aa0756c153f7985d8813';
+
+      await metamaskController.createNewVaultAndKeychain('test@123');
+      const accounts = await metamaskController.keyringController.getAccounts();
+      const txMeta = await metamaskController.getApi().addUnapprovedTransaction(
+        {
+          from: accounts[0],
+          to: recipientAddress,
+        },
+        ORIGIN_METAMASK,
+      );
+
+      const [transaction1, transaction2] = await Promise.all([
+        metamaskController
+          .getApi()
+          .updateTransactionSendFlowHistory(txMeta.id, 2, ['foo1', 'foo2']),
+        Promise.resolve(1).then(() =>
+          metamaskController
+            .getApi()
+            .updateTransactionSendFlowHistory(txMeta.id, 2, ['foo1', 'foo2']),
+        ),
+      ]);
+      assert.deepEqual(transaction1, transaction2);
+    });
+  });
 });

ui/ducks/send/send.js

@@ -2269,6 +2269,7 @@ export function signTransaction() {
       await dispatch(
         updateTransactionSendFlowHistory(
           draftTransaction.id,
+          unapprovedTx.sendFlowHistory?.length || 0,
           draftTransaction.history,
         ),
       );

ui/ducks/send/send.test.js

@@ -97,7 +97,7 @@ setBackgroundConnection({
   addUnapprovedTransaction: jest.fn((_v, _w, _x, _y, _z, cb) => {
     cb(null);
   }),
-  updateTransactionSendFlowHistory: jest.fn((_x, _y, cb) => cb(null)),
+  updateTransactionSendFlowHistory: jest.fn((_x, _y, _z, cb) => cb(null)),
 });
 
 const getTestUUIDTx = (state) => state.draftTransactions['test-uuid'];

ui/store/actions.js

@@ -761,17 +761,22 @@ export function updateEditableParams(txId, editableParams) {
  * Appends new send flow history to a transaction
  *
  * @param {string} txId - the id of the transaction to update
+ * @param {number} currentSendFlowHistoryLength - sendFlowHistory entries currently
  * @param {Array<{event: string, timestamp: number}>} sendFlowHistory - the new send flow history to append to the
  *  transaction
  * @returns {import('../../shared/constants/transaction').TransactionMeta}
  */
-export function updateTransactionSendFlowHistory(txId, sendFlowHistory) {
+export function updateTransactionSendFlowHistory(
+  txId,
+  currentSendFlowHistoryLength,
+  sendFlowHistory,
+) {
   return async (dispatch) => {
     let updatedTransaction;
     try {
       updatedTransaction = await submitRequestToBackground(
         'updateTransactionSendFlowHistory',
-        [txId, sendFlowHistory],
+        [txId, currentSendFlowHistoryLength, sendFlowHistory],
       );
     } catch (error) {
       dispatch(txError(error));