diff --git a/crates/autopilot/src/arguments.rs b/crates/autopilot/src/arguments.rs index 677237ea84..5d7b155cb7 100644 --- a/crates/autopilot/src/arguments.rs +++ b/crates/autopilot/src/arguments.rs @@ -1,15 +1,22 @@ use { crate::{domain::fee::FeeFactor, infra}, - anyhow::Context, + anyhow::{anyhow, ensure, Context}, clap::ValueEnum, - primitive_types::H160, + primitive_types::{H160, U256}, shared::{ - arguments::{display_list, display_option, ExternalSolver}, + arguments::{display_list, display_option}, bad_token::token_owner_finder, http_client, price_estimation::{self, NativePriceEstimators}, }, - std::{net::SocketAddr, num::NonZeroUsize, str::FromStr, time::Duration}, + std::{ + fmt, + fmt::{Display, Formatter}, + net::SocketAddr, + num::NonZeroUsize, + str::FromStr, + time::Duration, + }, url::Url, }; @@ -137,9 +144,10 @@ pub struct Arguments { )] pub trusted_tokens_update_interval: Duration, - /// A list of drivers in the following format: `|,|` + /// A list of drivers in the following format: + /// `|||` #[clap(long, env, use_value_delimiter = true)] - pub drivers: Vec, + pub drivers: Vec, /// The maximum number of blocks to wait for a settlement to appear on /// chain. @@ -366,6 +374,77 @@ impl std::fmt::Display for Arguments { } } +/// External solver driver configuration +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Solver { + pub name: String, + pub url: Url, + pub submission_account: Account, + pub fairness_threshold: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum Account { + /// AWS KMS is used to retrieve the solver public key + Kms(Arn), + /// Solver public key + Address(H160), +} + +// Wrapper type for AWS ARN identifiers +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Arn(pub String); + +impl FromStr for Arn { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + // Could be more strict here, but this should suffice to catch unintended + // configuration mistakes + if s.starts_with("arn:aws:kms:") { + Ok(Self(s.to_string())) + } else { + Err(anyhow!("Invalid ARN identifier: {}", s)) + } + } +} + +impl Display for Solver { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}({})", self.name, self.url) + } +} + +impl FromStr for Solver { + type Err = anyhow::Error; + + fn from_str(solver: &str) -> anyhow::Result { + let parts: Vec<&str> = solver.split('|').collect(); + ensure!(parts.len() >= 3, "not enough arguments for external solver"); + let (name, url) = (parts[0], parts[1]); + let url: Url = url.parse()?; + let submission_account = if let Ok(value) = Arn::from_str(parts[2]) { + Account::Kms(value) + } else { + Account::Address(H160::from_str(parts[2]).context("failed to parse submission")?) + }; + + let fairness_threshold = match parts.get(3) { + Some(value) => { + Some(U256::from_dec_str(value).context("failed to parse fairness threshold")?) + } + None => None, + }; + + Ok(Self { + name: name.to_owned(), + url, + fairness_threshold, + submission_account, + }) + } +} + /// A fee policy to be used for orders base on it's class. /// Examples: /// - Surplus with a high enough cap for limit orders: surplus:0.5:0.9:limit @@ -524,7 +603,7 @@ impl FromStr for CowAmmConfig { #[cfg(test)] mod test { - use super::*; + use {super::*, hex_literal::hex}; #[test] fn test_fee_factor_limits() { @@ -549,4 +628,49 @@ mod test { .contains("Factor must be in the range [0, 1)"),) } } + + #[test] + fn parse_driver_submission_account_address() { + let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"; + let driver = Solver::from_str(argument).unwrap(); + let expected = Solver { + name: "name1".into(), + url: Url::parse("http://localhost:8080").unwrap(), + fairness_threshold: None, + submission_account: Account::Address(H160::from_slice(&hex!( + "C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2" + ))), + }; + assert_eq!(driver, expected); + } + + #[test] + fn parse_driver_submission_account_arn() { + let argument = "name1|http://localhost:8080|arn:aws:kms:supersecretstuff"; + let driver = Solver::from_str(argument).unwrap(); + let expected = Solver { + name: "name1".into(), + url: Url::parse("http://localhost:8080").unwrap(), + fairness_threshold: None, + submission_account: Account::Kms( + Arn::from_str("arn:aws:kms:supersecretstuff").unwrap(), + ), + }; + assert_eq!(driver, expected); + } + + #[test] + fn parse_driver_with_threshold() { + let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2|1000000000000000000"; + let driver = Solver::from_str(argument).unwrap(); + let expected = Solver { + name: "name1".into(), + url: Url::parse("http://localhost:8080").unwrap(), + submission_account: Account::Address(H160::from_slice(&hex!( + "C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2" + ))), + fairness_threshold: Some(U256::exp10(18)), + }; + assert_eq!(driver, expected); + } } diff --git a/crates/autopilot/src/infra/solvers/mod.rs b/crates/autopilot/src/infra/solvers/mod.rs index 0d243805ce..161918bf02 100644 --- a/crates/autopilot/src/infra/solvers/mod.rs +++ b/crates/autopilot/src/infra/solvers/mod.rs @@ -1,9 +1,10 @@ use { self::dto::{reveal, settle, solve}, - crate::{domain::eth, util}, + crate::{arguments::Account, domain::eth, util}, anyhow::{anyhow, Context, Result}, reqwest::{Client, StatusCode}, std::time::Duration, + thiserror::Error, url::Url, }; @@ -19,20 +20,57 @@ pub struct Driver { // winning solution should be discarded if it contains at least one order, which // another driver solved with surplus exceeding this driver's surplus by `threshold` pub fairness_threshold: Option, + pub submission_address: eth::Address, client: Client, } +#[derive(Error, Debug)] +pub enum Error { + #[error("unable to load KMS account")] + UnableToLoadKmsAccount, + #[error("failed to build client")] + FailedToBuildClient(#[source] reqwest::Error), +} + impl Driver { - pub fn new(url: Url, name: String, fairness_threshold: Option) -> Self { - Self { + pub async fn try_new( + url: Url, + name: String, + fairness_threshold: Option, + submission_account: Account, + ) -> Result { + let submission_address = match submission_account { + Account::Kms(key_id) => { + let config = ethcontract::aws_config::load_from_env().await; + let account = + ethcontract::transaction::kms::Account::new((&config).into(), &key_id.0) + .await + .map_err(|_| { + tracing::error!(?name, ?key_id, "Unable to load KMS account"); + Error::UnableToLoadKmsAccount + })?; + account.public_address() + } + Account::Address(address) => address, + }; + tracing::info!( + ?name, + ?url, + ?fairness_threshold, + ?submission_address, + "Creating solver" + ); + + Ok(Self { name, url, fairness_threshold, client: Client::builder() .timeout(RESPONSE_TIME_LIMIT) .build() - .unwrap(), - } + .map_err(Error::FailedToBuildClient)?, + submission_address: submission_address.into(), + }) } pub async fn solve(&self, request: &solve::Request) -> Result { diff --git a/crates/autopilot/src/run.rs b/crates/autopilot/src/run.rs index df4f8371c3..1ea1f6de93 100644 --- a/crates/autopilot/src/run.rs +++ b/crates/autopilot/src/run.rs @@ -28,7 +28,7 @@ use { contracts::{BalancerV2Vault, IUniswapV3Factory}, ethcontract::{common::DeploymentInformation, dyns::DynWeb3, errors::DeployError, BlockNumber}, ethrpc::block_stream::block_number_to_block_number_hash, - futures::StreamExt, + futures::stream::StreamExt, model::DomainSeparator, observe::metrics::LivenessChecking, shared::{ @@ -348,7 +348,12 @@ pub async fn run(args: Arguments) { let price_estimator = price_estimator_factory .price_estimator( - &args.order_quoting.price_estimation_drivers, + &args + .order_quoting + .price_estimation_drivers + .iter() + .map(|price_estimator_driver| price_estimator_driver.clone().into()) + .collect::>(), native_price_estimator.clone(), gas_price_estimator.clone(), ) @@ -543,21 +548,32 @@ pub async fn run(args: Arguments) { max_winners_per_auction: args.max_winners_per_auction, max_solutions_per_solver: args.max_solutions_per_solver, }; + let drivers_futures = args + .drivers + .into_iter() + .map(|driver| async move { + infra::Driver::try_new( + driver.url, + driver.name.clone(), + driver.fairness_threshold.map(Into::into), + driver.submission_account, + ) + .await + .map(Arc::new) + .expect("failed to load solver configuration") + }) + .collect::>(); + + let drivers = futures::future::join_all(drivers_futures) + .await + .into_iter() + .collect(); let run = RunLoop::new( run_loop_config, eth, persistence.clone(), - args.drivers - .into_iter() - .map(|driver| { - Arc::new(infra::Driver::new( - driver.url, - driver.name, - driver.fairness_threshold.map(Into::into), - )) - }) - .collect(), + drivers, solvable_orders_cache, trusted_tokens, liveness.clone(), @@ -574,16 +590,25 @@ async fn shadow_mode(args: Arguments) -> ! { args.shadow.expect("missing shadow mode configuration"), ); - let drivers = args + let drivers_futures = args .drivers .into_iter() - .map(|driver| { - Arc::new(infra::Driver::new( + .map(|driver| async move { + infra::Driver::try_new( driver.url, - driver.name, + driver.name.clone(), driver.fairness_threshold.map(Into::into), - )) + driver.submission_account, + ) + .await + .map(Arc::new) + .expect("failed to load solver configuration") }) + .collect::>(); + + let drivers = futures::future::join_all(drivers_futures) + .await + .into_iter() .collect(); let trusted_tokens = { diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 3b55738364..76f42cd348 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -513,6 +513,24 @@ impl RunLoop { std::cmp::Reverse(participant.solution().score().get().0) }); + // Filter out solutions that don't come from their corresponding submission + // address + let mut solutions = solutions + .into_iter() + .filter(|participant| { + let submission_address = participant.driver().submission_address; + let is_solution_from_driver = participant.solution().solver() == submission_address; + if !is_solution_from_driver { + tracing::warn!( + driver = participant.driver().name, + ?submission_address, + "the solution received is not from the driver submission address" + ); + } + is_solution_from_driver + }) + .collect::>(); + // Limit the number of accepted solutions per solver. Do not alter the ordering // of solutions let mut counter = HashMap::new(); @@ -708,6 +726,26 @@ impl RunLoop { request: &solve::Request, ) -> Result>, SolveError> { + let authenticator = self.eth.contracts().authenticator(); + let is_allowed = authenticator + .is_solver(driver.submission_address.into()) + .call() + .await + .map_err(|err| { + tracing::warn!( + driver = driver.name, + ?driver.submission_address, + ?err, + "failed to check if solver is deny listed" + ); + SolveError::SolverDenyListed + })?; + + // Do not send the request to the driver if the solver is denied + if !is_allowed { + return Err(SolveError::SolverDenyListed); + } + let response = tokio::time::timeout(self.config.solve_deadline, driver.solve(request)) .await .map_err(|_| SolveError::Timeout)? @@ -715,33 +753,7 @@ impl RunLoop { if response.solutions.is_empty() { return Err(SolveError::NoSolutions); } - let solutions = response.into_domain(); - - // TODO: remove this workaround when implementing #2780 - // Discard any solutions from solvers that got deny listed in the mean time. - let futures = solutions.into_iter().map(|solution| async { - let solution = solution?; - let solver = solution.solver(); - let authenticator = self.eth.contracts().authenticator(); - let is_allowed = authenticator.is_solver(solver.into()).call().await; - - match is_allowed { - Ok(true) => Ok(solution), - Ok(false) => Err(domain::competition::SolutionError::SolverDenyListed), - Err(err) => { - // log warning but discard solution anyway to be on the safe side - tracing::warn!( - driver = driver.name, - ?solver, - ?err, - "failed to check if solver is deny listed" - ); - Err(domain::competition::SolutionError::SolverDenyListed) - } - } - }); - - Ok(futures::future::join_all(futures).await) + Ok(response.into_domain()) } /// Execute the solver's solution. Returns Ok when the corresponding @@ -865,6 +877,8 @@ enum SolveError { NoSolutions, #[error(transparent)] Failure(anyhow::Error), + #[error("the solver got deny listed")] + SolverDenyListed, } #[derive(Debug, thiserror::Error)] @@ -957,6 +971,7 @@ impl Metrics { SolveError::Timeout => "timeout", SolveError::NoSolutions => "no_solutions", SolveError::Failure(_) => "error", + SolveError::SolverDenyListed => "deny_listed", }; Self::get() .solve diff --git a/crates/e2e/src/setup/services.rs b/crates/e2e/src/setup/services.rs index 6e16bb80fa..5549d7cd1d 100644 --- a/crates/e2e/src/setup/services.rs +++ b/crates/e2e/src/setup/services.rs @@ -201,7 +201,7 @@ impl<'a> Services<'a> { vec![ colocation::start_baseline_solver( "test_solver".into(), - solver, + solver.clone(), self.contracts.weth.address(), vec![], 1, @@ -216,7 +216,10 @@ impl<'a> Services<'a> { None, [ vec![ - "--drivers=test_solver|http://localhost:11088/test_solver".to_string(), + format!( + "--drivers=test_solver|http://localhost:11088/test_solver|{}", + hex::encode(solver.address()) + ), "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver" .to_string(), ], @@ -261,7 +264,7 @@ impl<'a> Services<'a> { solvers.push( colocation::start_baseline_solver( "baseline_solver".into(), - solver, + solver.clone(), self.contracts.weth.address(), vec![], 1, @@ -273,7 +276,7 @@ impl<'a> Services<'a> { // Here we call the baseline_solver "test_quoter" to make the native price // estimation use the baseline_solver instead of the test_quoter let autopilot_args = vec![ - "--drivers=test_solver|http://localhost:11088/test_solver".to_string(), + format!("--drivers=test_solver|http://localhost:11088/test_solver|{}", hex::encode(solver.address())), "--price-estimation-drivers=test_quoter|http://localhost:11088/baseline_solver,test_solver|http://localhost:11088/test_solver".to_string(), "--native-price-estimators=test_quoter|http://localhost:11088/baseline_solver,test_solver|http://localhost:11088/test_solver".to_string(), ]; @@ -284,7 +287,10 @@ impl<'a> Services<'a> { (autopilot_args, api_args) } else { let autopilot_args = vec![ - "--drivers=test_solver|http://localhost:11088/test_solver".to_string(), + format!( + "--drivers=test_solver|http://localhost:11088/test_solver|{}", + hex::encode(solver.address()) + ), "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver" .to_string(), "--native-price-estimators=test_quoter|http://localhost:11088/test_solver" diff --git a/crates/e2e/tests/e2e/buffers.rs b/crates/e2e/tests/e2e/buffers.rs index 398b13a547..f33b31c614 100644 --- a/crates/e2e/tests/e2e/buffers.rs +++ b/crates/e2e/tests/e2e/buffers.rs @@ -45,7 +45,7 @@ async fn onchain_settlement_without_liquidity(web3: Web3) { vec![ colocation::start_baseline_solver( "test_solver".into(), - solver, + solver.clone(), onchain.contracts().weth.address(), vec![], 1, @@ -67,7 +67,10 @@ async fn onchain_settlement_without_liquidity(web3: Web3) { token_a = token_a.address(), token_b = token_b.address() ), - "--drivers=test_solver|http://localhost:11088/test_solver".to_string(), + format!( + "--drivers=test_solver|http://localhost:11088/test_solver|{}", + hex::encode(solver.address()) + ), "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver" .to_string(), ], diff --git a/crates/e2e/tests/e2e/cow_amm.rs b/crates/e2e/tests/e2e/cow_amm.rs index 8c419bcf1c..cf32f8eb6b 100644 --- a/crates/e2e/tests/e2e/cow_amm.rs +++ b/crates/e2e/tests/e2e/cow_amm.rs @@ -160,7 +160,10 @@ async fn cow_amm_jit(web3: Web3) { .start_autopilot( None, vec![ - "--drivers=mock_solver|http://localhost:11088/mock_solver".to_string(), + format!( + "--drivers=mock_solver|http://localhost:11088/mock_solver|{}", + hex::encode(solver.address()) + ), "--price-estimation-drivers=test_solver|http://localhost:11088/test_solver" .to_string(), ], @@ -484,7 +487,7 @@ async fn cow_amm_driver_support(web3: Web3) { .start_autopilot( None, vec![ - "--drivers=test_solver|http://localhost:11088/test_solver,mock_solver|http://localhost:11088/mock_solver".to_string(), + format!("--drivers=test_solver|http://localhost:11088/test_solver|{},mock_solver|http://localhost:11088/mock_solver|{}", hex::encode(solver.address()), hex::encode(solver.address())), "--price-estimation-drivers=test_solver|http://localhost:11088/test_solver" .to_string(), "--cow-amm-configs=0x3705ceee5eaa561e3157cf92641ce28c45a3999c|0x3705ceee5eaa561e3157cf92641ce28c45a3999c|20332744".to_string() @@ -743,7 +746,10 @@ async fn cow_amm_opposite_direction(web3: Web3) { .start_autopilot( None, vec![ - "--drivers=mock_solver|http://localhost:11088/mock_solver".to_string(), + format!( + "--drivers=mock_solver|http://localhost:11088/mock_solver|{}", + hex::encode(solver.address()) + ), "--price-estimation-drivers=mock_solver|http://localhost:11088/mock_solver" .to_string(), ], diff --git a/crates/e2e/tests/e2e/jit_orders.rs b/crates/e2e/tests/e2e/jit_orders.rs index a37514e894..556d58abb0 100644 --- a/crates/e2e/tests/e2e/jit_orders.rs +++ b/crates/e2e/tests/e2e/jit_orders.rs @@ -85,7 +85,10 @@ async fn single_limit_order_test(web3: Web3) { .start_autopilot( None, vec![ - "--drivers=mock_solver|http://localhost:11088/mock_solver".to_string(), + format!( + "--drivers=mock_solver|http://localhost:11088/mock_solver|{}", + hex::encode(solver.address()) + ), "--price-estimation-drivers=test_solver|http://localhost:11088/test_solver" .to_string(), ], diff --git a/crates/e2e/tests/e2e/limit_orders.rs b/crates/e2e/tests/e2e/limit_orders.rs index 4f7ed21714..ff7e636dec 100644 --- a/crates/e2e/tests/e2e/limit_orders.rs +++ b/crates/e2e/tests/e2e/limit_orders.rs @@ -349,7 +349,7 @@ async fn two_limit_orders_multiple_winners_test(web3: Web3) { .await, colocation::start_baseline_solver( "solver2".into(), - solver_b, + solver_b.clone(), onchain.contracts().weth.address(), vec![base_b.address()], 2, @@ -405,8 +405,8 @@ async fn two_limit_orders_multiple_winners_test(web3: Web3) { services.start_autopilot( None, vec![ - "--drivers=solver1|http://localhost:11088/test_solver|10000000000000000,solver2|http://localhost:11088/solver2" - .to_string(), + format!("--drivers=solver1|http://localhost:11088/test_solver|{}|10000000000000000,solver2|http://localhost:11088/solver2|{}", + hex::encode(solver_a.address()), hex::encode(solver_b.address())), "--price-estimation-drivers=solver1|http://localhost:11088/test_solver".to_string(), "--max-winners-per-auction=2".to_string(), ], diff --git a/crates/e2e/tests/e2e/liquidity.rs b/crates/e2e/tests/e2e/liquidity.rs index 1d10ca104a..f204075502 100644 --- a/crates/e2e/tests/e2e/liquidity.rs +++ b/crates/e2e/tests/e2e/liquidity.rs @@ -157,7 +157,10 @@ async fn zero_ex_liquidity(web3: Web3) { vec![ "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver" .to_string(), - "--drivers=test_solver|http://localhost:11088/test_solver".to_string(), + format!( + "--drivers=test_solver|http://localhost:11088/test_solver|{}", + hex::encode(solver.address()) + ), ], ) .await; diff --git a/crates/e2e/tests/e2e/solver_competition.rs b/crates/e2e/tests/e2e/solver_competition.rs index 8ed1936923..3955449e96 100644 --- a/crates/e2e/tests/e2e/solver_competition.rs +++ b/crates/e2e/tests/e2e/solver_competition.rs @@ -22,6 +22,12 @@ async fn local_node_fairness_check() { run_test(fairness_check).await; } +#[tokio::test] +#[ignore] +async fn local_node_wrong_solution_submission_address() { + run_test(wrong_solution_submission_address).await; +} + async fn solver_competition(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3).await; @@ -56,7 +62,7 @@ async fn solver_competition(web3: Web3) { .await, colocation::start_baseline_solver( "solver2".into(), - solver, + solver.clone(), onchain.contracts().weth.address(), vec![], 1, @@ -72,8 +78,8 @@ async fn solver_competition(web3: Web3) { services.start_autopilot( None, vec![ - "--drivers=test_solver|http://localhost:11088/test_solver,solver2|http://localhost:11088/solver2" - .to_string(), + format!("--drivers=test_solver|http://localhost:11088/test_solver|{},solver2|http://localhost:11088/solver2|{}", hex::encode(solver.address()), hex::encode(solver.address()) + ), "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver,solver2|http://localhost:11088/solver2".to_string(), ], ).await; @@ -184,7 +190,7 @@ async fn fairness_check(web3: Web3) { .await, colocation::start_baseline_solver( "solver2".into(), - solver, + solver.clone(), onchain.contracts().weth.address(), vec![base_b.address()], 1, @@ -201,8 +207,7 @@ async fn fairness_check(web3: Web3) { None, // Solver 1 has a fairness threshold of 0.01 ETH, which should be triggered by sub-optimally settling order_b vec![ - "--drivers=solver1|http://localhost:11088/test_solver|10000000000000000,solver2|http://localhost:11088/solver2" - .to_string(), + format!("--drivers=solver1|http://localhost:11088/test_solver|{}|10000000000000000,solver2|http://localhost:11088/solver2|{}", hex::encode(solver.address()), hex::encode(solver.address())), "--price-estimation-drivers=solver1|http://localhost:11088/test_solver".to_string(), ], ).await; @@ -273,3 +278,144 @@ async fn fairness_check(web3: Web3) { ); assert_eq!(competition.common.solutions.len(), 1); } + +async fn wrong_solution_submission_address(web3: Web3) { + let mut onchain = OnchainComponents::deploy(web3).await; + + let [solver] = onchain.make_solvers(to_wei(1)).await; + let [trader_a, trader_b] = onchain.make_accounts(to_wei(1)).await; + let [token_a, token_b] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) + .await; + + // Fund traders + token_a.mint(trader_a.address(), to_wei(10)).await; + token_b.mint(trader_b.address(), to_wei(10)).await; + + // Create more liquid routes between token_a (token_b) and weth via base_a + // (base_b). base_a has more liquidity then base_b, leading to the solver that + // knows about base_a to win + let [base_a, base_b] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(10_000), to_wei(10_000)) + .await; + onchain + .seed_uni_v2_pool((&token_a, to_wei(100_000)), (&base_a, to_wei(100_000))) + .await; + onchain + .seed_uni_v2_pool((&token_b, to_wei(10_000)), (&base_b, to_wei(10_000))) + .await; + + // Approve GPv2 for trading + tx!( + trader_a.account(), + token_a.approve(onchain.contracts().allowance, to_wei(100)) + ); + tx!( + trader_b.account(), + token_b.approve(onchain.contracts().allowance, to_wei(100)) + ); + + // Start system, with two solvers, one that knows about base_a and one that + // knows about base_b + colocation::start_driver( + onchain.contracts(), + vec![ + colocation::start_baseline_solver( + "test_solver".into(), + solver.clone(), + onchain.contracts().weth.address(), + vec![base_a.address()], + 1, + true, + ) + .await, + colocation::start_baseline_solver( + "solver2".into(), + solver.clone(), + onchain.contracts().weth.address(), + vec![base_b.address()], + 1, + true, + ) + .await, + ], + colocation::LiquidityProvider::UniswapV2, + false, + ); + + let services = Services::new(&onchain).await; + services.start_autopilot( + None, + // Solver 1 has a wrong submission address, meaning that the solutions should be discarded from solver1 + vec![ + format!("--drivers=solver1|http://localhost:11088/test_solver|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2,solver2|http://localhost:11088/solver2|{}", hex::encode(solver.address())), + "--price-estimation-drivers=solver1|http://localhost:11088/test_solver".to_string(), + ], + ).await; + services + .start_api(vec![ + "--price-estimation-drivers=solver1|http://localhost:11088/test_solver".to_string(), + ]) + .await; + + // Place Orders + let order_a = OrderCreation { + sell_token: token_a.address(), + sell_amount: to_wei(10), + buy_token: onchain.contracts().weth.address(), + buy_amount: to_wei(5), + valid_to: model::time::now_in_epoch_seconds() + 300, + kind: OrderKind::Sell, + ..Default::default() + } + .sign( + EcdsaSigningScheme::Eip712, + &onchain.contracts().domain_separator, + SecretKeyRef::from(&SecretKey::from_slice(trader_a.private_key()).unwrap()), + ); + let uid_a = services.create_order(&order_a).await.unwrap(); + + onchain.mint_block().await; + + let order_b = OrderCreation { + sell_token: token_b.address(), + sell_amount: to_wei(10), + buy_token: onchain.contracts().weth.address(), + buy_amount: to_wei(5), + valid_to: model::time::now_in_epoch_seconds() + 300, + kind: OrderKind::Sell, + ..Default::default() + } + .sign( + EcdsaSigningScheme::Eip712, + &onchain.contracts().domain_separator, + SecretKeyRef::from(&SecretKey::from_slice(trader_b.private_key()).unwrap()), + ); + services.create_order(&order_b).await.unwrap(); + + // Wait for trade + let indexed_trades = || async { + onchain.mint_block().await; + match services.get_trades(&uid_a).await.unwrap().first() { + Some(trade) => services + .get_solver_competition(trade.tx_hash.unwrap()) + .await + .is_ok(), + None => false, + } + }; + wait_for_condition(TIMEOUT, indexed_trades).await.unwrap(); + + // Verify that test_solver was excluded due to wrong driver address + let trades = services.get_trades(&uid_a).await.unwrap(); + let competition = services + .get_solver_competition(trades[0].tx_hash.unwrap()) + .await + .unwrap(); + tracing::info!(?competition, "competition"); + assert_eq!( + competition.common.solutions.last().unwrap().solver, + "solver2" + ); + assert_eq!(competition.common.solutions.len(), 1); +} diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index e1824713db..af71c20653 100644 --- a/crates/orderbook/src/run.rs +++ b/crates/orderbook/src/run.rs @@ -289,14 +289,24 @@ pub async fn run(args: Arguments) { let price_estimator = price_estimator_factory .price_estimator( - &args.order_quoting.price_estimation_drivers, + &args + .order_quoting + .price_estimation_drivers + .iter() + .map(|price_estimator| price_estimator.clone().into()) + .collect::>(), native_price_estimator.clone(), gas_price_estimator.clone(), ) .unwrap(); let fast_price_estimator = price_estimator_factory .fast_price_estimator( - &args.order_quoting.price_estimation_drivers, + &args + .order_quoting + .price_estimation_drivers + .iter() + .map(|price_estimator| price_estimator.clone().into()) + .collect::>(), args.fast_price_estimation_results_required, native_price_estimator.clone(), gas_price_estimator.clone(), diff --git a/crates/shared/src/arguments.rs b/crates/shared/src/arguments.rs index 70d218e108..2a3613a194 100644 --- a/crates/shared/src/arguments.rs +++ b/crates/shared/src/arguments.rs @@ -55,7 +55,6 @@ macro_rules! logging_args_with_default_filter { pub struct ExternalSolver { pub name: String, pub url: Url, - pub fairness_threshold: Option, } // The following arguments are used to configure the order creation process @@ -472,19 +471,16 @@ impl FromStr for ExternalSolver { fn from_str(solver: &str) -> Result { let parts: Vec<&str> = solver.split('|').collect(); - ensure!(parts.len() >= 2, "not enough arguments for external solver"); + ensure!( + parts.len() == 2, + "wrong number of arguments for external solver" + ); let (name, url) = (parts[0], parts[1]); let url: Url = url.parse()?; - let fairness_threshold = match parts.get(2) { - Some(value) => { - Some(U256::from_dec_str(value).context("failed to parse fairness threshold")?) - } - None => None, - }; + Ok(Self { name: name.to_owned(), url, - fairness_threshold, }) } } @@ -493,30 +489,6 @@ impl FromStr for ExternalSolver { mod test { use super::*; - #[test] - fn parse_driver() { - let argument = "name1|http://localhost:8080"; - let driver = ExternalSolver::from_str(argument).unwrap(); - let expected = ExternalSolver { - name: "name1".into(), - url: Url::parse("http://localhost:8080").unwrap(), - fairness_threshold: None, - }; - assert_eq!(driver, expected); - } - - #[test] - fn parse_driver_with_threshold() { - let argument = "name1|http://localhost:8080|1000000000000000000"; - let driver = ExternalSolver::from_str(argument).unwrap(); - let expected = ExternalSolver { - name: "name1".into(), - url: Url::parse("http://localhost:8080").unwrap(), - fairness_threshold: Some(U256::exp10(18)), - }; - assert_eq!(driver, expected); - } - #[test] fn parse_drivers_wrong_arguments() { // too few arguments diff --git a/crates/shared/src/price_estimation/factory.rs b/crates/shared/src/price_estimation/factory.rs index 941b0505f2..c07a287fa0 100644 --- a/crates/shared/src/price_estimation/factory.rs +++ b/crates/shared/src/price_estimation/factory.rs @@ -12,7 +12,7 @@ use { PriceEstimating, }, crate::{ - arguments::{self, ExternalSolver}, + arguments, bad_token::BadTokenDetecting, baseline_solver::BaseTokens, code_fetching::CachedCodeFetcher, @@ -23,6 +23,7 @@ use { buffered::{self, BufferedRequest, NativePriceBatchFetching}, competition::PriceRanking, native::NativePriceEstimating, + ExternalSolver, }, token_info::TokenInfoFetching, }, diff --git a/crates/shared/src/price_estimation/mod.rs b/crates/shared/src/price_estimation/mod.rs index c077f83906..7f10c9d592 100644 --- a/crates/shared/src/price_estimation/mod.rs +++ b/crates/shared/src/price_estimation/mod.rs @@ -1,10 +1,10 @@ use { self::trade_verifier::balance_overrides, crate::{ - arguments::{display_option, display_secret_option, ExternalSolver}, + arguments::{self, display_option, display_secret_option}, trade_finding::{Interaction, QuoteExecution}, }, - anyhow::Result, + anyhow::{ensure, Result}, bigdecimal::BigDecimal, ethcontract::{H160, U256}, futures::future::BoxFuture, @@ -41,6 +41,36 @@ pub mod trade_verifier; #[derive(Clone, Debug)] pub struct NativePriceEstimators(Vec>); +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct ExternalSolver { + pub name: String, + pub url: Url, +} + +impl From for ExternalSolver { + fn from(value: arguments::ExternalSolver) -> Self { + Self { + name: value.name, + url: value.url, + } + } +} + +impl FromStr for ExternalSolver { + type Err = anyhow::Error; + + fn from_str(solver: &str) -> Result { + let parts: Vec<&str> = solver.split('|').collect(); + ensure!(parts.len() >= 2, "not enough arguments for external solver"); + let (name, url) = (parts[0], parts[1]); + let url: Url = url.parse()?; + Ok(Self { + name: name.to_owned(), + url, + }) + } +} + #[derive(Clone, Debug, Hash, Eq, PartialEq)] pub enum NativePriceEstimator { Driver(ExternalSolver), @@ -566,7 +596,7 @@ mod tests { for repr in [ &NativePriceEstimator::Driver( - ExternalSolver::from_str("baseline|http://localhost:1234/").unwrap(), + ExternalSolver::from_str("baseline|http://localhost:1234/|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2").unwrap(), ) .to_string(), &NativePriceEstimator::OneInchSpotPriceApi.to_string(),