Skip to content

Commit 5cca126

Browse files
mikespositomcmire
andauthored
refactor: migrate AccountsController to @metamask/messenger (#6426)
## 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 `AccountsController` to the new `@metamask/messenger` message bus, 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 --------- Co-authored-by: Elliot Winkler <[email protected]>
1 parent a0a3194 commit 5cca126

File tree

8 files changed

+142
-118
lines changed

8 files changed

+142
-118
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ linkStyle default opacity:0.5
154154
account_tree_controller --> multichain_account_service;
155155
account_tree_controller --> profile_sync_controller;
156156
accounts_controller --> base_controller;
157+
accounts_controller --> messenger;
157158
accounts_controller --> controller_utils;
158159
accounts_controller --> keyring_controller;
159160
accounts_controller --> network_controller;

packages/accounts-controller/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ 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` ([#6426](https://github.com/MetaMask/core/pull/6426))
13+
- Previously, `AccountsController` accepted a `RestrictedMessenger` instance from `@metamask/base-controller`.
14+
1015
## [33.2.0]
1116

1217
### Added

packages/accounts-controller/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
"@metamask/keyring-api": "^21.0.0",
5454
"@metamask/keyring-internal-api": "^9.0.0",
5555
"@metamask/keyring-utils": "^3.1.0",
56+
"@metamask/messenger": "^0.3.0",
5657
"@metamask/snaps-sdk": "^9.0.0",
5758
"@metamask/snaps-utils": "^11.0.0",
5859
"@metamask/superstruct": "^3.1.0",

packages/accounts-controller/src/AccountsController.test.ts

Lines changed: 95 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Messenger, deriveStateFromMetadata } from '@metamask/base-controller';
1+
import { deriveStateFromMetadata } from '@metamask/base-controller/next';
22
import { InfuraNetworkType } from '@metamask/controller-utils';
33
import type {
44
AccountAssetListUpdatedEventPayload,
@@ -15,6 +15,13 @@ import {
1515
} from '@metamask/keyring-api';
1616
import { KeyringTypes } from '@metamask/keyring-controller';
1717
import type { InternalAccount } from '@metamask/keyring-internal-api';
18+
import {
19+
MOCK_ANY_NAMESPACE,
20+
Messenger,
21+
type MessengerActions,
22+
type MessengerEvents,
23+
type MockAnyNamespace,
24+
} from '@metamask/messenger';
1825
import type { NetworkClientId } from '@metamask/network-controller';
1926
import type { SnapControllerState } from '@metamask/snaps-controllers';
2027
import { SnapStatus } from '@metamask/snaps-utils';
@@ -23,11 +30,8 @@ import type { V4Options } from 'uuid';
2330
import * as uuid from 'uuid';
2431

2532
import type {
26-
AccountsControllerActions,
27-
AccountsControllerEvents,
33+
AccountsControllerMessenger,
2834
AccountsControllerState,
29-
AllowedActions,
30-
AllowedEvents,
3135
} from './AccountsController';
3236
import { AccountsController, EMPTY_ACCOUNT } from './AccountsController';
3337
import {
@@ -41,6 +45,17 @@ import {
4145
keyringTypeToName,
4246
} from './utils';
4347

48+
type AllAccountsControllerActions =
49+
MessengerActions<AccountsControllerMessenger>;
50+
51+
type AllAccountsControllerEvents = MessengerEvents<AccountsControllerMessenger>;
52+
53+
type RootMessenger = Messenger<
54+
MockAnyNamespace,
55+
AllAccountsControllerActions,
56+
AllAccountsControllerEvents
57+
>;
58+
4459
jest.mock('uuid');
4560
const mockUUID = jest.spyOn(uuid, 'v4');
4661
const actualUUID = jest.requireActual('uuid').v4; // We also use uuid.v4 in our mocks
@@ -217,64 +232,66 @@ function setExpectedLastSelectedAsAny(
217232
}
218233

219234
/**
220-
* Builds a new instance of the Messenger class for the AccountsController.
235+
* Builds a new instance of the root messenger.
221236
*
222-
* @returns A new instance of the Messenger class for the AccountsController.
237+
* @returns A new instance of the root messenger.
223238
*/
224-
function buildMessenger() {
225-
return new Messenger<
226-
AccountsControllerActions | AllowedActions,
227-
AccountsControllerEvents | AllowedEvents
228-
>();
239+
function buildMessenger(): RootMessenger {
240+
return new Messenger({ namespace: MOCK_ANY_NAMESPACE });
229241
}
230242

231243
/**
232-
* Builds a restricted messenger for the AccountsController.
244+
* Builds a messenger for the AccountsController.
233245
*
234-
* @param messenger - The messenger to restrict.
235-
* @returns The restricted messenger.
246+
* @param rootMessenger - The root messenger.
247+
* @returns The messenger for AccountsController.
236248
*/
237-
function buildAccountsControllerMessenger(messenger = buildMessenger()) {
238-
return messenger.getRestricted({
239-
name: 'AccountsController',
240-
allowedEvents: [
249+
function buildAccountsControllerMessenger(rootMessenger = buildMessenger()) {
250+
const accountsControllerMessenger = new Messenger<
251+
'AccountsController',
252+
AllAccountsControllerActions,
253+
AllAccountsControllerEvents,
254+
typeof rootMessenger
255+
>({
256+
namespace: 'AccountsController',
257+
parent: rootMessenger,
258+
});
259+
rootMessenger.delegate({
260+
messenger: accountsControllerMessenger,
261+
actions: [
262+
'KeyringController:getState',
263+
'KeyringController:getKeyringsByType',
264+
],
265+
events: [
241266
'SnapController:stateChange',
242267
'KeyringController:stateChange',
243268
'SnapKeyring:accountAssetListUpdated',
244269
'SnapKeyring:accountBalancesUpdated',
245270
'SnapKeyring:accountTransactionsUpdated',
246271
'MultichainNetworkController:networkDidChange',
247272
],
248-
allowedActions: [
249-
'KeyringController:getState',
250-
'KeyringController:getKeyringsByType',
251-
],
252273
});
274+
return accountsControllerMessenger;
253275
}
254276

255277
/**
256278
* Sets up an instance of the AccountsController class with the given initial state and callbacks.
257279
*
258280
* @param options - The options object.
259281
* @param [options.initialState] - The initial state to use for the AccountsController.
260-
* @param [options.messenger] - Messenger to use for the AccountsController.
282+
* @param [options.messenger] - The root messenger to use for creating the AccountsController messenger.
261283
* @returns An instance of the AccountsController class.
262284
*/
263285
function setupAccountsController({
264286
initialState = {},
265287
messenger = buildMessenger(),
266288
}: {
267289
initialState?: Partial<AccountsControllerState>;
268-
messenger?: Messenger<
269-
AccountsControllerActions | AllowedActions,
270-
AccountsControllerEvents | AllowedEvents
271-
>;
272-
} = {}): {
290+
messenger?: RootMessenger;
291+
}): {
273292
accountsController: AccountsController;
274-
messenger: Messenger<
275-
AccountsControllerActions | AllowedActions,
276-
AccountsControllerEvents | AllowedEvents
277-
>;
293+
messenger: RootMessenger;
294+
accountsControllerMessenger: AccountsControllerMessenger;
278295
triggerMultichainNetworkChange: (id: NetworkClientId | CaipChainId) => void;
279296
} {
280297
const accountsControllerMessenger =
@@ -288,7 +305,12 @@ function setupAccountsController({
288305
const triggerMultichainNetworkChange = (id: NetworkClientId | CaipChainId) =>
289306
messenger.publish('MultichainNetworkController:networkDidChange', id);
290307

291-
return { accountsController, messenger, triggerMultichainNetworkChange };
308+
return {
309+
accountsController,
310+
messenger,
311+
accountsControllerMessenger,
312+
triggerMultichainNetworkChange,
313+
};
292314
}
293315

294316
describe('AccountsController', () => {
@@ -1136,11 +1158,10 @@ describe('AccountsController', () => {
11361158

11371159
it('publishes accountAdded event', async () => {
11381160
const messenger = buildMessenger();
1139-
const messengerSpy = jest.spyOn(messenger, 'publish');
11401161

11411162
mockUUIDWithNormalAccounts([mockAccount, mockAccount2]);
11421163

1143-
setupAccountsController({
1164+
const { accountsControllerMessenger } = setupAccountsController({
11441165
initialState: {
11451166
internalAccounts: {
11461167
accounts: {
@@ -1152,6 +1173,8 @@ describe('AccountsController', () => {
11521173
messenger,
11531174
});
11541175

1176+
const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish');
1177+
11551178
const mockNewKeyringState = {
11561179
isUnlocked: true,
11571180
keyrings: [
@@ -1172,11 +1195,10 @@ describe('AccountsController', () => {
11721195
[],
11731196
);
11741197

1175-
// First call is 'KeyringController:stateChange'
1198+
// First call is 'AccountsController:stateChange'
11761199
expect(messengerSpy).toHaveBeenNthCalledWith(
1177-
// 1. KeyringController:stateChange
1178-
// 2. AccountsController:stateChange
1179-
3,
1200+
// 1. AccountsController:stateChange
1201+
2,
11801202
'AccountsController:accountAdded',
11811203
MockExpectedInternalAccountBuilder.from(mockAccount2)
11821204
.setExpectedLastSelectedAsAny()
@@ -1437,11 +1459,10 @@ describe('AccountsController', () => {
14371459

14381460
it('publishes accountRemoved event', async () => {
14391461
const messenger = buildMessenger();
1440-
const messengerSpy = jest.spyOn(messenger, 'publish');
14411462

14421463
mockUUIDWithNormalAccounts([mockAccount, mockAccount2]);
14431464

1444-
setupAccountsController({
1465+
const { accountsControllerMessenger } = setupAccountsController({
14451466
initialState: {
14461467
internalAccounts: {
14471468
accounts: {
@@ -1454,6 +1475,8 @@ describe('AccountsController', () => {
14541475
messenger,
14551476
});
14561477

1478+
const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish');
1479+
14571480
const mockNewKeyringState = {
14581481
isUnlocked: true,
14591482
keyrings: [
@@ -1473,11 +1496,10 @@ describe('AccountsController', () => {
14731496
[],
14741497
);
14751498

1476-
// First call is 'KeyringController:stateChange'
1499+
// First call is 'AccountsController:stateChange'
14771500
expect(messengerSpy).toHaveBeenNthCalledWith(
1478-
// 1. KeyringController:stateChange
1479-
// 2. AccountsController:stateChange
1480-
3,
1501+
// 1. AccountsController:stateChange
1502+
2,
14811503
'AccountsController:accountRemoved',
14821504
mockAccount3.id,
14831505
);
@@ -3165,19 +3187,20 @@ describe('AccountsController', () => {
31653187
},
31663188
type: BtcAccountType.P2wpkh,
31673189
});
3168-
const { accountsController, messenger } = setupAccountsController({
3169-
initialState: {
3170-
internalAccounts: {
3171-
accounts: {
3172-
[mockAccount.id]: mockAccount,
3173-
[mockNonEvmAccount.id]: mockNonEvmAccount,
3190+
const { accountsController, accountsControllerMessenger } =
3191+
setupAccountsController({
3192+
initialState: {
3193+
internalAccounts: {
3194+
accounts: {
3195+
[mockAccount.id]: mockAccount,
3196+
[mockNonEvmAccount.id]: mockNonEvmAccount,
3197+
},
3198+
selectedAccount: mockAccount.id,
31743199
},
3175-
selectedAccount: mockAccount.id,
31763200
},
3177-
},
3178-
});
3201+
});
31793202

3180-
const messengerSpy = jest.spyOn(messenger, 'publish');
3203+
const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish');
31813204

31823205
accountsController.setSelectedAccount(mockNonEvmAccount.id);
31833206

@@ -3271,10 +3294,10 @@ describe('AccountsController', () => {
32713294
});
32723295

32733296
it('publishes the accountRenamed event', () => {
3274-
const { accountsController, messenger } =
3297+
const { accountsController, accountsControllerMessenger } =
32753298
setupAccountsController(mockState);
32763299

3277-
const messengerSpy = jest.spyOn(messenger, 'publish');
3300+
const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish');
32783301

32793302
accountsController.setAccountNameAndSelectAccount(
32803303
mockAccount.id,
@@ -3349,16 +3372,17 @@ describe('AccountsController', () => {
33493372
});
33503373

33513374
it('publishes the accountRenamed event', () => {
3352-
const { accountsController, messenger } = setupAccountsController({
3353-
initialState: {
3354-
internalAccounts: {
3355-
accounts: { [mockAccount.id]: mockAccount },
3356-
selectedAccount: mockAccount.id,
3375+
const { accountsController, accountsControllerMessenger } =
3376+
setupAccountsController({
3377+
initialState: {
3378+
internalAccounts: {
3379+
accounts: { [mockAccount.id]: mockAccount },
3380+
selectedAccount: mockAccount.id,
3381+
},
33573382
},
3358-
},
3359-
});
3383+
});
33603384

3361-
const messengerSpy = jest.spyOn(messenger, 'publish');
3385+
const messengerSpy = jest.spyOn(accountsControllerMessenger, 'publish');
33623386

33633387
accountsController.setAccountName(mockAccount.id, 'new name');
33643388

@@ -3922,19 +3946,19 @@ describe('AccountsController', () => {
39223946

39233947
describe('metadata', () => {
39243948
it('includes expected state in debug snapshots', () => {
3925-
const { accountsController: controller } = setupAccountsController();
3949+
const { accountsController: controller } = setupAccountsController({});
39263950

39273951
expect(
39283952
deriveStateFromMetadata(
39293953
controller.state,
39303954
controller.metadata,
3931-
'anonymous',
3955+
'includeInDebugSnapshot',
39323956
),
39333957
).toMatchInlineSnapshot(`Object {}`);
39343958
});
39353959

39363960
it('includes expected state in state logs', () => {
3937-
const { accountsController: controller } = setupAccountsController();
3961+
const { accountsController: controller } = setupAccountsController({});
39383962

39393963
expect(
39403964
deriveStateFromMetadata(
@@ -3953,7 +3977,7 @@ describe('AccountsController', () => {
39533977
});
39543978

39553979
it('persists expected state', () => {
3956-
const { accountsController: controller } = setupAccountsController();
3980+
const { accountsController: controller } = setupAccountsController({});
39573981

39583982
expect(
39593983
deriveStateFromMetadata(
@@ -3972,7 +3996,7 @@ describe('AccountsController', () => {
39723996
});
39733997

39743998
it('exposes expected state to UI', () => {
3975-
const { accountsController: controller } = setupAccountsController();
3999+
const { accountsController: controller } = setupAccountsController({});
39764000

39774001
expect(
39784002
deriveStateFromMetadata(

0 commit comments

Comments
 (0)