Skip to content

perf: fix redundant RPC calls on wallet unlock (cache network health metadata + deprecate legacy balance polling)#33379

Open
juanmigdr wants to merge 11 commits into
mainfrom
chore/remove-30-plus-api-calls-on-wallet-unlock
Open

perf: fix redundant RPC calls on wallet unlock (cache network health metadata + deprecate legacy balance polling)#33379
juanmigdr wants to merge 11 commits into
mainfrom
chore/remove-30-plus-api-calls-on-wallet-unlock

Conversation

@juanmigdr

@juanmigdr juanmigdr commented Jul 15, 2026

Copy link
Copy Markdown
Member

Description

I recently started profiling MetaMask on mid-range Android devices and noticed something that surprised me - the wallet was making over 100 API calls the moment it unlocked. I decided to dig into it and figure out which of those were actually necessary.

Image of the 120+ API calls made on app unlock:
image

After tracing each call to its origin, the ones that stood out the most were the RPC calls - specifically because they were firing on every single wallet unlock, for every enabled network, before the user had done anything.

By default MetaMask ships with 8 enabled EVM popular networks. On unlock, the app was making 3 RPC calls per network, so a minimum of 24 RPC calls just to open the wallet. If you've added all popular networks that number jumps to 45.

Image of the 32 RPC API calls on a wallet of a user with 0 balance (new user). This happens on every wallet unlock!
image

Here's what each of those three calls was doing:

  • eth_blockNumber - called by AccountTrackerController's RPC balance fetcher before making a Multicall3 call. It needs a block reference to pass to eth_call.
  • eth_call to Multicall3 (0xca11bde...) - called by AccountTrackerController to fetch the user's native ETH balance on chains not covered by the Accounts API v4, using a batched aggregate3 call.
  • eth_blockNumber + eth_getBlockByNumber - called by Engine.lookupEnabledNetworks(), which was being triggered on every wallet mount by the network connection banner hook to probe whether each RPC endpoint was healthy.

The thing is, MetaMask has improved a lot in the past year. We now have the Accounts API v4 which fetches balances for all major EVM networks centrally, and the new unified AssetsController takes over balance fetching entirely when the assetsUnifyState flag is on. The old AccountTrackerController was still running its unlock handler and hitting RPC on chains the Accounts API should have covered, because isDeprecated was never wired to the feature flag.

For the network probe side, investigation uncovered a latent bug that was silently causing lookupEnabledNetworks to crash on every app start, then repeatedly re-probe all networks once that crash was unintentionally "fixed" by an unrelated user action (adding a custom RPC). The root cause: NetworkEnablementController had Sei (0x531) enabled by default in its initial state, but NetworkController had no network client configured for 0x531. Calling findNetworkClientIdByChainId('0x531') threw synchronously and killed the entire probe function before any network could be inspected. Separately, even once that crash was bypassed, lookupEnabledNetworks had no logic to skip networks whose health metadata was already persisted — so it would blindly re-probe every enabled network on every subsequent restart.

The upstream inconsistency (Sei enabled by default in NetworkEnablementController but absent from NetworkController) is addressed in a companion core PR: MetaMask/core#9542. That PR removes Sei from the default-enabled set so fresh installs start with a consistent state. The defensive try/catch added here in Engine.lookupEnabledNetworks is still necessary to protect existing users whose persisted state already has 0x531: true.

So this PR makes two changes:

1. Fix Engine.lookupEnabledNetworks to be robust and skip redundant probes. The original .map().filter() chain let a single bad chain (like 0x531) crash the whole function. Replacing it with a .flatMap() + try/catch gracefully skips any chain whose findNetworkClientIdByChainId throws, letting all other networks be probed correctly. Additionally, a skip guard checks NetworkController.state.networksMetadata: if a network already has a known (non-Unknown) status and its EIP-1559 support has been established, it is excluded from the probe. This means the eth_blockNumber + eth_getBlockByNumber pair only fires for a given network on its first cold start — subsequent restarts skip it entirely because the metadata is persisted.

2. Wire AccountTrackerController.isDeprecated to the assetsUnifyState flag. When the controller is listed in the flag's deprecatedControllers array, it short-circuits all its logic on unlock and makes zero RPC calls. The AssetsController handles balances from that point. This removes the eth_blockNumber + eth_call (Multicall3) pair per network on every unlock.

The end result: 0 RPC calls on wallet unlock for returning users (metadata already cached), and only a one-time probe per network on first launch for new users.

