Skip to content

Commit

Permalink
Fix controllers with missing or incorrect messenger action/event types (
Browse files Browse the repository at this point in the history
#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 MetaMask/metamask-mobile#10374

## References

- Fixes #4579
- Blocks MetaMask/metamask-mobile#10441

## Changelog

### `@metamask/logging-controller` (major)

```md
### Added

- Define and export new types: `LoggingControllerGetStateAction`, `LoggingControllerStateChangeEvent`, `LoggingControllerEvents` ([#4633](#4633))

### Changed

- **BREAKING:** `LoggingControllerMessenger` must allow internal events defined in the `LoggingControllerEvents` type ([#4633](#4633))
- `LoggingControllerActions` is widened to include the `LoggingController:getState` action ([#4633](#4633))
```

### `@metamask/phishing-controller` (major)

```md
### Added

- Define and export new types: `PhishingControllerGetStateAction`, `PhishingControllerStateChangeEvent`, `PhishingControllerEvents` ([#4633](#4633))

### Changed

- **BREAKING:** `PhishingControllerMessenger` must allow internal events defined in the `PhishingControllerEvents` type ([#4633](#4633))
- `PhishingControllerActions` is widened to include the `PhishingController:getState` action ([#4633](#4633))
```

### `@metamask/notification-services-controller` (minor)

```md
### Added

- Define and export new type `NotificationServicesControllerGetStateAction` ([#4633](#4633))

### Fixed

- Replace `getState` action in `NotificationServicesControllerActions` with correctly-defined `NotificationServicesControllerGetStateAction` type ([#4633](#4633))
```

### `@metamask/authentication-controller` (major)

```md
### Added

- Define and export new types: `AuthenticationControllerGetStateAction`, `AuthenticationControllerStateChangeEvent`, `Events` ([#4633](#4633))

### Changed

- **BREAKING:** `AuthenticationControllerMessenger` must allow internal events defined in the `Events` type ([#4633](#4633))
- `AuthenticationControllerActions` is widened to include the `AuthenticationController:getState` action ([#4633](#4633))
```

### `@metamask/user-storage-controller` (major)

```md
### Added

- Define and export new types: `UserStorageControllerGetStateAction`, `UserStorageControllerStateChangeEvent`, `Events` ([#4633](#4633))

### Changed

- **BREAKING:** `UserStorageControllerMessenger` must allow internal events defined in the `Events` type ([#4633](#4633))
- `UserStorageControllerActions` is widened to include the `UserStorageController:getState` action ([#4633](#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
  • Loading branch information
MajorLift authored Aug 23, 2024
1 parent cfb51b6 commit b4dda49
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 16 deletions.
26 changes: 19 additions & 7 deletions packages/logging-controller/src/LoggingController.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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
>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand All @@ -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 =
Expand Down
25 changes: 22 additions & 3 deletions packages/phishing-controller/src/PhishingController.ts
Original file line number Diff line number Diff line change
@@ -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/';
Expand Down Expand Up @@ -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
>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type {
ControllerGetStateAction,
ControllerStateChangeEvent,
RestrictedControllerMessenger,
StateMetadata,
} from '@metamask/base-controller';
Expand Down Expand Up @@ -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'];
Expand All @@ -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
Expand All @@ -104,7 +120,7 @@ export type AllowedEvents =
export type AuthenticationControllerMessenger = RestrictedControllerMessenger<
typeof controllerName,
Actions | AllowedActions,
AllowedEvents,
Events | AllowedEvents,
AllowedActions['type'],
AllowedEvents['type']
>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type {
ControllerGetStateAction,
ControllerStateChangeEvent,
RestrictedControllerMessenger,
StateMetadata,
} from '@metamask/base-controller';
Expand Down Expand Up @@ -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 =
Expand All @@ -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
Expand All @@ -114,7 +128,7 @@ export type AllowedEvents =
export type UserStorageControllerMessenger = RestrictedControllerMessenger<
typeof controllerName,
Actions | AllowedActions,
AllowedEvents,
Events | AllowedEvents,
AllowedActions['type'],
AllowedEvents['type']
>;
Expand Down

0 comments on commit b4dda49

Please sign in to comment.