From 8eb803056a1b9573ae9bad2eac6f7af38edaf367 Mon Sep 17 00:00:00 2001 From: sunce86 Date: Tue, 8 Oct 2024 17:28:23 +0200 Subject: [PATCH 1/7] Merge winners with participants --- .../autopilot/src/domain/competition/mod.rs | 19 ++- crates/autopilot/src/run_loop.rs | 160 ++++++++---------- 2 files changed, 88 insertions(+), 91 deletions(-) diff --git a/crates/autopilot/src/domain/competition/mod.rs b/crates/autopilot/src/domain/competition/mod.rs index 48896ba80a..e186d7f3eb 100644 --- a/crates/autopilot/src/domain/competition/mod.rs +++ b/crates/autopilot/src/domain/competition/mod.rs @@ -1,11 +1,11 @@ use { super::auction::order, crate::{ - domain, - domain::{auction, eth}, + domain::{self, auction, eth}, + infra, }, derive_more::Display, - std::collections::HashMap, + std::{collections::HashMap, sync::Arc}, }; type SolutionId = u64; @@ -91,6 +91,19 @@ impl Score { } } +#[derive(Clone)] +pub struct RawParticipant { + pub driver: Arc, + pub solution: Solution, +} + +#[derive(Clone)] +pub struct Participant { + pub driver: Arc, + pub solution: Solution, + pub winner: bool, +} + #[derive(Debug, thiserror::Error)] #[error("the solver proposed a 0-score solution")] pub struct ZeroScore; diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 4bad305684..a4b0e44915 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -254,20 +254,10 @@ impl RunLoop { // Collect valid solutions from all drivers let solutions = self.competition(&auction).await; observe::solutions(&solutions); - - // Pick winners for execution - let winners = self.select_winners(&solutions); - if winners.is_empty() { - tracing::info!("no winners for auction"); + if solutions.is_empty() { return; } - // Mark all non-winning orders as `Considered` for execution - self.persistence.store_order_events( - non_winning_orders(&solutions, &winners), - OrderEventLabel::Considered, - ); - let competition_simulation_block = self.eth.current_block().borrow().number; let block_deadline = competition_simulation_block + self.config.submission_deadline; @@ -277,9 +267,6 @@ impl RunLoop { .post_processing( &auction, competition_simulation_block, - // TODO: Support multiple winners - // https://github.com/cowprotocol/services/issues/3021 - &winners.first().expect("must exist").solution, &solutions, block_deadline, ) @@ -289,19 +276,33 @@ impl RunLoop { return; } - observe::unsettled(&solutions, &winners, &auction); - for Participant { driver, solution } in winners { - tracing::info!(driver = %driver.name, solution = %solution.id(), "winner"); + // Mark all non-winning orders as `Considered` for execution + self.persistence.store_order_events( + solutions + .iter() + .filter(|participant| !participant.winner) + .flat_map(|participant| participant.solution.order_ids().copied()), + OrderEventLabel::Considered, + ); - // Mark all winning orders as `Executing` - self.persistence - .store_order_events(solution.order_ids().copied(), OrderEventLabel::Executing); + // Mark all winning orders as `Executing` + self.persistence.store_order_events( + solutions + .iter() + .filter(|participant| participant.winner) + .flat_map(|participant| participant.solution.order_ids().copied()), + OrderEventLabel::Executing, + ); + + observe::unsettled(&solutions, &auction); + for winner in solutions.iter().filter(|participant| participant.winner) { + tracing::info!(driver = %winner.driver.name, solution = %winner.solution.id(), "winner"); self.start_settlement_execution( auction.id, single_run_start, - driver, - solution, + &winner.driver, + &winner.solution, block_deadline, ) .await; @@ -398,16 +399,20 @@ impl RunLoop { } } - #[allow(clippy::too_many_arguments)] async fn post_processing( &self, auction: &domain::Auction, competition_simulation_block: u64, - winning_solution: &competition::Solution, - solutions: &[Participant], + solutions: &[competition::Participant], block_deadline: u64, ) -> Result<()> { let start = Instant::now(); + // TODO: Support multiple winners + // https://github.com/cowprotocol/services/issues/3021 + let Some(winning_solution) = solutions.first().map(|participant| &participant.solution) + else { + return Err(anyhow::anyhow!("no winners found")); + }; let winner = winning_solution.solver().into(); let winning_score = winning_solution.score().get().0; let reference_score = solutions @@ -522,7 +527,7 @@ impl RunLoop { /// Runs the solver competition, making all configured drivers participate. /// Returns all fair solutions sorted by their score (best to worst). - async fn competition(&self, auction: &domain::Auction) -> Vec { + async fn competition(&self, auction: &domain::Auction) -> Vec { let request = solve::Request::new( auction, &self.trusted_tokens.all(), @@ -560,45 +565,45 @@ impl RunLoop { ); None } + }); + + // Winners are selected one by one, starting from the best solution, + // until `max_winners_per_auction` are selected. The solution is a winner + // if it swaps tokens that are not yet swapped by any other already + // selected winner. + let mut already_swapped_tokens = HashSet::new(); + let mut winners = 0; + let solutions = solutions + .map(|participant| { + let swapped_tokens = participant + .solution + .orders() + .iter() + .flat_map(|(_, order)| vec![order.sell.token, order.buy.token]) + .collect::>(); + + let is_winner = swapped_tokens.is_disjoint(&already_swapped_tokens) + && winners < self.config.max_winners_per_auction; + + if is_winner { + already_swapped_tokens.extend(swapped_tokens); + winners += 1; + } + competition::Participant { + driver: participant.driver.clone(), + solution: participant.solution.clone(), + winner: is_winner, + } }) .collect(); solutions } - /// Chooses the winners from the given participants. - /// - /// Participants are already sorted by their score (best to worst). - /// - /// Winners are selected one by one, starting from the best solution, - /// until `max_winners_per_auction` are selected. The solution is a winner - /// if it swaps tokens that are not yet swapped by any other already - /// selected winner. - fn select_winners<'a>(&self, participants: &'a [Participant]) -> Vec<&'a Participant> { - let mut winners = Vec::new(); - let mut already_swapped_tokens = HashSet::new(); - for participant in participants.iter() { - let swapped_tokens = participant - .solution - .orders() - .iter() - .flat_map(|(_, order)| vec![order.sell.token, order.buy.token]) - .collect::>(); - if swapped_tokens.is_disjoint(&already_swapped_tokens) { - winners.push(participant); - already_swapped_tokens.extend(swapped_tokens); - if winners.len() >= self.config.max_winners_per_auction { - break; - } - } - } - winners - } - /// Returns true if solution is fair to other solutions fn is_solution_fair( - solution: &Participant, - others: &[Participant], + solution: &competition::RawParticipant, + others: &[competition::RawParticipant], auction: &domain::Auction, ) -> bool { let Some(fairness_threshold) = solution.driver.fairness_threshold else { @@ -687,7 +692,7 @@ impl RunLoop { &self, driver: Arc, request: &solve::Request, - ) -> Vec { + ) -> Vec { let start = Instant::now(); let result = self.try_solve(&driver, request).await; let solutions = match result { @@ -711,7 +716,7 @@ impl RunLoop { .filter_map(|solution| match solution { Ok(solution) => { Metrics::solution_ok(&driver); - Some(Participant { + Some(competition::RawParticipant { driver: driver.clone(), solution, }) @@ -874,27 +879,6 @@ impl RunLoop { } } -fn non_winning_orders(solutions: &[Participant], winners: &[&Participant]) -> HashSet { - let proposed_orders: HashSet<_> = solutions - .iter() - .flat_map(|participant| participant.solution.order_ids().copied()) - .collect(); - let winning_orders: HashSet<_> = winners - .iter() - .flat_map(|participant| participant.solution.order_ids().copied()) - .collect(); - proposed_orders - .difference(&winning_orders) - .cloned() - .collect() -} - -#[derive(Clone)] -pub struct Participant { - driver: Arc, - solution: competition::Solution, -} - #[derive(Debug, thiserror::Error)] enum SolveError { #[error("the solver timed out")] @@ -1097,7 +1081,7 @@ pub mod observe { ); } - pub fn solutions(solutions: &[super::Participant]) { + pub fn solutions(solutions: &[domain::competition::Participant]) { if solutions.is_empty() { tracing::info!("no solutions for auction"); } @@ -1112,19 +1096,19 @@ pub mod observe { } /// Records metrics for the matched but unsettled orders. - pub fn unsettled( - solutions: &[super::Participant], - winners: &[&super::Participant], - auction: &domain::Auction, - ) { - let Some(winner) = winners.first() else { + pub fn unsettled(solutions: &[domain::competition::Participant], auction: &domain::Auction) { + let Some(winner) = solutions.first() else { // no solutions means nothing to report return; }; let auction_uids = auction.orders.iter().map(|o| o.uid).collect::>(); - let mut non_winning_orders = super::non_winning_orders(solutions, winners); + let mut non_winning_orders = solutions + .iter() + .filter(|participant| !participant.winner) + .flat_map(|participant| participant.solution.order_ids().copied()) + .collect::>(); // Report orders that were part of a non-winning solution candidate // but only if they were part of the auction (filter out jit orders) non_winning_orders.retain(|uid| auction_uids.contains(uid)); From e63cfec5d4a60ecb0667a9315dc251879eeeb519 Mon Sep 17 00:00:00 2001 From: sunce86 Date: Tue, 8 Oct 2024 18:01:11 +0200 Subject: [PATCH 2/7] Small refactor --- .../autopilot/src/domain/competition/mod.rs | 16 ++++++++++++ crates/autopilot/src/run_loop.rs | 25 ++++++------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/crates/autopilot/src/domain/competition/mod.rs b/crates/autopilot/src/domain/competition/mod.rs index e186d7f3eb..ecca079c1c 100644 --- a/crates/autopilot/src/domain/competition/mod.rs +++ b/crates/autopilot/src/domain/competition/mod.rs @@ -104,6 +104,22 @@ pub struct Participant { pub winner: bool, } +/// Returns the order IDs of the winning solutions. +pub fn winning_orders(participants: &[Participant]) -> impl Iterator { + participants + .iter() + .filter(|p| p.winner) + .flat_map(|p| p.solution.order_ids()) +} + +/// Returns the order IDs of the non-winning solutions. +pub fn non_winning_orders(participants: &[Participant]) -> impl Iterator { + participants + .iter() + .filter(|p| !p.winner) + .flat_map(|p| p.solution.order_ids()) +} + #[derive(Debug, thiserror::Error)] #[error("the solver proposed a 0-score solution")] pub struct ZeroScore; diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index a4b0e44915..e208ea5c8e 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -246,10 +246,8 @@ impl RunLoop { let auction = self.remove_in_flight_orders(auction).await; // Mark all auction orders as `Ready` for competition - self.persistence.store_order_events( - auction.orders.iter().map(|o| OrderUid(o.uid.0)), - OrderEventLabel::Ready, - ); + self.persistence + .store_order_events(auction.orders.iter().map(|o| o.uid), OrderEventLabel::Ready); // Collect valid solutions from all drivers let solutions = self.competition(&auction).await; @@ -278,19 +276,13 @@ impl RunLoop { // Mark all non-winning orders as `Considered` for execution self.persistence.store_order_events( - solutions - .iter() - .filter(|participant| !participant.winner) - .flat_map(|participant| participant.solution.order_ids().copied()), + competition::non_winning_orders(&solutions).copied(), OrderEventLabel::Considered, ); // Mark all winning orders as `Executing` self.persistence.store_order_events( - solutions - .iter() - .filter(|participant| participant.winner) - .flat_map(|participant| participant.solution.order_ids().copied()), + competition::winning_orders(&solutions).copied(), OrderEventLabel::Executing, ); @@ -1017,7 +1009,7 @@ impl Metrics { .observe(elapsed.as_secs_f64()); } - fn matched_unsettled(winning: &infra::Driver, unsettled: HashSet) { + fn matched_unsettled(winning: &infra::Driver, unsettled: HashSet<&domain::OrderUid>) { if !unsettled.is_empty() { tracing::debug!(?unsettled, "some orders were matched but not settled"); } @@ -1104,11 +1096,8 @@ pub mod observe { let auction_uids = auction.orders.iter().map(|o| o.uid).collect::>(); - let mut non_winning_orders = solutions - .iter() - .filter(|participant| !participant.winner) - .flat_map(|participant| participant.solution.order_ids().copied()) - .collect::>(); + let mut non_winning_orders = + domain::competition::non_winning_orders(solutions).collect::>(); // Report orders that were part of a non-winning solution candidate // but only if they were part of the auction (filter out jit orders) non_winning_orders.retain(|uid| auction_uids.contains(uid)); From 84e21200bb2457a2cce1619e6f6fb090c25a3ede Mon Sep 17 00:00:00 2001 From: sunce86 Date: Wed, 9 Oct 2024 11:41:49 +0200 Subject: [PATCH 3/7] cr fixes --- .../autopilot/src/domain/competition/mod.rs | 16 ------- crates/autopilot/src/run_loop.rs | 43 +++++++++++++------ crates/autopilot/src/shadow.rs | 2 +- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/crates/autopilot/src/domain/competition/mod.rs b/crates/autopilot/src/domain/competition/mod.rs index ecca079c1c..e186d7f3eb 100644 --- a/crates/autopilot/src/domain/competition/mod.rs +++ b/crates/autopilot/src/domain/competition/mod.rs @@ -104,22 +104,6 @@ pub struct Participant { pub winner: bool, } -/// Returns the order IDs of the winning solutions. -pub fn winning_orders(participants: &[Participant]) -> impl Iterator { - participants - .iter() - .filter(|p| p.winner) - .flat_map(|p| p.solution.order_ids()) -} - -/// Returns the order IDs of the non-winning solutions. -pub fn non_winning_orders(participants: &[Participant]) -> impl Iterator { - participants - .iter() - .filter(|p| !p.winner) - .flat_map(|p| p.solution.order_ids()) -} - #[derive(Debug, thiserror::Error)] #[error("the solver proposed a 0-score solution")] pub struct ZeroScore; diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index e208ea5c8e..db2e9ab9a3 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -274,19 +274,24 @@ impl RunLoop { return; } - // Mark all non-winning orders as `Considered` for execution - self.persistence.store_order_events( - competition::non_winning_orders(&solutions).copied(), - OrderEventLabel::Considered, - ); - // Mark all winning orders as `Executing` + let winning_orders = solutions + .iter() + .filter(|p| p.winner) + .flat_map(|p| p.solution.order_ids().copied()) + .collect::>(); + self.persistence + .store_order_events(winning_orders.clone(), OrderEventLabel::Executing); + + // Mark the rest as `Considered` for execution self.persistence.store_order_events( - competition::winning_orders(&solutions).copied(), - OrderEventLabel::Executing, + solutions + .iter() + .flat_map(|p| p.solution.order_ids().copied()) + .filter(|order_id| !winning_orders.contains(order_id)), + OrderEventLabel::Considered, ); - observe::unsettled(&solutions, &auction); for winner in solutions.iter().filter(|participant| participant.winner) { tracing::info!(driver = %winner.driver.name, solution = %winner.solution.id(), "winner"); @@ -299,6 +304,7 @@ impl RunLoop { ) .await; } + observe::unsettled(&solutions, &auction); } /// Starts settlement execution in a background task. The function is async @@ -571,7 +577,7 @@ impl RunLoop { .solution .orders() .iter() - .flat_map(|(_, order)| vec![order.sell.token, order.buy.token]) + .flat_map(|(_, order)| [order.sell.token, order.buy.token]) .collect::>(); let is_winner = swapped_tokens.is_disjoint(&already_swapped_tokens) @@ -1094,12 +1100,21 @@ pub mod observe { return; }; - let auction_uids = auction.orders.iter().map(|o| o.uid).collect::>(); - - let mut non_winning_orders = - domain::competition::non_winning_orders(solutions).collect::>(); + let mut non_winning_orders = { + let winning_orders = solutions + .iter() + .filter(|p| p.winner) + .flat_map(|p| p.solution.order_ids()) + .collect::>(); + solutions + .iter() + .flat_map(|p| p.solution.order_ids()) + .filter(|uid| !winning_orders.contains(uid)) + .collect::>() + }; // Report orders that were part of a non-winning solution candidate // but only if they were part of the auction (filter out jit orders) + let auction_uids = auction.orders.iter().map(|o| o.uid).collect::>(); non_winning_orders.retain(|uid| auction_uids.contains(uid)); super::Metrics::matched_unsettled(&winner.driver, non_winning_orders); } diff --git a/crates/autopilot/src/shadow.rs b/crates/autopilot/src/shadow.rs index 00f191398b..4e92c65f74 100644 --- a/crates/autopilot/src/shadow.rs +++ b/crates/autopilot/src/shadow.rs @@ -238,7 +238,7 @@ impl RunLoop { let swapped_tokens = solution .orders() .iter() - .flat_map(|(_, order)| vec![order.sell.token, order.buy.token]) + .flat_map(|(_, order)| [order.sell.token, order.buy.token]) .collect::>(); if swapped_tokens.is_disjoint(&already_swapped_tokens) { winners.push(participant); From 1e91d9fcbdd4b8f0bfc9cb0de8c88030cfcab30b Mon Sep 17 00:00:00 2001 From: sunce86 Date: Wed, 9 Oct 2024 12:11:44 +0200 Subject: [PATCH 4/7] find winner --- crates/autopilot/src/run_loop.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index db2e9ab9a3..4c66bab05c 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -407,7 +407,10 @@ impl RunLoop { let start = Instant::now(); // TODO: Support multiple winners // https://github.com/cowprotocol/services/issues/3021 - let Some(winning_solution) = solutions.first().map(|participant| &participant.solution) + let Some(winning_solution) = solutions + .iter() + .find(|participant| participant.winner) + .map(|participant| &participant.solution) else { return Err(anyhow::anyhow!("no winners found")); }; From f29ab45531363688b08e069b618fc323af17f440 Mon Sep 17 00:00:00 2001 From: sunce86 Date: Wed, 9 Oct 2024 12:56:42 +0200 Subject: [PATCH 5/7] remove clones --- crates/autopilot/src/domain/competition/mod.rs | 2 -- crates/autopilot/src/run_loop.rs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/autopilot/src/domain/competition/mod.rs b/crates/autopilot/src/domain/competition/mod.rs index e186d7f3eb..c4712dd9a2 100644 --- a/crates/autopilot/src/domain/competition/mod.rs +++ b/crates/autopilot/src/domain/competition/mod.rs @@ -91,13 +91,11 @@ impl Score { } } -#[derive(Clone)] pub struct RawParticipant { pub driver: Arc, pub solution: Solution, } -#[derive(Clone)] pub struct Participant { pub driver: Arc, pub solution: Solution, diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 4c66bab05c..248254cfac 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -558,7 +558,7 @@ impl RunLoop { .enumerate() .filter_map(|(index, participant)| { if Self::is_solution_fair(participant, &solutions[index..], auction) { - Some(participant.clone()) + Some(participant) } else { tracing::warn!( invalidated = participant.driver.name, From 1fe93f33762904df48bfcf67ec97b804ed4121a6 Mon Sep 17 00:00:00 2001 From: sunce86 Date: Wed, 9 Oct 2024 13:37:52 +0200 Subject: [PATCH 6/7] encapsulate Participant --- .../autopilot/src/domain/competition/mod.rs | 32 +++++++++- crates/autopilot/src/run_loop.rs | 59 ++++++++++--------- 2 files changed, 59 insertions(+), 32 deletions(-) diff --git a/crates/autopilot/src/domain/competition/mod.rs b/crates/autopilot/src/domain/competition/mod.rs index c4712dd9a2..d7484c6c87 100644 --- a/crates/autopilot/src/domain/competition/mod.rs +++ b/crates/autopilot/src/domain/competition/mod.rs @@ -91,15 +91,41 @@ impl Score { } } +#[derive(Clone)] pub struct RawParticipant { pub driver: Arc, pub solution: Solution, } pub struct Participant { - pub driver: Arc, - pub solution: Solution, - pub winner: bool, + inner: RawParticipant, + is_winner: bool, +} + +impl Participant { + pub fn new(inner: RawParticipant, is_winner: bool) -> Self { + Self { inner, is_winner } + } + + pub fn driver(&self) -> &Arc { + &self.inner.driver + } + + pub fn solution(&self) -> &Solution { + &self.inner.solution + } + + pub fn is_winner(&self) -> bool { + self.is_winner + } + + pub fn score(&self) -> Score { + self.inner.solution.score() + } + + pub fn solver(&self) -> eth::Address { + self.inner.solution.solver() + } } #[derive(Debug, thiserror::Error)] diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 248254cfac..07d9172b34 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -277,8 +277,8 @@ impl RunLoop { // Mark all winning orders as `Executing` let winning_orders = solutions .iter() - .filter(|p| p.winner) - .flat_map(|p| p.solution.order_ids().copied()) + .filter(|p| p.is_winner()) + .flat_map(|p| p.solution().order_ids().copied()) .collect::>(); self.persistence .store_order_events(winning_orders.clone(), OrderEventLabel::Executing); @@ -287,19 +287,23 @@ impl RunLoop { self.persistence.store_order_events( solutions .iter() - .flat_map(|p| p.solution.order_ids().copied()) + .flat_map(|p| p.solution().order_ids().copied()) .filter(|order_id| !winning_orders.contains(order_id)), OrderEventLabel::Considered, ); - for winner in solutions.iter().filter(|participant| participant.winner) { - tracing::info!(driver = %winner.driver.name, solution = %winner.solution.id(), "winner"); + for winner in solutions + .iter() + .filter(|participant| participant.is_winner()) + { + let (driver, solution) = (winner.driver(), winner.solution()); + tracing::info!(driver = %driver.name, solution = %solution.id(), "winner"); self.start_settlement_execution( auction.id, single_run_start, - &winner.driver, - &winner.solution, + driver, + solution, block_deadline, ) .await; @@ -409,8 +413,8 @@ impl RunLoop { // https://github.com/cowprotocol/services/issues/3021 let Some(winning_solution) = solutions .iter() - .find(|participant| participant.winner) - .map(|participant| &participant.solution) + .find(|participant| participant.is_winner()) + .map(|participant| participant.solution()) else { return Err(anyhow::anyhow!("no winners found")); }; @@ -419,16 +423,16 @@ impl RunLoop { let reference_score = solutions // todo multiple winners per auction .get(1) - .map(|participant| participant.solution.score().get().0) + .map(|participant| participant.score().get().0) .unwrap_or_default(); let participants = solutions .iter() - .map(|participant| participant.solution.solver().into()) + .map(|participant| participant.solver().into()) .collect::>(); let mut fee_policies = Vec::new(); for order_id in solutions .iter() - .flat_map(|participant| participant.solution.order_ids()) + .flat_map(|participant| participant.solution().order_ids()) .unique() { match auction @@ -466,12 +470,12 @@ impl RunLoop { .rev() .enumerate() .map(|(index, participant)| SolverSettlement { - solver: participant.driver.name.clone(), - solver_address: participant.solution.solver().0, - score: Some(Score::Solver(participant.solution.score().get().0)), + solver: participant.driver().name.clone(), + solver_address: participant.solver().0, + score: Some(Score::Solver(participant.score().get().0)), ranking: solutions.len() - index, orders: participant - .solution + .solution() .orders() .iter() .map(|(id, order)| Order::Colocated { @@ -481,7 +485,7 @@ impl RunLoop { }) .collect(), clearing_prices: participant - .solution + .solution() .prices() .iter() .map(|(token, price)| (token.0, price.get().into())) @@ -575,6 +579,7 @@ impl RunLoop { let mut already_swapped_tokens = HashSet::new(); let mut winners = 0; let solutions = solutions + .cloned() .map(|participant| { let swapped_tokens = participant .solution @@ -590,11 +595,7 @@ impl RunLoop { already_swapped_tokens.extend(swapped_tokens); winners += 1; } - competition::Participant { - driver: participant.driver.clone(), - solution: participant.solution.clone(), - winner: is_winner, - } + competition::Participant::new(participant, is_winner) }) .collect(); @@ -1088,9 +1089,9 @@ pub mod observe { } for participant in solutions { tracing::debug!( - driver = %participant.driver.name, - orders = ?participant.solution.order_ids(), - solution = %participant.solution.id(), + driver = %participant.driver().name, + orders = ?participant.solution().order_ids(), + solution = %participant.solution().id(), "proposed solution" ); } @@ -1106,12 +1107,12 @@ pub mod observe { let mut non_winning_orders = { let winning_orders = solutions .iter() - .filter(|p| p.winner) - .flat_map(|p| p.solution.order_ids()) + .filter(|p| p.is_winner()) + .flat_map(|p| p.solution().order_ids()) .collect::>(); solutions .iter() - .flat_map(|p| p.solution.order_ids()) + .flat_map(|p| p.solution().order_ids()) .filter(|uid| !winning_orders.contains(uid)) .collect::>() }; @@ -1119,6 +1120,6 @@ pub mod observe { // but only if they were part of the auction (filter out jit orders) let auction_uids = auction.orders.iter().map(|o| o.uid).collect::>(); non_winning_orders.retain(|uid| auction_uids.contains(uid)); - super::Metrics::matched_unsettled(&winner.driver, non_winning_orders); + super::Metrics::matched_unsettled(winner.driver(), non_winning_orders); } } From 1567ce375b00cbc262d7e7c36563e5e0d3538a5e Mon Sep 17 00:00:00 2001 From: sunce86 Date: Thu, 10 Oct 2024 11:46:13 +0200 Subject: [PATCH 7/7] refactor Participant --- .../autopilot/src/domain/competition/mod.rs | 48 +++---------------- .../src/domain/competition/participant.rs | 48 +++++++++++++++++++ crates/autopilot/src/run_loop.rs | 36 +++++++------- 3 files changed, 71 insertions(+), 61 deletions(-) create mode 100644 crates/autopilot/src/domain/competition/participant.rs diff --git a/crates/autopilot/src/domain/competition/mod.rs b/crates/autopilot/src/domain/competition/mod.rs index d7484c6c87..d239ae6f44 100644 --- a/crates/autopilot/src/domain/competition/mod.rs +++ b/crates/autopilot/src/domain/competition/mod.rs @@ -1,13 +1,14 @@ use { super::auction::order, - crate::{ - domain::{self, auction, eth}, - infra, - }, + crate::domain::{self, auction, eth}, derive_more::Display, - std::{collections::HashMap, sync::Arc}, + std::collections::HashMap, }; +mod participant; + +pub use participant::{Participant, Ranked, Unranked}; + type SolutionId = u64; #[derive(Debug, Clone)] @@ -91,43 +92,6 @@ impl Score { } } -#[derive(Clone)] -pub struct RawParticipant { - pub driver: Arc, - pub solution: Solution, -} - -pub struct Participant { - inner: RawParticipant, - is_winner: bool, -} - -impl Participant { - pub fn new(inner: RawParticipant, is_winner: bool) -> Self { - Self { inner, is_winner } - } - - pub fn driver(&self) -> &Arc { - &self.inner.driver - } - - pub fn solution(&self) -> &Solution { - &self.inner.solution - } - - pub fn is_winner(&self) -> bool { - self.is_winner - } - - pub fn score(&self) -> Score { - self.inner.solution.score() - } - - pub fn solver(&self) -> eth::Address { - self.inner.solution.solver() - } -} - #[derive(Debug, thiserror::Error)] #[error("the solver proposed a 0-score solution")] pub struct ZeroScore; diff --git a/crates/autopilot/src/domain/competition/participant.rs b/crates/autopilot/src/domain/competition/participant.rs new file mode 100644 index 0000000000..d9563ec3d5 --- /dev/null +++ b/crates/autopilot/src/domain/competition/participant.rs @@ -0,0 +1,48 @@ +use {super::Solution, crate::infra, std::sync::Arc}; + +#[derive(Clone)] +pub struct Participant { + solution: Solution, + driver: Arc, + state: State, +} + +#[derive(Clone)] +pub struct Unranked; +pub struct Ranked { + is_winner: bool, +} + +impl Participant { + pub fn solution(&self) -> &Solution { + &self.solution + } + + pub fn driver(&self) -> &Arc { + &self.driver + } +} + +impl Participant { + pub fn new(solution: Solution, driver: Arc) -> Self { + Self { + solution, + driver, + state: Unranked, + } + } + + pub fn rank(self, is_winner: bool) -> Participant { + Participant:: { + state: Ranked { is_winner }, + solution: self.solution, + driver: self.driver, + } + } +} + +impl Participant { + pub fn is_winner(&self) -> bool { + self.state.is_winner + } +} diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 07d9172b34..fe6aa88937 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -5,7 +5,7 @@ use { domain::{ self, auction::Id, - competition::{self, Solution, SolutionError, TradedOrder}, + competition::{self, Solution, SolutionError, TradedOrder, Unranked}, eth::{self, TxId}, OrderUid, }, @@ -423,11 +423,11 @@ impl RunLoop { let reference_score = solutions // todo multiple winners per auction .get(1) - .map(|participant| participant.score().get().0) + .map(|participant| participant.solution().score().get().0) .unwrap_or_default(); let participants = solutions .iter() - .map(|participant| participant.solver().into()) + .map(|participant| participant.solution().solver().into()) .collect::>(); let mut fee_policies = Vec::new(); for order_id in solutions @@ -471,8 +471,8 @@ impl RunLoop { .enumerate() .map(|(index, participant)| SolverSettlement { solver: participant.driver().name.clone(), - solver_address: participant.solver().0, - score: Some(Score::Solver(participant.score().get().0)), + solver_address: participant.solution().solver().0, + score: Some(Score::Solver(participant.solution().score().get().0)), ranking: solutions.len() - index, orders: participant .solution() @@ -553,7 +553,7 @@ impl RunLoop { // Shuffle so that sorting randomly splits ties. solutions.shuffle(&mut rand::thread_rng()); solutions.sort_unstable_by_key(|participant| { - std::cmp::Reverse(participant.solution.score().get().0) + std::cmp::Reverse(participant.solution().score().get().0) }); // Filter out solutions that are not fair @@ -565,7 +565,7 @@ impl RunLoop { Some(participant) } else { tracing::warn!( - invalidated = participant.driver.name, + invalidated = participant.driver().name, "fairness check invalidated of solution" ); None @@ -582,7 +582,7 @@ impl RunLoop { .cloned() .map(|participant| { let swapped_tokens = participant - .solution + .solution() .orders() .iter() .flat_map(|(_, order)| [order.sell.token, order.buy.token]) @@ -595,7 +595,8 @@ impl RunLoop { already_swapped_tokens.extend(swapped_tokens); winners += 1; } - competition::Participant::new(participant, is_winner) + + participant.rank(is_winner) }) .collect(); @@ -604,11 +605,11 @@ impl RunLoop { /// Returns true if solution is fair to other solutions fn is_solution_fair( - solution: &competition::RawParticipant, - others: &[competition::RawParticipant], + solution: &competition::Participant, + others: &[competition::Participant], auction: &domain::Auction, ) -> bool { - let Some(fairness_threshold) = solution.driver.fairness_threshold else { + let Some(fairness_threshold) = solution.driver().fairness_threshold else { return true; }; @@ -637,7 +638,7 @@ impl RunLoop { // Record best execution per order let mut best_executions = HashMap::new(); for other in others { - for (uid, execution) in other.solution.orders() { + for (uid, execution) in other.solution().orders() { best_executions .entry(uid) .and_modify(|best_execution| { @@ -653,7 +654,7 @@ impl RunLoop { // solution is more than `fairness_threshold` worse than the // order's best execution across all solutions let unfair = solution - .solution + .solution() .orders() .iter() .any(|(uid, current_execution)| { @@ -694,7 +695,7 @@ impl RunLoop { &self, driver: Arc, request: &solve::Request, - ) -> Vec { + ) -> Vec> { let start = Instant::now(); let result = self.try_solve(&driver, request).await; let solutions = match result { @@ -718,10 +719,7 @@ impl RunLoop { .filter_map(|solution| match solution { Ok(solution) => { Metrics::solution_ok(&driver); - Some(competition::RawParticipant { - driver: driver.clone(), - solution, - }) + Some(competition::Participant::new(solution, driver.clone())) } Err(err) => { Metrics::solution_err(&driver, &err);