-
Notifications
You must be signed in to change notification settings - Fork 64
[TradingMode] fix convertor with simulated liimit order #1127
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
Conversation
octobot_trading/modes/modes_util.py
Outdated
| initialized_order = await trading_mode.create_order(order) | ||
| if ( | ||
| isinstance(initialized_order, trading_personal_data.LimitOrder) | ||
| and initialized_order.trader.exchange_manager.trader.simulate |
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.
| and initialized_order.trader.exchange_manager.trader.simulate | |
| and initialized_order.is_simulated() |
I don't remember if this method exists
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.
right, there is a simulated attribute, I update this
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.
up
octobot_trading/modes/modes_util.py
Outdated
| initialized_order = await trading_mode.create_order(order) | ||
| if isinstance(initialized_order, trading_personal_data.LimitOrder) and initialized_order.simulated: | ||
| # on simulator, this order should be instantly filled now as its price is meant to be instantly filled | ||
| await initialized_order.on_fill() |
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.
Shouldn't we create a wrapper like create_market_order that creates a market order or a limit order (depending on what's available for this exchange) and that triggers order.on_fill() for the limit order.
It seems to be an issue that could happen again in another function who would do the same thing
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.
right, it's now in convert_with_market_or_limit_order
Herklos
left a comment
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.
👍
No description provided.