Known onboarding performance impact and future work: Before this fix, the Sei crash meant lookupEnabledNetworks never completed successfully — so networksMetadata was never written and no RPC probes were ever made at startup, even on new installs. This PR fixes that crash, which is the correct behavior, but it means new users will now see eth_blockNumber + eth_getBlockByNumber fired once per enabled network on their very first launch (2 calls × 8 networks = 16 calls). On every subsequent launch those networks are already cached and skipped. A follow-up PR will remove the Engine.lookupEnabledNetworks() call from useNetworkConnectionBanner on mount entirely, since the banner does not need a proactive startup probe — it can rely on NetworkController's reactive rpcEndpointChainUnavailable event when the RPC is actually used. That change will bring first-launch RPC traffic to zero as well.

Changelog

CHANGELOG entry: [performance] Eliminate redundant RPC calls on wallet unlock by caching network health metadata and deprecating legacy balance polling when the unified assets controller is active

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3622

Related: MetaMask/core#9542

Manual testing steps

Feature: RPC calls on wallet unlock

  Scenario: No redundant RPC calls for returning users
    Given a user has previously unlocked the wallet at least once
    And the assetsUnifyState flag has AccountTrackerController in deprecatedControllers

    When the user unlocks the wallet
    Then no eth_blockNumber, eth_getBlockByNumber, or eth_call (Multicall3) RPC calls are made
    And the network connection banner does not appear unless an RPC is genuinely unreachable

  Scenario: One-time probe on first cold start
    Given a fresh install with no persisted networksMetadata
    And at least one enabled EVM network has no prior health metadata

    When the user unlocks the wallet for the first time
    Then eth_blockNumber and eth_getBlockByNumber are called once per uncached network
    And on the next unlock those networks are skipped (metadata is already known)

  Scenario: Sei (0x531) does not crash the probe
    Given NetworkEnablementController has 0x531 enabled
    And NetworkController has no client configured for 0x531

    When Engine.lookupEnabledNetworks runs
    Then the probe for 0x531 is silently skipped
    And all other enabled networks are probed normally

Screenshots/Recordings

Before

Screen.Recording.2026-07-16.at.10.08.21.mov

After

Screen.Recording.2026-07-16.at.10.17.28.mov

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Touches unlock-time network probing and balance-fetch gating; behavior changes are flag-driven but affect core wallet startup and RPC usage across enabled chains.

Overview
Cuts unlock-time RPC traffic by changing how enabled networks are probed and by letting the unified assets path retire legacy balance polling.

Engine.lookupEnabledNetworks now resolves client IDs with flatMap + try/catch so one bad enabled chain (e.g. Sei 0x531 with no client) no longer aborts the whole run. It also skips lookupNetwork when persisted networksMetadata already has a non-Unknown status and EIP-1559 is known, so health probes are not repeated every unlock.

AccountTrackerController init passes isDeprecated from selectIsControllerDeprecated('AccountTrackerController') on Redux state, so when assetsUnifyState lists that controller as deprecated, unlock no longer triggers its Multicall/eth_blockNumber RPC path.

Tests cover throw-skipping, metadata skip behavior, and isDeprecated wiring.

Reviewed by Cursor Bugbot for commit 09a3a63. Bugbot is set up for automated code reviews on this repo. Configure here.

@juanmigdr juanmigdr added the area-performance Issues relating to slowness of app, cpu usage, and/or blank screens. label Jul 15, 2026
@sonarqubecloud

Copy link
Copy Markdown

@juanmigdr juanmigdr changed the title chore: [performance] remove 30+ unnecessary RPC API calls on wallet unlock chore: remove 30+ unnecessary RPC API calls on wallet unlock Jul 16, 2026
Base automatically changed from chore/support-deprecating-each-controller to main July 16, 2026 08:25
@juanmigdr juanmigdr requested a review from a team as a code owner July 16, 2026 08:25
@metamask-ci

metamask-ci Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@github-actions github-actions Bot added size-M risk:medium AI analysis: medium risk and removed size-S labels Jul 16, 2026
Comment thread app/selectors/featureFlagController/assetsUnifyState/index.ts
@juanmigdr juanmigdr enabled auto-merge July 16, 2026 08:37
@github-actions github-actions Bot added risk:high AI analysis: high risk and removed risk:medium AI analysis: medium risk labels Jul 16, 2026
…hub.com:MetaMask/metamask-mobile into chore/remove-30-plus-api-calls-on-wallet-unlock
@juanmigdr juanmigdr requested a review from a team as a code owner July 16, 2026 12:21
@github-actions github-actions Bot removed the size-M label Jul 16, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 94d764d. Configure here.

