-
-
Notifications
You must be signed in to change notification settings - Fork 264
feat: migrate old token metadata endpoints to new caip-19 endpoints #7572
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
base: main
Are you sure you want to change the base?
feat: migrate old token metadata endpoints to new caip-19 endpoints #7572
Conversation
- Introduced support for Real World Assets (RWA) in token fetching and metadata retrieval. - Updated `fetchTokenListByChainId` to handle pagination, limiting to 10 pages to optimize performance. - Refactored token metadata fetching to include RWA data and improved error handling. - Added mock data for testing RWA tokens and updated related tests to ensure coverage. This update enhances the token service's capabilities and improves the overall efficiency of token data retrieval.
| } from './assetsUtil'; | ||
|
|
||
| export const TOKEN_END_POINT_API = 'https://token.api.cx.metamask.io'; | ||
| export const TOKENS_END_POINT_API = 'https://tokens.dev-api.cx.metamask.io'; |
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.
TODO - update to prod endpoint. includeRwaData has been deployed.
| * @returns The tokens URL. | ||
| */ | ||
| function getTokensURL(chainId: Hex): string { | ||
| function getTokensURL(chainId: Hex, nextCursor?: string): string { |
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.
API change
- https://token.api.cx.metamask.io/tokens/1?...
+ https://tokens.dev-api.cx.metamask.io/tokens?...&includeRwaData=trueUnderlying structure is mostly similar but did need to modify the query parameters & now have to handle pagination.
| queryParams.append('includeMetadata', 'true'); | ||
| queryParams.append('includeRwaData', 'true'); | ||
|
|
||
| return `${TOKENS_END_POINT_API}/v3/assets?assetIds=${assetId}&${queryParams.toString()}`; |
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.
Endpoint change:
- https://token.api.cx.metamask.io/tokens/1?address=...
+ https://tokens.dev-api.cx.metamask.io/v3/assets?assetIds=...Note the response structure has changed a bit, and needs tweaking to be compatible with controllers.
- CAIP19 AssetID -> Hex Asset Address
- Missing IconUrl -> generate v1/v2 image paths
- Missing occurrances -> polyfill this based on number of aggregators.
| queryParams.append('includeOccurrences', 'true'); | ||
| queryParams.append('includeIconUrl', 'true'); | ||
| queryParams.append('includeRwaData', 'true'); | ||
| queryParams.append('first', '3000'); |
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've set each page to fetch 3000 items. Mostly to reduce the number of requests to "get all token metadata"
| // TODO: We really need to move away from fetching all tokens at once | ||
| // This is expensive - uses up a lot of memory and bandwidth | ||
| // Need to discuss how we can fully deprecate this - many areas require this metadata (decimals, icon, rwaData) | ||
| const allTokens: GetTokensUrlResponse['data'] = []; | ||
| let nextCursor: string | undefined; | ||
|
|
||
| // If we are still fetching tokens past 10 pages of 3000 tokens (30000), | ||
| // then we really need to re-evaluate our approach | ||
| const hardPaginationLimit = 10; | ||
| let paginationCount = 1; |
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.
Added TODO comments - will need to discuss this @MetaMask/metamask-assets
It is pretty crazy we have to fetch all token metadata - super expensive!
Maybe we can fetch metadata on the fly? Update and store in a 1d cache?
- But the difficulty is that many areas in the app expect this to be synchronous 😅
| elm.aggregators.length >= 3, | ||
| ); | ||
|
|
||
| // Ensure result is typed with GetTokensUrlResponse and handles pagination |
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.
TODO - remove comment
| ticker?: string; | ||
| instrumentType?: string; | ||
| }; | ||
| // Special filter logic for linea-mainnet (preserved from original) |
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.
Todo remove comment
| } from './assetsUtil'; | ||
|
|
||
| export const TOKEN_END_POINT_API = 'https://token.api.cx.metamask.io'; | ||
| export const TOKENS_END_POINT_API = 'https://tokens.dev-api.cx.metamask.io'; |
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.
Dev API endpoint used instead of production endpoint
High Severity
The new TOKENS_END_POINT_API constant is set to https://tokens.dev-api.cx.metamask.io which contains dev-api in the URL, indicating this is a development environment endpoint. The existing production endpoint pattern is https://token.api.cx.metamask.io (without "dev-api"). This appears to be a development configuration that was accidentally left in the code and needs to be updated to the production endpoint before merging.
| console.warn( | ||
| `TokenService: Token list pagination limit reached for chainId ${chainId}`, | ||
| ); | ||
| return allTokens; |
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.
Linea mainnet filter bypassed when pagination limit triggers
Medium Severity
When the pagination limit condition is met (paginationCount >= hardPaginationLimit), the function returns early at line 288, completely bypassing the Linea-mainnet specific filter logic at lines 291-298. This filter removes tokens that don't meet aggregator criteria for Linea. If Linea mainnet ever triggers the pagination limit, unfiltered tokens would be returned, potentially including invalid or low-quality tokens that shouldn't appear for that network.
Additional Locations (1)
| paginationCount += 1; | ||
| } while (nextCursor && paginationCount <= hardPaginationLimit); | ||
|
|
||
| if (paginationCount >= hardPaginationLimit) { |
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.
Pagination warning fires incorrectly for nine pages
Low Severity
The condition paginationCount >= hardPaginationLimit incorrectly triggers when exactly 9 pages are fetched naturally. After 9 iterations, paginationCount becomes 10. If page 9 was the last page (hasNextPage=false), the loop exits due to nextCursor being undefined, but the check 10 >= 10 still evaluates to true, causing the warning to fire even though no limit was actually hit. The condition needs to use > instead of >= to only fire when the limit is actually reached.
|
|
||
| nextCursor = typedResult.pageInfo.hasNextPage | ||
| ? typedResult.pageInfo.endCursor | ||
| : undefined; |
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.
Missing validation for pageInfo before accessing it
Medium Severity
The validation at lines 267-272 only verifies that data exists and is an array, but does not check for pageInfo existence. The code then accesses typedResult.pageInfo.hasNextPage at line 277. If the API returns a response with data but without pageInfo (e.g., due to an API version mismatch or partial response), this would throw a runtime error when trying to access hasNextPage on undefined. The validation needs to also check that pageInfo exists before the type assertion and property access.
…-update-token-metadata-to-include-rwadata
|
Postponing this work until RWA feature is shipped. This is to reduce the number of changes and to prevent unintentionally breaking stuff. |
|
Demo on extension with tokens that have RWA data. |
Explanation
This migrates the endpoints used for storing token metadata to newer caip-19 endpoints to support
includeRwaDataflag.NOTE - the underlying controller/s is still EVM and does not use CAIP-19 asset IDs.
fetchTokenListByChainIdto handle pagination, limiting to 10 pages to optimize performance.This update enhances the token service's capabilities and improves the overall efficiency of token data retrieval.
References
Checklist
Note
Introduces CAIP-19-based token APIs and surfaces Real World Asset (RWA) data across controllers, while adding pagination and more robust fetch behavior.
TOKENS_END_POINT_APIwith CAIP-19v3/assets; builds URLs via CAIP utils and addsincludeRwaDataTokenRwaDatatyping and propagatesrwaDatathroughfetchTokenMetadata,TokenListToken, and token search display datafetchTokenMetadatato map CAIP response to EVM fields, generateiconUrl, computeoccurrences, and handle unsupported networksfetchTokenListByChainIdto paginate (max 10 pages), return[]on abort/error/timeout, and preserve Linea-specific filteringTokenListControllerto map fields explicitly, accept optionaliconUrl, and handle empty results; includesrwaDataTokensController.testto mockfetchTokenMetadatathrowingWritten by Cursor Bugbot for commit 6b094f9. This will update automatically on new commits. Configure here.