Skip to content

Commit

Permalink
refactor: remove eth-keyring-controller from DecryptMessageController
Browse files Browse the repository at this point in the history
  • Loading branch information
mikesposito committed Sep 12, 2023
1 parent 973e9ce commit 8ef55f4
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 37 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
diff --git a/dist/KeyringController.d.ts b/dist/KeyringController.d.ts
index ccfd5ca9accf782b0a612ab1666501898cf2abb9..f772552156491b308f2aa29a734a5138237d951e 100644
index d6d70eea2de56310b49fa9dcf1e90afae3ddb202..4518aed8e03376d76e5ab6ef0971529e65563fd3 100644
--- a/dist/KeyringController.d.ts
+++ b/dist/KeyringController.d.ts
@@ -1,10 +1,11 @@
Expand All @@ -10,13 +10,12 @@ index ccfd5ca9accf782b0a612ab1666501898cf2abb9..f772552156491b308f2aa29a734a5138
import { BaseControllerV2 } from '@metamask/base-controller';
import type { PersonalMessageParams, TypedMessageParams } from '@metamask/message-manager';
import type { PreferencesController } from '@metamask/preferences-controller';
-import { type Hex, type Keyring, type Json } from '@metamask/utils';
+import type { Hex, Keyring, Json } from '@metamask/utils';
import type { Eip1024EncryptedData, Hex, Keyring, Json } from '@metamask/utils';
+import type { KeyringController as EthKeyringController } from '@metamask/eth-keyring-controller';
import type { Patch } from 'immer';
declare const name = "KeyringController";
/**
@@ -135,6 +136,10 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
@@ -155,6 +156,10 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
* @param opts.state - Initial state to set on this controller.
*/
constructor({ removeIdentity, syncIdentities, updateIdentities, setSelectedAddress, setAccountLabel, encryptor, keyringBuilders, cacheEncryptionKey, messenger, state, }: KeyringControllerOptions);
Expand All @@ -28,12 +27,12 @@ index ccfd5ca9accf782b0a612ab1666501898cf2abb9..f772552156491b308f2aa29a734a5138
* Adds a new account to the default (first) HD seed phrase keyring.
*
diff --git a/dist/KeyringController.js b/dist/KeyringController.js
index 9e241ba3b5445a053e3b20ee70a579118b7b2934..eb1f7bce9d3607017b39e087db227e8a7dadd849 100644
index bce9884ea54bf8e24274dee3876db95ed862abf0..471a06b30f2275de821c33b7d94cb1de85b074c7 100644
--- a/dist/KeyringController.js
+++ b/dist/KeyringController.js
@@ -153,6 +153,12 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
this.setSelectedAddress = setSelectedAddress;
@@ -154,6 +154,12 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
this.setAccountLabel = setAccountLabel;
__classPrivateFieldGet(this, _KeyringController_instances, "m", _KeyringController_registerMessageHandlers).call(this);
}
+ /**
+ * Gets the internal keyring controller.
Expand Down
45 changes: 36 additions & 9 deletions app/scripts/controllers/decrypt-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const messageMock = {
type: 'testType',
rawSig: undefined,
} as any as AbstractMessage;
const messageDataMock =
'0x7b2276657273696f6e223a227832353531392d7873616c736132302d706f6c7931333035222c226e6f6e6365223a226b45586143524c3045646142766f77756e35675979357175784a4a6967304548222c22657068656d5075626c69634b6579223a224863334636506d314734385a567955424763365866537839682b77784b6958587238456a51434253466e553d222c2263697068657274657874223a22546a41556b68554a5968656e7a2f655a6e57454a2b31456c7861354f77765939613830507a62746c7a7a48746934634175525941227d';

const mockExtState = {};

Expand Down Expand Up @@ -52,7 +54,7 @@ const createDecryptMessageManagerMock = <T>() =>
},
} as any as jest.Mocked<T>);

describe('EncryptionPublicKeyController', () => {
describe('DecryptMessageController', () => {
let decryptMessageController: DecryptMessageController;

const decryptMessageManagerConstructorMock =
Expand All @@ -65,6 +67,19 @@ describe('EncryptionPublicKeyController', () => {
const decryptMessageManagerMock =
createDecryptMessageManagerMock<DecryptMessageManager>();

const mockMessengerAction = (
action: string,
callback: (actionName: string, ...args: any[]) => any,
) => {
messengerMock.call.mockImplementation((actionName, ...rest) => {
if (actionName === action) {
return callback(actionName, ...rest);
}

return Promise.resolve();
});
};

beforeEach(() => {
jest.resetAllMocks();

Expand Down Expand Up @@ -116,12 +131,18 @@ describe('EncryptionPublicKeyController', () => {
it('should decrypt message', async () => {
const messageToDecrypt = {
...messageMock,
data: '0x7b22666f6f223a22626172227d',
data: messageDataMock,
};
const decryptMessageActionCallbackMock = jest
.fn()
.mockReturnValue('decryptedMessage');
decryptMessageManagerMock.approveMessage.mockResolvedValue(
messageToDecrypt,
);
keyringControllerMock.decryptMessage.mockResolvedValue('decryptedMessage');
mockMessengerAction(
'KeyringController:decryptMessage',
decryptMessageActionCallbackMock,
);
getStateMock.mockReturnValue(mockExtState);

const result = await decryptMessageController.decryptMessage(
Expand All @@ -132,8 +153,9 @@ describe('EncryptionPublicKeyController', () => {
expect(decryptMessageManagerMock.approveMessage).toBeCalledWith(
messageToDecrypt,
);
expect(keyringControllerMock.decryptMessage).toBeCalledTimes(1);
expect(keyringControllerMock.decryptMessage).toBeCalledWith(
expect(decryptMessageActionCallbackMock).toBeCalledTimes(1);
expect(decryptMessageActionCallbackMock).toBeCalledWith(
'KeyringController:decryptMessage',
messageToDecrypt,
);
expect(decryptMessageManagerMock.setMessageStatusAndResult).toBeCalledTimes(
Expand All @@ -150,12 +172,14 @@ describe('EncryptionPublicKeyController', () => {
it('should cancel decrypt request', async () => {
const messageToDecrypt = {
...messageMock,
data: '0x7b22666f6f223a22626172227d',
data: messageDataMock,
};
decryptMessageManagerMock.approveMessage.mockResolvedValue(
messageToDecrypt,
);
keyringControllerMock.decryptMessage.mockRejectedValue(new Error('error'));
mockMessengerAction('KeyringController:decryptMessage', async () => {
throw new Error('error');
});
getStateMock.mockReturnValue(mockExtState);

return expect(
Expand All @@ -166,10 +190,13 @@ describe('EncryptionPublicKeyController', () => {
it('should decrypt message inline', async () => {
const messageToDecrypt = {
...messageMock,
data: '0x7b22666f6f223a22626172227d',
data: messageDataMock,
};
decryptMessageManagerMock.getMessage.mockReturnValue(messageToDecrypt);
keyringControllerMock.decryptMessage.mockResolvedValue('decryptedMessage');
mockMessengerAction(
'KeyringController:decryptMessage',
async () => 'decryptedMessage',
);
getStateMock.mockReturnValue(mockExtState);

const result = await decryptMessageController.decryptMessageInline(
Expand Down
49 changes: 39 additions & 10 deletions app/scripts/controllers/decrypt-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
DecryptMessageParams,
DecryptMessageParamsMetamask,
} from '@metamask/message-manager';
import { KeyringController } from '@metamask/eth-keyring-controller';
import {
AbstractMessage,
AbstractMessageManager,
Expand All @@ -25,6 +24,8 @@ import {
} from '@metamask/approval-controller';
import { ApprovalType, ORIGIN_METAMASK } from '@metamask/controller-utils';
import { Patch } from 'immer';
import type { KeyringControllerDecryptMessageAction } from '@metamask/keyring-controller';
import { Eip1024EncryptedData, hasProperty } from '@metamask/utils';
import { MetaMetricsEventCategory } from '../../../shared/constants/metametrics';
import { stripHexPrefix } from '../../../shared/modules/hexstring-utils';

Expand All @@ -35,6 +36,27 @@ const stateMetadata = {
unapprovedDecryptMsgCount: { persist: false, anonymous: false },
};

export const isEIP1024EncryptedMessage = (message: {
from: string;
data: unknown;
}): message is {
from: string;
data: Eip1024EncryptedData;
} => {
if (
message.data &&
typeof message.data === 'object' &&
hasProperty(message.data, 'version') &&
hasProperty(message.data, 'nonce') &&
hasProperty(message.data, 'ephemPublicKey') &&
hasProperty(message.data, 'ciphertext')
) {
return true;
}

return false;
};

export const getDefaultState = () => ({
unapprovedDecryptMsgs: {},
unapprovedDecryptMsgCount: 0,
Expand Down Expand Up @@ -70,7 +92,11 @@ export type DecryptMessageControllerActions = GetDecryptMessageState;

export type DecryptMessageControllerEvents = DecryptMessageStateChange;

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

export type DecryptMessageControllerMessenger = RestrictedControllerMessenger<
typeof controllerName,
Expand All @@ -82,7 +108,6 @@ export type DecryptMessageControllerMessenger = RestrictedControllerMessenger<

export type DecryptMessageControllerOptions = {
getState: () => any;
keyringController: KeyringController;
messenger: DecryptMessageControllerMessenger;
metricsEvent: (payload: any, options?: any) => void;
};
Expand All @@ -99,8 +124,6 @@ export default class DecryptMessageController extends BaseControllerV2<

private _getState: () => any;

private _keyringController: KeyringController;

private _metricsEvent: (payload: any, options?: any) => void;

private _decryptMessageManager: DecryptMessageManager;
Expand All @@ -110,13 +133,11 @@ export default class DecryptMessageController extends BaseControllerV2<
*
* @param options - The controller options.
* @param options.getState - Callback to retrieve all user state.
* @param options.keyringController - An instance of a keyring controller used to decrypt message
* @param options.messenger - A reference to the messaging system.
* @param options.metricsEvent - A function for emitting a metric event.
*/
constructor({
getState,
keyringController,
metricsEvent,
messenger,
}: DecryptMessageControllerOptions) {
Expand All @@ -127,7 +148,6 @@ export default class DecryptMessageController extends BaseControllerV2<
state: getDefaultState(),
});
this._getState = getState;
this._keyringController = keyringController;
this._metricsEvent = metricsEvent;

