Skip to content

Commit

Permalink
Refactor driver timeouts (#2061)
Browse files Browse the repository at this point in the history
# Description
Tackles checkboxes 1 and 2 from
#2041 (comment)


![timeouts](https://github.com/cowprotocol/services/assets/19595624/0f7490e7-6c36-4278-bbcf-9a4ca0f91f1a)

# Changes
This is my attempt to make timeouts more explicit and clear.
First, we split the deadline given to driver into 3 pieces:
1. Solving time given to solvers
2. Competition time, used to validate, merge, simulate, score and rank
solutions.
3. Http delay, to cover potential slow propagating of http response back
to autopilot (`http_delay`)

The time is split in a following way: competition time and http delay
are values read from the configuration (so hardcoded), while the solving
time is whatever is left after deducting those two from the deadline
given to driver.

It's important to note that:
1. Http delay is reduced from driver deadline at the very beginning of
the solve/quote handling, because it's a non-domain information. Core
domain logic doesn't care about network delays.
2. Competition time is reduced from deadline in the domain, because how
we split given time to solving and competition is actually domain
related. For some solvers we want 19s for solving and 1s for
competition, while for some others like Baseline, we might want 10s for
solving and 10s for competition.

Default values are set to something like:
/solve: 20s given to driver, http delay is 0.5s, competition is 4.5s,
solving time 15s
/quote: 3s given to driver, http delay is 0.5s, competition time 1s,
solving time 1.5s

Release notes: check the default timeouts and if they need to be
adjusted.
  • Loading branch information
sunce86 authored Nov 21, 2023
1 parent def4f38 commit 82b01c1
Show file tree
Hide file tree
Showing 17 changed files with 224 additions and 123 deletions.
42 changes: 6 additions & 36 deletions crates/driver/src/domain/competition/auction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ use {
super::{order, Score},
crate::{
domain::{
competition::{self, auction, solution},
competition::{self, auction},
eth,
liquidity,
time,
},
infra::{self, blockchain, observe, Ethereum},
util,
Expand All @@ -30,7 +31,7 @@ pub struct Auction {
/// The tokens that are used in the orders of this auction.
tokens: Tokens,
gas_price: eth::GasPrice,
deadline: Deadline,
deadline: time::Deadline,
score_cap: Score,
}

Expand All @@ -39,7 +40,7 @@ impl Auction {
id: Option<Id>,
orders: Vec<competition::Order>,
tokens: impl Iterator<Item = Token>,
deadline: Deadline,
deadline: time::Deadline,
eth: &Ethereum,
score_cap: Score,
) -> Result<Self, Error> {
Expand Down Expand Up @@ -103,7 +104,8 @@ impl Auction {
self.gas_price
}

pub fn deadline(&self) -> Deadline {
/// The deadline for the driver to start sending solution to autopilot.
pub fn deadline(&self) -> time::Deadline {
self.deadline
}

Expand Down Expand Up @@ -397,34 +399,6 @@ impl From<eth::U256> for Price {
}
}

/// Each auction has a deadline, limiting the maximum time that can be allocated
/// to solving the auction.
#[derive(Debug, Default, Clone, Copy)]
pub struct Deadline(chrono::DateTime<chrono::Utc>);

impl Deadline {
/// Computes the timeout for solving an auction.
pub fn timeout(self) -> Result<solution::SolverTimeout, solution::DeadlineExceeded> {
solution::SolverTimeout::new(self.into(), Self::time_buffer())
}

pub fn time_buffer() -> chrono::Duration {
chrono::Duration::seconds(1)
}
}

impl From<chrono::DateTime<chrono::Utc>> for Deadline {
fn from(value: chrono::DateTime<chrono::Utc>) -> Self {
Self(value)
}
}

impl From<Deadline> for chrono::DateTime<chrono::Utc> {
fn from(value: Deadline) -> Self {
value.0
}
}

#[derive(Debug, Clone, Copy)]
pub struct Id(pub i64);

Expand Down Expand Up @@ -452,10 +426,6 @@ impl std::fmt::Display for Id {
}
}

#[derive(Debug, Error)]
#[error("the solution deadline has been exceeded")]
pub struct DeadlineExceeded;

#[derive(Debug, Error)]
#[error("invalid auction id")]
pub struct InvalidId;
Expand Down
10 changes: 5 additions & 5 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::Mempools,
super::{time, Mempools},
crate::{
domain::{competition::solution::Settlement, eth},
infra::{
Expand Down Expand Up @@ -68,7 +68,7 @@ impl Competition {
// Fetch the solutions from the solver.
let solutions = self
.solver
.solve(auction, &liquidity, auction.deadline().timeout()?)
.solve(auction, &liquidity, auction.deadline().solvers()?.into())
.await
.tap_err(|err| {
if err.is_timeout() {
Expand Down Expand Up @@ -214,7 +214,7 @@ impl Competition {
// Re-simulate the solution on every new block until the deadline ends to make
// sure we actually submit a working solution close to when the winner
// gets picked by the procotol.
if let Ok(deadline) = auction.deadline().timeout() {
if let Ok(remaining) = auction.deadline().driver() {
let score_ref = &mut score;
let simulate_on_new_blocks = async move {
let mut stream =
Expand All @@ -231,7 +231,7 @@ impl Competition {
}
}
};
let timeout = deadline.duration().to_std().unwrap_or_default();
let timeout = remaining.to_std().unwrap_or_default();
let _ = tokio::time::timeout(timeout, simulate_on_new_blocks).await;
}

Expand Down Expand Up @@ -372,7 +372,7 @@ pub enum Error {
)]
SolutionNotAvailable,
#[error("{0:?}")]
DeadlineExceeded(#[from] solution::DeadlineExceeded),
DeadlineExceeded(#[from] time::DeadlineExceeded),
#[error("solver error: {0:?}")]
Solver(#[from] solver::Error),
#[error("failed to submit the solution")]
Expand Down
31 changes: 8 additions & 23 deletions crates/driver/src/domain/competition/solution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ use {
eth::{self, TokenAddress},
},
infra::{
self,
blockchain::{self, Ethereum},
simulator,
solver::Solver,
time,
Simulator,
},
},
Expand Down Expand Up @@ -252,25 +252,8 @@ impl std::fmt::Debug for Solution {
pub struct SolverTimeout(chrono::Duration);

impl SolverTimeout {
/// Solvers are given a time limit that's `buffer` less than the specified
/// deadline. The reason for this is to allow the solver sufficient time to
/// search for the most optimal solution, but still ensure there is time
/// left for the driver to do some other necessary work and forward the
/// results back to the protocol.
pub fn new(
deadline: chrono::DateTime<chrono::Utc>,
buffer: chrono::Duration,
) -> Result<Self, DeadlineExceeded> {
let deadline = deadline - time::now() - buffer;
if deadline < chrono::Duration::zero() {
Err(DeadlineExceeded)
} else {
Ok(Self(deadline))
}
}

pub fn deadline(self) -> chrono::DateTime<chrono::Utc> {
time::now() + self.0
infra::time::now() + self.0
}

pub fn duration(self) -> chrono::Duration {
Expand All @@ -283,6 +266,12 @@ impl SolverTimeout {
}
}

impl From<chrono::Duration> for SolverTimeout {
fn from(duration: chrono::Duration) -> Self {
Self(duration)
}
}

/// Carries information how the score should be calculated.
#[derive(Debug, Clone)]
pub enum SolverScore {
Expand Down Expand Up @@ -335,10 +324,6 @@ pub enum Error {
DifferentSolvers,
}

#[derive(Debug, Error)]
#[error("the solution deadline has been exceeded")]
pub struct DeadlineExceeded;

#[derive(Debug, Error)]
#[error("invalid clearing prices")]
pub struct InvalidClearingPrices;
1 change: 1 addition & 0 deletions crates/driver/src/domain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pub mod eth;
pub mod liquidity;
pub mod mempools;
pub mod quote;
pub mod time;

pub use {
competition::Competition,
Expand Down
42 changes: 10 additions & 32 deletions crates/driver/src/domain/quote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ use {
crate::{
boundary,
domain::{
competition::{self, order, solution},
competition::{self, order},
eth,
liquidity,
time,
},
infra::{
self,
Expand Down Expand Up @@ -59,7 +60,7 @@ pub struct Order {
pub tokens: Tokens,
pub amount: order::TargetAmount,
pub side: order::Side,
pub deadline: Deadline,
pub deadline: time::Deadline,
}

impl Order {
Expand All @@ -82,9 +83,13 @@ impl Order {
}
solver::Liquidity::Skip => Default::default(),
};
let timeout = self.deadline.timeout()?;

let solutions = solver
.solve(&self.fake_auction(eth, tokens).await?, &liquidity, timeout)
.solve(
&self.fake_auction(eth, tokens).await?,
&liquidity,
self.deadline.solvers()?.into(),
)
.await?;
Quote::new(
eth,
Expand Down Expand Up @@ -209,33 +214,6 @@ impl Order {
}
}

/// The deadline for computing a quote for an order.
#[derive(Clone, Copy, Debug, Default)]
pub struct Deadline(chrono::DateTime<chrono::Utc>);

impl Deadline {
/// Computes the timeout for solving an auction.
pub fn timeout(self) -> Result<solution::SolverTimeout, solution::DeadlineExceeded> {
solution::SolverTimeout::new(self.into(), Self::time_buffer())
}

pub fn time_buffer() -> chrono::Duration {
chrono::Duration::seconds(1)
}
}

impl From<chrono::DateTime<chrono::Utc>> for Deadline {
fn from(value: chrono::DateTime<chrono::Utc>) -> Self {
Self(value)
}
}

impl From<Deadline> for chrono::DateTime<chrono::Utc> {
fn from(value: Deadline) -> Self {
value.0
}
}

/// The sell and buy tokens to quote for. This type maintains the invariant that
/// the sell and buy tokens are distinct.
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
Expand Down Expand Up @@ -270,7 +248,7 @@ pub enum Error {
#[error(transparent)]
QuotingFailed(#[from] QuotingFailed),
#[error("{0:?}")]
DeadlineExceeded(#[from] solution::DeadlineExceeded),
DeadlineExceeded(#[from] time::DeadlineExceeded),
/// Encountered an unexpected error reading blockchain data.
#[error("blockchain error: {0:?}")]
Blockchain(#[from] blockchain::Error),
Expand Down
60 changes: 60 additions & 0 deletions crates/driver/src/domain/time.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use {
crate::infra::{
solver::Timeouts,
{self},
},
thiserror::Error,
};

/// Deadlines for different parts of the driver execution.
/// The driver is expected to return the solution to the autopilot before the
/// driver deadline.
/// The solvers are expected to return the solution to the driver before the
/// solvers deadline.
#[derive(Copy, Clone, Debug, Default)]
pub struct Deadline {
driver: chrono::DateTime<chrono::Utc>,
solvers: chrono::DateTime<chrono::Utc>,
}

impl Deadline {
pub fn new(deadline: chrono::DateTime<chrono::Utc>, timeouts: Timeouts) -> Self {
let deadline = deadline - timeouts.http_delay;
Self {
driver: deadline,
solvers: {
let now = infra::time::now();
let duration = deadline - now;
now + duration * (timeouts.solving_share_of_deadline.get() * 100.0).round() as i32
/ 100
},
}
}

/// Remaining time until the deadline for driver to return solution to
/// autopilot is reached.
pub fn driver(self) -> Result<chrono::Duration, DeadlineExceeded> {
Self::remaining(self.driver)
}

/// Remaining time until the deadline for solvers to return solution to
/// driver is reached.
pub fn solvers(self) -> Result<chrono::Duration, DeadlineExceeded> {
Self::remaining(self.solvers)
}

fn remaining(
deadline: chrono::DateTime<chrono::Utc>,
) -> Result<chrono::Duration, DeadlineExceeded> {
let deadline = deadline - infra::time::now();
if deadline <= chrono::Duration::zero() {
Err(DeadlineExceeded)
} else {
Ok(deadline)
}
}
}

#[derive(Debug, Error)]
#[error("the deadline has been exceeded")]
pub struct DeadlineExceeded;
13 changes: 12 additions & 1 deletion crates/driver/src/infra/api/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
use {
crate::{
domain::{self, Mempools},
infra::{self, liquidity, solver::Solver, tokens, Ethereum, Simulator},
infra::{
self,
liquidity,
solver::{Solver, Timeouts},
tokens,
Ethereum,
Simulator,
},
},
error::Error,
futures::Future,
Expand Down Expand Up @@ -118,6 +125,10 @@ impl State {
fn pre_processor(&self) -> &domain::competition::AuctionProcessor {
&self.0.pre_processor
}

fn timeouts(&self) -> Timeouts {
self.0.solver.timeouts()
}
}

struct Inner {
Expand Down
7 changes: 4 additions & 3 deletions crates/driver/src/infra/api/routes/quote/dto/order.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use {
crate::{
domain::{competition, eth, quote},
domain::{competition, eth, quote, time},
infra::solver::Timeouts,
util::serialize,
},
serde::Deserialize,
serde_with::serde_as,
};

impl Order {
pub fn into_domain(self) -> Result<quote::Order, Error> {
pub fn into_domain(self, timeouts: Timeouts) -> Result<quote::Order, Error> {
Ok(quote::Order {
tokens: quote::Tokens::new(self.sell_token.into(), self.buy_token.into())
.map_err(|quote::SameTokens| Error::SameTokens)?,
Expand All @@ -17,7 +18,7 @@ impl Order {
Kind::Sell => competition::order::Side::Sell,
Kind::Buy => competition::order::Side::Buy,
},
deadline: self.deadline.into(),
deadline: time::Deadline::new(self.deadline, timeouts),
})
}
}
Expand Down
Loading

0 comments on commit 82b01c1

Please sign in to comment.