fix: notification slowness and crashes (#25946)

## **Description** This is a series of fixes added to core libraries. Adding to extension, and soon we will migrate extension to use shared libraries. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25946?quickstart=1) ## **Related issues** MetaMask/core#4530 MetaMask/core#4531 MetaMask/core#4532 MetaMask/core#4533 MetaMask/core#4536 #25749 ## **Manual testing steps** 1. Create multiple accounts 2. Go to notification settings page 3. Wait for settings to load, and try toggling notifications Before: some settings would error or not load After: you should be able to toggle accounts and not see errors. ## **Screenshots/Recordings**  ### **Before** [](#25749) ### **After** https://www.loom.com/share/49b582e8c33b4199bdafc994a3f6087f?sid=9f94a885-351b-4fee-84d8-f72c97506e7d ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
MetaMask · Jul 19, 2024 · 47ff61d · 47ff61d
1 parent 1b8a65a
commit 47ff61d
Show file tree

Hide file tree

Showing 9 changed files with 233 additions and 117 deletions.
diff --git a/app/scripts/controllers/authentication/authentication-controller.test.ts b/app/scripts/controllers/authentication/authentication-controller.test.ts
@@ -280,11 +280,7 @@ function createMockAuthenticationMessenger() {
       );
     }
 
-    function exhaustedMessengerMocks(action: never) {
-      throw new Error(`MOCK_FAIL - unsupported messenger call: ${action}`);
-    }
-
-    return exhaustedMessengerMocks(actionType);
+    throw new Error(`MOCK_FAIL - unsupported messenger call: ${actionType}`);
   });
 
   return { messenger, mockSnapGetPublicKey, mockSnapSignMessage };

diff --git a/app/scripts/controllers/authentication/authentication-controller.ts b/app/scripts/controllers/authentication/authentication-controller.ts
@@ -4,7 +4,6 @@ import {
   StateMetadata,
 } from '@metamask/base-controller';
 import { HandleSnapRequest } from '@metamask/snaps-controllers';
-import { UserStorageControllerDisableProfileSyncing } from '../user-storage/user-storage-controller';
 import {
   createSnapPublicKeyRequest,
   createSnapSignMessageRequest,
@@ -84,9 +83,7 @@ export type AuthenticationControllerGetSessionProfile =
 export type AuthenticationControllerIsSignedIn = ActionsObj['isSignedIn'];
 
 // Allowed Actions
-export type AllowedActions =
-  | HandleSnapRequest
-  | UserStorageControllerDisableProfileSyncing;
+export type AllowedActions = HandleSnapRequest;
 
 // Messenger
 export type AuthenticationControllerMessenger = RestrictedControllerMessenger<
@@ -271,8 +268,6 @@ export default class AuthenticationController extends BaseController<
       };
     } catch (e) {
       console.error('Failed to authenticate', e);
-      // Disable Profile Syncing
-      this.messagingSystem.call('UserStorageController:disableProfileSyncing');
       const errorMessage =
         e instanceof Error ? e.message : JSON.stringify(e ?? '');
       throw new Error(
@@ -299,28 +294,48 @@ export default class AuthenticationController extends BaseController<
     return THIRTY_MIN_MS > diffMs;
   }
 
+  #_snapPublicKeyCache: string | undefined;
+
   /**
    * Returns the auth snap public key.
    *
    * @returns The snap public key.
    */
-  #snapGetPublicKey(): Promise<string> {
-    return this.messagingSystem.call(
+  async #snapGetPublicKey(): Promise<string> {
+    if (this.#_snapPublicKeyCache) {
+      return this.#_snapPublicKeyCache;
+    }
+
+    const result = (await this.messagingSystem.call(
       'SnapController:handleRequest',
       createSnapPublicKeyRequest(),
-    ) as Promise<string>;
+    )) as string;
+
+    this.#_snapPublicKeyCache = result;
+
+    return result;
   }
 
+  #_snapSignMessageCache: Record<`metamask:${string}`, string> = {};
+
   /**
    * Signs a specific message using an underlying auth snap.
    *
    * @param message - A specific tagged message to sign.
    * @returns A Signature created by the snap.
    */
