Skip to content

fix: network enablement imparity#9542

Open
juanmigdr wants to merge 3 commits into
mainfrom
fix/network-enablement-imparity
Open

fix: network enablement imparity#9542
juanmigdr wants to merge 3 commits into
mainfrom
fix/network-enablement-imparity

Conversation

@juanmigdr

@juanmigdr juanmigdr commented Jul 16, 2026

Copy link
Copy Markdown
Member

Explanation

NetworkEnablementController's default state had Sei Mainnet (0x531) enabled, but NetworkController has no built-in client for 0x531. This created a silent inconsistency: the enablement layer said "Sei is on" while the network layer had no idea what Sei was.

This was discovered during a performance investigation in MetaMask Mobile — Engine.lookupEnabledNetworks() calls findNetworkClientIdByChainId() for each enabled chain, which threw synchronously on 0x531 and silently crashed the entire probe. The full story and the defensive fix for existing users is in the companion PR: MetaMask/metamask-mobile#33379.

This PR removes SeiMainnet from getDefaultNetworkEnablementControllerState so fresh installs start with a consistent state. Sei remains in POPULAR_NETWORKS and can still be added by the user. Persisted state for existing users is unchanged — only the initial default changes.

Removal rather than setting false is intentional: the default map should only list networks that NetworkController actually manages. A false entry would still assert an opinion about a network the controller doesn't know about.

A comment has been added above the default EVM map noting that every entry must have a corresponding built-in client in NetworkController (see DEFAULT_INFURA_NETWORKS).

References

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

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

Low Risk
Small default-state fix for fresh installs; no auth or payment paths, and Sei remains available via popular networks.

Overview
Removes Sei Mainnet (0x531) from the default enabledNetworkMap in getDefaultNetworkEnablementControllerState so new installs no longer mark Sei as enabled when NetworkController has no built-in client for that chain. That mismatch caused consumers (e.g. mobile lookupEnabledNetworks) to throw in findNetworkClientIdByChainId and fail silently.

Sei is not removed from POPULAR_NETWORKS; users can still add it manually. Persisted enablement for existing profiles is unchanged—only the initial default changes.

A comment on the default EVM map documents that each entry must have a corresponding built-in NetworkController client (DEFAULT_INFURA_NETWORKS). Tests and the package changelog are updated to match.

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

@juanmigdr juanmigdr requested a review from a team as a code owner July 16, 2026 14:12
@juanmigdr juanmigdr enabled auto-merge July 16, 2026 17:32
pull Bot pushed a commit to Pe44e/metamask-mobile that referenced this pull request Jul 16, 2026
…metadata + deprecate legacy balance polling) (MetaMask#33379)

## **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:
<img width="500" alt="image"
src="https://github.com/user-attachments/assets/cbf7cbbb-36bb-4d30-aa14-8aa6ad96e580"
/>

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!
<img width="500" alt="image"
src="https://github.com/user-attachments/assets/acb87c4a-ced6-4b7c-9a49-1a587f7577e6"
/>

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. 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 —
meaning `networksMetadata` was never populated, and the probe re-fired
on every subsequent restart. The upstream inconsistency is addressed in
a companion core PR:
[MetaMask/core#9542](MetaMask/core#9542), which
removes Sei from the default-enabled set so fresh installs start with a
consistent state.

This investigation also revealed that the startup probe was never
actually necessary for the banner to work correctly. The banner reads
from `NetworkController.state.networksMetadata`, which gets populated as
a side-effect of normal RPC usage (transactions, balance polls, etc.). A
dedicated on-mount probe is redundant — if no metadata exists yet, the
banner simply has nothing to act on and stays hidden. Once an RPC is
genuinely used and fails, the controller writes the degraded/unavailable
status and the banner reacts to it.

So this PR makes two changes:

**1. Remove `Engine.lookupEnabledNetworks` and its on-mount call in
`useNetworkConnectionBanner`.** The method and its singleton export are
deleted entirely. The banner hook no longer triggers a startup RPC
probe. The network connection banner continues to work correctly — it
reacts to metadata written by `NetworkController` during normal app
usage, rather than probing upfront. This also eliminates the Sei crash
path for existing users whose persisted state already has `0x531: true`.

**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 all users** — both on
first launch and on every subsequent restart.

<!-- mms-check: type=text required=true -->

## **Changelog**

<!-- mms-check: type=changelog required=true blocking=true -->

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

## **Related issues**

<!-- mms-check: type=issue-link required=true -->

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

Related: MetaMask/core#9542

## **Manual testing steps**

<!-- mms-check: type=manual-testing required=true -->

```gherkin
Feature: RPC calls on wallet unlock

  Scenario: No redundant RPC calls on wallet unlock
    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 during normal app usage
```

## **Screenshots/Recordings**

<!-- mms-check: type=screenshot required=true -->

### **Before**


https://github.com/user-attachments/assets/163fd655-c70b-4fbd-8af1-f74742d9e05b

### **After**


https://github.com/user-attachments/assets/95d19f65-3259-439b-8ddc-d7c4d59cc614

## **Pre-merge author checklist**

<!-- mms-check: type=checklist required=true -->

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I've included tests if applicable
- [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

#### 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](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93)
to import wallets with many accounts and tokens
- [ ] I've instrumented key operations with Sentry traces for production
performance metrics
- See [`trace()`](/app/util/trace.ts) for usage and
[`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274)
for an example

For performance guidelines and tooling, see the [Performance
Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers).

## **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.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
const getDefaultNetworkEnablementControllerState =
(): NetworkEnablementControllerState => ({
enabledNetworkMap: {
// Each chain ID here must have a built-in client in NetworkController

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I feel like it shouldn't be on the maintainers of this controller to manually keep the list of enableable networks up to date with NetworkController. Also, at some point in the future we are going to remove the concept of default / built-in networks from NetworkController — since built-in networks will come from ConfigRegistryController — so this comment will fall out of date when that happens. Plus, we're not calling findNetworkClientIdByChainId here, we're calling that in the clients, so the knowledge that we are referring to is somewhere else, and a maintainer would have to know where to look.

Removing Sei is fine here, but I wonder if we could also add an init method that reads NetworkController state and then automatically removes enabled networks that aren't listed. What do you think about that solution?

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