Skip to content

Commit

Permalink
Added auction_id field to /settle and /reveal requests (#3113)
Browse files Browse the repository at this point in the history
# Description
Added `auction_id` field to the `/settle` and `/reveal` API requests.
This feature was requested by one of the solvers team
([ref](https://cowservices.slack.com/archives/C0375NV72SC/p1730890962693039?thread_ts=1730890895.729609&cid=C0375NV72SC)).

# Changes
Updated `driver` and `autopilot` DTOs, added code to use these new
fields and added additional condition on `driver` side to verify if
provided `solution_id` is competing in provided `auction_id`.
Refactored `driver` tests to support new fields.

## How to test
Added new scenario to `driver_handles_solutions_based_on_id()` test
which validates error on wrong `auction_id` specified (success cases are
already validated by other scenarios in this test).
  • Loading branch information
mstrug authored Nov 12, 2024
1 parent 822d980 commit 5671152
Show file tree
Hide file tree
Showing 13 changed files with 103 additions and 37 deletions.
3 changes: 3 additions & 0 deletions crates/autopilot/src/infra/solvers/dto/reveal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ pub struct Request {
/// Unique ID of the solution (per driver competition), to reveal.
#[serde_as(as = "serde_with::DisplayFromStr")]
pub solution_id: u64,
/// Auction ID in which the specified solution ID is competing.
#[serde_as(as = "serde_with::DisplayFromStr")]
pub auction_id: i64,
}

#[serde_as]
Expand Down
3 changes: 3 additions & 0 deletions crates/autopilot/src/infra/solvers/dto/settle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ pub struct Request {
pub solution_id: u64,
/// The last block number in which the solution TX can be included
pub submission_deadline_latest_block: u64,
/// Auction ID in which the specified solution ID is competing.
#[serde_as(as = "serde_with::DisplayFromStr")]
pub auction_id: i64,
}

#[serde_as]
Expand Down
1 change: 1 addition & 0 deletions crates/autopilot/src/run_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,7 @@ impl RunLoop {
let request = settle::Request {
solution_id,
submission_deadline_latest_block,
auction_id,
};

// Wait for either the settlement transaction to be mined or the driver returned
Expand Down
5 changes: 4 additions & 1 deletion crates/autopilot/src/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,10 @@ impl RunLoop {
.collect();

let revealed = driver
.reveal(&reveal::Request { solution_id })
.reveal(&reveal::Request {
solution_id,
auction_id: request.id,
})
.await
.map_err(Error::Reveal)?;
if !revealed
Expand Down
7 changes: 4 additions & 3 deletions crates/driver/src/domain/competition/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,13 @@ impl Competition {
Ok(score)
}

pub async fn reveal(&self, solution_id: u64) -> Result<Revealed, Error> {
pub async fn reveal(&self, solution_id: u64, auction_id: i64) -> Result<Revealed, Error> {
let settlement = self
.settlements
.lock()
.unwrap()
.iter()
.find(|s| s.solution().get() == solution_id)
.find(|s| s.solution().get() == solution_id && s.auction_id.0 == auction_id)
.cloned()
.ok_or(Error::SolutionNotAvailable)?;
Ok(Revealed {
Expand All @@ -292,14 +292,15 @@ impl Competition {
/// [`Competition::solve`] to generate the solution.
pub async fn settle(
&self,
auction_id: i64,
solution_id: u64,
submission_deadline: u64,
) -> Result<Settled, Error> {
let settlement = {
let mut lock = self.settlements.lock().unwrap();
let index = lock
.iter()
.position(|s| s.solution().get() == solution_id)
.position(|s| s.solution().get() == solution_id && s.auction_id.0 == auction_id)
.ok_or(Error::SolutionNotAvailable)?;
// remove settlement to ensure we can't settle it twice by accident
lock.swap_remove_front(index)
Expand Down
3 changes: 3 additions & 0 deletions crates/driver/src/infra/api/routes/reveal/dto/solution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,7 @@ pub struct Solution {
/// Unique ID of the solution (per driver competition), to reveal.
#[serde_as(as = "serde_with::DisplayFromStr")]
pub solution_id: u64,
/// Auction ID in which the specified solution ID is competing.
#[serde_as(as = "serde_with::DisplayFromStr")]
pub auction_id: i64,
}
9 changes: 5 additions & 4 deletions crates/driver/src/infra/api/routes/reveal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,18 @@ async fn route(
state: axum::extract::State<State>,
req: axum::Json<dto::Solution>,
) -> Result<axum::Json<dto::Revealed>, (hyper::StatusCode, axum::Json<Error>)> {
let competition = state.competition();
let auction_id = competition.auction_id(req.solution_id).map(|id| id.0);
let handle_request = async {
observe::revealing();
let result = competition.reveal(req.solution_id).await;
let result = state
.competition()
.reveal(req.solution_id, req.auction_id)
.await;
observe::revealed(state.solver().name(), &result);
let result = result?;
Ok(axum::Json(dto::Revealed::new(result)))
};

handle_request
.instrument(tracing::info_span!("/reveal", solver = %state.solver().name(), auction_id))
.instrument(tracing::info_span!("/reveal", solver = %state.solver().name(), req.auction_id))
.await
}
3 changes: 3 additions & 0 deletions crates/driver/src/infra/api/routes/settle/dto/solution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@ pub struct Solution {
pub solution_id: u64,
/// The last block number in which the solution TX can be included
pub submission_deadline_latest_block: u64,
/// Auction ID in which this solution is competing.
#[serde_as(as = "serde_with::DisplayFromStr")]
pub auction_id: i64,
}
12 changes: 6 additions & 6 deletions crates/driver/src/infra/api/routes/settle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ async fn route(
state: axum::extract::State<State>,
req: axum::Json<dto::Solution>,
) -> Result<(), (hyper::StatusCode, axum::Json<Error>)> {
let state = state.clone();
let auction_id = state
.competition()
.auction_id(req.solution_id)
.map(|id| id.0);
let auction_id = req.auction_id;
let solver = state.solver().name().to_string();

let handle_request = async move {
observe::settling();
let result = state
.competition()
.settle(req.solution_id, req.submission_deadline_latest_block)
.settle(
req.auction_id,
req.solution_id,
req.submission_deadline_latest_block,
)
.await;
observe::settled(state.solver().name(), &result);
result.map(|_| ()).map_err(Into::into)
Expand Down
37 changes: 31 additions & 6 deletions crates/driver/src/tests/cases/parallel_auctions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,53 @@ use crate::tests::setup::{eth_order, eth_solution, setup, weth_pool};
#[ignore]
async fn driver_handles_solutions_based_on_id() {
let order = eth_order();
let test = setup()
let mut test = setup()
.pool(weth_pool())
.order(order.clone())
.solution(eth_solution())
.auction_id(1)
.done()
.await;

let id = test.solve().await.ok().id();
let solution_id = test.solve().await.ok().id();

// calling `/reveal` or `/settle` with incorrect solution ids
// results in an error.
test.settle("99").await.err().kind("SolutionNotAvailable");
test.reveal("99").await.err().kind("SolutionNotAvailable");

// calling `/reveal` or `/settle` with a reasonable id works
// but wrong auction id results in an error.
test.set_auction_id(100);
test.reveal(&solution_id)
.await
.err()
.kind("SolutionNotAvailable");
test.settle(&solution_id)
.await
.err()
.kind("SolutionNotAvailable");

// calling `/reveal` or `/settle` with a reasonable id works.
test.reveal(&id).await.ok();
test.settle(&id).await.ok().await.eth_order_executed().await;
test.set_auction_id(1);
test.reveal(&solution_id).await.ok();
test.settle(&solution_id)
.await
.ok()
.await
.eth_order_executed()
.await;

// calling `/reveal` or `/settle` with for a legit solution that
// has already been settled also fails.
test.settle(&id).await.err().kind("SolutionNotAvailable");
test.reveal(&id).await.err().kind("SolutionNotAvailable");
test.settle(&solution_id)
.await
.err()
.kind("SolutionNotAvailable");
test.reveal(&solution_id)
.await
.err()
.kind("SolutionNotAvailable");
}

/// Tests that the driver can correctly settle a solution that
Expand Down
12 changes: 9 additions & 3 deletions crates/driver/src/tests/setup/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pub fn solve_req(test: &Test) -> serde_json::Value {
}
}
json!({
"id": "1",
"id": test.auction_id.to_string(),
"tokens": tokens_json,
"orders": orders_json,
"deadline": test.deadline,
Expand All @@ -147,17 +147,23 @@ pub fn solve_req(test: &Test) -> serde_json::Value {
}

/// Create a request for the driver /reveal endpoint.
pub fn reveal_req(solution_id: &str) -> serde_json::Value {
pub fn reveal_req(solution_id: &str, auction_id: &str) -> serde_json::Value {
json!({
"solutionId": solution_id,
"auctionId": auction_id,
})
}

/// Create a request for the driver /settle endpoint.
pub fn settle_req(submission_deadline_latest_block: u64, solution_id: &str) -> serde_json::Value {
pub fn settle_req(
submission_deadline_latest_block: u64,
solution_id: &str,
auction_id: &str,
) -> serde_json::Value {
json!({
"solutionId": solution_id,
"submissionDeadlineLatestBlock": submission_deadline_latest_block,
"auctionId": auction_id,
})
}

Expand Down
43 changes: 30 additions & 13 deletions crates/driver/src/tests/setup/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,26 +493,17 @@ pub enum Mempool {
/// Create a builder for the setup process.
pub fn setup() -> Setup {
Setup {
name: Default::default(),
pools: Default::default(),
orders: Default::default(),
order_priority_strategies: Default::default(),
trusted: Default::default(),
config_file: Default::default(),
solutions: Default::default(),
quote: Default::default(),
solvers: vec![test_solver()],
enable_simulation: true,
settlement_address: Default::default(),
mempools: vec![Mempool::Public],
rpc_args: vec!["--gas-limit".into(), "10000000".into()],
jit_orders: Default::default(),
surplus_capturing_jit_order_owners: Default::default(),
allow_multiple_solve_requests: false,
auction_id: 1,
..Default::default()
}
}

#[derive(Debug)]
#[derive(Debug, Default)]
pub struct Setup {
name: Option<String>,
pools: Vec<blockchain::Pool>,
Expand All @@ -539,6 +530,8 @@ pub struct Setup {
surplus_capturing_jit_order_owners: Vec<H160>,
/// In case your test requires multiple `/solve` requests
allow_multiple_solve_requests: bool,
/// Auction ID used during tests
auction_id: i64,
}

/// The validity of a solution.
Expand Down Expand Up @@ -839,6 +832,16 @@ impl Setup {
self
}

/// Set specific auction ID which will be used during test.
/// Setting auction ID can be used to test various scenarios when handling
/// competing solutions in autopilot, and for interface tests between
/// autopilot and solvers.
/// By default auction ID is set to 1.
pub fn auction_id(mut self, auction_id: i64) -> Self {
self.auction_id = auction_id;
self
}

/// Create the test: set up onchain contracts and pools, start a mock HTTP
/// server for the solver and start the HTTP server for the driver.
pub async fn done(self) -> Test {
Expand Down Expand Up @@ -959,6 +962,7 @@ impl Setup {
quoted_orders: quotes,
quote: self.quote,
surplus_capturing_jit_order_owners,
auction_id: self.auction_id,
}
}

Expand Down Expand Up @@ -999,6 +1003,7 @@ pub struct Test {
quote: bool,
/// List of surplus capturing JIT-order owners
surplus_capturing_jit_order_owners: Vec<H160>,
auction_id: i64,
}

impl Test {
Expand Down Expand Up @@ -1035,7 +1040,10 @@ impl Test {
self.driver.addr,
solver::NAME
))
.json(&driver::reveal_req(solution_id))
.json(&driver::reveal_req(
solution_id,
&self.auction_id.to_string(),
))
.send()
.await
.unwrap();
Expand Down Expand Up @@ -1094,6 +1102,7 @@ impl Test {
.json(&driver::settle_req(
submission_deadline_latest_block,
solution_id,
&self.auction_id.to_string(),
))
.send()
.await
Expand Down Expand Up @@ -1149,6 +1158,14 @@ impl Test {
pub fn web3(&self) -> &web3::Web3<DynTransport> {
&self.blockchain.web3
}

/// Changes auction ID for current test.
/// Can be used in autopilot/solver related test cases to
/// test context changes for competing solutions.
/// Default value is set by Setup builder.
pub fn set_auction_id(&mut self, auction_id: i64) {
self.auction_id = auction_id;
}
}

/// A /solve response.
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/tests/setup/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ impl Solver {
.0
.to_string();
let expected = json!({
"id": if config.quote { None } else { Some("1") },
"id": (!config.quote).then_some("1" ),
"tokens": tokens_json,
"orders": orders_json,
"liquidity": [],
Expand Down

0 comments on commit 5671152

Please sign in to comment.