Skip to content

Commit

Permalink
Refactor score calculation (#1993)
Browse files Browse the repository at this point in the history
# Description
No new functionality (actually fixes bug, because before this we did not
check the validity of score if provided by solver as is, we did the
checks only for risk adjusted scores).

Moves out score calculation out of the boundary settlement.
A step forward for #1973
and prerequisite for implementing task #4 from
#1891

# Changes
- Added objective_value calculation on boundary settlement
- Added score forwarding on boundary settlement
- With these two, score calculation is moved out to domain::settlement
- Score calculator moved to boundary::score and used by ☝️ 
- These two allowed to define domain::score::Error and propagate errors
properly.
sunce86 authored Oct 20, 2023
1 parent 400e9de commit f63fa1b
Showing 17 changed files with 214 additions and 130 deletions.
1 change: 1 addition & 0 deletions crates/driver/src/boundary/mod.rs
Original file line number Diff line number Diff line change
@@ -25,6 +25,7 @@
pub mod liquidity;
pub mod mempool;
pub mod quote;
pub mod score;
pub mod settlement;

// The [`anyhow::Error`] type is re-exported because the legacy code mostly
38 changes: 38 additions & 0 deletions crates/driver/src/boundary/score.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
use {
crate::{
domain::{
competition::{
self,
score::{self, SuccessProbability},
},
eth,
},
util::conv::u256::U256Ext,
},
solver::settlement_rater::{ScoreCalculator, ScoringError},
};

pub fn score(
score_cap: eth::U256,
objective_value: eth::U256,
success_probability: SuccessProbability,
failure_cost: eth::U256,
) -> Result<competition::Score, score::Error> {
match ScoreCalculator::new(score_cap.to_big_rational()).compute_score(
&objective_value.to_big_rational(),
failure_cost.to_big_rational(),
success_probability.0,
) {
Ok(score) => Ok(score.into()),
Err(ScoringError::ObjectiveValueNonPositive(_)) => {
Err(score::Error::ObjectiveValueNonPositive)
}
Err(ScoringError::ScoreHigherThanObjective(_)) => {
Err(score::Error::ScoreHigherThanObjective)
}
Err(ScoringError::SuccessProbabilityOutOfRange(_)) => Err(score::Error::Boundary(
anyhow::anyhow!("unreachable, should have been checked by solvers"),
)),
Err(ScoringError::InternalError(err)) => Err(score::Error::Boundary(err)),
}
}
72 changes: 32 additions & 40 deletions crates/driver/src/boundary/settlement.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use {
crate::{
domain,
domain::{
competition::{
self,
@@ -161,7 +160,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: success_probability.0,
gas_amount: None,
}
}
@@ -202,50 +201,43 @@ impl Settlement {
}
}

pub fn score(
pub fn objective_value(
&self,
eth: &Ethereum,
auction: &competition::Auction,
gas: eth::Gas,
revert_protection: &domain::RevertProtection,
) -> Result<competition::Score> {
let score = match self.inner.score {
http_solver::model::Score::Solver { score } => score,
) -> Result<eth::U256> {
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()
};
eth::U256::from_big_rational(&objective_value)
}

pub fn score(&self) -> competition::SolverScore {
match self.inner.score {
http_solver::model::Score::Solver { score } => competition::SolverScore::Solver(score),
http_solver::model::Score::RiskAdjusted {
success_probability,
gas_amount,
} => {
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 inputs = solver::objective_value::Inputs::from_settlement(
&self.inner,
&prices,
gas_price.clone(),
&gas_amount.unwrap_or(gas.into()),
);
solver::settlement_rater::ScoreCalculator::new(
auction.score_cap().to_big_rational(),
matches!(revert_protection, domain::RevertProtection::Disabled),
)
.compute_score(
&inputs.objective_value(),
&inputs.gas_cost(),
success_probability,
)?
}
};
Ok(score.into())
..
} => competition::SolverScore::RiskAdjusted(competition::score::SuccessProbability(
success_probability,
)),
}
}

pub fn merge(self, other: Self) -> Result<Self> {
21 changes: 3 additions & 18 deletions crates/driver/src/domain/competition/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use {
self::solution::settlement,
super::{eth, Mempools},
super::Mempools,
crate::{
domain::competition::solution::Settlement,
infra::{
@@ -21,11 +21,13 @@ use {

pub mod auction;
pub mod order;
pub mod score;
pub mod solution;

pub use {
auction::Auction,
order::Order,
score::Score,
solution::{Solution, SolverScore, SolverTimeout},
};

@@ -233,23 +235,6 @@ impl Competition {
}
}

/// Represents a single value suitable for comparing/ranking solutions.
/// This is a final score that is observed by the autopilot.
#[derive(Debug, Default, PartialEq, Eq, PartialOrd, Ord, Copy, Clone)]
pub struct Score(pub eth::U256);

impl From<Score> for eth::U256 {
fn from(value: Score) -> Self {
value.0
}
}

impl From<eth::U256> for Score {
fn from(value: eth::U256) -> Self {
Self(value)
}
}

/// Solution information sent to the protocol by the driver before the solution
/// ranking happens.
#[derive(Debug)]
48 changes: 48 additions & 0 deletions crates/driver/src/domain/competition/score.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use crate::{boundary, domain::eth};

impl Score {
pub fn new(
score_cap: eth::U256,
objective_value: eth::U256,
success_probability: SuccessProbability,
failure_cost: eth::U256,
) -> Result<Self, Error> {
boundary::score::score(
score_cap,
objective_value,
success_probability,
failure_cost,
)
}
}

/// Represents a single value suitable for comparing/ranking solutions.
/// This is a final score that is observed by the autopilot.
#[derive(Debug, Default, PartialEq, Eq, PartialOrd, Ord, Copy, Clone)]
pub struct Score(pub eth::U256);

impl From<Score> for eth::U256 {
fn from(value: Score) -> Self {
value.0
}
}

impl From<eth::U256> for Score {
fn from(value: eth::U256) -> Self {
Self(value)
}
}

/// Represents the probability that a solution will be successfully settled.
#[derive(Debug, Clone)]
pub struct SuccessProbability(pub f64);

#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("objective value is non-positive")]
ObjectiveValueNonPositive,
#[error("objective value is higher than the objective")]
ScoreHigherThanObjective,
#[error("invalid objective value")]
Boundary(#[from] boundary::Error),
}
4 changes: 1 addition & 3 deletions crates/driver/src/domain/competition/solution/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use {
super::score::SuccessProbability,
crate::{
boundary,
domain::{
@@ -283,9 +284,6 @@ impl SolverTimeout {
}
}

/// Represents the probability that a solution will be successfully settled.
type SuccessProbability = f64;

/// Carries information how the score should be calculated.
#[derive(Debug, Clone)]
pub enum SolverScore {
38 changes: 34 additions & 4 deletions crates/driver/src/domain/competition/solution/settlement.rs
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ use {
crate::{
boundary,
domain::{
competition::{self, auction, order, solution},
competition::{self, auction, order, score, solution},
eth,
mempools,
},
@@ -266,9 +266,39 @@ impl Settlement {
eth: &Ethereum,
auction: &competition::Auction,
revert_protection: &mempools::RevertProtection,
) -> Result<competition::Score, boundary::Error> {
self.boundary
.score(eth, auction, self.gas.estimate, revert_protection)
) -> Result<competition::Score, score::Error> {
let objective_value = self
.boundary
.objective_value(eth, auction, self.gas.estimate)?;

let score = match self.boundary.score() {
competition::SolverScore::Solver(score) => competition::Score(score),
competition::SolverScore::RiskAdjusted(success_probability) => {
// The cost in case of a revert can deviate non-deterministically from the cost
// in case of success and it is often significantly smaller. Thus, we go with
// the full cost as a safe assumption.
let failure_cost =
matches!(revert_protection, mempools::RevertProtection::Disabled)
.then(|| self.gas.estimate.0 * auction.gas_price().effective().0 .0)
.unwrap_or(eth::U256::zero());
competition::Score::new(
auction.score_cap(),
objective_value,
success_probability,
failure_cost,
)?
}
};

if score > objective_value.into() {
return Err(score::Error::ScoreHigherThanObjective);
}

if score.0.is_zero() {
return Err(score::Error::ObjectiveValueNonPositive);
}

Ok(score)
}

// TODO(#1478): merge() should be defined on Solution rather than Settlement.
3 changes: 2 additions & 1 deletion crates/driver/src/infra/observe/mod.rs
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ use {
domain::{
competition::{
self,
score,
solution::{self, Settlement},
Auction,
Solution,
@@ -120,7 +121,7 @@ pub fn scoring(settlement: &Settlement) {
}

/// Observe that scoring failed.
pub fn scoring_failed(solver: &solver::Name, err: &boundary::Error) {
pub fn scoring_failed(solver: &solver::Name, err: &score::Error) {
tracing::info!(%solver, ?err, "discarded solution: scoring failed");
metrics::get()
.dropped_solutions
4 changes: 3 additions & 1 deletion crates/driver/src/infra/solver/dto/solution.rs
Original file line number Diff line number Diff line change
@@ -199,7 +199,9 @@ impl Solutions {
match solution.score {
Score::Solver(score) => competition::solution::SolverScore::Solver(score),
Score::RiskAdjusted(success_probability) => {
competition::solution::SolverScore::RiskAdjusted(success_probability)
competition::solution::SolverScore::RiskAdjusted(
competition::score::SuccessProbability(success_probability),
)
}
},
weth,
11 changes: 7 additions & 4 deletions crates/driver/src/tests/cases/score_competition.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Test that driver properly does competition.
use crate::tests::{
cases::{DEFAULT_SCORE_MAX, DEFAULT_SCORE_MIN},
cases::DEFAULT_SCORE_MIN,
setup::{ab_order, ab_pool, ab_solution, setup, Score},
};

@@ -11,12 +11,15 @@ async fn solver_score_winner() {
let test = setup()
.pool(ab_pool())
.order(ab_order())
.solution(ab_solution().score(Score::Solver(DEFAULT_SCORE_MAX.into())))
.solution(ab_solution().score(Score::RiskAdjusted(0.6)))
.solution(ab_solution().score(Score::Solver(2902421280589416499u128.into()))) // not higher than objective value
.solution(ab_solution().score(Score::RiskAdjusted(0.4)))
.done()
.await;

assert_eq!(test.solve().await.ok().score(), DEFAULT_SCORE_MAX.into());
assert_eq!(
test.solve().await.ok().score(),
2902421280589416499u128.into()
);
test.reveal().await.ok().orders(&[ab_order().name]);
}

12 changes: 5 additions & 7 deletions crates/solver/src/run.rs
Original file line number Diff line number Diff line change
@@ -349,13 +349,11 @@ pub async fn run(args: Arguments) {
settlement_contract: settlement_contract.clone(),
web3: web3.clone(),
code_fetcher: code_fetcher.clone(),
score_calculator: ScoreCalculator::new(
u256_to_big_rational(&args.score_cap),
args.transaction_strategy.iter().any(|s| {
matches!(s, TransactionStrategyArg::PublicMempool)
&& !args.disable_high_risk_public_mempool_transactions
}),
),
score_calculator: ScoreCalculator::new(u256_to_big_rational(&args.score_cap)),
consider_cost_failure: args.transaction_strategy.iter().any(|s| {
matches!(s, TransactionStrategyArg::PublicMempool)
&& !args.disable_high_risk_public_mempool_transactions
}),
});

let solver = crate::solver::create(
Loading

0 comments on commit f63fa1b

Please sign in to comment.