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 2 commits 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
There was a problem hiding this comment.
As I thought through this, I realized we can remove this block entirely if we require chain and tokenAddress query params for all tokens. Just pushed up a commit to do that.
| 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
msalcala11
force-pushed
the
more-rates
branch
3 times, most recently
from
1129b7a to
9e835f4
Compare
msalcala11
force-pushed
the
more-rates
branch
from
9e835f4 to
930cb2d
Compare
2 participants
Description
This PR refactors CoinGecko resolution for marketstats and v4/fiatrates so BWS can support all CoinGecko-listed tokens across BitPay-supported chains via deterministic chain + tokenAddress lookup, while preserving DB-cached defaults for the Exchange Rates view.
Changelog
Testing Notes