small refactor of check_handle_timedout

freqtrade/main.py

@@ -158,6 +158,53 @@ def _process(nb_assets: Optional[int] = 0) -> bool:
     return state_changed
 
 
+# FIX: 20180110, why is cancel.order unconditionally here, whereas
+#                it is conditionally called in the
+#                handle_timedout_limit_sell()?
+def handle_timedout_limit_buy(trade, order):
+    """Buy timeout - cancel order
+    :return: True if order was fully cancelled
+    """
+    exchange.cancel_order(trade.open_order_id)
+    if order['remaining'] == order['amount']:
+        # if trade is not partially completed, just delete the trade
+        Trade.session.delete(trade)
+        # FIX? do we really need to flush, caller of
+        #      check_handle_timedout will flush afterwards
+        Trade.session.flush()
+        logger.info('Buy order timeout for %s.', trade)
+        return True
+    else:
+        # if trade is partially complete, edit the stake details for the trade
+        # and close the order
+        trade.amount = order['amount'] - order['remaining']
+        trade.stake_amount = trade.amount * trade.open_rate
+        trade.open_order_id = None
+        logger.info('Partial buy order timeout for %s.', trade)
+        return False
+
+
+# FIX: 20180110, should cancel_order() be cond. or unconditionally called?
+def handle_timedout_limit_sell(trade, order):
+    """
+    Sell timeout - cancel order and update trade
+    :return: True if order was fully cancelled
+    """
+    if order['remaining'] == order['amount']:
+        # if trade is not partially completed, just cancel the trade
+        exchange.cancel_order(trade.open_order_id)
+        trade.close_rate = None
+        trade.close_profit = None
+        trade.close_date = None
+        trade.is_open = True
+        trade.open_order_id = None
+        logger.info('Sell order timeout for %s.', trade)
+        return True
+    else:
+        # TODO: figure out how to handle partially complete sell orders
+        return False
+
+
 def check_handle_timedout(timeoutvalue: int) -> None:
     """
     Check if any orders are timed out and cancel if neccessary
@@ -171,35 +218,16 @@ def check_handle_timedout(timeoutvalue: int) -> None:
         ordertime = arrow.get(order['opened'])
 
         if order['type'] == "LIMIT_BUY" and ordertime < timeoutthreashold:
-            # Buy timeout - cancel order
+            handle_timedout_limit_buy(trade, order)
-            exchange.cancel_order(trade.open_order_id)
+
-            if order['remaining'] == order['amount']:
-                # if trade is not partially completed, just delete the trade
-                Trade.session.delete(trade)
-                Trade.session.flush()
-                logger.info('Buy order timeout for %s.', trade)
-            else:
-                # if trade is partially complete, edit the stake details for the trade
-                # and close the order
-                trade.amount = order['amount'] - order['remaining']
-                trade.stake_amount = trade.amount * trade.open_rate
-                trade.open_order_id = None
-                logger.info('Partial buy order timeout for %s.', trade)
         elif order['type'] == "LIMIT_SELL" and ordertime < timeoutthreashold:
-            # Sell timeout - cancel order and update trade
+            if handle_timedout_limit_sell(trade, order):
-            if order['remaining'] == order['amount']:
+                # BUG? if there is more trades that are
-                # if trade is not partially completed, just cancel the trade
+                #      timed out, shouldn't we collect and
-                exchange.cancel_order(trade.open_order_id)
+                #      then return all of them?
-                trade.close_rate = None
+                #      Also the function signature is return None.
-                trade.close_profit = None
+                #      But we return True here.
-                trade.close_date = None
-                trade.is_open = True
-                trade.open_order_id = None
-                logger.info('Sell order timeout for %s.', trade)
                 return True
-            else:
-                # TODO: figure out how to handle partially complete sell orders
-                pass
 
 
 def execute_sell(trade: Trade, limit: float) -> None:

freqtrade/tests/test_main.py

@@ -18,11 +18,11 @@ from freqtrade.misc import get_state, State
 from freqtrade.persistence import Trade
 import freqtrade.main as main
 
-
 # Test that main() can start backtesting or hyperopt.
 # and also ensure we can pass some specific arguments
 # argument parsing is done in test_misc.py
 
+
 def test_parse_args_backtesting(mocker):
     backtesting_mock = mocker.patch(
         'freqtrade.optimize.backtesting.start', MagicMock())
@@ -261,6 +261,20 @@ def test_create_trade_no_pairs_after_blacklist(default_conf, ticker, mocker):
         create_trade(default_conf['stake_amount'])
 
 
+def test_create_trade_no_signal(default_conf, ticker, mocker):
+    default_conf['dry_run'] = True
+    mocker.patch.dict('freqtrade.main._CONF', default_conf)
+    mocker.patch('freqtrade.main.get_signal', MagicMock(return_value=False))
+    mocker.patch.multiple('freqtrade.exchange',
+                          get_ticker_history=MagicMock(return_value=20))
+    mocker.patch.multiple('freqtrade.main.exchange',
+                          get_balance=MagicMock(return_value=20))
+    stake_amount = 10
+    Trade.query = MagicMock()
+    Trade.query.filter = MagicMock()
+    assert not create_trade(stake_amount)
+
+
 def test_handle_trade(default_conf, limit_buy_order, limit_sell_order, mocker):
     mocker.patch.dict('freqtrade.main._CONF', default_conf)
     mocker.patch('freqtrade.main.get_signal', side_effect=lambda s, t: True)
@@ -414,6 +428,21 @@ def test_check_handle_timedout_buy(default_conf, ticker, health, limit_buy_order
     assert len(trades) == 0
 
 
+def test_handle_timedout_limit_buy(default_conf, mocker):
+    cancel_order = MagicMock()
+    mocker.patch('freqtrade.exchange.cancel_order', cancel_order)
+    Trade.session = MagicMock()
+    trade = MagicMock()
+    order = {}
+    order['remaining'] = 1
+    order['amount'] = 1
+    assert main.handle_timedout_limit_buy(trade, order)
+    assert cancel_order.call_count == 1
+    order['amount'] = 2
+    assert not main.handle_timedout_limit_buy(trade, order)
+    assert cancel_order.call_count == 2
+
+
 def test_check_handle_timedout_sell(default_conf, ticker, health, limit_sell_order_old, mocker):
     mocker.patch.dict('freqtrade.main._CONF', default_conf)
     cancel_order_mock = MagicMock()
@@ -446,6 +475,21 @@ def test_check_handle_timedout_sell(default_conf, ticker, health, limit_sell_ord
     assert trade_sell.is_open is True
 
 
+def test_handle_timedout_limit_sell(default_conf, mocker):
+    cancel_order = MagicMock()
+    mocker.patch('freqtrade.exchange.cancel_order', cancel_order)
+    trade = MagicMock()
+    order = {}
+    order['remaining'] = 1
+    order['amount'] = 1
+    assert main.handle_timedout_limit_sell(trade, order)
+    assert cancel_order.call_count == 1
+    order['amount'] = 2
+    assert not main.handle_timedout_limit_sell(trade, order)
+    # Assert cancel_order was not called (callcount remains unchanged)
+    assert cancel_order.call_count == 1
+
+
 def test_check_handle_timedout_partial(default_conf, ticker, limit_buy_order_old_partial,
                                        health, mocker):
     mocker.patch.dict('freqtrade.main._CONF', default_conf)