Support all coingecko coins in marketstats and v4 fiatrates endpoints#4110
Support all coingecko coins in marketstats and v4 fiatrates endpoints#4110msalcala11 wants to merge 1 commit intobitpay:masterfrom
Conversation
msalcala11
force-pushed
the
more-rates
branch
from
dcf0f8a to
c75f865
Compare
msalcala11
force-pushed
the
more-rates
branch
from
c75f865 to
4ac4ad3
Compare
|
|
||
| let ids: string[]; | ||
| let secondary: string | undefined; | ||
| for (const id of candidates.slice(0, 10)) { |
There was a problem hiding this comment.
each iteration makes a blocking /v3/coins/{id} request. if the goal is to get the first instance for secondary consider using Promise.all/Promise.allSettled here and picking the first match from the results
| API_KEY: config.coinGecko.apiKey, | ||
| }; | ||
| if (httpStatusCode === 429 || coinGeckoErrorCode === 429) { | ||
| const rateLimitErr: any = new Error('coinGecko rate limit'); |
There was a problem hiding this comment.
i think its worth it to keep the rate limit logging
logger.warn('CoinGecko rate limit: %o', { url, httpStatusCode, coinGeckoErrorCode, body });
| } | ||
| const hasCoinGeckoError = !!(status?.error_code || status?.error_message); | ||
| if ((httpStatusCode && httpStatusCode >= 400) || hasCoinGeckoError) { | ||
| const cgErr: any = new Error(status?.error_message || body?.status || 'coinGecko error'); |
There was a problem hiding this comment.
same thing here with maintaining original logging. and do we want to keep the body field in the error like before?
cgErr.body = body;
| try { | ||
| chainParam = getQueryString(req.query, 'chain'); | ||
| } catch { | ||
| chainParam = undefined; |
There was a problem hiding this comment.
is there a reason to not throw an error here?
| try { | ||
| chainParam = getQueryString(req.query, 'chain'); | ||
| } catch { | ||
| chainParam = undefined; |
There was a problem hiding this comment.
same here. undefined might lead to silent errors
2 participants
Description
This PR expands CoinGecko support for
/v1/marketstats/:code/and/v4/fiatrates/:code/so that rates and stats can also be returned for non-default coins. It adds symbol resolution via CoinGecko /search with /coins/list fallback, chain-aware disambiguation, and optional tokenAddress contract lookup for unambiguous token resolution.Changelog
Testing Notes
Manual URL response checks:
Default market stats response (default DB-cached set): http://localhost:3232/bws/api/v1/marketstats/USD
Default fiat rates response (default DB-cached set): http://localhost:3232/bws/api/v4/fiatrates/USD
Non-default symbol resolution via search: http://localhost:3232/bws/api/v4/fiatrates/USD?coin=WBTC
/coins/list fallback case: http://localhost:3232/bws/api/v4/fiatrates/USD?coin=ABC
Token-address resolution for market stats: http://localhost:3232/bws/api/v1/marketstats/USD?coin=USDC.e&chain=arb&tokenAddress=0xff970a61a04b1ca14834a43f5de4533ebddb5cc8
Token-address resolution for fiat rates: http://localhost:3232/bws/api/v4/fiatrates/USD?coin=USDC.e&chain=arb&tokenAddress=0xff970a61a04b1ca14834a43f5de4533ebddb5cc8
Missing chain with tokenAddress should error: http://localhost:3232/bws/api/v1/marketstats/USD?coin=USDC.e&tokenAddress=0xff970a61a04b1ca14834a43f5de4533ebddb5cc8
Unknown tokenAddress on valid chain should return Unsupported tokenAddress: http://localhost:3232/bws/api/v1/marketstats/USD?coin=USDC.e&chain=arb&tokenAddress=0x0000000000000000000000000000000000000001
Cache behavior to verify: