MetaMask / MetaMask/core

[base-controller] Controller constructors accept messengers that do not allow any of its internal actions and/or events

Open
#4,501 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug team-wallet-framework wf-bugs
Dominant language
TypeScript
Stars
413
Forks
308
Avg merge
1d 4h
Merged PRs (30d)
253

Description

## Problem

Constructor controllers currently raise a type error when instantiated with a messenger that was defined with an incomplete list of internal actions/events, but fail to raise that error if either the `Action` or `Event` type union of the messenger's parent `ControllerMessenger` doesn't contain any of the controller's internal actions/events.

## Repro

```ts
/**
* 1. Controller constructor erroneously accepts messenger if one or more of its internal message lists are "empty"
*/

/* A. Both internal message lists are empty */

const emptyInternalMessageListsControllerMessenger = new ControllerMessenger<
// `TokenRatesControllerGetStateAction` is the only member of `TokenRatesControllerActions`
Exclude | TokenRatesControllerAllowedActions,
// `TokenRatesControllerStateChangeEvent` is the only member of `TokenRatesControllerEvents`
Exclude | TokenRatesControllerAllowedEvents
>()

new TokenRatesController({
messenger: emptyInternalMessageListsControllerMessenger.getRestricted({
name: 'TokenRatesController',
allowedActions: [
'TokensController:getState',
'NetworkController:getNetworkClientById',
'NetworkController:getState',
'AccountsController:getAccount',
'AccountsController:getSelectedAccount',
],
allowedEvents: [
'TokensController:stateChange',
'NetworkController:stateChange',
'AccountsController:selectedEvmAccountChange'
],
}),
tokenPricesService: buildMockTokenPricesService(),
})

/* B. Only one internal message list is empty */

const emptyInternalMessageListControllerMessenger = new ControllerMessenger<
TokenRatesControllerActions | TokenRatesControllerAllowedActions,
// `TokenRatesControllerStateChangeEvent` is the only member of `TokenRatesControllerEvents`
Exclude | TokenRatesControllerAllowedEvents
>()

new TokenRatesController({
messenger: emptyInternalMessageListControllerMessenger.getRestricted({
name: 'TokenRatesController',
allowedActions: [
'TokensController:getState',
'NetworkController:getNetworkClientById',
'NetworkController:getState',
'AccountsController:getAccount',
'AccountsController:getSelectedAccount',
],
allowedEvents: [
'TokensController:stateChange',
'NetworkController:stateChange',
'AccountsController:selectedEvmAccountChange'
],
}),
tokenPricesService: buildMockTokenPricesService(),
})

// Argument of type '"TokenRatesController:getState"' is not assignable to parameter of type
// '"TokensController:getState" | "NetworkController:getNetworkClientById" | "NetworkController:getState" |
// "AccountsController:getAccount" | "AccountsController:getSelectedAccount"'.(2345)
emptyInternalMessageListsControllerMessenger.call('TokenRatesController:getState')
emptyInternalMessageListsControllerMessenger.call('NetworkController:getState')

// Argument of type '"TokenRatesController:stateChange"' is not assignable to parameter of type
// '"TokensController:stateChange" | "NetworkController:stateChange" | "AccountsController:selectedEvmAccountChange"'.(2345)
emptyInternalMessageListControllerMessenger.subscribe('TokenRatesController:stateChange', [])

/**
* 2. Controller constructor raises error when passed messenger with "incomplete" internal message lists
*/

const incompleteInternalMessageListControllerMessenger = new ControllerMessenger<
| Extract<
AccountsControllerActions,
| AccountsControllerGetAccountAction
| AccountsControllerGetSelectedAccountAction
>
| AccountsControllerAllowedActions,
| Extract<
AccountsControllerEvents,
AccountsControllerSelectedEvmAccountChangeEvent
>
| AccountsControllerAllowedEvents
>()

new AccountsController({
// Type 'ControllerMessenger'
// is not assignable to type 'AccountsControllerMessenger'.
// Property '#private' in type 'ControllerMessenger' refers to a different member
// that cannot be accessed from within type 'RestrictedControllerMessenger'.(2322)
// AccountsController.d.ts(102, 9): The expected type comes from property 'messenger'
// which is declared here on type '{ messenger: AccountsControllerMessenger; state: AccountsControllerState; }'
messenger: incompleteInternalMessageListControllerMessenger,
state: {
internalAccounts: {
accounts: {},
selectedAccount: '',
},
},
})

```

