Skip to content

Commit 249f9b0

Browse files
refactor: migrate PhishingController to @metamask/messenger (#6535)
## 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 `PhishingController` 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 `PhishingController` to use `@metamask/messenger` and renames metadata field `anonymous` to `includeInDebugSnapshot`. > > - **Breaking Changes** > - Migrate `PhishingController` to `@metamask/messenger` `Messenger` (replaces `RestrictedMessenger` from `@metamask/base-controller`). > - Rename metadata property `anonymous` to `includeInDebugSnapshot`. > - **Controller** (`packages/phishing-controller/src/PhishingController.ts`): > - Switch imports to `@metamask/base-controller/next` and `@metamask/messenger`. > - Replace `messagingSystem` calls with `messenger.registerActionHandler`/`messenger.subscribe`. > - Update `PhishingControllerMessenger` type to use `Messenger<...>`. > - **Tests**: > - Update test harness to create root/child messengers via `@metamask/messenger` and delegate `TransactionController:stateChange`. > - Use `deriveStateFromMetadata` from `@metamask/base-controller/next` and expect `includeInDebugSnapshot`. > - **Package/Build**: > - Add dependency `@metamask/messenger` and tsconfig references to `packages/messenger`. > - Update README dependency graph to show `phishing_controller --> messenger` and include `@metamask/messenger` in package list. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c2988bb. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Michele Esposito <michele@esposito.codes>
1 parent 10b12ec commit 249f9b0

File tree

9 files changed

+152
-68
lines changed

9 files changed

+152
-68
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ linkStyle default opacity:0.5
305305
permission_log_controller --> json_rpc_engine;
306306
phishing_controller --> base_controller;
307307
phishing_controller --> controller_utils;
308+
phishing_controller --> messenger;
308309
phishing_controller --> transaction_controller;
309310
polling_controller --> base_controller;
310311
polling_controller --> controller_utils;

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

1218
### Changed

packages/phishing-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
"@noble/hashes": "^1.8.0",
5354
"@types/punycode": "^2.1.0",
5455
"ethereum-cryptography": "^2.1.2",

packages/phishing-controller/src/BulkTokenScan.test.ts

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
1-
import { Messenger } from '@metamask/base-controller';
21
import { safelyExecuteWithTimeout } from '@metamask/controller-utils';
3-
import type { TransactionControllerStateChangeEvent } from '@metamask/transaction-controller';
2+
import {
3+
Messenger,
4+
MOCK_ANY_NAMESPACE,
5+
type MessengerActions,
6+
type MessengerEvents,
7+
type MockAnyNamespace,
8+
} from '@metamask/messenger';
49
import nock, { cleanAll } from 'nock';
510
import sinon from 'sinon';
611

7-
import type { PhishingControllerEvents } from './PhishingController';
12+
import type { PhishingControllerMessenger } from './PhishingController';
813
import {
914
PhishingController,
10-
type PhishingControllerActions,
1115
type PhishingControllerOptions,
1216
SECURITY_ALERTS_BASE_URL,
1317
TOKEN_BULK_SCANNING_ENDPOINT,
@@ -30,24 +34,55 @@ const mockSafelyExecuteWithTimeout =
3034

3135
const controllerName = 'PhishingController';
3236

37+
type AllPhishingControllerActions =
38+
MessengerActions<PhishingControllerMessenger>;
39+
40+
type AllPhishingControllerEvents = MessengerEvents<PhishingControllerMessenger>;
41+
42+
type RootMessenger = Messenger<
43+
MockAnyNamespace,
44+
AllPhishingControllerActions,
45+
AllPhishingControllerEvents,
46+
RootMessenger
47+
>;
48+
49+
/**
50+
* Creates and returns a root messenger for testing
51+
*
52+
* @returns A messenger instance
53+
*/
54+
function getRootMessenger(): RootMessenger {
55+
return new Messenger({
56+
namespace: MOCK_ANY_NAMESPACE,
57+
});
58+
}
59+
3360
/**
34-
* Constructs a restricted messenger with transaction events enabled.
61+
* Constructs a messenger with transaction events enabled.
3562
*
3663
* @returns A restricted messenger that can listen to TransactionController events.
3764
*/
38-
function getRestrictedMessengerWithTransactionEvents() {
65+
function getMessengerWithTransactionEvents() {
66+
const rootMessenger = getRootMessenger();
67+
3968
const messenger = new Messenger<
40-
PhishingControllerActions,
41-
PhishingControllerEvents | TransactionControllerStateChangeEvent
42-
>();
69+
typeof controllerName,
70+
AllPhishingControllerActions,
71+
AllPhishingControllerEvents,
72+
RootMessenger
73+
>({
74+
namespace: controllerName,
75+
parent: rootMessenger,
76+
});
77+
78+
rootMessenger.delegate({
79+
actions: [],
80+
events: ['TransactionController:stateChange'],
81+
messenger,
82+
});
4383

4484
return {
45-
messenger: messenger.getRestricted({
46-
name: controllerName,
47-
allowedActions: [],
48-
allowedEvents: ['TransactionController:stateChange'],
49-
}),
50-
globalMessenger: messenger,
85+
messenger,
5186
};
5287
}
5388

@@ -59,7 +94,7 @@ function getRestrictedMessengerWithTransactionEvents() {
5994
*/
6095
function getPhishingController(options?: Partial<PhishingControllerOptions>) {
6196
return new PhishingController({
62-
messenger: getRestrictedMessengerWithTransactionEvents().messenger,
97+
messenger: getMessengerWithTransactionEvents().messenger,
6398
...options,
6499
});
65100
}

packages/phishing-controller/src/PhishingController.test.ts

Lines changed: 66 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
1-
import { deriveStateFromMetadata, Messenger } from '@metamask/base-controller';
2-
import type { TransactionControllerStateChangeEvent } from '@metamask/transaction-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';
39
import { strict as assert } from 'assert';
410
import nock, { cleanAll, isDone, pendingMocks } from 'nock';
511
import sinon from 'sinon';
@@ -10,15 +16,14 @@ import {
1016
METAMASK_STALELIST_FILE,
1117
PhishingController,
1218
PHISHING_CONFIG_BASE_URL,
13-
type PhishingControllerActions,
14-
type PhishingControllerEvents,
1519
type PhishingControllerOptions,
1620
CLIENT_SIDE_DETECION_BASE_URL,
1721
C2_DOMAIN_BLOCKLIST_ENDPOINT,
1822
PHISHING_DETECTION_BASE_URL,
1923
PHISHING_DETECTION_SCAN_ENDPOINT,
2024
PHISHING_DETECTION_BULK_SCAN_ENDPOINT,
2125
type BulkPhishingDetectionScanResponse,
26+
type PhishingControllerMessenger,
2227
} from './PhishingController';
2328
import {
2429
createMockStateChangePayload,
@@ -32,24 +37,59 @@ import { getHostnameFromUrl } from './utils';
3237

3338
const controllerName = 'PhishingController';
3439

40+
type AllPhishingControllerActions =
41+
MessengerActions<PhishingControllerMessenger>;
42+
43+
type AllPhishingControllerEvents = MessengerEvents<PhishingControllerMessenger>;
44+
45+
type RootMessenger = Messenger<
46+
MockAnyNamespace,
47+
AllPhishingControllerActions,
48+
AllPhishingControllerEvents,
49+
RootMessenger
50+
>;
51+
3552
/**
36-
* Constructs a restricted messenger with transaction events enabled.
53+
* Creates and returns a root messenger for testing
3754
*
38-
* @returns A restricted messenger that can listen to TransactionController events.
55+
* @returns A messenger instance
3956
*/
40-
function getRestrictedMessengerWithTransactionEvents() {
57+
function getRootMessenger(): RootMessenger {
58+
return new Messenger({
59+
namespace: MOCK_ANY_NAMESPACE,
60+
});
61+
}
62+
63+
/**
64+
* Constructs a messenger for use in PhishingController tests.
65+
*
66+
* @returns A messenger and the root messenger.
67+
*/
68+
function setupMessenger(): {
69+
messenger: PhishingControllerMessenger;
70+
rootMessenger: RootMessenger;
71+
} {
72+
const rootMessenger = getRootMessenger();
73+
4174
const messenger = new Messenger<
42-
PhishingControllerActions,
43-
PhishingControllerEvents | TransactionControllerStateChangeEvent
44-
>();
75+
typeof controllerName,
76+
AllPhishingControllerActions,
77+
AllPhishingControllerEvents,
78+
RootMessenger
79+
>({
80+
namespace: controllerName,
81+
parent: rootMessenger,
82+
});
83+
84+
rootMessenger.delegate({
85+
actions: [],
86+
events: ['TransactionController:stateChange'],
87+
messenger,
88+
});
4589

4690
return {
47-
messenger: messenger.getRestricted({
48-
name: controllerName,
49-
allowedActions: [],
50-
allowedEvents: ['TransactionController:stateChange'],
51-
}),
52-
globalMessenger: messenger,
91+
messenger,
92+
rootMessenger,
5393
};
5494
}
5595

@@ -60,8 +100,9 @@ function getRestrictedMessengerWithTransactionEvents() {
60100
* @returns The constructed Phishing Controller.
61101
*/
62102
function getPhishingController(options?: Partial<PhishingControllerOptions>) {
103+
const { messenger } = setupMessenger();
63104
return new PhishingController({
64-
messenger: getRestrictedMessengerWithTransactionEvents().messenger,
105+
messenger,
65106
...options,
66107
});
67108
}
@@ -407,8 +448,9 @@ describe('PhishingController', () => {
407448
});
408449

409450
it('replaces existing phishing lists with completely new list from phishing detection API', async () => {
451+
const { messenger } = setupMessenger();
410452
const controller = new PhishingController({
411-
messenger: getRestrictedMessengerWithTransactionEvents().messenger,
453+
messenger,
412454
stalelistRefreshInterval: 10,
413455
state: {
414456
phishingLists: [
@@ -3495,7 +3537,7 @@ describe('URL Scan Cache', () => {
34953537
deriveStateFromMetadata(
34963538
controller.state,
34973539
controller.metadata,
3498-
'anonymous',
3540+
'includeInDebugSnapshot',
34993541
),
35003542
).toMatchInlineSnapshot(`Object {}`);
35013543
});
@@ -3561,18 +3603,16 @@ describe('URL Scan Cache', () => {
35613603

35623604
describe('Transaction Controller State Change Integration', () => {
35633605
let controller: PhishingController;
3564-
let globalMessenger: Messenger<
3565-
PhishingControllerActions,
3566-
PhishingControllerEvents | TransactionControllerStateChangeEvent
3567-
>;
3606+
let globalMessenger: RootMessenger;
35683607
let bulkScanTokensSpy: jest.SpyInstance;
35693608

35703609
beforeEach(() => {
3571-
const messengerSetup = getRestrictedMessengerWithTransactionEvents();
3572-
globalMessenger = messengerSetup.globalMessenger;
3610+
const { messenger, rootMessenger } = setupMessenger();
3611+
3612+
globalMessenger = rootMessenger;
35733613

35743614
controller = new PhishingController({
3575-
messenger: messengerSetup.messenger,
3615+
messenger,
35763616
});
35773617

35783618
bulkScanTokensSpy = jest

0 commit comments

Comments
 (0)