diff --git a/crates/orderbook/src/database/orders.rs b/crates/orderbook/src/database/orders.rs index 9972191210..809872c549 100644 --- a/crates/orderbook/src/database/orders.rs +++ b/crates/orderbook/src/database/orders.rs @@ -44,9 +44,8 @@ use { signing_scheme_from, signing_scheme_into, }, - fee::FeeParameters, order_quoting::Quote, - order_validation::{is_order_outside_market_price, Amounts, LimitOrderCounting}, + order_validation::LimitOrderCounting, }, sqlx::{types::BigDecimal, Connection, PgConnection}, std::convert::TryInto, @@ -63,7 +62,7 @@ pub trait OrderStoring: Send + Sync { &self, old_order: &OrderUid, new_order: &Order, - new_quote: Option, + new_quote: &Quote, ) -> Result<(), InsertionError>; async fn orders_for_tx(&self, tx_hash: &H256) -> Result>; async fn single_order(&self, uid: &OrderUid) -> Result>; @@ -272,7 +271,7 @@ impl OrderStoring for Postgres { &self, old_order: &model::order::OrderUid, new_order: &model::order::Order, - new_quote: Option, + new_quote: &Quote, ) -> anyhow::Result<(), super::orders::InsertionError> { let _timer = super::Metrics::get() .database_queries @@ -281,6 +280,7 @@ impl OrderStoring for Postgres { let old_order = *old_order; let new_order = new_order.clone(); + let new_quote = new_quote.clone(); let mut connection = self.pool.acquire().await?; connection .transaction(move |ex| { @@ -292,11 +292,8 @@ impl OrderStoring for Postgres { ) .await?; insert_order(&new_order, ex).await?; - if let Some(quote) = new_quote { - insert_quote(&new_order.metadata.uid, "e, ex).await?; - } + insert_quote(&new_order.metadata.uid, &new_quote, ex).await?; Self::insert_order_app_data(&new_order, ex).await?; - Ok(()) } .boxed() @@ -367,33 +364,12 @@ impl LimitOrderCounting for Postgres { .start_timer(); let mut ex = self.pool.acquire().await?; - Ok(database::orders::user_orders_with_quote( + Ok(database::orders::count_limit_orders_by_owner( &mut ex, now_in_epoch_seconds().into(), &ByteArray(owner.0), ) .await? - .into_iter() - .filter(|order_with_quote| { - is_order_outside_market_price( - &Amounts { - sell: big_decimal_to_u256(&order_with_quote.order_sell_amount).unwrap(), - buy: big_decimal_to_u256(&order_with_quote.order_buy_amount).unwrap(), - fee: big_decimal_to_u256(&order_with_quote.order_fee_amount).unwrap(), - }, - &Amounts { - sell: big_decimal_to_u256(&order_with_quote.quote_sell_amount).unwrap(), - buy: big_decimal_to_u256(&order_with_quote.quote_buy_amount).unwrap(), - fee: FeeParameters { - gas_amount: order_with_quote.quote_gas_amount, - gas_price: order_with_quote.quote_gas_price, - sell_token_price: order_with_quote.quote_sell_token_price, - } - .fee(), - }, - ) - }) - .count() .try_into() .unwrap()) } @@ -797,7 +773,8 @@ mod tests { }, ..Default::default() }; - db.replace_order(&old_order.metadata.uid, &new_order, None) + let quote = Quote::default(); + db.replace_order(&old_order.metadata.uid, &new_order, "e) .await .unwrap(); @@ -858,8 +835,9 @@ mod tests { db.insert_order(&new_order, None).await.unwrap(); // Attempt to replace an old order with one that already exists should fail. + let quote = Quote::default(); let err = db - .replace_order(&old_order.metadata.uid, &new_order, None) + .replace_order(&old_order.metadata.uid, &new_order, "e) .await .unwrap_err(); assert!(matches!(err, InsertionError::DuplicatedRecord)); diff --git a/crates/orderbook/src/orderbook.rs b/crates/orderbook/src/orderbook.rs index 3b1f44f555..218db0a5b5 100644 --- a/crates/orderbook/src/orderbook.rs +++ b/crates/orderbook/src/orderbook.rs @@ -216,10 +216,10 @@ impl Orderbook { if let Some(old_order) = replaced_order { self.replace_order(order, old_order, quote).await } else { - let quote_id = quote.as_ref().and_then(|quote| quote.id); + let quote_id = quote.id; self.database - .insert_order(&order, quote) + .insert_order(&order, Some(quote)) .await .map_err(|err| AddOrderError::from_insertion(err, &order))?; Metrics::on_order_operation(&order, OrderOperation::Created); @@ -346,7 +346,7 @@ impl Orderbook { &self, validated_new_order: Order, old_order: Order, - quote: Option, + quote: Quote, ) -> Result<(OrderUid, Option), AddOrderError> { // Replacement order signatures need to be validated meaning we cannot // accept `PreSign` orders, otherwise anyone can cancel a user order by @@ -363,10 +363,10 @@ impl Orderbook { return Err(AddOrderError::InvalidReplacement); } - let quote_id = quote.as_ref().and_then(|quote| quote.id); + let quote_id = quote.id; self.database - .replace_order(&old_order.metadata.uid, &validated_new_order, quote) + .replace_order(&old_order.metadata.uid, &validated_new_order, "e) .await .map_err(|err| AddOrderError::from_insertion(err, &validated_new_order))?; Metrics::on_order_operation(&old_order, OrderOperation::Cancelled); diff --git a/crates/shared/src/order_quoting.rs b/crates/shared/src/order_quoting.rs index 7644026f25..f938698876 100644 --- a/crates/shared/src/order_quoting.rs +++ b/crates/shared/src/order_quoting.rs @@ -18,7 +18,7 @@ use { futures::TryFutureExt as _, gas_estimation::GasPriceEstimating, model::{ - order::{OrderClass, OrderKind}, + order::OrderKind, quote::{OrderQuoteRequest, OrderQuoteSide, QuoteId, QuoteSigningScheme, SellAmount}, }, number::conversions::big_decimal_to_u256, @@ -543,7 +543,6 @@ impl From<&OrderQuoteRequest> for PreOrderData { buy_token_balance: quote_request.buy_token_balance, sell_token_balance: quote_request.sell_token_balance, signing_scheme: quote_request.signing_scheme.into(), - class: OrderClass::Market, } } } diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index f7939b8b93..30f1ee67b9 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -100,7 +100,7 @@ pub trait OrderValidating: Send + Sync { domain_separator: &DomainSeparator, settlement_contract: H160, full_app_data_override: Option, - ) -> Result<(Order, Option), ValidationError>; + ) -> Result<(Order, Quote), ValidationError>; } #[derive(Debug)] @@ -268,7 +268,6 @@ pub struct PreOrderData { pub buy_token_balance: BuyTokenDestination, pub sell_token_balance: SellTokenSource, pub signing_scheme: SigningScheme, - pub class: OrderClass, } fn actual_receiver(owner: H160, order: &OrderData) -> H160 { @@ -296,10 +295,6 @@ impl PreOrderData { buy_token_balance: order.buy_token_balance, sell_token_balance: order.sell_token_balance, signing_scheme, - class: match order.fee_amount.is_zero() { - true => OrderClass::Limit, - false => OrderClass::Market, - }, } } } @@ -412,10 +407,6 @@ impl OrderValidating for OrderValidator { return Err(PartialValidationError::Forbidden); } - if order.class == OrderClass::Market && order.partially_fillable { - return Err(PartialValidationError::UnsupportedOrderType); - } - if order.buy_token_balance != BuyTokenDestination::Erc20 { return Err(PartialValidationError::UnsupportedBuyTokenDestination( order.buy_token_balance, @@ -512,7 +503,7 @@ impl OrderValidating for OrderValidator { domain_separator: &DomainSeparator, settlement_contract: H160, full_app_data_override: Option, - ) -> Result<(Order, Option), ValidationError> { + ) -> Result<(Order, Quote), ValidationError> { // Happens before signature verification because a miscalculated app data hash // by the API user would lead to being unable to validate the signature below. let app_data = self.validate_app_data(&order.app_data, &full_app_data_override)?; @@ -558,7 +549,6 @@ impl OrderValidating for OrderValidator { } let pre_order = PreOrderData::from_order_creation(owner, &data, signing_scheme); - let class = pre_order.class; self.partial_validate(pre_order) .await .map_err(ValidationError::Partial)?; @@ -639,93 +629,38 @@ impl OrderValidating for OrderValidator { // Check if we need to re-classify the market order if it is outside the market // price. We consider out-of-price orders as liquidity orders. See // . - let (class, quote) = match class { - // This has to be here in order to keep the previous behaviour - OrderClass::Market => { - let quote = get_quote_and_check_fee( - &*self.quoter, - "e_parameters, - order.quote_id, - Some(data.fee_amount), - self.market_orders_deprecation_date, - ) - .await?; - tracing::debug!( - ?uid, - ?order, - ?quote, - "checking if order is outside market price" - ); - if is_order_outside_market_price( - &Amounts { - sell: data.sell_amount, - buy: data.buy_amount, - fee: data.fee_amount, - }, - &Amounts { - sell: quote.sell_amount, - buy: quote.buy_amount, - fee: quote.fee_amount, - }, - ) { - tracing::debug!(%uid, ?owner, ?class, "order being flagged as outside market price"); - (OrderClass::Limit, Some(quote)) - } else { - (class, Some(quote)) - } - } - OrderClass::Limit => { - let quote = get_quote_and_check_fee( - &*self.quoter, - "e_parameters, - order.quote_id, - None, - self.market_orders_deprecation_date, - ) - .await?; - // If the order is not "In-Market", check for the limit orders - if is_order_outside_market_price( - &Amounts { - sell: data.sell_amount, - buy: data.buy_amount, - fee: data.fee_amount, - }, - &Amounts { - sell: quote.sell_amount, - buy: quote.buy_amount, - fee: quote.fee_amount, - }, - ) { - self.check_max_limit_orders(owner).await?; - } - (class, Some(quote)) - } - OrderClass::Liquidity => { - let quote = get_quote_and_check_fee( - &*self.quoter, - "e_parameters, - order.quote_id, - None, - self.market_orders_deprecation_date, - ) - .await?; - // If the order is not "In-Market", check for the limit orders - if is_order_outside_market_price( - &Amounts { - sell: data.sell_amount, - buy: data.buy_amount, - fee: data.fee_amount, - }, - &Amounts { - sell: quote.sell_amount, - buy: quote.buy_amount, - fee: quote.fee_amount, - }, - ) { - self.check_max_limit_orders(owner).await?; - } - (OrderClass::Limit, None) - } + let fee_amount = (!data.fee_amount.is_zero()).then(|| data.fee_amount); + let quote = get_quote_and_check_fee( + &*self.quoter, + "e_parameters, + order.quote_id, + fee_amount, + self.market_orders_deprecation_date, + ) + .await?; + tracing::debug!( + ?uid, + ?order, + ?quote, + "checking if order is outside market price" + ); + let class = if is_order_outside_market_price( + &Amounts { + sell: data.sell_amount, + buy: data.buy_amount, + fee: data.fee_amount, + }, + &Amounts { + sell: quote.sell_amount, + buy: quote.buy_amount, + fee: quote.fee_amount, + }, + ) { + tracing::debug!(%uid, ?owner, "order being flagged as outside market price"); + self.check_max_limit_orders(owner).await?; + OrderClass::Limit + } else { + OrderClass::Market }; let order = Order { @@ -791,12 +726,7 @@ impl OrderValidPeriodConfiguration { if order.signing_scheme == SigningScheme::PreSign { return Duration::MAX; } - - match order.class { - OrderClass::Market => self.max_market, - OrderClass::Limit => self.max_limit, - OrderClass::Liquidity => Duration::MAX, - } + self.max_limit } }