Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(accounts-controller): re-publish Snap keyring events #5190

Merged
merged 4 commits into from
Jan 27, 2025
Merged

feat(accounts-controller): re-publish Snap keyring events #5190

merged 4 commits into from
Jan 27, 2025

Conversation

ccharly
Copy link
Contributor

@ccharly ccharly commented Jan 23, 2025

Explanation

The SnapKeyring now forward some keyring events coming from a Snap. To avoid having controllers to depend the SnapKeyring for those new events, we subscribe to them from the AccountsController and re-publish them from it. This way, the AccountsController is the sole dependency for those events (and is already a dependency of many controllers anyway).

References

Changelog

@metamask/accounts-controller

  • ADDED: Re-publish Snap keyring events

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 highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@ccharly
Copy link
Contributor Author

ccharly commented Jan 23, 2025

@metamaskbot publish-preview

@github-actions GitHub Actions
Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions. 
{
  "@metamask-previews/accounts-controller": "21.0.2-preview-b1518d1",
  "@metamask-previews/address-book-controller": "6.0.2-preview-b1518d1",
  "@metamask-previews/announcement-controller": "7.0.2-preview-b1518d1",
  "@metamask-previews/approval-controller": "7.1.2-preview-b1518d1",
  "@metamask-previews/assets-controllers": "46.0.1-preview-b1518d1",
  "@metamask-previews/base-controller": "7.1.1-preview-b1518d1",
  "@metamask-previews/build-utils": "3.0.2-preview-b1518d1",
  "@metamask-previews/composable-controller": "10.0.0-preview-b1518d1",
  "@metamask-previews/controller-utils": "11.4.5-preview-b1518d1",
  "@metamask-previews/ens-controller": "15.0.1-preview-b1518d1",
  "@metamask-previews/eth-json-rpc-provider": "4.1.7-preview-b1518d1",
  "@metamask-previews/gas-fee-controller": "22.0.2-preview-b1518d1",
  "@metamask-previews/json-rpc-engine": "10.0.2-preview-b1518d1",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.6-preview-b1518d1",
  "@metamask-previews/keyring-controller": "19.0.4-preview-b1518d1",
  "@metamask-previews/logging-controller": "6.0.3-preview-b1518d1",
  "@metamask-previews/message-manager": "12.0.0-preview-b1518d1",
  "@metamask-previews/multichain": "2.1.0-preview-b1518d1",
  "@metamask-previews/multichain-transactions-controller": "0.0.1-preview-b1518d1",
  "@metamask-previews/name-controller": "8.0.2-preview-b1518d1",
  "@metamask-previews/network-controller": "22.1.1-preview-b1518d1",
  "@metamask-previews/notification-services-controller": "0.16.0-preview-b1518d1",
  "@metamask-previews/permission-controller": "11.0.5-preview-b1518d1",
  "@metamask-previews/permission-log-controller": "3.0.2-preview-b1518d1",
  "@metamask-previews/phishing-controller": "12.3.1-preview-b1518d1",
  "@metamask-previews/polling-controller": "12.0.2-preview-b1518d1",
  "@metamask-previews/preferences-controller": "15.0.1-preview-b1518d1",
  "@metamask-previews/profile-sync-controller": "4.1.1-preview-b1518d1",
  "@metamask-previews/queued-request-controller": "9.0.0-preview-b1518d1",
  "@metamask-previews/rate-limit-controller": "6.0.2-preview-b1518d1",
  "@metamask-previews/remote-feature-flag-controller": "1.3.0-preview-b1518d1",
  "@metamask-previews/selected-network-controller": "21.0.0-preview-b1518d1",
  "@metamask-previews/signature-controller": "23.2.0-preview-b1518d1",
  "@metamask-previews/token-search-discovery-controller": "1.0.0-preview-b1518d1",
  "@metamask-previews/transaction-controller": "43.0.0-preview-b1518d1",
  "@metamask-previews/user-operation-controller": "22.0.0-preview-b1518d1"
}

@ccharly ccharly force-pushed the feat/forward-keyring-events branch from b1518d1 to 37da015 Compare January 24, 2025 21:35
@socket-security Socket Security
Copy link

socket-security bot commented Jan 24, 2025 •
edited
Loading

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/[email protected] None +21 4.65 MB metamaskbot
npm/@metamask/[email protected] None +3 936 kB metamaskbot

View full report↗︎

@ccharly ccharly marked this pull request as ready for review January 24, 2025 21:45
@ccharly ccharly requested review from a team as code owners January 24, 2025 21:45
@ccharly
Copy link
Contributor Author

ccharly commented Jan 27, 2025

@metamaskbot publish-preview

@github-actions GitHub Actions
Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions. 
{
  "@metamask-previews/accounts-controller": "21.0.2-preview-37da015",
  "@metamask-previews/address-book-controller": "6.0.2-preview-37da015",
  "@metamask-previews/announcement-controller": "7.0.2-preview-37da015",
  "@metamask-previews/approval-controller": "7.1.2-preview-37da015",
  "@metamask-previews/assets-controllers": "46.0.1-preview-37da015",
  "@metamask-previews/base-controller": "7.1.1-preview-37da015",
  "@metamask-previews/build-utils": "3.0.2-preview-37da015",
  "@metamask-previews/composable-controller": "10.0.0-preview-37da015",
  "@metamask-previews/controller-utils": "11.4.5-preview-37da015",
  "@metamask-previews/ens-controller": "15.0.1-preview-37da015",
  "@metamask-previews/eth-json-rpc-provider": "4.1.7-preview-37da015",
  "@metamask-previews/gas-fee-controller": "22.0.2-preview-37da015",
  "@metamask-previews/json-rpc-engine": "10.0.2-preview-37da015",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.6-preview-37da015",
  "@metamask-previews/keyring-controller": "19.0.4-preview-37da015",
  "@metamask-previews/logging-controller": "6.0.3-preview-37da015",
  "@metamask-previews/message-manager": "12.0.0-preview-37da015",
  "@metamask-previews/multichain": "2.1.0-preview-37da015",
  "@metamask-previews/multichain-transactions-controller": "0.0.1-preview-37da015",
  "@metamask-previews/name-controller": "8.0.2-preview-37da015",
  "@metamask-previews/network-controller": "22.1.1-preview-37da015",
  "@metamask-previews/notification-services-controller": "0.16.0-preview-37da015",
  "@metamask-previews/permission-controller": "11.0.5-preview-37da015",
  "@metamask-previews/permission-log-controller": "3.0.2-preview-37da015",
  "@metamask-previews/phishing-controller": "12.3.1-preview-37da015",
  "@metamask-previews/polling-controller": "12.0.2-preview-37da015",
  "@metamask-previews/preferences-controller": "15.0.1-preview-37da015",
  "@metamask-previews/profile-sync-controller": "4.1.1-preview-37da015",
  "@metamask-previews/queued-request-controller": "9.0.0-preview-37da015",
  "@metamask-previews/rate-limit-controller": "6.0.2-preview-37da015",
  "@metamask-previews/remote-feature-flag-controller": "1.3.0-preview-37da015",
  "@metamask-previews/selected-network-controller": "21.0.0-preview-37da015",
  "@metamask-previews/signature-controller": "23.2.0-preview-37da015",
  "@metamask-previews/token-search-discovery-controller": "1.0.0-preview-37da015",
  "@metamask-previews/transaction-controller": "43.0.0-preview-37da015",
  "@metamask-previews/user-operation-controller": "22.0.0-preview-37da015"
}

@ccharly ccharly mentioned this pull request Jan 27, 2025
Draft
7 tasks
salimtb
salimtb approved these changes Jan 27, 2025
View reviewed changes
Copy link
Contributor

@salimtb salimtb left a comment

Choose a reason for hiding this comment

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

Assets changes LGTM

@ccharly ccharly enabled auto-merge (squash) January 27, 2025 14:34
@ccharly ccharly merged commit 9791790 into main Jan 27, 2025
359 checks passed
@ccharly ccharly deleted the feat/forward-keyring-events branch January 27, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danroc danroc danroc approved these changes

@salimtb salimtb salimtb approved these changes

@Prithpal-Sooriya Prithpal-Sooriya Prithpal-Sooriya approved these changes

@sahar-fehri sahar-fehri sahar-fehri approved these changes

@zone-live zone-live zone-live approved these changes

Assignees
No one assigned
Labels
None yet
Projects
PR review queue
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ccharly @zone-live @sahar-fehri @Prithpal-Sooriya @salimtb @danroc