diff --git a/packages/message-manager/CHANGELOG.md b/packages/message-manager/CHANGELOG.md index ac90e79a53d..f9ca2a88193 100644 --- a/packages/message-manager/CHANGELOG.md +++ b/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 diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index 740ed38e2ca..fcf520854c0 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 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); + }); + }); }); diff --git a/packages/message-manager/src/AbstractMessageManager.ts b/packages/message-manager/src/AbstractMessageManager.ts index bdd8401f54c..10bcc6d4520 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: false, anonymous: false }, + unapprovedMessagesCount: { persist: false, 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,15 @@ 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; -} +}; + +export type UpdateBadgeEvent = { + type: `${string}:updateBadge`; + payload: []; +}; /** * A function for verifying a message, whether it is malicious or not @@ -108,32 +111,82 @@ 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; +/** + * 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, +> = { + additionalFinishStatuses?: string[]; + messenger: RestrictedControllerMessenger< + string, + Action, + Event | UpdateBadgeEvent, + 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< + string, + Action, + Event | UpdateBadgeEvent, + string, + string + > +> { + protected messages: Message[]; private readonly securityProviderRequest: SecurityProviderRequest | undefined; private readonly additionalFinishStatuses: string[]; + internalEvents = new EventEmitter(); + + 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,11 +236,16 @@ 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'); + this.messagingSystem.publish(`${this.name as string}:updateBadge`); } } @@ -200,18 +258,26 @@ 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.hub.emit(`${messageId}:${status}`, message); + const updatedMessage = { + ...message, + status, + }; + this.updateMessage(updatedMessage); + this.internalEvents.emit(`${messageId}:${status}`, updatedMessage); if ( status === 'rejected' || status === 'signed' || status === 'errored' || this.additionalFinishStatuses.includes(status) ) { - this.hub.emit(`${messageId}:finished`, message); + this.internalEvents.emit( + `${messageId as string}:finished`, + updatedMessage, + ); } } @@ -222,7 +288,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 +303,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,42 +317,11 @@ 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(); + clearUnapprovedMessages() { + this.update((state) => { + state.unapprovedMessages = {}; + state.unapprovedMessagesCount = 0; + }); } /** @@ -306,10 +341,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 +353,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 +387,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 +449,13 @@ export abstract class AbstractMessageManager< if (!message) { return; } - message.rawSig = result; - this.updateMessage(message, false); + this.updateMessage( + { + ...message, + rawSig: result, + }, + false, + ); } /** @@ -424,14 +464,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 +487,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 +502,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 +529,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.internalEvents.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 d81b336141a..1e59533ae5f 100644 --- a/packages/message-manager/src/DecryptMessageManager.test.ts +++ b/packages/message-manager/src/DecryptMessageManager.test.ts @@ -1,4 +1,18 @@ 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, + securityProviderRequest: undefined, +}; describe('DecryptMessageManager', () => { let controller: DecryptMessageManager; @@ -9,7 +23,7 @@ describe('DecryptMessageManager', () => { const dataMock = '0x12345'; beforeEach(() => { - controller = new DecryptMessageManager(); + controller = new DecryptMessageManager(mockInitialOptions); }); it('sets default state', () => { @@ -19,10 +33,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 +62,7 @@ describe('DecryptMessageManager', () => { describe('addUnapprovedMessageAsync', () => { beforeEach(() => { - controller = new DecryptMessageManager(undefined, undefined, undefined, [ - 'decrypted', - ]); + controller = new DecryptMessageManager(mockInitialOptions); jest .spyOn(controller, 'addUnapprovedMessage') @@ -72,7 +80,7 @@ describe('DecryptMessageManager', () => { data: dataMock, }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'decrypted', rawSig: rawSigMock, }); @@ -88,7 +96,7 @@ describe('DecryptMessageManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'rejected', }); }, 100); @@ -105,7 +113,7 @@ describe('DecryptMessageManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'errored', }); }, 100); @@ -122,7 +130,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 4036e79d901..571ca9e760d 100644 --- a/packages/message-manager/src/DecryptMessageManager.ts +++ b/packages/message-manager/src/DecryptMessageManager.ts @@ -1,14 +1,52 @@ +import type { + ActionConstraint, + EventConstraint, + 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 managerName = 'DecryptMessageManager'; + +export type DecryptMessageManagerState = MessageManagerState; + +export type DecryptMessageManagerUnapprovedMessageAddedEvent = { + type: `${typeof managerName}:unapprovedMessage`; + payload: [AbstractMessageParamsMetamask]; +}; + +export type DecryptMessageManagerUpdateBadgeEvent = { + type: `${typeof managerName}:updateBadge`; + payload: []; +}; + +export type DecryptMessageManagerMessenger = RestrictedControllerMessenger< + string, + ActionConstraint, + | EventConstraint + | DecryptMessageManagerUnapprovedMessageAddedEvent + | DecryptMessageManagerUpdateBadgeEvent, + string, + string +>; + +type DecryptMessageManagerOptions = { + messenger: DecryptMessageManagerMessenger; + securityProviderRequest?: SecurityProviderRequest; + state?: MessageManagerState; + additionalFinishStatuses?: string[]; +}; + /** * @type DecryptMessage * @@ -19,12 +57,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 +67,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 +95,26 @@ export interface DecryptMessageParamsMetamask export class DecryptMessageManager extends AbstractMessageManager< DecryptMessage, DecryptMessageParams, - DecryptMessageParamsMetamask + DecryptMessageParamsMetamask, + ActionConstraint, + | EventConstraint + | DecryptMessageManagerUnapprovedMessageAddedEvent + | DecryptMessageManagerUpdateBadgeEvent > { - /** - * Name of this controller used during composition - */ - override name = 'DecryptMessageManager' as const; + constructor({ + additionalFinishStatuses, + messenger, + securityProviderRequest, + state, + }: DecryptMessageManagerOptions) { + super({ + additionalFinishStatuses, + messenger, + name: managerName, + securityProviderRequest, + state, + }); + } /** * Creates a new Message with an 'unapproved' status using the passed messageParams. @@ -86,32 +132,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, + )}`, + ), + ); + } + }, + ); }); } @@ -144,7 +193,7 @@ export class DecryptMessageManager extends AbstractMessageManager< const messageId = messageData.id; await this.addMessage(messageData); - this.hub.emit(`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 8617c165340..81735a5fdaa 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.test.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.test.ts @@ -1,4 +1,18 @@ 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, + securityProviderRequest: undefined, +}; describe('EncryptionPublicKeyManager', () => { let controller: EncryptionPublicKeyManager; @@ -8,7 +22,7 @@ describe('EncryptionPublicKeyManager', () => { const rawSigMock = '231124fe67213512='; beforeEach(() => { - controller = new EncryptionPublicKeyManager(); + controller = new EncryptionPublicKeyManager(mockInitialOptions); }); it('sets default state', () => { @@ -18,10 +32,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 +58,7 @@ describe('EncryptionPublicKeyManager', () => { describe('addUnapprovedMessageAsync', () => { beforeEach(() => { - controller = new EncryptionPublicKeyManager( - undefined, - undefined, - undefined, - ['received'], - ); + controller = new EncryptionPublicKeyManager(mockInitialOptions); jest .spyOn(controller, 'addUnapprovedMessage') @@ -70,7 +75,7 @@ describe('EncryptionPublicKeyManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'received', rawSig: rawSigMock, }); @@ -85,7 +90,7 @@ describe('EncryptionPublicKeyManager', () => { }); setTimeout(() => { - controller.hub.emit(`${messageIdMock}:finished`, { + controller.internalEvents.emit(`${messageIdMock}:finished`, { status: 'rejected', }); }, 100); @@ -101,7 +106,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 6fdf1518f70..6654bc01329 100644 --- a/packages/message-manager/src/EncryptionPublicKeyManager.ts +++ b/packages/message-manager/src/EncryptionPublicKeyManager.ts @@ -1,14 +1,53 @@ +import type { + ActionConstraint, + EventConstraint, + 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 managerName = 'EncryptionPublicKeyManager'; + +export type EncryptionPublicKeyManagerState = + MessageManagerState; + +export type EncryptionPublicKeyManagerUnapprovedMessageAddedEvent = { + type: `${typeof managerName}:unapprovedMessage`; + payload: [AbstractMessageParamsMetamask]; +}; + +export type EncryptionPublicKeyManagerUpdateBadgeEvent = { + type: `${typeof managerName}:updateBadge`; + payload: []; +}; + +export type EncryptionPublicKeyManagerMessenger = RestrictedControllerMessenger< + string, + ActionConstraint, + | EventConstraint + | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent + | EncryptionPublicKeyManagerUpdateBadgeEvent, + string, + string +>; + +type EncryptionPublicKeyManagerOptions = { + messenger: EncryptionPublicKeyManagerMessenger; + securityProviderRequest?: SecurityProviderRequest; + state?: MessageManagerState; + additionalFinishStatuses?: string[]; +}; + /** * @type EncryptionPublicKey * @@ -20,12 +59,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 +82,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 +93,26 @@ export interface EncryptionPublicKeyParamsMetamask export class EncryptionPublicKeyManager extends AbstractMessageManager< EncryptionPublicKey, EncryptionPublicKeyParams, - EncryptionPublicKeyParamsMetamask + EncryptionPublicKeyParamsMetamask, + ActionConstraint, + | EventConstraint + | EncryptionPublicKeyManagerUnapprovedMessageAddedEvent + | EncryptionPublicKeyManagerUpdateBadgeEvent > { - /** - * Name of this controller used during composition - */ - override name = 'EncryptionPublicKeyManager' as const; + constructor({ + additionalFinishStatuses, + messenger, + securityProviderRequest, + state, + }: EncryptionPublicKeyManagerOptions) { + super({ + additionalFinishStatuses, + messenger, + name: managerName, + securityProviderRequest, + state, + }); + } /** * Creates a new Message with an 'unapproved' status using the passed messageParams. @@ -83,26 +130,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, + )}`, + ), + ); + } + }, + ); }); } @@ -134,7 +184,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, });