From f5d7deedf4d025ba972deb1fc7c19414ef43b1ab Mon Sep 17 00:00:00 2001 From: Sam Germain Date: Sun, 27 Jun 2021 00:19:58 -0600 Subject: [PATCH] added exception checks to LocalTrade.leverage and LocalTrade.borrowed --- docs/leverage.md | 10 ++++++++ freqtrade/persistence/models.py | 44 +++++++++++++++----------------- tests/conftest.py | 6 +++-- tests/test_persistence_margin.py | 22 +++++++++------- 4 files changed, 48 insertions(+), 34 deletions(-) create mode 100644 docs/leverage.md diff --git a/docs/leverage.md b/docs/leverage.md new file mode 100644 index 000000000..658146c6f --- /dev/null +++ b/docs/leverage.md @@ -0,0 +1,10 @@ +An instance of a `Trade`/`LocalTrade` object is given either a value for `leverage` or a value for `borrowed`, but not both, on instantiation/update with a short/long. + +- If given a value for `leverage`, then the `amount` value of the `Trade`/`Local` object is multiplied by the `leverage` value to obtain the new value for `amount`. The borrowed value is also calculated from the `amount` and `leverage` value +- If given a value for `borrowed`, then the `leverage` value is calculated from `borrowed` and `amount` + +For shorts, the currency which pays the interest fee for the `borrowed` currency is purchased at the same time of the closing trade (This means that the amount purchased in short closing trades is greater than the amount sold in short opening trades). + +For longs, the currency which pays the interest fee for the `borrowed` will already be owned by the user and does not need to be purchased + +The interest fee is paid following the closing trade, or simultaneously depending on the exchange diff --git a/freqtrade/persistence/models.py b/freqtrade/persistence/models.py index b56876c9d..dd81faa17 100644 --- a/freqtrade/persistence/models.py +++ b/freqtrade/persistence/models.py @@ -268,9 +268,9 @@ class LocalTrade(): collateral_currency: str = None interest_rate: float = 0.0 liquidation_price: float = None + is_short: bool = False __leverage: float = 1.0 # * You probably want to use self.leverage instead | - __borrowed: float = 0.0 # * You probably want to use self.borrowed instead | - __is_short: bool = False # * You probably want to use self.is_short instead V + __borrowed: float = 0.0 # * You probably want to use self.borrowed instead V @property def leverage(self) -> float: @@ -280,24 +280,22 @@ class LocalTrade(): def borrowed(self) -> float: return self.__borrowed or 0.0 - @property - def is_short(self) -> bool: - return self.__is_short or False - - @is_short.setter - def is_short(self, val: bool): - self.__is_short = val - @leverage.setter def leverage(self, lev: float): + if self.is_short is None or self.amount is None: + raise OperationalException( + 'LocalTrade.amount and LocalTrade.is_short must be assigned before LocalTrade.leverage') self.__leverage = lev self.__borrowed = self.amount * (lev-1) self.amount = self.amount * lev @borrowed.setter def borrowed(self, bor: float): - self.__leverage = self.amount / (self.amount - self.borrowed) + if not self.amount: + raise OperationalException( + 'LocalTrade.amount must be assigned before LocalTrade.borrowed') self.__borrowed = bor + self.__leverage = self.amount / (self.amount - self.borrowed) # End of margin trading properties @property @@ -314,6 +312,8 @@ class LocalTrade(): raise OperationalException('Cannot pass both borrowed and leverage to Trade') for key in kwargs: setattr(self, key, kwargs[key]) + if not self.is_short: + self.is_short = False self.recalc_open_trade_value() def __repr__(self): @@ -464,16 +464,14 @@ class LocalTrade(): Determines if the trade is an opening (long buy or short sell) trade :param side (string): the side (buy/sell) that order happens on """ - is_short = self.is_short or False - return (side == 'buy' and not is_short) or (side == 'sell' and is_short) + return (side == 'buy' and not self.is_short) or (side == 'sell' and self.is_short) def is_closing_trade(self, side) -> bool: """ Determines if the trade is an closing (long sell or short buy) trade :param side (string): the side (buy/sell) that order happens on """ - is_short = self.is_short or False - return (side == 'sell' and not is_short) or (side == 'buy' and is_short) + return (side == 'sell' and not self.is_short) or (side == 'buy' and self.is_short) def update(self, order: Dict) -> None: """ @@ -483,11 +481,13 @@ class LocalTrade(): """ order_type = order['type'] - # if ('leverage' in order and 'borrowed' in order): - # raise OperationalException('Cannot update a trade with both borrowed and leverage') + if ('leverage' in order and 'borrowed' in order): + raise OperationalException( + 'Pass only one of Leverage or Borrowed to the order in update trade') - # TODO: I don't like this, but it might be the only way if 'is_short' in order and order['side'] == 'sell': + # Only set's is_short on opening trades, ignores non-shorts + # TODO-mg: I don't like this, but it might be the only way self.is_short = order['is_short'] # Ignore open and cancelled orders @@ -499,9 +499,6 @@ class LocalTrade(): if order_type in ('market', 'limit') and self.is_opening_trade(order['side']): # Update open rate and actual amount - # self.is_short = safe_value_fallback(order, 'is_short', default_value=False) - # self.borrowed = safe_value_fallback(order, 'is_short', default_value=False) - self.open_rate = float(safe_value_fallback(order, 'average', 'price')) self.amount = float(safe_value_fallback(order, 'filled', 'amount')) if 'borrowed' in order: @@ -654,7 +651,8 @@ class LocalTrade(): if self.is_short: amount = Decimal(self.amount) + interest else: - amount = Decimal(self.amount) - interest + # The interest does not need to be purchased on longs because the user already owns that currency in your wallet + amount = Decimal(self.amount) close_trade = Decimal(amount) * Decimal(rate or self.close_rate) # type: ignore fees = close_trade * Decimal(fee or self.fee_close) @@ -662,7 +660,7 @@ class LocalTrade(): if (self.is_short): return float(close_trade + fees) else: - return float(close_trade - fees) + return float(close_trade - fees - interest) def calc_profit(self, rate: Optional[float] = None, fee: Optional[float] = None) -> float: diff --git a/tests/conftest.py b/tests/conftest.py index 362fb8b33..3d62a33e2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2132,6 +2132,7 @@ def limit_exit_short_order(limit_exit_short_order_open): order['status'] = 'closed' return order + @pytest.fixture(scope='function') def market_short_order(): return { @@ -2162,11 +2163,12 @@ def market_exit_short_order(): 'amount': 91.99181073, 'filled': 91.99181073, 'remaining': 0.0, - 'status': 'closed' + 'status': 'closed', + 'leverage': 3, + 'interest_rate': 0.0005 } - @pytest.fixture def interest_rate(): return MagicMock(return_value=0.0005) diff --git a/tests/test_persistence_margin.py b/tests/test_persistence_margin.py index bbca52e50..94deb4a36 100644 --- a/tests/test_persistence_margin.py +++ b/tests/test_persistence_margin.py @@ -101,8 +101,14 @@ def test_update_with_binance(limit_short_order, limit_exit_short_order, fee, int # caplog # ): # """Test Kraken and leverage arguments as well as update market order - - +# fee: 0.25% +# interest_rate: 0.05% per 4 hrs +# open_rate: 0.00004173 +# close_rate: 0.00004099 +# amount: 91.99181073 * leverage(3) = 275.97543219 +# borrowed: 183.98362146 +# time: 10 minutes(rounds to min of 4hrs) +# interest # """ # trade = Trade( # id=1, @@ -114,27 +120,25 @@ def test_update_with_binance(limit_short_order, limit_exit_short_order, fee, int # fee_open=fee.return_value, # fee_close=fee.return_value, # open_date=ten_minutes_ago, -# exchange='kraken', -# interest_rate=interest_rate.return_value +# exchange='kraken' # ) # trade.open_order_id = 'something' # trade.update(market_buy_order) # assert trade.leverage is 3 -# assert trade.is_short is true -# assert trade.leverage is 3 +# assert trade.is_short is True # assert trade.open_order_id is None -# assert trade.open_rate == 0.00004099 +# assert trade.open_rate == 0.00004173 # assert trade.close_profit is None # assert trade.close_date is None # assert log_has_re(r"MARKET_BUY has been fulfilled for Trade\(id=1, " -# r"pair=ETH/BTC, amount=91.99181073, open_rate=0.00004099, open_since=.*\).", +# r"pair=ETH/BTC, amount=275.97543219, open_rate=0.00004173, open_since=.*\).", # caplog) # caplog.clear() # trade.is_open = True # trade.open_order_id = 'something' # trade.update(market_sell_order) # assert trade.open_order_id is None -# assert trade.close_rate == 0.00004173 +# assert trade.close_rate == 0.00004099 # assert trade.close_profit == 0.01297561 # assert trade.close_date is not None # assert log_has_re(r"MARKET_SELL has been fulfilled for Trade\(id=1, "