Comment thread app/core/Engine/Engine.ts
return alreadyKnown ? [] : [id];
} catch {
return [];
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

EIPS access skips network lookup

Low Severity

In lookupEnabledNetworks, alreadyKnown uses m?.EIPS[1559] without optional chaining on EIPS. When metadata has a non-Unknown status but no EIPS object, that expression throws, the flatMap catch returns an empty list, and lookupNetwork is never called for that client—so connectivity for that network is not refreshed.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 94d764d. Configure here.

@juanmigdr juanmigdr changed the title chore: remove 30+ unnecessary RPC API calls on wallet unlock perf: fix redundant RPC calls on wallet unlock (cache network health metadata + deprecate legacy balance polling) Jul 16, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/core/Engine/controllers/account-tracker-controller-init.test.ts 0/125 0/157 0/370

AI-detected flaky patterns

app/core/Engine/controllers/account-tracker-controller-init.test.ts

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • Tests override mock implementations via .mockReturnValue() on selectors (lines ~88, ~104, ~118, ~137). beforeEach only clears call counts; it does not reset implementations set by mockReturnValue. Under randomized test order this shared mock state (J3) can cause intermittent failures. No other J1-J10 patterns matched (no timers, no waitFor, no act(), no spyOn, no mutable lets, no sleeps, no live Date/Math.random). History showed 0 failures so hint not used.
    • Suggested fix in app/core/Engine/controllers/account-tracker-controller-init.test.ts:
      -  beforeEach(() => {
      -    jest.clearAllMocks();
      -  });
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +  });
      +
      +  afterEach(() => {
      +    jest.resetAllMocks();
      +  });

This check is informational only and does not block merging.

@github-actions

Copy link
Copy Markdown
Contributor

⚡ Performance Test Results

ℹ️ Performance test results are currently non-blocking and will not block this PR.

1 test failed · 7 tests · 1 device

📱 Devices tested (1)

Android: Google Pixel 8 Pro (v14.0)

❌ Failed Tests (1)

@metamask-mobile-platform

Test Platform Device Reason Recording
Cold Start after importing a wallet Android Google Pixel 8 Pro (v14.0) Quality gates exceeded 📹 Watch
✅ Passed Tests (6)
Test Platform Device Duration Team Recording
Cold Start: Measure ColdStart To Login Screen Android Google Pixel 8 Pro (v14.0) 300.42s @metamask-mobile-platform
Aggregated Balance Loading Time, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 300.39s @assets-dev-team
Asset View, SRP 1 + SRP 2 + SRP 3 Android Google Pixel 8 Pro (v14.0) 300.33s @assets-dev-team
Measure Warm Start: Warm Start to Login Screen Android Google Pixel 8 Pro (v14.0) 1.23s @metamask-mobile-platform 📹 Watch
Measure Warm Start: Login To Wallet Screen Android Google Pixel 8 Pro (v14.0) 2.39s @metamask-mobile-platform 📹 Watch
Measure Cold Start To Onboarding Screen Android Google Pixel 8 Pro (v14.0) 3.88s @metamask-mobile-platform 📹 Watch

Branch: chore/remove-30-plus-api-calls-on-wallet-unlock · Build: Normal · Commit: 74a80b8 · View full run

@github-actions github-actions Bot added size-M and removed size-S labels Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmations, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeSwap, SmokeStake, SmokeWalletPlatform, SmokeMoney, SmokePerps, SmokeMultiChainAPI, SmokePredictions, SmokeSeedlessOnboarding, SmokeBrowser, SmokeSnaps
  • Selected Performance tags: @PerformanceLaunch, @PerformanceAssetLoading
  • Risk Level: high
  • AI Confidence: 100%
click to see 🤖 AI reasoning details

E2E Test Selection:
Hard rule (global-infrastructure-change): Global infrastructure changed: app/core/Engine/Engine.ts. Running all tests.

Performance Test Selection:
Two performance-relevant changes: (1) The lookupEnabledNetworks() optimization skips redundant network probes at startup, which could improve cold/warm launch times measured by @PerformanceLaunch. (2) The AccountTrackerController isDeprecated flag could alter how account balances are fetched and loaded, directly impacting @PerformanceAssetLoading which covers token list rendering, balance fetching, and portfolio value calculation. Both changes are in the Engine core initialization path which runs at app startup.

View GitHub Actions results

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

Labels

area-performance Issues relating to slowness of app, cpu usage, and/or blank screens. risk:high AI analysis: high risk size-M team-assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant