Skip to content

Commit 36b9c65

Browse files
authored
refactor: migrate LoggingController to @metamask/messenger (#6463)
## 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 `LoggingController` 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 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Migrates `LoggingController` to `@metamask/messenger`, renames metadata key `anonymous` to `includeInDebugSnapshot`, and updates tests/configs accordingly. > > - **Logging Controller** > - Migrate to `@metamask/messenger` and `@metamask/base-controller/next`; register actions via `messenger.registerActionHandler`. > - Update `metadata`: replace `anonymous` with `includeInDebugSnapshot` and type with `StateMetadata`. > - Update `type` aliases to use `Messenger` instead of `RestrictedMessenger`. > - Add dependency `@metamask/messenger@^0.3.0` and TS project references. > - Changelog: mark BREAKING changes for messenger migration and metadata key rename. > - **Tests** > - Rewrite messenger setup using root/child `Messenger` from `@metamask/messenger` and update imports from `@metamask/base-controller/next`. > - Adjust calls to `LoggingController:add` via root messenger. > - **Repo Docs/Graph** > - Dependency graph: add `logging_controller --> messenger`; add `eip_7702_internal_rpc_middleware --> controller_utils`. > - **Lockfile** > - Update for new `@metamask/messenger` dependency. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 28d9e41. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 29dcbc8 commit 36b9c65

File tree

8 files changed

+79
-59
lines changed

8 files changed

+79
-59
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ linkStyle default opacity:0.5
256256
keyring_controller --> messenger;
257257
logging_controller --> base_controller;
258258
logging_controller --> controller_utils;
259+
logging_controller --> messenger;
259260
message_manager --> base_controller;
260261
message_manager --> controller_utils;
261262
message_manager --> messenger;

packages/logging-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` ([#6463](https://github.com/MetaMask/core/pull/6463))
13+
- Previously, `LoggingController` accepted a `RestrictedMessenger` instance from `@metamask/base-controller`.
14+
- **BREAKING:** Metadata property `anonymous` renamed to `includeInDebugSnapshot` ([#6463](https://github.com/MetaMask/core/pull/6463))
15+
1016
## [6.1.1]
1117

1218
### Changed

packages/logging-controller/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
"dependencies": {
5050
"@metamask/base-controller": "^8.4.2",
5151
"@metamask/controller-utils": "^11.14.1",
52+
"@metamask/messenger": "^0.3.0",
5253
"uuid": "^8.3.2"
5354
},
5455
"devDependencies": {

packages/logging-controller/src/LoggingController.test.ts

Lines changed: 58 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
1-
import { Messenger, deriveStateFromMetadata } from '@metamask/base-controller';
1+
import { deriveStateFromMetadata } from '@metamask/base-controller/next';
2+
import {
3+
Messenger,
4+
MOCK_ANY_NAMESPACE,
5+
type MessengerActions,
6+
type MessengerEvents,
7+
type MockAnyNamespace,
8+
} from '@metamask/messenger';
29
import * as uuid from 'uuid';
310

4-
import type { LoggingControllerActions } from './LoggingController';
11+
import type { LoggingControllerMessenger } from './LoggingController';
512
import { LoggingController } from './LoggingController';
613
import { LogType } from './logTypes';
714
import { SigningMethod, SigningStage } from './logTypes/EthSignLog';
@@ -15,44 +22,56 @@ jest.mock('uuid', () => {
1522
};
1623
});
1724

18-
const name = 'LoggingController';
25+
type AllLoggingControllerActions = MessengerActions<LoggingControllerMessenger>;
26+
27+
type AllLoggingControllerEvents = MessengerEvents<LoggingControllerMessenger>;
28+
29+
type RootMessenger = Messenger<
30+
MockAnyNamespace,
31+
AllLoggingControllerActions,
32+
AllLoggingControllerEvents
33+
>;
34+
35+
const namespace = 'LoggingController';
1936

2037
/**
21-
* Constructs a unrestricted messenger.
38+
* Constructs a root messenger instance.
2239
*
23-
* @returns A unrestricted messenger.
40+
* @returns A root messenger.
2441
*/
25-
function getUnrestrictedMessenger() {
26-
return new Messenger<LoggingControllerActions, never>();
42+
function getRootMessenger(): RootMessenger {
43+
return new Messenger({ namespace: MOCK_ANY_NAMESPACE });
2744
}
2845

2946
/**
30-
* Constructs a restricted messenger.
47+
* Constructs a messenger instance for LoggingController.
3148
*
32-
* @param messenger - An optional unrestricted messenger
33-
* @returns A restricted messenger.
49+
* @param messenger - An optional root messenger
50+
* @returns A controller messenger.
3451
*/
35-
function getRestrictedMessenger(messenger = getUnrestrictedMessenger()) {
36-
return messenger.getRestricted({
37-
name,
38-
allowedActions: [],
39-
allowedEvents: [],
52+
function getLoggingControllerMessenger(messenger = getRootMessenger()) {
53+
return new Messenger<
54+
typeof namespace,
55+
AllLoggingControllerActions,
56+
AllLoggingControllerEvents,
57+
RootMessenger
58+
>({
59+
namespace,
60+
parent: messenger,
4061
});
4162
}
4263

4364
describe('LoggingController', () => {
4465
it('action: LoggingController:add with generic log', async () => {
45-
const unrestricted = getUnrestrictedMessenger();
46-
const messenger = getRestrictedMessenger(unrestricted);
66+
const rootMessenger = getRootMessenger();
67+
const messenger = getLoggingControllerMessenger(rootMessenger);
4768

4869
const controller = new LoggingController({
4970
messenger,
5071
});
5172

5273
expect(
53-
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
54-
// eslint-disable-next-line @typescript-eslint/await-thenable
55-
await unrestricted.call('LoggingController:add', {
74+
rootMessenger.call('LoggingController:add', {
5675
type: LogType.GenericLog,
5776
data: `Generic log`,
5877
}),
@@ -70,17 +89,15 @@ describe('LoggingController', () => {
7089
});
7190

7291
it('action: LoggingController:add for a signing request', async () => {
73-
const unrestricted = getUnrestrictedMessenger();
74-
const messenger = getRestrictedMessenger(unrestricted);
92+
const rootMessenger = getRootMessenger();
93+
const messenger = getLoggingControllerMessenger(rootMessenger);
7594

7695
const controller = new LoggingController({
7796
messenger,
7897
});
7998

8099
expect(
81-
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
82-
// eslint-disable-next-line @typescript-eslint/await-thenable
83-
await unrestricted.call('LoggingController:add', {
100+
rootMessenger.call('LoggingController:add', {
84101
type: LogType.EthSignLog,
85102
data: {
86103
signingMethod: SigningMethod.PersonalSign,
@@ -106,18 +123,16 @@ describe('LoggingController', () => {
106123
});
107124

108125
it('action: LoggingController:add prevents possible collision of ids', async () => {
109-
const unrestricted = getUnrestrictedMessenger();
110-
const messenger = getRestrictedMessenger(unrestricted);
126+
const rootMessenger = getRootMessenger();
127+
const messenger = getLoggingControllerMessenger(rootMessenger);
111128
const uuidV1Spy = jest.spyOn(uuid, 'v1');
112129

113130
const controller = new LoggingController({
114131
messenger,
115132
});
116133

117134
expect(
118-
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
119-
// eslint-disable-next-line @typescript-eslint/await-thenable
120-
await unrestricted.call('LoggingController:add', {
135+
rootMessenger.call('LoggingController:add', {
121136
type: LogType.GenericLog,
122137
data: `Generic log`,
123138
}),
@@ -128,9 +143,7 @@ describe('LoggingController', () => {
128143
uuidV1Spy.mockReturnValueOnce(id);
129144

130145
expect(
131-
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
132-
// eslint-disable-next-line @typescript-eslint/await-thenable
133-
await unrestricted.call('LoggingController:add', {
146+
rootMessenger.call('LoggingController:add', {
134147
type: LogType.GenericLog,
135148
data: `Generic log 2`,
136149
}),
@@ -159,17 +172,15 @@ describe('LoggingController', () => {
159172
});
160173

161174
it('internal method: clear', async () => {
162-
const unrestricted = getUnrestrictedMessenger();
163-
const messenger = getRestrictedMessenger(unrestricted);
175+
const rootMessenger = getRootMessenger();
176+
const messenger = getLoggingControllerMessenger(rootMessenger);
164177

165178
const controller = new LoggingController({
166179
messenger,
167180
});
168181

169182
expect(
170-
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
171-
// eslint-disable-next-line @typescript-eslint/await-thenable
172-
await unrestricted.call('LoggingController:add', {
183+
rootMessenger.call('LoggingController:add', {
173184
type: LogType.EthSignLog,
174185
data: {
175186
signingMethod: SigningMethod.PersonalSign,
@@ -186,8 +197,8 @@ describe('LoggingController', () => {
186197

187198
describe('metadata', () => {
188199
it('includes expected state in debug snapshots', () => {
189-
const unrestricted = getUnrestrictedMessenger();
190-
const messenger = getRestrictedMessenger(unrestricted);
200+
const rootMessenger = getRootMessenger();
201+
const messenger = getLoggingControllerMessenger(rootMessenger);
191202
const controller = new LoggingController({
192203
messenger,
193204
});
@@ -196,14 +207,14 @@ describe('LoggingController', () => {
196207
deriveStateFromMetadata(
197208
controller.state,
198209
controller.metadata,
199-
'anonymous',
210+
'includeInDebugSnapshot',
200211
),
201212
).toMatchInlineSnapshot(`Object {}`);
202213
});
203214

204215
it('includes expected state in state logs', () => {
205-
const unrestricted = getUnrestrictedMessenger();
206-
const messenger = getRestrictedMessenger(unrestricted);
216+
const rootMessenger = getRootMessenger();
217+
const messenger = getLoggingControllerMessenger(rootMessenger);
207218
const controller = new LoggingController({
208219
messenger,
209220
});
@@ -222,8 +233,8 @@ describe('LoggingController', () => {
222233
});
223234

224235
it('persists expected state', () => {
225-
const unrestricted = getUnrestrictedMessenger();
226-
const messenger = getRestrictedMessenger(unrestricted);
236+
const rootMessenger = getRootMessenger();
237+
const messenger = getLoggingControllerMessenger(rootMessenger);
227238
const controller = new LoggingController({
228239
messenger,
229240
});
@@ -242,8 +253,8 @@ describe('LoggingController', () => {
242253
});
243254

244255
it('exposes expected state to UI', () => {
245-
const unrestricted = getUnrestrictedMessenger();
246-
const messenger = getRestrictedMessenger(unrestricted);
256+
const rootMessenger = getRootMessenger();
257+
const messenger = getLoggingControllerMessenger(rootMessenger);
247258
const controller = new LoggingController({
248259
messenger,
249260
});

packages/logging-controller/src/LoggingController.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import type {
22
ControllerGetStateAction,
33
ControllerStateChangeEvent,
4-
RestrictedMessenger,
5-
} from '@metamask/base-controller';
6-
import { BaseController } from '@metamask/base-controller';
4+
StateMetadata,
5+
} from '@metamask/base-controller/next';
6+
import { BaseController } from '@metamask/base-controller/next';
7+
import type { Messenger } from '@metamask/messenger';
78
import { v1 as random } from 'uuid';
89

910
import type { Log } from './logTypes';
@@ -54,19 +55,17 @@ export type LoggingControllerStateChangeEvent = ControllerStateChangeEvent<
5455

5556
export type LoggingControllerEvents = LoggingControllerStateChangeEvent;
5657

57-
export type LoggingControllerMessenger = RestrictedMessenger<
58+
export type LoggingControllerMessenger = Messenger<
5859
typeof name,
5960
LoggingControllerActions,
60-
LoggingControllerEvents,
61-
never,
62-
never
61+
LoggingControllerEvents
6362
>;
6463

65-
const metadata = {
64+
const metadata: StateMetadata<LoggingControllerState> = {
6665
logs: {
6766
includeInStateLogs: true,
6867
persist: true,
69-
anonymous: false,
68+
includeInDebugSnapshot: false,
7069
usedInUi: false,
7170
},
7271
};
@@ -107,9 +106,8 @@ export class LoggingController extends BaseController<
107106
},
108107
});
109108

110-
this.messagingSystem.registerActionHandler(
111-
`${name}:add` as const,
112-
(log: Log) => this.add(log),
109+
this.messenger.registerActionHandler(`${name}:add` as const, (log: Log) =>
110+
this.add(log),
113111
);
114112
}
115113

packages/logging-controller/tsconfig.build.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
},
88
"references": [
99
{ "path": "../base-controller/tsconfig.build.json" },
10+
{ "path": "../messenger/tsconfig.build.json" },
1011
{ "path": "../controller-utils/tsconfig.build.json" }
1112
],
1213
"include": ["../../types", "./src"]

packages/logging-controller/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
},
66
"references": [
77
{ "path": "../base-controller" },
8+
{ "path": "../messenger" },
89
{ "path": "../controller-utils" }
910
],
1011
"include": ["../../types", "./src"]

yarn.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4017,6 +4017,7 @@ __metadata:
40174017
"@metamask/auto-changelog": "npm:^3.4.4"
40184018
"@metamask/base-controller": "npm:^8.4.2"
40194019
"@metamask/controller-utils": "npm:^11.14.1"
4020+
"@metamask/messenger": "npm:^0.3.0"
40204021
"@types/jest": "npm:^27.4.1"
40214022
deepmerge: "npm:^4.2.2"
40224023
jest: "npm:^27.5.1"

0 commit comments

Comments
 (0)