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

Extends exchange rates #166

Merged
merged 10 commits into from
Oct 20, 2021
Merged

Extends exchange rates #166

merged 10 commits into from
Oct 20, 2021

Conversation

gpgpublickey
Copy link

@gpgpublickey gpgpublickey commented Oct 17, 2021

Extends exchange rates with more APIs than Kraken

What?:

Adds a new Exchange Rate for Argentinian pounds (ARS).
Adds local time in the amount value display.
Adds more info in the README and useful scripts to the package.json

Why?:

-Kraken doesn't include rate conversion for most countries from South America, Argentina for instance.
-It is more customizable to use a simple rate conversion from local currency to USD and then based on the BCHUSD exchange calculate the final exchange rate.
-In many 3rd world countries users checks continuously the time of the exchange rate
-It was hard to understand how to run the local environment from scratch.

Where?:

-These changes affect the value-display general component to show the last refresh local time of the exchange rate.
-These changes only apply when the --no-rates option is disabled

How looks?:

image
image

Cristian Giagante added 2 commits October 17, 2021 07:01
Adds local time zone near of the value-display
Adds a script to run the local environment with parameteres
app/coins/bch.js Show resolved Hide resolved
app/utils.js Outdated Show resolved Hide resolved
app/utils.js Show resolved Hide resolved
@sickpig
Copy link
Owner

sickpig commented Oct 18, 2021

@mryo1997 thanks so much for the PR!

I did brief review, please have a look at my comment.

One thing I'd like to explorer further is about the exchange rate.

Most of the time there's no perfect match between a given exchange rate pair value and the value you get by using the BCHUSD pair exchange rate multiplied by USD ratio against your local currency.

This is true for whatever pair that is officially supported by some major exchange, e.g BCHEUR, BCHGBP, BCHYEN, etc etc

So while I like the concept to extending trading pairs, I think we should differentiate between currency that are actually traded against BCH and those who are not. For the former we should use the current method, for the latter we should use the one you proposed in this PR.

@gpgpublickey
Copy link
Author

gpgpublickey commented Oct 18, 2021

Thanks for your feedback @sickpig I will talk with the Bitcoin Cash Argentina community to request feedback about it. Will be enough to put a small info icon near these special rates with a disclaimer explaining the way the rate is being calculated?

@ianblas
Copy link

ianblas commented Oct 18, 2021

Hello, many employees of businesses that accept BCH in Argentina use the block explorer to verify payments. But no block explorer is listed in ARS and Argentina time.
We believe that it would be really useful for merchant employees if the BU block explorer has the correct exchange rate in pesos and the Argentine time.
The rule is the following:
We take the US dollar value in ARS from https://yadio.io/simple.html and then multiply it by the Bitcoin Cash value in dollars. Essentially:

BCH price (in USD) × USD price (in ARS)

This rule is the one currently used in wallet.bitcoin.com, register app, and Electron Cash (dektop and android)

@sickpig
Copy link
Owner

sickpig commented Oct 19, 2021

Hello, many employees of businesses that accept BCH in Argentina use the block explorer to verify payments. But no block explorer is listed in ARS and Argentina time. We believe that it would be really useful for merchant employees if the BU block explorer has the correct exchange rate in pesos and the Argentine time. The rule is the following: We take the US dollar value in ARS from yadio.io/simple.html and then multiply it by the Bitcoin Cash value in dollars. Essentially:

BCH price (in USD) × USD price (in ARS)

This rule is the one currently used in wallet.bitcoin.com, register app, and Electron Cash (dektop and android)

I don't disagree and I recognise the importance to have such a function in the explorer especially for fostering adoption in markets/places for which there's no official trading pair for the local currency.

In fact I do think that using the formula above is the right thing to have in those cases. What I'm saying is that we should kept also the current system for local currency for which we have a supported exchange pair supported in some major exchange (e.g. EUR, YEN, etc)

@sickpig
Copy link
Owner

sickpig commented Oct 19, 2021

Thanks for your feedback @sickpig I will talk with the Bitcoin Cash Argentina community to request feedback about it. Will be enough to put a small info icon near these special rates with a disclaimer explaining the way the rate is being calculated?

this will be for sure a good thing to have.

what I think would even better (along with the icon) is to have both system to get values in local currency. For a local currency without an official exchange pari we should use the method developed in your PR, whereas for fiat currency supported by some major exchanges (eg EUR) we should use the current method

(this is based of my understanding of your code which implies that all fiat value are calculated starting from BCHUSD rate)

@gpgpublickey
Copy link
Author

Good morning @sickpig

what I think would even better (along with the icon) is to have both system to get values in local currency. For a local currency without an official exchange pari we should use the method developed in your PR, whereas for fiat currency supported by some major exchanges (eg EUR) we should use the current method

We continue using the original method for those exchange rates like EUR, the extended one is being used just for ARS, but I can add a flag to identify the extended ones and just use the new method if is required. What do you think about it?

@sickpig
Copy link
Owner

sickpig commented Oct 19, 2021

Good morning @sickpig

what I think would even better (along with the icon) is to have both system to get values in local currency. For a local currency without an official exchange pari we should use the method developed in your PR, whereas for fiat currency supported by some major exchanges (eg EUR) we should use the current method

We continue using the original method for those exchange rates like EUR, the extended one is being used just for ARS, but I can add a flag to identify the extended ones and just use the new method if is required. What do you think about it?

it would be great

@gpgpublickey
Copy link
Author

gpgpublickey commented Oct 20, 2021

Hi @sickpig good night, I already added the disclaimer and flag 👍
Getting the exchange rate through an extension API;
image

Getting exchange rates supported by the core API:
image

Cristian Giagante and others added 3 commits October 20, 2021 08:06
use the same naming convention used for exchangedRateData
…y view

if we select a local currency which is handled by the exchangeRateDataExtention
object, we need to change the code to reference the correct jsonUrl in the
"Exchange" label tooltip
@sickpig sickpig merged commit 1d70586 into sickpig:master Oct 20, 2021
@gpgpublickey gpgpublickey deleted the extends-exchange-rates branch October 21, 2021 12:01
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