Fix migrations, revert some parts related to amount properties

freqtrade/persistence/migrations.py

@@ -91,7 +91,8 @@ def migrate_trades_table(decl_base, inspector, engine, table_back_name: str, col
             stoploss_order_id, stoploss_last_update,
             max_rate, min_rate, sell_reason, sell_order_status, strategy,
             timeframe, open_trade_value, close_profit_abs,
-            leverage, borrowed, borrowed_currency, collateral_currency, interest_rate, liquidation_price, is_short
+            leverage, borrowed, borrowed_currency, collateral_currency, interest_rate,
+            liquidation_price, is_short
             )
         select id, lower(exchange),
             case
@@ -152,14 +153,17 @@ def migrate_orders_table(decl_base, inspector, engine, table_back_name: str, col
 
     # let SQLAlchemy create the schema as required
     decl_base.metadata.create_all(engine)
+    leverage = get_column_def(cols, 'leverage', 'null')
+    is_short = get_column_def(cols, 'is_short', 'False')
     with engine.begin() as connection:
         connection.execute(text(f"""
             insert into orders ( id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id,
             status, symbol, order_type, side, price, amount, filled, average, remaining, cost,
-            order_date, order_filled_date, order_update_date, leverage)
+            order_date, order_filled_date, order_update_date, leverage, is_short)
             select id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id,
             status, symbol, order_type, side, price, amount, filled, null average, remaining, cost,
-            order_date, order_filled_date, order_update_date, leverage
+            order_date, order_filled_date, order_update_date,
+            {leverage} leverage, {is_short} is_short
             from {table_back_name}
             """))
 
@@ -174,8 +178,9 @@ def check_migrate(engine, decl_base, previous_tables) -> None:
     tabs = get_table_names_for_table(inspector, 'trades')
     table_back_name = get_backup_name(tabs, 'trades_bak')
 
-    # Check for latest column
+    # Last added column of trades table
-    if not has_column(cols, 'open_trade_value'):
+    # To determine if migrations need to run
+    if not has_column(cols, 'collateral_currency'):
         logger.info(f'Running database migration for trades - backup: {table_back_name}')
         migrate_trades_table(decl_base, inspector, engine, table_back_name, cols)
         # Reread columns - the above recreated the table!
@@ -188,9 +193,11 @@ def check_migrate(engine, decl_base, previous_tables) -> None:
     else:
         cols_order = inspector.get_columns('orders')
 
-        if not has_column(cols_order, 'average'):
+        # Last added column of order table
+        # To determine if migrations need to run
+        if not has_column(cols_order, 'leverage'):
             tabs = get_table_names_for_table(inspector, 'orders')
             # Empty for now - as there is only one iteration of the orders table so far.
             table_back_name = get_backup_name(tabs, 'orders_bak')
 
-            migrate_orders_table(decl_base, inspector, engine, table_back_name, cols)
+            migrate_orders_table(decl_base, inspector, engine, table_back_name, cols_order)

freqtrade/persistence/models.py

@@ -234,7 +234,7 @@ class LocalTrade():
     close_profit: Optional[float] = None
     close_profit_abs: Optional[float] = None
     stake_amount: float = 0.0
-    _amount: float = 0.0
+    amount: float = 0.0
     amount_requested: Optional[float] = None
     open_date: datetime
     close_date: Optional[datetime] = None
@@ -266,8 +266,8 @@ class LocalTrade():
     interest_rate: float = 0.0
     liquidation_price: float = None
     is_short: bool = False
-    _borrowed: float = 0.0
+    borrowed: float = 0.0
-    _leverage: float = None  # * You probably want to use LocalTrade.leverage instead
+    leverage: float = None
 
     # @property
     # def base_currency(self) -> str:
@@ -275,42 +275,45 @@ class LocalTrade():
     #         raise OperationalException('LocalTrade.pair must be assigned')
     #     return self.pair.split("/")[1]
 
-    @property
+    # TODO: @samgermain: Amount should be persisted "as is".
-    def amount(self) -> float:
+    # I've partially reverted this (this killed most of your tests)
-        if self._leverage is not None:
+    # but leave this here as i'm not sure where you intended to use this.
-            return self._amount * self.leverage
+    # @property
-        else:
+    # def amount(self) -> float:
-            return self._amount
+    #     if self._leverage is not None:
+    #         return self._amount * self.leverage
+    #     else:
+    #         return self._amount
 
-    @amount.setter
+    # @amount.setter
-    def amount(self, value):
+    # def amount(self, value):
-        self._amount = value
+    #     self._amount = value
 
-    @property
+    # @property
-    def borrowed(self) -> float:
+    # def borrowed(self) -> float:
-        if self._leverage is not None:
+    #     if self._leverage is not None:
-            if self.is_short:
+    #         if self.is_short:
-                # If shorting the full amount must be borrowed
+    #             # If shorting the full amount must be borrowed
-                return self._amount * self._leverage
+    #             return self._amount * self._leverage
-            else:
+    #         else:
-                # If not shorting, then the trader already owns a bit
+    #             # If not shorting, then the trader already owns a bit
-                return self._amount * (self._leverage-1)
+    #             return self._amount * (self._leverage-1)
-        else:
+    #     else:
-            return self._borrowed
+    #         return self._borrowed
 
-    @borrowed.setter
+    # @borrowed.setter
-    def borrowed(self, value):
+    # def borrowed(self, value):
-        self._borrowed = value
+    #     self._borrowed = value
-        self._leverage = None
+    #     self._leverage = None
 
-    @property
+    # @property
-    def leverage(self) -> float:
+    # def leverage(self) -> float:
-        return self._leverage
+    #     return self._leverage
 
-    @leverage.setter
+    # @leverage.setter
-    def leverage(self, value):
+    # def leverage(self, value):
-        self._leverage = value
+    #     self._leverage = value
-        self._borrowed = None
+    #     self._borrowed = None
 
     # End of margin trading properties
 
@@ -877,7 +880,7 @@ class Trade(_DECL_BASE, LocalTrade):
     close_profit = Column(Float)
     close_profit_abs = Column(Float)
     stake_amount = Column(Float, nullable=False)
-    _amount = Column(Float)
+    amount = Column(Float)
     amount_requested = Column(Float)
     open_date = Column(DateTime, nullable=False, default=datetime.utcnow)
     close_date = Column(DateTime)
@@ -904,8 +907,8 @@ class Trade(_DECL_BASE, LocalTrade):
     timeframe = Column(Integer, nullable=True)
 
     # Margin trading properties
-    _leverage: float = None  # * You probably want to use LocalTrade.leverage instead
+    leverage = Column(Float, nullable=True)   # TODO: can this be nullable, or should it default to 1? (must also be changed in migrations eventually)
-    _borrowed = Column(Float, nullable=False, default=0.0)
+    borrowed = Column(Float, nullable=False, default=0.0)
     interest_rate = Column(Float, nullable=False, default=0.0)
     liquidation_price = Column(Float, nullable=True)
     is_short = Column(Boolean, nullable=False, default=False)

tests/test_persistence.py

@@ -723,13 +723,11 @@ def test_migrate_new(mocker, default_conf, fee, caplog):
                 order_date DATETIME,
                 order_filled_date DATETIME,
                 order_update_date DATETIME,
-                leverage FLOAT,
                 PRIMARY KEY (id),
                 CONSTRAINT _order_pair_order_id UNIQUE (ft_pair, order_id),
                 FOREIGN KEY(ft_trade_id) REFERENCES trades (id)
             )
             """))
-        # TODO-mg @xmatthias: Had to add field leverage to this table, check that this is correct
         connection.execute(text("""
         insert into orders ( id, ft_trade_id, ft_order_side, ft_pair, ft_is_open, order_id, status,
             symbol, order_type, side, price, amount, filled, remaining, cost, order_date,
@@ -752,6 +750,7 @@ def test_migrate_new(mocker, default_conf, fee, caplog):
 
     assert orders[1].order_id == 'stop_order_id222'
     assert orders[1].ft_order_side == 'stoploss'
+    assert orders[0].is_short is False
 
 
 def test_migrate_mid_state(mocker, default_conf, fee, caplog):