Convert AssetsContractController from BaseControllerV1 to BaseControllerV2 #4072

Activity

1. desi

   changed the title [-](`assets-controllers`) AssetsContractController[/-] [+]Convert AssetsContractController from BaseControllerV1 to BaseControllerV2[/+] on Mar 18, 2024
2. desi

   added

   team-wallet-frameworkDeprecated: Please use `team-core-platform` instead.Deprecated: Please use `team-core-platform` instead.

   on Mar 18, 2024
3. MajorLift commented on May 23, 2024

   

   MajorLift

   on May 23, 2024

   Last edited by MajorLift

   Contributor

   More actions

   • Define internal messenger actions/events types.

     • Define messenger actions for all public methods of AssetsContractController class.
     • Define AssetsContractControllerActions, AssetsContractControllerEvents, AssetsContractControllerGetStateAction (using ControllerGetStateAction type), AssetsContractControllerStateChangeEvent (using ControllerStateChangeEvent type)

   • Replace constructor option callbacks with messenger actions/events

     • onPreferencesStateChange
       • PreferencesController:stateChange
       • Use PreferencesControllerStateChangeEvent
     • onNetworkDidChange
       • NetworkController:networkDidChange
       • Use NetworkControllerNetworkDidChangeEvent
     • getNetworkClientById
       • NetworkController:getNetworkClientById
       • Use NetworkControllerGetNetworkClientByIdAction
     • Define AllowedActions union 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 AssetsContractControllerMessenger using the types defined in the above bullet points.

   • AssetsContractController extends BaseControllerV2.

   • AssetsContractConfig

     • Move properties of this type to private class fields of the AssetsContractController class.
     • Set default values from this.defaultConfig object.
     • Replace this.configure calls with assignments to corresponding class fields.

   • constructor

     • Replace super call so that it conforms to V2 requirements.
     • Remove this.initialize()

   • Remove override name class field.

   • Replace all private keywords for class fields and methods with # prefix.

   • Refactor tests. Use withController pattern.

   • Consider replacing type annotation for SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID with satisfies keyword.

   • 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?
4. desi commented on May 23, 2024

   

   desi

   on May 23, 2024

   Last edited by desi

   ContributorAuthor

   More actions

   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.
5. cryptodev-2s commented on May 23, 2024

   

   cryptodev-2s

   on May 23, 2024

   Last edited by cryptodev-2s

   Contributor

   More actions

   Some functions seems to be just helper function used within the controller only for example getERC20Standard and getChainId which can be private instead
6. MajorLift

   self-assigned this

   on Jun 5, 2024
7. MajorLift

   closed this as completedin #4397on Jul 19, 2024
8. MajorLift

   closed this as completedin 4485e8aon Jul 19, 2024
9. MajorLift

   added a commit that references this issue on Jul 22, 2024

   [assets-contract-controller] Apply messenger pattern and remove inher…

   ...

   4485e8a
10. MajorLift

    reopened this on Jul 26, 2024
11. MajorLift

    linked a pull request that will close this issueAdd public `name` property to `AssetsContractController` class #4564on Jul 26, 2024
12. MajorLift

    added a commit that references this issue on Jul 30, 2024

    Add public `name` property to `AssetsContractController` class (#4564)

    ...

    4398a99
13. MajorLift

    closed this as completedin #4564on Jul 30, 2024
14. AugmentedMode

    added a commit that references this issue on Jul 30, 2024

    Add public `name` property to `AssetsContractController` class (#4564)

    ...

    f6e81a0
15. georgewrmarshall

    added a parent issue on Jun 1, 2025

    • Ensure that all controllers used across clients are using BaseControllerV2 #1509