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

Fiat api refactor #503

Merged
merged 2 commits into from
Sep 2, 2024
Merged

Fiat api refactor #503

merged 2 commits into from
Sep 2, 2024

Conversation

danimoh
Copy link
Member

@danimoh danimoh commented Jul 12, 2024

  • Switch back to the public CoinGecko API as we shut down our custom API endpoint. The public API is accessible from the keyguard domain with no issues and the usage limits are sufficient for the Keyguard's needs.
  • Update the list of currencies supported by CoinGecko.
  • Refactor FiatApi and translations mentioning the provider, to possibly support other providers in the future. Initially, I thought this would be more work, as I thought the implementation would be close to the one in FiatApi in @nimiq/utils, with dynamic providers, currency support depending on the chosen provider, type generics for the chosen provider etc. However, in the Keyguard, it can be quite a bit easier, as the provider can be considered a fixed value. Based on that value (which currently is only CoinGecko), I'm simply setting, for example, the appropriate list of supported currencies and the api endpoint, also as fixed values. Typescript seems to be happy with this. Maybe a more modern typescript version might complain, or if a second provider is actually added, but in that case all code and types for the inactive provider can simply be commented out to circumvent such issues.
  • This PR is probably easier reviewed with whitespace changes ignored.

danimoh added 2 commits July 11, 2024 19:47
…list

Switch back to the public CoinGecko API as we shut down our custom API endpoint. The public
API is accessible from the keyguard domain with no issues and the usage limits are sufficient
for the Keyguard's needs.

Also update the list of currencies supported by CoinGecko.
@danimoh danimoh requested review from sisou and mraveux July 12, 2024 19:12
@danimoh danimoh self-assigned this Jul 12, 2024
src/lib/FiatApi.js Outdated Show resolved Hide resolved
src/lib/FiatApi.js Outdated Show resolved Hide resolved
Copy link
Member

@mraveux mraveux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simpler than I would've expected, but clean nonetheless.

@danimoh danimoh force-pushed the daniel/fiat-api-refactor branch from dcbd088 to 583af46 Compare August 16, 2024 21:04
@danimoh danimoh requested a review from sisou August 16, 2024 21:11
@danimoh danimoh merged commit 583af46 into master Sep 2, 2024
2 checks passed
@danimoh danimoh deleted the daniel/fiat-api-refactor branch September 2, 2024 21:32
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

Successfully merging this pull request may close these issues.

3 participants