chore: create new assets controller package#7587
Conversation
There was a problem hiding this comment.
Two questions:
-
What do you think about copying and pasting one of the controllers in the
sample-controllerspackage? This way we can keep the general look and feel for a controller consistent across all packages. -
I see that we are creating a new package called
assets-controller, when we already have a package calledassets-controllers(with an "s"). I'm a bit worried this will be a constant source of confusion, and so perhaps we shouldn't make a new package yet? Or — are there any near-term plans to split apartassets-controllers?
There was a problem hiding this comment.
@mcmire yes, there are near-term plans to split apart assets-controllers. The goal is to deprecate the existing assets-controllers (plural) package and break it into several focused, single-responsibility packages:
@metamask/assets-controller- consolidated asset tracking (balances, detection)- Other controllers from the package will be split into their own dedicated packages as well
The naming is intentional:
assets-controllers= legacy "kitchen sink" package (to be deprecated)assets-controller= new single unified controller
This follows the pattern we've used elsewhere in the monorepo where packages contain a single controller (e.g., @metamask/network-controller, @metamask/keyring-controller).
Once the migration is complete and assets-controllers is deprecated and deleted, the confusion will be resolved. In the meantime, we can add a note to the assets-controllers README indicating it's being deprecated and pointing to the new packages.
There was a problem hiding this comment.
@salimtb Okay, that sounds like a great plan. Thanks for clarifying!
mcmire
left a comment
There was a problem hiding this comment.
Left another comment, but it's non-blocking. LGTM!
## Explanation This PR adds comprehensive unit tests for the new `@metamask/assets-controller` package to ensure code quality and maintainability. **Current state:** The assets-controller package was recently introduced with a middleware-based architecture for unified asset management across all blockchain networks. However, test coverage was limited, leaving critical code paths untested. **Changes in this PR:** - Added unit tests for all data sources: - `AccountsApiDataSource` (18 tests, includes timer cleanup fix using `.unref()` to prevent Jest worker process hanging) - `BackendWebsocketDataSource` (19 tests) - `PriceDataSource` (29 tests) - `TokenDataSource` (20 tests) - `SnapDataSource` (47 tests) - Added unit tests for `DetectionMiddleware` (13 tests, 100% coverage) - Expanded unit tests for `AssetsController` (42 tests, coverage increased from 33% to 81%) **Test architecture patterns used:** - `setupController` / `withController` helper functions for consistent test setup - Proper messenger mocking with `MOCK_ANY_NAMESPACE` - Test cleanup to prevent resource leaks - `it.each` for parameterized tests where applicable - Flat test structure avoiding deeply nested describe blocks ## References - Related to #7685 (AssetsController middleware architecture) - Related to #7587 (Initial assets-controller release) ## 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 communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Significantly increases test coverage and validates core behaviors across the new middleware-based assets stack. > > - Add extensive unit tests for `AssetsController`, `DetectionMiddleware`, and data sources: `AbstractDataSource`, `AccountsApiDataSource`, `BackendWebsocketDataSource`, `PriceDataSource`, `RpcDataSource`, and `SnapDataSource` > - Cover subscriptions, middleware chaining, lifecycle/events (app/keyring/network), WebSocket handling, pricing polls, and snap-based balances > - Update `RpcDataSource` to import `BalanceFetcher`, `MulticallClient`, and `TokenDetector` from `evm-rpc-services` (was `rpc-datasource`) > - Update `CHANGELOG.md` to note test additions > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 9b08b0a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Explanation
This PR introduces a new
@metamask/assets-controllerpackage as a placeholder for future consolidation of asset tracking functionality.Current state:
Asset tracking functionality (account balances, token balances, asset detection) is currently spread across multiple controllers in the
@metamask/assets-controllerspackage.Solution:
This PR creates an empty
@metamask/assets-controllerpackage as a foundation for future work to consolidate asset tracking into a single, unified controller. The package currently exports nothing and serves as a placeholder with TODOs indicating the intended scope:No functional changes are included in this PR.
References
Checklist
Note
Introduces a scaffolded
@metamask/assets-controllerpackage (empty controller, exports, tests, build/test/docs configs, changelog, license) as a foundation for future asset tracking consolidation.packages/assets-controllerwithAssetsControllerstub, unit tests, TypeScript/Jest/Typedoc configs, and package metadatatsconfig.json/tsconfig.build.jsonreferences,yarn.lock,.github/CODEOWNERS, andteams.jsonWritten by Cursor Bugbot for commit 65d631f. This will update automatically on new commits. Configure here.