chore!: Expose confirmations-owned controller/service methods through messenger#8183
Merged
cryptodev-2s merged 13 commits intomainfrom Mar 17, 2026
Conversation
… messenger Migrate 10 confirmations-team controllers to the MESSENGER_EXPOSED_METHODS + registerMethodActionHandlers pattern with auto-generated action types. BREAKING CHANGE: ApprovalController methods renamed: clear→clearRequests, has→hasRequest, accept→acceptRequest, reject→rejectRequest, success→showSuccess, error→showError. TransactionController action types standardized with Action suffix.
Replace AddApprovalRequest, AcceptApprovalRequest, HasApprovalRequest, RejectApprovalRequest aliases with their full generated names across all consumer packages. Add CHANGELOG entries for permission-controller and assets-controllers.
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
matthewwalsh0
requested changes
Mar 11, 2026
packages/transaction-pay-controller/src/TransactionPayController-method-action-types.ts
Show resolved
Hide resolved
|
|
||
| // === MESSENGER === | ||
|
|
||
| const MESSENGER_EXPOSED_METHODS = [ |
Member
There was a problem hiding this comment.
Could these not be auto-generated also since it correlates to any public methods?
We could store these constants in the action-types file also?
Contributor
Author
There was a problem hiding this comment.
Actually, the action-types file is auto-generated from this constant and shouldn’t be updated manually. This constant is what triggers the action types generation.
Member
There was a problem hiding this comment.
Exactly, but this constant surely just maps to every public method, so could we skip a step?
Contributor
There was a problem hiding this comment.
@matthewwalsh0 we can have public methods that are available but not exposed through the messenger, that's why we can't auto-generate this array.
Member
There was a problem hiding this comment.
Why would we want that though? Is the intent of this PR not to assume all our public methods should be in the messenger? Is the end goal not to exclusively interact with our controllers via messenger actions in the clients and other controllers?
Contributor
There was a problem hiding this comment.
@matthewwalsh0 Yes, that is true. In the end we do want to use the messenger to facilitate communication between controllers and services in the platform, and even between the background/engine layer and UI layer in the clients. So at this stage we should be exposing all public methods through the messenger and we should be making any methods that are not accessed in the background/engine or UI layers private (or removing them if they are dead code).
We just don't have a way to automatically expose all public methods through the messenger because when we added this MESSENGER_EXPOSED_METHODS constant and the accompanying script, we weren't at the stage where we were ready to make the change we are making now, we just wanted to make it easier for teams to do what they were already doing at the time. I guess we could do that now, but we didn't want to change the workflow. Once we are finished going through all controllers and exposing methods through the messenger, we can follow your suggestion and reduce the boilerplate around the messenger even further.
7 tasks
GuillaumeRx
requested changes
Mar 12, 2026
Contributor
There was a problem hiding this comment.
I'm wondering if renaming the methods here isn't a stretch of the OG task 🤔
Contributor
Author
There was a problem hiding this comment.
I did this because when reading the action type it was meaning what it does. and thats why I asked the team if they are okay with the change. both are breaking the same way either renaming the type or the methods.
Contributor
There was a problem hiding this comment.
Renaming the type is breaking but didn't introduce any needed changes in the clients, most of the time we only use them in other controllers. Renaming the method requires the client to update all the usage occurrences of these methods.
Contributor
Author
There was a problem hiding this comment.
That might be true for this specific controller, but it’s not true in general, it depends on the controller and how it’s being used. We’ve generally been leaning toward using the Messenger, and the work I’m doing in this PR is specifically to make clients consume these directly via the Messenger.
Ultimately, it comes down to which version is clearer in the UI. I’m not particularly in favor of either option, I raised the question with the team on Slack to avoid renaming later and having to introduce a breaking change. I will let them decide.
Contributor
There was a problem hiding this comment.
@cryptodev-2s I would suggest not renaming the methods themselves, but merely exposing them through the messenger. We should certainly let the team decide whether they want to do that but regardless I would agree that kind of change seems out of scope for this ticket.
Contributor
There was a problem hiding this comment.
@cryptodev-2s Oh, but I see, the names of the messenger actions currently don't match the names of the methods. Hmm 🤔 I will still recommend an approach that does not force us or the team to make many changes to the clients. So maybe we should favor the names of the methods and name the actions after that.
Contributor
Author
There was a problem hiding this comment.
@mcmire clients PRs are already ready as shared in the description
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
…s-through-messenger
Contributor
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
matthewwalsh0
previously approved these changes
Mar 16, 2026
Contributor
|
Changelog check is failing :( |
Contributor
Author
|
@metamaskbot publish-previews |
GuillaumeRx
previously approved these changes
Mar 17, 2026
matthewwalsh0
approved these changes
Mar 17, 2026
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
matthewwalsh0
requested changes
Mar 17, 2026
…tions-controller-methods-through-messenger
GuillaumeRx
approved these changes
Mar 17, 2026
matthewwalsh0
approved these changes
Mar 17, 2026
ffmcgee725
approved these changes
Mar 17, 2026
Member
ffmcgee725
left a comment
ffmcgee725
left a comment
There was a problem hiding this comment.
LGTM in regards to @MetaMask/wallet-integrations owned files!
salimtb
approved these changes
Mar 17, 2026
Prithpal-Sooriya
approved these changes
Mar 17, 2026
cryptodev-2s
deleted the
chore/expose-confirmations-controller-methods-through-messenger
branch
Contributor
Author
|
@metamaskbot publish-previews |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Confirmations-team controllers currently register messenger action handlers manually, with hand-written action types. The rest of the codebase (Assets, Notifications, Earn) has already migrated to the
MESSENGER_EXPOSED_METHODS+registerMethodActionHandlerspattern with auto-generated action types.This PR migrates all 10 confirmations-owned controllers to that same pattern:
clear→clearRequests,has→hasRequest,accept→acceptRequest,reject→rejectRequest,success→showSuccess,error→showError), newaddRequest()public method, 16 exposed methodsgetGasFeeTokens()public wrapper, action type names standardized withActionsuffixgetDelegationTransaction()andgetStrategy()public wrappersclear)clear)Downstream consumers (
PermissionController,NftController,TokensController,NftDetectionController,TransactionControllerIntegrationtests) are updated to use the renamed methods and types.Clients PRs
References
Checklist
Note
Medium Risk
Wide-ranging refactor across multiple controllers that changes public method/type names (notably in
ApprovalController) and updates downstream consumers; risk is mainly integration breakage from renamed APIs and newly exposed messenger methods.Overview
Migrates confirmations-owned controllers to the
MESSENGER_EXPOSED_METHODS+registerMethodActionHandlerspattern, adding auto-generated*-method-action-types.tsfiles and exporting the new action types.Introduces breaking
ApprovalControllerAPI renames (accept→acceptRequest,reject→rejectRequest,has→hasRequest,clear→clearRequests,success→showSuccess,error→showError) and adds a publicaddRequest()wrapper; similarly promotes previously-internal/handler-only methods to public wrappers inTransactionController(getGasFeeTokens) andTransactionPayController(getDelegationTransaction,getStrategy).Updates dependent controllers/tests (e.g.
PermissionController, Assets controllers, transaction integration tests) to use the renamed methods and new action-type imports, and wires up new messenger-exposed methods forAddressBookController,EnsController,GasFeeController,LoggingController,NameController,SignatureController, andTransactionController. Addstsxand per-packagegenerate-method-action-typesscripts, plus changelog entries reflecting the new/breaking APIs.Written by Cursor Bugbot for commit 33534a4. This will update automatically on new commits. Configure here.