Skip to content

Narrow getStateForAssetSelector inputs: whole-controller spreads over-subscribe selectAssetsBySelectedAccountGroup, re-deriving all assets on asset-irrelevant controller churn #31561

Description

@MajorLift

Performance audit finding · Severity: Sev3 (corrected from High — see below) · Effort: Low–Medium · Fix risk: Low (behavior-preserving input narrowing) · Test safety net: Partial
Owner: @MetaMask/metamask-assets (suggested)
File: app/selectors/assets/assets-list.ts:107,157

What is this about?

Correction to the original framing. This ticket previously claimed the cost was a per-check deep compare of the entire asset state that "scales with power-user data." That mechanism does not hold up under inspection:

  • createDeepEqualSelector uses fast-equals deepEqual (app/selectors/util.ts:30), not lodash. deepEqual short-circuits on reference-equal sub-values (an a[key] === b[key] fast path per key) and bails on the first difference. A fresh wrapper object does not force a full traversal.
  • The bulk slices in the composite — allTokens, tokenBalances, marketData, balances, conversionRates, assetsMetadata, accountsAssets, … — are each produced by a createDeepEqualSelector in assets-migration.ts, so they are reference-stable when unchanged. deepEqual hits === on those keys and never descends into the thousands-of-entries token/balance subtrees.

So the per-check compare is ~O(number of top-level composite keys) pointer comparisons, not O(asset count). The deep-compare cost is not the problem.

The real issue is input over-subscription → unnecessary result recomputation. getStateForAssetSelector (:107) spreads three whole controllers into the composite:

return {
  ...AccountTreeController,   // 5 fields; consumer reads only `accountTree`
  ...AccountsController,      // ≈ internalAccounts (consumed)
  /* … stabilized get* slices: allTokens, tokenBalances, marketData, … … */
  ...NetworkController,       // 3 fields; consumer reads only `networkConfigurationsByChainId`
  accountsByChainId,
};

But the upstream consumer _selectAssetsBySelectedAccountGroup reads a narrow AssetListState (@metamask/assets-controllers, selectors/token-selectors.ts:94) — from those three controllers it uses only accountTree, internalAccounts, and networkConfigurationsByChainId. The spreads drag every other field into the deep-equal'd composite, where any change to an extraneous field flips deepEqual to false and re-runs the full asset derivation — producing a new output reference that cascades to the 15+ dependent selectors and the per-row selectAsset (#31492).

The extraneous fields are exactly the ones that churn on background timers, unrelated to any balance/price change:

  • NetworkController.networksMetadata — network availability / EIP-1559 status polling (per enabled network).
  • NetworkController.selectedNetworkClientId — changes on every network switch.
  • AccountTreeController.isAccountTreeSyncingInProgress — a boolean that toggles on every account-tree sync cycle (≥2 deepEqual=false flips per sync).
  • AccountTreeController.hasAccountTreeSyncingSyncedAtLeastOnce, accountGroupsMetadata, accountWalletsMetadata — sync / metadata churn.

Each of these, while an asset surface is mounted, triggers a full asset re-derivation + a per-row selectAsset cascade across every visible token-list row — even though no token, balance, or price changed.

Component footprint (the cascade blast radius — GitHub code search, two hops from the root):

  • Direct consumers: useTokensWithBalance (Bridge), useTokenAtomicActions (TokenDetails), and the send flow's useAccountTokens/useRouteParams.
  • Via selectSortedAssetsBySelectedAccountGroup: the wallet token list itself (app/components/UI/Tokens/index.tsx — which renders TokenListItem per row, where selectAsset adds the per-row multiplier of Fix 3 per-row parameterized-selector cache misses in TokenListItem (fresh object-literal args) #31492; 26 component-side selectAsset references) and the Homepage tokens section.
  • Via the useTokensWithBalance hook alone: 29 files — Bridge token selection and batch-sell, Card (asset balances, swaps, spending limit), Perps payment tokens, and the leaderboard QuickBuy flow.

Net: roughly 50 unique non-test files across 8 product surfaces (wallet home, homepage, token details, send, bridge, card, perps, quick-buy). On each spurious recompute the token list multiplies the per-row selectAsset re-derivation by every visible row.

Scenario

N/A — see Technical Details.

Design

N/A — internal performance change; no UI/design impact.

Technical Details

Fix direction — narrow the three spreads to the exact consumed fields; leave the already-stabilized get* slices untouched:

const getStateForAssetSelector = (state: RootState) => {
  const { AccountTreeController, AccountsController, NetworkController } =
    state.engine.backgroundState;

  return {
    accountTree: AccountTreeController.accountTree,
    internalAccounts: AccountsController.internalAccounts,
    allTokens: getTokensControllerAllTokens(state),
    allIgnoredTokens: getTokensControllerAllIgnoredTokens(state),
    tokenBalances: getTokenBalancesControllerTokenBalances(state),
    marketData: getTokenRatesControllerMarketData(state),
    assetsMetadata: getMultiChainAssetsControllerAssetsMetadata(state),
    accountsAssets: getMultiChainAssetsControllerAccountsAssets(state),
    allIgnoredAssets: getMultiChainAssetsControllerAllIgnoredAssets(state),
    balances: getMultiChainBalancesControllerBalances(state),
    conversionRates: getMultichainAssetsRatesControllerConversionRates(state),
    currencyRates: getCurrencyRateControllerCurrencyRates(state),
    currentCurrency: getCurrencyRateControllerCurrentCurrency(state),
    networkConfigurationsByChainId:
      NetworkController.networkConfigurationsByChainId,
    accountsByChainId: getAccountTrackerControllerAccountsByChainId(state) as /* … */,
  };
};

After this, the composite's only recompute triggers are genuine asset-relevant changes; networksMetadata / selectedNetworkClientId / account-tree sync-flag churn no longer invalidates it.

Keep createDeepEqualSelector — do not switch to a plain createSelector. The three narrowed fields (accountTree, internalAccounts, networkConfigurationsByChainId) are raw state reads, not deep-equal-stabilized, so a reducer that replaces one with a new-but-deeply-equal reference would force a recompute under a plain selector; deepEqual absorbs that. (This is the opposite of the original ticket's suggestion to drop the deep-equal wrapper, which would regress recompute frequency on these surfaces.)

Verify before narrowing that _selectAllAssets (via selectAllAssetsGrouped) and the Tron path (selectTronSpecialAssetsBySelectedAccountGroup) consume the same AssetListState and read no field beyond it.

Threat Modeling Framework

N/A — performance-only change; behavior is preserved, no new data flow / trust boundary / attack surface.

Acceptance Criteria

  • Dispatching a NetworkController.networksMetadata update (or toggling AccountTreeController.isAccountTreeSyncingInProgress) with token/balance/price data unchanged does not increment selectAssetsBySelectedAccountGroup.recomputations().
  • Recompute count on selectAssetsBySelectedAccountGroup increments ~once per actual asset-state change (token add/remove, balance, price, account-tree structure), not per background poll/sync.
  • Profiler on a power-user profile: token list / asset surfaces stop re-deriving during network-status polling and account-tree sync; re-measure Fix 3 per-row parameterized-selector cache misses in TokenListItem (fresh object-literal args) #31492 after this lands.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    Sev1-highAn issue that may have caused fund loss or access to wallet in the past & may still be ongoingSev2area-performanceIssues relating to slowness of app, cpu usage, and/or blank screens.ta-needs-engineer-escalationTriage-Agent - Applied when confidence is below thresholdta-triagedteam-assets

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    To be fixed

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions