Skip to content

Commit 29dcbc8

Browse files
authored
refactor: migrate KeyringController to @metamask/messenger (#6370)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> This PR migrates the `KeyringController` controller class to the new `@metamask/messenger` comm system, as opposed to the one exported from `@metamask/base-controller`. ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> * Related to #5626 ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Migrates `KeyringController` to `@metamask/messenger`, updates metadata flag and tests, and adds new messenger dependency/config refs. > > - **Keyring Controller**: > - Migrates messaging from `RestrictedMessenger` (`@metamask/base-controller`) to `Messenger` (`@metamask/messenger`); switches to `BaseController` from `@metamask/base-controller/next`. > - Replaces `messagingSystem.publish/registerActionHandler` with `messenger.publish/registerActionHandler` and updates `KeyringControllerMessenger` type. > - Renames metadata key `anonymous` to `includeInDebugSnapshot` and updates calls to `deriveStateFromMetadata`. > - **Tests**: > - Refactors to build a root `Messenger` and namespaced controller messenger; updates expectations for metadata snapshots and messaging APIs. > - **Repo/Config**: > - Adds dependency `@metamask/messenger` and TypeScript project references; updates README dependency graph to show `keyring_controller --> messenger` and `eip_7702_internal_rpc_middleware --> controller_utils`. > - **Changelog**: > - Notes BREAKING changes: switch to `@metamask/messenger` and metadata flag rename. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 7ad33d9. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent bfb8cbb commit 29dcbc8

File tree

8 files changed

+83
-51
lines changed

8 files changed

+83
-51
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ linkStyle default opacity:0.5
253253
gator_permissions_controller --> messenger;
254254
json_rpc_middleware_stream --> json_rpc_engine;
255255
keyring_controller --> base_controller;
256+
keyring_controller --> messenger;
256257
logging_controller --> base_controller;
257258
logging_controller --> controller_utils;
258259
message_manager --> base_controller;

packages/keyring-controller/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
12+
- **BREAKING:** Use new `Messenger` from `@metamask/messenger` ([#6370](https://github.com/MetaMask/core/pull/6370))
13+
- Previously, `KeyringController` accepted a `RestrictedMessenger` instance from `@metamask/base-controller`.
14+
- **BREAKING:** Metadata property `anonymous` renamed to `includeInDebugSnapshot` ([#6370](https://github.com/MetaMask/core/pull/6370))
15+
1016
## [23.2.0]
1117

1218
### Added

packages/keyring-controller/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
"@metamask/eth-simple-keyring": "^11.0.0",
5656
"@metamask/keyring-api": "^21.0.0",
5757
"@metamask/keyring-internal-api": "^9.0.0",
58+
"@metamask/messenger": "^0.3.0",
5859
"@metamask/utils": "^11.8.1",
5960
"async-mutex": "^0.5.0",
6061
"ethereumjs-wallet": "^1.0.1",

packages/keyring-controller/src/KeyringController.test.ts

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Chain, Common, Hardfork } from '@ethereumjs/common';
22
import type { TypedTxData } from '@ethereumjs/tx';
33
import { TransactionFactory } from '@ethereumjs/tx';
4-
import { Messenger, deriveStateFromMetadata } from '@metamask/base-controller';
4+
import { deriveStateFromMetadata } from '@metamask/base-controller/next';
55
import { HdKeyring } from '@metamask/eth-hd-keyring';
66
import {
77
normalize,
@@ -14,6 +14,13 @@ import {
1414
import SimpleKeyring from '@metamask/eth-simple-keyring';
1515
import type { EthKeyring } from '@metamask/keyring-internal-api';
1616
import type { KeyringClass } from '@metamask/keyring-utils';
17+
import {
18+
MOCK_ANY_NAMESPACE,
19+
Messenger,
20+
type MessengerActions,
21+
type MessengerEvents,
22+
type MockAnyNamespace,
23+
} from '@metamask/messenger';
1724
import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english';
1825
import { bytesToHex, isValidHexAddress, type Hex } from '@metamask/utils';
1926
import sinon from 'sinon';
@@ -44,6 +51,16 @@ import { MockKeyring } from '../tests/mocks/mockKeyring';
4451
import MockShallowKeyring from '../tests/mocks/mockShallowKeyring';
4552
import { buildMockTransaction } from '../tests/mocks/mockTransaction';
4653

54+
type AllKeyringControllerActions = MessengerActions<KeyringControllerMessenger>;
55+
56+
type AllKeyringControllerEvents = MessengerEvents<KeyringControllerMessenger>;
57+
58+
type RootMessenger = Messenger<
59+
MockAnyNamespace,
60+
AllKeyringControllerActions,
61+
AllKeyringControllerEvents
62+
>;
63+
4764
jest.mock('uuid', () => {
4865
return {
4966
...jest.requireActual('uuid'),
@@ -4272,7 +4289,7 @@ describe('KeyringController', () => {
42724289
deriveStateFromMetadata(
42734290
controller.state,
42744291
controller.metadata,
4275-
'anonymous',
4292+
'includeInDebugSnapshot',
42764293
),
42774294
).toMatchInlineSnapshot(`
42784295
Object {
@@ -4356,7 +4373,7 @@ type WithControllerCallback<ReturnValue> = ({
43564373
controller: KeyringController;
43574374
encryptor: MockEncryptor;
43584375
initialState: KeyringControllerState;
4359-
messenger: KeyringControllerMessenger;
4376+
messenger: RootMessenger;
43604377
}) => Promise<ReturnValue> | ReturnValue;
43614378

43624379
type WithControllerOptions = Partial<KeyringControllerOptions> & {
@@ -4387,27 +4404,28 @@ function stubKeyringClassWithAccount(
43874404
}
43884405

43894406
/**
4390-
* Build a messenger that includes all events used by the keyring
4391-
* controller.
4407+
* Build a root messenger.
43924408
*
4393-
* @returns The messenger.
4409+
* @returns The root messenger.
43944410
*/
4395-
function buildMessenger() {
4396-
return new Messenger<KeyringControllerActions, KeyringControllerEvents>();
4411+
function buildRootMessenger(): RootMessenger {
4412+
return new Messenger({ namespace: MOCK_ANY_NAMESPACE });
43974413
}
43984414

43994415
/**
4400-
* Build a restricted messenger for the keyring controller.
4416+
* Build a messenger for the keyring controller.
44014417
*
4402-
* @param messenger - A messenger.
4418+
* @param messenger - An optional root messenger to use as the base for the
4419+
* controller messenger
44034420
* @returns The keyring controller restricted messenger.
44044421
*/
4405-
function buildKeyringControllerMessenger(messenger = buildMessenger()) {
4406-
return messenger.getRestricted({
4407-
name: 'KeyringController',
4408-
allowedActions: [],
4409-
allowedEvents: [],
4410-
});
4422+
function buildKeyringControllerMessenger(messenger = buildRootMessenger()) {
4423+
return new Messenger<
4424+
'KeyringController',
4425+
KeyringControllerActions,
4426+
KeyringControllerEvents,
4427+
typeof messenger
4428+
>({ namespace: 'KeyringController', parent: messenger });
44114429
}
44124430

44134431
/**
@@ -4425,10 +4443,11 @@ async function withController<ReturnValue>(
44254443
): Promise<ReturnValue> {
44264444
const [{ ...rest }, fn] = args.length === 2 ? args : [{}, args[0]];
44274445
const encryptor = new MockEncryptor();
4428-
const messenger = buildKeyringControllerMessenger();
4446+
const messenger = buildRootMessenger();
4447+
const keyringControllerMessenger = buildKeyringControllerMessenger(messenger);
44294448
const controller = new KeyringController({
44304449
encryptor,
4431-
messenger,
4450+
messenger: keyringControllerMessenger,
44324451
...rest,
44334452
});
44344453
if (!rest.skipVaultCreation) {

packages/keyring-controller/src/KeyringController.ts

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { TypedTransaction, TypedTxData } from '@ethereumjs/tx';
22
import { isValidPrivate, getBinarySize } from '@ethereumjs/util';
3-
import type { RestrictedMessenger } from '@metamask/base-controller';
4-
import { BaseController } from '@metamask/base-controller';
3+
import { BaseController } from '@metamask/base-controller/next';
54
import * as encryptorUtils from '@metamask/browser-passworder';
65
import { HdKeyring } from '@metamask/eth-hd-keyring';
76
import { normalize as ethNormalize } from '@metamask/eth-sig-util';
@@ -15,6 +14,7 @@ import type {
1514
} from '@metamask/keyring-api';
1615
import type { EthKeyring } from '@metamask/keyring-internal-api';
1716
import type { KeyringClass } from '@metamask/keyring-utils';
17+
import type { Messenger } from '@metamask/messenger';
1818
import type { Eip1024EncryptedData, Hex, Json } from '@metamask/utils';
1919
import {
2020
add0x,
@@ -246,12 +246,10 @@ export type KeyringControllerEvents =
246246
| KeyringControllerUnlockEvent
247247
| KeyringControllerAccountRemovedEvent;
248248

249-
export type KeyringControllerMessenger = RestrictedMessenger<
249+
export type KeyringControllerMessenger = Messenger<
250250
typeof name,
251251
KeyringControllerActions,
252-
KeyringControllerEvents,
253-
never,
254-
never
252+
KeyringControllerEvents
255253
>;
256254

257255
export type KeyringControllerOptions = {
@@ -694,31 +692,31 @@ export class KeyringController extends BaseController<
694692
vault: {
695693
includeInStateLogs: false,
696694
persist: true,
697-
anonymous: false,
695+
includeInDebugSnapshot: false,
698696
usedInUi: false,
699697
},
700698
isUnlocked: {
701699
includeInStateLogs: true,
702700
persist: false,
703-
anonymous: true,
701+
includeInDebugSnapshot: true,
704702
usedInUi: true,
705703
},
706704
keyrings: {
707705
includeInStateLogs: true,
708706
persist: false,
709-
anonymous: false,
707+
includeInDebugSnapshot: false,
710708
usedInUi: true,
711709
},
712710
encryptionKey: {
713711
includeInStateLogs: false,
714712
persist: false,
715-
anonymous: false,
713+
includeInDebugSnapshot: false,
716714
usedInUi: false,
717715
},
718716
encryptionSalt: {
719717
includeInStateLogs: false,
720718
persist: false,
721-
anonymous: false,
719+
includeInDebugSnapshot: false,
722720
usedInUi: false,
723721
},
724722
},
@@ -1191,7 +1189,7 @@ export class KeyringController extends BaseController<
11911189
}
11921190
});
11931191

1194-
this.messagingSystem.publish(`${name}:accountRemoved`, address);
1192+
this.messenger.publish(`${name}:accountRemoved`, address);
11951193
}
11961194

11971195
/**
@@ -1213,7 +1211,7 @@ export class KeyringController extends BaseController<
12131211
delete state.encryptionSalt;
12141212
});
12151213

1216-
this.messagingSystem.publish(`${name}:lock`);
1214+
this.messenger.publish(`${name}:lock`);
12171215
});
12181216
}
12191217

@@ -1709,96 +1707,96 @@ export class KeyringController extends BaseController<
17091707
}
17101708

17111709
/**
1712-
* Constructor helper for registering this controller's messaging system
1710+
* Constructor helper for registering this controller's messeger
17131711
* actions.
17141712
*/
17151713
#registerMessageHandlers() {
1716-
this.messagingSystem.registerActionHandler(
1714+
this.messenger.registerActionHandler(
17171715
`${name}:signMessage`,
17181716
this.signMessage.bind(this),
17191717
);
17201718

1721-
this.messagingSystem.registerActionHandler(
1719+
this.messenger.registerActionHandler(
17221720
`${name}:signEip7702Authorization`,
17231721
this.signEip7702Authorization.bind(this),
17241722
);
17251723

1726-
this.messagingSystem.registerActionHandler(
1724+
this.messenger.registerActionHandler(
17271725
`${name}:signPersonalMessage`,
17281726
this.signPersonalMessage.bind(this),
17291727
);
17301728

1731-
this.messagingSystem.registerActionHandler(
1729+
this.messenger.registerActionHandler(
17321730
`${name}:signTypedMessage`,
17331731
this.signTypedMessage.bind(this),
17341732
);
17351733

1736-
this.messagingSystem.registerActionHandler(
1734+
this.messenger.registerActionHandler(
17371735
`${name}:decryptMessage`,
17381736
this.decryptMessage.bind(this),
17391737
);
17401738

1741-
this.messagingSystem.registerActionHandler(
1739+
this.messenger.registerActionHandler(
17421740
`${name}:getEncryptionPublicKey`,
17431741
this.getEncryptionPublicKey.bind(this),
17441742
);
17451743

1746-
this.messagingSystem.registerActionHandler(
1744+
this.messenger.registerActionHandler(
17471745
`${name}:getAccounts`,
17481746
this.getAccounts.bind(this),
17491747
);
17501748

1751-
this.messagingSystem.registerActionHandler(
1749+
this.messenger.registerActionHandler(
17521750
`${name}:getKeyringsByType`,
17531751
this.getKeyringsByType.bind(this),
17541752
);
17551753

1756-
this.messagingSystem.registerActionHandler(
1754+
this.messenger.registerActionHandler(
17571755
`${name}:getKeyringForAccount`,
17581756
this.getKeyringForAccount.bind(this),
17591757
);
17601758

1761-
this.messagingSystem.registerActionHandler(
1759+
this.messenger.registerActionHandler(
17621760
`${name}:persistAllKeyrings`,
17631761
this.persistAllKeyrings.bind(this),
17641762
);
17651763

1766-
this.messagingSystem.registerActionHandler(
1764+
this.messenger.registerActionHandler(
17671765
`${name}:prepareUserOperation`,
17681766
this.prepareUserOperation.bind(this),
17691767
);
17701768

1771-
this.messagingSystem.registerActionHandler(
1769+
this.messenger.registerActionHandler(
17721770
`${name}:patchUserOperation`,
17731771
this.patchUserOperation.bind(this),
17741772
);
17751773

1776-
this.messagingSystem.registerActionHandler(
1774+
this.messenger.registerActionHandler(
17771775
`${name}:signUserOperation`,
17781776
this.signUserOperation.bind(this),
17791777
);
17801778

1781-
this.messagingSystem.registerActionHandler(
1779+
this.messenger.registerActionHandler(
17821780
`${name}:addNewAccount`,
17831781
this.addNewAccount.bind(this),
17841782
);
17851783

1786-
this.messagingSystem.registerActionHandler(
1784+
this.messenger.registerActionHandler(
17871785
`${name}:withKeyring`,
17881786
this.withKeyring.bind(this),
17891787
);
17901788

1791-
this.messagingSystem.registerActionHandler(
1789+
this.messenger.registerActionHandler(
17921790
`${name}:addNewKeyring`,
17931791
this.addNewKeyring.bind(this),
17941792
);
17951793

1796-
this.messagingSystem.registerActionHandler(
1794+
this.messenger.registerActionHandler(
17971795
`${name}:createNewVaultAndKeychain`,
17981796
this.createNewVaultAndKeychain.bind(this),
17991797
);
18001798

1801-
this.messagingSystem.registerActionHandler(
1799+
this.messenger.registerActionHandler(
18021800
`${name}:createNewVaultAndRestore`,
18031801
this.createNewVaultAndRestore.bind(this),
18041802
);
@@ -2472,7 +2470,7 @@ export class KeyringController extends BaseController<
24722470
this.update((state) => {
24732471
state.isUnlocked = true;
24742472
});
2475-
this.messagingSystem.publish(`${name}:unlock`);
2473+
this.messenger.publish(`${name}:unlock`);
24762474
}
24772475

24782476
/**

packages/keyring-controller/tsconfig.build.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
},
1212
{
1313
"path": "../message-manager/tsconfig.build.json"
14+
},
15+
{
16+
"path": "../messenger/tsconfig.build.json"
1417
}
1518
],
1619
"include": ["../../types", "./src"]

packages/keyring-controller/tsconfig.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
},
1010
{
1111
"path": "../message-manager"
12+
},
13+
{
14+
"path": "../messenger"
1215
}
1316
],
1417
"include": ["../../types", "./src", "./tests"]

yarn.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3937,6 +3937,7 @@ __metadata:
39373937
"@metamask/keyring-api": "npm:^21.0.0"
39383938
"@metamask/keyring-internal-api": "npm:^9.0.0"
39393939
"@metamask/keyring-utils": "npm:^3.1.0"
3940+
"@metamask/messenger": "npm:^0.3.0"
39403941
"@metamask/scure-bip39": "npm:^2.1.1"
39413942
"@metamask/utils": "npm:^11.8.1"
39423943
"@types/jest": "npm:^27.4.1"

0 commit comments

Comments
 (0)