fix: remove ws balance freshness guard#9273
Open
salimtb wants to merge 11 commits into
Open
Conversation
The polling-source freshness lock is unnecessary alongside fetch-then-subscribe and Accounts API cache bypass on forceUpdate.
…iably Improve BackendWebSocketService routing for wildcard channels and stale subscription IDs, harden BackendWebsocketDataSource subscribe/notify flow, and use update-mode force refresh so partial API snapshots do not leave stale token balances in state.
salimtb
changed the title
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. |
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. |
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. |
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
1 participant
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.
Explanation
What is the current state of things and why does it need to change?
Token balances in
@metamask/assets-controllercould stay stale after transactions, account switches, or WebSocket reconnects. Several independent issues contributed:WebSocket notifications were not reaching
BackendWebsocketDataSource— Clients subscribe on wildcard channels (account-activity.v1.eip155:0:0x…) but the server sends notifications on specific chains (…eip155:42161:0x…).BackendWebSocketServicematched channels exactly, so post-confirm balance pushes were silently dropped even though frames appeared in the Network tab.Stale
subscriptionIdafter reconnect/resubscribe — Notifications could arrive with a server subscription id that no longer matched the local map. The subscription callback was skipped and channel fallback was unreliable.Race conditions on account switch — Overlapping subscribe/unsubscribe and fetch work could interleave, dropping notifications or applying responses out of order.
Stale balances from cached/partial fetches —
AccountsApiDataSourceused TanStack Query’s 60s balance cache even onforceUpdate, and force-refresh pipelines used merge semantics that never removed tokens absent from partial API responses (e.g. USDC still shown after a swap when the API only returned ETH).Websocket freshness guard blocked corrections — A 120s guard prevented polling/API updates from overwriting recent WS balances. That helped in some send scenarios but also blocked legitimate corrections (e.g. receiver account showing wrong balance after account switch).
What is the solution your changes offer and how does it work?
@metamask/core-backend—BackendWebSocketServiceeip155:0:0xaddrmatcheseip155:42161:0xaddrfor both subscription callbacks and channel callbacks.subscriptionIdlookup fails, resolve the handler by matching the notification channel against registered subscription channels.channel/subscriptionIdfrom nesteddatawhen the server wraps notifications.@metamask/assets-controller—BackendWebsocketDataSourceaddChannelCallbackper channel as a fallback when subscription-id routing fails.onAssetsUpdatefrom stored subscription state when notifications arrive with stale ids.@metamask/assets-controller—AssetsController#accountRefreshMutexserializes overlapping refresh work.@metamask/assets-controller— balance update modesupdatemode: patches balance amounts only; does not remove tokens or overwrite metadata/prices. Used forgetAssets({ forceUpdate: true })and Accounts API / Snap fetch responses when the API returns a partial snapshot (e.g. only ETH after a swap).mergeremains the default for event-driven updates (WS, polling).fullremains for responses that should be authoritative for a chain scope.AccountsApiDataSource{ staleTime: 0, gcTime: 0 }to the API client whenrequest.forceUpdateis true.End-to-end flow after a confirmed swap:
Note
Medium Risk
Changes core balance merge semantics and live WebSocket routing; user-visible balances are affected, though behavior is covered by expanded unit tests.
Overview
Fixes stale token balances after transactions, account switches, and reconnects by changing how partial balance data is applied and by making WebSocket account-activity delivery reliable.
AssetsControllerdrops the 120-second websocket freshness guard and internalsourceIdplumbing that blocked API/polling from correcting recent WS values. It addsupdateas a balance-only overlay mode: amounts in the response are patched in, tokens missing from partial snapshots are not removed, and existing metadata/prices are not overwritten (only missing metadata/prices are seeded).getAssets({ forceUpdate: true })and the fast/slow fetch pipelines now commit withupdateinstead ofmerge.Balance data sources (
AccountsApi, RPC, Snap, etc.) returnupdateMode: 'update'for fetches.ParallelMiddlewarepropagatesupdate/merge/fullwhen combining parallel responses.BackendWebsocketDataSourceregisters subscription state and handlers before the server subscribe handshake, rolls back on subscribe failure, and only pushes updates when there are actual balances; notifications can use stored handlers when subscription IDs are stale.BackendWebSocketServicematches account-activity channels when subscribe uses chain ref0but notifications use a specific chain, falls back to subscription lookup by channel whensubscriptionIdis stale, and normalizes nested notification payloads before routing.Reviewed by Cursor Bugbot for commit ba933fc. Bugbot is set up for automated code reviews on this repo. Configure here.