Skip to content

Commit

Permalink
[PROPOSAL] Correctly label the orderbook orders metric (#2917)
Browse files Browse the repository at this point in the history
# Description
We need to properly adjust the orderbook `orders` metric. The reason is
that we got a false alarm alert, because as it is now, everything is a
`limit` order, so we got a period of time in which we got orders (real
`limit` ones) and no trades (because those orders were out of market).

This could be solved in many ways, as discussed with @MartinquaXD , one
way could be to only report "market" orders. But after giving it a deep
thought, I think this is the best way, because we don't lose any type of
order, and we report all of them labelled correctly.

# Changes
- Since all the orders are `limit` orders, I changed the `OrderClass` of
the `orders` metric to reflect real `limit` orders or `user` orders
(order not outside the market price). This way we keep all the orders
tracked. I meant this class to be exclusively of the metric system, as
we don't intent to store `user` order (aka `market` order) in the
domain. But it is handy to have it in the metrics.
- Use `strum` to serialize properly the enums
- Classify the metric's `OrderClass` accordingly to the quote price if
present.

## How to test
1. Checking / adjusting the metrics in grafana in staging

---------

Co-authored-by: Martin Beckmann <[email protected]>
  • Loading branch information
m-lord-renkse and MartinquaXD authored Sep 12, 2024
1 parent b8e6d79 commit 61240bc
Show file tree
Hide file tree
Showing 5 changed files with 293 additions and 42 deletions.
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(
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 {
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>> {
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

0 comments on commit 61240bc

Please sign in to comment.