Refactor participant score

crates/autopilot/src/domain/competition/mod.rs

@@ -12,7 +12,7 @@ mod participation_guard;
 pub mod winner_selection;
 
 pub use {
-    participant::{Participant, Ranked, Unranked},
+    participant::{Participant, RankType, Ranked, Scored, Unscored},
     participation_guard::SolverParticipationGuard,
 };
 
@@ -25,10 +25,6 @@ pub struct Solution {
     solver: Address,
     orders: HashMap<domain::OrderUid, TradedOrder>,
     prices: auction::Prices,
-    /// Score computed by the autopilot based on the solution
-    /// of the solver.
-    // TODO: refactor this to compute the score in the constructor
-    score: Option<Score>,
 }
 
 impl Solution {
@@ -43,10 +39,11 @@ impl Solution {
             solver,
             orders,
             prices,
-            score: None,
         }
     }
+}
 
+impl Solution {
     pub fn id(&self) -> SolutionId {
         self.id
     }
@@ -55,12 +52,6 @@ impl Solution {
         self.solver
     }
 
-    pub fn score(&self) -> Score {
-        self.score.expect(
-            "this function only gets called after the winner selection populated this value",
-        )
-    }
-
     pub fn order_ids(&self) -> impl Iterator<Item = &domain::OrderUid> + std::fmt::Debug {
         self.orders.keys()
     }

crates/autopilot/src/domain/competition/participant.rs

@@ -1,6 +1,6 @@
 use {
-    super::{Score, Solution},
-    crate::infra,
+    super::Score,
+    crate::{domain::competition::Solution, infra},
     std::sync::Arc,
 };
 
@@ -12,8 +12,21 @@ pub struct Participant<State = Ranked> {
 }
 
 #[derive(Clone)]
-pub struct Unranked;
-pub enum Ranked {
+pub struct Unscored;
+
+#[derive(Clone)]
+pub struct Scored {
+    pub(super) score: Score,
+}
+
+#[derive(Clone)]
+pub struct Ranked {
+    pub(super) rank_type: RankType,
+    pub(super) score: Score,
+}
+
+#[derive(Clone)]
+pub enum RankType {
     Winner,
     NonWinner,
     FilteredOut,
@@ -24,39 +37,56 @@ impl<T> Participant<T> {
         &self.solution
     }
 
-    pub fn set_score(&mut self, score: Score) {
-        self.solution.score = Some(score);
-    }
-
     pub fn driver(&self) -> &Arc<infra::Driver> {
         &self.driver
     }
 }
 
-impl Participant<Unranked> {
+impl Participant<Unscored> {
     pub fn new(solution: Solution, driver: Arc<infra::Driver>) -> Self {
         Self {
             solution,
             driver,
-            state: Unranked,
+            state: Unscored,
         }
     }
 
-    pub fn rank(self, rank: Ranked) -> Participant<Ranked> {
-        Participant::<Ranked> {
-            state: rank,
+    pub fn with_score(self, score: Score) -> Participant<Scored> {
+        Participant {
             solution: self.solution,
             driver: self.driver,
+            state: Scored { score },
+        }
+    }
+}
+
+impl Participant<Scored> {
+    pub fn score(&self) -> Score {
+        self.state.score
+    }
+
+    pub fn with_rank(self, rank_type: RankType) -> Participant<Ranked> {
+        Participant {
+            solution: self.solution,
+            driver: self.driver,
+            state: Ranked {
+                rank_type,
+                score: self.state.score,
+            },
         }
     }
 }
 
 impl Participant<Ranked> {
+    pub fn score(&self) -> Score {
+        self.state.score
+    }
+
     pub fn is_winner(&self) -> bool {
-        matches!(self.state, Ranked::Winner)
+        matches!(self.state.rank_type, RankType::Winner)
     }
 
     pub fn filtered_out(&self) -> bool {
-        matches!(self.state, Ranked::FilteredOut)
+        matches!(self.state.rank_type, RankType::FilteredOut)
     }
 }

crates/autopilot/src/domain/competition/winner_selection.rs

@@ -32,7 +32,7 @@ use {
             Prices,
             order::{self, TargetAmount},
         },
-        competition::{Participant, Ranked, Score, Solution, Unranked},
+        competition::{Participant, RankType, Ranked, Score, Scored, Solution, Unscored},
         eth::{self, WrappedNativeToken},
         fee,
         settlement::{
@@ -57,14 +57,14 @@ impl Arbitrator {
     /// Runs the entire auction mechanism on the passed in solutions.
     pub fn arbitrate(
         &self,
-        participants: Vec<Participant<Unranked>>,
+        participants: Vec<Participant<Unscored>>,
         auction: &domain::Auction,
     ) -> Ranking {
         let partitioned = self.partition_unfair_solutions(participants, auction);
         let filtered_out = partitioned
             .discarded
             .into_iter()
-            .map(|participant| participant.rank(Ranked::FilteredOut))
+            .map(|participant| participant.with_rank(RankType::FilteredOut))
             .collect();
 
         let mut ranked = self.mark_winners(partitioned.kept);
@@ -73,7 +73,7 @@ impl Arbitrator {
                 // winners before non-winners
                 std::cmp::Reverse(participant.is_winner()),
                 // high score before low score
-                std::cmp::Reverse(participant.solution().score()),
+                std::cmp::Reverse(participant.score()),
             )
         });
         Ranking {
@@ -85,16 +85,17 @@ impl Arbitrator {
     /// Removes unfair solutions from the set of all solutions.
     fn partition_unfair_solutions(
         &self,
-        mut participants: Vec<Participant<Unranked>>,
+        participants: Vec<Participant<Unscored>>,
         auction: &domain::Auction,
     ) -> PartitionedSolutions {
         // Discard all solutions where we can't compute the aggregate scores
         // accurately because the fairness guarantees heavily rely on them.
-        let scores_by_solution = compute_scores_by_solution(&mut participants, auction);
+        let (mut participants, scores_by_solution) =
+            compute_scores_by_solution(participants, auction);
         participants.sort_by_key(|participant| {
             std::cmp::Reverse(
                 // we use the computed score to not trust the score provided by solvers
-                participant.solution().score().get().0,
+                participant.score().get().0,
             )
         });
         let baseline_scores = compute_baseline_scores(&scores_by_solution);
@@ -130,17 +131,17 @@ impl Arbitrator {
 
     /// Picks winners and sorts all solutions where winners come before
     /// non-winners and higher scores come before lower scores.
-    fn mark_winners(&self, participants: Vec<Participant<Unranked>>) -> Vec<Participant> {
+    fn mark_winners(&self, participants: Vec<Participant<Scored>>) -> Vec<Participant<Ranked>> {
         let winner_indexes = self.pick_winners(participants.iter().map(|p| p.solution()));
         participants
             .into_iter()
             .enumerate()
             .map(|(index, participant)| {
                 let rank = match winner_indexes.contains(&index) {
-                    true => Ranked::Winner,
-                    false => Ranked::NonWinner,
+                    true => RankType::Winner,
+                    false => RankType::NonWinner,
                 };
-                participant.rank(rank)
+                participant.with_rank(rank)
             })
             .collect()
     }
@@ -165,17 +166,17 @@ impl Arbitrator {
                 continue;
             }
 
-            let solutions_without_solver = ranking
+            let participants_without_solver = ranking
                 .ranked
                 .iter()
-                .filter(|p| p.driver().submission_address != solver)
-                .map(|p| p.solution());
-            let winner_indices = self.pick_winners(solutions_without_solver.clone());
+                .filter(|p| p.driver().submission_address != solver);
+            let solutions = participants_without_solver.clone().map(|p| p.solution());
+            let winner_indices = self.pick_winners(solutions);
 
-            let score = solutions_without_solver
+            let score = participants_without_solver
                 .enumerate()
                 .filter(|(index, _)| winner_indices.contains(index))
-                .filter_map(|(_, solution)| solution.score)
+                .map(|(_, p)| p.score())
                 .reduce(Score::saturating_add)
                 .unwrap_or_default();
             reference_scores.insert(solver, score);
@@ -244,39 +245,40 @@ fn compute_baseline_scores(scores_by_solution: &ScoresBySolution) -> ScoreByDire
 /// Solutions get discarded because fairness guarantees heavily
 /// depend on these scores being accurate.
 fn compute_scores_by_solution(
-    participants: &mut Vec<Participant<Unranked>>,
+    participants: Vec<Participant<Unscored>>,
     auction: &domain::Auction,
-) -> ScoresBySolution {
+) -> (Vec<Participant<Scored>>, ScoresBySolution) {
     let auction = Auction::from(auction);
-    let mut scores = HashMap::default();
-
-    participants.retain_mut(|p| match score_by_token_pair(p.solution(), &auction) {
-        Ok(score) => {
-            let total_score = score
-                .values()
-                .fold(Score::default(), |acc, score| acc.saturating_add(*score));
-            scores.insert(
-                SolutionKey {
-                    driver: p.driver().submission_address,
-                    solution_id: p.solution().id,
-                },
-                score,
-            );
-            p.set_score(total_score);
-            true
-        }
-        Err(err) => {
-            tracing::warn!(
-                driver = p.driver().name,
-                ?err,
-                solution = ?p.solution(),
-                "discarding solution where scores could not be computed"
-            );
-            false
+    let mut scores_by_solution = HashMap::default();
+    let mut scored_participants = Vec::with_capacity(participants.len());
+
+    for participant in participants {
+        match score_by_token_pair(participant.solution(), &auction) {
+            Ok(score) => {
+                let total_score = score
+                    .values()
+                    .fold(Score::default(), |acc, score| acc.saturating_add(*score));
+                scores_by_solution.insert(
+                    SolutionKey {
+                        driver: participant.driver().submission_address,
+                        solution_id: participant.solution().id(),
+                    },
+                    score,
+                );
+                scored_participants.push(participant.with_score(total_score));
+            }
+            Err(err) => {
+                tracing::warn!(
+                    driver = participant.driver().name,
+                    ?err,
+                    solution = ?participant.solution(),
+                    "discarding solution where scores could not be computed"
+                );
+            }
         }
-    });
+    }
 
-    scores
+    (scored_participants, scores_by_solution)
 }
 
 /// Returns the total scores for each directed token pair of the solution.
@@ -450,8 +452,8 @@ impl Ranking {
 }
 
 struct PartitionedSolutions {
-    kept: Vec<Participant<Unranked>>,
-    discarded: Vec<Participant<Unranked>>,
+    kept: Vec<Participant<Scored>>,
+    discarded: Vec<Participant<Scored>>,
 }
 
 #[cfg(test)]
@@ -466,7 +468,7 @@ mod tests {
                     Price,
                     order::{self, AppDataHash},
                 },
-                competition::{Participant, Solution, TradedOrder, Unranked},
+                competition::{Participant, Solution, TradedOrder, Unscored},
                 eth::{self, TokenAddress},
             },
             infra::Driver,
@@ -1214,7 +1216,7 @@ mod tests {
                 // winners before non-winners
                 std::cmp::Reverse(a.is_winner()),
                 // high score before low score
-                std::cmp::Reverse(a.solution().score())
+                std::cmp::Reverse(a.score())
             )));
             assert_eq!(winners.len(), self.expected_winners.len());
             for (actual, expected) in winners.iter().zip(&self.expected_winners) {
@@ -1374,7 +1376,7 @@ mod tests {
         solver_address: Address,
         trades: Vec<(OrderUid, TradedOrder)>,
         prices: Option<HashMap<TokenAddress, Price>>,
-    ) -> Participant<Unranked> {
+    ) -> Participant<Unscored> {
         // The prices of the tokens do not affect the result but they keys must exist
         // for every token of every trade
         let prices = prices.unwrap_or({

crates/autopilot/src/infra/persistence/mod.rs

@@ -228,7 +228,7 @@ impl Persistence {
                         is_winner: participant.is_winner(),
                         filtered_out: participant.filtered_out(),
                         score: number::conversions::alloy::u256_to_big_decimal(
-                            &participant.solution().score().get().0,
+                            &participant.score().get().0,
                         ),
                         orders: participant
                             .solution()