core icon indicating copy to clipboard operation
core copied to clipboard

Convert TokensController from BaseControllerV1 to BaseControllerV2

Open desi opened this issue 1 year ago • 2 comments

Considering that converting TokenDetectionController to BaseController v2 took longer than expected due to unforeseen changes, we want to be more conscientious about converting TokensController.

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.

desi avatar Mar 18 '24 18:03 desi

TokensController is mostly in good shape. It already uses a messenger, it already broadcasts events through it and it already handles actions properly.

Along with the main change of extending BaseControllerV2 class instead of V1, we can:

  • Rename TokenState type to TokensControllerState, to match other controllers.
  • Same for TokensConfig -> TokensControllerConfig
  • We should be able to use the already declared allowed events and actions
  • We should lower the visibility of these functions to #:
    • _createEthersContract -> #createEthersContract
    • _detectIsERC721 -> #detectIsERC721
    • _generateRandomId -> #generateRandomId
    • _getNewAllTokensState -> #getNewAllTokensState
    • _getProvider -> #getProvider
    • _requestApproval -> #requestApproval
  • We should be able to remove the hub property, since it appears unused (TODO: verify this statement)
  • Tests should be refactored to support the new controller version
    • In addition to that, we can create a withController helper to make tests more readable and easier to be maintained

mikesposito avatar Apr 17 '24 09:04 mikesposito

hub is used in Mobile in one place.

desi avatar Apr 17 '24 15:04 desi