From 186f7d816a6bfc4fbf64027c3d7ecea10b97db38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Bouya?= Date: Mon, 12 Mar 2018 01:55:17 +0100 Subject: Fix fullfiled not having correct currencies --- portfolio.py | 17 +++++++------- test.py | 75 +++++++++++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/portfolio.py b/portfolio.py index 0f2c011..65fdc6c 100644 --- a/portfolio.py +++ b/portfolio.py @@ -291,6 +291,7 @@ class Trade: self.orders = [] self.market = market self.closed = False + self.inverted = None assert self.value_from.value * self.value_to.value >= 0 assert self.value_from.currency == self.value_to.currency if self.value_from != 0: @@ -315,8 +316,8 @@ class Trade: else: return "dispose" - def order_action(self, inverted): - if (self.value_from < self.value_to) != inverted: + def order_action(self): + if (self.value_from < self.value_to) != self.inverted: return "buy" else: return "sell" @@ -339,7 +340,7 @@ class Trade: @property def is_fullfiled(self): - return abs(self.filled_amount(in_base_currency=True)) >= abs(self.delta) + return abs(self.filled_amount(in_base_currency=(not self.inverted))) >= abs(self.delta) def filled_amount(self, in_base_currency=False): filled_amount = 0 @@ -385,17 +386,17 @@ class Trade: if self.action is None: return None ticker = self.market.get_ticker(self.currency, self.base_currency) - inverted = ticker["inverted"] - if inverted: + self.inverted = ticker["inverted"] + if self.inverted: ticker = ticker["original"] - rate = Computation.compute_value(ticker, self.order_action(inverted), compute_value=compute_value) + rate = Computation.compute_value(ticker, self.order_action(), compute_value=compute_value) # FIXME: Dust amount should be removed from there if they werent # honored in other sales delta_in_base = abs(self.delta) # 9 BTC's worth of move (10 - 1 or 1 - 10 depending on case) - if not inverted: + if not self.inverted: base_currency = self.base_currency # BTC if self.action == "dispose": @@ -453,7 +454,7 @@ class Trade: self.market.report.log_error("prepare_order", message="Less to do than already filled: {}".format(delta)) return None - order = Order(self.order_action(inverted), + order = Order(self.order_action(), delta, rate, base_currency, self.trade_type, self.market, self, close_if_possible=close_if_possible) self.orders.append(order) diff --git a/test.py b/test.py index a45010b..33a817d 100644 --- a/test.py +++ b/test.py @@ -1375,16 +1375,20 @@ class TradeTest(WebMockTestCase): value_to = portfolio.Amount("BTC", "1.0") trade = portfolio.Trade(value_from, value_to, "ETH", self.m) - self.assertEqual("buy", trade.order_action(False)) - self.assertEqual("sell", trade.order_action(True)) + trade.inverted = False + self.assertEqual("buy", trade.order_action()) + trade.inverted = True + self.assertEqual("sell", trade.order_action()) value_from = portfolio.Amount("BTC", "0") value_from.linked_to = portfolio.Amount("ETH", "0") value_to = portfolio.Amount("BTC", "-1.0") trade = portfolio.Trade(value_from, value_to, "ETH", self.m) - self.assertEqual("sell", trade.order_action(False)) - self.assertEqual("buy", trade.order_action(True)) + trade.inverted = False + self.assertEqual("sell", trade.order_action()) + trade.inverted = True + self.assertEqual("buy", trade.order_action()) def test_trade_type(self): value_from = portfolio.Amount("BTC", "0.5") @@ -1402,26 +1406,59 @@ class TradeTest(WebMockTestCase): self.assertEqual("short", trade.trade_type) def test_is_fullfiled(self): - value_from = portfolio.Amount("BTC", "0.5") - value_from.linked_to = portfolio.Amount("ETH", "10.0") - value_to = portfolio.Amount("BTC", "1.0") - trade = portfolio.Trade(value_from, value_to, "ETH", self.m) + with self.subTest(inverted=False): + value_from = portfolio.Amount("BTC", "0.5") + value_from.linked_to = portfolio.Amount("ETH", "10.0") + value_to = portfolio.Amount("BTC", "1.0") + trade = portfolio.Trade(value_from, value_to, "ETH", self.m) - order1 = mock.Mock() - order1.filled_amount.return_value = portfolio.Amount("BTC", "0.3") + order1 = mock.Mock() + order1.filled_amount.return_value = portfolio.Amount("BTC", "0.3") - order2 = mock.Mock() - order2.filled_amount.return_value = portfolio.Amount("BTC", "0.01") - trade.orders.append(order1) - trade.orders.append(order2) + order2 = mock.Mock() + order2.filled_amount.return_value = portfolio.Amount("BTC", "0.01") + trade.orders.append(order1) + trade.orders.append(order2) + + self.assertFalse(trade.is_fullfiled) + + order3 = mock.Mock() + order3.filled_amount.return_value = portfolio.Amount("BTC", "0.19") + trade.orders.append(order3) + + self.assertTrue(trade.is_fullfiled) + + order1.filled_amount.assert_called_with(in_base_currency=True) + order2.filled_amount.assert_called_with(in_base_currency=True) + order3.filled_amount.assert_called_with(in_base_currency=True) + + with self.subTest(inverted=True): + value_from = portfolio.Amount("BTC", "0.5") + value_from.linked_to = portfolio.Amount("USDT", "1000.0") + value_to = portfolio.Amount("BTC", "1.0") + trade = portfolio.Trade(value_from, value_to, "USDT", self.m) + trade.inverted = True + + order1 = mock.Mock() + order1.filled_amount.return_value = portfolio.Amount("BTC", "0.3") + + order2 = mock.Mock() + order2.filled_amount.return_value = portfolio.Amount("BTC", "0.01") + trade.orders.append(order1) + trade.orders.append(order2) + + self.assertFalse(trade.is_fullfiled) + + order3 = mock.Mock() + order3.filled_amount.return_value = portfolio.Amount("BTC", "0.19") + trade.orders.append(order3) - self.assertFalse(trade.is_fullfiled) + self.assertTrue(trade.is_fullfiled) - order3 = mock.Mock() - order3.filled_amount.return_value = portfolio.Amount("BTC", "0.19") - trade.orders.append(order3) + order1.filled_amount.assert_called_with(in_base_currency=False) + order2.filled_amount.assert_called_with(in_base_currency=False) + order3.filled_amount.assert_called_with(in_base_currency=False) - self.assertTrue(trade.is_fullfiled) def test_filled_amount(self): value_from = portfolio.Amount("BTC", "0.5") -- cgit v1.2.3 From d8deb0e9a6b7b2805e61dc19a287d5474c271cc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Bouya?= Date: Mon, 12 Mar 2018 01:55:46 +0100 Subject: Fix mark finished order not alway called when necessary --- portfolio.py | 9 ++++----- test.py | 13 +++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/portfolio.py b/portfolio.py index 65fdc6c..ed50b57 100644 --- a/portfolio.py +++ b/portfolio.py @@ -550,7 +550,7 @@ class Order: @property def finished(self): - return self.status == "closed" or self.status == "canceled" or self.status == "error" + return self.status.startswith("closed") or self.status == "canceled" or self.status == "error" @retry(InsufficientFunds) def run(self): @@ -594,15 +594,13 @@ class Order: # other states are "closed" and "canceled" if not self.finished: self.fetch() - if self.finished: - self.mark_finished_order() return self.status def mark_finished_order(self): - if self.market.debug: + if self.status.startswith("closed") and self.market.debug: self.market.report.log_debug_action("Mark {} as finished".format(self)) return - if self.status == "closed": + if self.status.startswith("closed"): if self.trade_type == "short" and self.action == "buy" and self.close_if_possible: self.market.ccxt.close_margin_position(self.amount.currency, self.base_currency) @@ -621,6 +619,7 @@ class Order: self.fetch_mouvements() + self.mark_finished_order() # FIXME: consider open order with dust remaining as closed def dust_amount_remaining(self): diff --git a/test.py b/test.py index 33a817d..921af9f 100644 --- a/test.py +++ b/test.py @@ -2118,7 +2118,8 @@ class OrderTest(WebMockTestCase): self.m.report.log_debug_action.assert_called_once() @mock.patch.object(portfolio.Order, "fetch_mouvements") - def test_fetch(self, fetch_mouvements): + @mock.patch.object(portfolio.Order, "mark_finished_order") + def test_fetch(self, mark_finished_order, fetch_mouvements): order = portfolio.Order("buy", portfolio.Amount("ETH", 10), D("0.1"), "BTC", "long", self.m, "trade") order.id = 45 @@ -2128,6 +2129,7 @@ class OrderTest(WebMockTestCase): self.m.report.log_debug_action.assert_called_once() self.m.report.log_debug_action.reset_mock() self.m.ccxt.fetch_order.assert_not_called() + mark_finished_order.assert_not_called() fetch_mouvements.assert_not_called() with self.subTest(debug=False): @@ -2144,17 +2146,19 @@ class OrderTest(WebMockTestCase): self.assertEqual("timestamp", order.timestamp) self.assertEqual(1, len(order.results)) self.m.report.log_debug_action.assert_not_called() + mark_finished_order.assert_called_once() + mark_finished_order.reset_mock() with self.subTest(missing_order=True): self.m.ccxt.fetch_order.side_effect = [ portfolio.OrderNotCached, ] order.fetch() self.assertEqual("closed_unknown", order.status) + mark_finished_order.assert_called_once() @mock.patch.object(portfolio.Order, "fetch") - @mock.patch.object(portfolio.Order, "mark_finished_order") - def test_get_status(self, mark_finished_order, fetch): + def test_get_status(self, fetch): with self.subTest(debug=True): self.m.debug = True order = portfolio.Order("buy", portfolio.Amount("ETH", 10), @@ -2173,10 +2177,8 @@ class OrderTest(WebMockTestCase): return update_status fetch.side_effect = _fetch(order) self.assertEqual("open", order.get_status()) - mark_finished_order.assert_not_called() fetch.assert_called_once() - mark_finished_order.reset_mock() fetch.reset_mock() with self.subTest(debug=False, finished=True): self.m.debug = False @@ -2188,7 +2190,6 @@ class OrderTest(WebMockTestCase): return update_status fetch.side_effect = _fetch(order) self.assertEqual("closed", order.get_status()) - mark_finished_order.assert_called_once() fetch.assert_called_once() def test_run(self): -- cgit v1.2.3