Market data: Poloniex to Coingecko - #475
Conversation
| * BTC_XEM ticker this replaces (highestBid, baseVolume, percentChange | ||
| * as a fraction) so existing consumers don't need to change. | ||
| * | ||
| * Cached for CACHE_TTL_MS to stay within CoinGecko's public API rate |
There was a problem hiding this comment.
I think it would be acceptable to remove the cache if it is included only for rate-limit protection.
CoinGecko’s keyless Public API uses IP-based rate limiting, and these requests are made directly from each user’s wallet, so users generally do not share one central server quota. Market data is also requested only during connection or manual refresh.
| @@ -0,0 +1,80 @@ | |||
| /** @module utils/marketData */ | |||
There was a problem hiding this comment.
I think marketData.js would be more suitable under the services directory as src/app/services/marketData.service.js, because it performs HTTP requests.
it can then be registered in services/index.js and injected into DataBridge and MarketDataCtrl instead of being imported directly.
|
Great work! I’ve left a few comments on the PR. Please don’t forget to add tests as well. 🙏🏼 |
|
Thanks for the review. |
| return response.json(); | ||
| }).then((json) => { | ||
| let nemData = json && json.nem; | ||
| if (!nemData || typeof nemData.btc !== 'number') { |
There was a problem hiding this comment.
I recommend you check another field as well.
Those fields might return null.
typeof nemData.btc_24h_vol !== 'number' ||
typeof nemData.btc_24h_change !== 'number'
| done(); | ||
| }); | ||
|
|
||
| it('should fall back to the last successfully fetched data when a later request fails', async (done) => { |
There was a problem hiding this comment.
This unit test failed.
You might check again.
| done(); | ||
| }); | ||
|
|
||
| it('should always request fresh data instead of short-circuiting on a previous value', async (done) => { |
There was a problem hiding this comment.
This unit test failed.
You might check again.
| * On fetch failure, falls back to the last successfully fetched value | ||
| * (even if stale), rather than erroring out, if one is available. | ||
| * | ||
| * @return {Promise} - Resolves with a {highestBid, baseVolume, percentChange} object |
There was a problem hiding this comment.
You can make the return type more explicit by documenting the resolved object shape:
/**
- @returns {Promise<{
- highestBid: number,
- baseVolume: number,
- percentChange: number
- }>} Market data shaped like the former Poloniex response.
*/
|
Thanks for the review. |
2 participants
No description provided.