Feat/notify beyond first srp

packages/notification-services-controller/CHANGELOG.md

@@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Debounce `KeyringController:stateChange` handler to reduce redundant notification subscription calls during rapid account syncing ([#7980](https://github.com/MetaMask/core/pull/7980))
 - Filter out Product Account announcements notifications older than 3 months ([#7884](https://github.com/MetaMask/core/pull/7884))
 - Bump `@metamask/controller-utils` from `^11.18.0` to `^11.19.0` ([#7995](https://github.com/MetaMask/core/pull/7995))
+- Register notification accounts from all keyrings instead of only the first HD keyring, so notification setup now includes addresses from HD, hardware, imported, and snap keyrings ([#8108](https://github.com/MetaMask/core/pull/8108))
+- Add push token unlink support for account removal by deleting `/api/v2/token` links for `{ address, platform }` pairs when notification accounts are disabled (for example during SRP removal) ([#8108](https://github.com/MetaMask/core/pull/8108))
 
 ## [22.0.0]
 

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

@@ -47,6 +47,7 @@ import { notificationsConfigCache } from './services/notification-config-cache';
 import type { INotification, OrderInput } from './types';
 import type {
   NotificationServicesPushControllerDisablePushNotificationsAction,
+  NotificationServicesPushControllerDeletePushNotificationLinksAction,
   NotificationServicesPushControllerEnablePushNotificationsAction,
   NotificationServicesPushControllerSubscribeToNotificationsAction,
 } from '../NotificationServicesPushController';
@@ -498,6 +499,158 @@ describe('NotificationServicesController', () => {
       expect(mockEnablePushNotifications).toHaveBeenCalled();
     });
 
+    it('tracks accounts from all keyrings when creating triggers', async () => {
+      const {
+        messenger,
+        mockGetConfig,
+        mockUpdateNotifications,
+        mockKeyringControllerGetState,
+      } = arrangeMocks({
+        // Mock no existing notifications
+        mockGetConfig: () =>
+          mockGetOnChainNotificationsConfig({
+            status: 200,
+            body: [],
+          }),
+      });
+
+      mockKeyringControllerGetState.mockReturnValue({
+        isUnlocked: true,
+        keyrings: [
+          {
+            accounts: [ADDRESS_1],
+            type: KeyringTypes.hd,
+            metadata: {
+              id: 'srp-1',
+              name: 'SRP 1',
+            },
+          },
+          {
+            accounts: [ADDRESS_2],
+            type: KeyringTypes.hd,
+            metadata: {
+              id: 'srp-2',
+              name: 'SRP 2',
+            },
+          },
+        ],
+      });
+
+      const controller = new NotificationServicesController({
+        messenger,
+        env: { featureAnnouncements: featureAnnouncementsEnv },
+      });
+
+      await controller.createOnChainTriggers();
+
+      expect(mockGetConfig.isDone()).toBe(true);
+      expect(mockUpdateNotifications.isDone()).toBe(true);
+      expect(controller.state.subscriptionAccountsSeen).toStrictEqual([
+        ADDRESS_1,
+        ADDRESS_2,
+      ]);
+    });
+
+    it('deduplicates and filters non-Ethereum accounts when creating triggers', async () => {
+      const {
+        messenger,
+        mockGetConfig,
+        mockUpdateNotifications,
+        mockKeyringControllerGetState,
+      } = arrangeMocks({
+        // Mock no existing notifications
+        mockGetConfig: () =>
+          mockGetOnChainNotificationsConfig({
+            status: 200,
+            body: [],
+          }),
+      });
+
+      mockKeyringControllerGetState.mockReturnValue({
+        isUnlocked: true,
+        keyrings: [
+          {
+            accounts: [ADDRESS_1, ADDRESS_1.toLowerCase(), 'NotAnAddress'],
+            type: KeyringTypes.hd,
+            metadata: {
+              id: 'srp-1',
+              name: 'SRP 1',
+            },
+          },
+          {
+            accounts: [
+              ADDRESS_2,
+              '7xKXtg2CW6y7J2wMmkf8VbM8dYb6u3H3V8bLxT64d4oR',
+            ],
+            type: KeyringTypes.hd,
+            metadata: {
+              id: 'srp-2',
+              name: 'SRP 2',
+            },
+          },
+        ],
+      });
+
+      const controller = new NotificationServicesController({
+        messenger,
+        env: { featureAnnouncements: featureAnnouncementsEnv },
+      });
+
+      await controller.createOnChainTriggers();
+
+      expect(mockGetConfig.isDone()).toBe(true);
+      expect(mockUpdateNotifications.isDone()).toBe(true);
+      expect(controller.state.subscriptionAccountsSeen).toStrictEqual([
+        ADDRESS_1,
+        ADDRESS_2,
+      ]);
+    });
+
+    it('normalizes non-checksummed mixed-case addresses before filtering', async () => {
+      const {
+        messenger,
+        mockGetConfig,
+        mockUpdateNotifications,
+        mockKeyringControllerGetState,
+      } = arrangeMocks({
+        mockGetConfig: () =>
+          mockGetOnChainNotificationsConfig({
+            status: 200,
+            body: [],
+          }),
+      });
+
+      const nonChecksummedMixedCaseAddress =
+        '0xd8Da6bf26964af9d7eeD9e03E53415D37aa96045';
+
+      mockKeyringControllerGetState.mockReturnValue({
+        isUnlocked: true,
+        keyrings: [
+          {
+            accounts: [nonChecksummedMixedCaseAddress],
+            type: KeyringTypes.hd,
+            metadata: {
+              id: 'srp-1',
+              name: 'SRP 1',
+            },
+          },
+        ],
+      });
+
+      const controller = new NotificationServicesController({
+        messenger,
+        env: { featureAnnouncements: featureAnnouncementsEnv },
+      });
+
+      await controller.createOnChainTriggers();
+
+      expect(mockGetConfig.isDone()).toBe(true);
+      expect(mockUpdateNotifications.isDone()).toBe(true);
+      expect(controller.state.subscriptionAccountsSeen).toStrictEqual([
+        ADDRESS_1,
+      ]);
+    });
+
     it('does not register notifications when notifications already exist and not resetting (however does update push registrations)', async () => {
       const {
         messenger,
@@ -618,7 +771,11 @@ describe('NotificationServicesController', () => {
     };
 
     it('disables notifications for given accounts', async () => {
-      const { messenger, mockUpdateNotifications } = arrangeMocks();
+      const {
+        messenger,
+        mockUpdateNotifications,
+        mockDeletePushNotificationLinks,
+      } = arrangeMocks();
       const controller = new NotificationServicesController({
         messenger,
         env: { featureAnnouncements: featureAnnouncementsEnv },
@@ -627,6 +784,7 @@ describe('NotificationServicesController', () => {
       await controller.disableAccounts([ADDRESS_1]);
 
       expect(mockUpdateNotifications.isDone()).toBe(true);
+      expect(mockDeletePushNotificationLinks).toHaveBeenCalledWith([ADDRESS_1]);
     });
 
     it('throws errors when invalid auth', async () => {
@@ -1554,6 +1712,7 @@ function mockNotificationMessenger(): {
   mockIsSignedIn: jest.Mock;
   mockAuthPerformSignIn: jest.Mock;
   mockDisablePushNotifications: jest.Mock;
+  mockDeletePushNotificationLinks: jest.Mock;
   mockEnablePushNotifications: jest.Mock;
   mockSubscribeToPushNotifications: jest.Mock;
   mockKeyringControllerGetState: jest.Mock;
@@ -1578,6 +1737,7 @@ function mockNotificationMessenger(): {
       'AuthenticationController:isSignedIn',
       'AuthenticationController:performSignIn',
       'NotificationServicesPushController:disablePushNotifications',
+      'NotificationServicesPushController:deletePushNotificationLinks',
       'NotificationServicesPushController:enablePushNotifications',
       'NotificationServicesPushController:subscribeToPushNotifications',
     ],
@@ -1608,6 +1768,11 @@ function mockNotificationMessenger(): {
   const mockDisablePushNotifications =
     typedMockAction<NotificationServicesPushControllerDisablePushNotificationsAction>();
 
+  const mockDeletePushNotificationLinks =
+    typedMockAction<NotificationServicesPushControllerDeletePushNotificationLinksAction>().mockResolvedValue(
+      true,
+    );
+
   const mockEnablePushNotifications =
     typedMockAction<NotificationServicesPushControllerEnablePushNotificationsAction>();
 
@@ -1656,6 +1821,13 @@ function mockNotificationMessenger(): {
       return mockDisablePushNotifications();
     }
 
+    if (
+      actionType ===
+      'NotificationServicesPushController:deletePushNotificationLinks'
+    ) {
+      return mockDeletePushNotificationLinks(params[0]);
+    }
+
     if (
       actionType ===
       'NotificationServicesPushController:enablePushNotifications'
@@ -1682,6 +1854,7 @@ function mockNotificationMessenger(): {
     mockIsSignedIn,
     mockAuthPerformSignIn,
     mockDisablePushNotifications,
+    mockDeletePushNotificationLinks,
     mockEnablePushNotifications,
     mockSubscribeToPushNotifications,
     mockKeyringControllerGetState,

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

@@ -8,7 +8,6 @@ import {
   isValidHexAddress,
   toChecksumHexAddress,
 } from '@metamask/controller-utils';
-import { KeyringTypes } from '@metamask/keyring-controller';
 import type {
   KeyringControllerStateChangeEvent,
   KeyringControllerGetStateAction,
@@ -45,6 +44,7 @@ import type { OrderInput } from './types/perps';
 import type {
   NotificationServicesPushControllerEnablePushNotificationsAction,
   NotificationServicesPushControllerDisablePushNotificationsAction,
+  NotificationServicesPushControllerDeletePushNotificationLinksAction,
   NotificationServicesPushControllerSubscribeToNotificationsAction,
   NotificationServicesPushControllerStateChangeEvent,
   NotificationServicesPushControllerOnNewNotificationEvent,
@@ -234,6 +234,7 @@ type AllowedActions =
   // Push Notifications Controller Requests
   | NotificationServicesPushControllerEnablePushNotificationsAction
   | NotificationServicesPushControllerDisablePushNotificationsAction
+  | NotificationServicesPushControllerDeletePushNotificationLinksAction
   | NotificationServicesPushControllerSubscribeToNotificationsAction;
 
 // Events
@@ -358,6 +359,16 @@ export default class NotificationServicesController extends BaseController<
         // Do nothing, failing silently.
       }
     },
+    deletePushNotificationLinks: async (addresses: string[]): Promise<void> => {
+      try {
+        await this.messenger.call(
+          'NotificationServicesPushController:deletePushNotificationLinks',
+          addresses,
+        );
+      } catch {
+        // Do nothing, failing silently.
+      }
+    },
     subscribe: (): void => {
       this.messenger.subscribe(
         'NotificationServicesPushController:onNewNotifications',
@@ -399,11 +410,24 @@ export default class NotificationServicesController extends BaseController<
 
     getNotificationAccounts: (): string[] | null => {
       const { keyrings } = this.messenger.call('KeyringController:getState');
-      const firstHDKeyring = keyrings.find(
-        (keyring) => keyring.type === KeyringTypes.hd.toString(),
-      );
-      const keyringAccounts = firstHDKeyring?.accounts ?? null;
-      return keyringAccounts;
+      const keyringAccounts = [
+        ...new Set(
+          keyrings
+            .flatMap((keyring) => keyring.accounts)
+            .map((address) => {
+              try {
+                return toChecksumHexAddress(address);
+              } catch {
+                return null;
+              }
+            })
+            .filter(
+              (address): address is string =>
+                address !== null && isValidHexAddress(address),
+            ),
+        ),
+      ];
+      return keyringAccounts.length > 0 ? keyringAccounts : null;
     },
 
     /**
@@ -950,6 +974,8 @@ export default class NotificationServicesController extends BaseController<
         accounts.map((address) => ({ address, enabled: false })),
         this.#env,
       );
+
+      await this.#pushNotifications.deletePushNotificationLinks(accounts);
     } catch {
       throw new Error('Failed to delete OnChain triggers');
     } finally {