Merge pull request #1720 from freqtrade/fix/fee_not_adjusted

Fix/fee not adjusted

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,
@@ -501,6 +500,10 @@ class FreqtradeBot(object):
             ticker_interval=constants.TICKER_INTERVAL_MINUTES[self.config['ticker_interval']]
         )
 
+        # Update fees if order is closed
+        if order_status == 'closed':
+            self.update_trade_state(trade, order)
+
         Trade.session.add(trade)
         Trade.session.flush()
 
@@ -531,24 +534,7 @@ class FreqtradeBot(object):
         :return: True if executed
         """
         try:
-            # Get order details for actual price per unit
+            self.update_trade_state(trade)
-            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)
 
             if self.strategy.order_types.get('stoploss_on_exchange') and trade.is_open:
                 result = self.handle_stoploss_on_exchange(trade)
@@ -613,6 +599,28 @@ class FreqtradeBot(object):
                         f"(from {order_amount} to {real_amount}) from Trades")
         return real_amount
 
+    def update_trade_state(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
@@ -683,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,
+                stop_price = trade.open_rate * (1 + 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)
+                # 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.
+                stoploss_order_id = self.exchange.stoploss_limit(
-            # 0.99 is arbitrary here.
+                    pair=trade.pair, amount=trade.amount, stop_price=stop_price, rate=limit_price
-            limit_price = stop_price * 0.99
+                )['id']
-
+                trade.stoploss_order_id = str(stoploss_order_id)
-            stoploss_order_id = self.exchange.stoploss_limit(
+                trade.stoploss_last_update = datetime.now()
-                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)
 
+            # 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):

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:
@@ -1281,17 +1288,84 @@ 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)
 
+
+def test_process_maybe_execute_sell_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 DependencyException exception
+    mocker.patch(
+        '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_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))
+    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_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
+    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_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
-    assert freqtrade.process_maybe_execute_sell(trade)
+    freqtrade.update_trade_state(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,
+def test_update_trade_state_withorderdict(default_conf, trades_for_order, limit_buy_order, mocker):
-                                              limit_buy_order, caplog) -> None:
+    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_trade_state(trade, limit_buy_order)
+    assert trade.amount != amount
+    assert trade.amount == limit_buy_order['amount']
+
+
+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)
 
@@ -1304,17 +1378,9 @@ def test_process_maybe_execute_sell_exception(mocker, default_conf,
         'freqtrade.freqtradebot.FreqtradeBot.get_real_amount',
         side_effect=OperationalException()
     )
-    freqtrade.process_maybe_execute_sell(trade)
+    freqtrade.update_trade_state(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: