Skip to content

Commit 1860493

Browse files
mikespositoGudahtt
andauthored
refactor: migrate {Approval,Transaction,Network,GasFee,AssetsContract,Nft,Tokens}Controller messengers (#6386)
## 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 `ApprovalController`, `TransactionController`, `GasFeeController`, `NetworkController`, `AssetsContractController`, `TokensController` and `NftController` classes to the new `@metamask/messenger` comm system along with their tests, as opposed to the one exported from `@metamask/base-controller`. The three packages are being migrated together because the `TransactionControllerIntegration.test.ts` file ties them together. ## 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 major controllers to the new @metamask/messenger with updated state metadata and tests, adds StaticIntervalPollingControllerNext, and updates package configs and changelogs (breaking). > > - **Messaging/Controllers (BREAKING)**: > - Migrate `ApprovalController`, `TransactionController`, `NetworkController`, `GasFeeController`, `AssetsContractController`, `NftController`, `TokensController` from `RestrictedMessenger` to new `@metamask/messenger`. > - Replace `@metamask/base-controller` APIs with `@metamask/base-controller/next` where applicable; update state metadata keys (`anonymous` -> `includeInDebugSnapshot`). > - Update controller messenger types, action/event registrations (`this.messenger.registerActionHandler/subscribe/publish/call`). > - Refactor tests to use root and namespaced messengers with delegated actions/events. > - **Gas Fee/Pooling**: > - Add `StaticIntervalPollingControllerNext`; `GasFeeController` now extends it. > - Export `GasFeeMessenger` type. > - **Configs/Deps**: > - Add `@metamask/messenger` dependency and TS project references across affected packages; adjust imports to `/next` variants. > - Update README controller dependency graph to include `messenger` links. > - **Changelogs**: > - Note BREAKING changes for the controllers above; document new export in gas-fee-controller. > - **Misc**: > - Minor eslint threshold tweak; yarn.lock updates. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c2dee6e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Mark Stacey <markjstacey@gmail.com>
1 parent 8f2acc5 commit 1860493

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+859
-621
lines changed

README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,10 @@ linkStyle default opacity:0.5
166166
app_metadata_controller --> base_controller;
167167
app_metadata_controller --> messenger;
168168
approval_controller --> base_controller;
169+
approval_controller --> messenger;
169170
assets_controllers --> base_controller;
170171
assets_controllers --> controller_utils;
172+
assets_controllers --> messenger;
171173
assets_controllers --> polling_controller;
172174
assets_controllers --> account_tree_controller;
173175
assets_controllers --> accounts_controller;
@@ -275,6 +277,7 @@ linkStyle default opacity:0.5
275277
network_controller --> eth_json_rpc_middleware;
276278
network_controller --> eth_json_rpc_provider;
277279
network_controller --> json_rpc_engine;
280+
network_controller --> messenger;
278281
network_controller --> error_reporting_service;
279282
network_enablement_controller --> base_controller;
280283
network_enablement_controller --> controller_utils;
@@ -333,6 +336,7 @@ linkStyle default opacity:0.5
333336
token_search_discovery_controller --> base_controller;
334337
transaction_controller --> base_controller;
335338
transaction_controller --> controller_utils;
339+
transaction_controller --> messenger;
336340
transaction_controller --> accounts_controller;
337341
transaction_controller --> approval_controller;
338342
transaction_controller --> eth_block_tracker;

eslint-warning-thresholds.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
},
6969
"packages/assets-controllers/src/TokensController.test.ts": {
7070
"import-x/namespace": 1,
71-
"import-x/order": 4,
71+
"import-x/order": 3,
7272
"jest/no-conditional-in-test": 2
7373
},
7474
"packages/assets-controllers/src/TokensController.ts": {

packages/approval-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` ([#6386](https://github.com/MetaMask/core/pull/6386))
13+
- Previously, `ApprovalController` accepted a `RestrictedMessenger` instance from `@metamask/base-controller`.
14+
1015
## [7.2.1]
1116

1217
### Changed

packages/approval-controller/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
},
4949
"dependencies": {
5050
"@metamask/base-controller": "^8.4.2",
51+
"@metamask/messenger": "^0.3.0",
5152
"@metamask/rpc-errors": "^7.0.2",
5253
"@metamask/utils": "^11.8.1",
5354
"nanoid": "^3.3.8"

packages/approval-controller/src/ApprovalController.test.ts

Lines changed: 52 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
/* eslint-disable jest/expect-expect */
22

3-
import { deriveStateFromMetadata, Messenger } from '@metamask/base-controller';
3+
import { deriveStateFromMetadata } from '@metamask/base-controller/next';
4+
import {
5+
MOCK_ANY_NAMESPACE,
6+
Messenger,
7+
type MessengerActions,
8+
type MessengerEvents,
9+
type MockAnyNamespace,
10+
} from '@metamask/messenger';
411
import { errorCodes, JsonRpcError } from '@metamask/rpc-errors';
512
import { nanoid } from 'nanoid';
613

@@ -9,6 +16,7 @@ import type {
916
AddApprovalOptions,
1017
ApprovalControllerActions,
1118
ApprovalControllerEvents,
19+
ApprovalControllerMessenger,
1220
ErrorOptions,
1321
StartFlowOptions,
1422
SuccessOptions,
@@ -28,6 +36,12 @@ import {
2836

2937
jest.mock('nanoid');
3038

39+
type AllActions = MessengerActions<ApprovalControllerMessenger>;
40+
41+
type AllEvents = MessengerEvents<ApprovalControllerMessenger>;
42+
43+
type RootMessenger = Messenger<MockAnyNamespace, AllActions, AllEvents>;
44+
3145
const nanoidMock = jest.mocked(nanoid);
3246

3347
const PENDING_APPROVALS_STORE_KEY = 'pendingApprovals';
@@ -223,20 +237,26 @@ function getError(message: string, code?: number) {
223237
}
224238

225239
/**
226-
* Constructs a restricted messenger.
240+
* Constructs a controller messenger.
227241
*
228-
* @returns A restricted messenger.
242+
* @returns A controller messenger.
229243
*/
230-
function getRestrictedMessenger() {
231-
const messenger = new Messenger<
232-
ApprovalControllerActions,
233-
ApprovalControllerEvents
234-
>();
235-
return messenger.getRestricted({
236-
name: 'ApprovalController',
237-
allowedActions: [],
238-
allowedEvents: [],
244+
function getMessengers() {
245+
const rootMessenger: RootMessenger = new Messenger({
246+
namespace: MOCK_ANY_NAMESPACE,
239247
});
248+
return {
249+
rootMessenger,
250+
approvalControllerMessenger: new Messenger<
251+
typeof controllerName,
252+
ApprovalControllerActions,
253+
ApprovalControllerEvents,
254+
typeof rootMessenger
255+
>({
256+
namespace: controllerName,
257+
parent: rootMessenger,
258+
}),
259+
};
240260
}
241261

242262
describe('approval controller', () => {
@@ -250,7 +270,7 @@ describe('approval controller', () => {
250270
showApprovalRequest = jest.fn();
251271

252272
approvalController = new ApprovalController({
253-
messenger: getRestrictedMessenger(),
273+
messenger: getMessengers().approvalControllerMessenger,
254274
showApprovalRequest,
255275
});
256276
});
@@ -445,7 +465,7 @@ describe('approval controller', () => {
445465

446466
it('does not throw on origin and type collision if type excluded', () => {
447467
approvalController = new ApprovalController({
448-
messenger: getRestrictedMessenger(),
468+
messenger: getMessengers().approvalControllerMessenger,
449469
showApprovalRequest,
450470
typesExcludedFromRateLimiting: ['myType'],
451471
});
@@ -638,7 +658,7 @@ describe('approval controller', () => {
638658

639659
it('gets the count when specifying origin and type with type excluded from rate limiting', () => {
640660
approvalController = new ApprovalController({
641-
messenger: getRestrictedMessenger(),
661+
messenger: getMessengers().approvalControllerMessenger,
642662
showApprovalRequest,
643663
typesExcludedFromRateLimiting: [TYPE],
644664
});
@@ -678,7 +698,7 @@ describe('approval controller', () => {
678698

679699
it('gets the total approval count with type excluded from rate limiting', () => {
680700
approvalController = new ApprovalController({
681-
messenger: getRestrictedMessenger(),
701+
messenger: getMessengers().approvalControllerMessenger,
682702
showApprovalRequest,
683703
typesExcludedFromRateLimiting: ['type0'],
684704
});
@@ -1269,23 +1289,16 @@ describe('approval controller', () => {
12691289

12701290
describe('actions', () => {
12711291
it('addApprovalRequest: shouldShowRequest = true', async () => {
1272-
const messenger = new Messenger<
1273-
ApprovalControllerActions,
1274-
ApprovalControllerEvents
1275-
>();
1292+
const { rootMessenger, approvalControllerMessenger } = getMessengers();
12761293

12771294
approvalController = new ApprovalController({
1278-
messenger: messenger.getRestricted({
1279-
name: controllerName,
1280-
allowedActions: [],
1281-
allowedEvents: [],
1282-
}),
1295+
messenger: approvalControllerMessenger,
12831296
showApprovalRequest,
12841297
});
12851298

12861299
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
12871300
// eslint-disable-next-line @typescript-eslint/no-floating-promises
1288-
messenger.call(
1301+
rootMessenger.call(
12891302
'ApprovalController:addRequest',
12901303
{ id: 'foo', origin: 'bar.baz', type: TYPE },
12911304
true,
@@ -1295,23 +1308,16 @@ describe('approval controller', () => {
12951308
});
12961309

12971310
it('addApprovalRequest: shouldShowRequest = false', async () => {
1298-
const messenger = new Messenger<
1299-
ApprovalControllerActions,
1300-
ApprovalControllerEvents
1301-
>();
1311+
const { rootMessenger, approvalControllerMessenger } = getMessengers();
13021312

13031313
approvalController = new ApprovalController({
1304-
messenger: messenger.getRestricted({
1305-
name: controllerName,
1306-
allowedActions: [],
1307-
allowedEvents: [],
1308-
}),
1314+
messenger: approvalControllerMessenger,
13091315
showApprovalRequest,
13101316
});
13111317

13121318
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
13131319
// eslint-disable-next-line @typescript-eslint/no-floating-promises
1314-
messenger.call(
1320+
rootMessenger.call(
13151321
'ApprovalController:addRequest',
13161322
{ id: 'foo', origin: 'bar.baz', type: TYPE },
13171323
false,
@@ -1321,17 +1327,10 @@ describe('approval controller', () => {
13211327
});
13221328

13231329
it('updateRequestState', () => {
1324-
const messenger = new Messenger<
1325-
ApprovalControllerActions,
1326-
ApprovalControllerEvents
1327-
>();
1330+
const { approvalControllerMessenger } = getMessengers();
13281331

13291332
approvalController = new ApprovalController({
1330-
messenger: messenger.getRestricted({
1331-
name: controllerName,
1332-
allowedActions: [],
1333-
allowedEvents: [],
1334-
}),
1333+
messenger: approvalControllerMessenger,
13351334
showApprovalRequest,
13361335
});
13371336

@@ -1344,10 +1343,13 @@ describe('approval controller', () => {
13441343
requestState: { foo: 'bar' },
13451344
});
13461345

1347-
messenger.call('ApprovalController:updateRequestState', {
1348-
id: 'foo',
1349-
requestState: { foo: 'foobar' },
1350-
});
1346+
approvalControllerMessenger.call(
1347+
'ApprovalController:updateRequestState',
1348+
{
1349+
id: 'foo',
1350+
requestState: { foo: 'foobar' },
1351+
},
1352+
);
13511353

13521354
expect(approvalController.get('foo')?.requestState).toStrictEqual({
13531355
foo: 'foobar',
@@ -1719,7 +1721,7 @@ describe('approval controller', () => {
17191721
deriveStateFromMetadata(
17201722
approvalController.state,
17211723
approvalController.metadata,
1722-
'anonymous',
1724+
'includeInDebugSnapshot',
17231725
),
17241726
).toMatchInlineSnapshot(`
17251727
Object {

packages/approval-controller/src/ApprovalController.ts

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1-
import type { ControllerGetStateAction } from '@metamask/base-controller';
1+
import type {
2+
ControllerGetStateAction,
3+
StateMetadata,
4+
} from '@metamask/base-controller/next';
25
import {
36
BaseController,
47
type ControllerStateChangeEvent,
5-
type RestrictedMessenger,
6-
} from '@metamask/base-controller';
8+
} from '@metamask/base-controller/next';
9+
import type { Messenger } from '@metamask/messenger';
710
import type { JsonRpcError, DataWithOptionalCause } from '@metamask/rpc-errors';
811
import { rpcErrors } from '@metamask/rpc-errors';
912
import type { Json, OptionalField } from '@metamask/utils';
@@ -26,23 +29,23 @@ export const APPROVAL_TYPE_RESULT_SUCCESS = 'result_success';
2629

2730
const controllerName = 'ApprovalController';
2831

29-
const stateMetadata = {
32+
const stateMetadata: StateMetadata<ApprovalControllerState> = {
3033
pendingApprovals: {
3134
includeInStateLogs: true,
3235
persist: false,
33-
anonymous: true,
36+
includeInDebugSnapshot: true,
3437
usedInUi: true,
3538
},
3639
pendingApprovalCount: {
3740
includeInStateLogs: true,
3841
persist: false,
39-
anonymous: false,
42+
includeInDebugSnapshot: false,
4043
usedInUi: true,
4144
},
4245
approvalFlows: {
4346
includeInStateLogs: true,
4447
persist: false,
45-
anonymous: false,
48+
includeInDebugSnapshot: false,
4649
usedInUi: true,
4750
},
4851
};
@@ -134,12 +137,10 @@ export type ApprovalControllerState = {
134137
approvalFlows: ApprovalFlowState[];
135138
};
136139

137-
export type ApprovalControllerMessenger = RestrictedMessenger<
140+
export type ApprovalControllerMessenger = Messenger<
138141
typeof controllerName,
139142
ApprovalControllerActions,
140-
ApprovalControllerEvents,
141-
never,
142-
never
143+
ApprovalControllerEvents
143144
>;
144145

145146
// Option Types
@@ -413,12 +414,12 @@ export class ApprovalController extends BaseController<
413414
* actions.
414415
*/
415416
private registerMessageHandlers(): void {
416-
this.messagingSystem.registerActionHandler(
417+
this.messenger.registerActionHandler(
417418
`${controllerName}:clearRequests` as const,
418419
this.clear.bind(this),
419420
);
420421

421-
this.messagingSystem.registerActionHandler(
422+
this.messenger.registerActionHandler(
422423
`${controllerName}:addRequest` as const,
423424
(opts: AddApprovalOptions, shouldShowRequest: boolean) => {
424425
if (shouldShowRequest) {
@@ -428,47 +429,47 @@ export class ApprovalController extends BaseController<
428429
},
429430
);
430431

431-
this.messagingSystem.registerActionHandler(
432+
this.messenger.registerActionHandler(
432433
`${controllerName}:hasRequest` as const,
433434
this.has.bind(this),
434435
);
435436

436-
this.messagingSystem.registerActionHandler(
437+
this.messenger.registerActionHandler(
437438
`${controllerName}:acceptRequest` as const,
438439
this.accept.bind(this),
439440
);
440441

441-
this.messagingSystem.registerActionHandler(
442+
this.messenger.registerActionHandler(
442443
`${controllerName}:rejectRequest` as const,
443444
this.reject.bind(this),
444445
);
445446

446-
this.messagingSystem.registerActionHandler(
447+
this.messenger.registerActionHandler(
447448
`${controllerName}:updateRequestState` as const,
448449
this.updateRequestState.bind(this),
449450
);
450451

451-
this.messagingSystem.registerActionHandler(
452+
this.messenger.registerActionHandler(
452453
`${controllerName}:startFlow` as const,
453454
this.startFlow.bind(this),
454455
);
455456

456-
this.messagingSystem.registerActionHandler(
457+
this.messenger.registerActionHandler(
457458
`${controllerName}:endFlow` as const,
458459
this.endFlow.bind(this),
459460
);
460461

461-
this.messagingSystem.registerActionHandler(
462+
this.messenger.registerActionHandler(
462463
`${controllerName}:setFlowLoadingText` as const,
463464
this.setFlowLoadingText.bind(this),
464465
);
465466

466-
this.messagingSystem.registerActionHandler(
467+
this.messenger.registerActionHandler(
467468
`${controllerName}:showSuccess` as const,
468469
this.success.bind(this),
469470
);
470471

471-
this.messagingSystem.registerActionHandler(
472+
this.messenger.registerActionHandler(
472473
`${controllerName}:showError` as const,
473474
this.error.bind(this),
474475
);

0 commit comments

Comments
 (0)