From 4fa736114c1321e274803a4235d08c7e684f0043 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 15:30:22 +0200 Subject: [PATCH 01/10] Don't set order_id to none here - it's used in "update_open_order". should fix bugs observed in #1371 connected to stoploss --- freqtrade/freqtradebot.py | 1 - 1 file changed, 1 deletion(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 6f1fb2c99..291b74913 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -472,7 +472,6 @@ class FreqtradeBot(object): stake_amount = order['cost'] amount = order['amount'] buy_limit_filled_price = order['price'] - order_id = None self.rpc.send_msg({ 'type': RPCMessageType.BUY_NOTIFICATION, From 8f4cca47e9a31d08015bc0093d252b4a202f80e5 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 15:39:41 +0200 Subject: [PATCH 02/10] Refactor update_open_order into it's own function --- freqtrade/freqtradebot.py | 41 ++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 291b74913..f78a9078e 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -530,24 +530,7 @@ class FreqtradeBot(object): :return: True if executed """ try: - # Get order details for actual price per unit - if trade.open_order_id: - # Update trade with order values - logger.info('Found open order for %s', trade) - order = self.exchange.get_order(trade.open_order_id, trade.pair) - # Try update amount (binance-fix) - try: - new_amount = self.get_real_amount(trade, order) - if order['amount'] != new_amount: - order['amount'] = new_amount - # Fee was applied, so set to 0 - trade.fee_open = 0 - - except OperationalException as exception: - logger.warning("Could not update trade amount: %s", exception) - - # This handles both buy and sell orders! - trade.update(order) + self.update_open_order(trade) if self.strategy.order_types.get('stoploss_on_exchange') and trade.is_open: result = self.handle_stoploss_on_exchange(trade) @@ -612,6 +595,28 @@ class FreqtradeBot(object): f"(from {order_amount} to {real_amount}) from Trades") return real_amount + def update_open_order(self, trade, action_order: dict = None): + """ + Checks trades with open orders and updates the amount if necessary + """ + # Get order details for actual price per unit + if trade.open_order_id: + # Update trade with order values + logger.info('Found open order for %s', trade) + order = action_order or self.exchange.get_order(trade.open_order_id, trade.pair) + # Try update amount (binance-fix) + try: + new_amount = self.get_real_amount(trade, order) + if order['amount'] != new_amount: + order['amount'] = new_amount + # Fee was applied, so set to 0 + trade.fee_open = 0 + + except OperationalException as exception: + logger.warning("Could not update trade amount: %s", exception) + + trade.update(order) + def get_sell_rate(self, pair: str, refresh: bool) -> float: """ Get sell rate - either using get-ticker bid or first bid based on orderbook From f11a1b01228801dd5ea5a6877c2f819b2564d236 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 15:40:16 +0200 Subject: [PATCH 03/10] Call update_open_order inline with buy captures FOK / market orders --- freqtrade/freqtradebot.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index f78a9078e..74c74324f 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -500,6 +500,10 @@ class FreqtradeBot(object): ticker_interval=constants.TICKER_INTERVAL_MINUTES[self.config['ticker_interval']] ) + # Update fees if order is closed already. + if order_status == 'closed': + self.update_open_order(trade, order) + Trade.session.add(trade) Trade.session.flush() From 5c8fbe2c6ff44d36322f3268deabdbe29c6e8779 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 15:41:10 +0200 Subject: [PATCH 04/10] Handle exception for stoploss independently of sell order --- freqtrade/freqtradebot.py | 67 ++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 74c74324f..a06e146f9 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -691,43 +691,44 @@ class FreqtradeBot(object): """ result = False + try: + # If trade is open and the buy order is fulfilled but there is no stoploss, + # then we add a stoploss on exchange + if not trade.open_order_id and not trade.stoploss_order_id: + if self.edge: + stoploss = self.edge.stoploss(pair=trade.pair) + else: + stoploss = self.strategy.stoploss - # If trade is open and the buy order is fulfilled but there is no stoploss, - # then we add a stoploss on exchange - if not trade.open_order_id and not trade.stoploss_order_id: - if self.edge: - stoploss = self.edge.stoploss(pair=trade.pair) - else: - stoploss = self.strategy.stoploss + stop_price = trade.open_rate * (1 + stoploss) - stop_price = trade.open_rate * (1 + stoploss) + # limit price should be less than stop price. + # 0.99 is arbitrary here. + limit_price = stop_price * 0.99 - # limit price should be less than stop price. - # 0.99 is arbitrary here. - limit_price = stop_price * 0.99 - - stoploss_order_id = self.exchange.stoploss_limit( - pair=trade.pair, amount=trade.amount, stop_price=stop_price, rate=limit_price - )['id'] - trade.stoploss_order_id = str(stoploss_order_id) - trade.stoploss_last_update = datetime.now() - - # Or the trade open and there is already a stoploss on exchange. - # so we check if it is hit ... - elif trade.stoploss_order_id: - logger.debug('Handling stoploss on exchange %s ...', trade) - order = self.exchange.get_order(trade.stoploss_order_id, trade.pair) - if order['status'] == 'closed': - trade.sell_reason = SellType.STOPLOSS_ON_EXCHANGE.value - trade.update(order) - self.notify_sell(trade) - result = True - elif self.config.get('trailing_stop', False): - # if trailing stoploss is enabled we check if stoploss value has changed - # in which case we cancel stoploss order and put another one with new - # value immediately - self.handle_trailing_stoploss_on_exchange(trade, order) + stoploss_order_id = self.exchange.stoploss_limit( + pair=trade.pair, amount=trade.amount, stop_price=stop_price, rate=limit_price + )['id'] + trade.stoploss_order_id = str(stoploss_order_id) + trade.stoploss_last_update = datetime.now() + # Or the trade open and there is already a stoploss on exchange. + # so we check if it is hit ... + elif trade.stoploss_order_id: + logger.debug('Handling stoploss on exchange %s ...', trade) + order = self.exchange.get_order(trade.stoploss_order_id, trade.pair) + if order['status'] == 'closed': + trade.sell_reason = SellType.STOPLOSS_ON_EXCHANGE.value + trade.update(order) + self.notify_sell(trade) + result = True + elif self.config.get('trailing_stop', False): + # if trailing stoploss is enabled we check if stoploss value has changed + # in which case we cancel stoploss order and put another one with new + # value immediately + self.handle_trailing_stoploss_on_exchange(trade, order) + except DependencyException as exception: + logger.warning('Unable to create stoploss order: %s', exception) return result def handle_trailing_stoploss_on_exchange(self, trade: Trade, order): From e46dac3fbd2846d7a22d1c54ca9ff1a64368d2e8 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 15:45:22 +0200 Subject: [PATCH 05/10] Test stoploss does not raise dependencyexception --- freqtrade/tests/test_freqtradebot.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/freqtrade/tests/test_freqtradebot.py b/freqtrade/tests/test_freqtradebot.py index 5bf0bfcbb..e53d26793 100644 --- a/freqtrade/tests/test_freqtradebot.py +++ b/freqtrade/tests/test_freqtradebot.py @@ -1031,6 +1031,13 @@ def test_handle_stoploss_on_exchange(mocker, default_conf, fee, caplog, assert trade.stoploss_order_id is None assert trade.is_open is False + mocker.patch( + 'freqtrade.exchange.Exchange.stoploss_limit', + side_effect=DependencyException() + ) + freqtrade.handle_stoploss_on_exchange(trade) + assert log_has('Unable to create stoploss order: ', caplog.record_tuples) + def test_handle_stoploss_on_exchange_trailing(mocker, default_conf, fee, caplog, markets, limit_buy_order, limit_sell_order) -> None: From b2ad402df4ec8b6a4e35a2abaf368b09fce1bda8 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 15:51:45 +0200 Subject: [PATCH 06/10] Split tests for update-open_order --- freqtrade/tests/test_freqtradebot.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/freqtrade/tests/test_freqtradebot.py b/freqtrade/tests/test_freqtradebot.py index e53d26793..38a6df695 100644 --- a/freqtrade/tests/test_freqtradebot.py +++ b/freqtrade/tests/test_freqtradebot.py @@ -1306,22 +1306,32 @@ def test_process_maybe_execute_sell_exception(mocker, default_conf, trade.open_order_id = '123' trade.open_fee = 0.001 + # Test raise of DependencyException exception + mocker.patch( + 'freqtrade.freqtradebot.FreqtradeBot.update_open_order', + side_effect=DependencyException() + ) + freqtrade.process_maybe_execute_sell(trade) + assert log_has('Unable to sell trade: ', caplog.record_tuples) + + +def test_update_open_order_exception(mocker, default_conf, + limit_buy_order, caplog) -> None: + freqtrade = get_patched_freqtradebot(mocker, default_conf) + mocker.patch('freqtrade.exchange.Exchange.get_order', return_value=limit_buy_order) + + trade = MagicMock() + trade.open_order_id = '123' + trade.open_fee = 0.001 + # Test raise of OperationalException exception mocker.patch( 'freqtrade.freqtradebot.FreqtradeBot.get_real_amount', side_effect=OperationalException() ) - freqtrade.process_maybe_execute_sell(trade) + freqtrade.update_open_order(trade) assert log_has('Could not update trade amount: ', caplog.record_tuples) - # Test raise of DependencyException exception - mocker.patch( - 'freqtrade.freqtradebot.FreqtradeBot.get_real_amount', - side_effect=DependencyException() - ) - freqtrade.process_maybe_execute_sell(trade) - assert log_has('Unable to sell trade: ', caplog.record_tuples) - def test_handle_trade(default_conf, limit_buy_order, limit_sell_order, fee, markets, mocker) -> None: From 0ddafeeabf1322fa679a1c56f77ed709cd3ac4bf Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 16:05:00 +0200 Subject: [PATCH 07/10] Split test for open_orders from maybe_sell --- freqtrade/tests/test_freqtradebot.py | 39 +++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/freqtrade/tests/test_freqtradebot.py b/freqtrade/tests/test_freqtradebot.py index 38a6df695..a3b01c542 100644 --- a/freqtrade/tests/test_freqtradebot.py +++ b/freqtrade/tests/test_freqtradebot.py @@ -1291,7 +1291,7 @@ def test_process_maybe_execute_sell(mocker, default_conf, limit_buy_order, caplo trade.is_open = True trade.open_order_id = None # Assert we call handle_trade() if trade is feasible for execution - assert freqtrade.process_maybe_execute_sell(trade) + freqtrade.update_open_order(trade) regexp = re.compile('Found open order for.*') assert filter(regexp.match, caplog.record_tuples) @@ -1315,6 +1315,43 @@ def test_process_maybe_execute_sell_exception(mocker, default_conf, assert log_has('Unable to sell trade: ', caplog.record_tuples) +def test_update_open_order(mocker, default_conf, limit_buy_order, caplog) -> None: + freqtrade = get_patched_freqtradebot(mocker, default_conf) + + mocker.patch('freqtrade.freqtradebot.FreqtradeBot.handle_trade', MagicMock(return_value=True)) + mocker.patch('freqtrade.exchange.Exchange.get_order', return_value=limit_buy_order) + mocker.patch('freqtrade.exchange.Exchange.get_trades_for_order', return_value=[]) + mocker.patch('freqtrade.freqtradebot.FreqtradeBot.get_real_amount', + return_value=limit_buy_order['amount']) + + trade = Trade() + # Mock session away + Trade.session = MagicMock() + trade.open_order_id = '123' + trade.open_fee = 0.001 + freqtrade.update_open_order(trade) + # Test amount not modified by fee-logic + assert not log_has_re(r'Applying fee to .*', caplog.record_tuples) + assert trade.open_order_id is None + assert trade.amount == limit_buy_order['amount'] + + trade.open_order_id = '123' + mocker.patch('freqtrade.freqtradebot.FreqtradeBot.get_real_amount', return_value=90.81) + assert trade.amount != 90.81 + # test amount modified by fee-logic + freqtrade.update_open_order(trade) + assert trade.amount == 90.81 + assert trade.open_order_id is None + + trade.is_open = True + trade.open_order_id = None + # Assert we call handle_trade() if trade is feasible for execution + freqtrade.update_open_order(trade) + + regexp = re.compile('Found open order for.*') + assert filter(regexp.match, caplog.record_tuples) + + def test_update_open_order_exception(mocker, default_conf, limit_buy_order, caplog) -> None: freqtrade = get_patched_freqtradebot(mocker, default_conf) From 19d3a0cbacd21b58589abc4c5100fe0b335bb69f Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 19:41:17 +0200 Subject: [PATCH 08/10] Update comment --- freqtrade/freqtradebot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index a06e146f9..5b1e5625d 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -500,7 +500,7 @@ class FreqtradeBot(object): ticker_interval=constants.TICKER_INTERVAL_MINUTES[self.config['ticker_interval']] ) - # Update fees if order is closed already. + # Update fees if order is closed if order_status == 'closed': self.update_open_order(trade, order) From 7be90f71d379b235d2cbfa507b5d528bf42756fc Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 31 Mar 2019 19:56:01 +0200 Subject: [PATCH 09/10] Add test as called from execute_buy --- freqtrade/tests/test_freqtradebot.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/freqtrade/tests/test_freqtradebot.py b/freqtrade/tests/test_freqtradebot.py index a3b01c542..af8fc3b09 100644 --- a/freqtrade/tests/test_freqtradebot.py +++ b/freqtrade/tests/test_freqtradebot.py @@ -1352,6 +1352,26 @@ def test_update_open_order(mocker, default_conf, limit_buy_order, caplog) -> Non assert filter(regexp.match, caplog.record_tuples) +def test_update_open_order_withorderdict(default_conf, trades_for_order, limit_buy_order, mocker): + mocker.patch('freqtrade.exchange.Exchange.get_trades_for_order', return_value=trades_for_order) + # get_order should not be called!! + mocker.patch('freqtrade.exchange.Exchange.get_order', MagicMock(side_effect=ValueError)) + patch_exchange(mocker) + Trade.session = MagicMock() + amount = sum(x['amount'] for x in trades_for_order) + freqtrade = get_patched_freqtradebot(mocker, default_conf) + trade = Trade( + pair='LTC/ETH', + amount=amount, + exchange='binance', + open_rate=0.245441, + open_order_id="123456" + ) + freqtrade.update_open_order(trade, limit_buy_order) + assert trade.amount != amount + assert trade.amount == limit_buy_order['amount'] + + def test_update_open_order_exception(mocker, default_conf, limit_buy_order, caplog) -> None: freqtrade = get_patched_freqtradebot(mocker, default_conf) From 0cfdce0d5e69a8b79c95bbac839ed37583dcd9b8 Mon Sep 17 00:00:00 2001 From: Matthias Date: Tue, 2 Apr 2019 07:12:48 +0200 Subject: [PATCH 10/10] Update function name from update_open_order to update_trade_state --- freqtrade/freqtradebot.py | 6 +++--- freqtrade/tests/test_freqtradebot.py | 28 ++++++++++------------------ 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 5b1e5625d..55ef6f611 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -502,7 +502,7 @@ class FreqtradeBot(object): # Update fees if order is closed if order_status == 'closed': - self.update_open_order(trade, order) + self.update_trade_state(trade, order) Trade.session.add(trade) Trade.session.flush() @@ -534,7 +534,7 @@ class FreqtradeBot(object): :return: True if executed """ try: - self.update_open_order(trade) + self.update_trade_state(trade) if self.strategy.order_types.get('stoploss_on_exchange') and trade.is_open: result = self.handle_stoploss_on_exchange(trade) @@ -599,7 +599,7 @@ class FreqtradeBot(object): f"(from {order_amount} to {real_amount}) from Trades") return real_amount - def update_open_order(self, trade, action_order: dict = None): + def update_trade_state(self, trade, action_order: dict = None): """ Checks trades with open orders and updates the amount if necessary """ diff --git a/freqtrade/tests/test_freqtradebot.py b/freqtrade/tests/test_freqtradebot.py index af8fc3b09..416a085ad 100644 --- a/freqtrade/tests/test_freqtradebot.py +++ b/freqtrade/tests/test_freqtradebot.py @@ -1288,14 +1288,6 @@ def test_process_maybe_execute_sell(mocker, default_conf, limit_buy_order, caplo # test amount modified by fee-logic assert not freqtrade.process_maybe_execute_sell(trade) - trade.is_open = True - trade.open_order_id = None - # Assert we call handle_trade() if trade is feasible for execution - freqtrade.update_open_order(trade) - - regexp = re.compile('Found open order for.*') - assert filter(regexp.match, caplog.record_tuples) - def test_process_maybe_execute_sell_exception(mocker, default_conf, limit_buy_order, caplog) -> None: @@ -1308,14 +1300,14 @@ def test_process_maybe_execute_sell_exception(mocker, default_conf, # Test raise of DependencyException exception mocker.patch( - 'freqtrade.freqtradebot.FreqtradeBot.update_open_order', + 'freqtrade.freqtradebot.FreqtradeBot.update_trade_state', side_effect=DependencyException() ) freqtrade.process_maybe_execute_sell(trade) assert log_has('Unable to sell trade: ', caplog.record_tuples) -def test_update_open_order(mocker, default_conf, limit_buy_order, caplog) -> None: +def test_update_trade_state(mocker, default_conf, limit_buy_order, caplog) -> None: freqtrade = get_patched_freqtradebot(mocker, default_conf) mocker.patch('freqtrade.freqtradebot.FreqtradeBot.handle_trade', MagicMock(return_value=True)) @@ -1329,7 +1321,7 @@ def test_update_open_order(mocker, default_conf, limit_buy_order, caplog) -> Non Trade.session = MagicMock() trade.open_order_id = '123' trade.open_fee = 0.001 - freqtrade.update_open_order(trade) + freqtrade.update_trade_state(trade) # Test amount not modified by fee-logic assert not log_has_re(r'Applying fee to .*', caplog.record_tuples) assert trade.open_order_id is None @@ -1339,20 +1331,20 @@ def test_update_open_order(mocker, default_conf, limit_buy_order, caplog) -> Non mocker.patch('freqtrade.freqtradebot.FreqtradeBot.get_real_amount', return_value=90.81) assert trade.amount != 90.81 # test amount modified by fee-logic - freqtrade.update_open_order(trade) + freqtrade.update_trade_state(trade) assert trade.amount == 90.81 assert trade.open_order_id is None trade.is_open = True trade.open_order_id = None # Assert we call handle_trade() if trade is feasible for execution - freqtrade.update_open_order(trade) + freqtrade.update_trade_state(trade) regexp = re.compile('Found open order for.*') assert filter(regexp.match, caplog.record_tuples) -def test_update_open_order_withorderdict(default_conf, trades_for_order, limit_buy_order, mocker): +def test_update_trade_state_withorderdict(default_conf, trades_for_order, limit_buy_order, mocker): mocker.patch('freqtrade.exchange.Exchange.get_trades_for_order', return_value=trades_for_order) # get_order should not be called!! mocker.patch('freqtrade.exchange.Exchange.get_order', MagicMock(side_effect=ValueError)) @@ -1367,13 +1359,13 @@ def test_update_open_order_withorderdict(default_conf, trades_for_order, limit_b open_rate=0.245441, open_order_id="123456" ) - freqtrade.update_open_order(trade, limit_buy_order) + freqtrade.update_trade_state(trade, limit_buy_order) assert trade.amount != amount assert trade.amount == limit_buy_order['amount'] -def test_update_open_order_exception(mocker, default_conf, - limit_buy_order, caplog) -> None: +def test_update_trade_state_exception(mocker, default_conf, + limit_buy_order, caplog) -> None: freqtrade = get_patched_freqtradebot(mocker, default_conf) mocker.patch('freqtrade.exchange.Exchange.get_order', return_value=limit_buy_order) @@ -1386,7 +1378,7 @@ def test_update_open_order_exception(mocker, default_conf, 'freqtrade.freqtradebot.FreqtradeBot.get_real_amount', side_effect=OperationalException() ) - freqtrade.update_open_order(trade) + freqtrade.update_trade_state(trade) assert log_has('Could not update trade amount: ', caplog.record_tuples)