> [playground link](https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBDAnmApnA3nAUHOBBAYwIgFcA7GAZwGEIKoIAbRlKQmYOygGh3yNIUadGA2asAogDcUQ3rkLFyVWvSYsoAcRQxFg3QQ51u2BQOXC14rToDKKFoZQATPcvacyJvm6GrR6qz2jjAu0iC+MNQAFgCGZADmKNKyMLwAvnAAZgwgcADkAAIgOrEgsZQA1gD0seZCALTEVhr5WKCQsBim-Ep+ImIaGdm5BcWl5VW19VRNA4FQbR3Q8EioGHwAKhCVspYB1towtjCxoR7GWzt7-oNBp6Ex8UkpFPJw27tkAEpnKPt3NiGTw8HqfWS-UIAhavKjeXDgn5-aGHOwPFAXLxg65IqG3BYnP5PRLJGRvLCZHIQPJFEqnSY1CqUHSUOYtViUJbgFbdRGQ-746xwSmjWkTCqMyjM2bNA4aTntbldTCCjQAWX+zJJUGFI2pYzpZQl1QARhUUGy5awuZ1Vsg0Og+ABpFCIKDARKq1hHF1uj0JABi0EimPevvdnvmqJg4f9lAAQohNvbQ87XRGEl6bAY+lRU7hY5H2VBCY84iTYcMqTTxvTjbs-YlLYCbTy1g64AA5HQAd2glSzR1LGOBxi7vf7g503ZgfagA8YwFSiYAkq5R1iZ3OB1GNMPiS8yfARfqxXWpmQJ-PmwtW11291bGRYmB9+WkrrqwbxVNKM+wKysqApyADcWBYNUABUkE4JBcAAIwAHRwFmcDNJQogkIY0BwKwDCXqQlCMIgcB1AQKBgFQcAlFKshJDqwBZHAdBoDhIDQKxTHAFRHqhFAz6MNRmqxB+i4YZQpFQGgABEKDgEg0mwdU4FQfgyHxhAMDRHAvGsAJQlSiJaBiVRsRSbh8kkZBylYOh8ByZRiArhQemxIwGqGUkAAywDiVmHlavRcAALxwJePYoburABXRrAADx8NU1RwAABnyyJTsc6KYilOkSVprFkMRQkgCarDMUxaU4vyKIaJilApXwEgAB4EIwJDOCgcXpXiUVAkYZCgj1Ap9UO2UbgAfHAAA+HzVRlfV4MwEA9i49XvElqXDbV9xEu+pKpLlvkINEhXFSUpXlRAlXbVmsINU1rXtZ13Xzb1xb3SYt19W+zwHRQU2zd9xZLYwK1hEelBYBNAAUACUfBYOFc1fDVWYw46uA0YFrAAFwWY5zl8QJAVGT5fl9TF2qIUkMDfP8ojAE4zgY3wuDPiU+P5MDVqLO8uBuWDq3rgNlD4wA2mzuAFIiO1QLjtPDvk-PS-kW6Tn1CvTleC5LhQq7OMrUu4GrOtZlrWV-Eb0uq5EcsW5E1s2wUdvm4rDgoMzjsqwAuirgvg8490S8bMs4vbGF7X9Ts26bs4a8WuOR2W0cqybrua8yIQQxEMwHigbQ237fDpHD7wwDiAAK7rkZQ9hQFITMoPjJokMAjDOGqEAEJUiLV03desI35HwxkCMQXB8bIQA8kVJEsTpLn8W5BmUEZcAmXlBNIHA1ngXZ29OUvJPCd5vlRJTmqxTqoXI-5V-aglCJvSNIMbhJQMv3LoOB+tiXJVVVGC1iy-QrEeI6+VTrMTniVMqOprpbS-ndSGjVcAtTah1LqPNASfRRhCYBvNQGHlSIDPBuJX68x-sLe60N4aI2RtghYrMsYP3ovjBySAiauXcqfFA5ML7FipvRGmOh6YYRrqEFmmNpYc2bmHIB71eYx1IstYW9UQ7O3kTcTWit0TKJNurecbttbxyMYuZcTlDZpwKIYnciddFW2sfkDO9idDe1Ds4mY9t3bZ3XLmZRxcbYB2oZDDRztubh3NsnFA+d9E2LNpndEsSnEuN5knD2zNwiRFiVLQJwoy58Arl8futd67Dzka3dundu69yrjXf4ZSm6jwpOPTaeAoAJBICUCgFUED2gKNJRh1gLbDmkvkLeZBNKkSlMABIz4TQsAQBAOAYAzJlB0FdJiD4ILJXyIMyJOi0R-GkjNOA0lbHGJgBc8x+tLEnNmuchJrjLahHudgTa0lUmAgdjMN5nyvGXOCJ7SRkQxmIRhgAJgAMwABYACsCMOFH2Jm5UmZ8KaCNYawRCBBBYwwiQoih3yHGhHyIiyyXDl48M8nw8+cshHYtxcwfFFzDkvILq05K7TOndPgAgh8eyhkaCTkk-aYyJlTKZLM+ZiyK4rLWXSTZfT1g7IGbLKJoq-p-NZYnaJ+c-lfIWOk3xWS85ivyOC6F8LyWE2Pqi3h-D760WppQEgJpKAEHdGVfFQq8Z6v2srOA4sfbjwnjBXecAIXIVQnZKAWEK46igLEXy-xcJQAYDqHsp0yDytos4Ve184A9m4tpaSHpiDgBYK8xeKLBLY3XiZKGu8bIH3LdSMAVaUCUpPjSx1l9nVBVvigCKTqcZQCfqclqog6gwAnTbQ11h1o9BtrNBdGgjghg3Mu6Wq6AWjTsBkkFMxMRSymnwXduZv6qLWu-d4s0p1JsMHO6Wa7JCQ2sa+ksh6c7ZP2rCU9PQL36CvULCGqQoaw3HsjT9rNNoIn6fkUd184qfo3cerdQGLCZSBV7dDA13nJUw-0EG16RYghMJ+nDkjTW5nzrCU5n6qFgaEBNcZqrjqTPgFKuZsQFloDlQKz9DLFiIVVbgauEBUCwBIvkAAxGAd0UgraL2VWgRD-ax3jKklkDkSzSJwGcIxHTUkekXTgQRk6Zw0LxE43AMqpEiBX3zV+YtWkPSqYKGIxmzMkPagtZCqFEKIUIzaXu4siFnCISoDDeCAAGCFJgACccN8abCgSgZqqBmYeYramr8CnJOsB3vkbG182ObSzUzbSx1OptTMi4OAp1zJ0A8-kTApXtT4yE1iqAIE4DRK62Fwh6I+vpELgWzri8K0do2d2+1vbz6+fou8AbGwba6SpXbfG0igleO2+kaxWdgU3tzFzZRB2S5jxUlyjpXTUi9IFf8y9gL0Tio45KmZPG+N6dWUmxV8Ctn9NVXsz9PzcwGqG8Sg9vjQWnOkoWTMbKEdBiBODizDyEeXIRwmJM9o-mY7ZXbMFAXrXtDINNztc3qVr3RQI3mwmcV4s8c9tlSsEZtsrbNu11OyaLY09fRnzLmfAcud7AoUIcz6DXGS8CYaDMUVkJ1cnS4JJ7ywA+X1bBSP1RCoBshcsxp-ExHrnVvMjjXL1jAA2J7cCzVN4CQ35wNznt6CL-dkv3DO9t67rD7uqMnf0JiMC6v+ma6Y0HSGuvcAu-VT9TVYDUgu-twSePxCKAu8o9+iPudaN-qPMHh8jHtfvxCi7gnxYfTpn9CjzdA0y9V6LGbnQ2PEzJlQDb055em8e6EEH8ChfIcLHD-dXXLunwviISgevjZEcgNT-9GAwesjkC3ZUjuXce593qYPBuTS4ZrbgFJGAJB+IH4FpQRA5Psg6AINELfA94Zn+lkfk-Ob0DpDAjbC7NslOLmcFHD0NcWwEgMAW0FwGGAAfVv2TTIDXHxnIEqEmR7DIBSzgCgLiEAPzWOhSli2agABJ0BxF-R0hcodtn8dBX8EA40UBP9pZv9pZf9gB-9HgT8TMCBEBgDQCVhwCoDWDZB2D4CyBECVoUD8ZeD01+CSJjoiDEgn9cAX9T9MIaCpZ6D0gKRwIgA)

## Acceptance Criteria

Controllers inheriting from `BaseControllerV2` must raise a type and/or runtime error if they are initialized with a messenger that is derived from an unrestricted `ControllerMessenger` that does not contain any of the controller's internal actions and/or events.

## References

- Similar https://github.com/MetaMask/core/issues/4414
- Related https://github.com/MetaMask/core/issues/4213

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading BaseControllerV2, ControllerMessenger, and the restricted messenger types, then reproduce the empty internal action and event cases in the linked TypeScript playground. Compare the behavior with the incomplete-list AccountsController example and related issue 4414; done means controllers reject messengers whose parent type contains none of their internal actions or events.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend-api-design
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.