-  #snapSignMessage(message: `metamask:${string}`): Promise<string> {
-    return this.messagingSystem.call(
+  async #snapSignMessage(message: `metamask:${string}`): Promise<string> {
+    if (this.#_snapSignMessageCache[message]) {
+      return this.#_snapSignMessageCache[message];
+    }
+
+    const result = (await this.messagingSystem.call(
       'SnapController:handleRequest',
       createSnapSignMessageRequest(message),
-    ) as Promise<string>;
+    )) as string;
+
+    this.#_snapSignMessageCache[message] = result;
+
+    return result;
   }
 }
diff --git a/app/scripts/controllers/metamask-notifications/metamask-notifications.ts b/app/scripts/controllers/metamask-notifications/metamask-notifications.ts
@@ -266,22 +266,34 @@ export class MetamaskNotificationsController extends BaseController<
 
   #pushNotifications = {
     enablePushNotifications: async (UUIDs: string[]) => {
-      return await this.messagingSystem.call(
-        'PushPlatformNotificationsController:enablePushNotifications',
-        UUIDs,
-      );
+      try {
+        await this.messagingSystem.call(
+          'PushPlatformNotificationsController:enablePushNotifications',
+          UUIDs,
+        );
+      } catch (e) {
+        log.error('Silently failed to enable push notifications', e);
+      }
     },
     disablePushNotifications: async (UUIDs: string[]) => {
-      return await this.messagingSystem.call(
-        'PushPlatformNotificationsController:disablePushNotifications',
-        UUIDs,
-      );
+      try {
+        await this.messagingSystem.call(
+          'PushPlatformNotificationsController:disablePushNotifications',
+          UUIDs,
+        );
+      } catch (e) {
+        log.error('Silently failed to disable push notifications', e);
+      }
     },
     updatePushNotifications: async (UUIDs: string[]) => {
-      return await this.messagingSystem.call(
-        'PushPlatformNotificationsController:updateTriggerPushNotifications',
-        UUIDs,
-      );
+      try {
+        await this.messagingSystem.call(
+          'PushPlatformNotificationsController:updateTriggerPushNotifications',
+          UUIDs,
+        );
+      } catch (e) {
+        log.error('Silently failed to update push notifications', e);
+      }
     },
     subscribe: () => {
       this.messagingSystem.subscribe(

diff --git a/app/scripts/controllers/metamask-notifications/services/feature-announcements.ts b/app/scripts/controllers/metamask-notifications/services/feature-announcements.ts
@@ -26,7 +26,7 @@ export type ContentfulResult = {
 
 async function fetchFromContentful(
   url: string,
-  retries = 3,
+  retries = 1,
   retryDelay = 1000,
 ): Promise<ContentfulResult | null> {
   let lastError: Error | null = null;
@@ -113,10 +113,17 @@ async function fetchFeatureAnnouncementNotifications(): Promise<
 export async function getFeatureAnnouncementNotifications(): Promise<
   Notification[]
 > {
-  const rawNotifications = await fetchFeatureAnnouncementNotifications();
-  const notifications = rawNotifications.map((notification) =>
-    processFeatureAnnouncement(notification),
-  );
+  if (
+    process.env.CONTENTFUL_ACCESS_SPACE_ID &&
+    process.env.CONTENTFUL_ACCESS_TOKEN
+  ) {
+    const rawNotifications = await fetchFeatureAnnouncementNotifications();
+    const notifications = rawNotifications.map((notification) =>
+      processFeatureAnnouncement(notification),
+    );
+
+    return notifications;
+  }
 
-  return notifications;
+  return [];
 }
diff --git a/app/scripts/controllers/metamask-notifications/utils/utils.ts b/app/scripts/controllers/metamask-notifications/utils/utils.ts
@@ -603,7 +603,7 @@ export async function makeApiCall<T>(
   endpoint: string,
   method: 'POST' | 'DELETE',
   body: T,
-  retries = 3,
+  retries = 1,
   retryDelay = 1000,
 ): Promise<Response> {
   const options: RequestInit = {

diff --git a/app/scripts/controllers/user-storage/user-storage-controller.ts b/app/scripts/controllers/user-storage/user-storage-controller.ts
@@ -365,17 +365,27 @@ export default class UserStorageController extends BaseController<
     return storageKey;
   }
 
+  #_snapSignMessageCache: Record<`metamask:${string}`, string> = {};
+
   /**
    * Signs a specific message using an underlying auth snap.
    *
    * @param message - A specific tagged message to sign.
    * @returns A Signature created by the snap.
    */
-  #snapSignMessage(message: `metamask:${string}`): Promise<string> {
-    return this.messagingSystem.call(
+  async #snapSignMessage(message: `metamask:${string}`): Promise<string> {
+    if (this.#_snapSignMessageCache[message]) {
+      return this.#_snapSignMessageCache[message];
+    }
+
+    const result = (await this.messagingSystem.call(
       'SnapController:handleRequest',
       createSnapSignMessageRequest(message),
-    ) as Promise<string>;
+    )) as string;
+
+    this.#_snapSignMessageCache[message] = result;
+
+    return result;
   }
 
   async #setIsProfileSyncingUpdateLoading(

diff --git a/ui/hooks/metamask-notifications/useSwitchNotifications.ts b/ui/hooks/metamask-notifications/useSwitchNotifications.ts
@@ -1,5 +1,5 @@
-import { useState, useCallback } from 'react';
-import { useDispatch } from 'react-redux';
+import { useState, useCallback, useMemo, useEffect } from 'react';
+import { useDispatch, useSelector } from 'react-redux';
 import log from 'loglevel';
 import {
   setFeatureAnnouncementsEnabled,
@@ -8,6 +8,7 @@ import {
   updateOnChainTriggersByAccount,
   hideLoadingIndication,
 } from '../../store/actions';
+import { getIsUpdatingMetamaskNotificationsAccount } from '../../selectors/metamask-notifications/metamask-notifications';
 
 export function useSwitchFeatureAnnouncementsChange(): {
   onChange: (state: boolean) => Promise<void>;
@@ -114,3 +115,75 @@ export function useSwitchAccountNotificationsChange(): {
     error,
   };
 }
+
+function useRefetchAccountSettings() {
+  const dispatch = useDispatch();
+
+  const getAccountSettings = useCallback(async (accounts: string[]) => {
+    try {
+      const result = (await dispatch(
+        checkAccountsPresence(accounts),
+      )) as unknown as UseSwitchAccountNotificationsData;
+
+      return result;
+    } catch {
+      return {};
+    }
+  }, []);
+
+  return getAccountSettings;
+}
+
+/**
+ * Account Settings Hook.
+ * Gets initial loading states, and returns enable/disable account states.
+ * Also exposes an update() method so each switch can be manually updated.
+ *
+ * @param accounts - the accounts we are checking to see if notifications are enabled/disabled
+ * @returns props for settings page
+ */
+export function useAccountSettingsProps(accounts: string[]) {
+  const accountsBeingUpdated = useSelector(
+    getIsUpdatingMetamaskNotificationsAccount,
+  );
+  const fetchAccountSettings = useRefetchAccountSettings();
+  const [data, setData] = useState<UseSwitchAccountNotificationsData>({});
+  const [loading, setLoading] = useState<boolean>(false);
+  const [error, setError] = useState<string | null>(null);
+
+  // Memoize the accounts array to avoid unnecessary re-fetching
+  const jsonAccounts = useMemo(() => JSON.stringify(accounts), [accounts]);
+
+  const update = useCallback(async (addresses: string[]) => {
+    try {
+      setLoading(true);
+      setError(null);
+      const res = await fetchAccountSettings(addresses);
+      setData(res);
+    } catch {
+      setError('Failed to get account settings');
+    } finally {
+      setLoading(false);
+    }
+  }, []);
+
+  // Effect - async get if accounts are enabled/disabled
+  useEffect(() => {
+    try {
+      const memoAccounts: string[] = JSON.parse(jsonAccounts);
+      update(memoAccounts);
+    } catch {
+      setError('Failed to get account settings');
+    } finally {
+      setLoading(false);
+    }
+  }, [jsonAccounts, fetchAccountSettings]);
+
+  return {
+    data,
+    initialLoading: loading,
+    error,
+    accountsBeingUpdated,
+    update,
+  };
+}