Skip to content
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

Trade API: next steps #61

Open
1 of 3 tasks
cgewecke opened this issue Jun 9, 2021 · 1 comment
Open
1 of 3 tasks

Trade API: next steps #61

cgewecke opened this issue Jun 9, 2021 · 1 comment

Comments

@cgewecke
Copy link
Contributor

cgewecke commented Jun 9, 2021

⚠️ Actively edited. ⚠️

Several things have been raised by the #60 review that should be addressed in subsequent PRs

  • Service is can easily fail if one of the (many) external resource calls fails
  • Gas price oracles
    • Use fallbacks? e.g gasNow and EthGasStation ?
  • CoinGecko token list: especially bad for polygon which makes 4 external calls to compose a list.
    • (from review) In a follow up, we should make this fault tolerant to CoinGecko API failures, return default values for things like estimated USD value, and return some kind of error that indicates CoinGecko failed.

    • Pricing seems pretty time sensitive... perhaps we can have redundancy via another service?
    • Could also maintain our own token list (for metadata) and generate in a CI chron job / publish to github?
    • fallback on the dynamic generation if that fails?

Resolved in #60

  • Currently formats prices as dollar strings.
  • Should this be configurable, e.g pick your national currency
  • Should formatting be delegated to quote consumer?
@asoong
Copy link
Contributor

asoong commented Jun 9, 2021

Service is can easily fail if one of the (many) external resource calls fails

  • Remove the token list fetching
  • Have generate function pass in decimals for from_token and to_token; see Postman
  • Add new optional param for fetching gas prices; fetch from gas_now
  • For coin prices, call CoinGecko. If entire API call fails, assume $0 for all assets. If any prices are missing, assume $0 price.
  • Don't need to return any of the coin data like Name, Symbol, etc. Coin response can be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants