From fffb056ad34aab6707cf2196146a8ee96df1a745 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 13 May 2023 13:43:53 +0200 Subject: [PATCH 1/7] load_exchange - force kwargs for non-required arguments --- freqtrade/resolvers/exchange_resolver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/freqtrade/resolvers/exchange_resolver.py b/freqtrade/resolvers/exchange_resolver.py index e888028dc..448fdeb8b 100644 --- a/freqtrade/resolvers/exchange_resolver.py +++ b/freqtrade/resolvers/exchange_resolver.py @@ -19,7 +19,7 @@ class ExchangeResolver(IResolver): object_type = Exchange @staticmethod - def load_exchange(config: Config, validate: bool = True, + def load_exchange(config: Config, *, validate: bool = True, load_leverage_tiers: bool = False) -> Exchange: """ Load the custom class from config parameter From fe36e774129a8c62ec297172162cd03de1d15c70 Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 13 May 2023 15:33:13 +0200 Subject: [PATCH 2/7] Split exchange_config before passing through the strategy --- freqtrade/exchange/exchange.py | 26 ++++++++++++------------ freqtrade/freqtradebot.py | 3 ++- freqtrade/resolvers/exchange_resolver.py | 8 +++++--- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index bb9c7c1b3..a70527a7f 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -92,8 +92,8 @@ class Exchange: # TradingMode.SPOT always supported and not required in this list ] - def __init__(self, config: Config, *, validate: bool = True, - load_leverage_tiers: bool = False) -> None: + def __init__(self, config: Config, *, exchange_config: Optional[Config] = None, + validate: bool = True, load_leverage_tiers: bool = False) -> None: """ Initializes this module with the given config, it does basic validation whether the specified exchange and pairs are valid. @@ -136,8 +136,8 @@ class Exchange: if config['dry_run']: logger.info('Instance is running with dry_run enabled') logger.info(f"Using CCXT {ccxt.__version__}") - exchange_config = config['exchange'] - self.log_responses = exchange_config.get('log_responses', False) + exchange_conf: Dict[str, Any] = exchange_config if exchange_config else config['exchange'] + self.log_responses = exchange_conf.get('log_responses', False) # Leverage properties self.trading_mode: TradingMode = config.get('trading_mode', TradingMode.SPOT) @@ -152,8 +152,8 @@ class Exchange: self._ft_has = deep_merge_dicts(self._ft_has, deepcopy(self._ft_has_default)) if self.trading_mode == TradingMode.FUTURES: self._ft_has = deep_merge_dicts(self._ft_has_futures, self._ft_has) - if exchange_config.get('_ft_has_params'): - self._ft_has = deep_merge_dicts(exchange_config.get('_ft_has_params'), + if exchange_conf.get('_ft_has_params'): + self._ft_has = deep_merge_dicts(exchange_conf.get('_ft_has_params'), self._ft_has) logger.info("Overriding exchange._ft_has with config params, result: %s", self._ft_has) @@ -165,18 +165,18 @@ class Exchange: # Initialize ccxt objects ccxt_config = self._ccxt_config - ccxt_config = deep_merge_dicts(exchange_config.get('ccxt_config', {}), ccxt_config) - ccxt_config = deep_merge_dicts(exchange_config.get('ccxt_sync_config', {}), ccxt_config) + ccxt_config = deep_merge_dicts(exchange_conf.get('ccxt_config', {}), ccxt_config) + ccxt_config = deep_merge_dicts(exchange_conf.get('ccxt_sync_config', {}), ccxt_config) - self._api = self._init_ccxt(exchange_config, ccxt_kwargs=ccxt_config) + self._api = self._init_ccxt(exchange_conf, ccxt_kwargs=ccxt_config) ccxt_async_config = self._ccxt_config - ccxt_async_config = deep_merge_dicts(exchange_config.get('ccxt_config', {}), + ccxt_async_config = deep_merge_dicts(exchange_conf.get('ccxt_config', {}), ccxt_async_config) - ccxt_async_config = deep_merge_dicts(exchange_config.get('ccxt_async_config', {}), + ccxt_async_config = deep_merge_dicts(exchange_conf.get('ccxt_async_config', {}), ccxt_async_config) self._api_async = self._init_ccxt( - exchange_config, ccxt_async, ccxt_kwargs=ccxt_async_config) + exchange_conf, ccxt_async, ccxt_kwargs=ccxt_async_config) logger.info(f'Using Exchange "{self.name}"') self.required_candle_call_count = 1 @@ -189,7 +189,7 @@ class Exchange: self._startup_candle_count, config.get('timeframe', '')) # Converts the interval provided in minutes in config to seconds - self.markets_refresh_interval: int = exchange_config.get( + self.markets_refresh_interval: int = exchange_conf.get( "markets_refresh_interval", 60) * 60 if self.trading_mode != TradingMode.SPOT and load_leverage_tiers: diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index ef480a8e2..9df21a4b1 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -63,6 +63,7 @@ class FreqtradeBot(LoggingMixin): # Init objects self.config = config + exchange_config = deepcopy(config['exchange']) self.strategy: IStrategy = StrategyResolver.load_strategy(self.config) @@ -70,7 +71,7 @@ class FreqtradeBot(LoggingMixin): validate_config_consistency(config) self.exchange = ExchangeResolver.load_exchange( - self.config, load_leverage_tiers=True) + self.config, exchange_config=exchange_config, load_leverage_tiers=True) init_db(self.config['db_url']) diff --git a/freqtrade/resolvers/exchange_resolver.py b/freqtrade/resolvers/exchange_resolver.py index 448fdeb8b..6e5357da5 100644 --- a/freqtrade/resolvers/exchange_resolver.py +++ b/freqtrade/resolvers/exchange_resolver.py @@ -2,6 +2,7 @@ This module loads custom exchanges """ import logging +from typing import Any, Dict, Optional import freqtrade.exchange as exchanges from freqtrade.constants import Config @@ -19,8 +20,8 @@ class ExchangeResolver(IResolver): object_type = Exchange @staticmethod - def load_exchange(config: Config, *, validate: bool = True, - load_leverage_tiers: bool = False) -> Exchange: + def load_exchange(config: Config, *, exchange_config: Optional[Dict[str, Any]] = None, + validate: bool = True, load_leverage_tiers: bool = False) -> Exchange: """ Load the custom class from config parameter :param exchange_name: name of the Exchange to load @@ -37,13 +38,14 @@ class ExchangeResolver(IResolver): kwargs={ 'config': config, 'validate': validate, + 'exchange_config': exchange_config, 'load_leverage_tiers': load_leverage_tiers} ) except ImportError: logger.info( f"No {exchange_name} specific subclass found. Using the generic class instead.") if not exchange: - exchange = Exchange(config, validate=validate) + exchange = Exchange(config, validate=validate, exchange_config=exchange_config,) return exchange @staticmethod From b2a631e93a7062a75165797bd59f61a7d25fcccf Mon Sep 17 00:00:00 2001 From: Matthias Date: Sat, 13 May 2023 15:38:40 +0200 Subject: [PATCH 3/7] refactor remove_exchange_credentials --- freqtrade/constants.py | 2 ++ freqtrade/exchange/__init__.py | 2 +- freqtrade/exchange/common.py | 16 ++++++++-------- freqtrade/exchange/exchange.py | 12 ++++++------ freqtrade/rpc/api_server/api_backtest.py | 2 ++ tests/exchange/test_exchange.py | 10 ++++------ 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/freqtrade/constants.py b/freqtrade/constants.py index b8e240419..3802ec3ad 100644 --- a/freqtrade/constants.py +++ b/freqtrade/constants.py @@ -690,4 +690,6 @@ BidAsk = Literal['bid', 'ask'] OBLiteral = Literal['asks', 'bids'] Config = Dict[str, Any] +# Exchange part of the configuration. +ExchangeConfig = Dict[str, Any] IntOrInf = float diff --git a/freqtrade/exchange/__init__.py b/freqtrade/exchange/__init__.py index 8092d5af8..12fb0c55e 100644 --- a/freqtrade/exchange/__init__.py +++ b/freqtrade/exchange/__init__.py @@ -1,6 +1,6 @@ # flake8: noqa: F401 # isort: off -from freqtrade.exchange.common import remove_credentials, MAP_EXCHANGE_CHILDCLASS +from freqtrade.exchange.common import remove_exchange_credentials, MAP_EXCHANGE_CHILDCLASS from freqtrade.exchange.exchange import Exchange # isort: on from freqtrade.exchange.binance import Binance diff --git a/freqtrade/exchange/common.py b/freqtrade/exchange/common.py index e60207573..10dfdf178 100644 --- a/freqtrade/exchange/common.py +++ b/freqtrade/exchange/common.py @@ -4,7 +4,7 @@ import time from functools import wraps from typing import Any, Callable, Optional, TypeVar, cast, overload -from freqtrade.constants import Config +from freqtrade.constants import ExchangeConfig from freqtrade.exceptions import DDosProtection, RetryableOrderError, TemporaryError from freqtrade.mixins import LoggingMixin @@ -89,18 +89,18 @@ EXCHANGE_HAS_OPTIONAL = [ ] -def remove_credentials(config: Config) -> None: +def remove_exchange_credentials(exchange_config: ExchangeConfig, dry_run: bool) -> None: """ Removes exchange keys from the configuration and specifies dry-run Used for backtesting / hyperopt / edge and utils. Modifies the input dict! """ - if config.get('dry_run', False): - config['exchange']['key'] = '' - config['exchange']['apiKey'] = '' - config['exchange']['secret'] = '' - config['exchange']['password'] = '' - config['exchange']['uid'] = '' + if dry_run: + exchange_config['key'] = '' + exchange_config['apiKey'] = '' + exchange_config['secret'] = '' + exchange_config['password'] = '' + exchange_config['uid'] = '' def calculate_backoff(retrycount, max_retries): diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index a70527a7f..7c68eaa99 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -20,16 +20,16 @@ from dateutil import parser from pandas import DataFrame, concat from freqtrade.constants import (DEFAULT_AMOUNT_RESERVE_PERCENT, NON_OPEN_EXCHANGE_STATES, BidAsk, - BuySell, Config, EntryExit, ListPairsWithTimeframes, MakerTaker, - OBLiteral, PairWithTimeframe) + BuySell, Config, EntryExit, ExchangeConfig, + ListPairsWithTimeframes, MakerTaker, OBLiteral, PairWithTimeframe) from freqtrade.data.converter import clean_ohlcv_dataframe, ohlcv_to_dataframe, trades_dict_to_list from freqtrade.enums import OPTIMIZE_MODES, CandleType, MarginMode, TradingMode from freqtrade.enums.pricetype import PriceType from freqtrade.exceptions import (DDosProtection, ExchangeError, InsufficientFundsError, InvalidOrderException, OperationalException, PricingError, RetryableOrderError, TemporaryError) -from freqtrade.exchange.common import (API_FETCH_ORDER_RETRY_COUNT, remove_credentials, retrier, - retrier_async) +from freqtrade.exchange.common import (API_FETCH_ORDER_RETRY_COUNT, remove_exchange_credentials, + retrier, retrier_async) from freqtrade.exchange.exchange_utils import (ROUND, ROUND_DOWN, ROUND_UP, CcxtModuleType, amount_to_contract_precision, amount_to_contracts, amount_to_precision, contracts_to_amount, @@ -92,7 +92,7 @@ class Exchange: # TradingMode.SPOT always supported and not required in this list ] - def __init__(self, config: Config, *, exchange_config: Optional[Config] = None, + def __init__(self, config: Config, *, exchange_config: Optional[ExchangeConfig] = None, validate: bool = True, load_leverage_tiers: bool = False) -> None: """ Initializes this module with the given config, @@ -131,12 +131,12 @@ class Exchange: # Holds all open sell orders for dry_run self._dry_run_open_orders: Dict[str, Any] = {} - remove_credentials(config) if config['dry_run']: logger.info('Instance is running with dry_run enabled') logger.info(f"Using CCXT {ccxt.__version__}") exchange_conf: Dict[str, Any] = exchange_config if exchange_config else config['exchange'] + remove_exchange_credentials(exchange_conf, config.get('dry_run', False)) self.log_responses = exchange_conf.get('log_responses', False) # Leverage properties diff --git a/freqtrade/rpc/api_server/api_backtest.py b/freqtrade/rpc/api_server/api_backtest.py index d9d7a27f1..b168affc3 100644 --- a/freqtrade/rpc/api_server/api_backtest.py +++ b/freqtrade/rpc/api_server/api_backtest.py @@ -11,6 +11,7 @@ from freqtrade.configuration.config_validation import validate_config_consistenc from freqtrade.data.btanalysis import get_backtest_resultlist, load_and_merge_backtest_result from freqtrade.enums import BacktestState from freqtrade.exceptions import DependencyException, OperationalException +from freqtrade.exchange.common import remove_exchange_credentials from freqtrade.misc import deep_merge_dicts from freqtrade.rpc.api_server.api_schemas import (BacktestHistoryEntry, BacktestRequest, BacktestResponse) @@ -38,6 +39,7 @@ async def api_start_backtest( # noqa: C901 raise HTTPException(status_code=500, detail="base64 encoded strategies are not allowed.") btconfig = deepcopy(config) + remove_exchange_credentials(btconfig['exchange'], True) settings = dict(bt_settings) if settings.get('freqai', None) is not None: settings['freqai'] = dict(settings['freqai']) diff --git a/tests/exchange/test_exchange.py b/tests/exchange/test_exchange.py index ebbecdad0..4c7a7dcc8 100644 --- a/tests/exchange/test_exchange.py +++ b/tests/exchange/test_exchange.py @@ -20,7 +20,7 @@ from freqtrade.exchange import (Binance, Bittrex, Exchange, Kraken, amount_to_pr timeframe_to_minutes, timeframe_to_msecs, timeframe_to_next_date, timeframe_to_prev_date, timeframe_to_seconds) from freqtrade.exchange.common import (API_FETCH_ORDER_RETRY_COUNT, API_RETRY_COUNT, - calculate_backoff, remove_credentials) + calculate_backoff, remove_exchange_credentials) from freqtrade.exchange.exchange import amount_to_contract_precision from freqtrade.resolvers.exchange_resolver import ExchangeResolver from tests.conftest import (EXMS, generate_test_data_raw, get_mock_coro, get_patched_exchange, @@ -137,16 +137,14 @@ def test_init(default_conf, mocker, caplog): assert log_has('Instance is running with dry_run enabled', caplog) -def test_remove_credentials(default_conf, caplog) -> None: +def test_remove_exchange_credentials(default_conf) -> None: conf = deepcopy(default_conf) - conf['dry_run'] = False - remove_credentials(conf) + remove_exchange_credentials(conf['exchange'], False) assert conf['exchange']['key'] != '' assert conf['exchange']['secret'] != '' - conf['dry_run'] = True - remove_credentials(conf) + remove_exchange_credentials(conf['exchange'], True) assert conf['exchange']['key'] == '' assert conf['exchange']['secret'] == '' assert conf['exchange']['password'] == '' From 66c3eb28209accdb38664b7cc26475da9fe5e6ad Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 15 May 2023 07:06:18 +0200 Subject: [PATCH 4/7] Remove keys from config before loading strategy --- freqtrade/freqtradebot.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index 9df21a4b1..ed13be5b3 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -23,6 +23,7 @@ from freqtrade.exceptions import (DependencyException, ExchangeError, Insufficie InvalidOrderException, PricingError) from freqtrade.exchange import (ROUND_DOWN, ROUND_UP, timeframe_to_minutes, timeframe_to_next_date, timeframe_to_seconds) +from freqtrade.exchange.common import remove_exchange_credentials from freqtrade.misc import safe_value_fallback, safe_value_fallback2 from freqtrade.mixins import LoggingMixin from freqtrade.persistence import Order, PairLocks, Trade, init_db @@ -64,6 +65,8 @@ class FreqtradeBot(LoggingMixin): # Init objects self.config = config exchange_config = deepcopy(config['exchange']) + # Remove credentials from original exchange config to avoid accidental credentail exposure + remove_exchange_credentials(config['exchange'], True) self.strategy: IStrategy = StrategyResolver.load_strategy(self.config) From c242d89cda06338231bf2b7a76110157428dd924 Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 15 May 2023 07:10:55 +0200 Subject: [PATCH 5/7] Improve test format --- tests/test_freqtradebot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 8aa3f63d5..97765a75b 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -121,7 +121,7 @@ def test_order_dict(default_conf_usdt, mocker, runmode, caplog) -> None: freqtrade = FreqtradeBot(conf) if runmode == RunMode.LIVE: - assert not log_has_re(".*stoploss_on_exchange .* dry-run", caplog) + assert not log_has_re(r".*stoploss_on_exchange .* dry-run", caplog) assert freqtrade.strategy.order_types['stoploss_on_exchange'] caplog.clear() @@ -136,7 +136,7 @@ def test_order_dict(default_conf_usdt, mocker, runmode, caplog) -> None: } freqtrade = FreqtradeBot(conf) assert not freqtrade.strategy.order_types['stoploss_on_exchange'] - assert not log_has_re(".*stoploss_on_exchange .* dry-run", caplog) + assert not log_has_re(r".*stoploss_on_exchange .* dry-run", caplog) def test_get_trade_stake_amount(default_conf_usdt, mocker) -> None: From 68f67c5ae8c7e49745ca8454bbae7b88b9603308 Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 15 May 2023 07:19:15 +0200 Subject: [PATCH 6/7] Test proper removal of exchange keys --- tests/test_freqtradebot.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_freqtradebot.py b/tests/test_freqtradebot.py index 97765a75b..bd78e2fda 100644 --- a/tests/test_freqtradebot.py +++ b/tests/test_freqtradebot.py @@ -149,6 +149,34 @@ def test_get_trade_stake_amount(default_conf_usdt, mocker) -> None: assert result == default_conf_usdt['stake_amount'] +@pytest.mark.parametrize('runmode', [ + RunMode.DRY_RUN, + RunMode.LIVE +]) +def test_load_strategy_no_keys(default_conf_usdt, mocker, runmode, caplog) -> None: + patch_RPCManager(mocker) + patch_exchange(mocker) + conf = deepcopy(default_conf_usdt) + conf['runmode'] = runmode + erm = mocker.patch('freqtrade.freqtradebot.ExchangeResolver.load_exchange') + + freqtrade = FreqtradeBot(conf) + strategy_config = freqtrade.strategy.config + assert id(strategy_config['exchange']) == id(conf['exchange']) + # Keys have been removed and are not passed to the exchange + assert strategy_config['exchange']['key'] == '' + assert strategy_config['exchange']['secret'] == '' + + assert erm.call_count == 1 + ex_conf = erm.call_args_list[0][1]['exchange_config'] + assert id(ex_conf) != id(conf['exchange']) + # Keys are still present + assert ex_conf['key'] != '' + assert ex_conf['key'] == default_conf_usdt['exchange']['key'] + assert ex_conf['secret'] != '' + assert ex_conf['secret'] == default_conf_usdt['exchange']['secret'] + + @pytest.mark.parametrize("amend_last,wallet,max_open,lsamr,expected", [ (False, 120, 2, 0.5, [60, None]), (True, 120, 2, 0.5, [60, 58.8]), From 9b1028789980b8a0b6e46fa6df7f7d6503c8dd3a Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 15 May 2023 07:27:08 +0200 Subject: [PATCH 7/7] Improve typing --- freqtrade/freqtradebot.py | 4 ++-- freqtrade/resolvers/exchange_resolver.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/freqtrade/freqtradebot.py b/freqtrade/freqtradebot.py index ed13be5b3..21426623f 100644 --- a/freqtrade/freqtradebot.py +++ b/freqtrade/freqtradebot.py @@ -13,7 +13,7 @@ from schedule import Scheduler from freqtrade import constants from freqtrade.configuration import validate_config_consistency -from freqtrade.constants import BuySell, Config, LongShort +from freqtrade.constants import BuySell, Config, ExchangeConfig, LongShort from freqtrade.data.converter import order_book_to_dataframe from freqtrade.data.dataprovider import DataProvider from freqtrade.edge import Edge @@ -64,7 +64,7 @@ class FreqtradeBot(LoggingMixin): # Init objects self.config = config - exchange_config = deepcopy(config['exchange']) + exchange_config: ExchangeConfig = deepcopy(config['exchange']) # Remove credentials from original exchange config to avoid accidental credentail exposure remove_exchange_credentials(config['exchange'], True) diff --git a/freqtrade/resolvers/exchange_resolver.py b/freqtrade/resolvers/exchange_resolver.py index 6e5357da5..c5c4e1a68 100644 --- a/freqtrade/resolvers/exchange_resolver.py +++ b/freqtrade/resolvers/exchange_resolver.py @@ -2,10 +2,10 @@ This module loads custom exchanges """ import logging -from typing import Any, Dict, Optional +from typing import Optional import freqtrade.exchange as exchanges -from freqtrade.constants import Config +from freqtrade.constants import Config, ExchangeConfig from freqtrade.exchange import MAP_EXCHANGE_CHILDCLASS, Exchange from freqtrade.resolvers import IResolver @@ -20,7 +20,7 @@ class ExchangeResolver(IResolver): object_type = Exchange @staticmethod - def load_exchange(config: Config, *, exchange_config: Optional[Dict[str, Any]] = None, + def load_exchange(config: Config, *, exchange_config: Optional[ExchangeConfig] = None, validate: bool = True, load_leverage_tiers: bool = False) -> Exchange: """ Load the custom class from config parameter