From ab0925076f4516b14ab7ffa8e3e22097bc0d15aa Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 11:19:11 +0000 Subject: [PATCH 01/12] Feat: support all accounts for all keyrings --- .../NotificationServicesController.test.ts | 52 +++++++++++++++++++ .../NotificationServicesController.ts | 8 +-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts index c09a7f953af..dc6d301f394 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts @@ -498,6 +498,58 @@ 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('does not register notifications when notifications already exist and not resetting (however does update push registrations)', async () => { const { messenger, diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts index 957ff38a450..ad3c0d81023 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts +++ b/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, @@ -399,11 +398,8 @@ 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 = keyrings.flatMap((keyring) => keyring.accounts); + return keyringAccounts.length > 0 ? keyringAccounts : null; }, /** From bb924b41f817a39c73b47072fa69134f38ba3914 Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 11:38:41 +0000 Subject: [PATCH 02/12] Feat: unregister FCM tokens when SRPs are removed --- .../NotificationServicesController.test.ts | 23 ++++++++- .../NotificationServicesController.ts | 14 +++++ ...NotificationServicesPushController.test.ts | 40 +++++++++++++++ .../NotificationServicesPushController.ts | 39 +++++++++++++- .../__fixtures__/mockServices.ts | 19 ++++++- .../mocks/mockResponse.ts | 10 +++- .../services/services.test.ts | 37 +++++++++++++- .../services/services.ts | 51 +++++++++++++++++++ 8 files changed, 228 insertions(+), 5 deletions(-) diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts index dc6d301f394..075d3d7c339 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts +++ b/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'; @@ -670,7 +671,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 }, @@ -679,6 +684,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 () => { @@ -1606,6 +1612,7 @@ function mockNotificationMessenger(): { mockIsSignedIn: jest.Mock; mockAuthPerformSignIn: jest.Mock; mockDisablePushNotifications: jest.Mock; + mockDeletePushNotificationLinks: jest.Mock; mockEnablePushNotifications: jest.Mock; mockSubscribeToPushNotifications: jest.Mock; mockKeyringControllerGetState: jest.Mock; @@ -1630,6 +1637,7 @@ function mockNotificationMessenger(): { 'AuthenticationController:isSignedIn', 'AuthenticationController:performSignIn', 'NotificationServicesPushController:disablePushNotifications', + 'NotificationServicesPushController:deletePushNotificationLinks', 'NotificationServicesPushController:enablePushNotifications', 'NotificationServicesPushController:subscribeToPushNotifications', ], @@ -1660,6 +1668,11 @@ function mockNotificationMessenger(): { const mockDisablePushNotifications = typedMockAction(); + const mockDeletePushNotificationLinks = + typedMockAction().mockResolvedValue( + true, + ); + const mockEnablePushNotifications = typedMockAction(); @@ -1708,6 +1721,13 @@ function mockNotificationMessenger(): { return mockDisablePushNotifications(); } + if ( + actionType === + 'NotificationServicesPushController:deletePushNotificationLinks' + ) { + return mockDeletePushNotificationLinks(params[0]); + } + if ( actionType === 'NotificationServicesPushController:enablePushNotifications' @@ -1734,6 +1754,7 @@ function mockNotificationMessenger(): { mockIsSignedIn, mockAuthPerformSignIn, mockDisablePushNotifications, + mockDeletePushNotificationLinks, mockEnablePushNotifications, mockSubscribeToPushNotifications, mockKeyringControllerGetState, diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts index ad3c0d81023..1310847a60b 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts @@ -44,6 +44,7 @@ import type { OrderInput } from './types/perps'; import type { NotificationServicesPushControllerEnablePushNotificationsAction, NotificationServicesPushControllerDisablePushNotificationsAction, + NotificationServicesPushControllerDeletePushNotificationLinksAction, NotificationServicesPushControllerSubscribeToNotificationsAction, NotificationServicesPushControllerStateChangeEvent, NotificationServicesPushControllerOnNewNotificationEvent, @@ -233,6 +234,7 @@ type AllowedActions = // Push Notifications Controller Requests | NotificationServicesPushControllerEnablePushNotificationsAction | NotificationServicesPushControllerDisablePushNotificationsAction + | NotificationServicesPushControllerDeletePushNotificationLinksAction | NotificationServicesPushControllerSubscribeToNotificationsAction; // Events @@ -357,6 +359,16 @@ export default class NotificationServicesController extends BaseController< // Do nothing, failing silently. } }, + deletePushNotificationLinks: async (addresses: string[]): Promise => { + try { + await this.messenger.call( + 'NotificationServicesPushController:deletePushNotificationLinks', + addresses, + ); + } catch { + // Do nothing, failing silently. + } + }, subscribe: (): void => { this.messenger.subscribe( 'NotificationServicesPushController:onNewNotifications', @@ -946,6 +958,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 { diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts index f0d726f2630..ef2ce96dbd8 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts @@ -25,6 +25,7 @@ describe('NotificationServicesPushController', () => { ): { activatePushNotificationsMock: jest.SpyInstance; deactivatePushNotificationsMock: jest.SpyInstance; + deleteLinksAPIMock: jest.SpyInstance; } => { const activatePushNotificationsMock = jest .spyOn(services, 'activatePushNotifications') @@ -34,9 +35,14 @@ describe('NotificationServicesPushController', () => { .spyOn(services, 'deactivatePushNotifications') .mockResolvedValue(true); + const deleteLinksAPIMock = jest + .spyOn(services, 'deleteLinksAPI') + .mockResolvedValue(true); + return { activatePushNotificationsMock, deactivatePushNotificationsMock, + deleteLinksAPIMock, }; }; @@ -280,6 +286,40 @@ describe('NotificationServicesPushController', () => { }); }); + describe('deletePushNotificationLinks', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should call deleteLinksAPI with addresses and platform', async () => { + const mocks = arrangeServicesMocks(); + const { controller, messenger } = arrangeMockMessenger(); + mockAuthBearerTokenCall(messenger); + + const result = await controller.deletePushNotificationLinks(MOCK_ADDRESSES); + + expect(mocks.deleteLinksAPIMock).toHaveBeenCalledWith({ + bearerToken: MOCK_JWT, + addresses: MOCK_ADDRESSES, + platform: 'extension', + env: 'prd', + }); + expect(result).toBe(true); + }); + + it('should return false when push feature is disabled', async () => { + const mocks = arrangeServicesMocks(); + const { controller } = arrangeMockMessenger({ + isPushFeatureEnabled: false, + }); + + const result = await controller.deletePushNotificationLinks(MOCK_ADDRESSES); + + expect(mocks.deleteLinksAPIMock).not.toHaveBeenCalled(); + expect(result).toBe(false); + }); + }); + describe('metadata', () => { it('includes expected state in debug snapshots', () => { const { controller } = arrangeMockMessenger(); diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts index 164a46283ac..4fb61558c2d 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts @@ -11,6 +11,7 @@ import log from 'loglevel'; import type { ENV } from './services/endpoints'; import { activatePushNotifications, + deleteLinksAPI, deactivatePushNotifications, } from './services/services'; import type { PushNotificationEnv } from './types'; @@ -48,13 +49,19 @@ export type NotificationServicesPushControllerSubscribeToNotificationsAction = { type: `${typeof controllerName}:subscribeToPushNotifications`; handler: NotificationServicesPushController['subscribeToPushNotifications']; }; +export type NotificationServicesPushControllerDeletePushNotificationLinksAction = + { + type: `${typeof controllerName}:deletePushNotificationLinks`; + handler: NotificationServicesPushController['deletePushNotificationLinks']; + }; export type Actions = | NotificationServicesPushControllerGetStateAction | NotificationServicesPushControllerEnablePushNotificationsAction | NotificationServicesPushControllerDisablePushNotificationsAction | NotificationServicesPushControllerUpdateTriggerPushNotificationsAction - | NotificationServicesPushControllerSubscribeToNotificationsAction; + | NotificationServicesPushControllerSubscribeToNotificationsAction + | NotificationServicesPushControllerDeletePushNotificationLinksAction; type AllowedActions = AuthenticationController.AuthenticationControllerGetBearerTokenAction; @@ -217,6 +224,10 @@ export default class NotificationServicesPushController extends BaseController< 'NotificationServicesPushController:subscribeToPushNotifications', this.subscribeToPushNotifications.bind(this), ); + this.messenger.registerActionHandler( + 'NotificationServicesPushController:deletePushNotificationLinks', + this.deletePushNotificationLinks.bind(this), + ); } #clearLoadingStates(): void { @@ -383,6 +394,32 @@ export default class NotificationServicesPushController extends BaseController< this.#updatePushState({ type: 'disable' }); } + /** + * Deletes backend push notification links for the given addresses on the current platform. + * This is used when accounts are removed (for example SRP removal), so backend can remove + * all associated FCM tokens for those address/platform pairs. + * + * @param addresses - Addresses that should be unlinked from push notifications. + * @returns Whether the delete request succeeded. + */ + public async deletePushNotificationLinks(addresses: string[]): Promise { + if (!this.#config.isPushFeatureEnabled || addresses.length === 0) { + return false; + } + + try { + const bearerToken = await this.#getAndAssertBearerToken(); + return await deleteLinksAPI({ + bearerToken, + addresses, + platform: this.#config.platform, + env: this.#config.env ?? 'prd', + }); + } catch { + return false; + } + } + /** * Updates the triggers for push notifications. * This method is responsible for updating the server with the new set of addresses that should trigger push notifications. diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts b/packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts index 75129d47d51..c68d2f879f0 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts @@ -1,6 +1,9 @@ import nock from 'nock'; -import { getMockUpdatePushNotificationLinksResponse } from '../mocks/mockResponse'; +import { + getMockDeletePushNotificationLinksResponse, + getMockUpdatePushNotificationLinksResponse, +} from '../mocks/mockResponse'; type MockReply = { status: nock.StatusCode; @@ -20,3 +23,17 @@ export const mockEndpointUpdatePushNotificationLinks = ( return mockEndpoint; }; + +export const mockEndpointDeletePushNotificationLinks = ( + mockReply?: MockReply, +): nock.Scope => { + const mockResponse = getMockDeletePushNotificationLinksResponse(); + const reply = mockReply ?? { + status: 204, + body: mockResponse.response, + }; + + const mockEndpoint = nock(mockResponse.url).delete('').reply(reply.status); + + return mockEndpoint; +}; diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/mocks/mockResponse.ts b/packages/notification-services-controller/src/NotificationServicesPushController/mocks/mockResponse.ts index 9b83fca11b4..f180c48522a 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/mocks/mockResponse.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/mocks/mockResponse.ts @@ -2,7 +2,7 @@ import { REGISTRATION_TOKENS_ENDPOINT } from '../services/endpoints'; type MockResponse = { url: string | RegExp; - requestMethod: 'GET' | 'POST' | 'PUT'; + requestMethod: 'GET' | 'POST' | 'PUT' | 'DELETE'; response: unknown; }; @@ -16,6 +16,14 @@ export const getMockUpdatePushNotificationLinksResponse = (): MockResponse => { } satisfies MockResponse; }; +export const getMockDeletePushNotificationLinksResponse = (): MockResponse => { + return { + url: REGISTRATION_TOKENS_ENDPOINT(), + requestMethod: 'DELETE', + response: null, + } satisfies MockResponse; +}; + export const MOCK_FCM_RESPONSE = { name: '', token: 'fcm-token', diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts b/packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts index 69c3de3a2b5..36c7a66ebdc 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts @@ -3,9 +3,13 @@ import log from 'loglevel'; import { activatePushNotifications, deactivatePushNotifications, + deleteLinksAPI, updateLinksAPI, } from './services'; -import { mockEndpointUpdatePushNotificationLinks } from '../__fixtures__/mockServices'; +import { + mockEndpointDeletePushNotificationLinks, + mockEndpointUpdatePushNotificationLinks, +} from '../__fixtures__/mockServices'; import type { PushNotificationEnv } from '../types/firebase'; // Testing util to clean up verbose logs when testing errors @@ -125,6 +129,37 @@ describe('NotificationServicesPushController Services', () => { }); }); + describe('deleteLinksAPI', () => { + const act = async (): Promise => + await deleteLinksAPI({ + bearerToken: MOCK_JWT, + addresses: MOCK_ADDRESSES, + platform: 'extension', + }); + + it('should return true if links are successfully deleted', async () => { + const mockAPI = mockEndpointDeletePushNotificationLinks(); + const result = await act(); + expect(mockAPI.isDone()).toBe(true); + expect(result).toBe(true); + }); + + it('should return false if the links API delete fails', async () => { + const mockAPI = mockEndpointDeletePushNotificationLinks({ status: 500 }); + const result = await act(); + expect(mockAPI.isDone()).toBe(true); + expect(result).toBe(false); + }); + + it('should return false if an error is thrown', async () => { + jest + .spyOn(global, 'fetch') + .mockRejectedValue(new Error('MOCK FAIL FETCH')); + const result = await act(); + expect(result).toBe(false); + }); + }); + describe('deactivatePushNotifications', () => { // Internal testing utility - return type is inferred // eslint-disable-next-line @typescript-eslint/explicit-function-return-type diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts b/packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts index 45f33a37559..1cd409e93d9 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts @@ -13,6 +13,8 @@ export type RegToken = { oldToken?: string; }; +export type RegistrationPlatform = 'extension' | 'mobile'; + /** * Links API Response Shape */ @@ -28,6 +30,15 @@ export type PushTokenRequest = { }; }; +export type DeletePushTokenRequest = { + addresses: string[]; + // API request uses snake_case for this property + // eslint-disable-next-line @typescript-eslint/naming-convention + registration_token: { + platform: RegistrationPlatform; + }; +}; + type UpdatePushTokenParams = { bearerToken: string; addresses: string[]; @@ -66,6 +77,46 @@ export async function updateLinksAPI( } } +type DeletePushTokenParams = { + bearerToken: string; + addresses: string[]; + platform: RegistrationPlatform; + env?: ENV; +}; + +/** + * Deletes push notification links for addresses and platform. + * + * @param params - params for deleting registration links + * @returns A promise that resolves with true if the delete request was successful, false otherwise. + */ +export async function deleteLinksAPI( + params: DeletePushTokenParams, +): Promise { + try { + const body: DeletePushTokenRequest = { + addresses: params.addresses, + registration_token: { + platform: params.platform, + }, + }; + const response = await fetch( + endpoints.REGISTRATION_TOKENS_ENDPOINT(params.env), + { + method: 'DELETE', + headers: { + Authorization: `Bearer ${params.bearerToken}`, + 'Content-Type': 'application/json', + }, + body: JSON.stringify(body), + }, + ); + return response.ok; + } catch { + return false; + } +} + type ActivatePushNotificationsParams = { // Create Push Token env: PushNotificationEnv; From 1af525bb417c7c863a95a798541dce42ce4b0528 Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 11:56:04 +0000 Subject: [PATCH 03/12] Update Changelog --- packages/notification-services-controller/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/notification-services-controller/CHANGELOG.md b/packages/notification-services-controller/CHANGELOG.md index bf5750cbca9..47195da2ea9 100644 --- a/packages/notification-services-controller/CHANGELOG.md +++ b/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 additional SRP accounts are included in notification setup ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) +- 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) ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) ## [22.0.0] From a19bc42ffb8fea43a411efb4b74e9522d89714f4 Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 12:02:31 +0000 Subject: [PATCH 04/12] Lint --- .../NotificationServicesPushController.test.ts | 6 ++++-- .../NotificationServicesPushController.ts | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts index ef2ce96dbd8..b7087183d69 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts @@ -296,7 +296,8 @@ describe('NotificationServicesPushController', () => { const { controller, messenger } = arrangeMockMessenger(); mockAuthBearerTokenCall(messenger); - const result = await controller.deletePushNotificationLinks(MOCK_ADDRESSES); + const result = + await controller.deletePushNotificationLinks(MOCK_ADDRESSES); expect(mocks.deleteLinksAPIMock).toHaveBeenCalledWith({ bearerToken: MOCK_JWT, @@ -313,7 +314,8 @@ describe('NotificationServicesPushController', () => { isPushFeatureEnabled: false, }); - const result = await controller.deletePushNotificationLinks(MOCK_ADDRESSES); + const result = + await controller.deletePushNotificationLinks(MOCK_ADDRESSES); expect(mocks.deleteLinksAPIMock).not.toHaveBeenCalled(); expect(result).toBe(false); diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts index 4fb61558c2d..ac4d512802d 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts @@ -402,7 +402,9 @@ export default class NotificationServicesPushController extends BaseController< * @param addresses - Addresses that should be unlinked from push notifications. * @returns Whether the delete request succeeded. */ - public async deletePushNotificationLinks(addresses: string[]): Promise { + public async deletePushNotificationLinks( + addresses: string[], + ): Promise { if (!this.#config.isPushFeatureEnabled || addresses.length === 0) { return false; } From 8c41aa054182013b643af7395a56879e08b54b27 Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 12:09:30 +0000 Subject: [PATCH 05/12] Changelog --- packages/notification-services-controller/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/notification-services-controller/CHANGELOG.md b/packages/notification-services-controller/CHANGELOG.md index 47195da2ea9..d86538a4a1f 100644 --- a/packages/notification-services-controller/CHANGELOG.md +++ b/packages/notification-services-controller/CHANGELOG.md @@ -12,8 +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 additional SRP accounts are included in notification setup ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) -- 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) ([#PR_NUMBER](https://github.com/MetaMask/core/pull/PR_NUMBER)) +- Register notification accounts from all keyrings instead of only the first HD keyring, so additional SRP accounts are included in notification setup ([#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] From 4d1d3a723478819cb70f0af4b71caf679b9b7cdc Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 12:45:56 +0000 Subject: [PATCH 06/12] Filter out non-EVM addresses + dedup --- .../NotificationServicesController.test.ts | 52 +++++++++++++++++++ .../NotificationServicesController.ts | 9 +++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts index 075d3d7c339..c905f054588 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts @@ -551,6 +551,58 @@ describe('NotificationServicesController', () => { ]); }); + 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('does not register notifications when notifications already exist and not resetting (however does update push registrations)', async () => { const { messenger, diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts index 1310847a60b..04c216ba8b6 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts @@ -410,7 +410,14 @@ export default class NotificationServicesController extends BaseController< getNotificationAccounts: (): string[] | null => { const { keyrings } = this.messenger.call('KeyringController:getState'); - const keyringAccounts = keyrings.flatMap((keyring) => keyring.accounts); + const keyringAccounts = [ + ...new Set( + keyrings + .flatMap((keyring) => keyring.accounts) + .filter((address) => isValidHexAddress(address)) + .map((address) => toChecksumHexAddress(address)), + ), + ]; return keyringAccounts.length > 0 ? keyringAccounts : null; }, From 8a788a99bc006582cbd575821824fade58db825e Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 12:48:44 +0000 Subject: [PATCH 07/12] Update Changelog --- packages/notification-services-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/notification-services-controller/CHANGELOG.md b/packages/notification-services-controller/CHANGELOG.md index d86538a4a1f..d9da7fa4c58 100644 --- a/packages/notification-services-controller/CHANGELOG.md +++ b/packages/notification-services-controller/CHANGELOG.md @@ -12,7 +12,7 @@ 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 additional SRP accounts are included in notification setup ([#8108](https://github.com/MetaMask/core/pull/8108)) +- 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] From a28b3bba4b27a89a365786c0676e0e4cb833aa5f Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 12:52:55 +0000 Subject: [PATCH 08/12] Lint --- .../NotificationServicesController.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts index c905f054588..dbbb1fdf105 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts @@ -578,7 +578,10 @@ describe('NotificationServicesController', () => { }, }, { - accounts: [ADDRESS_2, '7xKXtg2CW6y7J2wMmkf8VbM8dYb6u3H3V8bLxT64d4oR'], + accounts: [ + ADDRESS_2, + '7xKXtg2CW6y7J2wMmkf8VbM8dYb6u3H3V8bLxT64d4oR', + ], type: KeyringTypes.hd, metadata: { id: 'srp-2', From d1e33490243a8891b11065cacee20c0213bbea9a Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 13:02:56 +0000 Subject: [PATCH 09/12] Checksum first --- .../NotificationServicesController.test.ts | 44 +++++++++++++++++++ .../NotificationServicesController.ts | 13 +++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts index dbbb1fdf105..eca7fc56b42 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts @@ -606,6 +606,50 @@ describe('NotificationServicesController', () => { ]); }); + 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, diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts index 04c216ba8b6..7b98aa6d850 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts @@ -414,8 +414,17 @@ export default class NotificationServicesController extends BaseController< ...new Set( keyrings .flatMap((keyring) => keyring.accounts) - .filter((address) => isValidHexAddress(address)) - .map((address) => toChecksumHexAddress(address)), + .map((address) => { + try { + return toChecksumHexAddress(address); + } catch { + return null; + } + }) + .filter( + (address): address is string => + Boolean(address) && isValidHexAddress(address), + ), ), ]; return keyringAccounts.length > 0 ? keyringAccounts : null; From b9a5b401bb07c359c2218cba1df37914874f47e6 Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 13:06:43 +0000 Subject: [PATCH 10/12] Include full fcm token in DELETE request --- ...NotificationServicesPushController.test.ts | 27 ++++++++++++++++++- .../NotificationServicesPushController.ts | 7 ++++- .../__fixtures__/mockServices.ts | 5 +++- .../services/services.test.ts | 20 ++++++++++++-- .../services/services.ts | 3 +++ 5 files changed, 57 insertions(+), 5 deletions(-) diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts index b7087183d69..bbe34d5c886 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts @@ -293,7 +293,13 @@ describe('NotificationServicesPushController', () => { it('should call deleteLinksAPI with addresses and platform', async () => { const mocks = arrangeServicesMocks(); - const { controller, messenger } = arrangeMockMessenger(); + const { controller, messenger } = arrangeMockMessenger({ + state: { + fcmToken: MOCK_FCM_TOKEN, + isPushEnabled: true, + isUpdatingFCMToken: false, + }, + }); mockAuthBearerTokenCall(messenger); const result = @@ -303,6 +309,7 @@ describe('NotificationServicesPushController', () => { bearerToken: MOCK_JWT, addresses: MOCK_ADDRESSES, platform: 'extension', + token: MOCK_FCM_TOKEN, env: 'prd', }); expect(result).toBe(true); @@ -320,6 +327,24 @@ describe('NotificationServicesPushController', () => { expect(mocks.deleteLinksAPIMock).not.toHaveBeenCalled(); expect(result).toBe(false); }); + + it('should return false when there is no token to delete', async () => { + const mocks = arrangeServicesMocks(); + const { controller, messenger } = arrangeMockMessenger({ + state: { + fcmToken: '', + isPushEnabled: true, + isUpdatingFCMToken: false, + }, + }); + mockAuthBearerTokenCall(messenger); + + const result = + await controller.deletePushNotificationLinks(MOCK_ADDRESSES); + + expect(mocks.deleteLinksAPIMock).not.toHaveBeenCalled(); + expect(result).toBe(false); + }); }); describe('metadata', () => { diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts index ac4d512802d..7d2546c2ed4 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts @@ -405,7 +405,11 @@ export default class NotificationServicesPushController extends BaseController< public async deletePushNotificationLinks( addresses: string[], ): Promise { - if (!this.#config.isPushFeatureEnabled || addresses.length === 0) { + if ( + !this.#config.isPushFeatureEnabled || + addresses.length === 0 || + !this.state.fcmToken + ) { return false; } @@ -415,6 +419,7 @@ export default class NotificationServicesPushController extends BaseController< bearerToken, addresses, platform: this.#config.platform, + token: this.state.fcmToken, env: this.#config.env ?? 'prd', }); } catch { diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts b/packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts index c68d2f879f0..d4610a919ec 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts @@ -26,6 +26,7 @@ export const mockEndpointUpdatePushNotificationLinks = ( export const mockEndpointDeletePushNotificationLinks = ( mockReply?: MockReply, + requestBody?: nock.RequestBodyMatcher, ): nock.Scope => { const mockResponse = getMockDeletePushNotificationLinksResponse(); const reply = mockReply ?? { @@ -33,7 +34,9 @@ export const mockEndpointDeletePushNotificationLinks = ( body: mockResponse.response, }; - const mockEndpoint = nock(mockResponse.url).delete('').reply(reply.status); + const mockEndpoint = nock(mockResponse.url) + .delete('', requestBody) + .reply(reply.status); return mockEndpoint; }; diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts b/packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts index 36c7a66ebdc..97e5103b9f6 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts @@ -135,17 +135,33 @@ describe('NotificationServicesPushController Services', () => { bearerToken: MOCK_JWT, addresses: MOCK_ADDRESSES, platform: 'extension', + token: MOCK_REG_TOKEN, }); it('should return true if links are successfully deleted', async () => { - const mockAPI = mockEndpointDeletePushNotificationLinks(); + const mockAPI = mockEndpointDeletePushNotificationLinks(undefined, { + addresses: MOCK_ADDRESSES, + registration_token: { + platform: 'extension', + token: MOCK_REG_TOKEN, + }, + }); const result = await act(); expect(mockAPI.isDone()).toBe(true); expect(result).toBe(true); }); it('should return false if the links API delete fails', async () => { - const mockAPI = mockEndpointDeletePushNotificationLinks({ status: 500 }); + const mockAPI = mockEndpointDeletePushNotificationLinks( + { status: 500 }, + { + addresses: MOCK_ADDRESSES, + registration_token: { + platform: 'extension', + token: MOCK_REG_TOKEN, + }, + }, + ); const result = await act(); expect(mockAPI.isDone()).toBe(true); expect(result).toBe(false); diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts b/packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts index 1cd409e93d9..26841844a21 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts @@ -36,6 +36,7 @@ export type DeletePushTokenRequest = { // eslint-disable-next-line @typescript-eslint/naming-convention registration_token: { platform: RegistrationPlatform; + token: string; }; }; @@ -81,6 +82,7 @@ type DeletePushTokenParams = { bearerToken: string; addresses: string[]; platform: RegistrationPlatform; + token: string; env?: ENV; }; @@ -98,6 +100,7 @@ export async function deleteLinksAPI( addresses: params.addresses, registration_token: { platform: params.platform, + token: params.token, }, }; const response = await fetch( From f4aaf01a62de49bd06c3f0377ccedefceac599f8 Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 13:10:21 +0000 Subject: [PATCH 11/12] Lint --- .../NotificationServicesController.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts index eca7fc56b42..b2789161ee3 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts @@ -620,7 +620,8 @@ describe('NotificationServicesController', () => { }), }); - const nonChecksummedMixedCaseAddress = '0xd8Da6bf26964af9d7eeD9e03E53415D37aa96045'; + const nonChecksummedMixedCaseAddress = + '0xd8Da6bf26964af9d7eeD9e03E53415D37aa96045'; mockKeyringControllerGetState.mockReturnValue({ isUnlocked: true, From e40e4972c2a16d4e52cce32a4366fe1b39af4242 Mon Sep 17 00:00:00 2001 From: manlio Date: Wed, 4 Mar 2026 13:18:27 +0000 Subject: [PATCH 12/12] Make lint happy --- .../NotificationServicesController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts index 7b98aa6d850..36351586f01 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts @@ -423,7 +423,7 @@ export default class NotificationServicesController extends BaseController< }) .filter( (address): address is string => - Boolean(address) && isValidHexAddress(address), + address !== null && isValidHexAddress(address), ), ), ];