-
-
Notifications
You must be signed in to change notification settings - Fork 267
feat: use dynamic supported networks from Price API #7716
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@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. |
| // This ensures the list stays fresh during normal polling | ||
| // Note: fetchSupportedNetworks handles errors internally and always resolves | ||
| // eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
| fetchSupportedNetworks(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont understand why we are calling the fetchSupportedNetworks function here. That function is not storing anything in state so why would we need to call it here if we are not making use of the response at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetchSupportedNetworks() updates a module-level cache (lastFetchedSupportedNetworks). The background call keeps this cache fresh so that subsequent calls to getSupportedChainIdsV3AsHex() return up-to-date data.
This is the same pattern used for fetchSupportedCurrencies() in fetchExchangeRates().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh missed that, thanks! Why did you change it to be blocking instead of non-blocking now?
| * | ||
| * @returns The supported networks response. | ||
| */ | ||
| export async function fetchSupportedNetworks(): Promise<SupportedNetworksResponse> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could find the call in core-backend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
packages/assets-controllers/src/token-prices-service/codefi-v2.ts
Outdated
Show resolved
Hide resolved
packages/assets-controllers/src/token-prices-service/codefi-v2.ts
Outdated
Show resolved
Hide resolved
4324802 to
863782e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we finally rename codefi-v2 😄
We can do this later.
Explanation
Current State
The
CodefiTokenPricesServiceV2inassets-controllersuses a hardcoded list (SPOT_PRICES_SUPPORT_INFO) to determine which blockchain networks are supported by the Price API's/v3/spot-pricesendpoint. This approach has several drawbacks:Additionally, the native asset CAIP-19 identifiers (e.g.,
eip155:1/slip44:60for ETH) were also hardcoded, duplicating data that already exists inNetworkEnablementController.state.nativeAssetIdentifiers.Solution
This PR introduces dynamic fetching of supported networks from the Price API's
/v2/supportedNetworksendpoint:fetchSupportedNetworks()function that fetches from/v2/supportedNetworksand caches the responsefetchTokenPrices()callsetNativeAssetIdentifiers()method to receive native asset mappings fromNetworkEnablementController, eliminating duplicationnativeAssetIdentifiersfromNetworkEnablementControllerto the token prices serviceChanges Summary
codefi-v2.ts: Added dynamic network fetching, caching, and native asset identifier supportabstract-token-prices-service.ts: AddedNativeAssetIdentifiersMaptype and optionalsetNativeAssetIdentifiers()method to interfaceTokenRatesController.ts: Integrated withNetworkEnablementControllerto pass native asset identifiers@metamask/network-enablement-controlleras a dependencyReferences
https://price.api.cx.metamask.io/v2/supportedNetworksNetworkEnablementController.state.nativeAssetIdentifiersfor CAIP-19 native token mappingsChecklist
Note
Introduces dynamic network support and native asset identifier integration for token pricing.
fetchSupportedNetworks(),getSupportedNetworks(), andresetSupportedNetworksCache()in token-prices-service (fallback to hardcoded list; background refresh duringfetchTokenPrices)CodefiTokenPricesServiceV2to:setNativeAssetIdentifiers()and use CAIP-19 IDs for native tokensupdateSupportedNetworks()TokenRatesControllernow callsNetworkEnablementController:getStateat init to passnativeAssetIdentifiersto the token prices serviceNativeAssetIdentifiersMap; add@metamask/network-enablement-controllerdependency and TS references; expand tests for new behaviorWritten by Cursor Bugbot for commit 863782e. This will update automatically on new commits. Configure here.