Merge pull request #1652 from freqtrade/fix/tif-market_order_combo

Fix/tif market order combo

freqtrade/exchange/exchange.py

@@ -352,7 +352,7 @@ class Exchange(object):
             return dry_order
 
         params = self._params.copy()
-        if time_in_force != 'gtc':
+        if time_in_force != 'gtc' and ordertype != 'market':
             params.update({'timeInForce': time_in_force})
 
         return self.create_order(pair, ordertype, 'buy', amount, rate, params)
@@ -365,7 +365,7 @@ class Exchange(object):
             return dry_order
 
         params = self._params.copy()
-        if time_in_force != 'gtc':
+        if time_in_force != 'gtc' and ordertype != 'market':
             params.update({'timeInForce': time_in_force})
 
         return self.create_order(pair, ordertype, 'sell', amount, rate, params)

freqtrade/tests/exchange/test_exchange.py

@@ -4,7 +4,7 @@ import copy
 import logging
 from datetime import datetime
 from random import randint
-from unittest.mock import Mock, MagicMock, PropertyMock
+from unittest.mock import MagicMock, Mock, PropertyMock
 
 import arrow
 import ccxt
@@ -12,11 +12,10 @@ import pytest
 from pandas import DataFrame
 
 from freqtrade import DependencyException, OperationalException, TemporaryError
-from freqtrade.exchange import Exchange, Kraken, Binance
+from freqtrade.exchange import Binance, Exchange, Kraken
 from freqtrade.exchange.exchange import API_RETRY_COUNT
-from freqtrade.tests.conftest import get_patched_exchange, log_has, log_has_re
 from freqtrade.resolvers.exchange_resolver import ExchangeResolver
-
+from freqtrade.tests.conftest import get_patched_exchange, log_has, log_has_re
 
 # Make sure to always keep one exchange here which is NOT subclassed!!
 EXCHANGES = ['bittrex', 'binance', 'kraken', ]
@@ -591,11 +590,10 @@ def test_buy_prod(default_conf, mocker, exchange_name):
                      amount=1, rate=200, time_in_force=time_in_force)
 
 
-def test_buy_considers_time_in_force(default_conf, mocker):
+@pytest.mark.parametrize("exchange_name", EXCHANGES)
+def test_buy_considers_time_in_force(default_conf, mocker, exchange_name):
     api_mock = MagicMock()
     order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6))
