From 9755e9b11f15841ed9ab9473b21defe23ee522d0 Mon Sep 17 00:00:00 2001 From: Kay Kurokawa Date: Mon, 27 Feb 2017 20:18:57 -0500 Subject: [PATCH] Improvements to exchange rate manager --- CHANGELOG.md | 2 +- lbrynet/core/Error.py | 7 ++ lbrynet/lbrynet_daemon/ExchangeRateManager.py | 77 +++++++++++------ requirements.txt | 1 - setup.py | 1 - tests/unit/core/test_ExchangeRateManager.py | 38 --------- .../test_ExchangeRateManager.py | 85 +++++++++++++++++++ 7 files changed, 142 insertions(+), 69 deletions(-) delete mode 100644 tests/unit/core/test_ExchangeRateManager.py create mode 100644 tests/unit/lbrynet_daemon/test_ExchangeRateManager.py diff --git a/CHANGELOG.md b/CHANGELOG.md index abd8249de..885c233aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ at anytime. * ### Fixed - * + * Fixed ExchangeRateManager freezing the app * * diff --git a/lbrynet/core/Error.py b/lbrynet/core/Error.py index cd7959235..077b4c88c 100644 --- a/lbrynet/core/Error.py +++ b/lbrynet/core/Error.py @@ -26,6 +26,13 @@ class KeyFeeAboveMaxAllowed(Exception): pass +class InvalidExchangeRateResponse(Exception): + def __init__(self, source, reason): + Exception.__init__(self, 'Failed to get exchange rate from {}:{}'.format(source, reason)) + self.source = source + self.reason = reason + + class UnknownNameError(Exception): def __init__(self, name): Exception.__init__(self, 'Name {} is unknown'.format(name)) diff --git a/lbrynet/lbrynet_daemon/ExchangeRateManager.py b/lbrynet/lbrynet_daemon/ExchangeRateManager.py index ff03c0304..46f4b0d22 100644 --- a/lbrynet/lbrynet_daemon/ExchangeRateManager.py +++ b/lbrynet/lbrynet_daemon/ExchangeRateManager.py @@ -2,13 +2,12 @@ import time import requests import logging import json -import googlefinance -from twisted.internet import defer, reactor +from twisted.internet import defer, threads, reactor from twisted.internet.task import LoopingCall from lbrynet import conf from lbrynet.metadata.Fee import FeeValidator - +from lbrynet.core.Error import InvalidExchangeRateResponse log = logging.getLogger(__name__) @@ -24,11 +23,18 @@ class ExchangeRate(object): self.spot = spot self.ts = ts + def __repr__(self): + out = "Currency pair:{}, spot:{}, ts:{}".format( + self.currency_pair, self.spot, self.ts) + return out + def as_dict(self): return {'spot': self.spot, 'ts': self.ts} class MarketFeed(object): + REQUESTS_TIMEOUT = 20 + EXCHANGE_RATE_UPDATE_RATE_SEC = 300 def __init__(self, market, name, url, params, fee): self.market = market self.name = name @@ -38,13 +44,13 @@ class MarketFeed(object): self.rate = None self._updater = LoopingCall(self._update_price) + @property + def rate_is_initialized(self): + return self.rate is not None + def _make_request(self): - try: - r = requests.get(self.url, self.params) - return defer.succeed(r.text) - except Exception as err: - log.error(err) - return defer.fail(err) + r = requests.get(self.url, self.params, timeout=self.REQUESTS_TIMEOUT) + return r.text def _handle_response(self, response): return NotImplementedError @@ -64,15 +70,16 @@ class MarketFeed(object): self.market, self.name) def _update_price(self): - d = self._make_request() + d = threads.deferToThread(self._make_request) d.addCallback(self._handle_response) d.addCallback(self._subtract_fee) d.addCallback(self._save_price) d.addErrback(self._log_error) + return d def start(self): if not self._updater.running: - self._updater.start(300) + self._updater.start(self.EXCHANGE_RATE_UPDATE_RATE_SEC) def stop(self): if self._updater.running: @@ -91,8 +98,17 @@ class BittrexFeed(MarketFeed): ) def _handle_response(self, response): - trades = json.loads(response)['result'] - vwap = sum([i['Total'] for i in trades]) / sum([i['Quantity'] for i in trades]) + json_response = json.loads(response) + if 'result' not in json_response: + raise InvalidExchangeRateResponse(self.name, 'result not found') + trades = json_response['result'] + if len(trades) == 0: + raise InvalidExchangeRateResponse(self.market, 'trades not found') + totals = sum([i['Total'] for i in trades]) + qtys = sum([i['Quantity'] for i in trades]) + if totals <= 0 or qtys <= 0: + raise InvalidExchangeRateResponse(self.market, 'quantities were not positive') + vwap = totals/qtys return defer.succeed(float(1.0 / vwap)) @@ -102,20 +118,20 @@ class GoogleBTCFeed(MarketFeed): self, "USDBTC", "Coinbase via Google finance", - None, - None, + 'http://finance.google.com/finance/info', + {'client':'ig', 'q':'CURRENCY:USDBTC'}, COINBASE_FEE ) - def _make_request(self): - try: - r = googlefinance.getQuotes('CURRENCY:USDBTC')[0] - return defer.succeed(r) - except Exception as err: - return defer.fail(err) - def _handle_response(self, response): - return float(response['LastTradePrice']) + response = response[3:] # response starts with "// " + json_response = json.loads(response)[0] + if 'l' not in json_response: + raise InvalidExchangeRateResponse(self.name, 'last trade not found') + last_trade_price = float(json_response['l']) + if last_trade_price <= 0: + raise InvalidExchangeRateResponse(self.name, 'trade price was not positive') + return defer.succeed(last_trade_price) def get_default_market_feed(currency_pair): @@ -149,14 +165,17 @@ class ExchangeRateManager(object): source.stop() def convert_currency(self, from_currency, to_currency, amount): - log.info("Converting %f %s to %s" % (amount, from_currency, to_currency)) + rates = [market.rate for market in self.market_feeds] + log.info("Converting %f %s to %s, rates: %s" % (amount, from_currency, to_currency, rates)) if from_currency == to_currency: return amount for market in self.market_feeds: - if market.rate.currency_pair == (from_currency, to_currency): + if (market.rate_is_initialized and + market.rate.currency_pair == (from_currency, to_currency)): return amount * market.rate.spot for market in self.market_feeds: - if market.rate.currency_pair[0] == from_currency: + if (market.rate_is_initialized and + market.rate.currency_pair[0] == from_currency): return self.convert_currency( market.rate.currency_pair[1], to_currency, amount * market.rate.spot) raise Exception( @@ -215,10 +234,12 @@ class DummyExchangeRateManager(object): def convert_currency(self, from_currency, to_currency, amount): log.debug("Converting %f %s to %s" % (amount, from_currency, to_currency)) for market in self.market_feeds: - if market.rate.currency_pair == (from_currency, to_currency): + if (market.rate_is_initialized and + market.rate.currency_pair == (from_currency, to_currency)): return amount * market.rate.spot for market in self.market_feeds: - if market.rate.currency_pair[0] == from_currency: + if (market.rate_is_initialized and + market.rate.currency_pair[0] == from_currency): return self.convert_currency( market.rate.currency_pair[1], to_currency, amount * market.rate.spot) diff --git a/requirements.txt b/requirements.txt index 273806de4..131595e39 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,6 @@ dnspython==1.12.0 ecdsa==0.13 envparse==0.2.0 gmpy==1.17 -googlefinance==0.7 jsonrpc==1.2 jsonrpclib==0.1.7 jsonschema==2.5.1 diff --git a/setup.py b/setup.py index 6155f8521..9c7831a8d 100644 --- a/setup.py +++ b/setup.py @@ -30,7 +30,6 @@ requires = [ 'appdirs', 'base58', 'envparse', - 'googlefinance', 'jsonrpc', 'jsonschema', 'lbryum>=2.7.6', diff --git a/tests/unit/core/test_ExchangeRateManager.py b/tests/unit/core/test_ExchangeRateManager.py deleted file mode 100644 index 1cd24b700..000000000 --- a/tests/unit/core/test_ExchangeRateManager.py +++ /dev/null @@ -1,38 +0,0 @@ -from lbrynet.metadata import Fee -from lbrynet.lbrynet_daemon import ExchangeRateManager - -from twisted.trial import unittest - -from tests import util - - -class FeeFormatTest(unittest.TestCase): - def test_fee_created_with_correct_inputs(self): - fee_dict = { - 'USD': { - 'amount': 10.0, - 'address': "bRcHraa8bYJZL7vkh5sNmGwPDERFUjGPP9" - } - } - fee = Fee.FeeValidator(fee_dict) - self.assertEqual(10.0, fee['USD']['amount']) - - -class FeeTest(unittest.TestCase): - def setUp(self): - util.resetTime(self) - - def test_fee_converts_to_lbc(self): - fee_dict = { - 'USD': { - 'amount': 10.0, - 'address': "bRcHraa8bYJZL7vkh5sNmGwPDERFUjGPP9" - } - } - rates = { - 'BTCLBC': {'spot': 3.0, 'ts': util.DEFAULT_ISO_TIME + 1}, - 'USDBTC': {'spot': 2.0, 'ts': util.DEFAULT_ISO_TIME + 2} - } - manager = ExchangeRateManager.DummyExchangeRateManager(rates) - result = manager.to_lbc(fee_dict).amount - self.assertEqual(60.0, result) diff --git a/tests/unit/lbrynet_daemon/test_ExchangeRateManager.py b/tests/unit/lbrynet_daemon/test_ExchangeRateManager.py new file mode 100644 index 000000000..34529c18c --- /dev/null +++ b/tests/unit/lbrynet_daemon/test_ExchangeRateManager.py @@ -0,0 +1,85 @@ +from lbrynet.metadata import Fee +from lbrynet.lbrynet_daemon import ExchangeRateManager +from lbrynet import conf +from lbrynet.core.Error import InvalidExchangeRateResponse + +from twisted.trial import unittest +from twisted.internet import defer +from tests import util + + +class FeeFormatTest(unittest.TestCase): + def test_fee_created_with_correct_inputs(self): + fee_dict = { + 'USD': { + 'amount': 10.0, + 'address': "bRcHraa8bYJZL7vkh5sNmGwPDERFUjGPP9" + } + } + fee = Fee.FeeValidator(fee_dict) + self.assertEqual(10.0, fee['USD']['amount']) + + +class FeeTest(unittest.TestCase): + def setUp(self): + util.resetTime(self) + + def test_fee_converts_to_lbc(self): + fee_dict = { + 'USD': { + 'amount': 10.0, + 'address': "bRcHraa8bYJZL7vkh5sNmGwPDERFUjGPP9" + } + } + rates = { + 'BTCLBC': {'spot': 3.0, 'ts': util.DEFAULT_ISO_TIME + 1}, + 'USDBTC': {'spot': 2.0, 'ts': util.DEFAULT_ISO_TIME + 2} + } + manager = ExchangeRateManager.DummyExchangeRateManager(rates) + result = manager.to_lbc(fee_dict).amount + self.assertEqual(60.0, result) + +class GoogleBTCFeedTest(unittest.TestCase): + + @defer.inlineCallbacks + def test_handle_response(self): + feed = ExchangeRateManager.GoogleBTCFeed() + + response = '// [ { "id": "-2001" ,"t" : "USDBTC" ,"e" : "CURRENCY" ,"l" : "0.0008" ,"l_fix" : "" ,"l_cur" : "" ,"s": "0" ,"ltt":"" ,"lt" : "Feb 27, 10:21PM GMT" ,"lt_dts" : "2017-02-27T22:21:39Z" ,"c" : "-0.00001" ,"c_fix" : "" ,"cp" : "-0.917" ,"cp_fix" : "" ,"ccol" : "chr" ,"pcls_fix" : "" } ]' + out = yield feed._handle_response(response) + self.assertEqual(0.0008, out) + + # check negative trade price throws exception + response = '// [ { "id": "-2001" ,"t" : "USDBTC" ,"e" : "CURRENCY" ,"l" : "-0.0008" ,"l_fix" : "" ,"l_cur" : "" ,"s": "0" ,"ltt":"" ,"lt" : "Feb 27, 10:21PM GMT" ,"lt_dts" : "2017-02-27T22:21:39Z" ,"c" : "-0.00001" ,"c_fix" : "" ,"cp" : "-0.917" ,"cp_fix" : "" ,"ccol" : "chr" ,"pcls_fix" : "" } ]' + with self.assertRaises(InvalidExchangeRateResponse): + out = yield feed._handle_response(response) + + + + +class BittrexFeedTest(unittest.TestCase): + def setUp(self): + conf.initialize_settings() + + def tearDown(self): + conf.settings = None + + @defer.inlineCallbacks + def test_handle_response(self): + feed = ExchangeRateManager.BittrexFeed() + + response ='{"success":true,"message":"","result":[{"Id":6902471,"TimeStamp":"2017-02-27T23:41:52.213","Quantity":56.12611239,"Price":0.00001621,"Total":0.00090980,"FillType":"PARTIAL_FILL","OrderType":"SELL"},{"Id":6902403,"TimeStamp":"2017-02-27T23:31:40.463","Quantity":430.99988180,"Price":0.00001592,"Total":0.00686151,"FillType":"PARTIAL_FILL","OrderType":"SELL"}]}' + out = yield feed._handle_response(response) + expected= 1.0 / ((0.00090980+0.00686151) / (56.12611239+430.99988180)) + self.assertEqual(expected, out) + + response='{}' + with self.assertRaises(InvalidExchangeRateResponse): + out = yield feed._handle_response(response) + + response='{"success":true,"result":[]}' + with self.assertRaises(InvalidExchangeRateResponse): + out = yield feed._handle_response(response) + + +