chore(api)!: Refactor limit and market order models

coordinator/src/node/expired_positions.rs

@@ -11,10 +11,9 @@ use anyhow::Result;
 use commons::average_execution_price;
 use commons::Match;
 use commons::MatchState;
-use commons::NewOrder;
+use commons::NewMarketOrder;
 use commons::OrderReason;
 use commons::OrderState;
-use commons::OrderType;
 use rust_decimal::prelude::FromPrimitive;
 use rust_decimal::prelude::ToPrimitive;
 use rust_decimal::Decimal;
@@ -80,25 +79,21 @@ pub async fn close(node: Node, trading_sender: mpsc::Sender<NewOrderMessage>) ->
 
         tracing::debug!(trader_pk=%position.trader, %position.expiry_timestamp, "Attempting to close expired position");
 
-        let new_order = NewOrder {
+        let new_order = NewMarketOrder {
             id: uuid::Uuid::new_v4(),
             contract_symbol: position.contract_symbol,
-            // TODO(holzeis): we should not have to set the price for a market order. we propably
-            // need separate models for a limit and a market order.
-            price: Decimal::ZERO,
             quantity: Decimal::try_from(position.quantity).expect("to fit into decimal"),
             trader_id: position.trader,
             direction: position.trader_direction.opposite(),
             leverage: Decimal::from_f32(position.trader_leverage).expect("to fit into decimal"),
-            order_type: OrderType::Market,
             // This order can basically not expire, but if the user does not come back online within
             // a certain time period we can assume the channel to be abandoned and we should force
             // close.
             expiry: OffsetDateTime::now_utc().add(EXPIRED_POSITION_TIMEOUT),
             stable: position.stable,
         };
 
-        let order = orders::insert(&mut conn, new_order.clone(), OrderReason::Expired)
+        let order = orders::insert_market_order(&mut conn, new_order.clone(), OrderReason::Expired)
             .map_err(|e| anyhow!(e))
             .context("Failed to insert expired order into DB")?;
 

coordinator/src/orderbook/db/orders.rs

@@ -7,7 +7,8 @@ use crate::orderbook::db::custom_types::OrderType;
 use crate::schema::matches;
 use crate::schema::orders;
 use bitcoin::secp256k1::PublicKey;
-use commons::NewOrder as OrderbookNewOrder;
+use commons::NewLimitOrder;
+use commons::NewMarketOrder;
 use commons::Order as OrderbookOrder;
 use commons::OrderReason as OrderBookOrderReason;
 use commons::OrderState as OrderBookOrderState;
@@ -159,8 +160,8 @@ struct NewOrder {
     pub stable: bool,
 }
 
-impl From<OrderbookNewOrder> for NewOrder {
-    fn from(value: OrderbookNewOrder) -> Self {
+impl From<NewLimitOrder> for NewOrder {
+    fn from(value: NewLimitOrder) -> Self {
         NewOrder {
             trader_order_id: value.id,
             price: value
@@ -175,7 +176,33 @@ impl From<OrderbookNewOrder> for NewOrder {
                 .round_dp(2)
                 .to_f32()
                 .expect("To be able to convert decimal to f32"),
-            order_type: value.order_type.into(),
+            order_type: OrderType::Limit,
+            expiry: value.expiry,
+            order_reason: OrderReason::Manual,
+            contract_symbol: value.contract_symbol.into(),
+            leverage: value
+                .leverage
+                .to_f32()
+                .expect("To be able to convert decimal to f32"),
+            stable: value.stable,
+        }
+    }
+}
+
+impl From<NewMarketOrder> for NewOrder {
+    fn from(value: NewMarketOrder) -> Self {
+        NewOrder {
+            trader_order_id: value.id,
+            // TODO: it would be cool to get rid of this as well
+            price: 0.0,
+            trader_id: value.trader_id.to_string(),
+            direction: value.direction.into(),
+            quantity: value
+                .quantity
+                .round_dp(2)
+                .to_f32()
+                .expect("To be able to convert decimal to f32"),
+            order_type: OrderType::Market,
             expiry: value.expiry,
             order_reason: OrderReason::Manual,
             contract_symbol: value.contract_symbol.into(),
@@ -242,9 +269,26 @@ pub fn get_all_orders(
 }
 
 /// Returns the number of affected rows: 1.
-pub fn insert(
+pub fn insert_limit_order(
+    conn: &mut PgConnection,
+    order: NewLimitOrder,
+    order_reason: OrderBookOrderReason,
+) -> QueryResult<OrderbookOrder> {
+    let new_order = NewOrder {
+        order_reason: OrderReason::from(order_reason),
+        ..NewOrder::from(order)
+    };
+    let order: Order = diesel::insert_into(orders::table)
+        .values(new_order)
+        .get_result(conn)?;
+
+    Ok(OrderbookOrder::from(order))
+}
+
+/// Returns the number of affected rows: 1.
+pub fn insert_market_order(
     conn: &mut PgConnection,
-    order: OrderbookNewOrder,
+    order: NewMarketOrder,
     order_reason: OrderBookOrderReason,
 ) -> QueryResult<OrderbookOrder> {
     let new_order = NewOrder {

coordinator/src/orderbook/routes.rs

@@ -14,6 +14,7 @@ use axum::extract::State;
 use axum::response::IntoResponse;
 use axum::Json;
 use commons::Message;
+use commons::NewOrder;
 use commons::NewOrderRequest;
 use commons::Order;
 use commons::OrderReason;
@@ -76,7 +77,7 @@ pub async fn post_order(
     let new_order = new_order_request.value;
 
     // TODO(holzeis): We should add a similar check eventually for limit orders (makers).
-    if new_order.order_type == OrderType::Market {
+    if let NewOrder::Market(new_order) = &new_order {
         let mut conn = state
             .pool
             .get()
@@ -87,7 +88,7 @@ pub async fn post_order(
 
     let settings = state.settings.read().await;
 
-    if OrderType::Limit == new_order.order_type {
+    if let NewOrder::Limit(new_order) = &new_order {
         if settings.whitelist_enabled && !settings.whitelisted_makers.contains(&new_order.trader_id)
         {
             tracing::warn!(
@@ -105,12 +106,20 @@ pub async fn post_order(
     }
 
     let pool = state.pool.clone();
+    let new_order = new_order.clone();
     let order = spawn_blocking(move || {
         let mut conn = pool.get()?;
 
-        let order = orders::insert(&mut conn, new_order.clone(), OrderReason::Manual)
-            .map_err(|e| anyhow!(e))
-            .context("Failed to insert new order into DB")?;
+        let order = match new_order {
+            NewOrder::Market(o) => {
+                orders::insert_market_order(&mut conn, o.clone(), OrderReason::Manual)
+            }
+            NewOrder::Limit(o) => {
+                orders::insert_limit_order(&mut conn, o.clone(), OrderReason::Manual)
+            }
+        }
+        .map_err(|e| anyhow!(e))
+        .context("Failed to insert new order into DB")?;
 
         anyhow::Ok(order)
     })

coordinator/src/orderbook/tests/sample_test.rs

@@ -3,10 +3,10 @@ use crate::orderbook::db::orders;
 use crate::orderbook::tests::setup_db;
 use crate::orderbook::tests::start_postgres;
 use bitcoin::secp256k1::PublicKey;
-use commons::NewOrder;
+use commons::NewLimitOrder;
+use commons::NewMarketOrder;
 use commons::OrderReason;
 use commons::OrderState;
-use commons::OrderType;
 use rust_decimal_macros::dec;
 use std::str::FromStr;
 use testcontainers::clients::Cli;
@@ -24,12 +24,9 @@ async fn crud_test() {
 
     let mut conn = setup_db(conn_spec);
 
-    let order = orders::insert(
+    let order = orders::insert_limit_order(
         &mut conn,
-        dummy_order(
-            OffsetDateTime::now_utc() + Duration::minutes(1),
-            OrderType::Market,
-        ),
+        dummy_limit_order(OffsetDateTime::now_utc() + Duration::minutes(1)),
         OrderReason::Manual,
     )
     .unwrap();
@@ -50,43 +47,39 @@ async fn test_all_limit_orders() {
     let orders = orders::all_limit_orders(&mut conn).unwrap();
     assert!(orders.is_empty());
 
-    orders::insert(
-        &mut conn,
-        dummy_order(
-            OffsetDateTime::now_utc() + Duration::minutes(1),
-            OrderType::Market,
-        ),
-        OrderReason::Manual,
-    )
-    .unwrap();
+    let order_1 = dummy_limit_order(OffsetDateTime::now_utc() + Duration::minutes(1));
+    orders::insert_limit_order(&mut conn, order_1, OrderReason::Manual).unwrap();
 
-    let second_order = orders::insert(
-        &mut conn,
-        dummy_order(
-            OffsetDateTime::now_utc() + Duration::minutes(1),
-            OrderType::Limit,
-        ),
-        OrderReason::Manual,
-    )
-    .unwrap();
-    orders::set_order_state(&mut conn, second_order.id, OrderState::Failed).unwrap();
+    let order_2 = dummy_market_order(OffsetDateTime::now_utc() + Duration::minutes(1));
+    orders::insert_market_order(&mut conn, order_2, OrderReason::Manual).unwrap();
 
-    orders::insert(
-        &mut conn,
-        dummy_order(
-            OffsetDateTime::now_utc() + Duration::minutes(1),
-            OrderType::Limit,
-        ),
-        OrderReason::Manual,
-    )
-    .unwrap();
+    let order_3 = dummy_limit_order(OffsetDateTime::now_utc() + Duration::minutes(1));
+    let second_limit_order =
+        orders::insert_limit_order(&mut conn, order_3, OrderReason::Manual).unwrap();
+    orders::set_order_state(&mut conn, second_limit_order.id, OrderState::Failed).unwrap();
 
     let orders = orders::all_limit_orders(&mut conn).unwrap();
     assert_eq!(orders.len(), 1);
 }
 
-fn dummy_order(expiry: OffsetDateTime, order_type: OrderType) -> NewOrder {
-    NewOrder {
+fn dummy_market_order(expiry: OffsetDateTime) -> NewMarketOrder {
+    NewMarketOrder {
+        id: Uuid::new_v4(),
+        trader_id: PublicKey::from_str(
+            "027f31ebc5462c1fdce1b737ecff52d37d75dea43ce11c74d25aa297165faa2007",
+        )
+        .unwrap(),
+        direction: Direction::Long,
+        quantity: dec!(100.0),
+        expiry,
+        contract_symbol: trade::ContractSymbol::BtcUsd,
+        leverage: dec!(1.0),
+        stable: false,
+    }
+}
+
+fn dummy_limit_order(expiry: OffsetDateTime) -> NewLimitOrder {
+    NewLimitOrder {
         id: Uuid::new_v4(),
         price: dec!(20000.00),
         trader_id: PublicKey::from_str(
@@ -95,7 +88,6 @@ fn dummy_order(expiry: OffsetDateTime, order_type: OrderType) -> NewOrder {
         .unwrap(),
         direction: Direction::Long,
         quantity: dec!(100.0),
-        order_type,
         expiry,
         contract_symbol: trade::ContractSymbol::BtcUsd,
         leverage: dec!(1.0),