Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
28 of 29 tasks
mcmire opened this issue Jul 14, 2023 · 4 comments
Open
28 of 29 tasks

Comments

@mcmire
Copy link
Contributor

mcmire commented Jul 14, 2023

These controllers are still using BaseController (v1) and need to be migrated to BaseControllerV2. Some controllers don't need to be controllers at all.

Core packages

Non-core packages

Extension controllers

@MajorLift
Copy link
Contributor

MajorLift commented Nov 22, 2023

These also need to be updated:

  • external:
    • SmartTransactionsController
    • SwapsController

Completed:

@MajorLift
Copy link
Contributor

MajorLift commented Dec 6, 2023

@desi @mcmire Could we convert this into an epic? I think it would be helpful keep track of our progress on these upgrades, especially since some of them are prerequisites for work currently being done on controllers by us (#1780) and DevEx, Confirmations etc.

@mcmire mcmire added the Epic label Dec 7, 2023
@mcmire
Copy link
Contributor Author

mcmire commented Dec 7, 2023

@MajorLift Done!

MajorLift added a commit that referenced this issue Dec 11, 2023
## Explanation

As part of the upcoming core wallet library initiative, we plan to
migrate all controllers to `BaseControllerV2`. This commit upgrades
`ComposableController`.

## References

- Closes #2082
- See #1509

## Changelog

### `@metamask/composable-controller`

## Added

- Add types `ComposableControllerState`,
`ComposableControllerStateChangeEvent`, `ComposableControllerEvents`,
`ComposableControllerMessenger`.

## Changed

- **BREAKING:** `ComposableController` is upgraded to extend
`BaseControllerV2`.
- The constructor now expects an options object with required properties
`controllers` and `messenger` as its only argument.
- `ComposableController` no longer has a `subscribe` method. Instead,
listeners for `ComposableController` events must be registered to the
controller messenger that generated the restricted messenger assigned to
the instance's `messagingSystem` class field.
- Any getters for `ComposableController` state that access the internal
class field directly should be refactored to instead use listeners that
are subscribed to `ComposableControllerStateChangeEvent`.

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Ellilot Winkler <[email protected]>
MajorLift added a commit that referenced this issue Dec 22, 2023
…ticIntervalPollingController` (#3609)

## Explanation

This upgrades `TokenDetectionController` to extend `BaseControllerV2`
and `StaticIntervalPollingController` as a preparation step for merging
`TokenDetectionController` with `DetectTokensController`.

## References

- See #1813 
- See #1509
- Closes #3625

## Changelog

### Added
- `TokenListController` now exports a `TokenListControllerMessenger`
type ([#3609](#3609)).
- `TokenDetectionController` exports types
`TokenDetectionControllerMessenger`, `TokenDetectionControllerActions`,
`TokenDetectionControllerGetStateAction`,
`TokenDetectionControllerEvents`,
`TokenDetectionControllerStateChangeEvent`
([#3609](#3609)).
- Add `enable` and `disable` methods to `TokenDetectionController`,
which control whether the controller is able to make polling requests or
all of its network calls are blocked.
([#3609](#3609)).
- Note that if the controller is initiated without the `disabled`
constructor option set to `false`, the `enable` method will need to be
called before the controller can make polling requests in response to
subscribed events.

### Changed
- **BREAKING:** `TokenDetectionController` is upgraded to extend
`BaseControllerV2` and `StaticIntervalPollingController`
([#3609](#3609)).
- The constructor now expects an options object as its only argument,
with required properties `messenger`, `networkClientId`, required
callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`,
`addDetectedTokens`, `getTokenState`, `getPreferencesState`, and
optional properties `disabled`, `interval`, `selectedAddress`.
  
## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Elliot Winkler <[email protected]>
@mcmire mcmire changed the title Ensure that all controllers are using BaseControllerV2 Ensure that all controllers used across clients are using BaseControllerV2 Jul 16, 2024
@desi
Copy link
Contributor

desi commented Nov 14, 2024

We have one additional controller that needs to be updated. AppMetadataController. Adding it to the check list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants