Move settlement ranking logic into separate component

crates/solver/src/driver.rs

@@ -6,14 +6,15 @@ use crate::{
     in_flight_orders::InFlightOrders,
     liquidity::order_converter::OrderConverter,
     liquidity_collector::{LiquidityCollecting, LiquidityCollector},
-    metrics::{SolverMetrics, SolverRunOutcome},
+    metrics::SolverMetrics,
     orderbook::OrderBookApi,
     settlement::{external_prices::ExternalPrices, PriceCheckTokens, Settlement},
     settlement_post_processing::PostProcessingPipeline,
-    settlement_rater::{SettlementRater, SettlementRating},
+    settlement_ranker::SettlementRanker,
+    settlement_rater::SettlementRater,
     settlement_simulation::{self, simulate_before_after_access_list, TenderlyApi},
     settlement_submission::{SolutionSubmitter, SubmissionError},
-    solver::{Auction, SettlementWithError, Solver, Solvers},
+    solver::{Auction, SettlementWithError, Solver, SolverRunError, Solvers},
 };
 use anyhow::{Context, Result};
 use contracts::GPv2Settlement;
@@ -29,7 +30,6 @@ use model::{
 };
 use num::{rational::Ratio, BigInt, BigRational, ToPrimitive};
 use primitive_types::{H160, H256};
