Skip to content

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

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

feat(assets-controller): clean and optimize data fetching#7950
Kriys94 wants to merge 3 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 asset fetching/concurrency and changes when tracking starts/stops based on UI/keyring state, which can impact freshness and subscription behavior across chains. Update-mode semantics also affect how balances are cleared/merged, so regressions could surface as missing/stale assets if misclassified by a data source.

Overview
Optimizes asset data fetching and state application. Adds ParallelMiddleware utilities to run balance sources with chain partitioning + fallback and to fetch token metadata and prices concurrently, with concurrency caps via p-limit and merged DataResponses.

Introduces DataResponse.updateMode ('full' | 'merge') and updates AssetsController state updates to respect authoritative vs incremental responses (preserving custom assets on full refresh), while marking appropriate data-source responses and forcing getAssets(..., forceUpdate) to apply as full.

Integrates @metamask/client-controller so tracking/subscriptions only run when both the UI is open and the keyring is unlocked, and updates tests/tooling exports/config to support the new lifecycle and middleware.

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

@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
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 3 potential issues.

[result.accountId]: newBalances,
},
assetsInfo,
updateMode: 'full',
Copy link

Choose a reason for hiding this comment

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

RPC subscription updates wipe balances from other sources

High Severity

Both #handleBalanceUpdate and #handleDetectionUpdate set updateMode: 'full' on their subscription responses. These responses contain data for only a single chain (per BalanceFetchResult.chainId) or only newly detected tokens. When this reaches #updateState in 'full' mode, balances[accountId] = effective replaces the entire account's balances with only what's in the response plus custom assets — wiping out balances from other chains and other data sources (e.g., AccountsAPI, Snap). These incremental subscription updates need 'merge' mode, not 'full'.

Additional Locations (1)

Fix in Cursor Fix in Web

};
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

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.

1 participant

Comments