fix(AssetsController): Remove messenger in Data sources#7859
Conversation
Kriys94
force-pushed
the
fix/AssetsControllerCleanMessenger
branch
4 times, most recently
from
438667d to
ef1c35d
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| this.#dataSources.set('BackendWebsocketDataSource', new Set()); | ||
| this.#dataSources.set('AccountsApiDataSource', new Set()); | ||
| this.#dataSources.set('SnapDataSource', new Set()); | ||
| this.#dataSources.set('RpcDataSource', new Set()); |
There was a problem hiding this comment.
Data sources instantiated before disabled check causes side effects
High Severity
All data sources (BackendWebsocketDataSource, AccountsApiDataSource, SnapDataSource, RpcDataSource, TokenDataSource, PriceDataSource, DetectionMiddleware) are instantiated at lines 428–457, before the #isEnabled check at line 464. Their constructors have significant side effects: AccountsApiDataSource and BackendWebsocketDataSource fire off API calls (fetchV2SupportedNetworks) and start 20-minute setInterval refresh timers; RpcDataSource subscribes to NetworkController:stateChange and reads NetworkController:getState; SnapDataSource discovers snaps and subscribes to balance events. When isEnabled is false, the early return at line 466 skips initialization but all these resources are already running and never cleaned up.
| .catch(console.error); | ||
| for (const subscription of this.activeSubscriptions.values()) { | ||
| subscription.onAssetsUpdate(response)?.catch(console.error); | ||
| } |
There was a problem hiding this comment.
Snap balance events silently dropped before controller subscribes
Medium Severity
SnapDataSource subscribes to AccountsController:accountBalancesUpdated in its constructor (line 240), but #handleSnapBalancesUpdated now delivers updates by iterating this.activeSubscriptions and calling each onAssetsUpdate callback. Subscriptions are only created later when AssetsController.#start() calls #subscribeToDataSources() (triggered by KeyringController:unlock). Any snap balance events arriving between construction and subscription are silently dropped because activeSubscriptions is empty. The old messenger-based approach called AssetsController:assetsUpdate directly, which always reached the controller regardless of subscription state. The same pattern exists in RpcDataSource.#handleBalanceUpdate.
Additional Locations (1)
Kriys94
force-pushed
the
fix/AssetsControllerCleanMessenger
branch
from
ef1c35d to
429bf06
Compare
| this.#backendWebsocketDataSource.setActiveChainsFromAccountsApi( | ||
| activeChains, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Wrong condition prevents BackendWebsocketDataSource chain sync
Medium Severity
The comment says "When BackendWebsocketDataSource is updated via AccountsApiDataSource callback, sync its state," but the condition checks dataSourceId === 'BackendWebsocketDataSource'. This means setActiveChainsFromAccountsApi is called when BackendWebsocketDataSource reports its own chains (a no-op, since they're already set internally), but is never called when AccountsApiDataSource reports chains. If the intent is to sync BackendWebsocketDataSource from AccountsApiDataSource, the condition needs to check for 'AccountsApiDataSource' instead.
Additional Locations (1)
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
|
changes test on the UI through the PR below: |
2 participants
Explanation
References
Checklist
Note
Medium Risk
Touches core asset fetching/subscription plumbing and introduces a new required constructor dependency (
queryApiClient), which can break consumers or subtly change subscription behavior. Logic changes around request shaping and WebSocket channel construction may affect which updates are received.Overview
BREAKING: Removes the external
initDataSources/messenger-driven data source setup and makesAssetsControllerrequire aqueryApiClient, instantiating all balance/price/metadata data sources and middleware internally.Data sources are refactored to stop exposing messenger actions/events; instead the controller calls instance methods/middlewares directly and subscriptions push updates via
SubscriptionRequest.onAssetsUpdate(with optionalgetAssetsStatefor price polling). Lifecycle wiring is simplified to keyring lock/unlock only, request building is centralized viaaccountsWithSupportedChains, and WebSocket subscriptions are updated to always includeeip155+solanachannels with namespace-appropriate address formatting.Tests/docs are updated to match the new construction/subscription model, and controller action types are moved to an auto-generated
AssetsController-method-action-types.ts.Written by Cursor Bugbot for commit 429bf06. This will update automatically on new commits. Configure here.