feat: use withKeyring to get notification accounts for main keyring (#5459)

## Explanation

For notification settings, we only want to fetch accounts that our on
the main keyring.

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/notification-services-controller`

- **CHANGED**: Replaced `KeyringController:getAccounts` with
`KeyringController:withKeyring`

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

Author: Prithpal-Sooriya

packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts

@@ -110,10 +110,10 @@ describe('metamask-notifications - constructor()', () => {
   });
 
   it('keyring Change Event but feature not enabled will not add or remove triggers', async () => {
-    const { messenger, globalMessenger, mockListAccounts } = arrangeMocks();
+    const { messenger, globalMessenger, mockWithKeyring } = arrangeMocks();
 
     // initialize controller with 1 address
-    mockListAccounts.mockResolvedValueOnce([ADDRESS_1]);
+    mockWithKeyring.mockResolvedValueOnce([ADDRESS_1]);
     const controller = new NotificationServicesController({
       messenger,
       env: { featureAnnouncements: featureAnnouncementsEnv },
@@ -127,15 +127,15 @@ describe('metamask-notifications - constructor()', () => {
       .mockResolvedValue({} as UserStorage);
 
     // listAccounts has a new address
-    mockListAccounts.mockResolvedValueOnce([ADDRESS_1, ADDRESS_2]);
+    mockWithKeyring.mockResolvedValueOnce([ADDRESS_1, ADDRESS_2]);
     await actPublishKeyringStateChange(globalMessenger);
 
     expect(mockUpdate).not.toHaveBeenCalled();
     expect(mockDelete).not.toHaveBeenCalled();
   });
 
   it('keyring Change Event with new triggers will update triggers correctly', async () => {
-    const { messenger, globalMessenger, mockListAccounts } = arrangeMocks();
+    const { messenger, globalMessenger, mockWithKeyring } = arrangeMocks();
 
     // initialize controller with 1 address
     const controller = new NotificationServicesController({
@@ -155,7 +155,7 @@ describe('metamask-notifications - constructor()', () => {
       .mockResolvedValue({} as UserStorage);
 
     const act = async (addresses: string[], assertion: () => void) => {
-      mockListAccounts.mockResolvedValueOnce(addresses);
+      mockWithKeyring.mockResolvedValueOnce(addresses);
       await actPublishKeyringStateChange(globalMessenger);
       await waitFor(() => {
         assertion();
@@ -184,9 +184,9 @@ describe('metamask-notifications - constructor()', () => {
       expect(mockDelete).toHaveBeenCalled();
     });
 
-    // If the address is added back to the list, because it is seen we won't update
+    // If the address is added back to the list, we will perform an update
     await act([ADDRESS_1, ADDRESS_2], () => {
-      expect(mockUpdate).not.toHaveBeenCalled();
+      expect(mockUpdate).toHaveBeenCalled();
       expect(mockDelete).not.toHaveBeenCalled();
     });
   });
@@ -300,42 +300,43 @@ describe('metamask-notifications - constructor()', () => {
   };
 
   it('should initialse accounts to track notifications on', async () => {
-    const { mockListAccounts } =
+    const { mockWithKeyring } =
       arrangeActInitialiseNotificationAccountTracking();
     await waitFor(() => {
-      expect(mockListAccounts).toHaveBeenCalled();
+      expect(mockWithKeyring).toHaveBeenCalled();
     });
   });
 
   it('should not initialise accounts if wallet is locked', async () => {
-    const { mockListAccounts } =
-      arrangeActInitialiseNotificationAccountTracking((mocks) => {
+    const { mockWithKeyring } = arrangeActInitialiseNotificationAccountTracking(
+      (mocks) => {
         mocks.mockKeyringControllerGetState.mockReturnValue({
           isUnlocked: false,
         } as MockVar);
-      });
+      },
+    );
     await waitFor(() => {
-      expect(mockListAccounts).not.toHaveBeenCalled();
+      expect(mockWithKeyring).not.toHaveBeenCalled();
     });
   });
 
   it('should re-initialise if the wallet was locked, and then unlocked', async () => {
     // Test Wallet Locked
-    const { globalMessenger, mockListAccounts } =
+    const { globalMessenger, mockWithKeyring } =
       arrangeActInitialiseNotificationAccountTracking((mocks) => {
         mocks.mockKeyringControllerGetState.mockReturnValue({
           isUnlocked: false,
         } as MockVar);
       });
     await waitFor(() => {
-      expect(mockListAccounts).not.toHaveBeenCalled();
+      expect(mockWithKeyring).not.toHaveBeenCalled();
     });
 
     // Test Wallet Unlock
     jest.clearAllMocks();
     globalMessenger.publish('KeyringController:unlock');
     await waitFor(() => {
-      expect(mockListAccounts).toHaveBeenCalled();
+      expect(mockWithKeyring).toHaveBeenCalled();
     });
   });
 });
@@ -927,7 +928,7 @@ describe('metamask-notifications - enableMetamaskNotifications()', () => {
 
   it('should sign a user in if not already signed in', async () => {
     const mocks = arrangeMocks();
-    mocks.mockListAccounts.mockResolvedValue([ADDRESS_1]);
+    mocks.mockWithKeyring.mockResolvedValue([ADDRESS_1]);
     mocks.mockIsSignedIn.mockReturnValue(false); // mock that auth is not enabled
     const controller = new NotificationServicesController({
       messenger: mocks.messenger,
@@ -943,7 +944,7 @@ describe('metamask-notifications - enableMetamaskNotifications()', () => {
 
   it('create new notifications when switched on and no new notifications', async () => {
     const mocks = arrangeMocks();
-    mocks.mockListAccounts.mockResolvedValue([ADDRESS_1]);
+    mocks.mockWithKeyring.mockResolvedValue([ADDRESS_1]);
     const controller = new NotificationServicesController({
       messenger: mocks.messenger,
       env: { featureAnnouncements: featureAnnouncementsEnv },
@@ -966,7 +967,7 @@ describe('metamask-notifications - enableMetamaskNotifications()', () => {
 
   it('not create new notifications when enabling an account already in storage', async () => {
     const mocks = arrangeMocks();
-    mocks.mockListAccounts.mockResolvedValue([ADDRESS_1]);
+    mocks.mockWithKeyring.mockResolvedValue([ADDRESS_1]);
     const userStorage = createMockFullUserStorage({ address: ADDRESS_1 });
     mocks.mockPerformGetStorage.mockResolvedValue(JSON.stringify(userStorage));
     const controller = new NotificationServicesController({
@@ -1153,7 +1154,7 @@ function mockNotificationMessenger() {
   const messenger = globalMessenger.getRestricted({
     name: 'NotificationServicesController',
     allowedActions: [
-      'KeyringController:getAccounts',
+      'KeyringController:withKeyring',
       'KeyringController:getState',
       'AuthenticationController:getBearerToken',
       'AuthenticationController:isSignedIn',
@@ -1175,7 +1176,7 @@ function mockNotificationMessenger() {
     ],
   });
 
-  const mockListAccounts =
+  const mockWithKeyring =
     typedMockAction<KeyringControllerGetAccountsAction>().mockResolvedValue([]);
 
   const mockGetBearerToken =
@@ -1230,8 +1231,8 @@ function mockNotificationMessenger() {
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const [, ...params]: any[] = args;
 
-    if (actionType === 'KeyringController:getAccounts') {
-      return mockListAccounts();
+    if (actionType === 'KeyringController:withKeyring') {
+      return mockWithKeyring();
     }
 
     if (actionType === 'KeyringController:getState') {
@@ -1299,7 +1300,7 @@ function mockNotificationMessenger() {
   return {
     globalMessenger,
     messenger,
-    mockListAccounts,
+    mockWithKeyring,
     mockGetBearerToken,
     mockIsSignedIn,
     mockAuthPerformSignIn,

packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts

@@ -9,12 +9,13 @@ import {
   isValidHexAddress,
   toChecksumHexAddress,
 } from '@metamask/controller-utils';
-import type {
-  KeyringControllerGetAccountsAction,
-  KeyringControllerStateChangeEvent,
-  KeyringControllerGetStateAction,
-  KeyringControllerLockEvent,
-  KeyringControllerUnlockEvent,
+import {
+  type KeyringControllerStateChangeEvent,
+  type KeyringControllerGetStateAction,
+  type KeyringControllerLockEvent,
+  type KeyringControllerUnlockEvent,
+  type KeyringControllerWithKeyringAction,
+  KeyringTypes,
 } from '@metamask/keyring-controller';
 import type {
   AuthenticationController,
@@ -199,7 +200,7 @@ export type Actions =
 // Allowed Actions
 export type AllowedActions =
   // Keyring Controller Requests
-  | KeyringControllerGetAccountsAction
+  | KeyringControllerWithKeyringAction
   | KeyringControllerGetStateAction
   // Auth Controller Requests
   | AuthenticationController.AuthenticationControllerGetBearerToken
@@ -408,16 +409,30 @@ export default class NotificationServicesController extends BaseController<
     // Flag to ensure we only setup once
     isNotificationAccountsSetup: false,
 
+    getNotificationAccounts: async () => {
+      const mainHDWalletAccounts = (await this.messagingSystem.call(
+        'KeyringController:withKeyring',
+        {
+          type: KeyringTypes.hd,
+          index: 0,
+        },
+        async ({ keyring }): Promise<string[]> => {
+          return await keyring.getAccounts();
+        },
+      )) as string[];
+
+      return mainHDWalletAccounts;
+    },
+
     /**
      * Used to get list of addresses from keyring (wallet addresses)
      *
      * @returns addresses removed, added, and latest list of addresses
      */
     listAccounts: async () => {
       // Get previous and current account sets
-      const nonChecksumAccounts = await this.messagingSystem.call(
-        'KeyringController:getAccounts',
-      );
+      const nonChecksumAccounts =
+        await this.#accounts.getNotificationAccounts();
       const accounts = nonChecksumAccounts
         .map((a) => toChecksumHexAddress(a))
         .filter((a) => isValidHexAddress(a));
@@ -442,7 +457,7 @@ export default class NotificationServicesController extends BaseController<
 
       // Update accounts seen
       this.update((state) => {
-        state.subscriptionAccountsSeen = [...prevAccountsSet, ...accountsAdded];
+        state.subscriptionAccountsSeen = [...currentAccountsSet];
       });
 
       return {