From 4649c2166278eceedb0013f159f1caf341a4e2b1 Mon Sep 17 00:00:00 2001 From: Mateo Date: Thu, 17 Oct 2024 12:43:05 +0200 Subject: [PATCH 01/10] Add driver submission driver to the autopilot --- crates/autopilot/src/arguments.rs | 100 +++++++++++- crates/autopilot/src/infra/solvers/mod.rs | 9 +- crates/autopilot/src/run.rs | 9 +- crates/autopilot/src/run_loop.rs | 70 ++++---- crates/e2e/src/setup/services.rs | 16 +- crates/e2e/tests/e2e/buffers.rs | 7 +- crates/e2e/tests/e2e/cow_amm.rs | 7 +- crates/e2e/tests/e2e/jit_orders.rs | 5 +- crates/e2e/tests/e2e/liquidity.rs | 5 +- crates/e2e/tests/e2e/solver_competition.rs | 153 +++++++++++++++++- crates/orderbook/src/run.rs | 14 +- crates/shared/src/arguments.rs | 38 +---- crates/shared/src/price_estimation/factory.rs | 3 +- crates/shared/src/price_estimation/mod.rs | 35 +++- 14 files changed, 381 insertions(+), 90 deletions(-) diff --git a/crates/autopilot/src/arguments.rs b/crates/autopilot/src/arguments.rs index 677237ea84..3b5369dc5b 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::{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,7 +144,8 @@ 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, @@ -366,6 +374,47 @@ impl std::fmt::Display for Arguments { } } +/// External solver driver configuration +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct ExternalSolver { + pub name: String, + pub url: Url, + pub submission_address: H160, + pub fairness_threshold: Option, +} + +impl Display for ExternalSolver { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "{}({})", self.name, self.url) + } +} + +impl FromStr for ExternalSolver { + 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_address = + H160::from_str(parts[2]).context("failed to parse submission address")?; + 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_address, + }) + } +} + /// 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 +573,7 @@ impl FromStr for CowAmmConfig { #[cfg(test)] mod test { - use super::*; + use {super::*, hex_literal::hex}; #[test] fn test_fee_factor_limits() { @@ -549,4 +598,43 @@ mod test { .contains("Factor must be in the range [0, 1)"),) } } + + #[test] + fn parse_driver() { + let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"; + let driver = ExternalSolver::from_str(argument).unwrap(); + let expected = ExternalSolver { + name: "name1".into(), + url: Url::parse("http://localhost:8080").unwrap(), + fairness_threshold: None, + submission_address: H160::from_slice(&hex!("C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")), + }; + assert_eq!(driver, expected); + } + + #[test] + fn parse_driver_with_threshold() { + let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2|1000000000000000000"; + let driver = ExternalSolver::from_str(argument).unwrap(); + let expected = ExternalSolver { + name: "name1".into(), + url: Url::parse("http://localhost:8080").unwrap(), + submission_address: H160::from_slice(&hex!("C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")), + fairness_threshold: Some(U256::exp10(18)), + }; + assert_eq!(driver, expected); + } + + #[test] + fn parse_driver_with_submission_address_and_threshold() { + let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2|1000000000000000000"; + let driver = ExternalSolver::from_str(argument).unwrap(); + let expected = ExternalSolver { + name: "name1".into(), + url: Url::parse("http://localhost:8080").unwrap(), + submission_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..de7d51bb56 100644 --- a/crates/autopilot/src/infra/solvers/mod.rs +++ b/crates/autopilot/src/infra/solvers/mod.rs @@ -19,11 +19,17 @@ 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, } impl Driver { - pub fn new(url: Url, name: String, fairness_threshold: Option) -> Self { + pub fn new( + url: Url, + name: String, + fairness_threshold: Option, + submission_address: eth::Address, + ) -> Self { Self { name, url, @@ -32,6 +38,7 @@ impl Driver { .timeout(RESPONSE_TIME_LIMIT) .build() .unwrap(), + submission_address, } } diff --git a/crates/autopilot/src/run.rs b/crates/autopilot/src/run.rs index 2ce897edc8..a5567c9dd1 100644 --- a/crates/autopilot/src/run.rs +++ b/crates/autopilot/src/run.rs @@ -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(), ) @@ -555,6 +560,7 @@ pub async fn run(args: Arguments) { driver.url, driver.name, driver.fairness_threshold.map(Into::into), + driver.submission_address.into(), )) }) .collect(), @@ -582,6 +588,7 @@ async fn shadow_mode(args: Arguments) -> ! { driver.url, driver.name, driver.fairness_threshold.map(Into::into), + driver.submission_address.into(), )) }) .collect(); diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index e86692ff4e..19fd0019e6 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -555,6 +555,24 @@ impl RunLoop { solutions.sort_unstable_by_key(|participant| { std::cmp::Reverse(participant.solution().score().get().0) }); + + // Filter out solutions that don't come from their corresponding submission + // address + let 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 is not received 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 @@ -751,6 +769,27 @@ 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; + + // Do not send the request to the driver if the solver is denied + match is_allowed { + Ok(true) => {} + Ok(false) => return Err(SolveError::SolverDenyListed), + Err(err) => { + tracing::warn!( + driver = driver.name, + ?driver.submission_address, + ?err, + "failed to check if solver is deny listed" + ); + return Err(SolveError::SolverDenyListed); + } + } + let response = tokio::time::timeout(self.config.solve_deadline, driver.solve(request)) .await .map_err(|_| SolveError::Timeout)? @@ -758,33 +797,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 @@ -908,6 +921,8 @@ enum SolveError { NoSolutions, #[error(transparent)] Failure(anyhow::Error), + #[error("the solver got deny listed")] + SolverDenyListed, } #[derive(Debug, thiserror::Error)] @@ -1000,6 +1015,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..70712ca338 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() 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/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..75b050220c 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,143 @@ 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; + 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 fairness check + 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); +} + +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()], + ) + .await, + colocation::start_baseline_solver( + "solver2".into(), + solver.clone(), + onchain.contracts().weth.address(), + vec![base_b.address()], + ) + .await, + ], + colocation::LiquidityProvider::UniswapV2, + ); + + 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; diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index 16c27567b0..1a337b1974 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 b23f23385a..095f79dd0d 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..4ad41e8947 100644 --- a/crates/shared/src/price_estimation/mod.rs +++ b/crates/shared/src/price_estimation/mod.rs @@ -2,9 +2,10 @@ use { self::trade_verifier::balance_overrides, crate::{ arguments::{display_option, display_secret_option, ExternalSolver}, + conversions::U256Ext, trade_finding::{Interaction, QuoteExecution}, }, - anyhow::Result, + anyhow::{ensure, Result}, bigdecimal::BigDecimal, ethcontract::{H160, U256}, futures::future::BoxFuture, @@ -41,6 +42,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 +597,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(), From 62ca56f5229066690ad885356b2ace3a88a011d0 Mon Sep 17 00:00:00 2001 From: Mateo Date: Fri, 18 Oct 2024 14:41:00 +0200 Subject: [PATCH 02/10] add arn to autopilot --- crates/autopilot/src/arguments.rs | 62 ++++++++++++++++++----- crates/autopilot/src/infra/solvers/mod.rs | 20 ++++++-- crates/autopilot/src/run.rs | 52 +++++++++++-------- crates/autopilot/src/run_loop.rs | 19 ++++--- 4 files changed, 106 insertions(+), 47 deletions(-) diff --git a/crates/autopilot/src/arguments.rs b/crates/autopilot/src/arguments.rs index 3b5369dc5b..d2e6b62f98 100644 --- a/crates/autopilot/src/arguments.rs +++ b/crates/autopilot/src/arguments.rs @@ -1,6 +1,6 @@ use { crate::{domain::fee::FeeFactor, infra}, - anyhow::{ensure, Context}, + anyhow::{anyhow, ensure, Context}, clap::ValueEnum, primitive_types::{H160, U256}, shared::{ @@ -379,10 +379,36 @@ impl std::fmt::Display for Arguments { pub struct ExternalSolver { pub name: String, pub url: Url, - pub submission_address: H160, + 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 ExternalSolver { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "{}({})", self.name, self.url) @@ -397,8 +423,12 @@ impl FromStr for ExternalSolver { ensure!(parts.len() >= 3, "not enough arguments for external solver"); let (name, url) = (parts[0], parts[1]); let url: Url = url.parse()?; - let submission_address = - H160::from_str(parts[2]).context("failed to parse submission address")?; + 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")?) @@ -410,7 +440,7 @@ impl FromStr for ExternalSolver { name: name.to_owned(), url, fairness_threshold, - submission_address, + submission_account, }) } } @@ -600,39 +630,45 @@ mod test { } #[test] - fn parse_driver() { + fn parse_driver_submission_account_address() { let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"; let driver = ExternalSolver::from_str(argument).unwrap(); let expected = ExternalSolver { name: "name1".into(), url: Url::parse("http://localhost:8080").unwrap(), fairness_threshold: None, - submission_address: H160::from_slice(&hex!("C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")), + submission_account: Account::Address(H160::from_slice(&hex!( + "C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2" + ))), }; assert_eq!(driver, expected); } #[test] - fn parse_driver_with_threshold() { - let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2|1000000000000000000"; + fn parse_driver_submission_account_arn() { + let argument = "name1|http://localhost:8080|arn:aws:kms:supersecretstuff"; let driver = ExternalSolver::from_str(argument).unwrap(); let expected = ExternalSolver { name: "name1".into(), url: Url::parse("http://localhost:8080").unwrap(), - submission_address: H160::from_slice(&hex!("C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")), - fairness_threshold: Some(U256::exp10(18)), + fairness_threshold: None, + submission_account: Account::Kms( + Arn::from_str("arn:aws:kms:supersecretstuff").unwrap(), + ), }; assert_eq!(driver, expected); } #[test] - fn parse_driver_with_submission_address_and_threshold() { + fn parse_driver_with_threshold() { let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2|1000000000000000000"; let driver = ExternalSolver::from_str(argument).unwrap(); let expected = ExternalSolver { name: "name1".into(), url: Url::parse("http://localhost:8080").unwrap(), - submission_address: H160::from_slice(&hex!("C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")), + 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 de7d51bb56..da6c5a4221 100644 --- a/crates/autopilot/src/infra/solvers/mod.rs +++ b/crates/autopilot/src/infra/solvers/mod.rs @@ -1,6 +1,6 @@ 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, @@ -24,12 +24,24 @@ pub struct Driver { } impl Driver { - pub fn new( + pub async fn new( url: Url, name: String, fairness_threshold: Option, - submission_address: eth::Address, + submission_account: Account, ) -> Self { + 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 + .unwrap_or_else(|_| panic!("Unable to load KMS account {:?}", key_id)); + account.public_address() + } + Account::Address(address) => address, + }; + Self { name, url, @@ -38,7 +50,7 @@ impl Driver { .timeout(RESPONSE_TIME_LIMIT) .build() .unwrap(), - submission_address, + submission_address: submission_address.into(), } } diff --git a/crates/autopilot/src/run.rs b/crates/autopilot/src/run.rs index a5567c9dd1..2ab8334cf0 100644 --- a/crates/autopilot/src/run.rs +++ b/crates/autopilot/src/run.rs @@ -548,22 +548,29 @@ 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 { + Arc::new( + infra::Driver::new( + driver.url, + driver.name, + driver.fairness_threshold.map(Into::into), + driver.submission_account, + ) + .await, + ) + }) + .collect::>(); + + let drivers = futures::future::join_all(drivers_futures).await; 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), - driver.submission_address.into(), - )) - }) - .collect(), + drivers, solvable_orders_cache, trusted_tokens, liveness.clone(), @@ -580,18 +587,23 @@ 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( - driver.url, - driver.name, - driver.fairness_threshold.map(Into::into), - driver.submission_address.into(), - )) + .map(|driver| async move { + Arc::new( + infra::Driver::new( + driver.url, + driver.name, + driver.fairness_threshold.map(Into::into), + driver.submission_account, + ) + .await, + ) }) - .collect(); + .collect::>(); + + let drivers = futures::future::join_all(drivers_futures).await; let trusted_tokens = { let web3 = shared::ethrpc::web3( diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 19fd0019e6..89e12d85aa 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -567,7 +567,7 @@ impl RunLoop { tracing::warn!( driver = participant.driver().name, ?submission_address, - "the solution is not received from the driver submission address" + "the solution received is not from the driver submission address" ); } is_solution_from_driver @@ -773,21 +773,20 @@ impl RunLoop { let is_allowed = authenticator .is_solver(driver.submission_address.into()) .call() - .await; - - // Do not send the request to the driver if the solver is denied - match is_allowed { - Ok(true) => {} - Ok(false) => return Err(SolveError::SolverDenyListed), - Err(err) => { + .await + .map_err(|err| { tracing::warn!( driver = driver.name, ?driver.submission_address, ?err, "failed to check if solver is deny listed" ); - return Err(SolveError::SolverDenyListed); - } + 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)) From ec2657d22bd65e22916e54ccda37699a446f14e5 Mon Sep 17 00:00:00 2001 From: Mateo Date: Mon, 21 Oct 2024 12:07:50 +0200 Subject: [PATCH 03/10] rework comments --- crates/autopilot/src/infra/solvers/mod.rs | 18 ++++++-- crates/autopilot/src/run.rs | 50 ++++++++++++++-------- crates/e2e/tests/e2e/solver_competition.rs | 2 +- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/crates/autopilot/src/infra/solvers/mod.rs b/crates/autopilot/src/infra/solvers/mod.rs index da6c5a4221..67c1062451 100644 --- a/crates/autopilot/src/infra/solvers/mod.rs +++ b/crates/autopilot/src/infra/solvers/mod.rs @@ -4,6 +4,7 @@ use { anyhow::{anyhow, Context, Result}, reqwest::{Client, StatusCode}, std::time::Duration, + thiserror::Error, url::Url, }; @@ -23,26 +24,35 @@ pub struct Driver { client: Client, } +#[derive(Error, Debug)] +pub enum Error { + #[error("unable to load KMS account")] + UnableToLoadKmsAccount, +} + impl Driver { pub async fn new( url: Url, name: String, fairness_threshold: Option, submission_account: Account, - ) -> Self { + ) -> 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 - .unwrap_or_else(|_| panic!("Unable to load KMS account {:?}", key_id)); + .map_err(|_| { + tracing::error!(?name, ?key_id, "Unable to load KMS account"); + Error::UnableToLoadKmsAccount + })?; account.public_address() } Account::Address(address) => address, }; - Self { + Ok(Self { name, url, fairness_threshold, @@ -51,7 +61,7 @@ impl Driver { .build() .unwrap(), 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 2ab8334cf0..8a9891eaa5 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::{ @@ -552,19 +552,25 @@ pub async fn run(args: Arguments) { .drivers .into_iter() .map(|driver| async move { - Arc::new( - infra::Driver::new( - driver.url, - driver.name, - driver.fairness_threshold.map(Into::into), - driver.submission_account, - ) - .await, + match infra::Driver::new( + driver.url, + driver.name.clone(), + driver.fairness_threshold.map(Into::into), + driver.submission_account, ) + .await + { + Ok(driver) => Some(Arc::new(driver)), + Err(_) => None, + } }) .collect::>(); - let drivers = futures::future::join_all(drivers_futures).await; + let drivers = futures::future::join_all(drivers_futures) + .await + .into_iter() + .flatten() + .collect(); let run = RunLoop::new( run_loop_config, @@ -591,19 +597,25 @@ async fn shadow_mode(args: Arguments) -> ! { .drivers .into_iter() .map(|driver| async move { - Arc::new( - infra::Driver::new( - driver.url, - driver.name, - driver.fairness_threshold.map(Into::into), - driver.submission_account, - ) - .await, + match infra::Driver::new( + driver.url, + driver.name.clone(), + driver.fairness_threshold.map(Into::into), + driver.submission_account, ) + .await + { + Ok(driver) => Some(Arc::new(driver)), + Err(_) => None, + } }) .collect::>(); - let drivers = futures::future::join_all(drivers_futures).await; + let drivers = futures::future::join_all(drivers_futures) + .await + .into_iter() + .flatten() + .collect(); let trusted_tokens = { let web3 = shared::ethrpc::web3( diff --git a/crates/e2e/tests/e2e/solver_competition.rs b/crates/e2e/tests/e2e/solver_competition.rs index 75b050220c..e26527b6dd 100644 --- a/crates/e2e/tests/e2e/solver_competition.rs +++ b/crates/e2e/tests/e2e/solver_competition.rs @@ -401,7 +401,7 @@ async fn wrong_solution_submission_address(web3: Web3) { }; wait_for_condition(TIMEOUT, indexed_trades).await.unwrap(); - // Verify that test_solver was excluded due to fairness check + // 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()) From fd86606a5ac58fb81620cd0fb806799d4be294fa Mon Sep 17 00:00:00 2001 From: Mateo Date: Fri, 25 Oct 2024 09:40:21 +0200 Subject: [PATCH 04/10] fix conflict --- crates/autopilot/src/run_loop.rs | 2 +- crates/e2e/tests/e2e/solver_competition.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 89e12d85aa..1cbaf53652 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -558,7 +558,7 @@ impl RunLoop { // Filter out solutions that don't come from their corresponding submission // address - let solutions = solutions + let mut solutions = solutions .into_iter() .filter(|participant| { let submission_address = participant.driver().submission_address; diff --git a/crates/e2e/tests/e2e/solver_competition.rs b/crates/e2e/tests/e2e/solver_competition.rs index e26527b6dd..5f3172b3db 100644 --- a/crates/e2e/tests/e2e/solver_competition.rs +++ b/crates/e2e/tests/e2e/solver_competition.rs @@ -325,6 +325,8 @@ async fn wrong_solution_submission_address(web3: Web3) { solver.clone(), onchain.contracts().weth.address(), vec![base_a.address()], + 1, + true, ) .await, colocation::start_baseline_solver( @@ -332,6 +334,8 @@ async fn wrong_solution_submission_address(web3: Web3) { solver.clone(), onchain.contracts().weth.address(), vec![base_b.address()], + 1, + true, ) .await, ], From 4deb8a5448b24f27e1af50bc5dfc91a9c5273428 Mon Sep 17 00:00:00 2001 From: Mateo Date: Mon, 11 Nov 2024 13:05:06 +0100 Subject: [PATCH 05/10] fix --- crates/autopilot/src/run_loop.rs | 2 +- crates/e2e/tests/e2e/cow_amm.rs | 5 ++++- crates/e2e/tests/e2e/limit_orders.rs | 6 +++--- crates/e2e/tests/e2e/solver_competition.rs | 3 ++- crates/shared/src/price_estimation/mod.rs | 4 ++-- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 1cbaf53652..625dc128ac 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -555,7 +555,7 @@ impl RunLoop { solutions.sort_unstable_by_key(|participant| { std::cmp::Reverse(participant.solution().score().get().0) }); - + // Filter out solutions that don't come from their corresponding submission // address let mut solutions = solutions diff --git a/crates/e2e/tests/e2e/cow_amm.rs b/crates/e2e/tests/e2e/cow_amm.rs index 70712ca338..cf32f8eb6b 100644 --- a/crates/e2e/tests/e2e/cow_amm.rs +++ b/crates/e2e/tests/e2e/cow_amm.rs @@ -746,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/limit_orders.rs b/crates/e2e/tests/e2e/limit_orders.rs index 94eb7e9e21..122c92c03e 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/solver_competition.rs b/crates/e2e/tests/e2e/solver_competition.rs index 5f3172b3db..3955449e96 100644 --- a/crates/e2e/tests/e2e/solver_competition.rs +++ b/crates/e2e/tests/e2e/solver_competition.rs @@ -79,7 +79,7 @@ async fn solver_competition(web3: Web3) { None, vec![ 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; @@ -340,6 +340,7 @@ async fn wrong_solution_submission_address(web3: Web3) { .await, ], colocation::LiquidityProvider::UniswapV2, + false, ); let services = Services::new(&onchain).await; diff --git a/crates/shared/src/price_estimation/mod.rs b/crates/shared/src/price_estimation/mod.rs index 4ad41e8947..5256740189 100644 --- a/crates/shared/src/price_estimation/mod.rs +++ b/crates/shared/src/price_estimation/mod.rs @@ -1,9 +1,9 @@ use { self::trade_verifier::balance_overrides, crate::{ + arguments, arguments::{display_option, display_secret_option, ExternalSolver}, - conversions::U256Ext, - trade_finding::{Interaction, QuoteExecution}, + trade_finding::Interaction, }, anyhow::{ensure, Result}, bigdecimal::BigDecimal, From ef08450706c6a8458cd3482fa816c23eb97af988 Mon Sep 17 00:00:00 2001 From: Mateo Date: Wed, 4 Dec 2024 14:38:55 +0100 Subject: [PATCH 06/10] rework comment --- crates/autopilot/src/arguments.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/autopilot/src/arguments.rs b/crates/autopilot/src/arguments.rs index d2e6b62f98..5d7b155cb7 100644 --- a/crates/autopilot/src/arguments.rs +++ b/crates/autopilot/src/arguments.rs @@ -147,7 +147,7 @@ pub struct Arguments { /// 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. @@ -376,7 +376,7 @@ impl std::fmt::Display for Arguments { /// External solver driver configuration #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct ExternalSolver { +pub struct Solver { pub name: String, pub url: Url, pub submission_account: Account, @@ -409,13 +409,13 @@ impl FromStr for Arn { } } -impl Display for ExternalSolver { +impl Display for Solver { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "{}({})", self.name, self.url) } } -impl FromStr for ExternalSolver { +impl FromStr for Solver { type Err = anyhow::Error; fn from_str(solver: &str) -> anyhow::Result { @@ -632,8 +632,8 @@ mod test { #[test] fn parse_driver_submission_account_address() { let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"; - let driver = ExternalSolver::from_str(argument).unwrap(); - let expected = ExternalSolver { + let driver = Solver::from_str(argument).unwrap(); + let expected = Solver { name: "name1".into(), url: Url::parse("http://localhost:8080").unwrap(), fairness_threshold: None, @@ -647,8 +647,8 @@ mod test { #[test] fn parse_driver_submission_account_arn() { let argument = "name1|http://localhost:8080|arn:aws:kms:supersecretstuff"; - let driver = ExternalSolver::from_str(argument).unwrap(); - let expected = ExternalSolver { + let driver = Solver::from_str(argument).unwrap(); + let expected = Solver { name: "name1".into(), url: Url::parse("http://localhost:8080").unwrap(), fairness_threshold: None, @@ -662,8 +662,8 @@ mod test { #[test] fn parse_driver_with_threshold() { let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2|1000000000000000000"; - let driver = ExternalSolver::from_str(argument).unwrap(); - let expected = ExternalSolver { + 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!( From 0ed42090b929504336b2ca87129b24b669825d4d Mon Sep 17 00:00:00 2001 From: Mateo Date: Wed, 11 Dec 2024 14:20:37 +0100 Subject: [PATCH 07/10] fix --- crates/shared/src/price_estimation/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/shared/src/price_estimation/mod.rs b/crates/shared/src/price_estimation/mod.rs index 5256740189..7f10c9d592 100644 --- a/crates/shared/src/price_estimation/mod.rs +++ b/crates/shared/src/price_estimation/mod.rs @@ -1,9 +1,8 @@ use { self::trade_verifier::balance_overrides, crate::{ - arguments, - arguments::{display_option, display_secret_option, ExternalSolver}, - trade_finding::Interaction, + arguments::{self, display_option, display_secret_option}, + trade_finding::{Interaction, QuoteExecution}, }, anyhow::{ensure, Result}, bigdecimal::BigDecimal, From ac72d70edb0b8ceb84211afdd9c868e87ba72bb9 Mon Sep 17 00:00:00 2001 From: Mateo Date: Thu, 12 Dec 2024 16:17:04 +0100 Subject: [PATCH 08/10] fix --- crates/autopilot/src/run.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/crates/autopilot/src/run.rs b/crates/autopilot/src/run.rs index 8a9891eaa5..c9a78a1a4b 100644 --- a/crates/autopilot/src/run.rs +++ b/crates/autopilot/src/run.rs @@ -552,17 +552,15 @@ pub async fn run(args: Arguments) { .drivers .into_iter() .map(|driver| async move { - match infra::Driver::new( + infra::Driver::new( driver.url, driver.name.clone(), driver.fairness_threshold.map(Into::into), driver.submission_account, ) .await - { - Ok(driver) => Some(Arc::new(driver)), - Err(_) => None, - } + .ok() + .map(Arc::new) }) .collect::>(); @@ -597,17 +595,15 @@ async fn shadow_mode(args: Arguments) -> ! { .drivers .into_iter() .map(|driver| async move { - match infra::Driver::new( + infra::Driver::new( driver.url, driver.name.clone(), driver.fairness_threshold.map(Into::into), driver.submission_account, ) .await - { - Ok(driver) => Some(Arc::new(driver)), - Err(_) => None, - } + .ok() + .map(Arc::new) }) .collect::>(); From 0f65cf71a26d3c58bb32ab437423dbefbdc34847 Mon Sep 17 00:00:00 2001 From: Mateo Date: Fri, 3 Jan 2025 08:44:20 +0100 Subject: [PATCH 09/10] rework comments --- crates/autopilot/src/infra/solvers/mod.rs | 6 ++++-- crates/autopilot/src/run.rs | 10 ++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/autopilot/src/infra/solvers/mod.rs b/crates/autopilot/src/infra/solvers/mod.rs index 67c1062451..0c35d42f92 100644 --- a/crates/autopilot/src/infra/solvers/mod.rs +++ b/crates/autopilot/src/infra/solvers/mod.rs @@ -28,10 +28,12 @@ pub struct Driver { pub enum Error { #[error("unable to load KMS account")] UnableToLoadKmsAccount, + #[error("failed to build client")] + FailedToBuildClient(#[source] reqwest::Error), } impl Driver { - pub async fn new( + pub async fn try_new( url: Url, name: String, fairness_threshold: Option, @@ -59,7 +61,7 @@ impl Driver { client: Client::builder() .timeout(RESPONSE_TIME_LIMIT) .build() - .unwrap(), + .map_err(Error::FailedToBuildClient)?, submission_address: submission_address.into(), }) } diff --git a/crates/autopilot/src/run.rs b/crates/autopilot/src/run.rs index fd3407d446..1ea1f6de93 100644 --- a/crates/autopilot/src/run.rs +++ b/crates/autopilot/src/run.rs @@ -552,22 +552,21 @@ pub async fn run(args: Arguments) { .drivers .into_iter() .map(|driver| async move { - infra::Driver::new( + infra::Driver::try_new( driver.url, driver.name.clone(), driver.fairness_threshold.map(Into::into), driver.submission_account, ) .await - .ok() .map(Arc::new) + .expect("failed to load solver configuration") }) .collect::>(); let drivers = futures::future::join_all(drivers_futures) .await .into_iter() - .flatten() .collect(); let run = RunLoop::new( @@ -595,22 +594,21 @@ async fn shadow_mode(args: Arguments) -> ! { .drivers .into_iter() .map(|driver| async move { - infra::Driver::new( + infra::Driver::try_new( driver.url, driver.name.clone(), driver.fairness_threshold.map(Into::into), driver.submission_account, ) .await - .ok() .map(Arc::new) + .expect("failed to load solver configuration") }) .collect::>(); let drivers = futures::future::join_all(drivers_futures) .await .into_iter() - .flatten() .collect(); let trusted_tokens = { From e7d8e2329fdcca77a5bbc8742eb8f2032bd1572c Mon Sep 17 00:00:00 2001 From: sunce86 Date: Fri, 17 Jan 2025 11:10:55 +0100 Subject: [PATCH 10/10] add log --- crates/autopilot/src/infra/solvers/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/autopilot/src/infra/solvers/mod.rs b/crates/autopilot/src/infra/solvers/mod.rs index 0c35d42f92..161918bf02 100644 --- a/crates/autopilot/src/infra/solvers/mod.rs +++ b/crates/autopilot/src/infra/solvers/mod.rs @@ -53,6 +53,13 @@ impl Driver { } Account::Address(address) => address, }; + tracing::info!( + ?name, + ?url, + ?fairness_threshold, + ?submission_address, + "Creating solver" + ); Ok(Self { name,