From b4dda49dd1335a6c01c953c153583e99b986b957 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 23 Aug 2024 16:48:12 -0400 Subject: [PATCH] Fix controllers with missing or incorrect messenger action/event types (#4633) ## Explanation Fixes controllers and messengers in the core repo that do not fulfill their intended specifications correctly: - [Define the `*:getState` action using the `ControllerGetStateAction` utility type](https://github.com/MetaMask/core/blob/add-controller-guidelines/docs/writing-controllers.md#define-the-getstate-action-using-the-controllergetstateaction-utility-type) - [Define the `*:stateChange` event using the `ControllerStateChangeEvent` utility type](https://github.com/MetaMask/core/blob/add-controller-guidelines/docs/writing-controllers.md#define-the-statechange-event-using-the-controllerstatechangeevent-utility-type) - [Define and export a type union for internal action types](https://github.com/MetaMask/core/blob/add-controller-guidelines/docs/writing-controllers.md#define-and-export-a-type-union-for-internal-action-types) - [Define and export a type union for internal event types](https://github.com/MetaMask/core/blob/add-controller-guidelines/docs/writing-controllers.md#define-and-export-a-type-union-for-internal-event-types) - [Define and export a type for the controller's messenger](https://github.com/MetaMask/core/blob/add-controller-guidelines/docs/writing-controllers.md#define-and-export-a-type-for-the-controllers-messenger) This also resolves downstream errors in mobile caused by `composable-controller` expecting all of its child controllers to have a `stateChange` event. - See https://github.com/MetaMask/metamask-mobile/pull/10374 ## References - Fixes https://github.com/MetaMask/core/issues/4579 - Blocks https://github.com/MetaMask/metamask-mobile/pull/10441 ## Changelog ### `@metamask/logging-controller` (major) ```md ### Added - Define and export new types: `LoggingControllerGetStateAction`, `LoggingControllerStateChangeEvent`, `LoggingControllerEvents` ([#4633](https://github.com/MetaMask/core/pull/4633)) ### Changed - **BREAKING:** `LoggingControllerMessenger` must allow internal events defined in the `LoggingControllerEvents` type ([#4633](https://github.com/MetaMask/core/pull/4633)) - `LoggingControllerActions` is widened to include the `LoggingController:getState` action ([#4633](https://github.com/MetaMask/core/pull/4633)) ``` ### `@metamask/phishing-controller` (major) ```md ### Added - Define and export new types: `PhishingControllerGetStateAction`, `PhishingControllerStateChangeEvent`, `PhishingControllerEvents` ([#4633](https://github.com/MetaMask/core/pull/4633)) ### Changed - **BREAKING:** `PhishingControllerMessenger` must allow internal events defined in the `PhishingControllerEvents` type ([#4633](https://github.com/MetaMask/core/pull/4633)) - `PhishingControllerActions` is widened to include the `PhishingController:getState` action ([#4633](https://github.com/MetaMask/core/pull/4633)) ``` ### `@metamask/notification-services-controller` (minor) ```md ### Added - Define and export new type `NotificationServicesControllerGetStateAction` ([#4633](https://github.com/MetaMask/core/pull/4633)) ### Fixed - Replace `getState` action in `NotificationServicesControllerActions` with correctly-defined `NotificationServicesControllerGetStateAction` type ([#4633](https://github.com/MetaMask/core/pull/4633)) ``` ### `@metamask/authentication-controller` (major) ```md ### Added - Define and export new types: `AuthenticationControllerGetStateAction`, `AuthenticationControllerStateChangeEvent`, `Events` ([#4633](https://github.com/MetaMask/core/pull/4633)) ### Changed - **BREAKING:** `AuthenticationControllerMessenger` must allow internal events defined in the `Events` type ([#4633](https://github.com/MetaMask/core/pull/4633)) - `AuthenticationControllerActions` is widened to include the `AuthenticationController:getState` action ([#4633](https://github.com/MetaMask/core/pull/4633)) ``` ### `@metamask/user-storage-controller` (major) ```md ### Added - Define and export new types: `UserStorageControllerGetStateAction`, `UserStorageControllerStateChangeEvent`, `Events` ([#4633](https://github.com/MetaMask/core/pull/4633)) ### Changed - **BREAKING:** `UserStorageControllerMessenger` must allow internal events defined in the `Events` type ([#4633](https://github.com/MetaMask/core/pull/4633)) - `UserStorageControllerActions` is widened to include the `UserStorageController:getState` action ([#4633](https://github.com/MetaMask/core/pull/4633)) ``` ## 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 --- .../src/LoggingController.ts | 26 ++++++++++++++----- .../NotificationServicesController.ts | 10 +++++-- .../src/PhishingController.ts | 25 +++++++++++++++--- .../AuthenticationController.ts | 20 ++++++++++++-- .../user-storage/UserStorageController.ts | 18 +++++++++++-- 5 files changed, 83 insertions(+), 16 deletions(-) diff --git a/packages/logging-controller/src/LoggingController.ts b/packages/logging-controller/src/LoggingController.ts index 150711afd4..502fbedd4c 100644 --- a/packages/logging-controller/src/LoggingController.ts +++ b/packages/logging-controller/src/LoggingController.ts @@ -1,4 +1,8 @@ -import type { RestrictedControllerMessenger } from '@metamask/base-controller'; +import type { + ControllerGetStateAction, + ControllerStateChangeEvent, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; import { v1 as random } from 'uuid'; @@ -36,16 +40,24 @@ export type AddLog = { handler: LoggingController['add']; }; -/** - * Currently only an alias, but the idea here is if future actions are needed - * this can transition easily into a union type. - */ -export type LoggingControllerActions = AddLog; +export type LoggingControllerGetStateAction = ControllerGetStateAction< + typeof name, + LoggingControllerState +>; + +export type LoggingControllerActions = LoggingControllerGetStateAction | AddLog; + +export type LoggingControllerStateChangeEvent = ControllerStateChangeEvent< + typeof name, + LoggingControllerState +>; + +export type LoggingControllerEvents = LoggingControllerStateChangeEvent; export type LoggingControllerMessenger = RestrictedControllerMessenger< typeof name, LoggingControllerActions, - never, + LoggingControllerEvents, never, never >; diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts index 480ecbb23b..09ce1819f9 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts @@ -168,6 +168,12 @@ export const defaultState: NotificationServicesControllerState = { isCheckingAccountsPresence: false, }; +export type NotificationServicesControllerGetStateAction = + ControllerGetStateAction< + typeof controllerName, + NotificationServicesControllerState + >; + export type NotificationServicesControllerUpdateMetamaskNotificationsList = { type: `${typeof controllerName}:updateMetamaskNotificationsList`; handler: NotificationServicesController['updateMetamaskNotificationsList']; @@ -186,10 +192,10 @@ export type NotificationServicesControllerSelectIsNotificationServicesEnabled = // Messenger Actions export type Actions = + | NotificationServicesControllerGetStateAction | NotificationServicesControllerUpdateMetamaskNotificationsList | NotificationServicesControllerDisableNotificationServices - | NotificationServicesControllerSelectIsNotificationServicesEnabled - | ControllerGetStateAction<'state', NotificationServicesControllerState>; + | NotificationServicesControllerSelectIsNotificationServicesEnabled; // Allowed Actions export type AllowedActions = diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index aa0690b622..bdeaee75c8 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -1,4 +1,8 @@ -import type { RestrictedControllerMessenger } from '@metamask/base-controller'; +import type { + ControllerGetStateAction, + ControllerStateChangeEvent, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; import { safelyExecute } from '@metamask/controller-utils'; import { toASCII } from 'punycode/'; @@ -227,12 +231,27 @@ export type TestOrigin = { handler: PhishingController['test']; }; -export type PhishingControllerActions = MaybeUpdateState | TestOrigin; +export type PhishingControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + PhishingControllerState +>; + +export type PhishingControllerActions = + | PhishingControllerGetStateAction + | MaybeUpdateState + | TestOrigin; + +export type PhishingControllerStateChangeEvent = ControllerStateChangeEvent< + typeof controllerName, + PhishingControllerState +>; + +export type PhishingControllerEvents = PhishingControllerStateChangeEvent; export type PhishingControllerMessenger = RestrictedControllerMessenger< typeof controllerName, PhishingControllerActions, - never, + PhishingControllerEvents, never, never >; diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 351dbda53d..2d518c5f1f 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -1,4 +1,6 @@ import type { + ControllerGetStateAction, + ControllerStateChangeEvent, RestrictedControllerMessenger, StateMetadata, } from '@metamask/base-controller'; @@ -81,7 +83,13 @@ type ActionsObj = CreateActionsObj< | 'getSessionProfile' | 'isSignedIn' >; -export type Actions = ActionsObj[keyof ActionsObj]; +export type Actions = + | ActionsObj[keyof ActionsObj] + | AuthenticationControllerGetStateAction; +export type AuthenticationControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + AuthenticationControllerState +>; export type AuthenticationControllerPerformSignIn = ActionsObj['performSignIn']; export type AuthenticationControllerPerformSignOut = ActionsObj['performSignOut']; @@ -91,6 +99,14 @@ export type AuthenticationControllerGetSessionProfile = ActionsObj['getSessionProfile']; export type AuthenticationControllerIsSignedIn = ActionsObj['isSignedIn']; +export type AuthenticationControllerStateChangeEvent = + ControllerStateChangeEvent< + typeof controllerName, + AuthenticationControllerState + >; + +export type Events = AuthenticationControllerStateChangeEvent; + // Allowed Actions export type AllowedActions = | HandleSnapRequest @@ -104,7 +120,7 @@ export type AllowedEvents = export type AuthenticationControllerMessenger = RestrictedControllerMessenger< typeof controllerName, Actions | AllowedActions, - AllowedEvents, + Events | AllowedEvents, AllowedActions['type'], AllowedEvents['type'] >; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts index f4d542ee03..498d84ec25 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts @@ -1,4 +1,6 @@ import type { + ControllerGetStateAction, + ControllerStateChangeEvent, RestrictedControllerMessenger, StateMetadata, } from '@metamask/base-controller'; @@ -79,7 +81,13 @@ type ActionsObj = CreateActionsObj< | 'enableProfileSyncing' | 'disableProfileSyncing' >; -export type Actions = ActionsObj[keyof ActionsObj]; +export type UserStorageControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + UserStorageControllerState +>; +export type Actions = + | ActionsObj[keyof ActionsObj] + | UserStorageControllerGetStateAction; export type UserStorageControllerPerformGetStorage = ActionsObj['performGetStorage']; export type UserStorageControllerPerformSetStorage = @@ -90,6 +98,12 @@ export type UserStorageControllerEnableProfileSyncing = export type UserStorageControllerDisableProfileSyncing = ActionsObj['disableProfileSyncing']; +export type UserStorageControllerStateChangeEvent = ControllerStateChangeEvent< + typeof controllerName, + UserStorageControllerState +>; +export type Events = UserStorageControllerStateChangeEvent; + // Allowed Actions export type AllowedActions = // Keyring Requests @@ -114,7 +128,7 @@ export type AllowedEvents = export type UserStorageControllerMessenger = RestrictedControllerMessenger< typeof controllerName, Actions | AllowedActions, - AllowedEvents, + Events | AllowedEvents, AllowedActions['type'], AllowedEvents['type'] >;