From a09364a65c3f052a35b460d25a9b2b5aaa93bd4d Mon Sep 17 00:00:00 2001 From: Richard Holzeis Date: Thu, 7 Sep 2023 21:54:56 +0200 Subject: [PATCH] chore: Remove taken from orders The taken flag is not needed anymore as it is replaced by a more elaborate order state. --- .../2023-09-08-090156_remove_taken/down.sql | 5 ++++ .../2023-09-08-090156_remove_taken/up.sql | 3 ++ coordinator/src/orderbook/db/orders.rs | 18 ++--------- .../src/orderbook/tests/sample_test.rs | 3 +- coordinator/src/orderbook/trading.rs | 5 ---- coordinator/src/schema.rs | 1 - crates/orderbook-commons/src/lib.rs | 2 -- crates/orderbook-commons/src/price.rs | 30 ++++++++----------- maker/src/schema.rs | 9 ------ 9 files changed, 26 insertions(+), 50 deletions(-) create mode 100644 coordinator/migrations/2023-09-08-090156_remove_taken/down.sql create mode 100644 coordinator/migrations/2023-09-08-090156_remove_taken/up.sql diff --git a/coordinator/migrations/2023-09-08-090156_remove_taken/down.sql b/coordinator/migrations/2023-09-08-090156_remove_taken/down.sql new file mode 100644 index 000000000..3a2d41c29 --- /dev/null +++ b/coordinator/migrations/2023-09-08-090156_remove_taken/down.sql @@ -0,0 +1,5 @@ +-- This file should undo anything in `up.sql` +ALTER TABLE "orders" + ADD COLUMN "taken" BOOLEAN NOT NULL DEFAULT FALSE; + +UPDATE orders SET taken = true WHERE order_state = 'Taken'; diff --git a/coordinator/migrations/2023-09-08-090156_remove_taken/up.sql b/coordinator/migrations/2023-09-08-090156_remove_taken/up.sql new file mode 100644 index 000000000..143165b08 --- /dev/null +++ b/coordinator/migrations/2023-09-08-090156_remove_taken/up.sql @@ -0,0 +1,3 @@ +-- This file should undo anything in `up.sql` +ALTER TABLE "orders" + DROP COLUMN "taken"; \ No newline at end of file diff --git a/coordinator/src/orderbook/db/orders.rs b/coordinator/src/orderbook/db/orders.rs index 4b46d730c..32f0c1f71 100644 --- a/coordinator/src/orderbook/db/orders.rs +++ b/coordinator/src/orderbook/db/orders.rs @@ -86,7 +86,6 @@ struct Order { pub trader_order_id: Uuid, pub price: f32, pub trader_id: String, - pub taken: bool, pub direction: Direction, pub quantity: f32, pub timestamp: OffsetDateTime, @@ -104,7 +103,6 @@ impl From for OrderbookOrder { id: value.trader_order_id, price: Decimal::from_f32(value.price).expect("To be able to convert f32 to decimal"), trader_id: value.trader_id.parse().expect("to have a valid pubkey"), - taken: value.taken, leverage: value.leverage, contract_symbol: value.contract_symbol.into(), direction: value.direction.into(), @@ -143,7 +141,6 @@ struct NewOrder { pub trader_order_id: Uuid, pub price: f32, pub trader_id: String, - pub taken: bool, pub direction: Direction, pub quantity: f32, pub order_type: OrderType, @@ -163,7 +160,6 @@ impl From for NewOrder { .to_f32() .expect("To be able to convert decimal to f32"), trader_id: value.trader_id.to_string(), - taken: false, direction: value.direction.into(), quantity: value .quantity @@ -196,13 +192,13 @@ pub fn all_by_direction_and_type( conn: &mut PgConnection, direction: OrderbookDirection, order_type: OrderBookOrderType, - taken: bool, filter_expired: bool, ) -> QueryResult> { let filters = orders::table .filter(orders::direction.eq(Direction::from(direction))) .filter(orders::order_type.eq(OrderType::from(order_type))) - .filter(orders::taken.eq(taken)); + .filter(orders::order_state.eq(OrderState::Open)); + let orders: Vec = if filter_expired { filters .filter(orders::expiry.gt(OffsetDateTime::now_utc())) @@ -270,17 +266,9 @@ pub fn set_order_state( id: Uuid, order_state: orderbook_commons::OrderState, ) -> QueryResult { - let mut is_taken = false; - if order_state == orderbook_commons::OrderState::Taken { - is_taken = true; - } - let order: Order = diesel::update(orders::table) .filter(orders::trader_order_id.eq(id)) - .set(( - orders::taken.eq(is_taken), - orders::order_state.eq(OrderState::from(order_state)), - )) + .set((orders::order_state.eq(OrderState::from(order_state)),)) .get_result(conn)?; Ok(OrderbookOrder::from(order)) diff --git a/coordinator/src/orderbook/tests/sample_test.rs b/coordinator/src/orderbook/tests/sample_test.rs index d92e93c95..67bbd65de 100644 --- a/coordinator/src/orderbook/tests/sample_test.rs +++ b/coordinator/src/orderbook/tests/sample_test.rs @@ -5,6 +5,7 @@ use crate::orderbook::tests::start_postgres; use bitcoin::secp256k1::PublicKey; use orderbook_commons::NewOrder; use orderbook_commons::OrderReason; +use orderbook_commons::OrderState; use orderbook_commons::OrderType; use rust_decimal_macros::dec; use std::str::FromStr; @@ -34,7 +35,7 @@ async fn crud_test() { .unwrap(); let order = orders::set_is_taken(&mut conn, order.id, true).unwrap(); - assert!(order.taken); + assert_eq!(order.order_state, OrderState::Taken); let deleted = orders::delete_with_id(&mut conn, order.id).unwrap(); assert_eq!(deleted, 1); diff --git a/coordinator/src/orderbook/trading.rs b/coordinator/src/orderbook/trading.rs index dfaa5797b..9cbded9b3 100644 --- a/coordinator/src/orderbook/trading.rs +++ b/coordinator/src/orderbook/trading.rs @@ -187,7 +187,6 @@ impl Trading { &mut conn, order.direction.opposite(), OrderType::Limit, - false, true, )?; @@ -480,7 +479,6 @@ pub mod tests { "027f31ebc5462c1fdce1b737ecff52d37d75dea43ce11c74d25aa297165faa2007", ) .unwrap(), - taken: false, direction: Direction::Long, leverage: 1.0, contract_symbol: ContractSymbol::BtcUsd, @@ -621,7 +619,6 @@ pub mod tests { "027f31ebc5462c1fdce1b737ecff52d37d75dea43ce11c74d25aa297165faa2007", ) .unwrap(), - taken: false, direction: Direction::Short, leverage: 1.0, contract_symbol: ContractSymbol::BtcUsd, @@ -696,7 +693,6 @@ pub mod tests { "027f31ebc5462c1fdce1b737ecff52d37d75dea43ce11c74d25aa297165faa2007", ) .unwrap(), - taken: false, direction: Direction::Short, leverage: 1.0, contract_symbol: ContractSymbol::BtcUsd, @@ -747,7 +743,6 @@ pub mod tests { "027f31ebc5462c1fdce1b737ecff52d37d75dea43ce11c74d25aa297165faa2007", ) .unwrap(), - taken: false, direction: Direction::Long, leverage: 1.0, contract_symbol: ContractSymbol::BtcUsd, diff --git a/coordinator/src/schema.rs b/coordinator/src/schema.rs index b63f97f58..155312272 100644 --- a/coordinator/src/schema.rs +++ b/coordinator/src/schema.rs @@ -92,7 +92,6 @@ diesel::table! { trader_order_id -> Uuid, price -> Float4, trader_id -> Text, - taken -> Bool, direction -> DirectionType, quantity -> Float4, timestamp -> Timestamptz, diff --git a/crates/orderbook-commons/src/lib.rs b/crates/orderbook-commons/src/lib.rs index 3dd77b8c7..fe4904b05 100644 --- a/crates/orderbook-commons/src/lib.rs +++ b/crates/orderbook-commons/src/lib.rs @@ -48,7 +48,6 @@ pub struct Order { pub leverage: f32, pub contract_symbol: ContractSymbol, pub trader_id: PublicKey, - pub taken: bool, pub direction: Direction, #[serde(with = "rust_decimal::serde::float")] pub quantity: Decimal, @@ -104,7 +103,6 @@ pub struct OrderResponse { #[serde(with = "rust_decimal::serde::float")] pub price: Decimal, pub trader_id: PublicKey, - pub taken: bool, pub direction: Direction, #[serde(with = "rust_decimal::serde::float")] pub quantity: Decimal, diff --git a/crates/orderbook-commons/src/price.rs b/crates/orderbook-commons/src/price.rs index aa9f266de..ad9783d84 100644 --- a/crates/orderbook-commons/src/price.rs +++ b/crates/orderbook-commons/src/price.rs @@ -1,4 +1,5 @@ use crate::Order; +use crate::OrderState; use crate::ToPrimitive; use rust_decimal::Decimal; use serde::Deserialize; @@ -55,7 +56,7 @@ fn best_price_for( let use_max = direction == Direction::Long; current_orders .iter() - .filter(|order| !order.taken && order.direction == direction) + .filter(|order| order.order_state == OrderState::Open && order.direction == direction) .map(|order| order.price.to_f64().expect("to represent decimal as f64")) // get the best price .fold(None, |acc, x| match acc { @@ -89,16 +90,11 @@ mod test { .unwrap() } - fn dummy_order(price: Decimal, direction: Direction, taken: bool) -> Order { - let mut order_state = OrderState::Open; - if taken { - order_state = OrderState::Taken; - } + fn dummy_order(price: Decimal, direction: Direction, order_state: OrderState) -> Order { Order { id: Uuid::new_v4(), price, trader_id: dummy_public_key(), - taken, direction, leverage: 1.0, contract_symbol: BtcUsd, @@ -114,10 +110,10 @@ mod test { #[test] fn test_best_bid_price() { let current_orders = vec![ - dummy_order(dec!(10_000), Direction::Long, false), - dummy_order(dec!(30_000), Direction::Long, false), - dummy_order(dec!(500_000), Direction::Long, true), // taken - dummy_order(dec!(50_000), Direction::Short, false), // wrong direction + dummy_order(dec!(10_000), Direction::Long, OrderState::Open), + dummy_order(dec!(30_000), Direction::Long, OrderState::Open), + dummy_order(dec!(500_000), Direction::Long, OrderState::Taken), // taken + dummy_order(dec!(50_000), Direction::Short, OrderState::Open), // wrong direction ]; assert_eq!(best_bid_price(¤t_orders, BtcUsd), Some(dec!(30_000))); } @@ -125,12 +121,12 @@ mod test { #[test] fn test_best_ask_price() { let current_orders = vec![ - dummy_order(dec!(10_000), Direction::Short, false), - dummy_order(dec!(30_000), Direction::Short, false), + dummy_order(dec!(10_000), Direction::Short, OrderState::Open), + dummy_order(dec!(30_000), Direction::Short, OrderState::Open), // ignored in the calculations - this order is taken - dummy_order(dec!(5_000), Direction::Short, true), + dummy_order(dec!(5_000), Direction::Short, OrderState::Taken), // ignored in the calculations - it's the bid price - dummy_order(dec!(50_000), Direction::Long, false), + dummy_order(dec!(50_000), Direction::Long, OrderState::Open), ]; assert_eq!(best_ask_price(¤t_orders, BtcUsd), Some(dec!(10_000))); } @@ -138,8 +134,8 @@ mod test { #[test] fn test_no_price() { let all_orders_taken = vec![ - dummy_order(dec!(10_000), Direction::Short, true), - dummy_order(dec!(30_000), Direction::Long, true), + dummy_order(dec!(10_000), Direction::Short, OrderState::Taken), + dummy_order(dec!(30_000), Direction::Long, OrderState::Taken), ]; assert_eq!(best_ask_price(&all_orders_taken, BtcUsd), None); diff --git a/maker/src/schema.rs b/maker/src/schema.rs index 34821d7a2..d9a52af7e 100644 --- a/maker/src/schema.rs +++ b/maker/src/schema.rs @@ -1,10 +1 @@ // @generated automatically by Diesel CLI. - -diesel::table! { - orders (id) { - id -> Int4, - price -> Int4, - maker_id -> Text, - taken -> Bool, - } -}