-    order_type = 'market'
-    time_in_force = 'ioc'
     api_mock.create_order = MagicMock(return_value={
         'id': order_id,
         'info': {
@@ -605,7 +603,27 @@ def test_buy_considers_time_in_force(default_conf, mocker):
     default_conf['dry_run'] = False
     mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y)
     mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y)
-    exchange = get_patched_exchange(mocker, default_conf, api_mock)
+    exchange = get_patched_exchange(mocker, default_conf, api_mock, id=exchange_name)
+
+    order_type = 'limit'
+    time_in_force = 'ioc'
+
+    order = exchange.buy(pair='ETH/BTC', ordertype=order_type,
+                         amount=1, rate=200, time_in_force=time_in_force)
+
+    assert 'id' in order
+    assert 'info' in order
+    assert order['id'] == order_id
+    assert api_mock.create_order.call_args[0][0] == 'ETH/BTC'
+    assert api_mock.create_order.call_args[0][1] == order_type
+    assert api_mock.create_order.call_args[0][2] == 'buy'
+    assert api_mock.create_order.call_args[0][3] == 1
+    assert api_mock.create_order.call_args[0][4] == 200
+    assert "timeInForce" in api_mock.create_order.call_args[0][5]
+    assert api_mock.create_order.call_args[0][5]["timeInForce"] == time_in_force
+
+    order_type = 'market'
+    time_in_force = 'ioc'
 
     order = exchange.buy(pair='ETH/BTC', ordertype=order_type,
                          amount=1, rate=200, time_in_force=time_in_force)
@@ -618,68 +636,8 @@ def test_buy_considers_time_in_force(default_conf, mocker):
     assert api_mock.create_order.call_args[0][2] == 'buy'
     assert api_mock.create_order.call_args[0][3] == 1
     assert api_mock.create_order.call_args[0][4] is None
-    assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc'}
+    # Market orders should not send timeInForce!!
-
+    assert "timeInForce" not in api_mock.create_order.call_args[0][5]
-
-def test_buy_kraken_trading_agreement(default_conf, mocker):
-    api_mock = MagicMock()
-    order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6))
-    order_type = 'market'
-    time_in_force = 'ioc'
-    api_mock.create_order = MagicMock(return_value={
-        'id': order_id,
-        'info': {
-            'foo': 'bar'
-        }
-    })
-    default_conf['dry_run'] = False
-
-    mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y)
-    mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y)
-    exchange = get_patched_exchange(mocker, default_conf, api_mock, id="kraken")
-
-    order = exchange.buy(pair='ETH/BTC', ordertype=order_type,
-                         amount=1, rate=200, time_in_force=time_in_force)
-
-    assert 'id' in order
-    assert 'info' in order
-    assert order['id'] == order_id
-    assert api_mock.create_order.call_args[0][0] == 'ETH/BTC'
-    assert api_mock.create_order.call_args[0][1] == order_type
-    assert api_mock.create_order.call_args[0][2] == 'buy'
-    assert api_mock.create_order.call_args[0][3] == 1
-    assert api_mock.create_order.call_args[0][4] is None
-    assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc',
-                                                     'trading_agreement': 'agree'}
-
-
-def test_sell_kraken_trading_agreement(default_conf, mocker):
-    api_mock = MagicMock()
-    order_id = 'test_prod_sell_{}'.format(randint(0, 10 ** 6))
-    order_type = 'market'
-    api_mock.create_order = MagicMock(return_value={
-        'id': order_id,
-        'info': {
-            'foo': 'bar'
-        }
-    })
-    default_conf['dry_run'] = False
-
-    mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y)
-    mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y)
-    exchange = get_patched_exchange(mocker, default_conf, api_mock, id="kraken")
-
-    order = exchange.sell(pair='ETH/BTC', ordertype=order_type, amount=1, rate=200)
-
-    assert 'id' in order
-    assert 'info' in order
-    assert order['id'] == order_id
-    assert api_mock.create_order.call_args[0][0] == 'ETH/BTC'
-    assert api_mock.create_order.call_args[0][1] == order_type
-    assert api_mock.create_order.call_args[0][2] == 'sell'
-    assert api_mock.create_order.call_args[0][3] == 1
-    assert api_mock.create_order.call_args[0][4] is None
-    assert api_mock.create_order.call_args[0][5] == {'trading_agreement': 'agree'}
 
 
 def test_sell_dry_run(default_conf, mocker):
@@ -750,6 +708,55 @@ def test_sell_prod(default_conf, mocker, exchange_name):
         exchange.sell(pair='ETH/BTC', ordertype=order_type, amount=1, rate=200)
 
 
