-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add CoinMarketCap external API #2971
base: main
Are you sure you want to change the base?
Conversation
// name: String, | ||
// symbol: String, | ||
// slug: String, | ||
// is_active: u8, // TODO: This field is available, should we at least emit a warning if the listing is not marked as active? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we care to do "liveness" checks (i.e. could emit a warning if the API says that the token is inactive)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point - lets absolutely do it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Thank you for bringing CMC API over and brushing up wiring. While we are here - would it make sense to bring the tests over too?
Also, is CMC client repeating the error discovered by @cytadela8 (with flipped numerator / denominator)?
What ❔
Adds support for CoinMarketCap as a price API for base token prices.
Why ❔
Currently, the only supported API is CoinGecko. It's not ideal to only support a single 3rd party API here, so this provides some more degrees of freedom, and makes base token less reliant on a single third-party provider.
Checklist
zk fmt
andzk lint
.