Skip to content

Commit

Permalink
Test settlements
Browse files Browse the repository at this point in the history
  • Loading branch information
squadgazzz committed Dec 13, 2024
1 parent c70114f commit ad7d020
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 56 deletions.
7 changes: 6 additions & 1 deletion crates/driver/src/domain/competition/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,14 @@ impl Competition {
response_sender: response_tx,
};

tracing::info!(
"newlog sending with capacity={:?}, solution_id={:?}",
self.settle_queue.capacity(),
solution_id
);
self.settle_queue.try_send(request).map_err(|err| {
tracing::error!(?err, "Failed to enqueue /settle request");
Error::SubmissionError
Error::SettlementQueueIsFull
})?;

response_rx.await.map_err(|err| {
Expand Down
4 changes: 3 additions & 1 deletion crates/driver/src/infra/api/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use {
enum Kind {
QuotingFailed,
SolverFailed,
SettlementQueueIsFull,
SolutionNotAvailable,
DeadlineExceeded,
Unknown,
Expand Down Expand Up @@ -51,6 +52,7 @@ impl From<Kind> for (hyper::StatusCode, axum::Json<Error>) {
or sell amount"
}
Kind::FailedToSubmit => "Could not submit the solution to the blockchain",
Kind::SettlementQueueIsFull => "Settlement queue is full",
};
(
hyper::StatusCode::BAD_REQUEST,
Expand Down Expand Up @@ -83,7 +85,7 @@ impl From<competition::Error> for (hyper::StatusCode, axum::Json<Error>) {
competition::Error::DeadlineExceeded(_) => Kind::DeadlineExceeded,
competition::Error::Solver(_) => Kind::SolverFailed,
competition::Error::SubmissionError => Kind::FailedToSubmit,
competition::Error::SettlementQueueIsFull => Kind::SolverFailed,
competition::Error::SettlementQueueIsFull => Kind::SettlementQueueIsFull,
};
error.into()
}
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/infra/config/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,5 +661,5 @@ fn default_max_order_age() -> Option<Duration> {
}

fn default_settle_queue_size() -> usize {
3
2
}
9 changes: 6 additions & 3 deletions crates/driver/src/tests/cases/parallel_auctions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ async fn driver_handles_solutions_based_on_id() {

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

// calling `/reveal` or `/settle` with a reasonable id works
Expand All @@ -38,7 +41,7 @@ async fn driver_handles_solutions_based_on_id() {
test.settle(&solution_id)
.await
.err()
.kind("SolutionNotAvailable");
.kind_eq("SolutionNotAvailable");

// calling `/reveal` or `/settle` with a reasonable id works.
test.set_auction_id(1);
Expand All @@ -55,7 +58,7 @@ async fn driver_handles_solutions_based_on_id() {
test.settle(&solution_id)
.await
.err()
.kind("SolutionNotAvailable");
.kind_eq("SolutionNotAvailable");
test.reveal(&solution_id)
.await
.err()
Expand Down
67 changes: 40 additions & 27 deletions crates/driver/src/tests/cases/settle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use {
},
futures::future::join_all,
itertools::Itertools,
std::{collections::HashSet, sync::Arc, time::Duration},
std::{collections::HashSet, sync::Arc},
web3::Transport,
};

Expand Down Expand Up @@ -56,7 +56,10 @@ async fn solution_not_available() {
.done()
.await;

test.settle("99").await.err().kind("SolutionNotAvailable");
test.settle("99")
.await
.err()
.kind_eq("SolutionNotAvailable");
}

/// Checks that settlements with revert risk are not submitted via public
Expand All @@ -80,7 +83,7 @@ async fn private_rpc_with_high_risk_solution() {

let id = test.solve().await.ok().id();
// Public cannot be used and private RPC is not available
test.settle(&id).await.err().kind("FailedToSubmit");
test.settle(&id).await.err().kind_eq("FailedToSubmit");
}

#[tokio::test]
Expand Down Expand Up @@ -129,14 +132,11 @@ async fn discards_excess_settle_requests() {
.pool(ab_pool())
.order(ab_order())
.solution(ab_solution())
.rpc_args(vec!["--no-mining".to_string()])
.solve_deadline_timeout(chrono::Duration::seconds(4))
.settle_submission_deadline(6)
.done()
.await,
);

test.mint_block().await.unwrap();

// MAX_SOLUTION_STORAGE = 5. Since this is hardcoded, no more solutions can be
// stored.
let solution_ids = join_all(vec![
Expand All @@ -158,6 +158,9 @@ async fn discards_excess_settle_requests() {
.len();
assert_eq!(unique_solutions_count, solution_ids.len());

// Disable auto mining to accumulate all the settlement requests.
test.disable_auto_mining().await;

// `collect_vec` is required to receive results in the same order.
let settlements = {
let test_clone = Arc::clone(&test);
Expand All @@ -173,7 +176,8 @@ async fn discards_excess_settle_requests() {
let results_fut = tokio::spawn(join_all(settlements));

tokio::time::sleep(Duration::from_secs(2)).await;
test.mint_block().await.unwrap();
// Enable auto mining to process all the settlement requests.
test.enable_auto_mining().await;

for (index, result) in results_fut.await.unwrap().into_iter().enumerate() {
match index {
Expand All @@ -184,10 +188,18 @@ async fn discards_excess_settle_requests() {
// We don't really care about the intermediate settlements. They are processed but due
// to the test framework limitation, the same solution settlements fail. We
// are fine with that to avoid huge changes in the framework.
1 | 2 => result.err().kind("FailedToSubmit"),
// Driver's settlement queue max size is 3. Next requests should be discarded.
3 => result.err().kind("DeadlineExceeded"),
4 => result.err().kind("DeadlineExceeded"),
1 => result.err().kind_eq("FailedToSubmit"),
2 => {
let kind = result.err().get_kind();
// `FailedToSubmit` can be here depending on the race condition
// where the first settlement gets dequeued before processing this settlement
// request.
assert!(kind == "FailedToSubmit" || kind == "SettlementQueueIsFull");
}
// Driver's settlement queue max size is 3. Next requests are discarded.
// todo: this condition is still flaky
3 => result.err().kind_eq("SettlementQueueIsFull"),
4 => result.err().kind_eq("SettlementQueueIsFull"),
_ => unreachable!(),
}
}
Expand All @@ -202,14 +214,11 @@ async fn accepts_new_settle_requests_after_timeout() {
.pool(ab_pool())
.order(ab_order())
.solution(ab_solution())
.rpc_args(vec!["--no-mining".to_string()])
.solve_deadline_timeout(chrono::Duration::seconds(4))
.settle_submission_deadline(6)
.done()
.await,
);

test.mint_block().await.unwrap();

// MAX_SOLUTION_STORAGE = 5. Since this is hardcoded, no more solutions can be
// stored.
let solution_ids = join_all(vec![
Expand Down Expand Up @@ -246,8 +255,8 @@ async fn accepts_new_settle_requests_after_timeout() {
};
let results_fut = tokio::spawn(join_all(first_solutions));

tokio::time::sleep(Duration::from_secs(2)).await;
test.mint_block().await.unwrap();
tokio::time::sleep(Duration::from_secs(1)).await;
test.enable_auto_mining().await;

for (index, result) in results_fut.await.unwrap().into_iter().enumerate() {
match index {
Expand All @@ -258,20 +267,24 @@ async fn accepts_new_settle_requests_after_timeout() {
// We don't really care about the intermediate settlements. They are processed but due
// to the test framework limitation, the same solution settlements fail. We
// are fine with that to avoid huge changes in the framework.
1 | 2 => result.err().kind("FailedToSubmit"),
// Driver's settlement queue max size is 3. Next requests should be discarded.
3 => result.err().kind("DeadlineExceeded"),
1 => result.err().kind_eq("FailedToSubmit"),
2 => {
let kind = result.err().get_kind();
// `FailedToSubmit` can be here depending on the race condition
// where the first settlement gets dequeued before processing this settlement
// request.
assert!(kind == "FailedToSubmit" || kind == "SettlementQueueIsFull");
}
// Driver's settlement queue max size is 3. Next requests are discarded.
3 => result.err().kind_eq("SettlementQueueIsFull"),
_ => unreachable!(),
}
}

// Wait for the timeout to expire, so all the settle requests are processed.
// Must a bit higher than `ethrpc_batch_delay`.
tokio::time::sleep(std::time::Duration::from_millis(1100)).await;

// Now we send the last settlement request.
// Now we send the last settlement request. It fails because with the current
// framework setup it is impossible to settle the same solution twice.
test.settle(&solution_ids[4])
.await
.err()
.kind("FailedToSubmit");
.kind_eq("FailedToSubmit");
}
8 changes: 4 additions & 4 deletions crates/driver/src/tests/setup/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use {
secp256k1::SecretKey,
serde_json::json,
std::collections::HashMap,
web3::{signing::Key, Error, Transport},
web3::{signing::Key, Transport},
};

// TODO Possibly might be a good idea to use an enum for tokens instead of
Expand Down Expand Up @@ -858,12 +858,12 @@ impl Blockchain {
}
}

pub async fn mint_block(&self) -> Result<(), Error> {
pub async fn set_auto_mining(&self, enabled: bool) {
self.web3
.transport()
.execute("anvil_mine", Default::default())
.execute("evm_setAutomine", vec![json!(enabled)])
.await
.map(|_| ())
.unwrap();
}
}

Expand Down
44 changes: 25 additions & 19 deletions crates/driver/src/tests/setup/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ use {
path::PathBuf,
str::FromStr,
},
web3::Error,
};

mod blockchain;
Expand Down Expand Up @@ -500,7 +499,7 @@ pub fn setup() -> Setup {
rpc_args: vec!["--gas-limit".into(), "10000000".into()],
allow_multiple_solve_requests: false,
auction_id: 1,
solve_deadline_timeout: chrono::Duration::seconds(2),
settle_submission_deadline: 3,
..Default::default()
}
}
Expand Down Expand Up @@ -534,8 +533,9 @@ pub struct Setup {
allow_multiple_solve_requests: bool,
/// Auction ID used during tests
auction_id: i64,
/// Auction solving deadline timeout
solve_deadline_timeout: chrono::Duration,
/// The maximum number of blocks to wait for a settlement to appear on
/// chain.
settle_submission_deadline: u64,
}

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

pub fn settle_submission_deadline(mut self, settle_submission_deadline: u64) -> Self {
self.settle_submission_deadline = settle_submission_deadline;
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 @@ -963,6 +968,7 @@ impl Setup {
trades: solutions.into_iter().flat_map(|s| s.trades).collect(),
trusted,
deadline,
settle_submission_deadline: self.settle_submission_deadline,
quoted_orders: quotes,
quote: self.quote,
surplus_capturing_jit_order_owners,
Expand All @@ -985,12 +991,7 @@ impl Setup {
}

fn deadline(&self) -> chrono::DateTime<chrono::Utc> {
crate::infra::time::now() + self.solve_deadline_timeout
}

pub fn solve_deadline_timeout(mut self, timeout: chrono::Duration) -> Self {
self.solve_deadline_timeout = timeout;
self
crate::infra::time::now() + chrono::Duration::seconds(2)
}

pub fn allow_multiple_solve_requests(mut self) -> Self {
Expand All @@ -1008,6 +1009,7 @@ pub struct Test {
trades: Vec<Trade>,
trusted: HashSet<&'static str>,
deadline: chrono::DateTime<chrono::Utc>,
settle_submission_deadline: u64,
/// Is this testing the /quote endpoint?
quote: bool,
/// List of surplus capturing JIT-order owners
Expand Down Expand Up @@ -1095,12 +1097,9 @@ impl Test {
}

pub async fn settle_with_solver(&self, solver_name: &str, solution_id: &str) -> Settle {
/// The maximum number of blocks to wait for a settlement to appear on
/// chain.
const SUBMISSION_DEADLINE: u64 = 3;
let submission_deadline_latest_block: u64 =
u64::try_from(self.web3().eth().block_number().await.unwrap()).unwrap()
+ SUBMISSION_DEADLINE;
+ self.settle_submission_deadline;
let old_balances = self.balances().await;
let res = self
.client
Expand Down Expand Up @@ -1175,8 +1174,12 @@ impl Test {
self.auction_id = auction_id;
}

pub async fn mint_block(&self) -> Result<(), Error> {
self.blockchain.mint_block().await
pub async fn disable_auto_mining(&self) {
self.blockchain.set_auto_mining(false).await
}

pub async fn enable_auto_mining(&self) {
self.blockchain.set_auto_mining(true).await
}
}

Expand Down Expand Up @@ -1614,13 +1617,16 @@ impl SettleOk {

impl SettleErr {
/// Check the kind field in the error response.
pub fn kind(self, expected_kind: &str) {
pub fn kind_eq(&self, expected_kind: &str) {
assert_eq!(self.get_kind(), expected_kind);
}

pub fn get_kind(&self) -> String {
let result: serde_json::Value = serde_json::from_str(&self.body).unwrap();
assert!(result.is_object());
assert_eq!(result.as_object().unwrap().len(), 2);
assert!(result.get("kind").is_some());
assert!(result.get("description").is_some());
let kind = result.get("kind").unwrap().as_str().unwrap();
assert_eq!(kind, expected_kind);
result.get("kind").unwrap().as_str().unwrap().to_string()
}
}

0 comments on commit ad7d020

Please sign in to comment.