feat: dYdX CMC Research #557
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #557 +/- ##
==========================================
- Coverage 56.63% 56.54% -0.10%
==========================================
Files 246 246
Lines 12459 12506 +47
==========================================
+ Hits 7056 7071 +15
- Misses 4705 4735 +30
- Partials 698 700 +2 ☔ View full report in Codecov by Sentry. |
davidterpay
requested review from
nivasan1,
aljo242,
Eric-Warehime and
technicallyty
as code owners
| @@ -49,6 +50,7 @@ var ProviderMapping = map[string]string{ | |||
| "Raydium": raydium.Name, | |||
| "UniswapV3-Ethereum": uniswapv3.ProviderNames[constants.ETHEREUM], | |||
| "UniswapV3-Base": uniswapv3.ProviderNames[constants.BASE], | |||
| coinmarketcap.Name: coinmarketcap.Name, | |||
There was a problem hiding this comment.
Needs to be added here since the existing logic inherits this class. Once we migrate to v6 we should move this logic directly into the research provider.
| // iterate over all entries in the research json + unmarshal it's market-params | ||
| resp := dydxtypes.QueryAllMarketParamsResponse{} | ||
| for _, market := range research { | ||
| if market.CMCID < 0 { |
There was a problem hiding this comment.
Does this limit us (even the normal research-json provider) to ignore any market that does not have a cmc-id?
There was a problem hiding this comment.
good catch, should also check that its cmc
| // if the fetcher is only for CoinMarketCap markets, filter out all non-CMC markets | ||
| if f.isCMCOnly { | ||
| for ticker, market := range dydxMainnetMarketMap.Markets { | ||
| market.Ticker.MinProviderCount = 1 |
There was a problem hiding this comment.
why do we need to do this? Is this meant to over-ride all markets that exist on mainnet, but not in research-json?
There was a problem hiding this comment.
this will only over-ride if its meant to be a CMC provider only. so research json would still have the correct min provider count.
| cmcProvider mmtypes.ProviderConfig | ||
| ) | ||
|
|
||
| for _, provider := range market.ProviderConfigs { |
There was a problem hiding this comment.
When would a mainnet market ever have the CMC provider?
There was a problem hiding this comment.
it wouldn't which is why we just cross reference with what is in our constants. the markets being added rn via the frontend should have CMC IDs.
| cmcMarket, ok := marketmaps.CoinMarketCapMarketMap.Markets[ticker] | ||
| if !ok { | ||
| f.logger.Info("did not find CMC market for ticker", zap.String("ticker", ticker)) | ||
| delete(dydxMainnetMarketMap.Markets, ticker) |
There was a problem hiding this comment.
Does this mean we can create invalid market-maps? I.e we remove an adjust by
There was a problem hiding this comment.
technically yes, but all of the important adjust bys are in the constants dir. this is a temp fix till we fully move to marketmap generation.
No description provided.