this.hub = new EventEmitter();
Expand Down Expand Up @@ -216,7 +236,12 @@ export default class DecryptMessageController extends BaseControllerV2<
await this._decryptMessageManager.approveMessage(messageParams);

cleanMessageParams.data = this._parseMessageData(cleanMessageParams.data);
const rawMessage = await this._keyringController.decryptMessage(
if (!isEIP1024EncryptedMessage(cleanMessageParams)) {
throw new Error('Invalid encrypted data.');
}

const rawMessage = await this.messagingSystem.call(
'KeyringController:decryptMessage',
cleanMessageParams,
);

Expand All @@ -243,7 +268,11 @@ export default class DecryptMessageController extends BaseControllerV2<
async decryptMessageInline(messageParams: DecryptMessageParamsMetamask) {
const messageId = messageParams.metamaskId as string;
messageParams.data = this._parseMessageData(messageParams.data);
const rawMessage = await this._keyringController.decryptMessage(
if (!isEIP1024EncryptedMessage(messageParams)) {
throw new Error('Invalid encrypted data.');
}
const rawMessage = await this.messagingSystem.call(
'KeyringController:decryptMessage',
messageParams,
);

Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@
"request@^2.88.2": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch",
"request@^2.85.0": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch",
"lavamoat-core@^14.2.0": "patch:lavamoat-core@npm%3A14.2.0#./.yarn/patches/lavamoat-core-npm-14.2.0-c453f4f755.patch",
"@metamask/keyring-controller@^7.2.0": "patch:@metamask/keyring-controller@npm%3A7.2.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch",
"@metamask/keyring-controller@^7.4.0": "patch:@metamask/keyring-controller@npm%3A7.4.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch",
"@metamask/signature-controller@^5.3.0": "patch:@metamask/signature-controller@npm%3A5.3.0#./.yarn/patches/@metamask-signature-controller-npm-5.3.0-225628460b.patch",
"[email protected]": "^7.5.4",
"[email protected]": "^7.5.4"
Expand Down Expand Up @@ -251,7 +251,7 @@
"@metamask/gas-fee-controller": "^6.0.1",
"@metamask/jazzicon": "^2.0.0",
"@metamask/key-tree": "^9.0.0",
"@metamask/keyring-controller": "^7.2.0",
"@metamask/keyring-controller": "^7.4.0",
"@metamask/logo": "^3.1.1",
"@metamask/message-manager": "^7.3.0",
"@metamask/metamask-eth-abis": "^3.0.0",
Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4285,9 +4285,9 @@ __metadata:
languageName: node
linkType: hard

"@metamask/keyring-controller@npm:7.2.0":
version: 7.2.0
resolution: "@metamask/keyring-controller@npm:7.2.0"
"@metamask/keyring-controller@npm:7.4.0":
version: 7.4.0
resolution: "@metamask/keyring-controller@npm:7.4.0"
dependencies:
"@keystonehq/metamask-airgapped-keyring": "npm:^0.13.1"
"@metamask/base-controller": "npm:^3.2.1"
Expand All @@ -4301,13 +4301,13 @@ __metadata:
immer: "npm:^9.0.6"
peerDependencies:
"@metamask/preferences-controller": ^4.4.0
checksum: 3535d698f68f732658787943eaff3ebabdc1e3d0e6bce731cff413b0fb98d19fb127841e525d9031fa96b50e75840686da9cd0615ab6772914e2b18fe1797df1
checksum: 17b3f24d57dc79689ee6c0159f931ba71b10cbb728cecce6595ec69c46bc706289e90e22b9e7709fef8719ff99594adccd842a3a687fcc651a55cd873f2bc219
languageName: node
linkType: hard

"@metamask/keyring-controller@patch:@metamask/keyring-controller@npm%3A7.2.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch":
version: 7.2.0
resolution: "@metamask/keyring-controller@patch:@metamask/keyring-controller@npm%3A7.2.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch::version=7.2.0&hash=6f51dc"
"@metamask/keyring-controller@patch:@metamask/keyring-controller@npm%3A7.4.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch":
version: 7.4.0
resolution: "@metamask/keyring-controller@patch:@metamask/keyring-controller@npm%3A7.4.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch::version=7.4.0&hash=4a7ca3"
dependencies:
"@keystonehq/metamask-airgapped-keyring": "npm:^0.13.1"
"@metamask/base-controller": "npm:^3.2.1"
Expand All @@ -4321,7 +4321,7 @@ __metadata:
immer: "npm:^9.0.6"
peerDependencies:
"@metamask/preferences-controller": ^4.4.0
checksum: b3f0ffac12077c19bef8d978237b24ebe40c2bbee1363ee5cfdf7a09b600540981d3bdbac96845645cd976e0bdb55f56f80442a2797e1538abb00744194edf17
checksum: d6965c9e3f7fe00481ed544fae172aa04f8212e570ccea4c1d16f52f42c775759df59a1009c52937e1f6c6a764c16b80b89c457a4765009ff9bb1d454aab42ab
languageName: node
linkType: hard

Expand Down Expand Up @@ -24221,7 +24221,7 @@ __metadata:
"@metamask/gas-fee-controller": "npm:^6.0.1"
"@metamask/jazzicon": "npm:^2.0.0"
"@metamask/key-tree": "npm:^9.0.0"
"@metamask/keyring-controller": "npm:^7.2.0"
"@metamask/keyring-controller": "npm:^7.4.0"
"@metamask/logo": "npm:^3.1.1"
"@metamask/message-manager": "npm:^7.3.0"
"@metamask/metamask-eth-abis": "npm:^3.0.0"
Expand Down

0 comments on commit 8ef55f4

Please sign in to comment.