Skip to content
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

multi_order_trades does not do what it says on the tin #683

Open
mzaja opened this issue Aug 15, 2023 · 1 comment
Open

multi_order_trades does not do what it says on the tin #683

mzaja opened this issue Aug 15, 2023 · 1 comment

Comments

@mzaja
Copy link
Contributor

mzaja commented Aug 15, 2023

This section also does not make sense to me. If self.multi_order_trades==True and the trade is already live, the rest of the validations is skipped.

def validate_order(self, runner_context: RunnerContext, order) -> bool:
# allow multiple orders per trade
if self.multi_order_trades:
if order.trade.id in runner_context.live_trades:
return True

Should this code not check if len(trade.orders) >= 1 and reject placement if self.multi_order_trades==False? Allowing multiple orders per trade is very different from allowing multiple trades per runner, so the validation should proceed with the other checks. If a trade does not have at least two orders (opening and closing), I would not even call it a trade but a straight bet.

Originally posted by @mzaja in #679 (comment)

@mzaja
Copy link
Contributor Author

mzaja commented Aug 15, 2023

Current behaviour

  • multi_order_trades is True: If a trade is already live allow it to be reused any number of times.
  • multi_order_trades is False: Validate the trade before order placement. It is possible to place any number of orders with a single trade as long as trade_count and live_trade_count are not exceeded. A Trade object can still be reused to place multiple orders.
  • multi_order_trades's value is irrelevant unless the number of trades or live trades is at the limit or has been exceeded, in which case it acts like force=True. Post bugfix in Bugfix/679 validation reused trades #682, this will change to "irrelevant unless the number of trades or live trades has been exceeded"

Expected behaviour

  • multi_order_trades is True: Do nothing - proceeed with validation as usual.
  • multi_order_trades is False: Reject order if len(trade.orders) >= 1.

Proposal

Since the current implementation of multi_order_trades argument does not do much on its own and certainly does not do what the name suggest, I propose replacing it with per-trade order limits: max_order_count and max_live_order_count. These would behave on the trade level the same as max_trade_count and max_live_trade_count behave on the strategy level. To maintain backwards compatibility, they should be None by default and optionally set by the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant