fix: Migrate AbstractMessageManager from BaseControllerV1 to `Bas…

…eControllerV2` (#5103)

## Explanation

<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

This PR aims to remove `BaseControllerV1` usage from
`AbstractMessageManager`. As expected this change affected both
`DecryptMessageManager` (`DMM`) and
`EncryptionPublicKeyManager`(`EPKM`).

Since extension already have wrapper classes for both `DMM` & `EPKM` in
the extension code, we want to keep changes minimal and make these both
classes in the core work like controller but let wrappers sync the state
in their classes as we currently do. You can find the extension PR in
the references below.

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

* Fixes : MetaMask/MetaMask-planning#3747
* Related extension PR:
MetaMask/metamask-extension#29237

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/message-manager`

- **BREAKING:** Base class of `DecryptMessageManager` and
`EncryptionPublicKeyManager`(`AbstractMessageManager`) now expects new
options to initialise
- **BREAKING:** Removed internal event emitter (`hub` property) from
`AbstractMessageManager`
- **BREAKING:** `unapprovedMessage` and `updateBadge` removed from
internal events. These events are now emitted from messaging system
- Controllers should now listen to `DerivedManagerName:X` event instead
of using internal event emitter.


## 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
- [X] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

packages/message-manager/CHANGELOG.md

@@ -9,8 +9,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **BREAKING:** Base class of `DecryptMessageManager` and `EncryptionPublicKeyManager`(`AbstractMessageManager`) now expects new options to initialise ([#5103](https://github.com/MetaMask/core/pull/5103))
 - Bump `@metamask/base-controller` from `^7.0.0` to `^7.1.0` ([#5079](https://github.com/MetaMask/core/pull/5079))
 
+### Removed
+
+- **BREAKING:** Removed internal event emitter (`hub` property) from `AbstractMessageManager` ([#5103](https://github.com/MetaMask/core/pull/5103))
+- **BREAKING:** `unapprovedMessage` and `updateBadge` removed from internal events. These events are now emitted from messaging system ([#5103](https://github.com/MetaMask/core/pull/5103))
+  - Controllers should now listen to `DerivedManagerName:X` event instead of using internal event emitter.
+
 ## [11.0.3]
 
 ### Changed

packages/message-manager/src/AbstractMessageManager.test.ts

@@ -1,3 +1,4 @@
+import type { RestrictedControllerMessenger } from '@metamask/base-controller';
 import { ApprovalType } from '@metamask/controller-utils';
 
 import type {
@@ -20,10 +21,15 @@ type ConcreteMessageParamsMetamask = ConcreteMessageParams & {
   metamaskId?: string;
 };
 
+type ConcreteMessageManagerActions = never;
+type ConcreteMessageManagerEvents = never;
+
 class AbstractTestManager extends AbstractMessageManager<
   ConcreteMessage,
   ConcreteMessageParams,
-  ConcreteMessageParamsMetamask
+  ConcreteMessageParamsMetamask,
+  ConcreteMessageManagerActions,
+  ConcreteMessageManagerEvents
 > {
   addRequestToMessageParams<MessageParams extends AbstractMessageParams>(
     messageParams: MessageParams,
@@ -56,6 +62,26 @@ class AbstractTestManager extends AbstractMessageManager<
   }
 }
 
+const MOCK_MESSENGER = {
+  clearEventSubscriptions: jest.fn(),
+  publish: jest.fn(),
+  registerActionHandler: jest.fn(),
+  registerInitialEventPayload: jest.fn(),
+} as unknown as RestrictedControllerMessenger<
+  'AbstractMessageManager',
+  never,
+  never,
+  string,
+  string
+>;
+
+const MOCK_INITIAL_OPTIONS = {
+  additionalFinishStatuses: undefined,
+  messenger: MOCK_MESSENGER,
+  name: 'AbstractMessageManager' as const,
+  securityProviderRequest: undefined,
+};
+
 const messageId = '1';
 const messageId2 = '2';
 const from = '0x0123';
@@ -78,20 +104,15 @@ const mockMessageParams = { from, test: testData };
 
 describe('AbstractTestManager', () => {
   it('should set default state', () => {
-    const controller = new AbstractTestManager();
+    const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
     expect(controller.state).toStrictEqual({
       unapprovedMessages: {},
       unapprovedMessagesCount: 0,
     });
   });
 
-  it('should set default config', () => {
-    const controller = new AbstractTestManager();
-    expect(controller.config).toStrictEqual({});
-  });
-
   it('should add a valid message', async () => {
-    const controller = new AbstractTestManager();
+    const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
     await controller.addMessage({
       id: messageId,
       messageParams: {
@@ -115,7 +136,7 @@ describe('AbstractTestManager', () => {
   });
 
   it('should get all messages', async () => {
-    const controller = new AbstractTestManager();
+    const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
     const message = {
       id: messageId,
       messageParams: {
@@ -148,11 +169,10 @@ describe('AbstractTestManager', () => {
     const securityProviderRequestMock: SecurityProviderRequest = jest
       .fn()
       .mockResolvedValue(securityProviderResponseMock);
-    const controller = new AbstractTestManager(
-      undefined,
-      undefined,
-      securityProviderRequestMock,
-    );
+    const controller = new AbstractTestManager({
+      ...MOCK_INITIAL_OPTIONS,
+      securityProviderRequest: securityProviderRequestMock,
+    });
     await controller.addMessage({
       id: messageId,
       messageParams: {
@@ -180,7 +200,7 @@ describe('AbstractTestManager', () => {
   });
 
   it('should reject a message', async () => {
-    const controller = new AbstractTestManager();
+    const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
     await controller.addMessage({
       id: messageId,
       messageParams: {
@@ -200,7 +220,7 @@ describe('AbstractTestManager', () => {
   });
 
   it('should sign a message', async () => {
-    const controller = new AbstractTestManager();
+    const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
     await controller.addMessage({
       id: messageId,
       messageParams: {
@@ -221,12 +241,10 @@ describe('AbstractTestManager', () => {
   });
 
   it('sets message to one of the allowed statuses', async () => {
-    const controller = new AbstractTestManager(
-      undefined,
-      undefined,
-      undefined,
-      ['test-status'],
-    );
+    const controller = new AbstractTestManager({
+      ...MOCK_INITIAL_OPTIONS,
+      additionalFinishStatuses: ['test-status'],
+    });
     await controller.addMessage({
       id: messageId,
       messageParams: {
@@ -246,12 +264,10 @@ describe('AbstractTestManager', () => {
   });
 
   it('should set a status to inProgress', async () => {
-    const controller = new AbstractTestManager(
-      undefined,
-      undefined,
-      undefined,
-      ['test-status'],
-    );
+    const controller = new AbstractTestManager({
+      ...MOCK_INITIAL_OPTIONS,
+      additionalFinishStatuses: ['test-status'],
+    });
     await controller.addMessage({
       id: messageId,
       messageParams: {
@@ -285,7 +301,7 @@ describe('AbstractTestManager', () => {
       time: 123,
       type: 'eth_signTypedData',
     };
-    const controller = new AbstractTestManager();
+    const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
     await controller.addMessage(firstMessage);
     await controller.addMessage(secondMessage);
     expect(controller.getUnapprovedMessagesCount()).toBe(2);
@@ -296,7 +312,7 @@ describe('AbstractTestManager', () => {
   });
 
   it('should approve message', async () => {
-    const controller = new AbstractTestManager();
+    const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
     const firstMessage = { from: '0xfoO', test: testData };
     await controller.addMessage({
       id: messageId,
@@ -319,7 +335,7 @@ describe('AbstractTestManager', () => {
 
   describe('addRequestToMessageParams', () => {
     it('adds original request id and origin to messageParams', () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
 
       const result = controller.addRequestToMessageParams(
         mockMessageParams,
@@ -336,7 +352,7 @@ describe('AbstractTestManager', () => {
 
   describe('createUnapprovedMessage', () => {
     it('creates a Message object with an unapproved status', () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
 
       const result = controller.createUnapprovedMessage(
         mockMessageParams,
@@ -361,7 +377,7 @@ describe('AbstractTestManager', () => {
         emit: jest.fn(),
       }));
 
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
       await controller.addMessage({
         id: messageId,
         messageParams: { ...mockMessageParams },
@@ -379,7 +395,7 @@ describe('AbstractTestManager', () => {
     });
 
     it('throws an error if the message is not found', async () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
 
       expect(() => controller.setMessageStatus(messageId, 'newstatus')).toThrow(
         'AbstractMessageManager: Message not found for id: 1.',
@@ -393,7 +409,7 @@ describe('AbstractTestManager', () => {
         emit: jest.fn(),
       }));
 
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
       await controller.addMessage({
         id: messageId,
         messageParams: { ...mockMessageParams },
@@ -407,14 +423,13 @@ describe('AbstractTestManager', () => {
       controller.setMessageStatusAndResult(messageId, 'newRawSig', 'newstatus');
       const messageAfter = controller.getMessage(messageId);
 
-      // expect(controller.hub.emit).toHaveBeenNthCalledWith(1, 'updateBadge');
       expect(messageAfter?.status).toBe('newstatus');
     });
   });
 
   describe('setMetadata', () => {
     it('should set the given message metadata', async () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
       await controller.addMessage({
         id: messageId,
         messageParams: { ...mockMessageParams },
@@ -432,7 +447,7 @@ describe('AbstractTestManager', () => {
     });
 
     it('should throw an error if message is not found', () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
 
       expect(() => controller.setMetadata(messageId, { foo: 'bar' })).toThrow(
         'AbstractMessageManager: Message not found for id: 1.',
@@ -442,7 +457,7 @@ describe('AbstractTestManager', () => {
 
   describe('waitForFinishStatus', () => {
     it('signs the message when status is "signed"', async () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
       const promise = controller.waitForFinishStatus(
         {
           from: fromMock,
@@ -452,7 +467,7 @@ describe('AbstractTestManager', () => {
       );
 
       setTimeout(() => {
-        controller.hub.emit(`${messageIdMock}:finished`, {
+        controller.internalEvents.emit(`${messageIdMock}:finished`, {
           status: 'signed',
           rawSig: rawSigMock,
         });
@@ -462,7 +477,7 @@ describe('AbstractTestManager', () => {
     });
 
     it('rejects with an error when status is "rejected"', async () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
       const promise = controller.waitForFinishStatus(
         {
           from: fromMock,
@@ -472,7 +487,7 @@ describe('AbstractTestManager', () => {
       );
 
       setTimeout(() => {
-        controller.hub.emit(`${messageIdMock}:finished`, {
+        controller.internalEvents.emit(`${messageIdMock}:finished`, {
           status: 'rejected',
         });
       }, 100);
@@ -483,7 +498,7 @@ describe('AbstractTestManager', () => {
     });
 
     it('rejects with an error when finishes with unknown status', async () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
       const promise = controller.waitForFinishStatus(
         {
           from: fromMock,
@@ -493,7 +508,7 @@ describe('AbstractTestManager', () => {
       );
 
       setTimeout(() => {
-        controller.hub.emit(`${messageIdMock}:finished`, {
+        controller.internalEvents.emit(`${messageIdMock}:finished`, {
           status: 'unknown',
         });
       }, 100);
@@ -508,7 +523,7 @@ describe('AbstractTestManager', () => {
     });
 
     it('rejects with an error when finishes with errored status', async () => {
-      const controller = new AbstractTestManager();
+      const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS);
       const promise = controller.waitForFinishStatus(
         {
           from: fromMock,
@@ -518,7 +533,7 @@ describe('AbstractTestManager', () => {
       );
 
       setTimeout(() => {
-        controller.hub.emit(`${messageIdMock}:finished`, {
+        controller.internalEvents.emit(`${messageIdMock}:finished`, {
           status: 'errored',
           error: 'error message',
         });
@@ -529,4 +544,26 @@ describe('AbstractTestManager', () => {
       );
     });
   });
+
+  describe('clearUnapprovedMessages', () => {
+    it('clears the unapproved messages', () => {
+      const controller = new AbstractTestManager({
+        ...MOCK_INITIAL_OPTIONS,
+        state: {
+          unapprovedMessages: {
+            '1': {
+              id: '1',
+              messageParams: { from: '0x1', test: 1 },
+              status: 'unapproved',
+              time: 10,
+              type: 'type',
+            },
+          },
+          unapprovedMessagesCount: 1,
+        },
+      });
+      controller.clearUnapprovedMessages();
+      expect(controller.getUnapprovedMessagesCount()).toBe(0);
+    });
+  });
 });