Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: Use latest DecryptMessageManager EncryptMessageManager to extend controllers #29237

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -1049,12 +1049,12 @@ export function setupController(
//
updateBadge();

controller.decryptMessageController.hub.on(
METAMASK_CONTROLLER_EVENTS.UPDATE_BADGE,
controller.controllerMessenger.subscribe(
METAMASK_CONTROLLER_EVENTS.DECRYPT_MESSAGE_MANAGER_UPDATE_BADGE,
updateBadge,
);
controller.encryptionPublicKeyController.hub.on(
METAMASK_CONTROLLER_EVENTS.UPDATE_BADGE,
controller.controllerMessenger.subscribe(
METAMASK_CONTROLLER_EVENTS.ENCRYPTION_PUBLIC_KEY_MANAGER_UPDATE_BADGE,
updateBadge,
);
controller.signatureController.hub.on(
Expand Down
37 changes: 12 additions & 25 deletions app/scripts/controllers/decrypt-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
DecryptMessageManager,
DecryptMessageParams,
} from '@metamask/message-manager';
import type { DecryptMessageManagerMessenger } from '@metamask/message-manager';
import { MetaMetricsEventCategory } from '../../../shared/constants/metametrics';
import DecryptMessageController, {
DecryptMessageControllerMessenger,
Expand Down Expand Up @@ -36,11 +37,15 @@ const createMessengerMock = () =>
({
registerActionHandler: jest.fn(),
registerInitialEventPayload: jest.fn(),
subscribe: jest.fn(),
publish: jest.fn(),
call: jest.fn(),
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any as jest.Mocked<DecryptMessageControllerMessenger>);
} as unknown as jest.Mocked<DecryptMessageControllerMessenger>);

const createManagerMessengerMock = () =>
({
subscribe: jest.fn(),
} as unknown as jest.Mocked<DecryptMessageManagerMessenger>);

const createDecryptMessageManagerMock = <T>() =>
({
Expand All @@ -64,20 +69,14 @@ const createDecryptMessageManagerMock = <T>() =>
} as any as jest.Mocked<T>);

describe('DecryptMessageController', () => {
class MockDecryptMessageController extends DecryptMessageController {
// update is protected, so we expose it for typechecking here
public update(callback: Parameters<DecryptMessageController['update']>[0]) {
return super.update(callback);
}
}

let decryptMessageController: MockDecryptMessageController;
let decryptMessageController: DecryptMessageController;

const decryptMessageManagerConstructorMock =
DecryptMessageManager as jest.MockedClass<typeof DecryptMessageManager>;
const getStateMock = jest.fn();
const keyringControllerMock = createKeyringControllerMock();
const messengerMock = createMessengerMock();
const managerMessengerMock = createManagerMessengerMock();
const metricsEventMock = jest.fn();

const decryptMessageManagerMock =
Expand Down Expand Up @@ -105,7 +104,7 @@ describe('DecryptMessageController', () => {
decryptMessageManagerMock,
);

decryptMessageController = new MockDecryptMessageController({
decryptMessageController = new DecryptMessageController({
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
getState: getStateMock as any,
Expand All @@ -118,6 +117,7 @@ describe('DecryptMessageController', () => {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
metricsEvent: metricsEventMock as any,
managerMessenger: managerMessengerMock,
} as DecryptMessageControllerOptions);
});

Expand All @@ -127,23 +127,10 @@ describe('DecryptMessageController', () => {
});

it('should reset state', () => {
decryptMessageController.update(() => ({
unapprovedDecryptMsgs: {
[messageIdMock]: messageMock,
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any,
unapprovedDecryptMsgCount: 1,
}));
decryptMessageController.resetState();
expect(decryptMessageController.state).toStrictEqual(getDefaultState());
});

it('should clear unapproved messages', () => {
decryptMessageController.clearUnapproved();
expect(decryptMessageController.state).toStrictEqual(getDefaultState());
expect(decryptMessageManagerMock.update).toBeCalledTimes(1);
});
it('should add unapproved messages', async () => {
await decryptMessageController.newRequestDecryptMessage(
messageMock,
Expand Down
100 changes: 49 additions & 51 deletions app/scripts/controllers/decrypt-message.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import EventEmitter from 'events';
import log from 'loglevel';
import {
AbstractMessage,
AbstractMessageManager,
AbstractMessageParams,
AbstractMessageParamsMetamask,
MessageManagerState,
Expand All @@ -11,6 +9,11 @@ import {
DecryptMessageParams,
DecryptMessageParamsMetamask,
} from '@metamask/message-manager';
import type {
DecryptMessageManagerMessenger,
DecryptMessageManagerState,
DecryptMessageManagerUnapprovedMessageAddedEvent,
} from '@metamask/message-manager';
import {
BaseController,
RestrictedControllerMessenger,
Expand All @@ -34,6 +37,8 @@ const stateMetadata = {
unapprovedDecryptMsgCount: { persist: false, anonymous: false },
};

export const managerName = 'DecryptMessageManager';

/**
* Type guard that checks for the presence of the required properties
* for EIP-1024 encrypted data.
Expand Down Expand Up @@ -86,38 +91,49 @@ export type DecryptMessageControllerState = {
unapprovedDecryptMsgCount: number;
};

export type GetDecryptMessageState = {
export type GetDecryptMessageControllerState = {
type: `${typeof controllerName}:getState`;
handler: () => DecryptMessageControllerState;
};

export type DecryptMessageStateChange = {
export type DecryptMessageControllerStateChange = {
type: `${typeof controllerName}:stateChange`;
payload: [DecryptMessageControllerState, Patch[]];
};

export type DecryptMessageControllerActions = GetDecryptMessageState;
export type DecryptMessageControllerActions = GetDecryptMessageControllerState;

export type DecryptMessageControllerEvents = DecryptMessageStateChange;
export type DecryptMessageControllerEvents =
DecryptMessageControllerStateChange;

type AllowedActions =
| AddApprovalRequest
| AcceptRequest
| RejectRequest
| KeyringControllerDecryptMessageAction;

type DecryptMessageManagerStateChangeEvent = {
type: `DecryptMessageManager:stateChange`;
payload: [DecryptMessageManagerState, Patch[]];
};

type AllowedEvents =
| DecryptMessageManagerStateChangeEvent
| DecryptMessageManagerUnapprovedMessageAddedEvent;

export type DecryptMessageControllerMessenger = RestrictedControllerMessenger<
typeof controllerName,
DecryptMessageControllerActions | AllowedActions,
DecryptMessageControllerEvents,
DecryptMessageControllerEvents | AllowedEvents,
AllowedActions['type'],
never
AllowedEvents['type']
>;

export type DecryptMessageControllerOptions = {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
getState: () => any;
managerMessenger: DecryptMessageManagerMessenger;
messenger: DecryptMessageControllerMessenger;
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand All @@ -132,8 +148,6 @@ export default class DecryptMessageController extends BaseController<
DecryptMessageControllerState,
DecryptMessageControllerMessenger
> {
hub: EventEmitter;

// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private _getState: () => any;
Expand All @@ -151,11 +165,13 @@ export default class DecryptMessageController extends BaseController<
* @param options.getState - Callback to retrieve all user state.
* @param options.messenger - A reference to the messaging system.
* @param options.metricsEvent - A function for emitting a metric event.
* @param options.managerMessenger - A reference to the messenger need by the message manager.
*/
constructor({
getState,
metricsEvent,
messenger,
managerMessenger,
}: DecryptMessageControllerOptions) {
super({
metadata: stateMetadata,
Expand All @@ -166,28 +182,18 @@ export default class DecryptMessageController extends BaseController<
this._getState = getState;
this._metricsEvent = metricsEvent;

this.hub = new EventEmitter();

this._decryptMessageManager = new DecryptMessageManager(
undefined,
undefined,
undefined,
['decrypted'],
);

this._decryptMessageManager.hub.on('updateBadge', () => {
this.hub.emit('updateBadge');
this._decryptMessageManager = new DecryptMessageManager({
Comment on lines -178 to -179
Copy link
Member Author

Choose a reason for hiding this comment

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

XMessageManagers will be emitting the X:updateBadge via messaging system hence these are not needed anymore, we directly listen manager event on background.js

additionalFinishStatuses: ['decrypted'],
messenger: managerMessenger,
});

this._decryptMessageManager.hub.on(
'unapprovedMessage',
(messageParams: AbstractMessageParamsMetamask) => {
this._requestApproval(messageParams);
},
messenger.subscribe(
`${managerName}:unapprovedMessage`,
this._requestApproval.bind(this),
);

this._subscribeToMessageState(
this._decryptMessageManager,
messenger,
(state, newMessages, messageCount) => {
state.unapprovedDecryptMsgs = newMessages;
state.unapprovedDecryptMsgCount = messageCount;
Expand Down Expand Up @@ -215,10 +221,7 @@ export default class DecryptMessageController extends BaseController<
* Clears all unapproved messages from memory.
*/
clearUnapproved() {
this._decryptMessageManager.update({
unapprovedMessages: {},
unapprovedMessagesCount: 0,
});
this._decryptMessageManager.clearUnapprovedMessages();
}

/**
Expand Down Expand Up @@ -331,11 +334,7 @@ export default class DecryptMessageController extends BaseController<
}

private _cancelAbstractMessage(
messageManager: AbstractMessageManager<
AbstractMessage,
AbstractMessageParams,
AbstractMessageParamsMetamask
>,
messageManager: DecryptMessageManager,
messageId: string,
reason?: string,
) {
Expand All @@ -356,27 +355,26 @@ export default class DecryptMessageController extends BaseController<
}

private _subscribeToMessageState(
messageManager: AbstractMessageManager<
AbstractMessage,
AbstractMessageParams,
AbstractMessageParamsMetamask
>,
controllerMessenger: DecryptMessageControllerMessenger,
updateState: (
state: DecryptMessageControllerState,
newMessages: Record<string, StateMessage>,
messageCount: number,
) => void,
) {
messageManager.subscribe((state: MessageManagerState<AbstractMessage>) => {
const newMessages = this._migrateMessages(
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
state.unapprovedMessages as any,
);
this.update((draftState) => {
updateState(draftState, newMessages, state.unapprovedMessagesCount);
});
});
controllerMessenger.subscribe(
`${managerName}:stateChange`,
(state: MessageManagerState<AbstractMessage>) => {
const newMessages = this._migrateMessages(
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
state.unapprovedMessages as any,
);
this.update((draftState) => {
updateState(draftState, newMessages, state.unapprovedMessagesCount);
});
},
);
}

private _migrateMessages(
Expand Down
Loading
Loading