feat(AssetController): add hiddenAssets state #7777
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| this.state.hiddenAssets[accountId]?.includes(normalizedAssetId) ?? false | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
getAssets does not filter out hidden assets
High Severity
The hideAsset method's JSDoc states "Hidden assets are excluded from the asset list returned by getAssets", but #getAssetsFromState doesn't actually filter hidden assets. The method iterates through accountBalances and filters by chain, metadata, and asset type, but never checks this.state.hiddenAssets[account.id] to skip hidden assets. Users who hide assets will still see them returned by getAssets(), completely defeating the feature's purpose.
Additional Locations (1)
| return ( | ||
| this.state.hiddenAssets[accountId]?.includes(normalizedAssetId) ?? false | ||
| ); | ||
| } |
There was a problem hiding this comment.
Method isAssetHidden is defined but never used
Low Severity
The isAssetHidden() method is defined but never used anywhere in the codebase. Unlike the other hidden asset methods (hideAsset, unhideAsset, getHiddenAssets), this method has no corresponding action type defined and is not registered as a messenger action handler. It cannot be called via the messenger pattern and is never called internally. This appears to be dead code that was added but never integrated.
There was a problem hiding this comment.
not a huge deal, we can add some tests to cover this.
| if (hiddenAssets.has(assetId)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Hmm does this mean we don't fetch and update balances for hidden assets?
Might be problematic for the swaps flow (you could be trying to swap a hidden asset, where the balance is outdated?)
High level, my initial thoughts are that "Hidden Assets" are a UI lever/control, we should still be performing data fetching if there are places that do show this asset.
| allIgnoredTokens: Record<string, Record<string, string[]>>; | ||
| assetsMetadata: Record<Caip19AssetId, AssetMetadata>; | ||
| assetsBalance: Record<string, Record<string, { amount: string }>>; | ||
| hiddenAssets: Record<string, string[]>; |
There was a problem hiding this comment.
nit (not a blocker) - I wonder if we add the JSDOC like we did for packages/assets-controller/src/data-sources/evm-rpc-services/types/state.ts, it helped readability on what this Record<string, string[]> is.
There was a problem hiding this comment.
(optional, needs discussion), the Accounts team export a type alias, that could assist with readability
export type AccountId = string;...Hesitant since it doesn't actually provide any additional type safety... maybe JSDOC is better?
There was a problem hiding this comment.
Actually I actually much more prefer the approach you used below in packages/assets-controller/src/AssetsController.ts
type Foo = { [accountId: string]: string[] };I think this is easier to read/understand then Record<string, string[]>
There was a problem hiding this comment.
Can we add some tests on the new exposed methods from this controller?
Prithpal-Sooriya
left a comment
Prithpal-Sooriya
left a comment
There was a problem hiding this comment.
Initial pass looks good. Lemme know when this PR is out of draft 😄
Explanation
This PR adds support for hidden assets in the
AssetsController, similar to howallIgnoredAssetsworks inMultichainAssetsController.Current State:
Users currently have no way to hide assets they don't want to see or track in the new AssetsController architecture. The
MultichainAssetsControlleralready has this capability viaallIgnoredAssets, but the new unifiedAssetsControllerwas missing this feature.Solution:
Added a
hiddenAssetsfield to theAssetsControllerstate that stores hidden assets per account (CAIP-19 asset IDs). This allows users to hide assets they don't want to track and/or see.Changes:
hiddenAssets: { [accountId: string]: string[] }toAssetsControllerStatehideAsset(),unhideAsset(),getHiddenAssets(), andisAssetHidden()methodsAssetsControllerHideAssetAction,AssetsControllerUnhideAssetAction,AssetsControllerGetHiddenAssetsAction) and registered handlersaddCustomAsset(), it's automatically unhidden if it was previously hidden (matching the behavior ofMultichainAssetsController.addAssets())RpcDataSourceandBalanceFetcherto filter out hidden assets from balance polling, so hidden assets won't be tracked or updatedType Updates:
hiddenAssetstoAssetsControllerStatehiddenAssetstoAssetsControllerStateInternalhiddenAssetstoAssetsBalanceStatein evm-rpc-services typesReferences
allIgnoredAssetsinMultichainAssetsControllerChecklist
Note
Medium Risk
Medium risk: introduces new persisted controller state and new messenger actions that change balance polling behavior by filtering out hidden assets, which could impact what assets are tracked/displayed if misapplied.
Overview
Adds a persisted
hiddenAssetsmap toAssetsControllerstate and exports new messenger actions tohideAsset,unhideAsset, andgetHiddenAssets(plus anisAssetHiddenhelper).Updates
addCustomAssetto automatically unhide an asset when it’s re-added, wireshiddenAssetsthroughRpcDataSource→BalanceFetcher, and filters hidden asset IDs out of token balance polling. Tests and shared types are updated to include the new state shape.Written by Cursor Bugbot for commit ea074cb. This will update automatically on new commits. Configure here.