From ef7b94310e2aa0f7c2b0ecd5137831fbc780f52e Mon Sep 17 00:00:00 2001 From: enenn Date: Wed, 7 Feb 2018 19:27:31 +0100 Subject: [PATCH] Implement buy/sell function and write tests, and add more detailed exception handling. --- freqtrade/__init__.py | 8 ++++ freqtrade/exchange/__init__.py | 52 ++++++++++++++++++++--- freqtrade/main.py | 11 ++--- freqtrade/tests/exchange/test_exchange.py | 37 ++++++++++++---- freqtrade/tests/rpc/test_rpc_telegram.py | 4 +- freqtrade/tests/test_main.py | 39 +++++++++-------- 6 files changed, 108 insertions(+), 43 deletions(-) diff --git a/freqtrade/__init__.py b/freqtrade/__init__.py index cd4515a3b..16f8b3b4a 100644 --- a/freqtrade/__init__.py +++ b/freqtrade/__init__.py @@ -14,3 +14,11 @@ class OperationalException(BaseException): Requires manual intervention. This happens when an exchange returns an unexpected error during runtime. """ + + +class NetworkException(BaseException): + """ + Network related error. + This could happen when an exchange is congested, unavailable, or the user + has networking problems. Usually resolves itself after a time. + """ diff --git a/freqtrade/exchange/__init__.py b/freqtrade/exchange/__init__.py index a1248057f..b3338670f 100644 --- a/freqtrade/exchange/__init__.py +++ b/freqtrade/exchange/__init__.py @@ -9,7 +9,7 @@ from datetime import datetime import arrow from cachetools import cached, TTLCache -from freqtrade import OperationalException +from freqtrade import OperationalException, DependencyException, NetworkException logger = logging.getLogger(__name__) @@ -88,7 +88,7 @@ def validate_pairs(pairs: List[str]) -> None: 'Pair {} is not available at {}'.format(pair, _API.id.lower())) -def buy(pair: str, rate: float, amount: float) -> str: +def buy(pair: str, rate: float, amount: float) -> Dict: if _CONF['dry_run']: global _DRY_RUN_OPEN_ORDERS order_id = 'dry_run_buy_{}'.format(randint(0, 10**6)) @@ -101,12 +101,31 @@ def buy(pair: str, rate: float, amount: float) -> str: 'opened': arrow.utcnow().datetime, 'closed': arrow.utcnow().datetime, } - return order_id + return {'id': order_id} - return _API.buy(pair, rate, amount) + try: + return _API.create_limit_buy_order(pair, amount, rate) + except ccxt.InsufficientFunds as e: + raise DependencyException( + 'Insufficient funds to create limit buy order on market {}.' + 'Tried to buy amount {} at rate {} (total {}).' + 'Message: {}'.format(pair, amount, rate, rate*amount, e) + ) + except ccxt.InvalidOrder as e: + raise DependencyException( + 'Could not create limit buy order on market {}.' + 'Tried to buy amount {} at rate {} (total {}).' + 'Message: {}'.format(pair, amount, rate, rate*amount, e) + ) + except ccxt.NetworkError as e: + raise NetworkException( + 'Could not place buy order due to networking error. Message: {}'.format(e) + ) + except ccxt.BaseError as e: + raise OperationalException(e) -def sell(pair: str, rate: float, amount: float) -> str: +def sell(pair: str, rate: float, amount: float) -> Dict: if _CONF['dry_run']: global _DRY_RUN_OPEN_ORDERS order_id = 'dry_run_sell_{}'.format(randint(0, 10**6)) @@ -119,9 +138,28 @@ def sell(pair: str, rate: float, amount: float) -> str: 'opened': arrow.utcnow().datetime, 'closed': arrow.utcnow().datetime, } - return order_id + return {'id': order_id} - return _API.sell(pair, rate, amount) + try: + return _API.create_limit_sell_order(pair, amount, rate) + except ccxt.InsufficientFunds as e: + raise DependencyException( + 'Insufficient funds to create limit sell order on market {}.' + 'Tried to sell amount {} at rate {} (total {}).' + 'Message: {}'.format(pair, amount, rate, rate*amount, e) + ) + except ccxt.InvalidOrder as e: + raise DependencyException( + 'Could not create limit sell order on market {}.' + 'Tried to sell amount {} at rate {} (total {}).' + 'Message: {}'.format(pair, amount, rate, rate*amount, e) + ) + except ccxt.NetworkError as e: + raise NetworkException( + 'Could not place sell order due to networking error. Message: {}'.format(e) + ) + except ccxt.BaseError as e: + raise OperationalException(e) def get_balance(currency: str) -> float: diff --git a/freqtrade/main.py b/freqtrade/main.py index 941fb5867..f8a2b9b38 100755 --- a/freqtrade/main.py +++ b/freqtrade/main.py @@ -1,6 +1,5 @@ #!/usr/bin/env python3 import copy -import json import logging import sys import time @@ -12,7 +11,7 @@ import arrow import requests from cachetools import cached, TTLCache -from freqtrade import (DependencyException, OperationalException, __version__, +from freqtrade import (DependencyException, OperationalException, NetworkException, __version__, exchange, persistence, rpc) from freqtrade.analyze import get_signal from freqtrade.fiat_convert import CryptoToFiatConverter @@ -128,7 +127,7 @@ def _process(interval: int, nb_assets: Optional[int] = 0) -> bool: check_handle_timedout(_CONF['unfilledtimeout']) Trade.session.flush() - except (requests.exceptions.RequestException, json.JSONDecodeError) as error: + except (NetworkException, DependencyException) as error: logger.warning( 'Got %s in _process(), retrying in 30 seconds...', error @@ -231,7 +230,8 @@ def execute_sell(trade: Trade, limit: float) -> None: :return: None """ # Execute sell and update trade record - order_id = exchange.sell(str(trade.pair), limit, trade.amount) + order = exchange.sell(str(trade.pair), limit, trade.amount) + order_id = order['id'] trade.open_order_id = order_id fmt_exp_profit = round(trade.calc_profit_percent(rate=limit) * 100, 2) @@ -404,7 +404,8 @@ def create_trade(stake_amount: float, interval: int) -> bool: buy_limit = get_target_bid(exchange.get_ticker(pair)) amount = stake_amount / buy_limit - order_id = exchange.buy(pair, buy_limit, amount) + order = exchange.buy(pair, buy_limit, amount) + order_id = order['id'] fiat_converter = CryptoToFiatConverter() stake_amount_fiat = fiat_converter.convert_amount( diff --git a/freqtrade/tests/exchange/test_exchange.py b/freqtrade/tests/exchange/test_exchange.py index 2adc1c0a0..86c73c680 100644 --- a/freqtrade/tests/exchange/test_exchange.py +++ b/freqtrade/tests/exchange/test_exchange.py @@ -90,38 +90,58 @@ def test_buy_dry_run(default_conf, mocker): default_conf['dry_run'] = True mocker.patch.dict('freqtrade.exchange._CONF', default_conf) - assert 'dry_run_buy_' in buy(pair='ETH/BTC', rate=200, amount=1) + order = buy(pair='ETH/BTC', rate=200, amount=1) + assert 'id' in order + assert 'dry_run_buy_' in order['id'] def test_buy_prod(default_conf, mocker): api_mock = MagicMock() - api_mock.buy = MagicMock( - return_value='dry_run_buy_{}'.format(randint(0, 10 ** 6))) + order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6)) + api_mock.create_limit_buy_order = MagicMock(return_value={ + 'id': order_id, + 'info': { + 'foo': 'bar' + } + }) mocker.patch('freqtrade.exchange._API', api_mock) default_conf['dry_run'] = False mocker.patch.dict('freqtrade.exchange._CONF', default_conf) - assert 'dry_run_buy_' in buy(pair='ETH/BTC', rate=200, amount=1) + order = buy(pair='ETH/BTC', rate=200, amount=1) + assert 'id' in order + assert 'info' in order + assert order['id'] == order_id def test_sell_dry_run(default_conf, mocker): default_conf['dry_run'] = True mocker.patch.dict('freqtrade.exchange._CONF', default_conf) - assert 'dry_run_sell_' in sell(pair='ETH/BTC', rate=200, amount=1) + order = sell(pair='ETH/BTC', rate=200, amount=1) + assert 'id' in order + assert 'dry_run_sell_' in order['id'] def test_sell_prod(default_conf, mocker): api_mock = MagicMock() - api_mock.sell = MagicMock( - return_value='dry_run_sell_{}'.format(randint(0, 10 ** 6))) + order_id = 'test_prod_sell_{}'.format(randint(0, 10 ** 6)) + api_mock.create_limit_sell_order = MagicMock(return_value={ + 'id': order_id, + 'info': { + 'foo': 'bar' + } + }) mocker.patch('freqtrade.exchange._API', api_mock) default_conf['dry_run'] = False mocker.patch.dict('freqtrade.exchange._CONF', default_conf) - assert 'dry_run_sell_' in sell(pair='ETH/BTC', rate=200, amount=1) + order = sell(pair='ETH/BTC', rate=200, amount=1) + assert 'id' in order + assert 'info' in order + assert order['id'] == order_id def test_get_balance_dry_run(default_conf, mocker): @@ -301,7 +321,6 @@ def test_get_fee(default_conf, mocker): mocker.patch('freqtrade.exchange._API', api_mock) assert get_fee() == 0.025 - # TODO: disable until caching implemented # def test_exchange_misc(mocker): # api_mock = MagicMock() diff --git a/freqtrade/tests/rpc/test_rpc_telegram.py b/freqtrade/tests/rpc/test_rpc_telegram.py index e6152fb7c..9af8d56ad 100644 --- a/freqtrade/tests/rpc/test_rpc_telegram.py +++ b/freqtrade/tests/rpc/test_rpc_telegram.py @@ -124,7 +124,7 @@ def test_status_table_handle(default_conf, update, ticker, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_order_id')) + buy=MagicMock(return_value={'id': 'mocked_order_id'})) init(default_conf, create_engine('sqlite://')) update_state(State.STOPPED) _status_table(bot=MagicMock(), update=update) @@ -524,7 +524,7 @@ def test_count_handle(default_conf, update, ticker, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_order_id')) + buy=MagicMock(return_value={'id': 'mocked_order_id'})) init(default_conf, create_engine('sqlite://')) update_state(State.STOPPED) _count(bot=MagicMock(), update=update) diff --git a/freqtrade/tests/test_main.py b/freqtrade/tests/test_main.py index bff973339..f0b87e2b3 100644 --- a/freqtrade/tests/test_main.py +++ b/freqtrade/tests/test_main.py @@ -5,12 +5,11 @@ from unittest.mock import MagicMock import arrow import pytest -import requests from sqlalchemy import create_engine import freqtrade.main as main import freqtrade.tests.conftest as tt # test tools -from freqtrade import DependencyException, OperationalException +from freqtrade import DependencyException, OperationalException, NetworkException from freqtrade.main import (_process, check_handle_timedout, create_trade, execute_sell, get_target_bid, handle_trade, init) from freqtrade.misc import State, get_state @@ -81,7 +80,7 @@ def test_process_trade_creation(default_conf, ticker, limit_buy_order, health, m mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy'), + buy=MagicMock(return_value={'id': 'mocked_limit_buy'}), get_order=MagicMock(return_value=limit_buy_order)) init(default_conf, create_engine('sqlite://')) @@ -111,7 +110,7 @@ def test_process_exchange_failures(default_conf, ticker, health, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(side_effect=requests.exceptions.RequestException)) + buy=MagicMock(side_effect=NetworkException)) init(default_conf, create_engine('sqlite://')) result = _process(interval=default_conf['ticker_interval']) assert result is False @@ -143,7 +142,7 @@ def test_process_trade_handling(default_conf, ticker, limit_buy_order, health, m mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy'), + buy=MagicMock(return_value={'id': 'mocked_limit_buy'}), get_order=MagicMock(return_value=limit_buy_order)) init(default_conf, create_engine('sqlite://')) @@ -165,7 +164,7 @@ def test_create_trade(default_conf, ticker, limit_buy_order, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) # Save state of current whitelist whitelist = copy.deepcopy(default_conf['exchange']['pair_whitelist']) @@ -193,7 +192,7 @@ def test_create_trade_minimal_amount(default_conf, ticker, mocker): mocker.patch.multiple('freqtrade.rpc', init=MagicMock(), send_msg=MagicMock()) mocker.patch('freqtrade.main.get_signal', side_effect=lambda s, t: (True, False)) buy_mock = mocker.patch( - 'freqtrade.main.exchange.buy', MagicMock(return_value='mocked_limit_buy') + 'freqtrade.main.exchange.buy', MagicMock(return_value={'id': 'mocked_limit_buy'}) ) mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), @@ -212,7 +211,7 @@ def test_create_trade_no_stake_amount(default_conf, ticker, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy'), + buy=MagicMock(return_value={'id': 'mocked_limit_buy'}), get_balance=MagicMock(return_value=default_conf['stake_amount'] * 0.5)) with pytest.raises(DependencyException, match=r'.*stake amount.*'): create_trade(default_conf['stake_amount'], default_conf['ticker_interval']) @@ -225,7 +224,7 @@ def test_create_trade_no_pairs(default_conf, ticker, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) with pytest.raises(DependencyException, match=r'.*No pair in whitelist.*'): conf = copy.deepcopy(default_conf) @@ -241,7 +240,7 @@ def test_create_trade_no_pairs_after_blacklist(default_conf, ticker, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) with pytest.raises(DependencyException, match=r'.*No pair in whitelist.*'): conf = copy.deepcopy(default_conf) @@ -277,8 +276,8 @@ def test_handle_trade(default_conf, limit_buy_order, limit_sell_order, mocker): 'last': 0.00001172 }), get_fee=MagicMock(return_value=0.0025), - buy=MagicMock(return_value='mocked_limit_buy'), - sell=MagicMock(return_value='mocked_limit_sell')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'}), + sell=MagicMock(return_value={'id': 'mocked_limit_sell'})) mocker.patch.multiple('freqtrade.fiat_convert.Pymarketcap', ticker=MagicMock(return_value={'price_usd': 15000.0}), _cache_symbols=MagicMock(return_value={'BTC': 1})) @@ -313,7 +312,7 @@ def test_handle_overlpapping_signals(default_conf, ticker, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) mocker.patch('freqtrade.main.min_roi_reached', return_value=False) init(default_conf, create_engine('sqlite://')) @@ -363,7 +362,7 @@ def test_handle_trade_roi(default_conf, ticker, mocker, caplog): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) mocker.patch('freqtrade.main.min_roi_reached', return_value=True) init(default_conf, create_engine('sqlite://')) @@ -395,7 +394,7 @@ def test_handle_trade_experimental(default_conf, ticker, mocker, caplog): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) mocker.patch('freqtrade.main.min_roi_reached', return_value=False) init(default_conf, create_engine('sqlite://')) @@ -420,7 +419,7 @@ def test_close_trade(default_conf, ticker, limit_buy_order, limit_sell_order, mo mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker, - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) # Create trade and sell it init(default_conf, create_engine('sqlite://')) @@ -747,7 +746,7 @@ def test_sell_profit_only_enable_profit(default_conf, limit_buy_order, mocker): 'last': 0.00002172 }), get_fee=MagicMock(return_value=0.0025), - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) init(default_conf, create_engine('sqlite://')) create_trade(0.001, default_conf['ticker_interval']) @@ -776,7 +775,7 @@ def test_sell_profit_only_disable_profit(default_conf, limit_buy_order, mocker): 'last': 0.00002172 }), get_fee=MagicMock(return_value=0.0025), - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) init(default_conf, create_engine('sqlite://')) create_trade(0.001, default_conf['ticker_interval']) @@ -805,7 +804,7 @@ def test_sell_profit_only_enable_loss(default_conf, limit_buy_order, mocker): 'last': 0.00000172 }), get_fee=MagicMock(return_value=0.0025), - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) init(default_conf, create_engine('sqlite://')) create_trade(0.001, default_conf['ticker_interval']) @@ -834,7 +833,7 @@ def test_sell_profit_only_disable_loss(default_conf, limit_buy_order, mocker): 'last': 0.00000172 }), get_fee=MagicMock(return_value=0.0025), - buy=MagicMock(return_value='mocked_limit_buy')) + buy=MagicMock(return_value={'id': 'mocked_limit_buy'})) init(default_conf, create_engine('sqlite://')) create_trade(0.001, default_conf['ticker_interval'])