Override subscriptionManager.middleware.destroy to also remove sub en…

…try (#4984)

## Explanation

Ensures that the subscription entry for a subscription is removed when
the middleware is destroyed

## References

Related: MetaMask/metamask-extension#28751

## 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/package-a`

- **<CATEGORY>**: Your change here
- **<CATEGORY>**: Your change here

### `@metamask/package-b`

- **<CATEGORY>**: Your change here
- **<CATEGORY>**: Your change here

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

packages/multichain/src/middlewares/MultichainSubscriptionManager.test.ts

@@ -1,4 +1,5 @@
 import createSubscriptionManager from '@metamask/eth-json-rpc-filters/subscriptionManager';
+import type SafeEventEmitter from '@metamask/safe-event-emitter';
 
 import { MultichainSubscriptionManager } from './MultichainSubscriptionManager';
 
@@ -51,15 +52,21 @@ const createMultichainSubscriptionManager = () => {
   return { multichainSubscriptionManager };
 };
 
-describe('MultichainSubscriptionManager', () => {
-  const mockSubscriptionManager = {
-    events: {
-      on: jest.fn(),
-    },
+const createMockSubscriptionManager = () => ({
+  events: {
+    on: jest.fn(),
+  } as unknown as jest.Mocked<SafeEventEmitter>,
+  destroy: jest.fn(),
+  middleware: {
     destroy: jest.fn(),
-  };
+  },
+});
+
+describe('MultichainSubscriptionManager', () => {
+  let mockSubscriptionManager = createMockSubscriptionManager();
 
   beforeEach(() => {
+    mockSubscriptionManager = createMockSubscriptionManager();
     MockCreateSubscriptionManager.mockReturnValue(mockSubscriptionManager);
   });
 
@@ -118,10 +125,6 @@ describe('MultichainSubscriptionManager', () => {
     multichainSubscriptionManager.subscribe({ scope, origin, tabId });
     multichainSubscriptionManager.unsubscribeByScopeAndOrigin(scope, origin);
 
-    mockSubscriptionManager.events.on.mock.calls[0][1](
-      newHeadsNotificationMock,
-    );
-
     expect(mockSubscriptionManager.destroy).toHaveBeenCalled();
   });
 
@@ -138,9 +141,6 @@ describe('MultichainSubscriptionManager', () => {
       'other-origin',
       123,
     );
-    mockSubscriptionManager.events.on.mock.calls[0][1](
-      newHeadsNotificationMock,
-    );
 
     expect(mockSubscriptionManager.destroy).not.toHaveBeenCalled();
   });
@@ -151,9 +151,14 @@ describe('MultichainSubscriptionManager', () => {
     multichainSubscriptionManager.subscribe({ scope, origin, tabId });
     multichainSubscriptionManager.unsubscribeByOriginAndTabId(origin, tabId);
 
-    mockSubscriptionManager.events.on.mock.calls[0][1](
-      newHeadsNotificationMock,
-    );
+    expect(mockSubscriptionManager.destroy).toHaveBeenCalled();
+  });
+
+  it('should unsubscribe when the middleware is destroyed', () => {
+    const { multichainSubscriptionManager } =
+      createMultichainSubscriptionManager();
+    multichainSubscriptionManager.subscribe({ scope, origin, tabId });
+    mockSubscriptionManager.middleware.destroy();
 
     expect(mockSubscriptionManager.destroy).toHaveBeenCalled();
   });

packages/multichain/src/middlewares/MultichainSubscriptionManager.ts

@@ -4,13 +4,14 @@ import type { NetworkController } from '@metamask/network-controller';
 import SafeEventEmitter from '@metamask/safe-event-emitter';
 import type { CaipChainId, Hex } from '@metamask/utils';
 import { parseCaipChainId } from '@metamask/utils';
-import type EventEmitter from 'events';
 
 import type { ExternalScopeString } from '../scope/types';
+import type { ExtendedJsonRpcMiddleware } from './MultichainMiddlewareManager';
 
 export type SubscriptionManager = {
-  events: EventEmitter;
+  events: SafeEventEmitter;
   destroy?: () => void;
+  middleware: ExtendedJsonRpcMiddleware;
 };
 
 type SubscriptionNotificationEvent = {
@@ -108,10 +109,17 @@ export class MultichainSubscriptionManager extends SafeEventEmitter {
       },
     );
 
-    this.#subscriptions.push({
+    const newSubscriptionManagerEntry = {
       ...subscriptionKey,
       subscriptionManager,
-    });
+    };
+    subscriptionManager.destroy = subscriptionManager.middleware.destroy;
+    subscriptionManager.middleware.destroy = this.#unsubscribe.bind(
+      this,
+      newSubscriptionManagerEntry,
+    );
+
+    this.#subscriptions.push(newSubscriptionManagerEntry);
 
     return subscriptionManager;
   }