feat(assets-controller): add getAsset#9521
Conversation
Kriys94
force-pushed
the
feat/WPN-1473-assets-controller-get-asset
branch
3 times, most recently
from
8bdabbe to
6aa280d
Compare
![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 6aa280d. Configure here.
Kriys94
force-pushed
the
feat/WPN-1473-assets-controller-get-asset
branch
from
6aa280d to
2ccbbe5
Compare
| const assets = this.#getAssetsFromState( | ||
| [{ id: accountId }], | ||
| [chainId], | ||
| ['fungible'], | ||
| ); |
There was a problem hiding this comment.
nit - A tad nuts that we are calculating all assets, only to return a single asset here.
This is okay for now, we can raise/improve moving forward.
There was a problem hiding this comment.
Great comment, I've refactored a bit to have #getAssetFromState and #getAssetsFromState is using #getAssetFromState
|
|
||
| #getAssetsFromState( | ||
| accounts: InternalAccount[], | ||
| accounts: Pick<InternalAccount, 'id'>[], |
There was a problem hiding this comment.
Very good! Keep interface minimal!
Easier to test and use.
| }); | ||
| }); | ||
|
|
||
| describe('getAsset', () => { |
There was a problem hiding this comment.
🌶️ this test file is too big, impossible to read!
Thoughts on:
- Making this spec more dry. I think we can leverage test tables (
it.each) - spicer -- separate test for each method, truly isolated, easy to read 😈
There was a problem hiding this comment.
I'm fine keeping this as is... but I'd like to pencil in test cleanup.
There was a problem hiding this comment.
YEah probably the best time will be after some cleaning
Prithpal-Sooriya
left a comment
Prithpal-Sooriya
left a comment
There was a problem hiding this comment.
Approving with comments
Kriys94
force-pushed
the
feat/WPN-1473-assets-controller-get-asset
branch
from
2ccbbe5 to
e3a2e51
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Cursor <cursoragent@cursor.com>
…assignment' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Kriys94
force-pushed
the
feat/WPN-1473-assets-controller-get-asset
branch
from
4e07a57 to
71d02f4
Compare
3 participants
Explanation
WPN-1472 centralizes the Assets domain in
@metamask/assets-controllerso consumers (including Snaps) can read controller-owned balances, metadata, prices, and fiat values instead of tracking them independently. Today the controller exposesgetAssets(bulk, per-account map),getAssetMetadata(metadata only), andgetAssetsPrice, but there is no way to read a single combined asset for a specific account/asset pair.This PR adds a
getAssetread API toAssetsController:getAsset(accountId, assetId): Asset | undefinedreturns the combinedAsset(balance+metadata+price+ computedfiatValue) for a single account/asset pair from current controller state, orundefinedwhen a complete renderable asset is not available.AssetsController:getAssetmessenger action so it can be called through the messenger (e.g. by preinstalled Snaps once WPC-1017 lands).Behavior details:
getAssets(via the private#getAssetsFromState) so the returned shape never drifts:balanceandmetadataare required; a missing price falls back to{ price: 0, lastUpdated: 0 }withfiatValue: 0; hidden/filtered assets resolve toundefined.undefined) on an emptyaccountIdor a malformed CAIP-19assetId, so consumers can distinguish bad requests from absent data.assetId(checksums EVM ERC-20 addresses) before lookup, consistent with how state is keyed.#getAssetsFromState'saccountsparameter was widened toPick<InternalAccount, 'id'>[](it only readsaccount.id) sogetAssetcan reuse it without constructing a full account object.The generated messenger action types (
AssetsControllerGetAssetAction) were regenerated and exported fromindex.ts, and a### Addedchangelog entry was added.References
getAssetread API for Snap consumersChecklist
Note
Low Risk
Additive read-only API with shared composition logic and tests; no fetch or persistence behavior changes beyond the internal refactor of
#getAssetsFromState.Overview
Adds
getAsset(accountId, assetId)and theAssetsController:getAssetmessenger action so callers can read one combinedAsset(balance, metadata, price,fiatValue) from current state without runninggetAssetsor triggering a fetch.getAssetnormalizes CAIP-19 IDs, throws on emptyaccountIdor invalidassetId, and returnsundefinedwhen balance/metadata are missing, the asset is hidden, or othergetAssets-style filters apply; missing price uses zero price/fiat like bulk reads.Bulk state assembly is refactored:
#getAssetFromStatecentralizes composition/filtering,#getAssetsFromStatedelegates per balance key (accounts typed asPick<InternalAccount, 'id'>[]), and messenger registration/exports/changelog/tests cover the new API.Reviewed by Cursor Bugbot for commit 71d02f4. Bugbot is set up for automated code reviews on this repo. Configure here.