-use rand::prelude::SliceRandom;
 use shared::{
     current_block::{self, CurrentBlockStream},
     recent_block_cache::Block,
@@ -50,7 +50,6 @@ pub struct Driver {
     gas_price_estimator: Arc<dyn GasPriceEstimating>,
     settle_interval: Duration,
     native_token: H160,
-    min_order_age: Duration,
     metrics: Arc<dyn SolverMetrics>,
     web3: Web3,
     network_id: String,
@@ -65,10 +64,8 @@ pub struct Driver {
     post_processing_pipeline: PostProcessingPipeline,
     simulation_gas_limit: u128,
     fee_objective_scaling_factor: BigRational,
-    max_settlement_price_deviation: Option<Ratio<BigInt>>,
-    token_list_restriction_for_price_checks: PriceCheckTokens,
     tenderly: Option<TenderlyApi>,
-    settlement_rater: Box<dyn SettlementRating>,
+    settlement_ranker: SettlementRanker,
 }
 impl Driver {
     #[allow(clippy::too_many_arguments)]
@@ -109,14 +106,21 @@ impl Driver {
             web3: web3.clone(),
         });
 
+        let settlement_ranker = SettlementRanker {
+            max_settlement_price_deviation,
+            token_list_restriction_for_price_checks,
+            metrics: metrics.clone(),
+            min_order_age,
+            settlement_rater,
+        };
+
         Self {
             settlement_contract,
             liquidity_collector,
             solvers,
             gas_price_estimator,
             settle_interval,
             native_token,
-            min_order_age,
             metrics,
             web3,
             network_id,
@@ -132,10 +136,8 @@ impl Driver {
             simulation_gas_limit,
             fee_objective_scaling_factor: BigRational::from_float(fee_objective_scaling_factor)
                 .unwrap(),
-            max_settlement_price_deviation,
-            token_list_restriction_for_price_checks,
             tenderly,
-            settlement_rater,
+            settlement_ranker,
         }
     }
 
@@ -477,8 +479,6 @@ impl Driver {
             .context("failed to estimate gas price")?;
         tracing::debug!("solving with gas price of {:?}", gas_price);
 
-        let mut solver_settlements = Vec::new();
-
         let next_solver_competition = auction.next_solver_competition;
         let auction = Auction {
             id: auction.next_solver_competition,
@@ -492,91 +492,9 @@ impl Driver {
 
         tracing::debug!(deadline =? auction.deadline, "solving auction");
         let run_solver_results = self.run_solvers(auction).await;
-        for (solver, settlements) in run_solver_results {
-            let name = solver.name();
-
-            let settlements = match settlements {
-                Ok(mut settlement) => {
-                    for settlement in &settlement {
-                        tracing::debug!(solver_name = %name, ?settlement, "found solution");
-                    }
-
-                    // Do not continue with settlements that are empty or only liquidity orders.
-                    let settlement_count = settlement.len();
-                    settlement.retain(solver_settlements::has_user_order);
-                    if settlement_count != settlement.len() {
-                        tracing::debug!(
-                            solver_name = %name,
-                            "settlement(s) filtered containing only liquidity orders",
-                        );
-                    }
-
-                    if let Some(max_settlement_price_deviation) =
-                        &self.max_settlement_price_deviation
-                    {
-                        let settlement_count = settlement.len();
-                        settlement.retain(|settlement| {
-                            settlement.satisfies_price_checks(
-                                solver.name(),
-                                &external_prices,
-                                max_settlement_price_deviation,
-                                &self.token_list_restriction_for_price_checks,
-                            )
-                        });
-                        if settlement_count != settlement.len() {
-                            tracing::debug!(
-                                solver_name = %name,
-                                "settlement(s) filtered for violating maximum external price deviation",
-                            );
-                        }
-                    }
-
-                    if settlement.is_empty() {
-                        self.metrics.solver_run(SolverRunOutcome::Empty, name);
-                        continue;
-                    }
-
-                    self.metrics.solver_run(SolverRunOutcome::Success, name);
-                    settlement
-                }
-                Err(err) => {
-                    match err {
-                        SolverRunError::Timeout => {
-                            self.metrics.solver_run(SolverRunOutcome::Timeout, name)
-                        }
-                        SolverRunError::Solving(_) => {
-                            self.metrics.solver_run(SolverRunOutcome::Failure, name)
-                        }
-                    }
-                    tracing::warn!(solver_name = %name, ?err, "solver error");
-                    continue;
-                }
-            };
-
-            solver_settlements.reserve(settlements.len());
-
-            for settlement in settlements {
-                solver_settlements.push((solver.clone(), settlement))
-            }
-        }
-
-        // filters out all non-mature settlements
-        let solver_settlements =
-            solver_settlements::retain_mature_settlements(self.min_order_age, solver_settlements);
-
-        // log considered settlements. While we already log all found settlements, this additonal
-        // statement allows us to figure out which settlements were filtered out and which ones are
-        // going to be simulated and considered for competition.
-        for (solver, settlement) in &solver_settlements {
-            tracing::debug!(
-                solver_name = %solver.name(), ?settlement,
-                "considering solution for solver competition",
-            );
-        }
-
         let (mut rated_settlements, errors) = self
-            .settlement_rater
-            .rate_settlements(solver_settlements, &external_prices, gas_price)
+            .settlement_ranker
+            .rank_legal_settlements(run_solver_results, &external_prices, gas_price)
             .await?;
 
         // We don't know the exact block because simulation can happen over multiple blocks but
@@ -596,11 +514,6 @@ impl Driver {
             self.metrics.settlement_simulation_succeeded(solver.name());
         }
 
-        // Before sorting, make sure to shuffle the settlements. This is to make sure we don't give
-        // preference to any specific solver when there is an objective value tie.
-        rated_settlements.shuffle(&mut rand::thread_rng());
-
-        rated_settlements.sort_by(|a, b| a.1.objective_value().cmp(&b.1.objective_value()));
         print_settlements(&rated_settlements, &self.fee_objective_scaling_factor);
 
         // Report solver competition data to the api.
@@ -803,12 +716,6 @@ fn print_settlements(
     tracing::info!("Rated Settlements: {}", text);
 }
 
-#[derive(Debug)]
-enum SolverRunError {
-    Timeout,
-    Solving(anyhow::Error),
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

crates/solver/src/lib.rs

@@ -12,6 +12,7 @@ pub mod orderbook;
 pub mod settlement;
 pub mod settlement_access_list;
 pub mod settlement_post_processing;
+pub mod settlement_ranker;
 pub mod settlement_rater;
 pub mod settlement_simulation;
 pub mod settlement_submission;

crates/solver/src/settlement_ranker.rs

@@ -0,0 +1,142 @@
+use crate::{
+    driver::solver_settlements::{self, retain_mature_settlements},
+    metrics::{SolverMetrics, SolverRunOutcome},
+    settlement::{external_prices::ExternalPrices, PriceCheckTokens, Settlement},
+    settlement_rater::{RatedSolverSettlement, SettlementRating},
+    solver::{SettlementWithError, Solver, SolverRunError},
+};
+use anyhow::Result;
+use gas_estimation::GasPrice1559;
+use num::{rational::Ratio, BigInt};
+use rand::prelude::SliceRandom;
+use std::{sync::Arc, time::Duration};
+
+type SolverResult = (Arc<dyn Solver>, Result<Vec<Settlement>, SolverRunError>);
+
+pub struct SettlementRanker {
+    pub metrics: Arc<dyn SolverMetrics>,
+    pub settlement_rater: Box<dyn SettlementRating>,
+    // TODO: these should probably come from the autopilot to make the test parameters identical for
+    // everyone.
+    pub min_order_age: Duration,
+    pub max_settlement_price_deviation: Option<Ratio<BigInt>>,
+    pub token_list_restriction_for_price_checks: PriceCheckTokens,
+}
+
+impl SettlementRanker {
+    /// Discards settlements without user orders and settlements which violate price checks.
+    /// Logs info and updates metrics about the out come of this run loop for each solver.
+    fn discard_illegal_settlements(
+        &self,
+        solver: &Arc<dyn Solver>,
+        settlements: Result<Vec<Settlement>, SolverRunError>,
+        external_prices: &ExternalPrices,
+    ) -> Vec<Settlement> {
+        let name = solver.name();
+        match settlements {
+            Ok(mut settlement) => {
+                for settlement in &settlement {
+                    tracing::debug!(solver_name = %name, ?settlement, "found solution");
+                }
+
+                // Do not continue with settlements that are empty or only liquidity orders.
+                let settlement_count = settlement.len();
+                settlement.retain(solver_settlements::has_user_order);
+                if settlement_count != settlement.len() {
+                    tracing::debug!(
+                        solver_name = %name,
+                        "settlement(s) filtered containing only liquidity orders",
+                    );
+                }
+
+                if let Some(max_settlement_price_deviation) = &self.max_settlement_price_deviation {
+                    let settlement_count = settlement.len();
+                    settlement.retain(|settlement| {
+                        settlement.satisfies_price_checks(
+                            solver.name(),
+                            external_prices,
+                            max_settlement_price_deviation,
+                            &self.token_list_restriction_for_price_checks,
+                        )
+                    });
+                    if settlement_count != settlement.len() {
+                        tracing::debug!(
+                            solver_name = %name,
+                            "settlement(s) filtered for violating maximum external price deviation",
+                        );
+                    }
+                }
+
+                let outcome = match settlement.is_empty() {
+                    true => SolverRunOutcome::Empty,
+                    false => SolverRunOutcome::Success,
+                };
+                self.metrics.solver_run(outcome, name);
+
+                settlement
+            }
+            Err(err) => {
+                let outcome = match err {
+                    SolverRunError::Timeout => SolverRunOutcome::Timeout,
+                    SolverRunError::Solving(_) => SolverRunOutcome::Failure,
+                };
+                self.metrics.solver_run(outcome, name);
+                tracing::warn!(solver_name = %name, ?err, "solver error");
+                vec![]
+            }
+        }
+    }
+
+    /// Computes a list of settlements which pass all pre-simulation sanity checks.
+    fn get_legal_settlements(
+        &self,
+        settlements: Vec<SolverResult>,
+        prices: &ExternalPrices,
+    ) -> Vec<(Arc<dyn Solver>, Settlement)> {
+        let mut solver_settlements = vec![];
+        for (solver, settlements) in settlements {
+            let settlements = self.discard_illegal_settlements(&solver, settlements, prices);
+            for settlement in settlements {
+                solver_settlements.push((solver.clone(), settlement));
+            }
+        }
+
+        // TODO this needs to move into the autopilot eventually.
+        // filters out all non-mature settlements
+        retain_mature_settlements(self.min_order_age, solver_settlements)
+    }
+
+    /// Determines legal settlements and ranks them by simulating them.
+    /// Settlements get partitioned into simulation errors and a list
+    /// of `RatedSettlement`s sorted by ascending order of objective value.
+    pub async fn rank_legal_settlements(
+        &self,
+        settlements: Vec<SolverResult>,
+        external_prices: &ExternalPrices,
+        gas_price: GasPrice1559,
+    ) -> Result<(Vec<RatedSolverSettlement>, Vec<SettlementWithError>)> {
+        let solver_settlements = self.get_legal_settlements(settlements, external_prices);
+
+        // log considered settlements. While we already log all found settlements, this additonal
+        // statement allows us to figure out which settlements were filtered out and which ones are
+        // going to be simulated and considered for competition.
+        for (solver, settlement) in &solver_settlements {
+            tracing::debug!(
+                solver_name = %solver.name(), ?settlement,
+                "considering solution for solver competition",
+            );
+        }
+
+        let (mut rated_settlements, errors) = self
+            .settlement_rater
+            .rate_settlements(solver_settlements, external_prices, gas_price)
+            .await?;
+
+        // Before sorting, make sure to shuffle the settlements. This is to make sure we don't give
+        // preference to any specific solver when there is an objective value tie.
+        rated_settlements.shuffle(&mut rand::thread_rng());
+
+        rated_settlements.sort_by(|a, b| a.1.objective_value().cmp(&b.1.objective_value()));
+        Ok((rated_settlements, errors))
+    }
+}

crates/solver/src/settlement_rater.rs

@@ -15,7 +15,7 @@ use std::sync::Arc;
 use web3::types::AccessList;
 
 type SolverSettlement = (Arc<dyn Solver>, Settlement);
-type RatedSolverSettlement = (Arc<dyn Solver>, RatedSettlement, Option<AccessList>);
+pub type RatedSolverSettlement = (Arc<dyn Solver>, RatedSettlement, Option<AccessList>);
 
 #[mockall::automock]
 #[async_trait::async_trait]

crates/solver/src/solver.rs

@@ -44,6 +44,12 @@ mod single_order_solver;
 pub mod uni_v3_router_solver;
 mod zeroex_solver;
 
+#[derive(Debug)]
+pub enum SolverRunError {
+    Timeout,
+    Solving(anyhow::Error),
+}
+
 /// Interface that all solvers must implement.
 ///
 /// A `solve` method transforming a collection of `Liquidity` (sources) into a list of