From 9e1eb58beae773dd4a6bab1c46e4a82f47a93e2b Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Wed, 27 Mar 2024 17:17:03 +0000 Subject: [PATCH] Merge solutions not settlements (#2471) # Description Currently merging multiple solutions into a larger one happens in the `Settlement` type and reuses the legacy settlement encoder. As part of our initiative to fully transition away from the legacy code base this PR moves solution merging logic into our solution domain model. This means we will now merge solutions before simulating and encoding them. This can potentially lead to a lot more solution candidates being generated and requiring simulation. I don't expect this to be an issue in practice as encoding is already part of a stream which will yield the best solution candidate found so far as soon as we approach the solving deadline (thought may have to go into in which order solution candidate should be attempted to be simulated). # Changes - [x] Implement `merge` method on `Solution` - [x] Change the `Id` type on `Solution` to be able to represent when a solution was generated from multiple sub-solutions. - [x] Replace `settlement.merge` with `solution.merge` (this means merging now happens before first simulation) - [x] Change the way merge candidates are generated - [x] Sort stream of to encode settlements by "simples" ie least merged solution (might want to use `score` instead once #2448 is merged) Depending on how risky we think this change is, we can also keep the existing merging logic and have a feature flag guard the two codepaths. ## How to test Existing unit test ## Related Issues Fixes #1478 --- crates/driver/src/boundary/settlement.rs | 7 - crates/driver/src/domain/competition/mod.rs | 94 +++++----- .../src/domain/competition/solution/mod.rs | 165 ++++++++++++++++-- .../domain/competition/solution/settlement.rs | 123 +++---------- crates/driver/src/infra/notify/mod.rs | 28 ++- crates/driver/src/infra/observe/mod.rs | 29 ++- .../src/infra/solver/dto/notification.rs | 21 ++- crates/solvers-dto/src/notification.rs | 10 +- .../solvers/src/api/routes/notify/dto/mod.rs | 7 +- crates/solvers/src/boundary/legacy.rs | 5 + crates/solvers/src/domain/notification.rs | 8 +- 11 files changed, 294 insertions(+), 203 deletions(-) diff --git a/crates/driver/src/boundary/settlement.rs b/crates/driver/src/boundary/settlement.rs index b2e7667423..6d1024e67f 100644 --- a/crates/driver/src/boundary/settlement.rs +++ b/crates/driver/src/boundary/settlement.rs @@ -242,13 +242,6 @@ impl Settlement { Ok(eth::U256::from_big_rational(&quality)?.into()) } - pub fn merge(self, other: Self) -> Result { - self.inner.merge(other.inner).map(|inner| Self { - inner, - solver: self.solver, - }) - } - pub fn clearing_prices(&self) -> HashMap { self.inner .clearing_prices() diff --git a/crates/driver/src/domain/competition/mod.rs b/crates/driver/src/domain/competition/mod.rs index 423c53e19d..301e9586a7 100644 --- a/crates/driver/src/domain/competition/mod.rs +++ b/crates/driver/src/domain/competition/mod.rs @@ -13,7 +13,7 @@ use { }, util::Bytes, }, - futures::{stream::FuturesUnordered, Stream, StreamExt}, + futures::{stream::FuturesUnordered, StreamExt}, itertools::Itertools, std::{ collections::{HashMap, HashSet}, @@ -83,7 +83,7 @@ impl Competition { // Discard solutions that don't have unique ID. let mut ids = HashSet::new(); let solutions = solutions.into_iter().filter(|solution| { - if !ids.insert(solution.id()) { + if !ids.insert(solution.id().clone()) { observe::duplicated_solution_id(self.solver.name(), solution.id()); notify::duplicated_solution_id(&self.solver, auction.id(), solution.id()); false @@ -96,18 +96,21 @@ impl Competition { let solutions = solutions.filter(|solution| { if solution.is_empty() { observe::empty_solution(self.solver.name(), solution.id()); - notify::empty_solution(&self.solver, auction.id(), solution.id()); + notify::empty_solution(&self.solver, auction.id(), solution.id().clone()); false } else { true } }); + let merged = merge(solutions, auction); + // Encode solutions into settlements (streamed). - let encoded = solutions + let encoded = merged + .into_iter() .map(|solution| async move { - let id = solution.id(); - observe::encoding(id); + let id = solution.id().clone(); + observe::encoding(&id); let settlement = solution.encode(auction, &self.eth, &self.simulator).await; (id, settlement) }) @@ -115,8 +118,8 @@ impl Competition { .filter_map(|(id, result)| async move { result .tap_err(|err| { - observe::encoding_failed(self.solver.name(), id, err); - notify::encoding_failed(&self.solver, auction.id(), id, err); + observe::encoding_failed(self.solver.name(), &id, err); + notify::encoding_failed(&self.solver, auction.id(), &id, err); }) .ok() }); @@ -124,9 +127,15 @@ impl Competition { // Merge settlements as they arrive until there are no more new settlements or // timeout is reached. let mut settlements = Vec::new(); + let future = async { + let mut encoded = std::pin::pin!(encoded); + while let Some(settlement) = encoded.next().await { + settlements.push(settlement); + } + }; if tokio::time::timeout( auction.deadline().driver().remaining().unwrap_or_default(), - merge_settlements(&mut settlements, encoded, &self.eth, &self.simulator), + future, ) .await .is_err() @@ -157,7 +166,7 @@ impl Competition { notify::scoring_failed( &self.solver, auction.id(), - settlement.notify_id(), + settlement.solution(), err, ); }) @@ -211,15 +220,13 @@ impl Competition { observe::winner_voided(block, &err); *score_ref = None; *self.settlement.lock().unwrap() = None; - if let Some(id) = settlement.notify_id() { - notify::simulation_failed( - &self.solver, - auction.id(), - id, - &infra::simulator::Error::Revert(err), - true, - ); - } + notify::simulation_failed( + &self.solver, + auction.id(), + settlement.solution(), + &infra::simulator::Error::Revert(err), + true, + ); return; } } @@ -268,7 +275,7 @@ impl Competition { notify::executed( &self.solver, settlement.auction_id, - settlement.notify_id(), + settlement.solution(), &executed, ); @@ -322,34 +329,37 @@ impl Competition { } } -/// Tries to merge the incoming stream of new settlements into existing ones. -/// Always adds the new settlement by itself. -async fn merge_settlements( - merged: &mut Vec, - new: impl Stream, - eth: &Ethereum, - simulator: &Simulator, -) { - let eth = eth.with_metric_label("mergeSettlements".into()); - let mut new = std::pin::pin!(new); - while let Some(settlement) = new.next().await { - // Try to merge [`settlement`] into some settlements. - for other in merged.iter_mut() { - match other.merge(&settlement, ð, simulator).await { - Ok(m) => { - *other = m; - observe::merged(&settlement, other); - // could possibly break here if we want to avoid merging - // into multiple settlements +/// Creates a vector with all possible combinations of the given solutions. +/// The result is sorted by the number of merges, so the first elements are the +/// original solutions. +fn merge(solutions: impl Iterator, auction: &Auction) -> Vec { + let mut merged: Vec = Vec::new(); + for solution in solutions { + let mut extension = vec![]; + for already_merged in merged.iter() { + match solution.merge(already_merged) { + Ok(merged) => { + observe::merged(&solution, already_merged, &merged); + extension.push(merged); } Err(err) => { - observe::not_merged(&settlement, other, err); + observe::not_merged(&solution, already_merged, err); } } } - // add [`settlement`] by itself - merged.push(settlement); + // At least insert the current solution + extension.push(solution); + merged.extend(extension); } + + // Sort merged solutions descending by score. + merged.sort_by_key(|solution| { + solution + .scoring(&auction.prices()) + .map(|score| score.0) + .unwrap_or_default() + }); + merged } /// Solution information sent to the protocol by the driver before the solution diff --git a/crates/driver/src/domain/competition/solution/mod.rs b/crates/driver/src/domain/competition/solution/mod.rs index 7c6989bc52..b6ff84a845 100644 --- a/crates/driver/src/domain/competition/solution/mod.rs +++ b/crates/driver/src/domain/competition/solution/mod.rs @@ -18,7 +18,8 @@ use { }, futures::future::try_join_all, itertools::Itertools, - std::collections::{BTreeSet, HashMap}, + num::{BigRational, One}, + std::collections::{hash_map::Entry, BTreeSet, HashMap, HashSet}, thiserror::Error, }; @@ -30,6 +31,8 @@ pub mod trade; pub use {error::Error, interaction::Interaction, settlement::Settlement, trade::Trade}; +type Prices = HashMap; + // TODO Add a constructor and ensure that the clearing prices are included for // each trade /// A solution represents a set of orders which the solver has found an optimal @@ -39,7 +42,7 @@ pub use {error::Error, interaction::Interaction, settlement::Settlement, trade:: pub struct Solution { id: Id, trades: Vec, - prices: HashMap, + prices: Prices, interactions: Vec, solver: Solver, score: SolverScore, @@ -53,7 +56,7 @@ impl Solution { pub fn new( id: Id, trades: Vec, - prices: HashMap, + prices: Prices, interactions: Vec, solver: Solver, score: SolverScore, @@ -111,8 +114,8 @@ impl Solution { } /// The ID of this solution. - pub fn id(&self) -> Id { - self.id + pub fn id(&self) -> &Id { + &self.id } /// Trades settled by this solution. @@ -229,6 +232,73 @@ impl Solution { self.user_trades().next().is_none() } + pub fn merge(&self, other: &Self) -> Result { + // We can only merge solutions from the same solver + if self.solver.account().address() != other.solver.account().address() { + return Err(error::Merge::Incompatible("Solvers")); + } + + // Solutions should not settle the same order twice + let uids: HashSet<_> = self.user_trades().map(|t| t.order().uid).collect(); + let other_uids: HashSet<_> = other.user_trades().map(|t| t.order().uid).collect(); + if !uids.is_disjoint(&other_uids) { + return Err(error::Merge::DuplicateTrade); + } + + // Solution prices need to be congruent, i.e. there needs to be a unique factor + // to scale all common tokens from one solution into the other. + let factor = + scaling_factor(&self.prices, &other.prices).ok_or(error::Merge::IncongruentPrices)?; + + // To avoid precision issues, make sure we always scale up settlements + if factor < BigRational::one() { + return other.merge(self); + } + + // Scale prices + let mut prices = self.prices.clone(); + for (token, price) in other.prices.iter() { + let scaled = number::conversions::big_rational_to_u256( + &(number::conversions::u256_to_big_rational(price) * &factor), + ) + .map_err(error::Merge::Math)?; + match prices.entry(*token) { + Entry::Occupied(entry) => { + // This shouldn't fail unless there are rounding errors given that the scaling + // factor is unique + if *entry.get() != scaled { + return Err(error::Merge::IncongruentPrices); + } + } + Entry::Vacant(entry) => { + entry.insert(scaled); + } + } + } + + // Merge remaining fields + Ok(Solution { + id: Id::Merged([self.id.ids(), other.id.ids()].concat()), + trades: [self.trades.clone(), other.trades.clone()].concat(), + prices, + interactions: [self.interactions.clone(), other.interactions.clone()].concat(), + solver: self.solver.clone(), + score: match self.score.merge(&other.score) { + Some(score) => score, + None => return Err(error::Merge::Incompatible("Scores")), + }, + weth: self.weth, + // Same solver are guaranteed to have the same fee handler + fee_handler: self.fee_handler, + gas: match (self.gas, other.gas) { + (Some(gas), Some(other_gas)) => Some(gas + other_gas), + (Some(gas), None) => Some(gas), + (None, Some(gas)) => Some(gas), + (None, None) => None, + }, + }) + } + /// Return the trades which fulfill non-liquidity auction orders. These are /// the orders placed by end users. fn user_trades(&self) -> impl Iterator { @@ -350,6 +420,33 @@ impl std::fmt::Debug for Solution { } } +/// Given two solutions returns the factors with +/// which prices of the second solution would have to be multiplied so that the +/// given token would have the same price in both solutions. +/// If the solutions have no prices in common any scaling factor is valid (we +/// return 1). Returns None if the solutions have more than one price in common +/// and the scaling factor is not unique. +fn scaling_factor(first: &Prices, second: &Prices) -> Option { + let factors: HashSet<_> = first + .keys() + .collect::>() + .intersection(&second.keys().collect::>()) + .map(|&token| { + let first_price = first[token]; + let second_price = second[token]; + BigRational::new( + number::conversions::u256_to_big_int(&second_price), + number::conversions::u256_to_big_int(&first_price), + ) + }) + .collect(); + match factors.len() { + 0 => Some(BigRational::one()), + 1 => factors.into_iter().next(), + _ => None, + } +} + /// Carries information how the score should be calculated. #[derive(Debug, Clone)] pub enum SolverScore { @@ -357,28 +454,66 @@ pub enum SolverScore { RiskAdjusted(f64), Surplus, } -/// A unique solution ID. This ID is generated by the solver and only needs to -/// be unique within a single round of competition. This ID is only important in -/// the communication between the driver and the solver, and it is not used by -/// the protocol. -#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] -pub struct Id(pub u64); + +impl SolverScore { + fn merge(&self, other: &SolverScore) -> Option { + match (self, other) { + (SolverScore::Solver(a), SolverScore::Solver(b)) => Some(SolverScore::Solver(a + b)), + (SolverScore::RiskAdjusted(a), SolverScore::RiskAdjusted(b)) => { + Some(SolverScore::RiskAdjusted(a * b)) + } + _ => None, + } + } +} + +/// A unique reference to a specific solution. Sings IDs are generated by the +/// solver and need to be unique within a single round of competition. If the +/// driver merges solutions it combines individual IDs. This reference is only +/// important in the communication between the driver and the solver, and it is +/// not used by the protocol. +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +pub enum Id { + Single(u64), + Merged(Vec), +} impl From for Id { fn from(value: u64) -> Self { - Self(value) + Id::Single(value) } } -impl From for u64 { - fn from(value: Id) -> Self { - value.0 +impl Id { + /// Returns the number of solutions that has gone into merging this + /// solution. + pub fn count_merges(&self) -> usize { + self.ids().len() + } + + pub fn ids(&self) -> Vec { + match self { + Id::Single(id) => vec![*id], + Id::Merged(ids) => ids.clone(), + } } } pub mod error { use super::*; + #[derive(Debug, thiserror::Error)] + pub enum Merge { + #[error("incompatible {0:?}")] + Incompatible(&'static str), + #[error("duplicate trade")] + DuplicateTrade, + #[error("incongruent prices")] + IncongruentPrices, + #[error("math error: {0:?}")] + Math(anyhow::Error), + } + #[derive(Debug, thiserror::Error)] pub enum Error { #[error("blockchain error: {0:?}")] diff --git a/crates/driver/src/domain/competition/solution/settlement.rs b/crates/driver/src/domain/competition/solution/settlement.rs index dd89bfa778..44372c2f39 100644 --- a/crates/driver/src/domain/competition/solution/settlement.rs +++ b/crates/driver/src/domain/competition/solution/settlement.rs @@ -10,7 +10,7 @@ use { infra::{blockchain::Ethereum, observe, Simulator}, }, futures::future::try_join_all, - std::collections::{BTreeSet, HashMap, HashSet}, + std::collections::{BTreeSet, HashMap}, }; /// A transaction calling into our settlement contract on the blockchain, ready @@ -42,8 +42,7 @@ pub struct Settlement { pub access_list: eth::AccessList, /// The gas parameters used by the settlement. pub gas: Gas, - /// See the [`Settlement::solutions`] method. - solutions: HashMap, + solution: Solution, } impl Settlement { @@ -73,20 +72,13 @@ impl Settlement { // Encode the solution into a settlement. let boundary = boundary::Settlement::encode(eth, &solution, auction).await?; - Self::new( - auction.id().unwrap(), - [(solution.id, solution)].into(), - boundary, - eth, - simulator, - ) - .await + Self::new(auction.id().unwrap(), solution, boundary, eth, simulator).await } /// Create a new settlement and ensure that it is valid. async fn new( auction_id: auction::Id, - solutions: HashMap, + solution: Solution, settlement: boundary::Settlement, eth: &Ethereum, simulator: &Simulator, @@ -102,10 +94,7 @@ impl Settlement { // The solution is to do access list estimation in two steps: first, simulate // moving 1 wei into every smart contract to get a partial access list, and then // use that partial access list to calculate the final access list. - let user_trades = solutions - .values() - .flat_map(|solution| solution.user_trades()); - let partial_access_lists = try_join_all(user_trades.map(|trade| async { + let partial_access_lists = try_join_all(solution.user_trades().map(|trade| async { if !trade.order().buys_eth() || !trade.order().pays_to_contract(eth).await? { return Ok(Default::default()); } @@ -144,9 +133,9 @@ impl Settlement { } // Is at least one interaction internalized? - if solutions - .values() - .flat_map(|solution| solution.interactions.iter()) + if solution + .interactions() + .iter() .any(|interaction| interaction.internalize()) { // Some rules which are enforced by the settlement contract for non-internalized @@ -168,7 +157,7 @@ impl Settlement { Ok(Self { auction_id, - solutions, + solution, boundary: settlement, access_list, gas, @@ -221,12 +210,7 @@ impl Settlement { ) -> Result { let prices = auction.prices(); - self.solutions - .values() - .map(|solution| solution.scoring(&prices)) - .try_fold(eth::Ether(0.into()), |acc, score| { - score.map(|score| acc + score) - }) + self.solution.scoring(&prices) } // TODO(#1494): score() should be defined on Solution rather than Settlement. @@ -239,7 +223,7 @@ impl Settlement { ) -> Result { // For testing purposes, calculate CIP38 even before activation let score = self.cip38_score(auction); - tracing::info!(?score, "CIP38 score for settlement: {:?}", self.solutions()); + tracing::info!(?score, "CIP38 score for settlement: {:?}", self.solution); let score = match self.boundary.score() { competition::SolverScore::Solver(score) => { @@ -281,47 +265,9 @@ impl Settlement { Ok(score) } - // TODO(#1478): merge() should be defined on Solution rather than Settlement. - /// Merge another settlement into this settlement. - /// - /// Merging settlements results in a score that can be anything due to the - /// fact that contracts can do basically anything, but in practice it can be - /// assumed that the score will be at least equal to the sum of the scores - /// of the merged settlements. - pub async fn merge( - &self, - other: &Self, - eth: &Ethereum, - simulator: &Simulator, - ) -> Result { - // The solver must be the same for both settlements. - if self.boundary.solver != other.boundary.solver { - return Err(Error::DifferentSolvers); - } - - // Merge the settlements. - let mut solutions = self.solutions.clone(); - solutions.extend( - other - .solutions - .iter() - .map(|(id, solution)| (*id, solution.clone())), - ); - Self::new( - self.auction_id, - solutions, - self.boundary.clone().merge(other.boundary.clone())?, - eth, - simulator, - ) - .await - } - - /// The solutions encoded in this settlement. This is a [`HashSet`] because - /// multiple solutions can be encoded in a single settlement due to - /// merging. See [`Self::merge`]. - pub fn solutions(&self) -> HashSet { - self.solutions.keys().copied().collect() + /// The solution encoded in this settlement. + pub fn solution(&self) -> &super::Id { + self.solution.id() } /// Address of the solver which generated this settlement. @@ -331,48 +277,33 @@ impl Settlement { /// The settled user orders with their in/out amounts. pub fn orders(&self) -> HashMap { - self.solutions - .values() - .fold(Default::default(), |mut acc, solution| { - for trade in solution.user_trades() { - let order = acc.entry(trade.order().uid).or_default(); - let prices = ClearingPrices { - sell: solution.prices - [&trade.order().sell.token.wrap(solution.weth)], - buy: solution.prices - [&trade.order().buy.token.wrap(solution.weth)], - }; - order.sell = trade.sell_amount(&prices).unwrap_or_else(|err| { + let mut acc: HashMap = HashMap::new(); + for trade in self.solution.user_trades() { + let order = acc.entry(trade.order().uid).or_default(); + let prices = ClearingPrices { + sell: self.solution.prices[&trade.order().sell.token.wrap(self.solution.weth)], + buy: self.solution.prices[&trade.order().buy.token.wrap(self.solution.weth)], + }; + order.sell = trade.sell_amount(&prices).unwrap_or_else(|err| { // This should never happen, returning 0 is better than panicking, but we // should still alert. - tracing::error!(?trade, prices=?solution.prices, ?err, "could not compute sell_amount"); + tracing::error!(?trade, prices=?self.solution.prices, ?err, "could not compute sell_amount"); 0.into() }); - order.buy = trade.buy_amount(&prices).unwrap_or_else(|err| { + order.buy = trade.buy_amount(&prices).unwrap_or_else(|err| { // This should never happen, returning 0 is better than panicking, but we // should still alert. - tracing::error!(?trade, prices=?solution.prices, ?err, "could not compute buy_amount"); + tracing::error!(?trade, prices=?self.solution.prices, ?err, "could not compute buy_amount"); 0.into() }); - } - acc - }) + } + acc } /// The uniform price vector this settlement proposes pub fn prices(&self) -> HashMap { self.boundary.clearing_prices() } - - /// Settlements have valid notify ID only if they are originated from a - /// single solution. Otherwise, for merged settlements, no notifications - /// are sent, therefore, notify id is None. - pub fn notify_id(&self) -> Option { - match self.solutions.len() { - 1 => self.solutions.keys().next().copied(), - _ => None, - } - } } /// Should the interactions be internalized? diff --git a/crates/driver/src/infra/notify/mod.rs b/crates/driver/src/infra/notify/mod.rs index 2c03ac4ec1..300dafb7e6 100644 --- a/crates/driver/src/infra/notify/mod.rs +++ b/crates/driver/src/infra/notify/mod.rs @@ -26,13 +26,9 @@ pub fn empty_solution(solver: &Solver, auction_id: Option, solution pub fn scoring_failed( solver: &Solver, auction_id: Option, - solution_id: Option, + solution_id: &solution::Id, err: &score::Error, ) { - if solution_id.is_none() { - return; - } - let notification = match err { score::Error::ZeroScore => { notification::Kind::ScoringFailed(notification::ScoreKind::ZeroScore) @@ -56,13 +52,13 @@ pub fn scoring_failed( score::Error::Scoring(_) => return, // TODO: should we notify? }; - solver.notify(auction_id, solution_id, notification); + solver.notify(auction_id, Some(solution_id.clone()), notification); } pub fn encoding_failed( solver: &Solver, auction_id: Option, - solution_id: solution::Id, + solution_id: &solution::Id, err: &solution::Error, ) { let notification = match err { @@ -82,13 +78,13 @@ pub fn encoding_failed( solution::Error::DifferentSolvers => return, }; - solver.notify(auction_id, Some(solution_id), notification); + solver.notify(auction_id, Some(solution_id.clone()), notification); } pub fn simulation_failed( solver: &Solver, auction_id: Option, - solution_id: solution::Id, + solution_id: &solution::Id, err: &simulator::Error, succeeded_at_least_once: SimulationSucceededAtLeastOnce, ) { @@ -100,19 +96,15 @@ pub fn simulation_failed( ), simulator::Error::Other(error) => notification::Kind::DriverError(error.to_string()), }; - solver.notify(auction_id, Some(solution_id), kind); + solver.notify(auction_id, Some(solution_id.clone()), kind); } pub fn executed( solver: &Solver, auction_id: auction::Id, - solution_id: Option, + solution_id: &solution::Id, res: &Result, ) { - if solution_id.is_none() { - return; - }; - let kind = match res { Ok(hash) => notification::Settlement::Success(hash.clone()), Err(Error::Revert(hash)) => notification::Settlement::Revert(hash.clone()), @@ -122,7 +114,7 @@ pub fn executed( solver.notify( Some(auction_id), - solution_id, + Some(solution_id.clone()), notification::Kind::Settled(kind), ); } @@ -130,11 +122,11 @@ pub fn executed( pub fn duplicated_solution_id( solver: &Solver, auction_id: Option, - solution_id: solution::Id, + solution_id: &solution::Id, ) { solver.notify( auction_id, - Some(solution_id), + Some(solution_id.clone()), notification::Kind::DuplicatedSolutionId, ); } diff --git a/crates/driver/src/infra/observe/mod.rs b/crates/driver/src/infra/observe/mod.rs index bf6a29f903..c81c95800f 100644 --- a/crates/driver/src/infra/observe/mod.rs +++ b/crates/driver/src/infra/observe/mod.rs @@ -62,7 +62,7 @@ pub fn fetching_liquidity_failed(err: &boundary::Error) { tracing::warn!(?err, "failed to fetch liquidity"); } -pub fn duplicated_solution_id(solver: &solver::Name, id: solution::Id) { +pub fn duplicated_solution_id(solver: &solver::Name, id: &solution::Id) { tracing::debug!(?id, "discarded solution: duplicated id"); metrics::get() .dropped_solutions @@ -80,7 +80,7 @@ pub fn solutions(solutions: &[Solution]) { } /// Observe that a solution was discarded because it is empty. -pub fn empty_solution(solver: &solver::Name, id: solution::Id) { +pub fn empty_solution(solver: &solver::Name, id: &solution::Id) { tracing::debug!(?id, "discarded solution: empty"); metrics::get() .dropped_solutions @@ -107,12 +107,12 @@ pub fn postprocessing_timed_out(completed: &[Settlement]) { } /// Observe that a solution is about to be encoded into a settlement. -pub fn encoding(id: solution::Id) { +pub fn encoding(id: &solution::Id) { tracing::trace!(?id, "encoding settlement"); } /// Observe that settlement encoding failed. -pub fn encoding_failed(solver: &solver::Name, id: solution::Id, err: &solution::Error) { +pub fn encoding_failed(solver: &solver::Name, id: &solution::Id, err: &solution::Error) { tracing::info!(?id, ?err, "discarded solution: settlement encoding"); metrics::get() .dropped_solutions @@ -121,28 +121,19 @@ pub fn encoding_failed(solver: &solver::Name, id: solution::Id, err: &solution:: } /// Observe that two solutions were merged. -pub fn merged(settlement: &Settlement, other: &Settlement) { - tracing::debug!( - settlement_1 = ?settlement.solutions(), - settlement_2 = ?other.solutions(), - "merged solutions" - ); +pub fn merged(first: &Solution, other: &Solution, result: &Solution) { + tracing::debug!(?first, ?other, ?result, "merged solutions"); } /// Observe that it was not possible to merge two solutions. -pub fn not_merged(settlement: &Settlement, other: &Settlement, err: solution::Error) { - tracing::debug!( - ?err, - settlement_1 = ?settlement.solutions(), - settlement_2 = ?other.solutions(), - "solutions can't be merged" - ); +pub fn not_merged(first: &Solution, other: &Solution, err: solution::error::Merge) { + tracing::debug!(?err, ?first, ?other, "solutions can't be merged"); } /// Observe that scoring is about to start. pub fn scoring(settlement: &Settlement) { tracing::trace!( - solutions = ?settlement.solutions(), + solution = ?settlement.solution(), gas = ?settlement.gas, "scoring settlement" ); @@ -160,7 +151,7 @@ pub fn scoring_failed(solver: &solver::Name, err: &score::Error) { /// Observe the settlement score. pub fn score(settlement: &Settlement, score: &competition::Score) { tracing::info!( - solutions = ?settlement.solutions(), + solution = ?settlement.solution(), score = ?score, "scored settlement" ); diff --git a/crates/driver/src/infra/solver/dto/notification.rs b/crates/driver/src/infra/solver/dto/notification.rs index b8cb5d1f5c..47ce712f26 100644 --- a/crates/driver/src/infra/solver/dto/notification.rs +++ b/crates/driver/src/infra/solver/dto/notification.rs @@ -21,7 +21,7 @@ impl Notification { ) -> Self { Self { auction_id: auction_id.as_ref().map(ToString::to_string), - solution_id: solution_id.map(|id| id.0), + solution_id: solution_id.map(SolutionId::from_domain), kind: match kind { notify::Kind::Timeout => Kind::Timeout, notify::Kind::EmptySolution => Kind::EmptySolution, @@ -89,11 +89,28 @@ impl Notification { #[serde(rename_all = "camelCase")] pub struct Notification { auction_id: Option, - solution_id: Option, + solution_id: Option, #[serde(flatten)] kind: Kind, } +#[serde_as] +#[derive(Debug, Serialize)] +#[serde(untagged)] +pub enum SolutionId { + Single(u64), + Merged(Vec), +} + +impl SolutionId { + pub fn from_domain(id: solution::Id) -> Self { + match id { + solution::Id::Single(id) => SolutionId::Single(id), + solution::Id::Merged(ids) => SolutionId::Merged(ids), + } + } +} + #[serde_as] #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase", tag = "kind")] diff --git a/crates/solvers-dto/src/notification.rs b/crates/solvers-dto/src/notification.rs index 135a1ebfd1..1136108abf 100644 --- a/crates/solvers-dto/src/notification.rs +++ b/crates/solvers-dto/src/notification.rs @@ -13,11 +13,19 @@ use { pub struct Notification { #[serde_as(as = "Option")] pub auction_id: Option, - pub solution_id: Option, + pub solution_id: Option, #[serde(flatten)] pub kind: Kind, } +#[serde_as] +#[derive(Debug, Deserialize)] +#[serde(untagged)] +pub enum SolutionId { + Single(u64), + Merged(Vec), +} + #[serde_as] #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase", tag = "kind")] diff --git a/crates/solvers/src/api/routes/notify/dto/mod.rs b/crates/solvers/src/api/routes/notify/dto/mod.rs index 2e4ee5cce2..a3a0d3eb45 100644 --- a/crates/solvers/src/api/routes/notify/dto/mod.rs +++ b/crates/solvers/src/api/routes/notify/dto/mod.rs @@ -1,4 +1,4 @@ -pub use solvers_dto::notification::Notification; +pub use solvers_dto::notification::{Notification, SolutionId}; use crate::domain::{auction, eth, notification}; @@ -11,7 +11,10 @@ pub fn to_domain( Some(id) => auction::Id::Solve(id), None => auction::Id::Quote, }, - solution_id: notification.solution_id.map(Into::into), + solution_id: notification.solution_id.as_ref().map(|id| match id { + SolutionId::Single(id) => notification::Id::Single((*id).into()), + SolutionId::Merged(ids) => notification::Id::Merged(ids.to_vec()), + }), kind: match ¬ification.kind { solvers_dto::notification::Kind::Timeout => notification::Kind::Timeout, solvers_dto::notification::Kind::EmptySolution => notification::Kind::EmptySolution, diff --git a/crates/solvers/src/boundary/legacy.rs b/crates/solvers/src/boundary/legacy.rs index a6371b6854..6bb7774b2b 100644 --- a/crates/solvers/src/boundary/legacy.rs +++ b/crates/solvers/src/boundary/legacy.rs @@ -643,6 +643,11 @@ fn to_boundary_auction_result(notification: ¬ification::Notification) -> (i64 AuctionResult::Rejected(SolverRejectionReason::DuplicatedSolutionId( notification .solution_id + .clone() + .and_then(|id| match id { + notification::Id::Single(id) => Some(id), + notification::Id::Merged(_) => None, + }) .expect("duplicated solution ID notification must have a solution ID") .0, )) diff --git a/crates/solvers/src/domain/notification.rs b/crates/solvers/src/domain/notification.rs index 85f72ba454..bdf92c8d3d 100644 --- a/crates/solvers/src/domain/notification.rs +++ b/crates/solvers/src/domain/notification.rs @@ -19,10 +19,16 @@ pub type SimulationSucceededAtLeastOnce = bool; #[derive(Debug)] pub struct Notification { pub auction_id: auction::Id, - pub solution_id: Option, + pub solution_id: Option, pub kind: Kind, } +#[derive(Debug, Clone)] +pub enum Id { + Single(solution::Id), + Merged(Vec), +} + /// All types of notifications solvers can be informed about. #[derive(Debug)] pub enum Kind {