rework comments

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: `<NAME>|<URL>,<NAME>|<URL>`
+    /// A list of drivers in the following format:
+    /// `<NAME>|<URL>|<SUBMISSION_ADDRESS>|<FAIRNESS_THRESHOLD>`
     #[clap(long, env, use_value_delimiter = true)]
     pub drivers: Vec<ExternalSolver>,
 
@@ -361,6 +369,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<U256>,
+}
+
+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<Self> {
+        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
@@ -519,7 +568,7 @@ impl FromStr for CowAmmConfig {
 
 #[cfg(test)]
 mod test {
-    use super::*;
+    use {super::*, hex_literal::hex};
 
     #[test]
     fn test_fee_factor_limits() {
@@ -544,4 +593,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);
+    }
 }

crates/autopilot/src/infra/solvers/mod.rs

@@ -19,7 +19,7 @@ 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<eth::Ether>,
-    pub submission_address: Option<eth::Address>,
+    pub submission_address: eth::Address,
     client: Client,
 }
 
@@ -28,7 +28,7 @@ impl Driver {
         url: Url,
         name: String,
         fairness_threshold: Option<eth::Ether>,
-        submission_address: Option<eth::Address>,
+        submission_address: eth::Address,
     ) -> Self {
         Self {
             name,

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::<Vec<_>>(),
             native_price_estimator.clone(),
             gas_price_estimator.clone(),
         )
@@ -541,7 +546,7 @@ pub async fn run(args: Arguments) {
                     driver.url,
                     driver.name,
                     driver.fairness_threshold.map(Into::into),
-                    driver.submission_address.map(Into::into),
+                    driver.submission_address.into(),
                 ))
             })
             .collect(),
@@ -570,7 +575,7 @@ async fn shadow_mode(args: Arguments) -> ! {
                 driver.url,
                 driver.name,
                 driver.fairness_threshold.map(Into::into),
-                driver.submission_address.map(Into::into),
+                driver.submission_address.into(),
             ))
         })
         .collect();

crates/autopilot/src/run_loop.rs

@@ -721,97 +721,57 @@ impl RunLoop {
         request: &solve::Request,
     ) -> Result<Vec<Result<competition::Solution, domain::competition::SolutionError>>, SolveError>
     {
-        // @TODO: to be deleted once the submission address in the driver configuration
-        // is not optional
-        if let Some(submission_address) = driver.submission_address {
-            let authenticator = self.eth.contracts().authenticator();
-            let is_allowed = authenticator
-                .is_solver(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) => {
-                    // log warning but discard solution anyway to be on the safe side
-                    tracing::warn!(
-                        driver = driver.name,
-                        ?driver.submission_address,
-                        ?err,
-                        "failed to check if solver is deny listed"
-                    );
-                    return Err(SolveError::SolverDenyListed);
-                }
-            }
+        let authenticator = self.eth.contracts().authenticator();
+        let is_allowed = authenticator
+            .is_solver(driver.submission_address.into())
+            .call()
+            .await;
 
-            let response = tokio::time::timeout(self.config.solve_deadline, driver.solve(request))
-                .await
-                .map_err(|_| SolveError::Timeout)?
-                .map_err(SolveError::Failure)?;
-            if response.solutions.is_empty() {
-                return Err(SolveError::NoSolutions);
+        // 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);
             }
-            // Filter the responses
-            Ok(response
-                .into_domain()
-                .into_iter()
-                .filter(|solution| {
-                    solution
-                        .as_ref()
-                        .ok()
-                        .map(|solution| solution.solver() == submission_address)
-                        .unwrap_or_default()
-                })
-                .collect())
-        } else {
-            self.try_solve_legacy(driver, request).await
         }
-    }
 
-    // @TODO: Legacy try_solve, to be deleted once the submission address in the
-    // driver configuration is not optional
-    async fn try_solve_legacy(
-        &self,
-        driver: &infra::Driver,
-        request: &solve::Request,
-    ) -> Result<Vec<Result<competition::Solution, domain::competition::SolutionError>>, SolveError>
-    {
         let response = tokio::time::timeout(self.config.solve_deadline, driver.solve(request))
             .await
             .map_err(|_| SolveError::Timeout)?
             .map_err(SolveError::Failure)?;
         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)
+        // Filter the responses
+        Ok(response
+            .into_domain()
+            .into_iter()
+            .filter(|solution| {
+                solution
+                    .as_ref()
+                    .ok()
+                    .map(|solution| {
+                        let is_solution_from_driver =
+                            solution.solver() == driver.submission_address;
+                        if !is_solution_from_driver {
+                            tracing::warn!(
+                                driver = driver.name,
+                                ?driver.submission_address,
+                                "the solution is not received from the driver submission address"
+                            );
+                        }
+                        is_solution_from_driver
+                    })
+                    .unwrap_or_default()
+            })
+            .collect())
     }
 
     /// Execute the solver's solution. Returns Ok when the corresponding

crates/e2e/src/setup/services.rs

@@ -281,7 +281,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"

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![],
             )
@@ -64,7 +64,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(),
             ],

crates/e2e/tests/e2e/cow_amm.rs

@@ -155,7 +155,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(),
             ],
@@ -475,7 +478,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()

crates/e2e/tests/e2e/jit_orders.rs

@@ -81,7 +81,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(),
             ],