Amm rankings explorer page#1295
Conversation
|
Semgrep found 10
Risk: Affected versions of axios are vulnerable to Improper Check for Unusual or Exceptional Conditions. It fails to correctly validate configuration keys during merging. This allows a crafted proto property to trigger an internal TypeError, causing the application to crash. Fix: Upgrade this library to at least version 1.13.5 at explorer/package-lock.json:27214. Reference(s): GHSA-43fc-jf86-j433, CVE-2026-25639 |
| * GET /api/v1/amms | ||
| * Fetch top AMMs with sorting | ||
| */ | ||
| const getAMMs = async (req, res) => { |
There was a problem hiding this comment.
You should cache these results so that Explorer server will not make a call to LOS on every page load (rate limiting)
See this example: https://github.com/ripple/explorer/blob/main/server/routes/v1/tokens.js#L85-L119
There was a problem hiding this comment.
| log.info(`Fetched ${response.data.results?.length || 0} AMMs`) | ||
|
|
||
| return res.status(200).json(response.data) | ||
| } catch (error) { |
There was a problem hiding this comment.
Also with caching if the call failed we can still use stale data
There was a problem hiding this comment.
| * Fetch aggregated AMM statistics | ||
| * This fetches the special "aggregated" document from the amms index | ||
| */ | ||
| const getAggregatedStats = async (req, res) => { |
There was a problem hiding this comment.
Same here, this should be cached
There was a problem hiding this comment.
| * Fetch historical trends for AMM data | ||
| */ | ||
| const getHistoricalTrends = async (req, res) => { | ||
| try { |
There was a problem hiding this comment.
| api.use('/metrics', getCurrentMetrics) | ||
| api.use('/tokens/search/:query', getTokensSearch) | ||
| api.post('/tokens/batch-get', batchGetTokens) | ||
| api.use('/tokens/:tokenId', getTokenById) |
There was a problem hiding this comment.
Is there a reason for this to be done server-side? For single token fetching without the need for caching I would prefer doing it on client-side to avoid rate limiting (IP of user browser vs IP of Explorer server)
There was a problem hiding this comment.
Changed to cache the data serverside instead of fetching per request here https://github.com/ripple/explorer/pull/1295/changes#diff-3d739c4ee7d382a3089fc92fe71742ee94b21e2d701aa157f072f31446d89fd7R236-R250
f1682aa to
ec81759
Compare
b6341b2 to
c030e5b
Compare
| ['ammRankings', sortField, sortOrder], | ||
| () => fetchAMMRankings(sortField, sortOrder), | ||
| { | ||
| refetchInterval: 60 * 1000, |
There was a problem hiding this comment.
nit: you can set a common refetch interval variable
| @@ -911,5 +911,15 @@ | |||
| "asset_2": "Asset 2", | |||
There was a problem hiding this comment.
Once you’ve finalized all the new keys for the AMM Ranking page, please add the same keys to translations.json for the other languages and set their values to null (you can refer to the AMM Object PR for examples).
| <span>{CATEGORY_LABELS[cat]}</span> | ||
| </button> | ||
| ))} | ||
| <div className="search-wrapper"> |
There was a problem hiding this comment.
Sorry, I meant the search box "Search AMMs" in the Top 1,000 AMMs section. I don't see it in the screenshot.
| @@ -2,6 +2,7 @@ | |||
| "action": "acció", | |||
There was a problem hiding this comment.
This comment applies to all the non en-US translation.json files. We introduce the following 11 new translation keys in the en-US/translation.json file:
"asset_pair"
"amms"
"top_1000_amms"
"general_info"
"number_of_amms"
"number_of_lps"
"number_of_amms_tooltip"
"number_of_lps_tooltip"
"search_amms"
"tvl_tooltip"
"volume_24h_all_tooltip"
What I expect are
- They will show up in other non en-US files
- No other changes are needed, but I'm seeing lots of other key addition/removals as well. Is it a result of running some sort of formatter? If that's the case, just make sure we don't accidentally delete/add any used/unused keys, then we are good.
There was a problem hiding this comment.
It is a formaI did use a formatter. I just confirmed that the changes made in the file are all cosmetic OR part of the list you gave above. Those 11 translation keys are the only additive change between this branch and your branch
| ? formatCurrencyAmount(getFees24h(amm)) | ||
| : DEFAULT_EMPTY_VALUE} | ||
| </td> | ||
| <td className="apr"> |
There was a problem hiding this comment.
The APR value doesn't need to be bold or semi-bold.
e8b65ba to
e09b907
Compare
This reverts commit c7df640.
…g, adding to translations.json
8f44fb4 to
b04fbe0
Compare
…, data_available_from_notice
High Level Overview of Change
These changes add an AMM page to the explorer.
Context of Change
The new AMMs page includes:
Type of Change