-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TradingModes] init optimize_initial_portfolio #1073
Conversation
# ExchangeError('kucoin This user is not a master user') | ||
if "not a master user" not in str(err): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😮
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a check to be sure kucoin doesn't change its behavior ^^
return [], tickers | ||
async with self.producers[0].producer_exchange_wide_lock(self.exchange_manager): | ||
self.logger.info(f"Optimizing portfolio: selling {sellable_assets} to buy {target_asset}") | ||
created_orders, tickers = await trading_modes.convert_assets_to_target_asset( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
# 1. cancel open orders | ||
cancelled_orders = [] | ||
for symbol in self.exchange_manager.exchange_config.traded_symbol_pairs: | ||
if producer.get_symbol_trading_config(symbol) is not None: | ||
configured_pairs.append(symbol) | ||
pair_bases.add(symbol_util.parse_symbol(symbol).base) | ||
for order in self.exchange_manager.exchange_personal_data.orders_manager.get_open_orders( | ||
symbol=symbol | ||
): | ||
if not (order.is_cancelled() or order.is_closed()): | ||
await self.cancel_order(order) | ||
cancelled_orders.append(order) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't there already be a method for canceling all open orders on traded symbols?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is but we need to do it with other conditions in if that are irrelevant here and using self.cancel_order
so I thing it's better to do it this way
@@ -251,6 +253,92 @@ def get_is_symbol_wildcard(cls) -> bool: | |||
def set_default_config(self): | |||
raise RuntimeError(f"Impossible to start {self.get_name()} without a valid configuration file.") | |||
|
|||
async def optimize_initial_portfolio(self, sellable_assets: list, tickers: dict = None) -> (list, dict): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should split it into multiple methods to increase readability
@@ -251,6 +253,92 @@ def get_is_symbol_wildcard(cls) -> bool: | |||
def set_default_config(self): | |||
raise RuntimeError(f"Impossible to start {self.get_name()} without a valid configuration file.") | |||
|
|||
async def optimize_initial_portfolio(self, sellable_assets: list, tickers: dict = None) -> (list, dict): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think we should add more exception catching because if something fails it will interrupt the entire process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed ! 👍
return [cancelled_orders, part_1_orders, part_2_orders], tickers | ||
return [cancelled_orders, part_1_orders, part_2_orders] | ||
|
||
async def _cancel_associated_orders(self, producer, pair_bases) -> list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -251,6 +253,110 @@ def get_is_symbol_wildcard(cls) -> bool: | |||
def set_default_config(self): | |||
raise RuntimeError(f"Impossible to start {self.get_name()} without a valid configuration file.") | |||
|
|||
async def single_exchange_process_optimize_initial_portfolio( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
# check portfolio is rebalanced | ||
final_portfolio = exchange_manager.exchange_personal_data.portfolio_manager.portfolio.portfolio | ||
assert final_portfolio["BTC"].available == decimal.Decimal('5.539455') # 5.545 - fees | ||
assert final_portfolio["USD"].available == decimal.Decimal("5545") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
requires Drakkar-Software/OctoBot-Trading#984
todo: