Skip to content

refactor: migrate PhishingController to @metamask/messenger - #6535

Merged
FrederikBolding merged 7 commits into
mainfrom
cryptodev-2s/messenger/phishing-controller
Oct 27, 2025
Merged

refactor: migrate PhishingController to @metamask/messenger#6535
FrederikBolding merged 7 commits into
mainfrom
cryptodev-2s/messenger/phishing-controller

Conversation

@cryptodev-2s

@cryptodev-2scryptodev-2s commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

Explanation

This PR migrates PhishingController to the new @metamask/messenger message bus, as opposed to the one exported from @metamask/base-controller.

References

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, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

Note

Migrates PhishingController to use @metamask/messenger and renames metadata field anonymous to includeInDebugSnapshot.

  • Breaking Changes
    • Migrate PhishingController to @metamask/messengerMessenger (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.

Written by Cursor Bugbot for commit c2988bb. This will update automatically on new commits. Configure here.

mcmire
mcmire previously approved these changes Sep 10, 2025
@cryptodev-2s
cryptodev-2sforce-pushed the cryptodev-2s/messenger/phishing-controller branch 4 times, most recently from ed9948c to 8687d79CompareOctober 7, 2025 13:39
@cryptodev-2s
cryptodev-2sforce-pushed the cryptodev-2s/messenger/phishing-controller branch from 8687d79 to 0ab7b74CompareOctober 7, 2025 13:40
cursor[bot]

This comment was marked as outdated.

mcmire
mcmire previously approved these changes Oct 7, 2025

@mcmiremcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

PhishingControllerEvents | AllowedEvents,
AllowedActions['type'],
AllowedEvents['type']
PhishingControllerEvents | AllowedEvents

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing Type Parameter in Messenger Definition

The PhishingControllerMessenger type definition is missing the fourth type parameter that was present in the old RestrictedMessenger type. The new Messenger type from @metamask/messenger requires a fourth type parameter (parent messenger type) for child messengers. The current definition Messenger<typeof controllerName, PhishingControllerActions | AllowedActions, PhishingControllerEvents | AllowedEvents> should be Messenger<typeof controllerName, PhishingControllerActions | AllowedActions, PhishingControllerEvents | AllowedEvents, ParentMessengerType> where ParentMessengerType is the type of the parent messenger that this controller's messenger is derived from. Without this fourth parameter, the messenger delegation pattern used in tests (lines 84-88 in PhishingController.test.ts) may not work correctly with type safety.

Fix in CursorFix in Web

@FrederikBolding
FrederikBolding added this pull request to the merge queueOct 27, 2025
@FrederikBolding
FrederikBolding removed this pull request from the merge queue due to the queue being cleared Oct 27, 2025
@FrederikBolding
FrederikBolding added this pull request to the merge queueOct 27, 2025
Merged via the queue into main with commit 249f9b0Oct 27, 2025
256 of 257 checks passed
@FrederikBolding
FrederikBolding deleted the cryptodev-2s/messenger/phishing-controller branch October 27, 2025 12:17
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@cryptodev-2s@mcmire@Mrtenz@FrederikBolding@mikesposito