Chore: fetch price api supported chain ids #7005
Open
Conversation
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
sahar-fehri
changed the title
|
@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. |
|
@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. |
7 tasks
|
@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. |
sahar-fehri
commented
Nov 4, 2025
|
|
||
| const DEFAULT_INTERVAL = 180000; | ||
|
|
||
| export const DEFAULT_CACHE_REFRESH_THRESHOLD = 1000 * 60 * 60 * 24; // 24 hours |
There was a problem hiding this comment.
I'm caching this supported chains data because its not the kind of data that will change very often.
| }, | ||
| "packages/assets-controllers/src/token-prices-service/codefi-v2.ts": { | ||
| "jsdoc/tag-lines": 2 | ||
| "jsdoc/tag-lines": 1 |
There was a problem hiding this comment.
Nice cleanup!
There's only 1 more warning left on this file... can we also clean it up? If it's difficult happy to leave it.
Prithpal-Sooriya
approved these changes
Nov 5, 2025
bergarces
reviewed
Nov 5, 2025
| }, | ||
| supportedChainIds: { | ||
| includeInStateLogs: false, | ||
| persist: true, |
There was a problem hiding this comment.
What is the benefit of making it part of the state and persisted? The client seems to just need so that it can pass it back to fetch the token rates, which is a round-trip and a synchronisation issue that we could avoid.
Couldn't we just keep it as part of the token prices service state? It can be initialised empty, fetched on the first load and then reused afterwards. That would also mean we don't need to use a cache, it will just get fetched whenever we start the client and remain there until it's restarted.
What do you think?
There was a problem hiding this comment.
Could you elaborate more on "Make it as part of the token prices service state" 🙏 Are you here referencing the codefi-v2.ts service?
There was a problem hiding this comment.
Ok, I've just seen that we actually need the list of chains because we are fetching historical prices directly from the client... Why don't we move that call to the price service then?
Seems like a bit of overkill having to orchestrate new state just for that reason.
There was a problem hiding this comment.
We fetch historical prices and exchangeRates (in some cases) directly from clients.
Again, if we move those calls to the price service; how would it solve the issue? I think the services we have do not have state; usually the controller is what saves the state correct?
Also this was not done only for clients, the polling we have on tokenRatesController (every 3mins) would use the cached value of the supported chainIds instead of making a call to the api every time
There was a problem hiding this comment.
Could you elaborate more on "Make it as part of the token prices service state" 🙏 Are you here referencing the codefi-v2.ts service?
Yes, instead of an exported constant SUPPORTED_CHAIN_IDS, make the supportedChainIds a property of CodefiTokenPricesServiceV2. Gets initialised as undefined or [] at constructor time then, when trying to access it, fetch it if empty and never fetch it again.
Whenever the wallet restarts and the service gets initialised again, it will fetch again only once.
Since it will be part of the the class state, you won't have to pass it to every single time to fetchTokenContractExchangeRates and fetchTokenExchangeRates.
Although on second look, it does look like we instantiate a new CodefiTokenPricesServiceV2 every time we call any of those methods (which is crazy, but I don't know how big a refactor it would be to change it).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Price api already exposes its supported chainIds here https://price.api.cx.metamask.io/v1/supportedNetworks.
Instead of relying on the old static array; this array pulls the supported chainIds from the API directly.
This would make it possible in future to add support for new tokens without making a core relase.
References
Checklist
Note
Replaces the static supported-chain list with a dynamic fetch from the Price API, adds a cached
supportedChainIdsstate toTokenRatesController, and updates APIs/types to use it.codefi-v2):fetchSupportedChainIds()(usesGET /v1/supportedNetworks), return hex chain IDs.SUPPORTED_CHAIN_IDSandvalidateChainIdSupported.Hexchain IDs; keepgetNativeTokenAddress.supportedChainIdscache in state withtimestampandDEFAULT_CACHE_REFRESH_THRESHOLD(24h).supportedChainIds; auto-refresh cache when stale.assetsUtil):fetchTokenContractExchangeRatesnow requiressupportedChainIdsarg and uses it for chain validation.SUPPORTED_CHAIN_IDSfrom public exports.Written by Cursor Bugbot for commit 8315c2e. This will update automatically on new commits. Configure here.