fix: guard ws by feature flag#9647
Conversation
GitHub Actions
Inactive
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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 5baa62b. Configure here.
| ); | ||
| return response.fullSupport | ||
| .map(decimalToChainId) | ||
| .filter((chainId) => shouldSupportChain(chainId, remoteFeatureFlags)); |
There was a problem hiding this comment.
WebSocket ignores live flag changes
Medium Severity
BackendWebsocketDataSource now filters active chains with shouldSupportChain and RemoteFeatureFlagController:getState, but unlike AccountsApiDataSource it never subscribes to RemoteFeatureFlagController:stateChange. After a migration flag turns off, the websocket source can keep claiming gated chains until the 20-minute refresh or reconnect, so account-activity ingestion may continue when it should not.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5baa62b. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR updates @metamask/assets-controller to normalize supported-network chain identifiers into CAIP-2 ChainIds and to gate which supported networks are surfaced as “active chains” for the backend WebSocket data source based on the Snaps → AssetsController migration remote feature flags.
Changes:
- Added a shared
decimalToChainIdutility to normalize numeric/string chain identifiers into CAIP-2ChainIdvalues (with CAIP pass-through). - Updated
BackendWebsocketDataSourceto filter its supported networks viashouldSupportChain(..., remoteFeatureFlags)before surfacing them as active chains. - Added unit tests for chain-id normalization and for migration-stage gating behavior in the WebSocket data source.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/assets-controller/src/utils/index.ts | Exposes the new CAIP/chain-id normalization helper from the utils entrypoint. |
| packages/assets-controller/src/utils/caip.ts | Adds decimalToChainId helper for converting API “decimal” chain identifiers into CAIP-2. |
| packages/assets-controller/src/utils/caip.test.ts | Adds tests for decimal/string conversion and CAIP pass-through. |
| packages/assets-controller/src/data-sources/BackendWebsocketDataSource.ts | Gates active-chain surfacing using migration feature flags and reuses the shared chain-id normalization helper. |
| packages/assets-controller/src/data-sources/BackendWebsocketDataSource.test.ts | Adds coverage for migration gating behavior and chain-id conversion inputs. |
| packages/assets-controller/src/data-sources/AccountsApiDataSource.ts | Removes duplicated helper by importing the shared decimalToChainId. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { remoteFeatureFlags } = this.#messenger.call( | ||
| 'RemoteFeatureFlagController:getState', | ||
| ); | ||
| return response.fullSupport | ||
| .map(decimalToChainId) |
There was a problem hiding this comment.
it is the same for accounts API
Explanation
References
Checklist
Note
Medium Risk
Changes which chains the websocket data source claims during the Snaps migration rollout; misconfigured flags could hide or prematurely expose Solana/Stellar/Tron balances, but EVM behavior and fail-safe
Offdefaults limit blast radius.Overview
BackendWebsocketDataSource now applies the same Snaps → AssetsController migration gating as AccountsApiDataSource when building active chains from
fetchV2SupportedNetworks. It readsRemoteFeatureFlagController:getStateand filtersfullSupportwithshouldSupportChain, so Solana, Stellar, and Tron are only claimed when their per-network flag stage is at leastReadAssetsControllerWithFallback; missing orOffflags keep those networks on the Snap path. EVM (eip155) chains are unchanged.Chain ID normalization is deduplicated into shared
decimalToChainIdinutils/caip.js(replacing local helpers in Accounts API and websocket code), with unit tests for decimals and pass-through CAIP IDs.AssetsController messenger typing now documents that the remote feature flag action is shared by both data sources. Changelog and websocket tests cover unset flags,
Off, enabled stages, and independent per-namespace gating.Reviewed by Cursor Bugbot for commit 9bb7068. Bugbot is set up for automated code reviews on this repo. Configure here.