From 0a95ef6ab2e0cf369143f7af6be6ee0d2c3d15a5 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 19 May 2022 06:42:38 +0200 Subject: [PATCH 1/7] Don't reset open orders in dry-run on restart --- freqtrade/exchange/exchange.py | 6 ++++ freqtrade/persistence/models.py | 15 --------- freqtrade/persistence/trade_model.py | 27 +++++++++++++++ tests/test_persistence.py | 50 ---------------------------- 4 files changed, 33 insertions(+), 65 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index d2766cd6d..4ee9d3f63 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -953,6 +953,12 @@ class Exchange: order = self.check_dry_limit_order_filled(order) return order except KeyError as e: + from freqtrade.persistence import Order + order = Order.order_by_id(order_id) + if order: + x = order.to_ccxt_object() + self._dry_run_open_orders[order_id] = x + return x # Gracefully handle errors with dry-run orders. raise InvalidOrderException( f'Tried to get an invalid dry-run-order (id: {order_id}). Message: {e}') from e diff --git a/freqtrade/persistence/models.py b/freqtrade/persistence/models.py index c31e50892..1e0a70784 100644 --- a/freqtrade/persistence/models.py +++ b/freqtrade/persistence/models.py @@ -64,10 +64,6 @@ def init_db(db_url: str, clean_open_orders: bool = False) -> None: _DECL_BASE.metadata.create_all(engine) check_migrate(engine, decl_base=_DECL_BASE, previous_tables=previous_tables) - # Clean dry_run DB if the db is not in-memory - if clean_open_orders and db_url != 'sqlite://': - clean_dry_run_db() - def cleanup_db() -> None: """ @@ -76,14 +72,3 @@ def cleanup_db() -> None: """ Trade.commit() - -def clean_dry_run_db() -> None: - """ - Remove open_order_id from a Dry_run DB - :return: None - """ - for trade in Trade.query.filter(Trade.open_order_id.isnot(None)).all(): - # Check we are updating only a dry_run order not a prod one - if 'dry_run' in trade.open_order_id: - trade.open_order_id = None - Trade.commit() diff --git a/freqtrade/persistence/trade_model.py b/freqtrade/persistence/trade_model.py index 358e776e3..57aeda76c 100644 --- a/freqtrade/persistence/trade_model.py +++ b/freqtrade/persistence/trade_model.py @@ -118,6 +118,25 @@ class Order(_DECL_BASE): self.order_filled_date = datetime.now(timezone.utc) self.order_update_date = datetime.now(timezone.utc) + def to_ccxt_object(self) -> Dict[str, Any]: + return { + 'id': self.order_id, + 'symbol': self.ft_pair, + 'price': self.price, + 'average': self.average, + 'amount': self.amount, + 'cost': self.cost, + 'type': self.order_type, + 'side': self.ft_order_side, + 'filled': self.filled, + 'remaining': self.remaining, + 'datetime': self.order_date_utc.strftime('%Y-%m-%dT%H:%M:%S.%3f'), + 'timestamp': int(self.order_date_utc.timestamp() * 1000), + 'status': self.status, + 'fee': None, + 'info': {}, + } + def to_json(self, entry_side: str) -> Dict[str, Any]: return { 'pair': self.ft_pair, @@ -190,6 +209,14 @@ class Order(_DECL_BASE): """ return Order.query.filter(Order.ft_is_open.is_(True)).all() + @staticmethod + def order_by_id(order_id: str) -> Optional['Order']: + """ + Retrieve order based on order_id + :return: Order or None + """ + return Order.query.filter(Order.order_id == order_id).first() + class LocalTrade(): """ diff --git a/tests/test_persistence.py b/tests/test_persistence.py index d84415938..8d033663e 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -1129,56 +1129,6 @@ def test_calc_profit( assert pytest.approx(trade.calc_profit_ratio(rate=close_rate)) == round(profit_ratio, 8) -@pytest.mark.usefixtures("init_persistence") -def test_clean_dry_run_db(default_conf, fee): - - # Simulate dry_run entries - trade = Trade( - pair='ADA/USDT', - stake_amount=0.001, - amount=123.0, - fee_open=fee.return_value, - fee_close=fee.return_value, - open_rate=0.123, - exchange='binance', - open_order_id='dry_run_buy_12345' - ) - Trade.query.session.add(trade) - - trade = Trade( - pair='ETC/BTC', - stake_amount=0.001, - amount=123.0, - fee_open=fee.return_value, - fee_close=fee.return_value, - open_rate=0.123, - exchange='binance', - open_order_id='dry_run_sell_12345' - ) - Trade.query.session.add(trade) - - # Simulate prod entry - trade = Trade( - pair='ETC/BTC', - stake_amount=0.001, - amount=123.0, - fee_open=fee.return_value, - fee_close=fee.return_value, - open_rate=0.123, - exchange='binance', - open_order_id='prod_buy_12345' - ) - Trade.query.session.add(trade) - - # We have 3 entries: 2 dry_run, 1 prod - assert len(Trade.query.filter(Trade.open_order_id.isnot(None)).all()) == 3 - - clean_dry_run_db() - - # We have now only the prod - assert len(Trade.query.filter(Trade.open_order_id.isnot(None)).all()) == 1 - - def test_migrate_new(mocker, default_conf, fee, caplog): """ Test Database migration (starting with new pairformat) From a3d9384bc0665a06c25400948ad4b45fdc2e55c1 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 19 May 2022 06:45:20 +0200 Subject: [PATCH 2/7] Remove clean-dry-run code --- freqtrade/commands/db_commands.py | 4 ++-- freqtrade/commands/list_commands.py | 2 +- freqtrade/data/btanalysis.py | 2 +- freqtrade/freqtradebot.py | 2 +- freqtrade/persistence/__init__.py | 2 +- freqtrade/persistence/models.py | 4 +--- tests/commands/test_commands.py | 2 +- tests/conftest.py | 2 +- tests/test_persistence.py | 20 ++++++++++---------- 9 files changed, 19 insertions(+), 21 deletions(-) diff --git a/freqtrade/commands/db_commands.py b/freqtrade/commands/db_commands.py index d93aafcb6..618b5cb6e 100644 --- a/freqtrade/commands/db_commands.py +++ b/freqtrade/commands/db_commands.py @@ -19,9 +19,9 @@ def start_convert_db(args: Dict[str, Any]) -> None: config = setup_utils_configuration(args, RunMode.UTIL_NO_EXCHANGE) - init_db(config['db_url'], False) + init_db(config['db_url']) session_target = Trade._session - init_db(config['db_url_from'], False) + init_db(config['db_url_from']) logger.info("Starting db migration.") trade_count = 0 diff --git a/freqtrade/commands/list_commands.py b/freqtrade/commands/list_commands.py index 2a5223917..eb761eeec 100644 --- a/freqtrade/commands/list_commands.py +++ b/freqtrade/commands/list_commands.py @@ -212,7 +212,7 @@ def start_show_trades(args: Dict[str, Any]) -> None: raise OperationalException("--db-url is required for this command.") logger.info(f'Using DB: "{parse_db_uri_for_logging(config["db_url"])}"') - init_db(config['db_url'], clean_open_orders=False) + init_db(config['db_url']) tfilter = [] if config.get('trade_ids'): diff --git a/freqtrade/data/btanalysis.py b/freqtrade/data/btanalysis.py index e29d9ebe4..fef432576 100644 --- a/freqtrade/data/btanalysis.py +++ b/freqtrade/data/btanalysis.py @@ -353,7 +353,7 @@ def load_trades_from_db(db_url: str, strategy: Optional[str] = None) -> pd.DataF Can also serve as protection to load the correct result. :return: Dataframe containing Trades """ - init_db(db_url, clean_open_orders=False) + init_db(db_url) filters = [] if strategy: diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 315db3ae6..da35c12ff 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -67,7 +67,7 @@ class FreqtradeBot(LoggingMixin): self.exchange = ExchangeResolver.load_exchange(self.config['exchange']['name'], self.config) - init_db(self.config.get('db_url', None), clean_open_orders=self.config['dry_run']) + init_db(self.config.get('db_url', None)) self.wallets = Wallets(self.config, self.exchange) diff --git a/freqtrade/persistence/__init__.py b/freqtrade/persistence/__init__.py index ab6e2f6a5..f4e7470a7 100644 --- a/freqtrade/persistence/__init__.py +++ b/freqtrade/persistence/__init__.py @@ -1,5 +1,5 @@ # flake8: noqa: F401 -from freqtrade.persistence.models import clean_dry_run_db, cleanup_db, init_db +from freqtrade.persistence.models import cleanup_db, init_db from freqtrade.persistence.pairlock_middleware import PairLocks from freqtrade.persistence.trade_model import LocalTrade, Order, Trade diff --git a/freqtrade/persistence/models.py b/freqtrade/persistence/models.py index 1e0a70784..154f2590a 100644 --- a/freqtrade/persistence/models.py +++ b/freqtrade/persistence/models.py @@ -21,14 +21,12 @@ logger = logging.getLogger(__name__) _SQL_DOCS_URL = 'http://docs.sqlalchemy.org/en/latest/core/engines.html#database-urls' -def init_db(db_url: str, clean_open_orders: bool = False) -> None: +def init_db(db_url: str) -> None: """ Initializes this module with the given config, registers all known command handlers and starts polling for message updates :param db_url: Database to use - :param clean_open_orders: Remove open orders from the database. - Useful for dry-run or if all orders have been reset on the exchange. :return: None """ kwargs = {} diff --git a/tests/commands/test_commands.py b/tests/commands/test_commands.py index b37edf9c7..d6e80675e 100644 --- a/tests/commands/test_commands.py +++ b/tests/commands/test_commands.py @@ -1495,7 +1495,7 @@ def test_start_convert_db(mocker, fee, tmpdir, caplog): ] assert not db_src_file.is_file() - init_db(db_from, False) + init_db(db_from) create_mock_trades(fee) diff --git a/tests/conftest.py b/tests/conftest.py index cc07de1de..8719c70f4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -384,7 +384,7 @@ def patch_coingekko(mocker) -> None: @pytest.fixture(scope='function') def init_persistence(default_conf): - init_db(default_conf['db_url'], default_conf['dry_run']) + init_db(default_conf['db_url']) @pytest.fixture(scope="function") diff --git a/tests/test_persistence.py b/tests/test_persistence.py index 8d033663e..ef17c4d1c 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -13,7 +13,7 @@ from sqlalchemy import create_engine, text from freqtrade import constants from freqtrade.enums import TradingMode from freqtrade.exceptions import DependencyException, OperationalException -from freqtrade.persistence import LocalTrade, Order, Trade, clean_dry_run_db, init_db +from freqtrade.persistence import LocalTrade, Order, Trade, init_db from freqtrade.persistence.migrations import get_last_sequence_ids, set_sequence_ids from freqtrade.persistence.models import PairLock from tests.conftest import create_mock_trades, create_mock_trades_with_leverage, log_has, log_has_re @@ -24,7 +24,7 @@ spot, margin, futures = TradingMode.SPOT, TradingMode.MARGIN, TradingMode.FUTURE def test_init_create_session(default_conf): # Check if init create a session - init_db(default_conf['db_url'], default_conf['dry_run']) + init_db(default_conf['db_url']) assert hasattr(Trade, '_session') assert 'scoped_session' in type(Trade._session).__name__ @@ -36,7 +36,7 @@ def test_init_custom_db_url(default_conf, tmpdir): default_conf.update({'db_url': f'sqlite:///{filename}'}) - init_db(default_conf['db_url'], default_conf['dry_run']) + init_db(default_conf['db_url']) assert Path(filename).is_file() r = Trade._session.execute(text("PRAGMA journal_mode")) assert r.first() == ('wal',) @@ -45,10 +45,10 @@ def test_init_custom_db_url(default_conf, tmpdir): def test_init_invalid_db_url(): # Update path to a value other than default, but still in-memory with pytest.raises(OperationalException, match=r'.*no valid database URL*'): - init_db('unknown:///some.url', True) + init_db('unknown:///some.url') with pytest.raises(OperationalException, match=r'Bad db-url.*For in-memory database, pl.*'): - init_db('sqlite:///', True) + init_db('sqlite:///') def test_init_prod_db(default_conf, mocker): @@ -57,7 +57,7 @@ def test_init_prod_db(default_conf, mocker): create_engine_mock = mocker.patch('freqtrade.persistence.models.create_engine', MagicMock()) - init_db(default_conf['db_url'], default_conf['dry_run']) + init_db(default_conf['db_url']) assert create_engine_mock.call_count == 1 assert create_engine_mock.mock_calls[0][1][0] == 'sqlite:///tradesv3.sqlite' @@ -70,7 +70,7 @@ def test_init_dryrun_db(default_conf, tmpdir): 'db_url': f'sqlite:///{filename}' }) - init_db(default_conf['db_url'], default_conf['dry_run']) + init_db(default_conf['db_url']) assert Path(filename).is_file() @@ -1260,7 +1260,7 @@ def test_migrate_new(mocker, default_conf, fee, caplog): connection.execute(text("create table trades_bak1 as select * from trades")) # Run init to test migration - init_db(default_conf['db_url'], default_conf['dry_run']) + init_db(default_conf['db_url']) assert len(Trade.query.filter(Trade.id == 1).all()) == 1 trade = Trade.query.filter(Trade.id == 1).first() @@ -1343,7 +1343,7 @@ def test_migrate_too_old(mocker, default_conf, fee, caplog): # Run init to test migration with pytest.raises(OperationalException, match=r'Your database seems to be very old'): - init_db(default_conf['db_url'], default_conf['dry_run']) + init_db(default_conf['db_url']) def test_migrate_get_last_sequence_ids(): @@ -1417,7 +1417,7 @@ def test_migrate_pairlocks(mocker, default_conf, fee, caplog): connection.execute(text(create_index2)) connection.execute(text(create_index3)) - init_db(default_conf['db_url'], default_conf['dry_run']) + init_db(default_conf['db_url']) assert len(PairLock.query.all()) == 2 assert len(PairLock.query.filter(PairLock.pair == '*').all()) == 1 From 5e18e51ce0ca1a36ef1d73f31f0d09a194a254dc Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 19 May 2022 06:56:38 +0200 Subject: [PATCH 3/7] Fix some tests --- tests/test_freqtradebot.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index e19d5f36a..d2df4e6a5 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -3044,6 +3044,7 @@ def test_handle_cancel_enter_corder_empty(mocker, default_conf_usdt, limit_order trade.entry_side = "buy" trade.open_rate = 200 trade.entry_side = "buy" + trade.open_order_id = "open_order_noop" l_order['filled'] = 0.0 l_order['status'] = 'open' reason = CANCEL_REASON['TIMEOUT'] @@ -4786,9 +4787,6 @@ def test_startup_update_open_orders(mocker, default_conf_usdt, fee, caplog, is_s freqtrade.config['dry_run'] = False freqtrade.startup_update_open_orders() - assert log_has_re(r"Error updating Order .*", caplog) - caplog.clear() - assert len(Order.get_open_orders()) == 3 matching_buy_order = mock_order_4(is_short=is_short) matching_buy_order.update({ @@ -4799,6 +4797,11 @@ def test_startup_update_open_orders(mocker, default_conf_usdt, fee, caplog, is_s # Only stoploss and sell orders are kept open assert len(Order.get_open_orders()) == 2 + caplog.clear() + mocker.patch('freqtrade.exchange.Exchange.fetch_order', side_effect=InvalidOrderException) + freqtrade.startup_update_open_orders() + assert log_has_re(r"Error updating Order .*", caplog) + @pytest.mark.usefixtures("init_persistence") @pytest.mark.parametrize("is_short", [False, True]) From 56a73575a13bccf5fddc8fa4d9f85ac7d28ee214 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 19 May 2022 19:29:39 +0200 Subject: [PATCH 4/7] Add explicit test for order_to_ccxt --- freqtrade/persistence/trade_model.py | 2 +- tests/conftest.py | 1 + tests/test_persistence.py | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/freqtrade/persistence/trade_model.py b/freqtrade/persistence/trade_model.py index 57aeda76c..d2abb48d6 100644 --- a/freqtrade/persistence/trade_model.py +++ b/freqtrade/persistence/trade_model.py @@ -130,7 +130,7 @@ class Order(_DECL_BASE): 'side': self.ft_order_side, 'filled': self.filled, 'remaining': self.remaining, - 'datetime': self.order_date_utc.strftime('%Y-%m-%dT%H:%M:%S.%3f'), + 'datetime': self.order_date_utc.strftime('%Y-%m-%dT%H:%M:%S.%f'), 'timestamp': int(self.order_date_utc.timestamp() * 1000), 'status': self.status, 'fee': None, diff --git a/tests/conftest.py b/tests/conftest.py index 8719c70f4..02738b0e9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1616,6 +1616,7 @@ def limit_buy_order_open(): 'datetime': arrow.utcnow().isoformat(), 'price': 0.00001099, 'amount': 90.99181073, + 'average': None, 'filled': 0.0, 'cost': 0.0009999, 'remaining': 90.99181073, diff --git a/tests/test_persistence.py b/tests/test_persistence.py index ef17c4d1c..be19a3f5f 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -2671,3 +2671,21 @@ def test_select_filled_orders(fee): orders = trades[4].select_filled_orders('sell') assert orders is not None assert len(orders) == 0 + + +@pytest.mark.usefixtures("init_persistence") +def test_order_to_ccxt(limit_buy_order_open): + + order = Order.parse_from_ccxt_object(limit_buy_order_open, 'mocked', 'buy') + order.query.session.add(order) + Order.query.session.commit() + + order_resp = Order.order_by_id(limit_buy_order_open['id']) + assert order_resp + + raw_order = order_resp.to_ccxt_object() + del raw_order['fee'] + del raw_order['datetime'] + del raw_order['info'] + del limit_buy_order_open['datetime'] + assert raw_order == limit_buy_order_open From 46ea135b6bf4e3b5f2e54e0c951ba40dde7cf9f2 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 19 May 2022 19:42:00 +0200 Subject: [PATCH 5/7] Update dry-run considerations --- docs/configuration.md | 2 +- freqtrade/persistence/models.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 80cd52c5b..7dc907b9f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -583,7 +583,7 @@ Once you will be happy with your bot performance running in the Dry-run mode, yo * Market orders fill based on orderbook volume the moment the order is placed. * Limit orders fill once the price reaches the defined level - or time out based on `unfilledtimeout` settings. * In combination with `stoploss_on_exchange`, the stop_loss price is assumed to be filled. -* Open orders (not trades, which are stored in the database) are reset on bot restart. +* Open orders (not trades, which are stored in the database) are kept open after bot restarts, with the assumption that they were not filled while being offline. ## Switch to production mode diff --git a/freqtrade/persistence/models.py b/freqtrade/persistence/models.py index 154f2590a..86d2f9f9c 100644 --- a/freqtrade/persistence/models.py +++ b/freqtrade/persistence/models.py @@ -69,4 +69,3 @@ def cleanup_db() -> None: :return: None """ Trade.commit() - From 2cf17e04be3d6cbd5444f6fb2f4e03c90ab3d022 Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 20 May 2022 06:26:16 +0200 Subject: [PATCH 6/7] Init persistence for tests that use dry-run orders --- tests/exchange/test_exchange.py | 3 +++ tests/exchange/test_ftx.py | 1 + tests/exchange/test_gateio.py | 1 + 3 files changed, 5 insertions(+) diff --git a/tests/exchange/test_exchange.py b/tests/exchange/test_exchange.py index 53e6cc3f3..07b2147d5 100644 --- a/tests/exchange/test_exchange.py +++ b/tests/exchange/test_exchange.py @@ -2808,6 +2808,7 @@ def test_get_historic_trades_notsupported(default_conf, mocker, caplog, exchange until=trades_history[-1][0]) +@pytest.mark.usefixtures("init_persistence") @pytest.mark.parametrize("exchange_name", EXCHANGES) def test_cancel_order_dry_run(default_conf, mocker, exchange_name): default_conf['dry_run'] = True @@ -2973,6 +2974,7 @@ def test_cancel_stoploss_order_with_result(default_conf, mocker, exchange_name): exchange.cancel_stoploss_order_with_result(order_id='_', pair='TKN/BTC', amount=123) +@pytest.mark.usefixtures("init_persistence") @pytest.mark.parametrize("exchange_name", EXCHANGES) def test_fetch_order(default_conf, mocker, exchange_name, caplog): default_conf['dry_run'] = True @@ -3025,6 +3027,7 @@ def test_fetch_order(default_conf, mocker, exchange_name, caplog): order_id='_', pair='TKN/BTC') +@pytest.mark.usefixtures("init_persistence") @pytest.mark.parametrize("exchange_name", EXCHANGES) def test_fetch_stoploss_order(default_conf, mocker, exchange_name): # Don't test FTX here - that needs a separate test diff --git a/tests/exchange/test_ftx.py b/tests/exchange/test_ftx.py index 0f16d4433..5a83b964a 100644 --- a/tests/exchange/test_ftx.py +++ b/tests/exchange/test_ftx.py @@ -174,6 +174,7 @@ def test_stoploss_adjust_ftx(mocker, default_conf, sl1, sl2, sl3, side): assert not exchange.stoploss_adjust(sl3, order, side=side) +@pytest.mark.usefixtures("init_persistence") def test_fetch_stoploss_order_ftx(default_conf, mocker, limit_sell_order, limit_buy_order): default_conf['dry_run'] = True order = MagicMock() diff --git a/tests/exchange/test_gateio.py b/tests/exchange/test_gateio.py index ad30a7d86..92f8186a6 100644 --- a/tests/exchange/test_gateio.py +++ b/tests/exchange/test_gateio.py @@ -34,6 +34,7 @@ def test_validate_order_types_gateio(default_conf, mocker): ExchangeResolver.load_exchange('gateio', default_conf, True) +@pytest.mark.usefixtures("init_persistence") def test_fetch_stoploss_order_gateio(default_conf, mocker): exchange = get_patched_exchange(mocker, default_conf, id='gateio') From c3e3188c6a66682fb7b9511b8615f0e5fa5087c6 Mon Sep 17 00:00:00 2001 From: Matthias Date: Fri, 20 May 2022 11:30:25 +0200 Subject: [PATCH 7/7] Rename variable --- freqtrade/exchange/exchange.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index 4ee9d3f63..06a30c99b 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -956,9 +956,9 @@ class Exchange: from freqtrade.persistence import Order order = Order.order_by_id(order_id) if order: - x = order.to_ccxt_object() - self._dry_run_open_orders[order_id] = x - return x + ccxt_order = order.to_ccxt_object() + self._dry_run_open_orders[order_id] = ccxt_order + return ccxt_order # Gracefully handle errors with dry-run orders. raise InvalidOrderException( f'Tried to get an invalid dry-run-order (id: {order_id}). Message: {e}') from e