Skip to content

Commit

Permalink
Eliminate the order class in post order #2469
Browse files Browse the repository at this point in the history
  • Loading branch information
m-lord-renkse committed Mar 7, 2024
1 parent b22bbc7 commit a442095
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 234 deletions.
17 changes: 3 additions & 14 deletions crates/e2e/tests/e2e/limit_orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ async fn single_limit_order_test(web3: Web3) {
let services = Services::new(onchain.contracts()).await;
services.start_protocol(solver).await;

let order = OrderCreation {
OrderCreation {
sell_token: token_a.address(),
sell_amount: to_wei(10),
buy_token: token_b.address(),
Expand All @@ -153,9 +153,6 @@ async fn single_limit_order_test(web3: Web3) {
&onchain.contracts().domain_separator,
SecretKeyRef::from(&SecretKey::from_slice(trader_a.private_key()).unwrap()),
);
let order_id = services.create_order(&order).await.unwrap();
let limit_order = services.get_order(&order_id).await.unwrap();
assert_eq!(limit_order.metadata.class, OrderClass::Limit);

// Drive solution
tracing::info!("Waiting for trade.");
Expand Down Expand Up @@ -237,7 +234,7 @@ async fn two_limit_orders_test(web3: Web3) {
let services = Services::new(onchain.contracts()).await;
services.start_protocol(solver).await;

let order_a = OrderCreation {
OrderCreation {
sell_token: token_a.address(),
sell_amount: to_wei(10),
buy_token: token_b.address(),
Expand All @@ -251,12 +248,8 @@ async fn two_limit_orders_test(web3: Web3) {
&onchain.contracts().domain_separator,
SecretKeyRef::from(&SecretKey::from_slice(trader_a.private_key()).unwrap()),
);
let order_id = services.create_order(&order_a).await.unwrap();

let limit_order = services.get_order(&order_id).await.unwrap();
assert!(limit_order.metadata.class.is_limit());

let order_b = OrderCreation {
OrderCreation {
sell_token: token_b.address(),
sell_amount: to_wei(5),
buy_token: token_a.address(),
Expand All @@ -270,10 +263,6 @@ async fn two_limit_orders_test(web3: Web3) {
&onchain.contracts().domain_separator,
SecretKeyRef::from(&SecretKey::from_slice(trader_b.private_key()).unwrap()),
);
let order_id = services.create_order(&order_b).await.unwrap();

let limit_order = services.get_order(&order_id).await.unwrap();
assert!(limit_order.metadata.class.is_limit());

wait_for_condition(TIMEOUT, || async { services.solvable_orders().await == 2 })
.await
Expand Down
42 changes: 10 additions & 32 deletions crates/orderbook/src/database/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -63,7 +62,7 @@ pub trait OrderStoring: Send + Sync {
&self,
old_order: &OrderUid,
new_order: &Order,
new_quote: Option<Quote>,
new_quote: &Quote,
) -> Result<(), InsertionError>;
async fn orders_for_tx(&self, tx_hash: &H256) -> Result<Vec<Order>>;
async fn single_order(&self, uid: &OrderUid) -> Result<Option<Order>>;
Expand Down Expand Up @@ -272,7 +271,7 @@ impl OrderStoring for Postgres {
&self,
old_order: &model::order::OrderUid,
new_order: &model::order::Order,
new_quote: Option<Quote>,
new_quote: &Quote,
) -> anyhow::Result<(), super::orders::InsertionError> {
let _timer = super::Metrics::get()
.database_queries
Expand All @@ -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| {
Expand All @@ -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, &quote, ex).await?;
}
insert_quote(&new_order.metadata.uid, &new_quote, ex).await?;
Self::insert_order_app_data(&new_order, ex).await?;

Ok(())
}
.boxed()
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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, &quote)
.await
.unwrap();

Expand Down Expand Up @@ -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, &quote)
.await
.unwrap_err();
assert!(matches!(err, InsertionError::DuplicatedRecord));
Expand Down
10 changes: 5 additions & 5 deletions crates/orderbook/src/orderbook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -346,7 +346,7 @@ impl Orderbook {
&self,
validated_new_order: Order,
old_order: Order,
quote: Option<Quote>,
quote: Quote,
) -> Result<(OrderUid, Option<i64>), AddOrderError> {
// Replacement order signatures need to be validated meaning we cannot
// accept `PreSign` orders, otherwise anyone can cancel a user order by
Expand All @@ -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, &quote)
.await
.map_err(|err| AddOrderError::from_insertion(err, &validated_new_order))?;
Metrics::on_order_operation(&old_order, OrderOperation::Cancelled);
Expand Down
6 changes: 3 additions & 3 deletions crates/shared/src/order_quoting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use {
crate::{
db_order_conversions::order_kind_from,
fee::FeeParameters,
order_validation::PreOrderData,
order_validation::{PreOrderClass, PreOrderData},
price_estimation::Verification,
},
anyhow::{Context, Result},
Expand All @@ -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,
Expand Down Expand Up @@ -543,7 +543,7 @@ 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,
class: PreOrderClass::Market,
}
}
}
Expand Down
Loading

0 comments on commit a442095

Please sign in to comment.