Improve validations for MarketIfTouchedOrder (#2577)

Author: nicolad

crates/model/src/orders/market_if_touched.rs

@@ -16,7 +16,10 @@
 use std::ops::{Deref, DerefMut};
 
 use indexmap::IndexMap;
-use nautilus_core::{UUID4, UnixNanos};
+use nautilus_core::{
+    UUID4, UnixNanos,
+    correctness::{FAILED, check_predicate_false},
+};
 use rust_decimal::Decimal;
 use serde::{Deserialize, Serialize};
 use ustr::Ustr;
@@ -32,7 +35,10 @@ use crate::{
         AccountId, ClientOrderId, ExecAlgorithmId, InstrumentId, OrderListId, PositionId,
         StrategyId, Symbol, TradeId, TraderId, Venue, VenueOrderId,
     },
-    types::{Currency, Money, Price, Quantity},
+    types::{
+        Currency, Money, Price, Quantity, price::check_positive_price,
+        quantity::check_positive_quantity,
+    },
 };
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -54,7 +60,7 @@ pub struct MarketIfTouchedOrder {
 impl MarketIfTouchedOrder {
     /// Creates a new [`MarketIfTouchedOrder`] instance.
     #[allow(clippy::too_many_arguments)]
-    pub fn new(
+    pub fn new_checked(
         trader_id: TraderId,
         strategy_id: StrategyId,
         instrument_id: InstrumentId,
@@ -80,7 +86,22 @@ impl MarketIfTouchedOrder {
         tags: Option<Vec<Ustr>>,
         init_id: UUID4,
         ts_init: UnixNanos,
-    ) -> Self {
+    ) -> anyhow::Result<Self> {
+        check_positive_quantity(quantity, "quantity")?;
+        check_positive_price(trigger_price, "trigger_price")?;
+
+        if let Some(disp) = display_qty {
+            check_positive_quantity(disp, "display_qty")?;
+            check_predicate_false(disp > quantity, "`display_qty` may not exceed `quantity`")?;
+        }
+
+        if time_in_force == TimeInForce::Gtd {
+            check_predicate_false(
+                expire_time.unwrap_or_default() == 0,
+                "Condition failed: `expire_time` is required for `GTD` order",
+            )?;
+        }
+
         let init_order = OrderInitialized::new(
             trader_id,
             strategy_id,
@@ -116,7 +137,8 @@ impl MarketIfTouchedOrder {
             exec_spawn_id,
             tags,
         );
-        Self {
+
+        Ok(Self {
             core: OrderCore::new(init_order),
             trigger_price,
             trigger_type,
@@ -125,7 +147,65 @@ impl MarketIfTouchedOrder {
             trigger_instrument_id,
             is_triggered: false,
             ts_triggered: None,
-        }
+        })
+    }
+
+    #[allow(clippy::too_many_arguments)]
+    pub fn new(
+        trader_id: TraderId,
+        strategy_id: StrategyId,
+        instrument_id: InstrumentId,
+        client_order_id: ClientOrderId,
+        order_side: OrderSide,
+        quantity: Quantity,
+        trigger_price: Price,
+        trigger_type: TriggerType,
+        time_in_force: TimeInForce,
+        expire_time: Option<UnixNanos>,
+        reduce_only: bool,
+        quote_quantity: bool,
+        display_qty: Option<Quantity>,
+        emulation_trigger: Option<TriggerType>,
+        trigger_instrument_id: Option<InstrumentId>,
+        contingency_type: Option<ContingencyType>,
+        order_list_id: Option<OrderListId>,
+        linked_order_ids: Option<Vec<ClientOrderId>>,
+        parent_order_id: Option<ClientOrderId>,
+        exec_algorithm_id: Option<ExecAlgorithmId>,
+        exec_algorithm_params: Option<IndexMap<Ustr, Ustr>>,
+        exec_spawn_id: Option<ClientOrderId>,
+        tags: Option<Vec<Ustr>>,
+        init_id: UUID4,
+        ts_init: UnixNanos,
+    ) -> Self {
+        Self::new_checked(
+            trader_id,
+            strategy_id,
+            instrument_id,
+            client_order_id,
+            order_side,
+            quantity,
+            trigger_price,
+            trigger_type,
+            time_in_force,
+            expire_time,
+            reduce_only,
+            quote_quantity,
+            display_qty,
+            emulation_trigger,
+            trigger_instrument_id,
+            contingency_type,
+            order_list_id,
+            linked_order_ids,
+            parent_order_id,
+            exec_algorithm_id,
+            exec_algorithm_params,
+            exec_spawn_id,
+            tags,
+            init_id,
+            ts_init,
+        )
+        .expect(FAILED)
     }
 }
 
@@ -429,13 +509,13 @@ impl From<OrderInitialized> for MarketIfTouchedOrder {
             event.order_side,
             event.quantity,
             event
-                .trigger_price // TODO: Improve this error, model order domain errors
-                .expect(
-                    "Error initializing order: `trigger_price` was `None` for `MarketIfTouchedOrder`",
-                ),
-            event
-                .trigger_type
-                .expect("Error initializing order: `trigger_type` was `None` for `MarketIfTouchedOrder`"),
+            .trigger_price // TODO: Improve this error, model order domain errors
+            .expect(
+                "Error initializing order: `trigger_price` was `None` for `MarketIfTouchedOrder`",
+            ),
+            event.trigger_type.expect(
+                "Error initializing order: `trigger_type` was `None` for `MarketIfTouchedOrder`",
+            ),
             event.time_in_force,
             event.expire_time,
             event.reduce_only,
@@ -456,3 +536,69 @@ impl From<OrderInitialized> for MarketIfTouchedOrder {
         )
     }
 }
+
+////////////////////////////////////////////////////////////////////////////////
+// Tests
+////////////////////////////////////////////////////////////////////////////////
+#[cfg(test)]
+mod tests {
+    use rstest::rstest;
+
+    use crate::{
+        enums::{OrderSide, OrderType, TimeInForce, TriggerType},
+        instruments::{CurrencyPair, stubs::*},
+        orders::builder::OrderTestBuilder,
+        types::{Price, Quantity},
+    };
+
+    #[rstest]
+    fn ok(audusd_sim: CurrencyPair) {
+        let _ = OrderTestBuilder::new(OrderType::MarketIfTouched)
+            .instrument_id(audusd_sim.id)
+            .side(OrderSide::Buy)
+            .trigger_price(Price::from("30000"))
+            .trigger_type(TriggerType::LastPrice)
+            .quantity(Quantity::from(1))
+            .build();
+    }
+
+    #[rstest]
+    #[should_panic(
+        expected = "Condition failed: invalid `Quantity` for 'quantity' not positive, was 0"
+    )]
+    fn quantity_zero(audusd_sim: CurrencyPair) {
+        let _ = OrderTestBuilder::new(OrderType::MarketIfTouched)
+            .instrument_id(audusd_sim.id)
+            .side(OrderSide::Buy)
+            .trigger_price(Price::from("30000"))
+            .trigger_type(TriggerType::LastPrice)
+            .quantity(Quantity::from(0))
+            .build();
+    }
+
+    #[rstest]
+    #[should_panic(expected = "Condition failed: `expire_time` is required for `GTD` order")]
+    fn gtd_without_expire(audusd_sim: CurrencyPair) {
+        let _ = OrderTestBuilder::new(OrderType::MarketIfTouched)
+            .instrument_id(audusd_sim.id)
+            .side(OrderSide::Buy)
+            .trigger_price(Price::from("30000"))
+            .trigger_type(TriggerType::LastPrice)
+            .quantity(Quantity::from(1))
+            .time_in_force(TimeInForce::Gtd)
+            .build();
+    }
+
+    #[rstest]
+    #[should_panic(expected = "`display_qty` may not exceed `quantity`")]
+    fn display_qty_gt_quantity(audusd_sim: CurrencyPair) {
+        let _ = OrderTestBuilder::new(OrderType::MarketIfTouched)
+            .instrument_id(audusd_sim.id)
+            .side(OrderSide::Buy)
+            .trigger_price(Price::from("30000"))
+            .trigger_type(TriggerType::LastPrice)
+            .quantity(Quantity::from(1))
+            .display_qty(Quantity::from(2))
+            .build();
+    }
+}