From 9f10adfdee39745efa3e8182811be0e61de7159e Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Fri, 9 Aug 2024 10:52:54 +0200 Subject: [PATCH] [EASY] Remove Observation object (#2873) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Extracts part of https://github.com/cowprotocol/services/pull/2861/files that can be merged now, and don't have to be part of that PR. This enables me to have a nice pub interface of `Settlement` and continue adding functions for https://github.com/cowprotocol/services/issues/2862 and not wait for ☝️ PR to be merged. Also https://github.com/cowprotocol/services/pull/2861 becomes smaller and easier to revert if we encounter some issue on weekly release. --- .../src/domain/settlement/auction.rs | 4 +- crates/autopilot/src/domain/settlement/mod.rs | 55 +++++++++---------- .../src/domain/settlement/observation.rs | 25 --------- .../src/domain/settlement/solution/mod.rs | 4 +- .../src/on_settlement_event_updater.rs | 30 +++++----- 5 files changed, 44 insertions(+), 74 deletions(-) delete mode 100644 crates/autopilot/src/domain/settlement/observation.rs diff --git a/crates/autopilot/src/domain/settlement/auction.rs b/crates/autopilot/src/domain/settlement/auction.rs index 9260eef6b7..a06646353f 100644 --- a/crates/autopilot/src/domain/settlement/auction.rs +++ b/crates/autopilot/src/domain/settlement/auction.rs @@ -2,7 +2,7 @@ use { crate::domain::{self}, - std::collections::HashMap, + std::collections::{HashMap, HashSet}, }; #[derive(Debug)] @@ -15,7 +15,7 @@ pub struct Auction { pub prices: domain::auction::Prices, /// JIT orders with surplus capturing JIT order owners should capture /// surplus if settled. - pub surplus_capturing_jit_order_owners: Vec, + pub surplus_capturing_jit_order_owners: HashSet, } impl Auction { diff --git a/crates/autopilot/src/domain/settlement/mod.rs b/crates/autopilot/src/domain/settlement/mod.rs index 5d253887fe..f086028348 100644 --- a/crates/autopilot/src/domain/settlement/mod.rs +++ b/crates/autopilot/src/domain/settlement/mod.rs @@ -3,20 +3,14 @@ //! A winning solution becomes a [`Settlement`] once it is executed on-chain. use { - super::competition, crate::{domain, domain::eth, infra}, + std::collections::HashMap, }; mod auction; -mod observation; mod solution; mod transaction; -pub use { - auction::Auction, - observation::Observation, - solution::Solution, - transaction::Transaction, -}; +pub use {auction::Auction, solution::Solution, transaction::Transaction}; /// A solution together with the `Auction` for which it was picked as a winner /// and executed on-chain. @@ -80,13 +74,6 @@ impl Settlement { score, ); } - if score < promised_solution.score() { - return Err(Error::ScoreMismatch { - expected: promised_solution.score(), - got: score, - }) - .map_err(with(auction_id)); - } Ok(Self { solution, @@ -95,15 +82,30 @@ impl Settlement { }) } - /// Returns the observation of the settlement. - pub fn observation(&self) -> Observation { - Observation { - gas: self.transaction.gas, - gas_price: self.transaction.effective_gas_price, - surplus: self.solution.native_surplus(&self.auction), - fee: self.solution.native_fee(&self.auction.prices), - order_fees: self.solution.fees(&self.auction.prices), - } + /// The gas used by the settlement. + pub fn gas(&self) -> eth::Gas { + self.transaction.gas + } + + /// The effective gas price at the time of settlement. + pub fn gas_price(&self) -> eth::EffectiveGasPrice { + self.transaction.effective_gas_price + } + + /// Total surplus expressed in native token. + pub fn native_surplus(&self) -> eth::Ether { + self.solution.native_surplus(&self.auction) + } + + /// Total fee expressed in native token. + pub fn native_fee(&self) -> eth::Ether { + self.solution.native_fee(&self.auction.prices) + } + + /// Per order fees denominated in sell token. Contains all orders from the + /// settlement + pub fn order_fees(&self) -> HashMap> { + self.solution.fees(&self.auction.prices) } } @@ -131,11 +133,6 @@ pub enum Error { expected: eth::Address, got: eth::Address, }, - #[error("score mismatch: expected competition score {expected}, settlement score {got}")] - ScoreMismatch { - expected: competition::Score, - got: competition::Score, - }, } /// Errors that can occur when fetching data from the persistence layer. diff --git a/crates/autopilot/src/domain/settlement/observation.rs b/crates/autopilot/src/domain/settlement/observation.rs deleted file mode 100644 index 1be82d2662..0000000000 --- a/crates/autopilot/src/domain/settlement/observation.rs +++ /dev/null @@ -1,25 +0,0 @@ -//! Aggregated type containing all important information about the mined -//! settlement, including the surplus and fees. -//! -//! Observation is a snapshot of the settlement state, which purpose is to save -//! the state of the settlement to the persistence layer. - -use { - crate::domain::{self, eth}, - std::collections::HashMap, -}; - -#[derive(Debug, Clone)] -pub struct Observation { - /// The gas used by the settlement. - pub gas: eth::Gas, - /// The effective gas price at the time of settlement. - pub gas_price: eth::EffectiveGasPrice, - /// Total surplus expressed in native token. - pub surplus: eth::Ether, - /// Total fee expressed in native token. - pub fee: eth::Ether, - /// Per order fees denominated in sell token. Contains all orders from the - /// settlement - pub order_fees: HashMap>, -} diff --git a/crates/autopilot/src/domain/settlement/solution/mod.rs b/crates/autopilot/src/domain/settlement/solution/mod.rs index 4be4cbfe9f..e004d88b20 100644 --- a/crates/autopilot/src/domain/settlement/solution/mod.rs +++ b/crates/autopilot/src/domain/settlement/solution/mod.rs @@ -308,7 +308,7 @@ mod tests { let auction = super::super::Auction { prices, - surplus_capturing_jit_order_owners: vec![], + surplus_capturing_jit_order_owners: Default::default(), id: 0, orders: HashMap::from([(domain::OrderUid(hex!("10dab31217bb6cc2ace0fe601c15d342f7626a1ee5ef0495449800e73156998740a50cf069e992aa4536211b23f286ef88752187ffffffff")), vec![])]), }; @@ -439,7 +439,7 @@ mod tests { let auction = super::super::Auction { prices, - surplus_capturing_jit_order_owners: vec![], + surplus_capturing_jit_order_owners: Default::default(), id: 0, orders: HashMap::from([(domain::OrderUid(hex!("c6a81144bc822569a0752c7a537fa9cbbf6344cb187ce0ff15a534b571e277eaf87da2093abee9b13a6f89671e4c3a3f80b427676799c219")), vec![domain::fee::Policy::Surplus { factor: 0.5f64.try_into().unwrap(), diff --git a/crates/autopilot/src/on_settlement_event_updater.rs b/crates/autopilot/src/on_settlement_event_updater.rs index 9ad0c001d8..ad90ff35bd 100644 --- a/crates/autopilot/src/on_settlement_event_updater.rs +++ b/crates/autopilot/src/on_settlement_event_updater.rs @@ -194,49 +194,47 @@ impl Inner { } (Ok(settlement), Some(auction_data)) => { // staging settlement properly built - let observation = settlement.observation(); - if observation.surplus.0 != auction_data.surplus { + let surplus = settlement.native_surplus(); + if surplus.0 != auction_data.surplus { tracing::warn!( ?auction_id, - ?observation.surplus, + ?surplus, ?auction_data.surplus, "automatic check error: surplus mismatch" ); } - if observation.fee.0 != auction_data.fee { + let fee = settlement.native_fee(); + if fee.0 != auction_data.fee { tracing::warn!( ?auction_id, - ?observation.fee, + ?fee, ?auction_data.fee, "automatic check error: fee mismatch" ); } - if observation.order_fees.len() != auction_data.order_executions.len() { + let order_fees = settlement.order_fees(); + if order_fees.len() != auction_data.order_executions.len() { tracing::warn!( ?auction_id, - ?observation.order_fees, + ?order_fees, ?auction_data.order_executions, "automatic check error: order_fees mismatch" ); } for fee in auction_data.order_executions { - if !observation - .order_fees - .contains_key(&domain::OrderUid(fee.0 .0)) - { + if !order_fees.contains_key(&domain::OrderUid(fee.0 .0)) { tracing::warn!( ?auction_id, ?fee, - ?observation.order_fees, + ?order_fees, "automatic check error: order_fees missing" ); } else { - let observation_fee = - observation.order_fees[&domain::OrderUid(fee.0 .0)]; - if observation_fee.unwrap_or_default().0 != fee.1 { + let settlement_fee = order_fees[&domain::OrderUid(fee.0 .0)]; + if settlement_fee.unwrap_or_default().0 != fee.1 { tracing::warn!( ?auction_id, - ?observation_fee, + ?settlement_fee, ?fee, "automatic check error: order_fees value mismatch" );