duckduckgo / zeroclickinfo-spice

DuckDuckGo Instant Answers based on JavaScript (JSON) APIs
https://duckduckhack.com/
Other
548 stars 942 forks source link

Currency Bug: BTC Price using rates from July 2016 #3419

Closed bgiesing closed 7 years ago

bgiesing commented 7 years ago

Description

The currency calculator is showing super inaccurate rates for Bitcoin for XE.com from July 2016. Example: 0.00045059 BTC shows up as $0.3 USD when it's currently ~$2.13.

Steps to recreate

https://duckduckgo.com/?q=0.00045059+btc https://duckduckgo.com/?q=1+xbt+in+usd

Using Firefox Nightly on Arch Linux.


Instant Answer Page: https://duck.co/ia/view/currency

J-tt commented 7 years ago

Ok, this seems like an issue with the API.

When we look at what is being returned (truncated):

"conversion":{
"rate-utc-timestamp": "2016-07-21 16:49:02",
"rate-frequency": "Daily Rates",
"from-amount": "1",
"from-currency-symbol": "XBT",
"from-currency-name": "Bitcoins",
"converted-amount": "885.585",
"to-currency-symbol": "AUD",
"to-currency-name": "Australia Dollars",
"conversion-rate": "1 XBT = 885.585 AUD",
"conversion-inverse": "1 AUD = 0.001129 XBT"
},

We can see that that rate-utc-timestamp is quite outdated. This is probably associated with https://github.com/duckduckgo/zeroclickinfo-spice/blob/master/lib/DDG/Spice/Currency.pm#L60. I can't test it with the XE api as I don't have an API key right now, but it's either an issue with caching by DDG or XE providing out of date results. If you check xe.com the results are up to date.

@pjhampton Could you provide some more information.

tagawa commented 7 years ago

Looks like this query should be using the cryptonator.com API, not XE, as we do for other cryptocurrency searches, e.g. https://duckduckgo.com/?q=1+eth+in+usd&ia=cryptocurrency

bgiesing commented 7 years ago

@tagawa that's exactly what I was thinking also. I actually tested it by doing that search as 0.00045059 BTC to LTC and manually selecting USD under the drop down and it was correct so this issue might easily be solved by making BTC use the Crypto calc instead of the standard one

J-tt commented 7 years ago

Yeah, that would probably be the best option, would the best course of action be removing them from the list, or changing providers for crypto?

bgiesing commented 7 years ago

@J-tt well Most of the crypto already use the Crypto specific converter, I think BTC is the only one that uses the normal one.

J-tt commented 7 years ago

Then for consistency it would probably be best to remove BTC from the converter.

pjhampton commented 7 years ago

Hmm, yeah - it seems like it is frozen in time 😕 Ok, I will remove the BTC stuff from the currency IA and trigger the cryptocurrency instead. I'm working on a patch now 👍

pjhampton commented 7 years ago

Hey everyone! Just to let you know this has been resolved (for now). The internal DDG employees are keeping a close eye on things and I will also be around to make any necessary patches if needed 💛 💙

bgiesing commented 7 years ago

@pjhampton it still triggers the Currency converter and not the crypto converter on the site for both searches in the OP. The rates are correct now but shouldn't it be using the Crypto one still and not XE.com?

Also the searches don't work at all on https://beta.duckduckgo.com

pjhampton commented 7 years ago

Thanks @bgiesing. This change is currently staged in https://github.com/duckduckgo/zeroclickinfo-spice/pull/3420 One of the DDG employees will merge and deploy next week 👍