Convert AssetsContractController from BaseControllerV1 to BaseControllerV2
Considering that converting TokenDetectionController to BaseController v2 took longer than expected due to unforeseen changes, we want to be more conscientious about converting AssetsContractController.
Figure out all of the changes we would want to make in upgrading this controller, outline those changes here and create new tickets if appropriate.
-
Define internal messenger actions/events types.
- Define messenger actions for all public methods of
AssetsContractControllerclass. - Define
AssetsContractControllerActions,AssetsContractControllerEvents,AssetsContractControllerGetStateAction(usingControllerGetStateActiontype),AssetsContractControllerStateChangeEvent(usingControllerStateChangeEventtype)
- Define messenger actions for all public methods of
-
Replace constructor option callbacks with messenger actions/events
-
onPreferencesStateChange-
PreferencesController:stateChange - Use
PreferencesControllerStateChangeEvent
-
-
onNetworkDidChange-
NetworkController:networkDidChange - Use
NetworkControllerNetworkDidChangeEvent
-
-
getNetworkClientById-
NetworkController:getNetworkClientById - Use
NetworkControllerGetNetworkClientByIdAction
-
- Define
AllowedActionsunion type and add the above as its members.- Export this type so it's usable by tests and other internal modules, but exclude it as a package-level export.
- Replace the callback invocations in the constructor with
subscribe()calls, using the existing listener logic.
-
-
Define
AssetsContractControllerMessengerusing the types defined in the above bullet points. -
AssetsContractController extends BaseControllerV2. -
AssetsContractConfig- Move properties of this type to private class fields of the
AssetsContractControllerclass. - Set default values from
this.defaultConfigobject. - Replace
this.configurecalls with assignments to corresponding class fields.
- Move properties of this type to private class fields of the
-
constructor
- Replace
supercall so that it conforms to V2 requirements. - Remove
this.initialize()
- Replace
-
Remove
override nameclass field. -
Replace all
privatekeywords for class fields and methods with#prefix. -
Refactor tests. Use
withControllerpattern. -
Consider replacing type annotation for
SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINIDwithsatisfieskeyword. -
Note: This controller has an empty state object
- May not need to be a controller class at all (i.e. should not extend from
BaseControllerV2). - Could be modeled as collection of utility functions or as a service class instead.
- Public class methods could be static methods instead of messenger actions.
- Devise new API for downstream invocation of methods (currently using constructor option callbacks), or define messenger actions even if they don't interact with state?
- May not need to be a controller class at all (i.e. should not extend from
All of the steps above are correct. Rename to not be called Controller and remove it from using the BaseController class. Whoever picks this up should decide how to make this class have the best api.
Acceptance criteria is basically we are not using BaseControllerV1 here.
Some functions seems to be just helper function used within the controller only for example getERC20Standard and getChainId which can be private instead