Refactor transaction ids to be UUIDs instead of random number

app/scripts/controllers/transactions/EtherscanRemoteTransactionSource.ts

@@ -1,7 +1,7 @@
 import { BNToHex } from '@metamask/controller-utils';
 import type { Hex } from '@metamask/utils';
 import { BN } from 'ethereumjs-util';
-import createId from '../../../../shared/modules/random-id';
+import { v4 as uuid } from 'uuid';
 
 import {
   TransactionMeta,
@@ -138,7 +138,7 @@ export class EtherscanRemoteTransactionSource
       blockNumber: txMeta.blockNumber,
       chainId: currentChainId,
       hash: txMeta.hash,
-      id: createId(),
+      id: uuid(),
       metamaskNetworkId: currentNetworkId,
       status: TransactionStatus.confirmed,
       time,

app/scripts/controllers/transactions/index.js

@@ -2825,7 +2825,7 @@ export default class TransactionController extends EventEmitter {
       const { origin } = txMeta;
 
       const approvalResult = await this._requestApproval(
-        String(txId),
+        txId,
         origin,
         { txId },
         {

app/scripts/controllers/transactions/tx-state-manager.js

@@ -2,7 +2,7 @@ import EventEmitter from '@metamask/safe-event-emitter';
 import { ObservableStore } from '@metamask/obs-store';
 import log from 'loglevel';
 import { values, keyBy, mapValues, omitBy, pickBy, sortBy } from 'lodash';
-import createId from '../../../../shared/modules/random-id';
+import { v4 as uuid } from 'uuid';
 import { TransactionStatus } from '../../../../shared/constants/transaction';
 import { METAMASK_CONTROLLER_EVENTS } from '../../metamask-controller';
 import { transactionMatchesNetwork } from '../../../../shared/modules/transaction.utils';
@@ -130,7 +130,7 @@ export default class TransactionStateManager extends EventEmitter {
     }
 
     return {
-      id: createId(),
+      id: uuid(),
       time: new Date().getTime(),
       status: TransactionStatus.unapproved,
       metamaskNetworkId: networkId,

app/scripts/migrations/098.test.ts

@@ -0,0 +1,83 @@
+import { migrate, version } from './098';
+
+const oldVersion = 97;
+describe('migration #98', () => {
+  it('updates the version metadata', async () => {
+    const oldStorage = {
+      meta: { version: oldVersion },
+      data: {},
+    };
+
+    const newStorage = await migrate(oldStorage);
+
+    expect(newStorage.meta).toStrictEqual({ version });
+  });
+
+  it('handles missing TransactionController', async () => {
+    const oldState = {
+      OtherController: {},
+    };
+
+    const transformedState = await migrate({
+      meta: { version: oldVersion },
+      data: oldState,
+    });
+
+    expect(transformedState.data).toEqual(oldState);
+  });
+
+  it('handles empty transactions', async () => {
+    const oldState = {
+      TransactionController: {
+        transactions: {},
+      },
+    };
+
+    const transformedState = await migrate({
+      meta: { version: oldVersion },
+      data: oldState,
+    });
+
+    expect(transformedState.data).toEqual(oldState);
+  });
+
+  it('handles missing state', async () => {
+    const transformedState = await migrate({
+      meta: { version: oldVersion },
+      data: {},
+    });
+
+    expect(transformedState.data).toEqual({});
+  });
+
+  it('updates transaction ids', async () => {
+    const oldState = {
+      TransactionController: {
+        transactions: {
+          1: {
+            id: 1,
+            otherProp: 1,
+          },
+          2: {
+            id: 2,
+            otherProp: 2,
+          },
+        },
+      },
+    };
+    const oldStorage = {
+      meta: { version: oldVersion },
+      data: oldState,
+    };
+
+    const newStorage = await migrate(oldStorage);
+
+    const migratedTransactions =
+      newStorage.data.TransactionController.transactions;
+
+    Object.keys(migratedTransactions).forEach((newTxId) => {
+      expect(typeof newTxId).toBe('string');
+      expect(newTxId).toBe(migratedTransactions[newTxId].id);
+    });
+  });
+});

app/scripts/migrations/098.ts

@@ -0,0 +1,56 @@
+import { cloneDeep, isEmpty } from 'lodash';
+import { v4 as uuid } from 'uuid';
+
+type VersionedData = {
+  meta: { version: number };
+  data: Record<string, unknown>;
+};
+
+export const version = 98;
+
+/**
+ * The core TransactionController uses strings for transaction IDs, specifically UUIDs generated by the uuid package.
+ * For the sake of standardisation and minimising code maintenance, the use of UUIDs is preferred.
+ * This migration updates the transaction IDs to UUIDs.
+ *
+ * @param originalVersionedData
+ */
+export async function migrate(
+  originalVersionedData: VersionedData,
+): Promise<VersionedData> {
+  const versionedData = cloneDeep(originalVersionedData);
+  versionedData.meta.version = version;
+  transformState(versionedData.data);
+  return versionedData;
+}
+
+function transformState(state: Record<string, any>) {
+  const transactionControllerState = state?.TransactionController || {};
+  const transactions = transactionControllerState?.transactions || {};
+
+  if (isEmpty(transactions)) {
+    return;
+  }
+
+  const newTxs = Object.keys(transactions).reduce(
+    (txs: { [key: string]: any }, oldTransactionId) => {
+      // Clone the transaction
+      const transaction = cloneDeep(transactions[oldTransactionId]);
+
+      // Assign a new id to the transaction
+      const newTransactionID = uuid();
+      transaction.id = newTransactionID;
+
+      return {
+        ...txs,
+        [newTransactionID]: transaction,
+      };
+    },
+    {},
+  );
+
+  state.TransactionController = {
+    ...transactionControllerState,
+    transactions: newTxs,
+  };
+}

shared/constants/transaction.ts

@@ -331,7 +331,7 @@ export interface TransactionMeta {
   blockNumber?: string;
   chainId: string;
   /** An internally unique tx identifier. */
-  id: number;
+  id: string;
   /** Time the transaction was first suggested, in unix epoch time (ms). */
   time: number;
   /** A string representing a name of transaction contract method. */

ui/ducks/app/app.ts

@@ -69,7 +69,7 @@ interface AppState {
   newTokensImported: string;
   onboardedInThisUISession: boolean;
   customTokenAmount: string;
-  txId: number | null;
+  txId: string | null;
   accountDetailsAddress: string;
   ///: BEGIN:ONLY_INCLUDE_IN(snaps)
   snapsInstallPrivacyWarningShown: boolean;

ui/pages/confirm-transaction/confirm-transaction.component.js

@@ -90,7 +90,7 @@ const ConfirmTransaction = () => {
   ]);
 
   const { id, type } = transaction;
-  const transactionId = id && String(id);
+  const transactionId = id;
   const isValidTokenMethod = isTokenMethodAction(type);
   const isValidTransactionId =
     transactionId &&

ui/store/actions.ts

@@ -699,7 +699,7 @@ export function decryptMsg(
     }
 
     dispatch(updateMetamaskState(newState));
-    dispatch(completedTx(decryptedMsgData.metamaskId));
+    dispatch(completedTx(decryptedMsgData.metamaskId as unknown as string));
     dispatch(closeCurrentNotificationWindow());
     return decryptedMsgData;
   };
@@ -732,7 +732,7 @@ export function encryptionPublicKeyMsg(
     }
 
     dispatch(updateMetamaskState(newState));
-    dispatch(completedTx(msgData.metamaskId));
+    dispatch(completedTx(msgData.metamaskId as unknown as string));
     dispatch(closeCurrentNotificationWindow());
     return msgData;
   };
@@ -774,7 +774,7 @@ const updateMetamaskStateFromBackground = (): Promise<
  * @param previousGasParams - Object of gas params to set as previous
  */
 export function updatePreviousGasParams(
-  txId: number,
+  txId: string,
   previousGasParams: Record<string, any>,
 ): ThunkAction<
   Promise<TransactionMeta>,
@@ -799,7 +799,7 @@ export function updatePreviousGasParams(
 }
 
 export function updateEditableParams(
-  txId: number,
+  txId: string,
   editableParams: Partial<TxParams>,
 ): ThunkAction<
   Promise<TransactionMeta>,
@@ -834,7 +834,7 @@ export function updateEditableParams(
  * @returns
  */
 export function updateTransactionSendFlowHistory(
-  txId: number,
+  txId: string,
   currentSendFlowHistoryLength: number,
   sendFlowHistory: DraftTransaction['history'],
 ): ThunkAction<
@@ -890,7 +890,7 @@ export async function restoreUserData(jsonString: Json): Promise<true> {
 
 // TODO: codeword: NOT_A_THUNK @brad-decker
 export function updateTransactionGasFees(
-  txId: number,
+  txId: string,
   txGasFees: Partial<TxGasFees>,
 ): ThunkAction<
   Promise<TransactionMeta>,
@@ -1098,7 +1098,7 @@ export async function getTransactions(
 }
 
 export function completedTx(
-  txId: number,
+  txId: string,
 ): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
   return (dispatch: MetaMaskReduxDispatch) => {
     dispatch({
@@ -1110,7 +1110,7 @@ export function completedTx(
   };
 }
 
-export function updateTransactionParams(txId: number, txParams: TxParams) {
+export function updateTransactionParams(txId: string, txParams: TxParams) {
   return {
     type: actionConstants.UPDATE_TRANSACTION_PARAMS,
     id: txId,
@@ -1322,7 +1322,7 @@ export function cancelDecryptMsg(
     }
 
     dispatch(updateMetamaskState(newState));
-    dispatch(completedTx(msgData.id));
+    dispatch(completedTx(msgData.id as unknown as string));
     dispatch(closeCurrentNotificationWindow());
     return msgData;
   };
@@ -1349,7 +1349,7 @@ export function cancelEncryptionPublicKeyMsg(
     }
 
     dispatch(updateMetamaskState(newState));
-    dispatch(completedTx(msgData.id));
+    dispatch(completedTx(msgData.id as unknown as string));
     dispatch(closeCurrentNotificationWindow());
     return msgData;
   };
@@ -2047,12 +2047,12 @@ export function clearPendingTokens(): Action {
 }
 
 export function createCancelTransaction(
-  txId: number,
+  txId: string,
   customGasSettings: CustomGasSettings,
   options: { estimatedBaseFee?: string } = {},
 ): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
   log.debug('background.cancelTransaction');
-  let newTxId: number;
+  let newTxId: string;
 
   return (dispatch: MetaMaskReduxDispatch) => {
     const actionId = generateActionId();