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

Merged
merged 22 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from 9 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
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
81 changes: 43 additions & 38 deletions app/scripts/controllers/decrypt-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import EventEmitter from 'events';
import log from 'loglevel';
import {
AbstractMessage,
AbstractMessageManager,
AbstractMessageParams,
AbstractMessageParamsMetamask,
MessageManagerState,
Expand All @@ -11,6 +10,10 @@ import {
DecryptMessageParams,
DecryptMessageParamsMetamask,
} from '@metamask/message-manager';
import type {
DecryptMessageManagerMessenger,
DecryptMessageManagerState,
} from '@metamask/message-manager';
import {
BaseController,
RestrictedControllerMessenger,
Expand Down Expand Up @@ -86,38 +89,47 @@ 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;

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 Down Expand Up @@ -151,11 +163,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 @@ -168,12 +182,11 @@ export default class DecryptMessageController extends BaseController<

this.hub = new EventEmitter();

this._decryptMessageManager = new DecryptMessageManager(
undefined,
undefined,
undefined,
['decrypted'],
);
this._decryptMessageManager = new DecryptMessageManager({
messenger: managerMessenger,
name: 'DecryptMessageManager',
additionalFinishStatuses: ['decrypted'],
});

this._decryptMessageManager.hub.on('updateBadge', () => {
this.hub.emit('updateBadge');
Expand All @@ -187,7 +200,7 @@ export default class DecryptMessageController extends BaseController<
);

this._subscribeToMessageState(
this._decryptMessageManager,
messenger,
(state, newMessages, messageCount) => {
state.unapprovedDecryptMsgs = newMessages;
state.unapprovedDecryptMsgCount = messageCount;
Expand Down Expand Up @@ -215,10 +228,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 +341,7 @@ export default class DecryptMessageController extends BaseController<
}

private _cancelAbstractMessage(
messageManager: AbstractMessageManager<
AbstractMessage,
AbstractMessageParams,
AbstractMessageParamsMetamask
>,
messageManager: DecryptMessageManager,
messageId: string,
reason?: string,
) {
Expand All @@ -356,27 +362,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(
'DecryptMessageManager: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
66 changes: 12 additions & 54 deletions app/scripts/controllers/encryption-public-key.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
EncryptionPublicKeyManager,
AbstractMessage,
OriginalRequest,
EncryptionPublicKeyManagerMessenger,
} from '@metamask/message-manager';
import { KeyringType } from '../../../shared/constants/keyring';
import { MetaMetricsEventCategory } from '../../../shared/constants/metametrics';
Expand Down Expand Up @@ -34,20 +35,7 @@ const messageMock = {
status: 'unapproved',
type: 'testType',
rawSig: undefined,
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any as AbstractMessage;

const coreMessageMock = {
...messageMock,
messageParams: messageParamsMock,
};

const stateMessageMock = {
...messageMock,
msgParams: addressMock,
origin: messageParamsMock.origin,
};
} as unknown as AbstractMessage;

const requestMock = {
origin: 'http://test2.com',
Expand All @@ -57,11 +45,15 @@ const createMessengerMock = () =>
({
registerActionHandler: jest.fn(),
publish: jest.fn(),
subscribe: jest.fn(),
call: jest.fn(),
registerInitialEventPayload: jest.fn(),
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any as jest.Mocked<EncryptionPublicKeyControllerMessenger>);
} as unknown as jest.Mocked<EncryptionPublicKeyControllerMessenger>);

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

const createEncryptionPublicKeyManagerMock = <T>() =>
({
Expand All @@ -76,9 +68,7 @@ const createEncryptionPublicKeyManagerMock = <T>() =>
hub: {
on: jest.fn(),
},
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any as jest.Mocked<T>);
} as unknown as jest.Mocked<T>);

describe('EncryptionPublicKeyController', () => {
let encryptionPublicKeyController: EncryptionPublicKeyController;
Expand All @@ -90,6 +80,7 @@ describe('EncryptionPublicKeyController', () => {
const encryptionPublicKeyManagerMock =
createEncryptionPublicKeyManagerMock<EncryptionPublicKeyManager>();
const messengerMock = createMessengerMock();
const managerMessengerMock = createManagerMessengerMock();
const getEncryptionPublicKeyMock = jest.fn();
const getAccountKeyringTypeMock = jest.fn();
const getStateMock = jest.fn();
Expand Down Expand Up @@ -118,6 +109,7 @@ describe('EncryptionPublicKeyController', () => {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
metricsEvent: metricsEventMock as any,
managerMessenger: managerMessengerMock,
} as EncryptionPublicKeyControllerOptions);
});

Expand Down Expand Up @@ -198,22 +190,6 @@ describe('EncryptionPublicKeyController', () => {
});
});

describe('clearUnapproved', () => {
it('resets state in all message managers', () => {
encryptionPublicKeyController.clearUnapproved();

const defaultState = {
unapprovedMessages: {},
unapprovedMessagesCount: 0,
};

expect(encryptionPublicKeyManagerMock.update).toHaveBeenCalledTimes(1);
expect(encryptionPublicKeyManagerMock.update).toHaveBeenCalledWith(
defaultState,
);
});
});

describe('newRequestEncryptionPublicKey', () => {
// @ts-expect-error This function is missing from the Mocha type definitions
it.each([
Expand Down Expand Up @@ -407,23 +383,5 @@ describe('EncryptionPublicKeyController', () => {
true,
);
});

it('updates state on EncryptionPublicKeyManager state change', async () => {
await encryptionPublicKeyManagerMock.subscribe.mock.calls[0][0]({
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
unapprovedMessages: { [messageIdMock]: coreMessageMock as any },
unapprovedMessagesCount: 3,
});

expect(encryptionPublicKeyController.state).toEqual({
unapprovedEncryptionPublicKeyMsgs: {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[messageIdMock]: stateMessageMock as any,
},
unapprovedEncryptionPublicKeyMsgCount: 3,
});
});
});
});
Loading
Loading