Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
306 changes: 190 additions & 116 deletions crates/database/src/trades.rs
Comment thread
MartinquaXD marked this conversation as resolved.

Large diffs are not rendered by default.

20 changes: 5 additions & 15 deletions crates/orderbook/src/api/get_trades.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,10 @@ enum TradeFilterError {
}

impl QueryParams {
fn trade_filter(&self) -> TradeFilter {
TradeFilter {
order_uid: self.order_uid,
owner: self.owner,
}
}

fn validate(&self) -> Result<TradeFilter, TradeFilterError> {
match (self.order_uid.as_ref(), self.owner.as_ref()) {
(Some(_), None) | (None, Some(_)) => Ok(self.trade_filter()),
match (self.owner, self.order_uid) {
(Some(owner), None) => Ok(TradeFilter::Owner(owner)),
(None, Some(uid)) => Ok(TradeFilter::OrderUid(uid)),
_ => Err(TradeFilterError::InvalidFilter(
"Must specify exactly one of owner or orderUid.".to_owned(),
)),
Expand Down Expand Up @@ -82,18 +76,14 @@ mod tests {
owner: Some(owner),
order_uid: None,
};
let result = query.validate().unwrap();
assert_eq!(result.owner, Some(owner));
assert_eq!(result.order_uid, None);
assert_eq!(query.validate().unwrap(), TradeFilter::Owner(owner));

let uid = OrderUid([1u8; 56]);
let query = QueryParams {
owner: None,
order_uid: Some(uid),
};
let result = query.validate().unwrap();
assert_eq!(result.owner, None);
assert_eq!(result.order_uid, Some(uid));
assert_eq!(query.validate().unwrap(), TradeFilter::OrderUid(uid));
}

#[test]
Expand Down
51 changes: 23 additions & 28 deletions crates/orderbook/src/api/get_trades_v2.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use {
crate::{
api::{AppState, error},
database::trades::{PaginatedTradeFilter, TradeRetrievingPaginated},
database::trades::{PaginatedTradeFilter, TradeFilter, TradeRetrievingPaginated},
},
alloy::primitives::Address,
anyhow::Context,
Expand Down Expand Up @@ -36,31 +36,28 @@ enum TradeFilterError {
}

impl QueryParams {
fn trade_filter(&self, offset: u64, limit: u64) -> PaginatedTradeFilter {
PaginatedTradeFilter {
order_uid: self.order_uid,
owner: self.owner,
offset,
limit,
}
}

fn validate(&self) -> Result<PaginatedTradeFilter, TradeFilterError> {
match (self.order_uid.as_ref(), self.owner.as_ref()) {
(Some(_), None) | (None, Some(_)) => {
let offset = self.offset.unwrap_or(DEFAULT_OFFSET);
let limit = self.limit.unwrap_or(DEFAULT_LIMIT);

if !(MIN_LIMIT..=MAX_LIMIT).contains(&limit) {
return Err(TradeFilterError::InvalidLimit(MIN_LIMIT, MAX_LIMIT));
}

Ok(self.trade_filter(offset, limit))
let filter = match (self.owner, self.order_uid) {
(Some(owner), None) => TradeFilter::Owner(owner),
(None, Some(uid)) => TradeFilter::OrderUid(uid),
_ => {
return Err(TradeFilterError::InvalidFilter(
"Must specify exactly one of owner or orderUid.".to_owned(),
));
}
_ => Err(TradeFilterError::InvalidFilter(
"Must specify exactly one of owner or orderUid.".to_owned(),
)),
};

let offset = self.offset.unwrap_or(DEFAULT_OFFSET);
let limit = self.limit.unwrap_or(DEFAULT_LIMIT);
if !(MIN_LIMIT..=MAX_LIMIT).contains(&limit) {
return Err(TradeFilterError::InvalidLimit(MIN_LIMIT, MAX_LIMIT));
}

Ok(PaginatedTradeFilter {
filter,
offset,
limit,
})
}
}

Expand Down Expand Up @@ -111,8 +108,7 @@ mod tests {
limit: None,
};
let result = query.validate().unwrap();
assert_eq!(result.owner, Some(owner));
assert_eq!(result.order_uid, None);
assert_eq!(result.filter, TradeFilter::Owner(owner));
assert_eq!(result.offset, DEFAULT_OFFSET);
assert_eq!(result.limit, DEFAULT_LIMIT);

Expand All @@ -124,8 +120,7 @@ mod tests {
limit: None,
};
let result = query.validate().unwrap();
assert_eq!(result.owner, None);
assert_eq!(result.order_uid, Some(uid));
assert_eq!(result.filter, TradeFilter::OrderUid(uid));
assert_eq!(result.offset, DEFAULT_OFFSET);
assert_eq!(result.limit, DEFAULT_LIMIT);

Expand All @@ -137,7 +132,7 @@ mod tests {
limit: Some(50),
};
let result = query.validate().unwrap();
assert_eq!(result.owner, Some(owner));
assert_eq!(result.filter, TradeFilter::Owner(owner));
assert_eq!(result.offset, 10);
assert_eq!(result.limit, 50);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/orderbook/src/database/debug_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl Postgres {
let proposed_solutions =
solver_competition_v2::find_solutions_for_order(&mut conn, &db_uid).await?;
let executions = order_execution::read_by_order_uid(&mut conn, &db_uid).await?;
let trades: Vec<DbTradesQueryRow> = trades::trades(&mut conn, None, Some(&db_uid), 0, 100)
let trades: Vec<DbTradesQueryRow> = trades::trades_by_order_uid(&mut conn, &db_uid, 0, 100)
.into_inner()
.await
.context("failed to fetch trades")?;
Expand Down
70 changes: 35 additions & 35 deletions crates/orderbook/src/database/trades.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ pub trait TradeRetrievingPaginated: Send + Sync {
async fn trades_paginated(&self, filter: &PaginatedTradeFilter) -> Result<Vec<Trade>>;
}

/// Any default value means that this field is unfiltered.
#[derive(Debug, Default, Eq, PartialEq)]
pub struct TradeFilter {
pub owner: Option<Address>,
pub order_uid: Option<OrderUid>,
/// Exactly one of the two filters is set. Enforced at the type level so the DB
/// layer never has to consider "both" or "neither" cases.
#[derive(Debug, Eq, PartialEq)]
pub enum TradeFilter {
Owner(Address),
OrderUid(OrderUid),
}

/// Trade filter with pagination support (for v2 API).
#[derive(Debug, Default, Eq, PartialEq)]
#[derive(Debug, Eq, PartialEq)]
pub struct PaginatedTradeFilter {
pub owner: Option<Address>,
pub order_uid: Option<OrderUid>,
pub filter: TradeFilter,
pub offset: u64,
pub limit: u64,
}
Expand All @@ -45,17 +45,15 @@ impl TradeRetrieving for Postgres {

let mut ex = self.pool.acquire().await?;
// For v1 API, return all results without pagination (use large default
// values)
let trades = database::trades::trades(
&mut ex,
filter.owner.map(|owner| ByteArray(owner.0.0)).as_ref(),
filter.order_uid.map(|uid| ByteArray(uid.0)).as_ref(),
0,
i64::MAX,
)
.into_inner()
.await
.map_err(anyhow::Error::from)?;
// values).
let trades = match filter {
TradeFilter::Owner(owner) => {
database::trades::trades_by_owner(&mut ex, &ByteArray(owner.0.0), 0, i64::MAX).await
}
TradeFilter::OrderUid(uid) => {
database::trades::trades_by_order_uid(&mut ex, &ByteArray(uid.0), 0, i64::MAX).await
}
}?;
timer.stop_and_record();

let auction_order_uids = trades
Expand Down Expand Up @@ -106,22 +104,24 @@ impl TradeRetrievingPaginated for Postgres {
.start_timer();

let mut ex = self.pool.acquire().await?;
let trades = database::trades::trades(
&mut ex,
filter.owner.map(|owner| ByteArray(owner.0.0)).as_ref(),
filter.order_uid.map(|uid| ByteArray(uid.0)).as_ref(),
filter
.offset
.try_into()
.context("offset too large for database")?,
filter
.limit
.try_into()
.context("limit too large for database")?,
)
.into_inner()
.await
.map_err(anyhow::Error::from)?;
let offset = filter
.offset
.try_into()
.context("offset too large for database")?;
let limit = filter
.limit
.try_into()
.context("limit too large for database")?;
let trades = match &filter.filter {
TradeFilter::Owner(owner) => {
database::trades::trades_by_owner(&mut ex, &ByteArray(owner.0.0), offset, limit)
.await
}
TradeFilter::OrderUid(uid) => {
database::trades::trades_by_order_uid(&mut ex, &ByteArray(uid.0), offset, limit)
.await
}
}?;
timer.stop_and_record();

let auction_order_uids = trades
Expand Down
8 changes: 1 addition & 7 deletions crates/orderbook/src/orderbook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,13 +571,7 @@ impl Orderbook {
// latest state of an already executed order is not `Traded`. To
// detect that we first check the trades table and return the
// appropriate competition data.
let trades = self
.database
.trades(&TradeFilter {
owner: None,
order_uid: Some(*uid),
})
.await?;
let trades = self.database.trades(&TradeFilter::OrderUid(*uid)).await?;

match trades.first().map(|trade| trade.tx_hash) {
Some(Some(tx_hash)) => {
Expand Down
1 change: 1 addition & 0 deletions database/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ Indexes:
- PRIMARY KEY: btree(`block_number`, `log_index`)
- trade\_order\_uid: btree (`order_uid`, `block_number`, `log_index`)
- trades_covering: btree(`order_uid`) INCLUDE (`buy_amount`, `sell_amount`, `fee_amount`)
- trades_owner_covering: btree(`substring(order_uid, 33, 20)`, `block_number` DESC, `log_index` DESC) INCLUDE (`order_uid`) — the substring is the owner encoded in the uid; the `INCLUDE` lets branch 1 of the account trades query satisfy the `orders` existence check via an index-only scan
Comment thread
MartinquaXD marked this conversation as resolved.
Outdated

### jit\_orders

Expand Down
14 changes: 14 additions & 0 deletions database/sql/V129__trades_owner_covering_index.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-- Adds a covering variant of the trades owner substring index that includes
-- `order_uid` to speed up the account trade history query.
--
-- The old `trades_order_uid_owner` index becomes redundant once this exists —
-- the planner always prefers the covering one — so we drop it in the same
-- migration.

CREATE INDEX IF NOT EXISTS trades_owner_covering ON trades (
substring(order_uid, 33, 20),
block_number DESC,
log_index DESC
) INCLUDE (order_uid);

DROP INDEX IF EXISTS trades_order_uid_owner;
Loading