-
Notifications
You must be signed in to change notification settings - Fork 186
(fast7) Kick off fast path handling in runloop #4862
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,7 @@ use { | |
| eth_domain_types as eth, | ||
| futures::{StreamExt, TryStreamExt}, | ||
| number::conversions::{big_decimal_to_u256, u256_to_big_decimal, u256_to_big_uint}, | ||
| shared::db_order_conversions::full_order_into_model_order, | ||
| shared::db_order_conversions::{fast_path_order_into_model, full_order_into_model_order}, | ||
| std::{ | ||
| collections::{HashMap, HashSet}, | ||
| ops::DerefMut, | ||
|
|
@@ -1037,6 +1037,143 @@ impl Persistence { | |
| .map(|o| crate::domain::OrderUid(o.0)) | ||
| .collect()) | ||
| } | ||
|
|
||
| /// Recovers what's needed to settle a fast-path order via the driver's | ||
| /// `/settle`, or `None` when `uid` is not a fast-path order (its quote's | ||
| /// competition was not persisted). | ||
| pub async fn fast_path_order( | ||
| &self, | ||
| uid: domain::OrderUid, | ||
| ) -> anyhow::Result<Option<FastPathOrder>> { | ||
| let _timer = Metrics::get() | ||
| .database_queries | ||
| .with_label_values(&["fast_path_order"]) | ||
| .start_timer(); | ||
|
|
||
| let mut ex = self.postgres.pool.acquire().await.context("acquire")?; | ||
| let key = ByteArray(uid.0); | ||
|
|
||
| let Some(row) = database::fast_path::single_fast_path_order(&mut ex, &key).await? else { | ||
| return Ok(None); | ||
| }; | ||
|
|
||
| let model_order = fast_path_order_into_model(&row)?; | ||
|
|
||
| let native_prices = row | ||
| .price_tokens | ||
| .iter() | ||
| .zip(&row.price_values) | ||
| .map(|(token, value)| { | ||
| let price = big_decimal_to_u256(value).context("invalid native price")?; | ||
| anyhow::Ok((eth::Address::from(token.0), price)) | ||
| }) | ||
| .collect::<anyhow::Result<HashMap<_, _>>>()?; | ||
|
|
||
| Ok(Some(FastPathOrder { | ||
| model_order, | ||
| auction_id: row.auction_id, | ||
| solution_id: row | ||
| .solution_id | ||
| .to_u64() | ||
| .context("solution id out of range")?, | ||
| solution_uid: row | ||
| .solution_uid | ||
| .to_usize() | ||
| .context("solution uid out of range")?, | ||
| solver: eth::Address::from(row.solver.0), | ||
| raw_sell: big_decimal_to_u256(&row.executed_sell) | ||
| .context("invalid executed sell amount")?, | ||
| raw_buy: big_decimal_to_u256(&row.executed_buy) | ||
| .context("invalid executed buy amount")?, | ||
| native_prices, | ||
| })) | ||
| } | ||
|
|
||
| /// Applies fees to every solver's bid on a fast-path order and stamps | ||
| /// the applicable fee policies. Runs after the autopilot picks up the | ||
| /// placed order and computes the policies via `ProtocolFees::apply`. | ||
| /// | ||
| /// Each bid's own raw `executed_sell`/`executed_buy` is adjusted by the | ||
| /// same volume factors so the recorded amounts stay consistent across | ||
| /// the whole competition — not just the winning row. | ||
| pub async fn record_fast_path_fees( | ||
| &self, | ||
| auction_id: database::auction::AuctionId, | ||
| order_uid: domain::OrderUid, | ||
| order_kind: model::order::OrderKind, | ||
| volume_factors: &[configs::fee_factor::FeeFactor], | ||
| fee_policies: &[domain::fee::Policy], | ||
| ) -> anyhow::Result<()> { | ||
| let _timer = Metrics::get() | ||
| .database_queries | ||
| .with_label_values(&["record_fast_path_fees"]) | ||
| .start_timer(); | ||
|
|
||
| let uid = ByteArray(order_uid.0); | ||
| let policy_rows: Vec<_> = fee_policies | ||
| .iter() | ||
| .map(|p| dto::fee_policy::from_domain(auction_id, order_uid, *p)) | ||
| .collect(); | ||
|
|
||
| let mut tx = self.postgres.pool.begin().await.context("begin")?; | ||
| let bids = database::fast_path::fast_path_bids(tx.deref_mut(), auction_id, uid) | ||
| .await | ||
| .context("fetch fast-path bids")?; | ||
| let adjusted_bids: Vec<_> = bids | ||
| .into_iter() | ||
| .map(|bid| { | ||
| let raw_sell = big_decimal_to_u256(&bid.executed_sell) | ||
| .context("bid executed_sell not a U256")?; | ||
| let raw_buy = big_decimal_to_u256(&bid.executed_buy) | ||
| .context("bid executed_buy not a U256")?; | ||
| let (adjusted_sell, adjusted_buy) = shared::fee::apply_volume_fees( | ||
| raw_sell, | ||
| raw_buy, | ||
| order_kind, | ||
| volume_factors.iter().copied(), | ||
| ); | ||
| anyhow::Ok(database::fast_path::FastPathBid { | ||
| solution_uid: bid.solution_uid, | ||
| executed_sell: u256_to_big_decimal(&adjusted_sell), | ||
| executed_buy: u256_to_big_decimal(&adjusted_buy), | ||
| }) | ||
| }) | ||
| .collect::<anyhow::Result<_>>()?; | ||
| database::fast_path::apply_fees_to_fast_path_bids( | ||
| tx.deref_mut(), | ||
| auction_id, | ||
| uid, | ||
| &adjusted_bids, | ||
| ) | ||
| .await | ||
| .context("apply_fees_to_fast_path_bids")?; | ||
| database::fee_policies::insert_batch(tx.deref_mut(), policy_rows) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: but should we drop any non volume fee policies to avoid confusion? |
||
| .await | ||
| .context("insert fast-path fee policies")?; | ||
| tx.commit().await.context("commit")?; | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| /// The data the autopilot needs to settle a fast-path order out of competition. | ||
| pub struct FastPathOrder { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normally, these live in In general, this 1k + line class looks pretty horrible. |
||
| /// The order in the raw API model form. Callers pass this to | ||
| /// `ProtocolFees::apply` and can then convert it to `domain::Order` via | ||
| /// `boundary::order::to_domain` once the resulting policies are known. | ||
| pub model_order: model::order::Order, | ||
| pub auction_id: database::auction::AuctionId, | ||
| pub solution_id: u64, | ||
| pub solution_uid: usize, | ||
| pub solver: eth::Address, | ||
| /// The `proposed_trade_executions` amounts as stored at quote time | ||
| /// (pre-fee-adjustment; identical to the quote's amounts since nothing | ||
| /// rewrites them between quote time and fast-path handling). Feed these | ||
| /// to `apply_volume_fees` alongside the Volume-type policies to obtain | ||
| /// the actual limit prices to settle at. | ||
| pub raw_sell: eth::U256, | ||
| pub raw_buy: eth::U256, | ||
| /// Native prices (token → normalized price) from the quote's auction. | ||
| pub native_prices: HashMap<eth::Address, eth::U256>, | ||
| } | ||
|
|
||
| #[derive(prometheus_metric_storage::MetricStorage)] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,11 @@ | ||
| pub mod fast_path; | ||
| pub mod settle_call_coordinator; | ||
|
|
||
| use { | ||
| self::settle_call_coordinator::{SettleCallCoordinator, SettleError}, | ||
| self::{ | ||
| fast_path::FastPathHandler, | ||
| settle_call_coordinator::{SettleCallCoordinator, SettleError}, | ||
| }, | ||
| crate::{ | ||
| domain::{ | ||
| self, | ||
|
|
@@ -170,8 +174,10 @@ pub struct RunLoop { | |
| /// Drivers that do NOT support delta auctions | ||
| drivers: Vec<Arc<infra::Driver>>, | ||
| /// Sends `/settle` calls to drivers and waits for the resulting | ||
| /// transaction to be mined. | ||
| /// transaction to be mined. Shared with the fast-path handler. | ||
| settle_coordinator: Arc<SettleCallCoordinator>, | ||
| /// Handles fast-path orders on the side. | ||
| fast_path: Arc<FastPathHandler>, | ||
| } | ||
|
|
||
| impl RunLoop { | ||
|
|
@@ -186,6 +192,8 @@ impl RunLoop { | |
| probes: Probes, | ||
| maintenance: MaintenanceSync, | ||
| new_orders_listener: mpsc::UnboundedReceiver<OrderUid>, | ||
| protocol_fees: Arc<crate::domain::ProtocolFees>, | ||
| surplus_capturing_jit_order_owners: Arc<Vec<alloy::primitives::Address>>, | ||
| ) -> Arc<Self> { | ||
| let max_winners = config.max_winners_per_auction.get(); | ||
| let weth = eth.contracts().wrapped_native_token(); | ||
|
|
@@ -200,6 +208,16 @@ impl RunLoop { | |
| config.max_settlement_transaction_wait, | ||
| )); | ||
|
|
||
| let fast_path = FastPathHandler::new( | ||
| eth.clone(), | ||
| persistence.clone(), | ||
| drivers.clone(), | ||
| protocol_fees, | ||
| surplus_capturing_jit_order_owners, | ||
| settle_coordinator.clone(), | ||
| config.submission_deadline, | ||
| ); | ||
|
|
||
| let self_ = Arc::new(Self { | ||
| delta_state: std::sync::Mutex::new(DeltaState::new( | ||
| config.auction_delta_checkpoint_interval, | ||
|
|
@@ -215,17 +233,34 @@ impl RunLoop { | |
| wake_notify: wake_runloop, | ||
| drivers, | ||
| settle_coordinator, | ||
| fast_path, | ||
| }); | ||
| Self::spawn_order_listener(self_.clone(), new_orders_listener); | ||
| self_ | ||
| } | ||
|
|
||
| /// Spawns a background task that listens to the creation of new orders | ||
| /// and wakes the run loop for each incoming order. | ||
| /// Spawns a background task that listens to the creation of new orders and | ||
| /// notifies the runloop to kick it off if necessary and initiates the | ||
| /// fast path handling if an order needs it. | ||
| fn spawn_order_listener(self: Arc<Self>, mut receiver: mpsc::UnboundedReceiver<OrderUid>) { | ||
| tokio::spawn(async move { | ||
| while let Some(_order_uid) = receiver.next().await { | ||
| while let Some(order_uid) = receiver.next().await { | ||
| self.wake_notify.notify_one(); | ||
|
Comment on lines
+247
to
248
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With fast_path coming (and more orders through the pipe I expect (?)), I wonder if we should instead collect batches here with recv_many, it would allow us to batch the DB queries instead of doing them 1 by 1 (of course it depends on whether, at any moment in time, more than one element is waiting in queue) This would've worked before to maybe reduce ms's of latency but now we're issuing DB queries 🤔 |
||
| let persistence = self.persistence.clone(); | ||
| let fast_path = self.fast_path.clone(); | ||
| // immediately spawn separate task to never delay processing | ||
| // fast path orders | ||
|
Comment on lines
+251
to
+252
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now, both the leader and secondary autopilot would process fast orders, which will cause a mess I believe. Somehow this also needs the "leader" check. |
||
| tokio::spawn( | ||
| async move { | ||
| match persistence.fast_path_order(order_uid).await { | ||
| // not a fast path order -> do nothing | ||
| Ok(None) => {} | ||
| Err(err) => tracing::error!(?err, "failed to look up fast path order"), | ||
| Ok(Some(order)) => fast_path.handle(order).await, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes me wonder if this and the settlement coordinator should have "queue interfaces" instead of actual functions we call, but at the same time, its just a different style (?) |
||
| }; | ||
| } | ||
| .instrument(tracing::info_span!("fast_path", ?order_uid)), | ||
| ); | ||
|
Comment on lines
+253
to
+263
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Every order insert now spawns an unbounded background task that runs Two things worth considering:
If fast path can be globally toggled, gating this lookup on that flag would avoid the per-order cost entirely when it's off.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds reasonable. In general having a quick check if an order is not fast path (and only run the joins if it is) probably makes sense. |
||
| } | ||
| }); | ||
| } | ||
|
|
@@ -518,6 +553,7 @@ impl RunLoop { | |
| solution_id, | ||
| submission_deadline_latest_block: block_deadline, | ||
| auction_id, | ||
| fast_path: None, | ||
| }; | ||
|
|
||
| match self_ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like a pretty logic heavy operation and not a simple persistence step. Can we move the actual computation into the fast path component and keep persistence as a simple convert and write to disk?