Colocation notify - bad scores refactor and fixes (#2022)

# Description
Contains various fixes around scores validation. Currently, score
validation is messy and very complicated. I tried to summarize all
checks to only 4 of them:

1. Score > 0
2. Score <= Quality, where Quality = surplus + fees.
3. SuccessProbability in range [0,1]
4. ObjectiveValue > 0, where ObjectiveValue = Quality - GasCost

(1) and (2) are checked for all scores as per CIP20. 
(3) and (4) are checked only for `RiskAdjusted` scores (scores based on
success probability used by `ScoreCalculator`), and they represent
validation of the input to the `ScoreCalculator` which demands these
checks in order for computation to be meaningful.

# Changes
- [ ] Moved out `ObjectiveValueNonPositive` and
`SuccessProbabilityOutOfRange` so that all errors from score calculator
are now boundary errors as they should be (I don't want to import
`ScoringError` into `boundary`)
- [ ] Fixed a bug: `score > quality` instead of `score >
objective_value`
- [ ] `ScoreHigherThanObjective` removed completely since this is an
internal error in score calculator and it should never happen unless
there is a bug in the code. Anyway, this is definitely not a special
case I want to handle.
- [ ] `TooHighScore` replaced with `ScoreHigherThanQuality`

crates/driver/Cargo.toml

@@ -29,6 +29,7 @@ hyper = "0.14"
 indexmap = { version = "2", features = ["serde"] }
 itertools = "0.11"
 num = "0.4"
+number = { path = "../number" }
 prometheus = "0.13"
 prometheus-metric-storage = { git = "https://github.com/cowprotocol/prometheus-metric-storage", tag = "v0.4.0" }
 rand = "0.8"

crates/driver/src/boundary/score.rs

@@ -1,39 +1,31 @@
 use {
     crate::{
+        boundary,
         domain::{
-            competition::score::{self, SuccessProbability},
+            competition::score::{
+                self,
+                risk::{ObjectiveValue, SuccessProbability},
+            },
             eth,
         },
         util::conv::u256::U256Ext,
     },
-    score::{ObjectiveValue, Score},
-    solver::settlement_rater::{ScoreCalculator, ScoringError},
+    score::Score,
+    solver::settlement_rater::ScoreCalculator,
 };
 
 pub fn score(
     score_cap: Score,
     objective_value: ObjectiveValue,
     success_probability: SuccessProbability,
-    failure_cost: eth::Ether,
-) -> Result<Score, score::Error> {
-    match ScoreCalculator::new(score_cap.0.to_big_rational()).compute_score(
-        &objective_value.0.to_big_rational(),
-        failure_cost.0.to_big_rational(),
+    failure_cost: eth::GasCost,
+) -> Result<Score, boundary::Error> {
+    match ScoreCalculator::new(score_cap.0.get().to_big_rational()).compute_score(
+        &objective_value.0.get().to_big_rational(),
+        failure_cost.0 .0.to_big_rational(),
         success_probability.0,
     ) {
-        Ok(score) => Ok(score.into()),
-        Err(ScoringError::ObjectiveValueNonPositive(_)) => {
-            Err(score::Error::ObjectiveValueNonPositive)
-        }
-        Err(ScoringError::ScoreHigherThanObjective(score, objective_value)) => {
-            Err(score::Error::ScoreHigherThanObjective(
-                eth::U256::from_big_rational(&score)?.into(),
-                eth::U256::from_big_rational(&objective_value)?.into(),
-            ))
-        }
-        Err(ScoringError::SuccessProbabilityOutOfRange(value)) => Err(
-            score::Error::SuccessProbabilityOutOfRange(SuccessProbability(value)),
-        ),
-        Err(ScoringError::InternalError(err)) => Err(score::Error::Boundary(err)),
+        Ok(score) => Ok(score.try_into()?),
+        Err(err) => Err(err.into()),
     }
 }

crates/driver/src/boundary/settlement.rs

@@ -1,10 +1,12 @@
 use {
     crate::{
+        boundary,
         domain::{
             competition::{
                 self,
                 auction,
                 order,
+                score,
                 solution::settlement::{self, Internalization},
             },
             eth,
@@ -14,7 +16,6 @@ use {
         util::conv::u256::U256Ext,
     },
     anyhow::{anyhow, Context, Result},
-    bigdecimal::Signed,
     model::{
         app_data::AppDataHash,
         interaction::InteractionData,
@@ -161,7 +162,7 @@ impl Settlement {
             competition::SolverScore::Solver(score) => http_solver::model::Score::Solver { score },
             competition::SolverScore::RiskAdjusted(success_probability) => {
                 http_solver::model::Score::RiskAdjusted {
-                    success_probability: success_probability.0,
+                    success_probability,
                     gas_amount: None,
                 }
             }
@@ -208,12 +209,36 @@ impl Settlement {
             http_solver::model::Score::RiskAdjusted {
                 success_probability,
                 ..
-            } => competition::SolverScore::RiskAdjusted(competition::score::SuccessProbability(
-                success_probability,
-            )),
+            } => competition::SolverScore::RiskAdjusted(success_probability),
         }
     }
 
+    /// Observed quality of the settlement defined as surplus + fees.
+    pub fn quality(
+        &self,
+        eth: &Ethereum,
+        auction: &competition::Auction,
+    ) -> Result<score::Quality, boundary::Error> {
+        let prices = ExternalPrices::try_from_auction_prices(
+            eth.contracts().weth().address(),
+            auction
+                .tokens()
+                .iter()
+                .filter_map(|token| {
+                    token
+                        .price
+                        .map(|price| (token.address.into(), price.into()))
+                })
+                .collect(),
+        )?;
+
+        let surplus = self.inner.total_surplus(&prices);
+        let solver_fees = self.inner.total_solver_fees(&prices);
+        let quality = surplus + solver_fees;
+
+        Ok(eth::U256::from_big_rational(&quality)?.into())
+    }
+
     pub fn merge(self, other: Self) -> Result<Self> {
         self.inner.merge(other.inner).map(|inner| Self {
             inner,
@@ -433,58 +458,3 @@ fn to_big_decimal(value: bigdecimal::BigDecimal) -> num::BigRational {
     let numerator = num::BigRational::new(base, 1.into());
     numerator / ten.pow(exp.try_into().expect("should not overflow"))
 }
-
-pub mod objective_value {
-    use {super::*, crate::domain::competition::score};
-
-    impl Settlement {
-        pub fn objective_value(
-            &self,
-            eth: &Ethereum,
-            auction: &competition::Auction,
-            gas: eth::Gas,
-        ) -> Result<score::ObjectiveValue, Error> {
-            let prices = ExternalPrices::try_from_auction_prices(
-                eth.contracts().weth().address(),
-                auction
-                    .tokens()
-                    .iter()
-                    .filter_map(|token| {
-                        token
-                            .price
-                            .map(|price| (token.address.into(), price.into()))
-                    })
-                    .collect(),
-            )?;
-            let gas_price = eth::U256::from(auction.gas_price().effective()).to_big_rational();
-            let objective_value = {
-                let surplus = self.inner.total_surplus(&prices);
-                let solver_fees = self.inner.total_solver_fees(&prices);
-                surplus + solver_fees - gas_price * gas.0.to_big_rational()
-            };
-
-            if !objective_value.is_positive() {
-                return Err(Error::ObjectiveValueNonPositive);
-            }
-
-            Ok(eth::U256::from_big_rational(&objective_value)?.into())
-        }
-    }
-
-    #[derive(Debug, thiserror::Error)]
-    pub enum Error {
-        #[error("objective value is non-positive")]
-        ObjectiveValueNonPositive,
-        #[error("invalid objective value")]
-        Boundary(#[from] crate::boundary::Error),
-    }
-
-    impl From<Error> for competition::score::Error {
-        fn from(err: Error) -> Self {
-            match err {
-                Error::ObjectiveValueNonPositive => Self::ObjectiveValueNonPositive,
-                Error::Boundary(err) => Self::Boundary(err),
-            }
-        }
-    }
-}

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

@@ -28,7 +28,10 @@ pub mod solution;
 pub use {
     auction::Auction,
     order::Order,
-    score::{ObjectiveValue, Score, SuccessProbability},
+    score::{
+        risk::{ObjectiveValue, SuccessProbability},
+        Score,
+    },
     solution::{Solution, SolverScore, SolverTimeout},
 };