From 59d85cba9abcf740866edc798ab8451eb3c443ff Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Fri, 13 Dec 2024 15:22:31 +0100 Subject: [PATCH 01/12] Migrate AbstractMessageManager from BaseControllerV1 to BaseControllerV2 --- .../src/AbstractMessageManager.test.ts | 98 +++--- .../src/AbstractMessageManager.ts | 296 ++++++++++-------- .../src/DecryptMessageManager.test.ts | 25 +- .../src/DecryptMessageManager.ts | 78 ++++- .../src/EncryptionPublicKeyManager.test.ts | 28 +- .../src/EncryptionPublicKeyManager.ts | 84 ++++- 6 files changed, 383 insertions(+), 226 deletions(-) diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index 740ed38e2c..ec1c1d005b 100644 --- a/packages/message-manager/src/AbstractMessageManager.test.ts +++ b/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: MessageParams, @@ -56,6 +62,26 @@ class AbstractTestManager extends AbstractMessageManager< } } +const mockMessenger = { + clearEventSubscriptions: jest.fn(), + publish: jest.fn(), + registerActionHandler: jest.fn(), + registerInitialEventPayload: jest.fn(), +} as unknown as RestrictedControllerMessenger< + 'AbstractMessageManager', + never, + never, + string, + string +>; + +const mockInitialOptions = { + additionalFinishStatuses: undefined, + messenger: mockMessenger, + 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(mockInitialOptions); 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(mockInitialOptions); 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(mockInitialOptions); 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({ + ...mockInitialOptions, + 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(mockInitialOptions); 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(mockInitialOptions); 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({ + ...mockInitialOptions, + 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({ + ...mockInitialOptions, + 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(mockInitialOptions); 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(mockInitialOptions); 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(mockInitialOptions); 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(mockInitialOptions); const result = controller.createUnapprovedMessage( mockMessageParams, @@ -361,7 +377,7 @@ describe('AbstractTestManager', () => { emit: jest.fn(), })); - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(mockInitialOptions); 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(mockInitialOptions); 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(mockInitialOptions); await controller.addMessage({ id: messageId, messageParams: { ...mockMessageParams }, @@ -414,7 +430,7 @@ describe('AbstractTestManager', () => { describe('setMetadata', () => { it('should set the given message metadata', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(mockInitialOptions); await controller.addMessage({ id: messageId, messageParams: { ...mockMessageParams }, @@ -432,7 +448,7 @@ describe('AbstractTestManager', () => { }); it('should throw an error if message is not found', () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(mockInitialOptions); expect(() => controller.setMetadata(messageId, { foo: 'bar' })).toThrow( 'AbstractMessageManager: Message not found for id: 1.', @@ -442,7 +458,7 @@ describe('AbstractTestManager', () => { describe('waitForFinishStatus', () => { it('signs the message when status is "signed"', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(mockInitialOptions); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -462,7 +478,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when status is "rejected"', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(mockInitialOptions); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -483,7 +499,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when finishes with unknown status', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(mockInitialOptions); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -508,7 +524,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when finishes with errored status', async () => { - const controller = new AbstractTestManager(); + const controller = new AbstractTestManager(mockInitialOptions); const promise = controller.waitForFinishStatus( { from: fromMock, diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index bdd8401f54..25bf9c2519 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -1,29 +1,41 @@ -import type { BaseConfig, BaseState } from '@metamask/base-controller'; -import { BaseControllerV1 } from '@metamask/base-controller'; +import { BaseController } from '@metamask/base-controller'; +import type { + ActionConstraint, + EventConstraint, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import type { ApprovalType } from '@metamask/controller-utils'; -import type { Hex, Json } from '@metamask/utils'; +import type { Json } from '@metamask/utils'; // This package purposefully relies on Node's EventEmitter module. // eslint-disable-next-line import/no-nodejs-modules import { EventEmitter } from 'events'; +import type { Draft } from 'immer'; import { v1 as random } from 'uuid'; +const stateMetadata = { + unapprovedMessages: { persist: true, anonymous: false }, + unapprovedMessagesCount: { persist: true, anonymous: false }, +}; + +const getDefaultState = () => ({ + unapprovedMessages: {}, + unapprovedMessagesCount: 0, +}); + /** * @type OriginalRequest * * Represents the original request object for adding a message. * @property origin? - Is it is specified, represents the origin */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface OriginalRequest { +export type OriginalRequest = { id?: number; origin?: string; securityAlertResponse?: Record; -} +}; /** - * @type Message + * @type AbstractMessage * * Represents and contains data about a signing type signature request. * @property id - An id to track and identify the message object @@ -33,10 +45,7 @@ export interface OriginalRequest { * @property securityProviderResponse - Response from a security provider, whether it is malicious or not * @property metadata - Additional data for the message, for example external identifiers */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface AbstractMessage { +export type AbstractMessage = { id: string; time: number; status: string; @@ -46,7 +55,7 @@ export interface AbstractMessage { securityAlertResponse?: Record; metadata?: Json; error?: string; -} +}; /** * @type AbstractMessageParams @@ -57,15 +66,12 @@ export interface AbstractMessage { * @property requestId? - Original request id * @property deferSetAsSigned? - Whether to defer setting the message as signed immediately after the keyring is told to sign it */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface AbstractMessageParams { +export type AbstractMessageParams = { from: string; origin?: string; requestId?: number; deferSetAsSigned?: boolean; -} +}; /** * @type MessageParamsMetamask @@ -76,12 +82,9 @@ export interface AbstractMessageParams { * @property from - Address from which the message is processed * @property origin? - Added for request origin identification */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface AbstractMessageParamsMetamask extends AbstractMessageParams { +export type AbstractMessageParamsMetamask = AbstractMessageParams & { metamaskId?: string; -} +}; /** * @type MessageManagerState @@ -90,15 +93,10 @@ export interface AbstractMessageParamsMetamask extends AbstractMessageParams { * @property unapprovedMessages - A collection of all Messages in the 'unapproved' state * @property unapprovedMessagesCount - The count of all Messages in this.unapprovedMessages */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions, @typescript-eslint/naming-convention -export interface MessageManagerState - extends BaseState { - unapprovedMessages: { [key: string]: M }; +export type MessageManagerState = { + unapprovedMessages: Record; unapprovedMessagesCount: number; -} +}; /** * A function for verifying a message, whether it is malicious or not @@ -108,32 +106,77 @@ export type SecurityProviderRequest = ( messageType: string, ) => Promise; -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/naming-convention -type getCurrentChainId = () => Hex; +type AbstractMessageManagerOptions< + Message extends AbstractMessage, + Action extends ActionConstraint, + Event extends EventConstraint, +> = { + additionalFinishStatuses?: string[]; + messenger: RestrictedControllerMessenger< + string, + Action, + Event, + string, + string + >; + name: string; + securityProviderRequest?: SecurityProviderRequest; + state?: MessageManagerState; +}; /** * Controller in charge of managing - storing, adding, removing, updating - Messages. */ export abstract class AbstractMessageManager< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, -> extends BaseControllerV1> { - protected messages: M[]; - - protected getCurrentChainId: getCurrentChainId | undefined; + Message extends AbstractMessage, + Params extends AbstractMessageParams, + ParamsMetamask extends AbstractMessageParamsMetamask, + Action extends ActionConstraint, + Event extends EventConstraint, +> extends BaseController< + string, + MessageManagerState, + RestrictedControllerMessenger +> { + protected messages: Message[]; private readonly securityProviderRequest: SecurityProviderRequest | undefined; private readonly additionalFinishStatuses: string[]; + hub: EventEmitter = new EventEmitter(); + + /** + * Creates an AbstractMessageManager instance. + * + * @param options - Options for the AbstractMessageManager. + * @param options.additionalFinishStatuses - Optional list of statuses that are accepted to emit a finished event. + * @param options.messenger - Controller messaging system. + * @param options.name - The name of the manager. + * @param options.securityProviderRequest - A function for verifying a message, whether it is malicious or not. + * @param options.state - Initial state to set on this controller. + */ + constructor({ + additionalFinishStatuses, + messenger, + name, + securityProviderRequest, + state = {} as MessageManagerState, + }: AbstractMessageManagerOptions) { + super({ + messenger, + metadata: stateMetadata, + name, + state: { + ...getDefaultState(), + ...state, + }, + }); + this.messages = []; + this.securityProviderRequest = securityProviderRequest; + this.additionalFinishStatuses = additionalFinishStatuses ?? []; + } + /** * Adds request props to the messsage params and returns a new messageParams object. * @param messageParams - The messageParams to add the request props to. @@ -183,9 +226,14 @@ export abstract class AbstractMessageManager< * @param emitUpdateBadge - Whether to emit the updateBadge event. */ protected saveMessageList(emitUpdateBadge = true) { - const unapprovedMessages = this.getUnapprovedMessages(); - const unapprovedMessagesCount = this.getUnapprovedMessagesCount(); - this.update({ unapprovedMessages, unapprovedMessagesCount }); + this.update((state) => { + state.unapprovedMessages = + this.getUnapprovedMessages() as unknown as Record< + string, + Draft + >; + state.unapprovedMessagesCount = this.getUnapprovedMessagesCount(); + }); if (emitUpdateBadge) { this.hub.emit('updateBadge'); } @@ -200,10 +248,14 @@ export abstract class AbstractMessageManager< protected setMessageStatus(messageId: string, status: string) { const message = this.getMessage(messageId); if (!message) { - throw new Error(`${this.name}: Message not found for id: ${messageId}.`); + throw new Error( + `${this.name as string}: Message not found for id: ${messageId}.`, + ); } - message.status = status; - this.updateMessage(message); + this.updateMessage({ + ...message, + status, + }); this.hub.emit(`${messageId}:${status}`, message); if ( status === 'rejected' || @@ -222,7 +274,7 @@ export abstract class AbstractMessageManager< * @param message - A Message that will replace an existing Message (with the id) in this.messages. * @param emitUpdateBadge - Whether to emit the updateBadge event. */ - protected updateMessage(message: M, emitUpdateBadge = true) { + protected updateMessage(message: Message, emitUpdateBadge = true) { const index = this.messages.findIndex((msg) => message.id === msg.id); /* istanbul ignore next */ if (index !== -1) { @@ -237,7 +289,7 @@ export abstract class AbstractMessageManager< * @param message - The message to verify. * @returns A promise that resolves to a secured message with additional security provider response data. */ - private async securityCheck(message: M): Promise { + private async securityCheck(message: Message): Promise { if (this.securityProviderRequest) { const securityProviderResponse = await this.securityProviderRequest( message, @@ -251,44 +303,6 @@ export abstract class AbstractMessageManager< return message; } - /** - * EventEmitter instance used to listen to specific message events - */ - hub: EventEmitter = new EventEmitter(); - - /** - * Name of this controller used during composition - */ - override name = 'AbstractMessageManager'; - - /** - * Creates an AbstractMessageManager instance. - * - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. - * @param securityProviderRequest - A function for verifying a message, whether it is malicious or not. - * @param additionalFinishStatuses - Optional list of statuses that are accepted to emit a finished event. - * @param getCurrentChainId - Optional function to get the current chainId. - */ - constructor( - config?: Partial, - state?: Partial>, - securityProviderRequest?: SecurityProviderRequest, - additionalFinishStatuses?: string[], - getCurrentChainId?: getCurrentChainId, - ) { - super(config, state); - this.defaultState = { - unapprovedMessages: {}, - unapprovedMessagesCount: 0, - }; - this.messages = []; - this.securityProviderRequest = securityProviderRequest; - this.additionalFinishStatuses = additionalFinishStatuses ?? []; - this.getCurrentChainId = getCurrentChainId; - this.initialize(); - } - /** * A getter for the number of 'unapproved' Messages in this.messages. * @@ -306,10 +320,10 @@ export abstract class AbstractMessageManager< getUnapprovedMessages() { return this.messages .filter((message) => message.status === 'unapproved') - .reduce((result: { [key: string]: M }, message: M) => { + .reduce((result: Record, message) => { result[message.id] = message; return result; - }, {}) as { [key: string]: M }; + }, {}); } /** @@ -318,7 +332,7 @@ export abstract class AbstractMessageManager< * * @param message - The Message to add to this.messages. */ - async addMessage(message: M) { + async addMessage(message: Message) { const securedMessage = await this.securityCheck(message); this.messages.push(securedMessage); this.saveMessageList(); @@ -352,7 +366,7 @@ export abstract class AbstractMessageManager< * plus data added by MetaMask. * @returns Promise resolving to the messageParams with the metamaskId property removed. */ - approveMessage(messageParams: PM): Promise

{ + approveMessage(messageParams: ParamsMetamask): Promise { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore this.setMessageStatusApproved(messageParams.metamaskId); @@ -414,8 +428,13 @@ export abstract class AbstractMessageManager< if (!message) { return; } - message.rawSig = result; - this.updateMessage(message, false); + this.updateMessage( + { + ...message, + rawSig: result, + }, + false, + ); } /** @@ -424,14 +443,20 @@ export abstract class AbstractMessageManager< * @param messageId - The id of the Message to update * @param metadata - The data with which to replace the metadata property in the message */ - setMetadata(messageId: string, metadata: Json) { const message = this.getMessage(messageId); if (!message) { - throw new Error(`${this.name}: Message not found for id: ${messageId}.`); + throw new Error( + `${this.name as string}: Message not found for id: ${messageId}.`, + ); } - message.metadata = metadata; - this.updateMessage(message, false); + this.updateMessage( + { + ...message, + metadata, + }, + false, + ); } /** @@ -441,7 +466,9 @@ export abstract class AbstractMessageManager< * @param messageParams - The messageParams to modify * @returns Promise resolving to the messageParams with the metamaskId property removed */ - abstract prepMessageForSigning(messageParams: PM): Promise

; + abstract prepMessageForSigning( + messageParams: ParamsMetamask, + ): Promise; /** * Creates a new Message with an 'unapproved' status using the passed messageParams. @@ -454,7 +481,7 @@ export abstract class AbstractMessageManager< * @returns The id of the newly created message. */ abstract addUnapprovedMessage( - messageParams: PM, + messageParams: ParamsMetamask, request: OriginalRequest, version?: string, ): Promise; @@ -481,34 +508,35 @@ export abstract class AbstractMessageManager< ): Promise { const { metamaskId: messageId, ...messageParams } = messageParamsWithId; return new Promise((resolve, reject) => { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - this.hub.once(`${messageId}:finished`, (data: AbstractMessage) => { - switch (data.status) { - case 'signed': - return resolve(data.rawSig as string); - case 'rejected': - return reject( - new Error( - `MetaMask ${messageName} Signature: User denied message signature.`, - ), - ); - case 'errored': - return reject( - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - new Error(`MetaMask ${messageName} Signature: ${data.error}`), - ); - default: - return reject( - new Error( - `MetaMask ${messageName} Signature: Unknown problem: ${JSON.stringify( - messageParams, - )}`, - ), - ); - } - }); + this.hub.once( + `${messageId as string}:finished`, + (data: AbstractMessage) => { + switch (data.status) { + case 'signed': + return resolve(data.rawSig as string); + case 'rejected': + return reject( + new Error( + `MetaMask ${messageName} Signature: User denied message signature.`, + ), + ); + case 'errored': + return reject( + new Error( + `MetaMask ${messageName} Signature: ${data.error as string}`, + ), + ); + default: + return reject( + new Error( + `MetaMask ${messageName} Signature: Unknown problem: ${JSON.stringify( + messageParams, + )}`, + ), + ); + } + }, + ); }); } } diff --git a/packages/message-manager/src/DecryptMessageManager.test.ts b/packages/message-manager/src/DecryptMessageManager.test.ts index d81b336141..b771688f76 100644 --- a/packages/message-manager/src/DecryptMessageManager.test.ts +++ b/packages/message-manager/src/DecryptMessageManager.test.ts @@ -1,4 +1,19 @@ import { DecryptMessageManager } from './DecryptMessageManager'; +import type { DecryptMessageManagerMessenger } from './DecryptMessageManager'; + +const mockMessenger = { + registerActionHandler: jest.fn(), + registerInitialEventPayload: jest.fn(), + publish: jest.fn(), + clearEventSubscriptions: jest.fn(), +} as unknown as DecryptMessageManagerMessenger; + +const mockInitialOptions = { + additionalFinishStatuses: undefined, + messenger: mockMessenger, + name: 'DecryptMessageManager', + securityProviderRequest: undefined, +}; describe('DecryptMessageManager', () => { let controller: DecryptMessageManager; @@ -9,7 +24,7 @@ describe('DecryptMessageManager', () => { const dataMock = '0x12345'; beforeEach(() => { - controller = new DecryptMessageManager(); + controller = new DecryptMessageManager(mockInitialOptions); }); it('sets default state', () => { @@ -19,10 +34,6 @@ describe('DecryptMessageManager', () => { }); }); - it('sets default config', () => { - expect(controller.config).toStrictEqual({}); - }); - it('adds a valid message', async () => { const messageData = '0x123'; const messageTime = Date.now(); @@ -52,9 +63,7 @@ describe('DecryptMessageManager', () => { describe('addUnapprovedMessageAsync', () => { beforeEach(() => { - controller = new DecryptMessageManager(undefined, undefined, undefined, [ - 'decrypted', - ]); + controller = new DecryptMessageManager(mockInitialOptions); jest .spyOn(controller, 'addUnapprovedMessage') diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index 4036e79d90..d7ae608c33 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -1,14 +1,56 @@ +import type { + ControllerGetStateAction, + ControllerStateChangeEvent, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { ApprovalType } from '@metamask/controller-utils'; import type { AbstractMessage, AbstractMessageParams, AbstractMessageParamsMetamask, + MessageManagerState, OriginalRequest, + SecurityProviderRequest, } from './AbstractMessageManager'; import { AbstractMessageManager } from './AbstractMessageManager'; import { normalizeMessageData, validateDecryptedMessageData } from './utils'; +const controllerName = 'DecryptMessageManager'; + +export type DecryptMessageManagerState = MessageManagerState; + +export type GetDecryptMessageState = ControllerGetStateAction< + typeof controllerName, + DecryptMessageManagerState +>; + +export type DecryptMessageManagerStateChange = ControllerStateChangeEvent< + typeof controllerName, + DecryptMessageManagerState +>; + +export type DecryptMessageManagerActions = GetDecryptMessageState; + +export type DecryptMessageManagerEvents = DecryptMessageManagerStateChange; + +type AllowedActions = never; + +export type DecryptMessageManagerMessenger = RestrictedControllerMessenger< + typeof controllerName, + DecryptMessageManagerActions | AllowedActions, + DecryptMessageManagerEvents, + AllowedActions['type'], + never +>; + +export type DecryptMessageManagerOptions = { + messenger: DecryptMessageManagerMessenger; + state?: MessageManagerState; + securityProviderRequest?: SecurityProviderRequest; + additionalFinishStatuses?: string[]; +}; + /** * @type DecryptMessage * @@ -19,12 +61,9 @@ import { normalizeMessageData, validateDecryptedMessageData } from './utils'; * @property type - The json-prc signing method for which a signature request has been made. * A 'DecryptMessage' which always has a 'eth_decrypt' type */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface DecryptMessage extends AbstractMessage { +export type DecryptMessage = AbstractMessage & { messageParams: DecryptMessageParams; -} +}; /** * @type DecryptMessageParams @@ -32,12 +71,9 @@ export interface DecryptMessage extends AbstractMessage { * Represents the parameters to pass to the eth_decrypt method once the request is approved. * @property data - A hex string conversion of the raw buffer data of the signature request */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface DecryptMessageParams extends AbstractMessageParams { +export type DecryptMessageParams = AbstractMessageParams & { data: string; -} +}; /** * @type DecryptMessageParamsMetamask @@ -63,12 +99,24 @@ export interface DecryptMessageParamsMetamask export class DecryptMessageManager extends AbstractMessageManager< DecryptMessage, DecryptMessageParams, - DecryptMessageParamsMetamask + DecryptMessageParamsMetamask, + DecryptMessageManagerActions, + DecryptMessageManagerEvents > { - /** - * Name of this controller used during composition - */ - override name = 'DecryptMessageManager' as const; + constructor({ + messenger, + state, + securityProviderRequest, + additionalFinishStatuses, + }: DecryptMessageManagerOptions) { + super({ + messenger, + name: controllerName, + state, + securityProviderRequest, + additionalFinishStatuses, + }); + } /** * Creates a new Message with an 'unapproved' status using the passed messageParams. diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts index 8617c16534..4704f1106d 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts @@ -1,4 +1,19 @@ import { EncryptionPublicKeyManager } from './EncryptionPublicKeyManager'; +import type { EncryptionPublicKeyManagerMessenger } from './EncryptionPublicKeyManager'; + +const mockMessenger = { + registerActionHandler: jest.fn(), + registerInitialEventPayload: jest.fn(), + publish: jest.fn(), + clearEventSubscriptions: jest.fn(), +} as unknown as EncryptionPublicKeyManagerMessenger; + +const mockInitialOptions = { + additionalFinishStatuses: undefined, + messenger: mockMessenger, + name: 'EncryptionPublicKeyManager', + securityProviderRequest: undefined, +}; describe('EncryptionPublicKeyManager', () => { let controller: EncryptionPublicKeyManager; @@ -8,7 +23,7 @@ describe('EncryptionPublicKeyManager', () => { const rawSigMock = '231124fe67213512='; beforeEach(() => { - controller = new EncryptionPublicKeyManager(); + controller = new EncryptionPublicKeyManager(mockInitialOptions); }); it('sets default state', () => { @@ -18,10 +33,6 @@ describe('EncryptionPublicKeyManager', () => { }); }); - it('sets default config', () => { - expect(controller.config).toStrictEqual({}); - }); - it('adds a valid message', async () => { const messageTime = Date.now(); const messageStatus = 'unapproved'; @@ -48,12 +59,7 @@ describe('EncryptionPublicKeyManager', () => { describe('addUnapprovedMessageAsync', () => { beforeEach(() => { - controller = new EncryptionPublicKeyManager( - undefined, - undefined, - undefined, - ['received'], - ); + controller = new EncryptionPublicKeyManager(mockInitialOptions); jest .spyOn(controller, 'addUnapprovedMessage') diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index 6fdf1518f7..ae26dd85ab 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -1,14 +1,58 @@ +import type { + ControllerGetStateAction, + ControllerStateChangeEvent, + RestrictedControllerMessenger, +} from '@metamask/base-controller'; import { ApprovalType } from '@metamask/controller-utils'; import type { AbstractMessage, AbstractMessageParams, AbstractMessageParamsMetamask, + MessageManagerState, OriginalRequest, + SecurityProviderRequest, } from './AbstractMessageManager'; import { AbstractMessageManager } from './AbstractMessageManager'; import { validateEncryptionPublicKeyMessageData } from './utils'; +const controllerName = 'EncryptionPublicKeyManager'; + +export type EncryptionPublicKeyManagerState = + MessageManagerState; + +export type GetEncryptionPublicKeyState = ControllerGetStateAction< + typeof controllerName, + EncryptionPublicKeyManagerState +>; + +export type EncryptionPublicKeyManagerStateChange = ControllerStateChangeEvent< + typeof controllerName, + EncryptionPublicKeyManagerState +>; + +export type EncryptionPublicKeyManagerActions = GetEncryptionPublicKeyState; + +export type EncryptionPublicKeyManagerEvents = + EncryptionPublicKeyManagerStateChange; + +type AllowedActions = never; + +export type EncryptionPublicKeyManagerMessenger = RestrictedControllerMessenger< + typeof controllerName, + EncryptionPublicKeyManagerActions | AllowedActions, + EncryptionPublicKeyManagerEvents, + AllowedActions['type'], + never +>; + +type EncryptionPublicKeyManagerOptions = { + messenger: EncryptionPublicKeyManagerMessenger; + state?: MessageManagerState; + securityProviderRequest?: SecurityProviderRequest; + additionalFinishStatuses?: string[]; +}; + /** * @type EncryptionPublicKey * @@ -20,12 +64,9 @@ import { validateEncryptionPublicKeyMessageData } from './utils'; * A 'Message' which always has a 'eth_getEncryptionPublicKey' type * @property rawSig - Encryption public key */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface EncryptionPublicKey extends AbstractMessage { +export type EncryptionPublicKey = AbstractMessage & { messageParams: EncryptionPublicKeyParams; -} +}; /** * @type EncryptionPublicKeyParams @@ -46,13 +87,10 @@ export type EncryptionPublicKeyParams = AbstractMessageParams; * @property from - Address from which to extract the encryption public key * @property origin? - Added for request origin identification */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface EncryptionPublicKeyParamsMetamask - extends AbstractMessageParamsMetamask { - data: string; -} +export type EncryptionPublicKeyParamsMetamask = + AbstractMessageParamsMetamask & { + data: string; + }; /** * Controller in charge of managing - storing, adding, removing, updating - Messages. @@ -60,12 +98,24 @@ export interface EncryptionPublicKeyParamsMetamask export class EncryptionPublicKeyManager extends AbstractMessageManager< EncryptionPublicKey, EncryptionPublicKeyParams, - EncryptionPublicKeyParamsMetamask + EncryptionPublicKeyParamsMetamask, + EncryptionPublicKeyManagerActions, + EncryptionPublicKeyManagerEvents > { - /** - * Name of this controller used during composition - */ - override name = 'EncryptionPublicKeyManager' as const; + constructor({ + messenger, + state, + securityProviderRequest, + additionalFinishStatuses, + }: EncryptionPublicKeyManagerOptions) { + super({ + messenger, + name: controllerName, + state, + securityProviderRequest, + additionalFinishStatuses, + }); + } /** * Creates a new Message with an 'unapproved' status using the passed messageParams. From cb80b96633f6a1552dd179ad705358cf281434fb Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 16 Dec 2024 10:27:03 +0100 Subject: [PATCH 02/12] Renaming --- .../message-manager/src/AbstractMessageManager.ts | 11 +++++++++-- packages/message-manager/src/DecryptMessageManager.ts | 4 ++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 25bf9c2519..6cc2bc01e3 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -13,8 +13,8 @@ import type { Draft } from 'immer'; import { v1 as random } from 'uuid'; const stateMetadata = { - unapprovedMessages: { persist: true, anonymous: false }, - unapprovedMessagesCount: { persist: true, anonymous: false }, + unapprovedMessages: { persist: false, anonymous: false }, + unapprovedMessagesCount: { persist: false, anonymous: false }, }; const getDefaultState = () => ({ @@ -303,6 +303,13 @@ export abstract class AbstractMessageManager< return message; } + protected clearUnapprovedMessages() { + this.update((state) => { + state.unapprovedMessages = {}; + state.unapprovedMessagesCount = 0; + }); + } + /** * A getter for the number of 'unapproved' Messages in this.messages. * diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index d7ae608c33..2440231d73 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -20,7 +20,7 @@ const controllerName = 'DecryptMessageManager'; export type DecryptMessageManagerState = MessageManagerState; -export type GetDecryptMessageState = ControllerGetStateAction< +export type GetDecryptMessageManagerState = ControllerGetStateAction< typeof controllerName, DecryptMessageManagerState >; @@ -30,7 +30,7 @@ export type DecryptMessageManagerStateChange = ControllerStateChangeEvent< DecryptMessageManagerState >; -export type DecryptMessageManagerActions = GetDecryptMessageState; +export type DecryptMessageManagerActions = GetDecryptMessageManagerState; export type DecryptMessageManagerEvents = DecryptMessageManagerStateChange; From 19e52ec083cf175036c95bb382ace5e68de0ceba Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 16 Dec 2024 10:38:53 +0100 Subject: [PATCH 03/12] Fix tests --- .../message-manager/src/AbstractMessageManager.test.ts | 8 ++++++++ packages/message-manager/src/AbstractMessageManager.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index ec1c1d005b..f69b65abf7 100644 --- a/packages/message-manager/src/AbstractMessageManager.test.ts +++ b/packages/message-manager/src/AbstractMessageManager.test.ts @@ -545,4 +545,12 @@ describe('AbstractTestManager', () => { ); }); }); + + describe('clearUnapprovedMessages', () => { + it('clears the unapproved messages', () => { + const controller = new AbstractTestManager(mockInitialOptions); + controller.clearUnapprovedMessages(); + expect(controller.getUnapprovedMessagesCount()).toBe(0); + }); + }); }); diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 6cc2bc01e3..53206bc1d4 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -303,7 +303,7 @@ export abstract class AbstractMessageManager< return message; } - protected clearUnapprovedMessages() { + clearUnapprovedMessages() { this.update((state) => { state.unapprovedMessages = {}; state.unapprovedMessagesCount = 0; From b277d30b8fa9bc3d67a93606f525056ec0548163 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 16 Dec 2024 10:43:20 +0100 Subject: [PATCH 04/12] Fix lint --- packages/message-manager/src/AbstractMessageManager.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index f69b65abf7..2e2e67f8e8 100644 --- a/packages/message-manager/src/AbstractMessageManager.test.ts +++ b/packages/message-manager/src/AbstractMessageManager.test.ts @@ -545,7 +545,7 @@ describe('AbstractTestManager', () => { ); }); }); - + describe('clearUnapprovedMessages', () => { it('clears the unapproved messages', () => { const controller = new AbstractTestManager(mockInitialOptions); From 9b30d39bb6d228efc5521082599ef7d7102d02a1 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 6 Jan 2025 10:37:37 +0100 Subject: [PATCH 05/12] Update --- .../src/DecryptMessageManager.ts | 52 +++++++------------ .../src/EncryptionPublicKeyManager.ts | 51 ++++++------------ 2 files changed, 35 insertions(+), 68 deletions(-) diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index 2440231d73..eea2827619 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -1,6 +1,6 @@ import type { - ControllerGetStateAction, - ControllerStateChangeEvent, + ActionConstraint, + EventConstraint, RestrictedControllerMessenger, } from '@metamask/base-controller'; import { ApprovalType } from '@metamask/controller-utils'; @@ -16,38 +16,21 @@ import type { import { AbstractMessageManager } from './AbstractMessageManager'; import { normalizeMessageData, validateDecryptedMessageData } from './utils'; -const controllerName = 'DecryptMessageManager'; - export type DecryptMessageManagerState = MessageManagerState; -export type GetDecryptMessageManagerState = ControllerGetStateAction< - typeof controllerName, - DecryptMessageManagerState ->; - -export type DecryptMessageManagerStateChange = ControllerStateChangeEvent< - typeof controllerName, - DecryptMessageManagerState ->; - -export type DecryptMessageManagerActions = GetDecryptMessageManagerState; - -export type DecryptMessageManagerEvents = DecryptMessageManagerStateChange; - -type AllowedActions = never; - export type DecryptMessageManagerMessenger = RestrictedControllerMessenger< - typeof controllerName, - DecryptMessageManagerActions | AllowedActions, - DecryptMessageManagerEvents, - AllowedActions['type'], - never + string, + ActionConstraint, + EventConstraint, + string, + string >; -export type DecryptMessageManagerOptions = { +type DecryptMessageManagerOptions = { messenger: DecryptMessageManagerMessenger; - state?: MessageManagerState; + name: string; securityProviderRequest?: SecurityProviderRequest; + state?: MessageManagerState; additionalFinishStatuses?: string[]; }; @@ -100,21 +83,22 @@ export class DecryptMessageManager extends AbstractMessageManager< DecryptMessage, DecryptMessageParams, DecryptMessageParamsMetamask, - DecryptMessageManagerActions, - DecryptMessageManagerEvents + ActionConstraint, + EventConstraint > { constructor({ + additionalFinishStatuses, messenger, - state, + name, securityProviderRequest, - additionalFinishStatuses, + state, }: DecryptMessageManagerOptions) { super({ + additionalFinishStatuses, messenger, - name: controllerName, - state, + name, securityProviderRequest, - additionalFinishStatuses, + state, }); } diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index ae26dd85ab..4e362d961f 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -1,6 +1,6 @@ import type { - ControllerGetStateAction, - ControllerStateChangeEvent, + ActionConstraint, + EventConstraint, RestrictedControllerMessenger, } from '@metamask/base-controller'; import { ApprovalType } from '@metamask/controller-utils'; @@ -16,40 +16,22 @@ import type { import { AbstractMessageManager } from './AbstractMessageManager'; import { validateEncryptionPublicKeyMessageData } from './utils'; -const controllerName = 'EncryptionPublicKeyManager'; - export type EncryptionPublicKeyManagerState = MessageManagerState; -export type GetEncryptionPublicKeyState = ControllerGetStateAction< - typeof controllerName, - EncryptionPublicKeyManagerState ->; - -export type EncryptionPublicKeyManagerStateChange = ControllerStateChangeEvent< - typeof controllerName, - EncryptionPublicKeyManagerState ->; - -export type EncryptionPublicKeyManagerActions = GetEncryptionPublicKeyState; - -export type EncryptionPublicKeyManagerEvents = - EncryptionPublicKeyManagerStateChange; - -type AllowedActions = never; - export type EncryptionPublicKeyManagerMessenger = RestrictedControllerMessenger< - typeof controllerName, - EncryptionPublicKeyManagerActions | AllowedActions, - EncryptionPublicKeyManagerEvents, - AllowedActions['type'], - never + string, + ActionConstraint, + EventConstraint, + string, + string >; type EncryptionPublicKeyManagerOptions = { messenger: EncryptionPublicKeyManagerMessenger; - state?: MessageManagerState; + name: string; securityProviderRequest?: SecurityProviderRequest; + state?: MessageManagerState; additionalFinishStatuses?: string[]; }; @@ -99,21 +81,22 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< EncryptionPublicKey, EncryptionPublicKeyParams, EncryptionPublicKeyParamsMetamask, - EncryptionPublicKeyManagerActions, - EncryptionPublicKeyManagerEvents + ActionConstraint, + EventConstraint > { constructor({ + additionalFinishStatuses, messenger, - state, + name, securityProviderRequest, - additionalFinishStatuses, + state, }: EncryptionPublicKeyManagerOptions) { super({ + additionalFinishStatuses, messenger, - name: controllerName, - state, + name, securityProviderRequest, - additionalFinishStatuses, + state, }); } From b5baa28390b8c12bcc5f7b66ebce573d37a18ee5 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 6 Jan 2025 14:08:48 +0100 Subject: [PATCH 06/12] Fix update message --- packages/message-manager/src/AbstractMessageManager.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 53206bc1d4..84c55919d2 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -252,11 +252,12 @@ export abstract class AbstractMessageManager< `${this.name as string}: Message not found for id: ${messageId}.`, ); } - this.updateMessage({ + const updatedMessage = { ...message, status, - }); - this.hub.emit(`${messageId}:${status}`, message); + }; + this.updateMessage(updatedMessage); + this.hub.emit(`${messageId}:${status}`, updatedMessage); if ( status === 'rejected' || status === 'signed' || From 544f13b4671e16ccf0f778dd0f95ef492e9addde Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Mon, 6 Jan 2025 15:48:05 +0100 Subject: [PATCH 07/12] Fix updatedMessage --- packages/message-manager/src/AbstractMessageManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 84c55919d2..0e7e5de175 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -264,7 +264,7 @@ export abstract class AbstractMessageManager< status === 'errored' || this.additionalFinishStatuses.includes(status) ) { - this.hub.emit(`${messageId}:finished`, message); + this.hub.emit(`${messageId}:finished`, updatedMessage); } } From 4b1515ba4b2ca7d1005a4fc0357f4c4baf444712 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Tue, 7 Jan 2025 11:09:07 +0100 Subject: [PATCH 08/12] Fix suggestions --- packages/message-manager/CHANGELOG.md | 7 ++ .../src/AbstractMessageManager.test.ts | 59 ++++++++--------- .../src/AbstractMessageManager.ts | 50 +++++++++----- .../src/DecryptMessageManager.test.ts | 8 +-- .../src/DecryptMessageManager.ts | 66 +++++++++++-------- .../src/EncryptionPublicKeyManager.test.ts | 6 +- .../src/EncryptionPublicKeyManager.ts | 54 ++++++++------- 7 files changed, 143 insertions(+), 107 deletions(-) diff --git a/packages/message-manager/CHANGELOG.md b/packages/message-manager/CHANGELOG.md index ac90e79a53..e023f006e8 100644 --- a/packages/message-manager/CHANGELOG.md +++ b/packages/message-manager/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** Base class of `DecryptMessageManager` and `EncryptionPublicKeyManager`(`AbstractMessageManager`) now expects new options to initialise ([#5103](https://github.com/MetaMask/core/pull/5103)) +- **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. + ### Changed - Bump `@metamask/base-controller` from `^7.0.0` to `^7.1.0` ([#5079](https://github.com/MetaMask/core/pull/5079)) diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index 2e2e67f8e8..e1360d2086 100644 --- a/packages/message-manager/src/AbstractMessageManager.test.ts +++ b/packages/message-manager/src/AbstractMessageManager.test.ts @@ -62,7 +62,7 @@ class AbstractTestManager extends AbstractMessageManager< } } -const mockMessenger = { +const MOCK_MESSENGER = { clearEventSubscriptions: jest.fn(), publish: jest.fn(), registerActionHandler: jest.fn(), @@ -75,9 +75,9 @@ const mockMessenger = { string >; -const mockInitialOptions = { +const MOCK_INITIAL_OPTIONS = { additionalFinishStatuses: undefined, - messenger: mockMessenger, + messenger: MOCK_MESSENGER, name: 'AbstractMessageManager' as const, securityProviderRequest: undefined, }; @@ -104,7 +104,7 @@ const mockMessageParams = { from, test: testData }; describe('AbstractTestManager', () => { it('should set default state', () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); expect(controller.state).toStrictEqual({ unapprovedMessages: {}, unapprovedMessagesCount: 0, @@ -112,7 +112,7 @@ describe('AbstractTestManager', () => { }); it('should add a valid message', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { @@ -136,7 +136,7 @@ describe('AbstractTestManager', () => { }); it('should get all messages', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const message = { id: messageId, messageParams: { @@ -170,7 +170,7 @@ describe('AbstractTestManager', () => { .fn() .mockResolvedValue(securityProviderResponseMock); const controller = new AbstractTestManager({ - ...mockInitialOptions, + ...MOCK_INITIAL_OPTIONS, securityProviderRequest: securityProviderRequestMock, }); await controller.addMessage({ @@ -200,7 +200,7 @@ describe('AbstractTestManager', () => { }); it('should reject a message', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { @@ -220,7 +220,7 @@ describe('AbstractTestManager', () => { }); it('should sign a message', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { @@ -242,7 +242,7 @@ describe('AbstractTestManager', () => { it('sets message to one of the allowed statuses', async () => { const controller = new AbstractTestManager({ - ...mockInitialOptions, + ...MOCK_INITIAL_OPTIONS, additionalFinishStatuses: ['test-status'], }); await controller.addMessage({ @@ -265,7 +265,7 @@ describe('AbstractTestManager', () => { it('should set a status to inProgress', async () => { const controller = new AbstractTestManager({ - ...mockInitialOptions, + ...MOCK_INITIAL_OPTIONS, additionalFinishStatuses: ['test-status'], }); await controller.addMessage({ @@ -301,7 +301,7 @@ describe('AbstractTestManager', () => { time: 123, type: 'eth_signTypedData', }; - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage(firstMessage); await controller.addMessage(secondMessage); expect(controller.getUnapprovedMessagesCount()).toBe(2); @@ -312,7 +312,7 @@ describe('AbstractTestManager', () => { }); it('should approve message', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const firstMessage = { from: '0xfoO', test: testData }; await controller.addMessage({ id: messageId, @@ -335,7 +335,7 @@ describe('AbstractTestManager', () => { describe('addRequestToMessageParams', () => { it('adds original request id and origin to messageParams', () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const result = controller.addRequestToMessageParams( mockMessageParams, @@ -352,7 +352,7 @@ describe('AbstractTestManager', () => { describe('createUnapprovedMessage', () => { it('creates a Message object with an unapproved status', () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const result = controller.createUnapprovedMessage( mockMessageParams, @@ -377,7 +377,7 @@ describe('AbstractTestManager', () => { emit: jest.fn(), })); - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { ...mockMessageParams }, @@ -395,7 +395,7 @@ describe('AbstractTestManager', () => { }); it('throws an error if the message is not found', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); expect(() => controller.setMessageStatus(messageId, 'newstatus')).toThrow( 'AbstractMessageManager: Message not found for id: 1.', @@ -409,7 +409,7 @@ describe('AbstractTestManager', () => { emit: jest.fn(), })); - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { ...mockMessageParams }, @@ -423,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(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); await controller.addMessage({ id: messageId, messageParams: { ...mockMessageParams }, @@ -448,7 +447,7 @@ describe('AbstractTestManager', () => { }); it('should throw an error if message is not found', () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); expect(() => controller.setMetadata(messageId, { foo: 'bar' })).toThrow( 'AbstractMessageManager: Message not found for id: 1.', @@ -458,7 +457,7 @@ describe('AbstractTestManager', () => { describe('waitForFinishStatus', () => { it('signs the message when status is "signed"', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -468,7 +467,7 @@ describe('AbstractTestManager', () => { ); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'signed', rawSig: rawSigMock, }); @@ -478,7 +477,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when status is "rejected"', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -488,7 +487,7 @@ describe('AbstractTestManager', () => { ); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'rejected', }); }, 100); @@ -499,7 +498,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when finishes with unknown status', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -509,7 +508,7 @@ describe('AbstractTestManager', () => { ); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'unknown', }); }, 100); @@ -524,7 +523,7 @@ describe('AbstractTestManager', () => { }); it('rejects with an error when finishes with errored status', async () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); const promise = controller.waitForFinishStatus( { from: fromMock, @@ -534,7 +533,7 @@ describe('AbstractTestManager', () => { ); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'errored', error: 'error message', }); @@ -548,7 +547,7 @@ describe('AbstractTestManager', () => { describe('clearUnapprovedMessages', () => { it('clears the unapproved messages', () => { - const controller = new AbstractTestManager(mockInitialOptions); + const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); controller.clearUnapprovedMessages(); expect(controller.getUnapprovedMessagesCount()).toBe(0); }); diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 0e7e5de175..34769fadc4 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -98,6 +98,11 @@ export type MessageManagerState = { unapprovedMessagesCount: number; }; +export type UpdateBadgeEvent = { + type: `${string}:updateBadge`; + payload: [unusedPayload: string]; +}; + /** * A function for verifying a message, whether it is malicious or not */ @@ -106,7 +111,16 @@ export type SecurityProviderRequest = ( messageType: string, ) => Promise; -type AbstractMessageManagerOptions< +/** + * AbstractMessageManager constructor options. + * + * @property additionalFinishStatuses - Optional list of statuses that are accepted to emit a finished event. + * @property messenger - Controller messaging system. + * @property name - The name of the manager. + * @property securityProviderRequest - A function for verifying a message, whether it is malicious or not. + * @property state - Initial state to set on this controller. + */ +export type AbstractMessageManagerOptions< Message extends AbstractMessage, Action extends ActionConstraint, Event extends EventConstraint, @@ -115,7 +129,7 @@ type AbstractMessageManagerOptions< messenger: RestrictedControllerMessenger< string, Action, - Event, + Event | UpdateBadgeEvent, string, string >; @@ -136,7 +150,13 @@ export abstract class AbstractMessageManager< > extends BaseController< string, MessageManagerState, - RestrictedControllerMessenger + RestrictedControllerMessenger< + string, + Action, + Event | UpdateBadgeEvent, + string, + string + > > { protected messages: Message[]; @@ -144,18 +164,8 @@ export abstract class AbstractMessageManager< private readonly additionalFinishStatuses: string[]; - hub: EventEmitter = new EventEmitter(); + internalEvents = new EventEmitter(); - /** - * Creates an AbstractMessageManager instance. - * - * @param options - Options for the AbstractMessageManager. - * @param options.additionalFinishStatuses - Optional list of statuses that are accepted to emit a finished event. - * @param options.messenger - Controller messaging system. - * @param options.name - The name of the manager. - * @param options.securityProviderRequest - A function for verifying a message, whether it is malicious or not. - * @param options.state - Initial state to set on this controller. - */ constructor({ additionalFinishStatuses, messenger, @@ -235,7 +245,8 @@ export abstract class AbstractMessageManager< state.unapprovedMessagesCount = this.getUnapprovedMessagesCount(); }); if (emitUpdateBadge) { - this.hub.emit('updateBadge'); + // Empty payload is used to satisfy event constraint for BaseControllerV2 + this.messagingSystem.publish(`${this.name as string}:updateBadge`, ''); } } @@ -257,14 +268,17 @@ export abstract class AbstractMessageManager< status, }; this.updateMessage(updatedMessage); - this.hub.emit(`${messageId}:${status}`, updatedMessage); + this.internalEvents.emit(`${messageId}:${status}`, updatedMessage); if ( status === 'rejected' || status === 'signed' || status === 'errored' || this.additionalFinishStatuses.includes(status) ) { - this.hub.emit(`${messageId}:finished`, updatedMessage); + this.internalEvents.emit( + `${messageId as string}:finished`, + updatedMessage, + ); } } @@ -516,7 +530,7 @@ export abstract class AbstractMessageManager< ): Promise { const { metamaskId: messageId, ...messageParams } = messageParamsWithId; return new Promise((resolve, reject) => { - this.hub.once( + this.internalEvents.once( `${messageId as string}:finished`, (data: AbstractMessage) => { switch (data.status) { diff --git a/packages/message-manager/src/DecryptMessageManager.test.ts b/packages/message-manager/src/DecryptMessageManager.test.ts index b771688f76..6614e221c4 100644 --- a/packages/message-manager/src/DecryptMessageManager.test.ts +++ b/packages/message-manager/src/DecryptMessageManager.test.ts @@ -81,7 +81,7 @@ describe('DecryptMessageManager', () => { data: dataMock, }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'decrypted', rawSig: rawSigMock, }); @@ -97,7 +97,7 @@ describe('DecryptMessageManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'rejected', }); }, 100); @@ -114,7 +114,7 @@ describe('DecryptMessageManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'errored', }); }, 100); @@ -131,7 +131,7 @@ describe('DecryptMessageManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'unknown', }); }, 100); diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index eea2827619..a3d2737ee5 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -18,10 +18,15 @@ import { normalizeMessageData, validateDecryptedMessageData } from './utils'; export type DecryptMessageManagerState = MessageManagerState; +type DecryptMessageManagerUnapprovedMessageAddedEvent = { + type: `${string}:unapprovedMessage`; + payload: [AbstractMessageParamsMetamask]; +}; + export type DecryptMessageManagerMessenger = RestrictedControllerMessenger< string, ActionConstraint, - EventConstraint, + EventConstraint | DecryptMessageManagerUnapprovedMessageAddedEvent, string, string >; @@ -84,7 +89,7 @@ export class DecryptMessageManager extends AbstractMessageManager< DecryptMessageParams, DecryptMessageParamsMetamask, ActionConstraint, - EventConstraint + EventConstraint | DecryptMessageManagerUnapprovedMessageAddedEvent > { constructor({ additionalFinishStatuses, @@ -118,32 +123,35 @@ export class DecryptMessageManager extends AbstractMessageManager< const messageId = await this.addUnapprovedMessage(messageParams, req); return new Promise((resolve, reject) => { - this.hub.once(`${messageId}:finished`, (data: DecryptMessage) => { - switch (data.status) { - case 'decrypted': - return resolve(data.rawSig as string); - case 'rejected': - return reject( - new Error( - 'MetaMask DecryptMessage: User denied message decryption.', - ), - ); - case 'errored': - return reject( - new Error( - 'MetaMask DecryptMessage: This message cannot be decrypted.', - ), - ); - default: - return reject( - new Error( - `MetaMask DecryptMessage: Unknown problem: ${JSON.stringify( - messageParams, - )}`, - ), - ); - } - }); + this.internalEvents.once( + `${messageId}:finished`, + (data: DecryptMessage) => { + switch (data.status) { + case 'decrypted': + return resolve(data.rawSig as string); + case 'rejected': + return reject( + new Error( + 'MetaMask DecryptMessage: User denied message decryption.', + ), + ); + case 'errored': + return reject( + new Error( + 'MetaMask DecryptMessage: This message cannot be decrypted.', + ), + ); + default: + return reject( + new Error( + `MetaMask DecryptMessage: Unknown problem: ${JSON.stringify( + messageParams, + )}`, + ), + ); + } + }, + ); }); } @@ -176,7 +184,7 @@ export class DecryptMessageManager extends AbstractMessageManager< const messageId = messageData.id; await this.addMessage(messageData); - this.hub.emit(`unapprovedMessage`, { + this.messagingSystem.publish(`${this.name as string}:unapprovedMessage`, { ...updatedMessageParams, metamaskId: messageId, }); diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts index 4704f1106d..d84b7ed17a 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts @@ -76,7 +76,7 @@ describe('EncryptionPublicKeyManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'received', rawSig: rawSigMock, }); @@ -91,7 +91,7 @@ describe('EncryptionPublicKeyManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'rejected', }); }, 100); @@ -107,7 +107,7 @@ describe('EncryptionPublicKeyManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'unknown', }); }, 100); diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index 4e362d961f..dbda3725bc 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -19,10 +19,15 @@ import { validateEncryptionPublicKeyMessageData } from './utils'; export type EncryptionPublicKeyManagerState = MessageManagerState; +type EncryptionPublicKeyManagerUnapprovedMessageAddedEvent = { + type: `${string}:unapprovedMessage`; + payload: [AbstractMessageParamsMetamask]; +}; + export type EncryptionPublicKeyManagerMessenger = RestrictedControllerMessenger< string, ActionConstraint, - EventConstraint, + EventConstraint | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent, string, string >; @@ -82,7 +87,7 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< EncryptionPublicKeyParams, EncryptionPublicKeyParamsMetamask, ActionConstraint, - EventConstraint + EventConstraint | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent > { constructor({ additionalFinishStatuses, @@ -116,26 +121,29 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< const messageId = await this.addUnapprovedMessage(messageParams, req); return new Promise((resolve, reject) => { - this.hub.once(`${messageId}:finished`, (data: EncryptionPublicKey) => { - switch (data.status) { - case 'received': - return resolve(data.rawSig as string); - case 'rejected': - return reject( - new Error( - 'MetaMask EncryptionPublicKey: User denied message EncryptionPublicKey.', - ), - ); - default: - return reject( - new Error( - `MetaMask EncryptionPublicKey: Unknown problem: ${JSON.stringify( - messageParams, - )}`, - ), - ); - } - }); + this.internalEvents.once( + `${messageId}:finished`, + (data: EncryptionPublicKey) => { + switch (data.status) { + case 'received': + return resolve(data.rawSig as string); + case 'rejected': + return reject( + new Error( + 'MetaMask EncryptionPublicKey: User denied message EncryptionPublicKey.', + ), + ); + default: + return reject( + new Error( + `MetaMask EncryptionPublicKey: Unknown problem: ${JSON.stringify( + messageParams, + )}`, + ), + ); + } + }, + ); }); } @@ -167,7 +175,7 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< const messageId = messageData.id; await this.addMessage(messageData); - this.hub.emit(`unapprovedMessage`, { + this.messagingSystem.publish(`${this.name as string}:unapprovedMessage`, { ...updatedMessageParams, metamaskId: messageId, }); From b9015142ddbe58b41aa24cfd10b444e62e6f17cb Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Tue, 7 Jan 2025 11:12:52 +0100 Subject: [PATCH 09/12] Fix changelog --- packages/message-manager/CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/message-manager/CHANGELOG.md b/packages/message-manager/CHANGELOG.md index e023f006e8..773c18ae29 100644 --- a/packages/message-manager/CHANGELOG.md +++ b/packages/message-manager/CHANGELOG.md @@ -7,15 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Changed +### Changed - **BREAKING:** Base class of `DecryptMessageManager` and `EncryptionPublicKeyManager`(`AbstractMessageManager`) now expects new options to initialise ([#5103](https://github.com/MetaMask/core/pull/5103)) - **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. - -### Changed - - Bump `@metamask/base-controller` from `^7.0.0` to `^7.1.0` ([#5079](https://github.com/MetaMask/core/pull/5079)) ## [11.0.3] From 77ba063e02c0c3e98b4a1635d1fbf447570ede13 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Tue, 7 Jan 2025 11:57:16 +0100 Subject: [PATCH 10/12] Export UnapprovedMessage event types --- packages/message-manager/src/DecryptMessageManager.ts | 2 +- packages/message-manager/src/EncryptionPublicKeyManager.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index a3d2737ee5..49ce1553d9 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -18,7 +18,7 @@ import { normalizeMessageData, validateDecryptedMessageData } from './utils'; export type DecryptMessageManagerState = MessageManagerState; -type DecryptMessageManagerUnapprovedMessageAddedEvent = { +export type DecryptMessageManagerUnapprovedMessageAddedEvent = { type: `${string}:unapprovedMessage`; payload: [AbstractMessageParamsMetamask]; }; diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index dbda3725bc..0998c437c1 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -19,7 +19,7 @@ import { validateEncryptionPublicKeyMessageData } from './utils'; export type EncryptionPublicKeyManagerState = MessageManagerState; -type EncryptionPublicKeyManagerUnapprovedMessageAddedEvent = { +export type EncryptionPublicKeyManagerUnapprovedMessageAddedEvent = { type: `${string}:unapprovedMessage`; payload: [AbstractMessageParamsMetamask]; }; From e5e6db483ba26ef3cf02cc81aed33c5e0abfb7be Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Tue, 7 Jan 2025 12:42:51 +0100 Subject: [PATCH 11/12] Add missing test state --- .../src/AbstractMessageManager.test.ts | 16 +++++++++++++++- .../src/DecryptMessageManager.test.ts | 1 - .../message-manager/src/DecryptMessageManager.ts | 10 +++++----- .../src/EncryptionPublicKeyManager.test.ts | 1 - .../src/EncryptionPublicKeyManager.ts | 8 ++++---- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index e1360d2086..fcf520854c 100644 --- a/packages/message-manager/src/AbstractMessageManager.test.ts +++ b/packages/message-manager/src/AbstractMessageManager.test.ts @@ -547,7 +547,21 @@ describe('AbstractTestManager', () => { describe('clearUnapprovedMessages', () => { it('clears the unapproved messages', () => { - const controller = new AbstractTestManager(MOCK_INITIAL_OPTIONS); + 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); }); diff --git a/packages/message-manager/src/DecryptMessageManager.test.ts b/packages/message-manager/src/DecryptMessageManager.test.ts index 6614e221c4..1e59533ae5 100644 --- a/packages/message-manager/src/DecryptMessageManager.test.ts +++ b/packages/message-manager/src/DecryptMessageManager.test.ts @@ -11,7 +11,6 @@ const mockMessenger = { const mockInitialOptions = { additionalFinishStatuses: undefined, messenger: mockMessenger, - name: 'DecryptMessageManager', securityProviderRequest: undefined, }; diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index 49ce1553d9..e3dcf7fd2c 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -16,10 +16,12 @@ import type { import { AbstractMessageManager } from './AbstractMessageManager'; import { normalizeMessageData, validateDecryptedMessageData } from './utils'; +const managerName = 'DecryptMessageManager'; + export type DecryptMessageManagerState = MessageManagerState; export type DecryptMessageManagerUnapprovedMessageAddedEvent = { - type: `${string}:unapprovedMessage`; + type: `${typeof managerName}:unapprovedMessage`; payload: [AbstractMessageParamsMetamask]; }; @@ -33,7 +35,6 @@ export type DecryptMessageManagerMessenger = RestrictedControllerMessenger< type DecryptMessageManagerOptions = { messenger: DecryptMessageManagerMessenger; - name: string; securityProviderRequest?: SecurityProviderRequest; state?: MessageManagerState; additionalFinishStatuses?: string[]; @@ -94,14 +95,13 @@ export class DecryptMessageManager extends AbstractMessageManager< constructor({ additionalFinishStatuses, messenger, - name, securityProviderRequest, state, }: DecryptMessageManagerOptions) { super({ additionalFinishStatuses, messenger, - name, + name: managerName, securityProviderRequest, state, }); @@ -184,7 +184,7 @@ export class DecryptMessageManager extends AbstractMessageManager< const messageId = messageData.id; await this.addMessage(messageData); - this.messagingSystem.publish(`${this.name as string}:unapprovedMessage`, { + this.messagingSystem.publish(`${managerName}:unapprovedMessage`, { ...updatedMessageParams, metamaskId: messageId, }); diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts index d84b7ed17a..81735a5fda 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts @@ -11,7 +11,6 @@ const mockMessenger = { const mockInitialOptions = { additionalFinishStatuses: undefined, messenger: mockMessenger, - name: 'EncryptionPublicKeyManager', securityProviderRequest: undefined, }; diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index 0998c437c1..d396aa034b 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -16,11 +16,13 @@ import type { import { AbstractMessageManager } from './AbstractMessageManager'; import { validateEncryptionPublicKeyMessageData } from './utils'; +const managerName = 'EncryptionPublicKeyManager'; + export type EncryptionPublicKeyManagerState = MessageManagerState; export type EncryptionPublicKeyManagerUnapprovedMessageAddedEvent = { - type: `${string}:unapprovedMessage`; + type: `${typeof managerName}:unapprovedMessage`; payload: [AbstractMessageParamsMetamask]; }; @@ -34,7 +36,6 @@ export type EncryptionPublicKeyManagerMessenger = RestrictedControllerMessenger< type EncryptionPublicKeyManagerOptions = { messenger: EncryptionPublicKeyManagerMessenger; - name: string; securityProviderRequest?: SecurityProviderRequest; state?: MessageManagerState; additionalFinishStatuses?: string[]; @@ -92,14 +93,13 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< constructor({ additionalFinishStatuses, messenger, - name, securityProviderRequest, state, }: EncryptionPublicKeyManagerOptions) { super({ additionalFinishStatuses, messenger, - name, + name: managerName, securityProviderRequest, state, }); From b273f9fdb176ba78b853f760c1b072ff26828e31 Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Thu, 9 Jan 2025 14:40:44 +0100 Subject: [PATCH 12/12] Fix suggestions --- packages/message-manager/CHANGELOG.md | 5 ++++- .../message-manager/src/AbstractMessageManager.ts | 5 ++--- .../message-manager/src/DecryptMessageManager.ts | 13 +++++++++++-- .../src/EncryptionPublicKeyManager.ts | 13 +++++++++++-- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/packages/message-manager/CHANGELOG.md b/packages/message-manager/CHANGELOG.md index 773c18ae29..f9ca2a8819 100644 --- a/packages/message-manager/CHANGELOG.md +++ b/packages/message-manager/CHANGELOG.md @@ -10,10 +10,13 @@ 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. -- Bump `@metamask/base-controller` from `^7.0.0` to `^7.1.0` ([#5079](https://github.com/MetaMask/core/pull/5079)) ## [11.0.3] diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index 34769fadc4..10bcc6d452 100644 --- a/packages/message-manager/src/AbstractMessageManager.ts +++ b/packages/message-manager/src/AbstractMessageManager.ts @@ -100,7 +100,7 @@ export type MessageManagerState = { export type UpdateBadgeEvent = { type: `${string}:updateBadge`; - payload: [unusedPayload: string]; + payload: []; }; /** @@ -245,8 +245,7 @@ export abstract class AbstractMessageManager< state.unapprovedMessagesCount = this.getUnapprovedMessagesCount(); }); if (emitUpdateBadge) { - // Empty payload is used to satisfy event constraint for BaseControllerV2 - this.messagingSystem.publish(`${this.name as string}:updateBadge`, ''); + this.messagingSystem.publish(`${this.name as string}:updateBadge`); } } diff --git a/packages/message-manager/src/DecryptMessageManager.ts b/packages/message-manager/src/DecryptMessageManager.ts index e3dcf7fd2c..571ca9e760 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -25,10 +25,17 @@ export type DecryptMessageManagerUnapprovedMessageAddedEvent = { payload: [AbstractMessageParamsMetamask]; }; +export type DecryptMessageManagerUpdateBadgeEvent = { + type: `${typeof managerName}:updateBadge`; + payload: []; +}; + export type DecryptMessageManagerMessenger = RestrictedControllerMessenger< string, ActionConstraint, - EventConstraint | DecryptMessageManagerUnapprovedMessageAddedEvent, + | EventConstraint + | DecryptMessageManagerUnapprovedMessageAddedEvent + | DecryptMessageManagerUpdateBadgeEvent, string, string >; @@ -90,7 +97,9 @@ export class DecryptMessageManager extends AbstractMessageManager< DecryptMessageParams, DecryptMessageParamsMetamask, ActionConstraint, - EventConstraint | DecryptMessageManagerUnapprovedMessageAddedEvent + | EventConstraint + | DecryptMessageManagerUnapprovedMessageAddedEvent + | DecryptMessageManagerUpdateBadgeEvent > { constructor({ additionalFinishStatuses, diff --git a/packages/message-manager/src/EncryptionPublicKeyManager.ts b/packages/message-manager/src/EncryptionPublicKeyManager.ts index d396aa034b..6654bc0132 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -26,10 +26,17 @@ export type EncryptionPublicKeyManagerUnapprovedMessageAddedEvent = { payload: [AbstractMessageParamsMetamask]; }; +export type EncryptionPublicKeyManagerUpdateBadgeEvent = { + type: `${typeof managerName}:updateBadge`; + payload: []; +}; + export type EncryptionPublicKeyManagerMessenger = RestrictedControllerMessenger< string, ActionConstraint, - EventConstraint | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent, + | EventConstraint + | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent + | EncryptionPublicKeyManagerUpdateBadgeEvent, string, string >; @@ -88,7 +95,9 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager< EncryptionPublicKeyParams, EncryptionPublicKeyParamsMetamask, ActionConstraint, - EventConstraint | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent + | EventConstraint + | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent + | EncryptionPublicKeyManagerUpdateBadgeEvent > { constructor({ additionalFinishStatuses,