feat: Refactor Transaction Confirmation selector (#18796)

app/scripts/background.js

@@ -713,7 +713,9 @@ export function setupController(
   // User Interface setup
   //
 
-  updateBadge();
+  controller.txController.initApprovals().then(() => {
+    updateBadge();
+  });
   controller.txController.on(
     METAMASK_CONTROLLER_EVENTS.UPDATE_BADGE,
     updateBadge,

app/scripts/controllers/transactions/index.js

@@ -394,6 +394,22 @@ export default class TransactionController extends EventEmitter {
     });
   }
 
+  /**
+   * Creates approvals for all unapproved transactions in the txStateManager.
+   *
+   * @returns {Promise<void>}
+   */
+  async initApprovals() {
+    const unapprovedTxs = this.txStateManager.getUnapprovedTxList();
+    return Promise.all(
+      Object.values(unapprovedTxs).map((txMeta) =>
+        this._requestApproval(txMeta, {
+          shouldShowRequest: false,
+        }),
+      ),
+    );
+  }
+
   // ====================================================================================================================================================
 
   /**
@@ -2646,13 +2662,16 @@ export default class TransactionController extends EventEmitter {
     );
   }
 
-  _requestApproval(txMeta) {
+  async _requestApproval(
+    txMeta,
+    { shouldShowRequest } = { shouldShowRequest: true },
+  ) {
     const id = this._getApprovalId(txMeta);
     const { origin } = txMeta;
     const type = ApprovalType.Transaction;
     const requestData = { txId: txMeta.id };
 
-    this.messagingSystem
+    return this.messagingSystem
       .call(
         'ApprovalController:addRequest',
         {
@@ -2661,7 +2680,7 @@ export default class TransactionController extends EventEmitter {
           type,
           requestData,
         },
-        true,
+        shouldShowRequest,
       )
       .catch(() => {
         // Intentionally ignored as promise not currently used

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

@@ -2992,4 +2992,58 @@ describe('Transaction Controller', function () {
       assert.equal(result.type, TransactionType.simpleSend);
     });
   });
+
+  describe('initApprovals', function () {
+    it('adds unapprovedTxs as approvals', async function () {
+      const firstTxId = '1';
+      txController.addTransaction(
+        {
+          id: firstTxId,
+          origin: ORIGIN_METAMASK,
+          status: TransactionStatus.unapproved,
+          metamaskNetworkId: currentNetworkId,
+          txParams: {
+            to: VALID_ADDRESS,
+            from: VALID_ADDRESS_TWO,
+          },
+        },
+        noop,
+      );
+      const secondTxId = '2';
+      txController.addTransaction(
+        {
+          id: secondTxId,
+          origin: ORIGIN_METAMASK,
+          status: TransactionStatus.unapproved,
+          metamaskNetworkId: currentNetworkId,
+          txParams: {
+            to: VALID_ADDRESS,
+            from: VALID_ADDRESS_TWO,
+          },
+        },
+        noop,
+      );
+      await txController.initApprovals();
+      assert.deepEqual(messengerMock.call.getCall(0).args, [
+        'ApprovalController:addRequest',
+        {
+          id: firstTxId,
+          origin: ORIGIN_METAMASK,
+          requestData: { txId: firstTxId },
+          type: ApprovalType.Transaction,
+        },
+        false,
+      ]);
+      assert.deepEqual(messengerMock.call.getCall(1).args, [
+        'ApprovalController:addRequest',
+        {
+          id: secondTxId,
+          origin: ORIGIN_METAMASK,
+          requestData: { txId: secondTxId },
+          type: ApprovalType.Transaction,
+        },
+        false,
+      ]);
+    });
+  });
 });

ui/helpers/utils/confirm-tx.util.ts

@@ -2,7 +2,6 @@ import currencyFormatter from 'currency-formatter';
 import currencies from 'currency-formatter/currencies';
 import { BigNumber } from 'bignumber.js';
 
-import { unconfirmedTransactionsCountSelector } from '../../selectors';
 import { Numeric } from '../../../shared/modules/Numeric';
 import { EtherDenomination } from '../../../shared/constants/common';
 import { TransactionMeta } from '../../../shared/constants/transaction';
@@ -86,22 +85,6 @@ export function convertTokenToFiat({
   return tokenInFiat.round(2).toString();
 }
 
-/**
- * This is a selector and probably doesn't belong here but its staying for now
- * Note: I did not go so far as to type the entirety of the MetaMask state tree
- * which definitely needs to be done for the full conversion of TypeScript to
- * be successful and as useful as possible.
- * TODO: Type the MetaMask state tree and use that type here.
- *
- * @param state - MetaMask state
- * @returns true if there are unconfirmed transactions in state
- */
-export function hasUnconfirmedTransactions(
-  state: Record<string, any>,
-): boolean {
-  return unconfirmedTransactionsCountSelector(state) > 0;
-}
-
 /**
  * Rounds the given decimal string to 4 significant digits.
  *

ui/pages/home/home.component.js

@@ -107,7 +107,7 @@ export default class Home extends PureComponent {
     history: PropTypes.object,
     forgottenPassword: PropTypes.bool,
     hasWatchAssetPendingApprovals: PropTypes.bool,
-    unconfirmedTransactionsCount: PropTypes.number,
+    hasTransactionPendingApprovals: PropTypes.bool.isRequired,
     shouldShowSeedPhraseReminder: PropTypes.bool.isRequired,
     isPopup: PropTypes.bool,
     isNotification: PropTypes.bool.isRequired,
@@ -197,7 +197,7 @@ export default class Home extends PureComponent {
       showAwaitingSwapScreen,
       hasWatchAssetPendingApprovals,
       swapsFetchParams,
-      unconfirmedTransactionsCount,
+      hasTransactionPendingApprovals,
     } = this.props;
 
     if (shouldCloseNotificationPopup(props)) {
@@ -205,7 +205,7 @@ export default class Home extends PureComponent {
       closeNotificationPopup();
     } else if (
       firstPermissionsRequestId ||
-      unconfirmedTransactionsCount > 0 ||
+      hasTransactionPendingApprovals ||
       hasWatchAssetPendingApprovals ||
       (!isNotification &&
         (showAwaitingSwapScreen || haveSwapsQuotes || swapsFetchParams))
@@ -269,7 +269,7 @@ export default class Home extends PureComponent {
       history,
       isNotification,
       hasWatchAssetPendingApprovals,
-      unconfirmedTransactionsCount,
+      hasTransactionPendingApprovals,
       haveSwapsQuotes,
       showAwaitingSwapScreen,
       swapsFetchParams,
@@ -288,7 +288,7 @@ export default class Home extends PureComponent {
       history.push(BUILD_QUOTE_ROUTE);
     } else if (firstPermissionsRequestId) {
       history.push(`${CONNECT_ROUTE}/${firstPermissionsRequestId}`);
-    } else if (unconfirmedTransactionsCount > 0) {
+    } else if (hasTransactionPendingApprovals) {
       history.push(CONFIRM_TRANSACTION_ROUTE);
     } else if (hasWatchAssetPendingApprovals) {
       history.push(CONFIRM_ADD_SUGGESTED_TOKEN_ROUTE);

ui/pages/home/home.container.js

@@ -22,7 +22,6 @@ import {
   getTotalUnapprovedCount,
   getUnapprovedTemplatedConfirmations,
   getWeb3ShimUsageStateForOrigin,
-  unconfirmedTransactionsCountSelector,
   getInfuraBlocked,
   getShowWhatsNewPopup,
   getSortedAnnouncementsToShow,
@@ -36,7 +35,7 @@ import {
   getNewTokensImported,
   getShouldShowSeedPhraseReminder,
   getRemoveNftMessage,
-  hasPendingApprovalsSelector,
+  hasPendingApprovals,
 } from '../../selectors';
 
 import {
@@ -71,6 +70,7 @@ import {
   AlertTypes,
   Web3ShimUsageAlertStates,
 } from '../../../shared/constants/alerts';
+import { hasTransactionPendingApprovals } from '../../selectors/transactions';
 import Home from './home.component';
 
 const mapStateToProps = (state) => {
@@ -121,7 +121,7 @@ const mapStateToProps = (state) => {
     hasUnsignedQRHardwareTransaction(state) ||
     hasUnsignedQRHardwareMessage(state);
 
-  const hasWatchAssetPendingApprovals = hasPendingApprovalsSelector(
+  const hasWatchAssetPendingApprovals = hasPendingApprovals(
     state,
     ApprovalType.WatchAsset,
   );
@@ -130,7 +130,7 @@ const mapStateToProps = (state) => {
     forgottenPassword,
     hasWatchAssetPendingApprovals,
     swapsEnabled,
-    unconfirmedTransactionsCount: unconfirmedTransactionsCountSelector(state),
+    hasTransactionPendingApprovals: hasTransactionPendingApprovals(state),
     shouldShowSeedPhraseReminder: getShouldShowSeedPhraseReminder(state),
     isPopup,
     isNotification,

ui/selectors/approvals.test.ts

@@ -1,10 +1,10 @@
 import { ApprovalType } from '@metamask/controller-utils';
-import { hasPendingApprovalsSelector } from './approvals';
+import { hasPendingApprovals } from './approvals';
 
 describe('approval selectors', () => {
   const mockedState = {
     metamask: {
-      pendingApprovalCount: 2,
+      pendingApprovalCount: 3,
       pendingApprovals: {
         '1': {
           id: '1',
@@ -18,28 +18,30 @@ describe('approval selectors', () => {
           id: '2',
           origin: 'origin',
           time: Date.now(),
-          type: ApprovalType.EthSignTypedData,
+          type: ApprovalType.Transaction,
           requestData: {},
           requestState: null,
         },
       },
+      unapprovedTxs: {
+        '2': {
+          id: '2',
+        },
+      },
     },
   };
 
-  describe('hasPendingApprovalsSelector', () => {
+  describe('hasPendingApprovals', () => {
     it('should return true if there is a pending approval request', () => {
-      const result = hasPendingApprovalsSelector(
+      const result = hasPendingApprovals(mockedState, ApprovalType.WatchAsset);
-        mockedState,
-        ApprovalType.WatchAsset,
-      );
 
       expect(result).toBe(true);
     });
 
     it('should return false if there is no pending approval request', () => {
-      const result = hasPendingApprovalsSelector(
+      const result = hasPendingApprovals(
         mockedState,
-        ApprovalType.Transaction,
+        ApprovalType.SnapDialogPrompt,
       );
 
       expect(result).toBe(false);

ui/selectors/approvals.ts

@@ -1,19 +1,35 @@
 import { ApprovalControllerState } from '@metamask/approval-controller';
 import { ApprovalType } from '@metamask/controller-utils';
+import { TransactionMeta } from '../../shared/constants/transaction';
 
 type ApprovalsMetaMaskState = {
   metamask: {
     pendingApprovals: ApprovalControllerState['pendingApprovals'];
+    unapprovedTxs: {
+      [transactionId: string]: TransactionMeta;
+    };
   };
 };
 
-export function hasPendingApprovalsSelector(
+export const getApprovalRequestsByType = (
   state: ApprovalsMetaMaskState,
   approvalType: ApprovalType,
-) {
+) => {
   const pendingApprovalRequests = Object.values(
     state.metamask.pendingApprovals,
   ).filter(({ type }) => type === approvalType);
 
+  return pendingApprovalRequests;
+};
+
+export function hasPendingApprovals(
+  state: ApprovalsMetaMaskState,
+  approvalType: ApprovalType,
+) {
+  const pendingApprovalRequests = getApprovalRequestsByType(
+    state,
+    approvalType,
+  );
+
   return pendingApprovalRequests.length > 0;
 }

ui/selectors/confirm-transaction.js

@@ -142,50 +142,6 @@ export const unconfirmedMessagesHashSelector = createSelector(
   },
 );
 
-const unapprovedMsgCountSelector = (state) => state.metamask.unapprovedMsgCount;
-const unapprovedPersonalMsgCountSelector = (state) =>
-  state.metamask.unapprovedPersonalMsgCount;
-const unapprovedDecryptMsgCountSelector = (state) =>
-  state.metamask.unapprovedDecryptMsgCount;
-const unapprovedEncryptionPublicKeyMsgCountSelector = (state) =>
-  state.metamask.unapprovedEncryptionPublicKeyMsgCount;
-const unapprovedTypedMessagesCountSelector = (state) =>
-  state.metamask.unapprovedTypedMessagesCount;
-
-export const unconfirmedTransactionsCountSelector = createSelector(
-  unapprovedTxsSelector,
-  unapprovedMsgCountSelector,
-  unapprovedPersonalMsgCountSelector,
-  unapprovedDecryptMsgCountSelector,
-  unapprovedEncryptionPublicKeyMsgCountSelector,
-  unapprovedTypedMessagesCountSelector,
-  deprecatedGetCurrentNetworkId,
-  getCurrentChainId,
-  (
-    unapprovedTxs = {},
-    unapprovedMsgCount = 0,
-    unapprovedPersonalMsgCount = 0,
-    unapprovedDecryptMsgCount = 0,
-    unapprovedEncryptionPublicKeyMsgCount = 0,
-    unapprovedTypedMessagesCount = 0,
-    network,
-    chainId,
-  ) => {
-    const filteredUnapprovedTxIds = Object.keys(unapprovedTxs).filter((txId) =>
-      transactionMatchesNetwork(unapprovedTxs[txId], chainId, network),
-    );
-
-    return (
-      filteredUnapprovedTxIds.length +
-      unapprovedTypedMessagesCount +
-      unapprovedMsgCount +
-      unapprovedPersonalMsgCount +
-      unapprovedDecryptMsgCount +
-      unapprovedEncryptionPublicKeyMsgCount
-    );
-  },
-);
-
 export const currentCurrencySelector = (state) =>
   state.metamask.currentCurrency;
 export const conversionRateSelector = (state) => state.metamask.conversionRate;

ui/selectors/confirm-transaction.test.js

@@ -1,7 +1,5 @@
-import { CHAIN_IDS } from '../../shared/constants/network';
 import { TransactionType } from '../../shared/constants/transaction';
 import {
-  unconfirmedTransactionsCountSelector,
   sendTokenTokenAmountAndToAddressSelector,
   contractExchangeRateSelector,
   conversionRateSelector,
@@ -17,32 +15,6 @@ const getEthersArrayLikeFromObj = (obj) => {
 };
 
 describe('Confirm Transaction Selector', () => {
-  describe('unconfirmedTransactionsCountSelector', () => {
-    const state = {
-      metamask: {
-        unapprovedTxs: {
-          1: {
-            metamaskNetworkId: '5',
-          },
-          2: {
-            chainId: CHAIN_IDS.MAINNET,
-          },
-        },
-        unapprovedMsgCount: 1,
-        unapprovedPersonalMsgCount: 1,
-        unapprovedTypedMessagesCount: 1,
-        networkId: '5',
-        providerConfig: {
-          chainId: '0x5',
-        },
-      },
-    };
-
-    it('returns number of txs in unapprovedTxs state with the same network plus unapproved signing method counts', () => {
-      expect(unconfirmedTransactionsCountSelector(state)).toStrictEqual(4);
-    });
-  });
-
   describe('sendTokenTokenAmountAndToAddressSelector', () => {
     const state = {
       confirmTransaction: {

ui/selectors/transactions.js

@@ -1,4 +1,5 @@
 import { createSelector } from 'reselect';
+import { ApprovalType } from '@metamask/controller-utils';
 import {
   PRIORITY_STATUS_HASH,
   PENDING_STATUS_HASH,
@@ -17,6 +18,7 @@ import {
   deprecatedGetCurrentNetworkId,
   getSelectedAddress,
 } from './selectors';
+import { hasPendingApprovals, getApprovalRequestsByType } from './approvals';
 
 const INVALID_INITIAL_TRANSACTION_TYPES = [
   TransactionType.cancel,
@@ -534,3 +536,34 @@ export const submittedPendingTransactionsSelector = createSelector(
       (transaction) => transaction.status === TransactionStatus.submitted,
     ),
 );
+
+const hasUnapprovedTransactionsInCurrentNetwork = (state) => {
+  const { unapprovedTxs } = state.metamask;
+  const unapprovedTxRequests = getApprovalRequestsByType(
+    state,
+    ApprovalType.Transaction,
+  );
+
+  const chainId = getCurrentChainId(state);
+
+  const filteredUnapprovedTxInCurrentNetwork = unapprovedTxRequests.filter(
+    ({ id }) => transactionMatchesNetwork(unapprovedTxs[id], chainId),
+  );
+
+  return filteredUnapprovedTxInCurrentNetwork.length > 0;
+};
+
+const TRANSACTION_APPROVAL_TYPES = [
+  ApprovalType.EthDecrypt,
+  ApprovalType.EthGetEncryptionPublicKey,
+  ApprovalType.EthSign,
+  ApprovalType.EthSignTypedData,
+  ApprovalType.PersonalSign,
+];
+
+export function hasTransactionPendingApprovals(state) {
+  return (
+    hasUnapprovedTransactionsInCurrentNetwork(state) ||
+    TRANSACTION_APPROVAL_TYPES.some((type) => hasPendingApprovals(state, type))
+  );
+}

ui/selectors/transactions.test.js

@@ -1,3 +1,4 @@
+import { ApprovalType } from '@metamask/controller-utils';
 import { CHAIN_IDS } from '../../shared/constants/network';
 import { TransactionStatus } from '../../shared/constants/transaction';
 import {
@@ -7,6 +8,7 @@ import {
   nonceSortedPendingTransactionsSelector,
   nonceSortedCompletedTransactionsSelector,
   submittedPendingTransactionsSelector,
+  hasTransactionPendingApprovals,
 } from './transactions';
 
 describe('Transaction Selectors', () => {
@@ -329,4 +331,78 @@ describe('Transaction Selectors', () => {
       );
     });
   });
+
+  describe('hasTransactionPendingApprovals', () => {
+    const mockNetworkId = 'mockNetworkId';
+    const mockedState = {
+      metamask: {
+        providerConfig: {
+          chainId: mockNetworkId,
+        },
+        pendingApprovalCount: 2,
+        pendingApprovals: {
+          1: {
+            id: '1',
+            origin: 'origin',
+            time: Date.now(),
+            type: ApprovalType.WatchAsset,
+            requestData: {},
+            requestState: null,
+          },
+          2: {
+            id: '2',
+            origin: 'origin',
+            time: Date.now(),
+            type: ApprovalType.Transaction,
+            requestData: {},
+            requestState: null,
+          },
+        },
+        unapprovedTxs: {
+          2: {
+            id: '2',
+            chainId: mockNetworkId,
+          },
+        },
+      },
+    };
+
+    it('should return true if there is a pending transaction on same network', () => {
+      const result = hasTransactionPendingApprovals(mockedState);
+      expect(result).toBe(true);
+    });
+    it('should return false if there is a pending transaction on different network', () => {
+      mockedState.metamask.unapprovedTxs['2'].chainId = 'differentNetworkId';
+      const result = hasTransactionPendingApprovals(mockedState);
+      expect(result).toBe(false);
+    });
+    it.each([
+      [ApprovalType.EthDecrypt],
+      [ApprovalType.EthGetEncryptionPublicKey],
+      [ApprovalType.EthSign],
+      [ApprovalType.EthSignTypedData],
+      [ApprovalType.PersonalSign],
+    ])(
+      'should return true if there is a pending transaction of %s type',
+      (type) => {
+        const result = hasTransactionPendingApprovals({
+          ...mockedState,
+          metamask: {
+            ...mockedState.metamask,
+            pendingApprovals: {
+              2: {
+                id: '2',
+                origin: 'origin',
+                time: Date.now(),
+                type,
+                requestData: {},
+                requestState: null,
+              },
+            },
+          },
+        });
+        expect(result).toBe(true);
+      },
+    );
+  });
 });

ui/store/actions.ts

@@ -23,12 +23,12 @@ import {
   POLLING_TOKEN_ENVIRONMENT_TYPES,
   MESSAGE_TYPE,
 } from '../../shared/constants/app';
-import { hasUnconfirmedTransactions } from '../helpers/utils/confirm-tx.util';
 import { getEnvironmentType, addHexPrefix } from '../../app/scripts/lib/util';
 import {
   getMetaMaskAccounts,
   getPermittedAccountsForCurrentTab,
   getSelectedAddress,
+  hasTransactionPendingApprovals,
   ///: BEGIN:ONLY_INCLUDE_IN(snaps)
   getNotifications,
   ///: END:ONLY_INCLUDE_IN
@@ -2698,7 +2698,7 @@ export function closeCurrentNotificationWindow(): ThunkAction<
   return (_, getState) => {
     if (
       getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION &&
-      !hasUnconfirmedTransactions(getState())
+      !hasTransactionPendingApprovals(getState())
     ) {
       closeNotificationPopup();
     }