+@pytest.mark.parametrize("exchange_name", EXCHANGES)
+def test_sell_considers_time_in_force(default_conf, mocker, exchange_name):
+    api_mock = MagicMock()
+    order_id = 'test_prod_sell_{}'.format(randint(0, 10 ** 6))
+    api_mock.create_order = MagicMock(return_value={
+        'id': order_id,
+        'info': {
+            'foo': 'bar'
+        }
+    })
+    default_conf['dry_run'] = False
+    mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y)
+    mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y)
+    exchange = get_patched_exchange(mocker, default_conf, api_mock, id=exchange_name)
+
+    order_type = 'limit'
+    time_in_force = 'ioc'
+
+    order = exchange.sell(pair='ETH/BTC', ordertype=order_type,
+                          amount=1, rate=200, time_in_force=time_in_force)
+
+    assert 'id' in order
+    assert 'info' in order
+    assert order['id'] == order_id
+    assert api_mock.create_order.call_args[0][0] == 'ETH/BTC'
+    assert api_mock.create_order.call_args[0][1] == order_type
+    assert api_mock.create_order.call_args[0][2] == 'sell'
+    assert api_mock.create_order.call_args[0][3] == 1
+    assert api_mock.create_order.call_args[0][4] == 200
+    assert "timeInForce" in api_mock.create_order.call_args[0][5]
+    assert api_mock.create_order.call_args[0][5]["timeInForce"] == time_in_force
+
+    order_type = 'market'
+    time_in_force = 'ioc'
+    order = exchange.sell(pair='ETH/BTC', ordertype=order_type,
+                          amount=1, rate=200, time_in_force=time_in_force)
+
+    assert 'id' in order
+    assert 'info' in order
+    assert order['id'] == order_id
+    assert api_mock.create_order.call_args[0][0] == 'ETH/BTC'
+    assert api_mock.create_order.call_args[0][1] == order_type
+    assert api_mock.create_order.call_args[0][2] == 'sell'
+    assert api_mock.create_order.call_args[0][3] == 1
+    assert api_mock.create_order.call_args[0][4] is None
+    # Market orders should not send timeInForce!!
+    assert "timeInForce" not in api_mock.create_order.call_args[0][5]
+
+
 def test_get_balance_dry_run(default_conf, mocker):
     default_conf['dry_run'] = True
 

freqtrade/tests/exchange/test_kraken.py

@@ -0,0 +1,67 @@
+# pragma pylint: disable=missing-docstring, C0103, bad-continuation, global-statement
+# pragma pylint: disable=protected-access
+from random import randint
+from unittest.mock import MagicMock
+
+from freqtrade.tests.conftest import get_patched_exchange
+
+
+def test_buy_kraken_trading_agreement(default_conf, mocker):
+    api_mock = MagicMock()
+    order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6))
+    order_type = 'limit'
+    time_in_force = 'ioc'
+    api_mock.create_order = MagicMock(return_value={
+        'id': order_id,
+        'info': {
+            'foo': 'bar'
+        }
+    })
+    default_conf['dry_run'] = False
+
+    mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y)
+    mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y)
+    exchange = get_patched_exchange(mocker, default_conf, api_mock, id="kraken")
+
+    order = exchange.buy(pair='ETH/BTC', ordertype=order_type,
+                         amount=1, rate=200, time_in_force=time_in_force)
+
+    assert 'id' in order
+    assert 'info' in order
+    assert order['id'] == order_id
+    assert api_mock.create_order.call_args[0][0] == 'ETH/BTC'
+    assert api_mock.create_order.call_args[0][1] == order_type
+    assert api_mock.create_order.call_args[0][2] == 'buy'
+    assert api_mock.create_order.call_args[0][3] == 1
+    assert api_mock.create_order.call_args[0][4] == 200
+    assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc',
+                                                     'trading_agreement': 'agree'}
+
+
+def test_sell_kraken_trading_agreement(default_conf, mocker):
+    api_mock = MagicMock()
+    order_id = 'test_prod_sell_{}'.format(randint(0, 10 ** 6))
+    order_type = 'market'
+    api_mock.create_order = MagicMock(return_value={
+        'id': order_id,
+        'info': {
+            'foo': 'bar'
+        }
+    })
+    default_conf['dry_run'] = False
+
+    mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y)
+    mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y)
+    exchange = get_patched_exchange(mocker, default_conf, api_mock, id="kraken")
+
+    order = exchange.sell(pair='ETH/BTC', ordertype=order_type, amount=1, rate=200)
+
+    assert 'id' in order
+    assert 'info' in order
+    assert order['id'] == order_id
+    assert api_mock.create_order.call_args[0][0] == 'ETH/BTC'
+    assert api_mock.create_order.call_args[0][1] == order_type
+    assert api_mock.create_order.call_args[0][2] == 'sell'
+    assert api_mock.create_order.call_args[0][3] == 1
+    assert api_mock.create_order.call_args[0][4] is None
+    assert api_mock.create_order.call_args[0][5] == {'trading_agreement': 'agree'}