Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PROPOSAL] Correctly label the orderbook orders metric #2917

Merged
merged 17 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

121 changes: 121 additions & 0 deletions crates/database/src/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,18 @@ pub struct FullOrder {
pub full_app_data: Option<Vec<u8>>,
}

#[derive(Debug, sqlx::FromRow)]
pub struct FullOrderWithQuote {
#[sqlx(flatten)]
pub full_order: FullOrder,
pub quote_buy_amount: Option<BigDecimal>,
pub quote_sell_amount: Option<BigDecimal>,
pub quote_gas_amount: Option<f64>,
pub quote_gas_price: Option<f64>,
pub quote_sell_token_price: Option<f64>,
pub solver: Option<Address>,
}

impl FullOrder {
pub fn valid_to(&self) -> i64 {
if let Some((_, valid_to)) = self.ethflow_data {
Expand Down Expand Up @@ -567,6 +579,26 @@ pub async fn single_full_order(
sqlx::query_as(QUERY).bind(uid).fetch_optional(ex).await
}

pub async fn single_full_order_with_quote(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a new query so we don't need to do two roundtrip to get an order with its quote data.

ex: &mut PgConnection,
uid: &OrderUid,
) -> Result<Option<FullOrderWithQuote>, sqlx::Error> {
#[rustfmt::skip]
const QUERY: &str = const_format::concatcp!(
"SELECT ", SELECT,
", o_quotes.sell_amount as quote_sell_amount",
", o_quotes.buy_amount as quote_buy_amount",
", o_quotes.gas_amount as quote_gas_amount",
", o_quotes.gas_price as quote_gas_price",
", o_quotes.sell_token_price as quote_sell_token_price",
", o_quotes.solver as solver",
" FROM ", FROM,
" LEFT JOIN order_quotes o_quotes ON o.uid = o_quotes.order_uid",
" WHERE o.uid = $1",
);
sqlx::query_as(QUERY).bind(uid).fetch_optional(ex).await
}

// Partial query for getting the log indices of events of a single settlement.
//
// This will fail if we ever have multiple settlements in the same transaction
Expand Down Expand Up @@ -1238,6 +1270,95 @@ mod tests {
assert_eq!(quote, quote_);
}

#[tokio::test]
#[ignore]
async fn postgres_order_with_quote_roundtrip() {
let mut db = PgConnection::connect("postgresql://").await.unwrap();
let mut db = db.begin().await.unwrap();
crate::clear_DANGER_(&mut db).await.unwrap();

let full_order = Order::default();
insert_order(&mut db, &full_order).await.unwrap();
let quote = Quote {
order_uid: Default::default(),
gas_amount: 1.,
gas_price: 2.,
sell_token_price: 3.,
sell_amount: 4.into(),
buy_amount: 5.into(),
solver: ByteArray([1; 20]),
};
insert_quote(&mut db, &quote).await.unwrap();
let order_with_quote = single_full_order_with_quote(&mut db, &quote.order_uid)
.await
.unwrap()
.unwrap();
assert_eq!(order_with_quote.quote_buy_amount.unwrap(), quote.buy_amount);
assert_eq!(
order_with_quote.quote_sell_amount.unwrap(),
quote.sell_amount
);
assert_eq!(order_with_quote.quote_gas_amount.unwrap(), quote.gas_amount);
assert_eq!(order_with_quote.quote_gas_price.unwrap(), quote.gas_price);
assert_eq!(
order_with_quote.quote_sell_token_price.unwrap(),
quote.sell_token_price
);
assert_eq!(order_with_quote.full_order.uid, full_order.uid);
assert_eq!(order_with_quote.full_order.owner, full_order.owner);
assert_eq!(
order_with_quote.full_order.creation_timestamp,
full_order.creation_timestamp
);
assert_eq!(
order_with_quote.full_order.sell_token,
full_order.sell_token
);
assert_eq!(order_with_quote.full_order.buy_token, full_order.buy_token);
assert_eq!(
order_with_quote.full_order.sell_amount,
full_order.sell_amount
);
assert_eq!(
order_with_quote.full_order.buy_amount,
full_order.buy_amount
);
assert_eq!(order_with_quote.full_order.valid_to, full_order.valid_to);
assert_eq!(order_with_quote.full_order.app_data, full_order.app_data);
assert_eq!(
order_with_quote.full_order.fee_amount,
full_order.fee_amount
);
assert_eq!(
order_with_quote.full_order.full_fee_amount,
full_order.full_fee_amount
);
assert_eq!(order_with_quote.full_order.kind, full_order.kind);
assert_eq!(order_with_quote.full_order.class, full_order.class);
assert_eq!(
order_with_quote.full_order.partially_fillable,
full_order.partially_fillable
);
assert_eq!(order_with_quote.full_order.signature, full_order.signature);
assert_eq!(order_with_quote.full_order.receiver, full_order.receiver);
assert_eq!(
order_with_quote.full_order.signing_scheme,
full_order.signing_scheme
);
assert_eq!(
order_with_quote.full_order.settlement_contract,
full_order.settlement_contract
);
assert_eq!(
order_with_quote.full_order.sell_token_balance,
full_order.sell_token_balance
);
assert_eq!(
order_with_quote.full_order.buy_token_balance,
full_order.buy_token_balance
);
}

#[tokio::test]
#[ignore]
async fn postgres_cancel_order() {
Expand Down
1 change: 1 addition & 0 deletions crates/orderbook/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ serde = { workspace = true }
serde_json = { workspace = true }
serde_with = { workspace = true }
shared = { path = "../shared" }
strum_macros = "0.26.4"
sqlx = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true, features = ["macros", "rt-multi-thread", "signal", "sync", "time"] }
Expand Down
91 changes: 89 additions & 2 deletions crates/orderbook/src/database/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use {
database::{
byte_array::ByteArray,
order_events::{insert_order_event, OrderEvent, OrderEventLabel},
orders::{FullOrder, OrderKind as DbOrderKind},
orders::{self, FullOrder, OrderKind as DbOrderKind},
},
ethcontract::H256,
futures::{stream::TryStreamExt, FutureExt, StreamExt},
Expand Down Expand Up @@ -76,13 +76,36 @@ pub trait OrderStoring: Send + Sync {
limit: Option<u64>,
) -> Result<Vec<Order>>;
async fn latest_order_event(&self, order_uid: &OrderUid) -> Result<Option<OrderEvent>>;
async fn single_order_with_quote(&self, uid: &OrderUid) -> Result<Option<OrderWithQuote>>;
}

pub struct SolvableOrders {
pub orders: Vec<Order>,
pub latest_settlement_block: u64,
}

pub struct OrderWithQuote {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a new object which contains an order with the quote information (the quote information corresponding to the one stored in order_quotes table, which is the only info we need, not the full quote).

pub order: Order,
pub quote: Option<orders::Quote>,
}

impl OrderWithQuote {
pub fn new(order: Order, quote: Option<Quote>) -> Self {
Self {
quote: quote.map(|quote| orders::Quote {
order_uid: ByteArray(order.metadata.uid.0),
gas_amount: quote.data.fee_parameters.gas_amount,
gas_price: quote.data.fee_parameters.gas_price,
sell_token_price: quote.data.fee_parameters.sell_token_price,
sell_amount: u256_to_big_decimal(&quote.sell_amount),
buy_amount: u256_to_big_decimal(&quote.buy_amount),
solver: ByteArray(quote.data.solver.0),
}),
order,
}
}
}

#[derive(Debug)]
pub enum InsertionError {
DuplicatedRecord,
Expand Down Expand Up @@ -323,6 +346,51 @@ impl OrderStoring for Postgres {
order.map(full_order_into_model_order).transpose()
}

async fn single_order_with_quote(&self, uid: &OrderUid) -> Result<Option<OrderWithQuote>> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could probably be part of an existing DB test which tests quote fetching in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good idea, adding them!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test at OrderStoring level and database level 👌

let _timer = super::Metrics::get()
.database_queries
.with_label_values(&["single_order_with_quote"])
.start_timer();

let mut ex = self.pool.acquire().await?;
let order = orders::single_full_order_with_quote(&mut ex, &ByteArray(uid.0)).await?;
order
.map(|order_with_quote| {
let quote = match (
order_with_quote.quote_buy_amount,
order_with_quote.quote_sell_amount,
order_with_quote.quote_gas_amount,
order_with_quote.quote_gas_price,
order_with_quote.quote_sell_token_price,
order_with_quote.solver,
) {
(
Some(buy_amount),
Some(sell_amount),
Some(gas_amount),
Some(gas_price),
Some(sell_token_price),
Some(solver),
) => Some(orders::Quote {
order_uid: order_with_quote.full_order.uid,
gas_amount,
gas_price,
sell_token_price,
sell_amount,
buy_amount,
solver,
}),
_ => None,
};

Ok(OrderWithQuote {
order: full_order_into_model_order(order_with_quote.full_order)?,
quote,
})
})
.transpose()
}

async fn orders_for_tx(&self, tx_hash: &H256) -> Result<Vec<Order>> {
tokio::try_join!(
self.user_order_for_tx(tx_hash),
Expand Down Expand Up @@ -599,6 +667,7 @@ mod tests {
order::{Order, OrderData, OrderMetadata, OrderStatus, OrderUid},
signature::{Signature, SigningScheme},
},
primitive_types::U256,
std::sync::atomic::{AtomicI64, Ordering},
};

Expand Down Expand Up @@ -1078,9 +1147,27 @@ mod tests {
..Default::default()
};

db.insert_order(&order, None).await.unwrap();
let quote = Quote {
id: Some(5),
sell_amount: U256::from(1),
buy_amount: U256::from(2),
..Default::default()
};
db.insert_order(&order, Some(quote.clone())).await.unwrap();

let interactions = db.single_order(&uid).await.unwrap().unwrap().interactions;
assert_eq!(interactions, order.interactions);

// Test `single_order_with_quote`
let single_order_with_quote = db.single_order_with_quote(&uid).await.unwrap().unwrap();
assert_eq!(single_order_with_quote.order, order);
assert_eq!(
single_order_with_quote.quote.clone().unwrap().sell_amount,
u256_to_big_decimal(&quote.sell_amount)
);
assert_eq!(
single_order_with_quote.quote.unwrap().buy_amount,
u256_to_big_decimal(&quote.buy_amount)
);
}
}
Loading
Loading