Skip to content

feat(assets-controller): clean and optimize data fetching#7950

Open
Kriys94 wants to merge 5 commits intomainfrom
feature/NewUpdate
Open

feat(assets-controller): clean and optimize data fetching#7950
Kriys94 wants to merge 5 commits intomainfrom
feature/NewUpdate

Conversation

@Kriys94
Copy link
Contributor

@Kriys94 Kriys94 commented Feb 17, 2026

Explanation

What is the current state of things and why does it need to change?

  • Asset data (balances, token list, prices) was previously fetched in a more sequential way and without a clear way to treat responses as "full" (authoritative for scope) vs "merge" (incremental).
  • Asset tracking did not take UI visibility into account, so work could continue when the wallet UI was closed.
  • The method-action-types generator was run per package with a path argument, which was awkward in the monorepo.

What is the solution your changes offer and how does it work?

  1. Parallel middlewares (@metamask/assets-controller)

    • createParallelBalanceMiddleware: Runs balance data sources (Accounts API, Snap, RPC) in parallel with chain partitioning and limited concurrency (p-limit, concurrency 3). Failed chains get a fallback round. Responses are merged via mergeDataResponses.
    • createParallelMiddleware: Runs TokenDataSource and PriceDataSource in parallel for the same request and merges their responses.
      Both use the new DataResponse.updateMode so merged results preserve 'full' when any source uses it.
  2. Full vs merge update modes

    • DataResponse gets an optional updateMode: 'full' | 'merge' (type AssetsUpdateMode).
    • Full: Response is treated as authoritative for its scope; custom assets outside that response are preserved. Used for initial fetch and sources that return a complete picture (e.g. Accounts API, RPC full fetch).
    • Merge: Response is merged into existing state. Used for subscriptions and incremental updates (e.g. PriceDataSource, BackendWebsocket). Default when updateMode is omitted is 'merge'.
  3. ClientController integration

    • @metamask/assets-controller adds a dependency on @metamask/client-controller and subscribes to ClientController:stateChange.
    • Asset tracking runs only when both the UI is open (ClientController) and the keyring is unlocked (KeyringController). It stops when the UI closes or the keyring locks (client + keyring lifecycle).
  4. Method-action-types script

    • scripts/generate-method-action-types.ts no longer takes a per-package path. It discovers all controllers with MESSENGER_EXPOSED_METHODS under packages/. The generate-method-action-types script and tsx devDependency are removed from individual packages (e.g. assets-controller, client-controller, analytics-controller, etc.) so generation is centralized.

Are there any changes whose purpose might not be obvious to those unfamiliar with the domain?

  • mergeDataResponses: Merges multiple DataResponse objects (e.g. from parallel balance sources). If any response has updateMode: 'full', the merged result is 'full'; otherwise it defaults to 'merge'.
  • Chain partitioning in parallel balance middleware: Balance sources are invoked in parallel per chain (and with a fallback round for failures) to improve latency without unbounded concurrency.
  • ClientController dependency: Consumers must have ClientController in the composition and allow ClientController:getState and ClientController:stateChange on the assets-controller messenger; otherwise asset tracking will not run when the UI is open.

If your primary goal was to update one package but you found you had to update another one along the way, why did you do so?

  • The script change in scripts/generate-method-action-types.ts affects the whole monorepo, so all packages that previously had a local generate-method-action-types script and used it with a path were updated to drop that script and the tsx devDependency.
  • assets-controllers CHANGELOG was updated to reflect dependency/version bumps (e.g. network-enablement-controller) that are part of this work.

If you had to upgrade a dependency, why did you do so?

  • @metamask/client-controller: New dependency for assets-controller to gate asset tracking on UI visibility.
  • p-limit: Added to assets-controller to cap concurrency in the parallel balance middleware.
  • Removed tsx from several packages: script execution is centralized; the root or a single package can run the generator.

Integration PR in extension: MetaMask/metamask-extension#40233


References


Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Touches core balance/price aggregation and start/stop lifecycle behavior; concurrency and new updateMode semantics could cause missing/overwritten state if edge cases aren’t covered.

Overview
Optimizes assets data fetching and state application. The AssetsController now runs balance sources in parallel with chain partitioning and a fallback retry, and runs token metadata + price enrichment in parallel via new ParallelMiddleware utilities (with concurrency limits via p-limit).

Adds DataResponse.updateMode ('full' | 'merge') and updates state writes to respect it (initial/force fetch is treated as full while most subscription updates are merge, preserving custom assets during full refreshes). Asset tracking is now gated by both UI-open and keyring-unlocked state via @metamask/client-controller, and tests/data sources were updated accordingly (including early returns when no accounts/chains).

Written by Cursor Bugbot for commit 7e0e148. This will update automatically on new commits. Configure here.

@Kriys94 Kriys94 force-pushed the feature/NewUpdate branch 2 times, most recently from 94338fd to 789b774 Compare February 19, 2026 12:40
@Kriys94 Kriys94 changed the title Feature/new update feat(assets-controller): clean and optimize data fetching Feb 19, 2026
@Kriys94 Kriys94 marked this pull request as ready for review February 19, 2026 15:20
@Kriys94 Kriys94 requested review from a team as code owners February 19, 2026 15:20
};
const fallbackRequests = partitionChainsBySource(
fallbackRequest,
sources,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallback retries the same failed source

Medium Severity

The fallback round calls partitionChainsBySource(fallbackRequest, sources) with the same sources array used in round 1. Since partitionChainsBySource assigns each chain to the first source that supports it, the source that originally failed gets the same chains again in the fallback. The stated intent — letting lower-priority sources try failed chains — is not achieved because the partitioning logic is identical both rounds.

Fix in Cursor Fix in Web

merged.detectedAssets = {
...(merged.detectedAssets ?? {}),
...response.detectedAssets,
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shallow merge of detectedAssets loses per-account arrays

Low Severity

mergeDataResponses uses a shallow object spread for detectedAssets, which replaces per-account arrays rather than concatenating them. If two responses contain detectedAssets for the same accountId, the first response's array is silently discarded. This is inconsistent with how assetsBalance is correctly deep-merged per account (lines 26–35). Since detectedAssets is Record<AccountId, Caip19AssetId[]>, the per-account arrays need to be concatenated (with deduplication), not replaced.

Fix in Cursor Fix in Web

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

*/
#subscribeAssets(): void {
if (this.#selectedAccounts.length === 0) {
if (this.#selectedAccounts.length === 0 || this.#enabledChains.size === 0) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleBasicFunctionalityChange bypasses new UI+keyring lifecycle guard

Low Severity

handleBasicFunctionalityChange calls #stop() then #subscribeAssets() directly, bypassing the new #updateActive() guard that requires both #uiOpen and #keyringUnlocked to be true. This creates subscriptions even when the controller should be inactive according to the new combined lifecycle model. It could call #updateActive() instead to respect the new invariant.

Additional Locations (1)

Fix in Cursor Fix in Web

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments