betcode-org/flumine

multi_order_trades does not do what it says on the tin

Opened this issue · 1 comments

mzaja commented

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 commented

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 #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.