Skip to content

Conversation

@mikesposito
Copy link
Member

@mikesposito mikesposito commented Aug 25, 2025

Explanation

This PR migrates the AccountTreeController class to the new @metamask/messenger comm system, 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 AccountTreeController and tests to the new @metamask/messenger API, updates state metadata and base-controller/next usage, and adds the messenger dependency.

  • AccountTreeController:
    • Switch to @metamask/messenger (replace messagingSystem/restricted messenger with messenger; update subscribe/publish/call and action handler registration).
    • Use @metamask/base-controller/next; change state metadata key from anonymous to includeInDebugSnapshot.
    • Pass messenger through backup/sync context; construct rules (EntropyRule, SnapRule, KeyringRule) with messenger.
  • Tests:
    • Add tests/mockMessenger to provide root and controller messengers; refactor all tests to use it; remove inline Messenger scaffolding and update spies/handlers accordingly.
  • Package/Config:
    • Add dependency @metamask/messenger and reference it in tsconfig files; update README dependency graph to include account-tree-controller --> messenger.
  • Changelog:
    • Document BREAKING change: controller now accepts Messenger from @metamask/messenger instead of RestrictedMessenger from @metamask/base-controller.

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

@mikesposito mikesposito force-pushed the mikesposito/messenger/account-tree-controller branch from 5562ae3 to cf24808 Compare August 25, 2025 22:21
@mikesposito mikesposito changed the title (wip) refactor: migrate AccountTreeController to @metamask/messenger refactor: migrate AccountTreeController to @metamask/messenger Aug 25, 2025
@mikesposito mikesposito marked this pull request as ready for review August 25, 2025 22:41
@mikesposito mikesposito requested review from a team as code owners August 25, 2025 22:41
@mikesposito mikesposito mentioned this pull request Aug 22, 2025
43 tasks
@mikesposito mikesposito force-pushed the mikesposito/messenger/account-tree-controller branch from cf772f1 to da25c0e Compare August 28, 2025 11:40
mcmire
mcmire previously approved these changes Aug 28, 2025
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

LGTM!

@mikesposito mikesposito enabled auto-merge (squash) September 23, 2025 09:58
cursor[bot]

This comment was marked as outdated.

mcmire
mcmire previously approved these changes Oct 7, 2025
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

LGTM!

AccountTreeControllerActions | AllowedActions,
AccountTreeControllerEvents | AllowedEvents
>;
messenger?: ReturnType<typeof getRootMessenger>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Do we want to export a RootMessenger type from mockMessenger.ts?

Copy link
Contributor

@cryptodev-2s cryptodev-2s left a comment

Choose a reason for hiding this comment

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

LGTM!

() => EMPTY_ACCOUNT_MOCK,
// Mock getSelectedAccount to return EMPTY_ACCOUNT_MOCK (id is '') BEFORE init
mockGetSelectedMultichainAccountActionHandler.mockReturnValue(
EMPTY_ACCOUNT_MOCK,
Copy link

Choose a reason for hiding this comment

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

Bug: Mock Handler Not Registered

The mockGetSelectedMultichainAccountActionHandler is defined but not registered as the handler for AccountsController:getSelectedMultichainAccount. Tests attempt to configure its return value, but the action is actually handled by mocks.AccountsController.getSelectedMultichainAccount. This means the test mocks have no effect, leading to unexpected test behavior.

Fix in Cursor Fix in Web

@mikesposito mikesposito merged commit a0a3194 into main Oct 27, 2025
255 checks passed
@mikesposito mikesposito deleted the mikesposito/messenger/account-tree-controller branch October 27, 2025 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants