Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions crates/autopilot/src/domain/competition/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,27 @@ pub struct Solution {
/// A solution ID provided by the solver.
id: SolutionId,
solver: Address,
/// Score reported by the solver in their response.
score: Score,
orders: HashMap<domain::OrderUid, TradedOrder>,
prices: auction::Prices,
/// Score computed by the autopilot based on the solution
/// of the solver.
// TODO: refactor this to compute the score in the constructor
computed_score: Option<Score>,
score: Option<Score>,
}

impl Solution {
pub fn new(
id: SolutionId,
solver: Address,
score: Score,
orders: HashMap<domain::OrderUid, TradedOrder>,
prices: auction::Prices,
) -> Self {
Self {
id,
solver,
score,
orders,
prices,
computed_score: None,
score: None,
}
}

Expand All @@ -60,11 +56,9 @@ impl Solution {
}

pub fn score(&self) -> Score {
self.score
}

pub fn computed_score(&self) -> Option<&Score> {
self.computed_score.as_ref()
self.score.expect(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really unfortunate and makes the refactor high prio, IMHO.

"this function only gets called after the winner selection populated this value",
)
}

pub fn order_ids(&self) -> impl Iterator<Item = &domain::OrderUid> + std::fmt::Debug {
Expand Down
4 changes: 2 additions & 2 deletions crates/autopilot/src/domain/competition/participant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ impl<T> Participant<T> {
&self.solution
}

pub fn set_computed_score(&mut self, score: Score) {
self.solution.computed_score = Some(score);
pub fn set_score(&mut self, score: Score) {
self.solution.score = Some(score);
}

pub fn driver(&self) -> &Arc<infra::Driver> {
Expand Down
26 changes: 7 additions & 19 deletions crates/autopilot/src/domain/competition/winner_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl Arbitrator {
// winners before non-winners
std::cmp::Reverse(participant.is_winner()),
// high score before low score
std::cmp::Reverse(participant.solution().computed_score().cloned()),
std::cmp::Reverse(participant.solution().score()),
)
});
Ranking {
Expand All @@ -94,12 +94,7 @@ impl Arbitrator {
participants.sort_by_key(|participant| {
std::cmp::Reverse(
// we use the computed score to not trust the score provided by solvers
participant
.solution()
.computed_score()
.expect("every remaining participant has a computed score")
.get()
.0,
participant.solution().score().get().0,
)
});
let baseline_scores = compute_baseline_scores(&scores_by_solution);
Expand Down Expand Up @@ -180,7 +175,7 @@ impl Arbitrator {
let score = solutions_without_solver
.enumerate()
.filter(|(index, _)| winner_indices.contains(index))
.filter_map(|(_, solution)| solution.computed_score)
.filter_map(|(_, solution)| solution.score)
.reduce(Score::saturating_add)
.unwrap_or_default();
reference_scores.insert(solver, score);
Expand Down Expand Up @@ -267,7 +262,7 @@ fn compute_scores_by_solution(
},
score,
);
p.set_computed_score(total_score);
p.set_score(total_score);
true
}
Err(err) => {
Expand Down Expand Up @@ -471,7 +466,7 @@ mod tests {
Price,
order::{self, AppDataHash},
},
competition::{Participant, Score, Solution, TradedOrder, Unranked},
competition::{Participant, Solution, TradedOrder, Unranked},
eth::{self, TokenAddress},
},
infra::Driver,
Expand Down Expand Up @@ -1216,7 +1211,7 @@ mod tests {
// winners before non-winners
std::cmp::Reverse(a.is_winner()),
// high score before low score
std::cmp::Reverse(a.solution().computed_score().unwrap())
std::cmp::Reverse(a.solution().score())
)));
assert_eq!(winners.len(), self.expected_winners.len());
for (actual, expected) in winners.iter().zip(&self.expected_winners) {
Expand Down Expand Up @@ -1391,14 +1386,7 @@ mod tests {
let trade_order_map: HashMap<OrderUid, TradedOrder> = trades.into_iter().collect();
let solver_address = solver_address.into_alloy();

let solution = Solution::new(
solution_id,
solver_address,
// provided score does not matter as it's computed automatically by the arbitrator
Score(eth::Ether(eth::U256::ZERO)),
trade_order_map,
prices,
);
let solution = Solution::new(solution_id, solver_address, trade_order_map, prices);

let driver = Driver::try_new(
url::Url::parse("http://localhost").unwrap(),
Expand Down
3 changes: 0 additions & 3 deletions crates/autopilot/src/infra/solvers/dto/solve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ impl Solution {
Ok(domain::competition::Solution::new(
self.solution_id,
self.submission_address,
domain::competition::Score::try_new(self.score.into())?,
self.orders
.into_iter()
.map(|(o, amounts)| (o.into(), amounts.into_domain()))
Expand Down Expand Up @@ -186,8 +185,6 @@ pub struct Solution {
/// Unique ID of the solution (per driver competition), used to identify
/// it in subsequent requests (reveal, settle).
pub solution_id: u64,
#[serde_as(as = "HexOrDecimalU256")]
pub score: U256,
/// Address used by the driver to submit the settlement onchain.
pub submission_address: Address,
pub orders: HashMap<boundary::OrderUid, TradedOrder>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ pub struct Solution {
/// Unique ID of the solution (per driver competition), used to identify it
/// in subsequent requests (reveal, settle).
solution_id: u64,
submission_address: eth::Address,
#[serde_as(as = "serialize::U256")]
score: eth::U256,
submission_address: eth::Address,
#[serde_as(as = "HashMap<serialize::Hex, _>")]
orders: HashMap<OrderId, TradedOrder>,
#[serde_as(as = "HashMap<_, serialize::U256>")]
Expand Down
Loading