refactor: consolidate MarketAPI sources and add currencies #130
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates the MarketAPI implementation by reducing the number of data sources from 9 to 4 (Kraken, CoinGecko, Yadio, Exir) and introduces an automatic fallback system. The changes migrate 26 existing currencies to appropriate sources, add 17 new currency entries, and remove unused code for deprecated sources.
Key Changes:
- Implemented automatic fallback mechanism (Kraken/Yadio → CoinGecko) to improve data retrieval reliability
- Removed 5 unused data sources and their associated error types
- Added 17 new currency configurations to expand geographic coverage
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| rates/src/lib.rs | Removed 5 source variants and error types, implemented fallback logic with new fallback() method, refactored price fetching to support fallback chain |
| assets/fiatUnits.json | Migrated 26 currencies from deprecated sources to Kraken/CoinGecko, added 17 new currency entries with CoinGecko as source |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| match Self::parse_price_json(&text, source, key) { | ||
| Ok(parsed) => Ok(Some(parsed)), | ||
| Err(_) => Ok(None), // Parsing failed, treat as source failure for fallback |
There was a problem hiding this comment.
The error from parsing is being silently discarded. Consider logging the parse error before returning None to aid debugging when fallback occurs.
| Err(_) => Ok(None), // Parsing failed, treat as source failure for fallback | |
| Err(e) => { | |
| log::warn!( | |
| "Failed to parse price JSON from source {} with key {}: {}", | |
| source, | |
| key, | |
| e | |
| ); | |
| Ok(None) // Parsing failed, treat as source failure for fallback | |
| } |
| async fn fetch_price_with_source( | ||
| &self, | ||
| source: &Source, | ||
| key: &str, | ||
| ) -> Result<Option<String>, RatesError> { |
There was a problem hiding this comment.
This new function lacks documentation. Add a doc comment explaining its purpose, parameters, return value, and the None return case for parse failures.
| } | ||
|
|
||
| impl Source { | ||
| /// Returns the next fallback source in the hierarchy, if any. |
There was a problem hiding this comment.
Add documentation for the fallback method explaining the fallback hierarchy and when None is returned (i.e., for sources with no fallback).
| /// Returns the next fallback source in the hierarchy, if any. | |
| /// Returns the next fallback source in the hierarchy, if any. | |
| /// | |
| /// # Fallback Hierarchy | |
| /// - `Kraken` and `Yadio` fallback to `CoinGecko`. | |
| /// - `Exir` and `CoinGecko` have no fallback and return `None`. | |
| /// | |
| /// # Returns | |
| /// - `Some(Source)` if a fallback source exists. | |
| /// - `None` for sources with no fallback. |
- Reduce data sources from 9 to 4 (Kraken, CoinGecko, Yadio, Exir) - Add automatic fallback system (Kraken/Yadio → CoinGecko) - Add 14 new currencies (VND, PKR, BDT, MMK, EGP, MAD, DZD, JOD, PEN, CRC, DOP, GTQ, BOB, BGN, GEL, KZT, BYN) - Remove 3 unsupported currencies (AMD, HRK, MZN) - Migrate currencies to appropriate sources based on API support - Remove unused Source variants and error types Final distribution: Kraken (7), CoinGecko (36), Yadio (31), Exir (2) Closes #129
Okazakee
force-pushed
the
feat/marketapi-rework
branch
from
5c45208 to
e9720d7
Compare
Closes #129