Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix controllers with missing or incorrect messenger action/event types #4633

Merged
merged 7 commits into from
Aug 23, 2024

Conversation

MajorLift
Copy link
Contributor

@MajorLift MajorLift commented Aug 21, 2024

Explanation

Fixes controllers and messengers in the core repo that do not fulfill their intended specifications correctly:

This also resolves downstream errors in mobile caused by composable-controller expecting all of its child controllers to have a stateChange event.

References

Changelog

@metamask/logging-controller (major)

### 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)

### 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)

### 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)

### 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)

### 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

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@MajorLift MajorLift self-assigned this Aug 21, 2024
@MajorLift MajorLift requested review from a team as code owners August 21, 2024 17:08
@MajorLift MajorLift marked this pull request as draft August 21, 2024 17:08
@MajorLift MajorLift requested a review from a team August 21, 2024 17:24
@MajorLift MajorLift marked this pull request as ready for review August 21, 2024 17:25
Copy link
Contributor

@AugmentedMode AugmentedMode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MajorLift MajorLift merged commit b4dda49 into main Aug 23, 2024
116 checks passed
@MajorLift MajorLift deleted the fix/controller-api,messenger-types branch August 23, 2024 20:48
MajorLift added a commit that referenced this pull request Aug 28, 2024
…ion/events (#4641)

## Explanation

Fixes various errors in the `NotificationServicesPushController`:

- [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)

## References

- Fixes #4579
- See #4633

## Changelog

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

### Added

- Add and export types
`NotificationServicesPushControllerGetStateAction`,
`NotificationServicesPushControllerStateChangeEvent`
([#4641](#4641))

### Changed

- **BREAKING:** Rename
`NotificationServicesPushControllerPushNotificationClicked` type to
`NotificationServicesPushControllerPushNotificationClickedEvent`
([#4641](#4641))
- **BREAKING:** Narrow `AllowedEvents` type for
`NotificationServicesPushControllerMessenger` to `never`
([#4641](#4641))
- `NotificationServicesPushControllerMessenger` must allow internal
event `NotificationServicesPushControllerStateChangeEvent`
([#4641](#4641))

### Fixed

- **BREAKING:** Fix package-level export for
`NotificationServicesPushController` from
"NotificationsServicesPushController" to
"NotificationsServicesPushController"
([#4641](#4641))
- **BREAKING:** Replace incorrectly-defined `getState` action in the
`Actions` type for `NotificationServicesPushControllerMessenger` with
new `NotificationServicesPushControllerGetStateAction` type
([#4641](#4641))

## 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
@AugmentedMode AugmentedMode mentioned this pull request Aug 28, 2024
3 tasks
AugmentedMode added a commit that referenced this pull request Aug 28, 2024
## Explanation

### Dependency Updates:
- **BREAKING**: Added `@noble/hashes` dependency version `^1.4.0`.
- **BREAKING**: Added `ethereum-cryptography` dependency version
`^2.1.2`.

These dependencies were added to support hashing C2 requests for
comparison against a C2 blocklist, enhancing the security and
cryptographic capabilities within the package.

## Changelog

### `@metamask/phishing-controller`

#### Breaking
- **BREAKING**: Added `@noble/hashes` dependency version `^1.4.0`.
- **BREAKING**: Added `ethereum-cryptography` dependency version
`^2.1.2`.
- **BREAKING**: `PhishingControllerMessenger` must allow internal events
defined in the `PhishingControllerEvents` type
([#4633](#4633)).

#### Added
- **ADDED**: Added the ability to block Client Side C2 Requests by
managing a hashed C2 Request Blocklist.

- **ADDED**: Defined and exported new types:
`PhishingControllerGetStateAction`,
`PhishingControllerStateChangeEvent`, `PhishingControllerEvents`
([#4633](#4633)).
- Added `requestBlocklist` type to `ListTypes`.
- **ADDED**: `isBlockedRequest` method to `PhishingController`.
- **ADDED**: Logic to update and check `requestBlocklist` in
`#updateStalelist`.
- **ADDED**: `isMaliciousRequestDomain` method to `PhishingDetector`.
- **ADDED**: Handling of `requestBlocklist` in `PhishingDetector`
configuration.
- **ADDED**: `sha256` and `toHex` imports from `ethereum-cryptography`.
- **ADDED**: `sha256Hash` function to generate SHA-256 hash of a domain.
- **ADDED**: Allowlist functionality to the C2 domain detection system.

#### Changed
- `PhishingControllerActions` is widened to include the
`PhishingController:getState` action
([#4633](#4633)).
- Bumped `@metamask/base-controller` from `^6.0.2` to `^6.0.3`
([#4625](#4625)).
- Bumped `@metamask/controller-utils` from `^11.0.2` to `^11.1.0`
([#4639](#4639)).

## 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.

---------

Co-authored-by: Jongsun Suh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix controllers with missing or incorrect messenger-related types
3 participants