-
Notifications
You must be signed in to change notification settings - Fork 19
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
refactor: use coinmarketcap to generate static fiat estimates #671
Conversation
✅ Deploy Preview for drips-app-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Code looks good 👍
Just a thing I noticed while testing locally:
If you manually refresh the page (I was testing http://localhost:5173/app/projects/github/libgit2/libgit2), you sometimes get the estimation, but some other times you get:
Uncaught (in promise) Error: Token info not found for tracked token
at assert (assert.ts:9:11)
at Object.convert (fiat-estimates.ts:139:3)
at token-amounts-table.svelte:37:49
at Array.forEach (<anonymous>)
at $$self.$$.update (token-amounts-table.svelte:36:13)
at update (index.mjs:1343:12)
at flush (index.mjs:1307:17)
type TokenAddress = string; | ||
type DataProviderTokenId = number; |
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.
Not for this PR: In case you want to take this further, check branded types. 🤓
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.
nice, never heard of this before!
@jtourkos great catch, I think I know why it's happening. Will push a fix shortly |
…claim project flow
It turns out that the Binance WS endpoint isn't accessible globally, so this PR changes our provider for fiat estimates to the Coinmarketcap API.
Important: Adds a new env variable
COINMARKETCAP_API_KEY
. To get one for local development, sign up for a free developer account with Coinmarketcap. To test this PR with mainnet, check out the branch, set .env valuePUBLIC_NETWORK
to1
, and populateCOINMARKETCAP_API_KEY
with your own key.This is largely re-using the existing API surface of the
fiat-estimates
util, in order to keep the door open to bring back real-time (or periodic) price updates in the future.All Coinmarketcap API calls are proxied through our SvelteKit backend. On call to
fiatEstimates.start
, the app fetches a map of ERC-20 token addresses to Coinmarketcap IDs, which is a lot safer than relying on symbols, the uniqueness of which is not guaranteed. On call totrack
, it now fetches the current USD value of the given tokens once. There's currently no logic for updating currently-tracked tokens over time, but we can bring this back in the future while keeping the current API surface.