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

Prices + CoinGecko integration #81

Merged
merged 2 commits into from
Feb 22, 2023
Merged

Prices + CoinGecko integration #81

merged 2 commits into from
Feb 22, 2023

Conversation

JSKitty
Copy link
Member

@JSKitty JSKitty commented Feb 13, 2023

Abstract

A prices system has been added to MPW, made to support multiple price providers in the future, as well as with existing multi-currency support (~61 currencies) and persistent user preference, defaulting to USD otherwise.

Some trivial cleanup was also done, along with linting and prettier being ran, catching a few out-of-scope files, better sooner than later!

image
image
image

What does this PR address?

This PR finishes the "display currency" functionality for the Dashboard UI, as well as making it easy in the future to display "conversion currencies" in future screens, like seeing your Stakes in USD or BTC, or having a "Send USD" option, which sends the equivalent of PIVX your favored currency.

What features or improvements were added?

A modular price system, along with multi-currency support and persistent user settings for each currency.

How does this benefit users?

Users can now view their PIVX in equivalent display currencies, and customise their currency out of a large selection from CoinGecko in the MPW settings, which is persisted across sessions.

A prices system has been added to MPW, made to support multiple price providers in the future, as well as with existing multi-currency support and persistent user preference, defaulting to USD otherwise.

Some trivial cleanup was also done, along with linting and prettier being ran, catching a few out-of-scope files, better sooner than later.
@JSKitty JSKitty added the Enhancement New feature or request label Feb 13, 2023
@JSKitty JSKitty self-assigned this Feb 13, 2023
Liquid369
Liquid369 previously approved these changes Feb 13, 2023
Copy link

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

tACK 99989be
Tested, works fine, looks great.
Screenshot 2023-02-13 at 1 48 33 PM
PIVX Price: .338106 at time of writing
Math checks out lol

nit: global.js new lines 408-415
my eyessss

@JSKitty
Copy link
Member Author

JSKitty commented Feb 13, 2023

ESLint/Prettier did not take kindly to global.js 408-415 😂

@JSKitty JSKitty requested a review from BreadJS February 13, 2023 20:28
Copy link
Member

@Duddino Duddino left a comment

Choose a reason for hiding this comment

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

Other than these two notes, I'm really liking this

* CoinGecko's endpoint for PIVX data, optimised for least bandwidth
* - No localisation, tickers, community data, developer data or sparklines
*/
export const COINGECKO_ENDPOINT =
Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep this in chain_params, and if it's null for the current chain we can either remove the price or set it to 0, this way testnet coins don't display mainnet prices

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there will ever be an instance where non-mainnet has a price (so "alternative" price tracking or per-network prices wouldn't really be useful).

But for consistency, I think keeping the prices as-is on all networks is fine, so tPIV would still show a value to keep the UI consistent for testing purposes, thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I'm still not convinced about this, tPIV should be valued at 0$, but it seems like i'm alone on this, since it's a small detail I think we can merge

Copy link

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

tACK 7a2b047

I think it should show pricing for testing purposes even though tPIV has no value, it's helpful than using the entire main net to cross check.

Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

tACK

Copy link
Member

@Duddino Duddino left a comment

Choose a reason for hiding this comment

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

tACK

@JSKitty JSKitty merged commit 8f502c9 into PIVX-Labs:master Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

5 participants