fix: add BackendWebsocketDataSource tests for Arbitrum USDC balance update - #9265
Merged
Conversation
salimtb
force-pushed
the
fix/ws-balance-stale-after-reconnect
branch
from
June 25, 2026 09:38
f6eaa4f to
39a67eb
Compare
salimtb
force-pushed
the
fix/ws-balance-stale-after-reconnect
branch
2 times, most recently
from
June 25, 2026 09:55
1cdc5c2 to
ca17234
Compare
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Prithpal-Sooriya
previously approved these changes
Jun 25, 2026
salimtb
force-pushed
the
fix/ws-balance-stale-after-reconnect
branch
from
June 25, 2026 14:24
432b1cd to
4e4e438
Compare
Contributor
Author
|
@metamaskbot publish-preview |
Prithpal-Sooriya
approved these changes
Jun 25, 2026
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
4 tasks
pull Bot
pushed a commit
to dmrazzy/core
that referenced
this pull request
Jul 23, 2026
…equest (MetaMask#9591) ## Explanation Current behavior: <img width="1917" height="631" alt="image" src="https://github.com/user-attachments/assets/a9eebaab-1ff2-48c4-b788-2ca169c7826e" /> After PR: <img width="1917" height="631" alt="image" src="https://github.com/user-attachments/assets/7f27a1cf-ccae-43d2-b9d2-70e41d3d0c8a" /> Balance refreshes triggered on-demand go through `AssetsController.getAssets(..., { forceUpdate: true })`, which flows down to `AccountsApiDataSource.fetch` as TanStack Query `fetchOptions`. Previously the `forceUpdate` path used `{ staleTime: 0, gcTime: 0 }`. These values has been added via MetaMask#9265 but it was mainly to fix a websocket issue. That was too aggressive: - `staleTime: 0` marks the query as stale immediately, so **every** forced refresh refetches — even two identical requests fired microseconds apart. - `gcTime: 0` evicts the cache entry the instant the query goes inactive (which, for imperative `fetchQuery`, is right after it resolves). Because the forced fetch uses the **same queryKey** as the 30s polling subscription, it not only bypasses the cache but also tears down the entry the poll would have reused. In practice almost every internal trigger calls `getAssets` with `forceUpdate: true` (unlock, account switch, network change, tx confirmation, price refresh, etc.), and several of these fire near-simultaneously. With `0/0` each trigger produced its own Accounts API request, causing bursts of duplicate `multiaccount/balances` calls. This PR changes the `forceUpdate` path to `{ staleTime: 200, gcTime: 200 }`. The small 200ms window lets a burst of near-simultaneous forced refreshes de-duplicate into a single Accounts API request while keeping the data effectively fresh (well within a user-perceptible refresh). Keeping `gcTime >= staleTime` also ensures the entry survives its freshness window instead of being evicted immediately. ## References N/A ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Narrow change to query cache options for forced balance fetches; balances may share a single response within ~100ms, which is an intentional tradeoff for fewer API calls. > > **Overview** > **Forced balance refreshes** (`getAssets` with `forceUpdate: true` → `AccountsApiDataSource.fetch`) no longer pass TanStack Query `{ staleTime: 0, gcTime: 0 }`. They now use **`staleTime` and `gcTime` of 100ms**, so near-simultaneous triggers (unlock, account/network switch, tx confirmation, etc.) **collapse to one** `multiaccount/balances` request instead of a burst of duplicates, while still avoiding the default long-lived cache. > > The unit test and changelog are updated to match the new short-lived cache window (replacing the “fully bypass cache” expectation from [MetaMask#9265](MetaMask#9265)). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1659296. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes stale or missing token balances when websocket live updates compete with Accounts API / RPC polling, and when websocket subscriptions are torn down and recreated during account switches or reconnects.
Manual verification (Arbitrum USDC):
eip155:42161/erc20:…) notification processed correctly after send/receiveExplanation
What was broken?
AssetsControllermerges balances from several sources: websocket push (BackendWebsocketDataSource), Accounts API polling (AccountsApiDataSource), RPC, Snap, etc. Several races caused the UI to show stale values or never recover after a transaction:staleTime), or polling could “win” right after a websocket update.subscriptionIdcould be dropped when the server-side subscription ID changed after reconnect; there was no per-channel callback fallback.forceUpdatestill stale —getAssets({ forceUpdate: true })did not bypass the Accounts API TanStack cache, and websocket freshness guards could block the forced fetch from applying (e.g. receiver switches to account 2 after a send but state still shows the pre-receive balance).What does this PR do?
AssetsControllersourceIdonDataResponse.BackendWebsocketDataSourcebalance push, marks thoseaccountId:assetIdentries fresh for 120s; passive polling sources (AccountsApiDataSource,RpcDataSource,SnapDataSource,StakedBalanceDataSource) cannot overwrite them during that window.getAssets({ forceUpdate: true })is taggedgetAssets:forceUpdate, clears freshness locks for applied balances, and is not subject to the websocket freshness filter — authoritative for account switch / manual refresh.forceUpdatefetch first, then subscribe, so API balances land before websocket recovery.#handleAccountGroupChanged).AccountsApiDataSourcerequest.forceUpdateis true, passes{ staleTime: 0, gcTime: 0 }tofetchV5MultiAccountBalancesso forced refreshes bypass the TanStack cache.BackendWebsocketDataSourcesubscriptionIdrouting is unreliable after reconnect.DataRequestwith balance updates.Non-obvious details
sourceIdis internal — added toDataResponsefor merge policy only; not a new public messenger action.forceUpdate.BackendWebSocketService:addChannelCallbackis not delegated in the client messenger, the primary websocket subscription path still works; callbacks are a fallback for reconnect routing mismatches.Scope
All changes are in
@metamask/assets-controlleronly. No dependency upgrades and no breaking public API changes.Test plan
yarn workspace @metamask/assets-controller test— new/updated unit tests passyarn workspace @metamask/assets-controller run changelog:validateforceUpdateshows correct balanceNew unit test coverage:
AssetsController: polling does not overwrite recent websocket balance;getAssetsforceUpdatewins over websocket freshnessAccountsApiDataSource:forceUpdatebypasses TanStack cacheBackendWebsocketDataSource: concurrent subscribe serialization, checksummed vs lowercase EVM addresses, channel-callback fallback, disconnect/reconnect chain reclaimReferences
Checklist
Note
Medium Risk
Changes how displayed balances are merged across websocket and polling paths and reorders fetch/subscribe on account changes; incorrect freshness or locking could show wrong balances until the next
forceUpdatefetch.Note
Medium Risk
Changes how displayed balances are merged across websocket and polling paths and reorders fetch/subscribe on account changes; incorrect freshness or locking could show wrong balances until the next forceUpdate fetch.
Overview
Fixes stale or flickering token balances when websocket pushes compete with Accounts API / RPC polling, and when subscriptions are recreated on account switch or reconnect.
AssetsControllertags applied updates with internalsourceIdonDataResponse. After websocket balance pushes, per-asset freshness locks block passive polling sources for 120s;getAssets({ forceUpdate: true })is taggedgetAssets:forceUpdate, skips that filter, and clears locks for applied balances. Account group and account-tree refreshes use a mutex and run force fetch before subscribe; account-tree updates with no overlapping account IDs are ignored.AccountsApiDataSourcepasses{ staleTime: 0, gcTime: 0 }to multi-account balance fetches whenforceUpdateis set.BackendWebsocketDataSourceserializes subscribe/unsubscribe, treats EVM addresses as unchanged when only checksumming differs, registers optional per-channel callbacks for reconnect routing, and passes the originalDataRequestwith balance updates.Reviewed by Cursor Bugbot for commit 773e4e8. Bugbot is set up for automated code reviews on this repo. Configure here.