Separate driver list for naitive price estimates (#2698)

# Description
Pass native price estimator drivers as a separate list. This allows us
to configure native price estimators which should not be used as a
regular price estimator. This is useful for 2 reasons:
1. sometimes a solver gives bad quotes for regular price estimates but
could still be used for native price estimates. This can not be done
without this PR.
2. specifically for the zeroex we need to use a different solver setup
for regular quotes as for native price estimates because they should use
different API keys for accounting purposes.

# Changes
Any solver that should be used in the `--native-price-estimators`
argument must also be passed in `--native-price-estimation-drivers` or
`--price-estimation-legacy-solvers`.
Unfortunately this is still quite weird to configure but at least
decouples regular quoting from native price estimation more.

## How to test
Adjusted e2e tests

crates/autopilot/src/arguments.rs

@@ -90,7 +90,7 @@ pub struct Arguments {
     /// native token. Estimators with the same name need to also be specified as
     /// built-in, legacy or external price estimators (lookup happens in this
     /// order in case of name collisions)
-    #[clap(long, env, default_value_t)]
+    #[clap(long, env)]
     pub native_price_estimators: NativePriceEstimators,
 
     /// How many successful price estimates for each order will cause a native

crates/autopilot/src/run.rs

@@ -425,10 +425,6 @@ pub async fn run(args: Arguments) {
     let native_price_estimator = price_estimator_factory
         .native_price_estimator(
             args.native_price_estimators.as_slice(),
-            &PriceEstimatorSource::for_args(
-                &args.order_quoting.price_estimation_drivers,
-                &args.order_quoting.price_estimation_legacy_solvers,
-            ),
             args.native_price_estimation_results_required,
         )
         .unwrap();

crates/e2e/src/setup/services.rs

@@ -94,7 +94,7 @@ impl<'a> Services<'a> {
 
     fn api_autopilot_arguments() -> impl Iterator<Item = String> {
         [
-            "--native-price-estimators=test_quoter".to_string(),
+            "--native-price-estimators=test_quoter|http://localhost:11088/test_solver".to_string(),
             "--amount-to-estimate-prices-with=1000000000000000000".to_string(),
             "--block-stream-poll-interval=1s".to_string(),
             "--simulation-node-url=http://localhost:8545".to_string(),
@@ -243,21 +243,27 @@ impl<'a> Services<'a> {
             let autopilot_args = vec![
                 "--drivers=test_solver|http://localhost:11088/test_solver".to_string(),
                 "--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(),
             ];
             let api_args = vec![
                 "--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(),
             ];
             (autopilot_args, api_args)
         } else {
             let autopilot_args = vec![
                 "--drivers=test_solver|http://localhost:11088/test_solver".to_string(),
                 "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver"
                     .to_string(),
+                "--native-price-estimators=test_quoter|http://localhost:11088/test_solver"
+                    .to_string(),
             ];
 
             let api_args = vec![
                 "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver"
                     .to_string(),
+                "--native-price-estimators=test_quoter|http://localhost:11088/test_solver"
+                    .to_string(),
             ];
             (autopilot_args, api_args)
         };

crates/e2e/tests/e2e/quoting.rs

@@ -215,7 +215,9 @@ async fn uses_stale_liquidity(web3: Web3) {
     wait_for_condition(TIMEOUT, || async {
         // Mint blocks until we evict the cached liquidty and fetch the new state.
         onchain.mint_block().await;
-        let next = services.submit_quote(&quote).await.unwrap();
+        let Ok(next) = services.submit_quote(&quote).await else {
+            return false;
+        };
         next.quote.buy_amount != first.quote.buy_amount
     })
     .await

crates/orderbook/src/arguments.rs

@@ -90,7 +90,7 @@ pub struct Arguments {
 
     /// Which estimators to use to estimate token prices in terms of the chain's
     /// native token.
-    #[clap(long, env, default_value_t)]
+    #[clap(long, env)]
     pub native_price_estimators: NativePriceEstimators,
 
     /// How many successful price estimates for each order will cause a fast

crates/orderbook/src/run.rs

@@ -380,10 +380,6 @@ pub async fn run(args: Arguments) {
     let native_price_estimator = price_estimator_factory
         .native_price_estimator(
             args.native_price_estimators.as_slice(),
-            &PriceEstimatorSource::for_args(
-                &args.order_quoting.price_estimation_drivers,
-                &args.order_quoting.price_estimation_legacy_solvers,
-            ),
             args.fast_price_estimation_results_required,
         )
         .unwrap();

crates/shared/src/arguments.rs

@@ -51,7 +51,7 @@ macro_rules! logging_args_with_default_filter {
     };
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct ExternalSolver {
     pub name: String,
     pub url: Url,

crates/shared/src/price_estimation.rs

@@ -1,6 +1,6 @@
 use {
     crate::{
-        arguments::{display_option, display_secret_option},
+        arguments::{display_option, display_secret_option, ExternalSolver},
         conversions::U256Ext,
         trade_finding::Interaction,
     },
@@ -19,6 +19,7 @@ use {
         fmt::{self, Display, Formatter},
         future::Future,
         hash::Hash,
+        str::FromStr,
         sync::Arc,
         time::{Duration, Instant},
     },
@@ -42,15 +43,15 @@ pub struct NativePriceEstimators(Vec<Vec<NativePriceEstimator>>);
 
 #[derive(Clone, Debug, Hash, Eq, PartialEq)]
 pub enum NativePriceEstimator {
-    GenericPriceEstimator(String),
+    Driver(ExternalSolver),
     OneInchSpotPriceApi,
 }
 
 impl Display for NativePriceEstimator {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         let formatter = match self {
-            NativePriceEstimator::GenericPriceEstimator(s) => s,
-            NativePriceEstimator::OneInchSpotPriceApi => "OneInchSpotPriceApi",
+            NativePriceEstimator::Driver(s) => format!("{}|{}", &s.name, s.url),
+            NativePriceEstimator::OneInchSpotPriceApi => "OneInchSpotPriceApi".into(),
         };
         write!(f, "{}", formatter)
     }
@@ -62,14 +63,6 @@ impl NativePriceEstimators {
     }
 }
 
-impl Default for NativePriceEstimators {
-    fn default() -> Self {
-        Self(vec![vec![NativePriceEstimator::GenericPriceEstimator(
-            "Baseline".into(),
-        )]])
-    }
-}
-
 impl Display for NativePriceEstimators {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         let formatter = self
@@ -85,26 +78,32 @@ impl Display for NativePriceEstimators {
     }
 }
 
-impl From<&str> for NativePriceEstimators {
-    fn from(s: &str) -> Self {
-        Self(
+impl FromStr for NativePriceEstimators {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(Self(
             s.split(';')
                 .map(|sub_list| {
                     sub_list
                         .split(',')
-                        .map(NativePriceEstimator::from)
-                        .collect::<Vec<NativePriceEstimator>>()
+                        .map(NativePriceEstimator::from_str)
+                        .collect::<Result<Vec<NativePriceEstimator>>>()
                 })
-                .collect::<Vec<Vec<NativePriceEstimator>>>(),
-        )
+                .collect::<Result<Vec<Vec<NativePriceEstimator>>>>()?,
+        ))
     }
 }
 
-impl From<&str> for NativePriceEstimator {
-    fn from(s: &str) -> Self {
+impl FromStr for NativePriceEstimator {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
-            "OneInchSpotPriceApi" => NativePriceEstimator::OneInchSpotPriceApi,
-            estimator => NativePriceEstimator::GenericPriceEstimator(estimator.into()),
+            "OneInchSpotPriceApi" => Ok(NativePriceEstimator::OneInchSpotPriceApi),
+            estimator => Ok(NativePriceEstimator::Driver(ExternalSolver::from_str(
+                estimator,
+            )?)),
         }
     }
 }
@@ -482,16 +481,19 @@ mod tests {
         // Arg::value_parser or #[arg(value_enum)]:
         // https://docs.rs/clap/latest/clap/_derive/index.html#arg-attributes
 
-        let parsed = |arg: &str| NativePriceEstimators::from(arg);
+        let parsed = |arg: &str| NativePriceEstimators::from_str(arg);
         let stringified = |arg: &NativePriceEstimators| format!("{arg}");
 
         for repr in [
-            &NativePriceEstimator::GenericPriceEstimator("Baseline".into()).to_string(),
+            &NativePriceEstimator::Driver(
+                ExternalSolver::from_str("baseline|http://localhost:1234/").unwrap(),
+            )
+            .to_string(),
             &NativePriceEstimator::OneInchSpotPriceApi.to_string(),
-            "one,two;three,four",
-            &format!("one,two;{},four", NativePriceEstimator::OneInchSpotPriceApi),
+            "one|http://localhost:1111/,two|http://localhost:2222/;three|http://localhost:3333/,four|http://localhost:4444/",
+            &format!("one|http://localhost:1111/,two|http://localhost:2222/;{},four|http://localhost:4444/", NativePriceEstimator::OneInchSpotPriceApi),
         ] {
-            assert_eq!(stringified(&parsed(repr)), repr);
+            assert_eq!(stringified(&parsed(repr).unwrap()), repr);
         }
     }
 }

crates/shared/src/price_estimation/factory.rs

@@ -29,7 +29,7 @@ use {
         },
         token_info::TokenInfoFetching,
     },
-    anyhow::{anyhow, Context as _, Result},
+    anyhow::{Context as _, Result},
     ethcontract::H160,
     ethrpc::current_block::CurrentBlockStream,
     gas_estimation::GasPriceEstimating,
@@ -206,33 +206,24 @@ impl<'a> PriceEstimatorFactory<'a> {
 
     fn create_native_estimator(
         &mut self,
-        source: NativePriceEstimatorSource,
-        external: &[PriceEstimatorSource],
+        source: &NativePriceEstimatorSource,
     ) -> Result<(String, Arc<dyn NativePriceEstimating>)> {
         match source {
-            NativePriceEstimatorSource::GenericPriceEstimator(estimator) => {
+            NativePriceEstimatorSource::Driver(driver) => {
                 let native_token_price_estimation_amount =
                     self.native_token_price_estimation_amount()?;
-                self.get_estimators(external, |entry| &entry.native)?
-                    .into_iter()
-                    .map(
-                        |(name, estimator)| -> (String, Arc<dyn NativePriceEstimating>) {
-                            (
-                                name,
-                                Arc::new(NativePriceEstimator::new(
-                                    Arc::new(self.sanitized(estimator)),
-                                    self.network.native_token,
-                                    native_token_price_estimation_amount,
-                                )),
-                            )
-                        },
-                    )
-                    .find(|external| external.0 == estimator)
-                    .ok_or(anyhow!(
-                        "Couldn't find generic price estimator with name {} to instantiate native \
-                         estimator",
-                        estimator
-                    ))
+                let estimator = self
+                    .get_estimator(&PriceEstimatorSource::External(driver.clone()))?
+                    .native
+                    .clone();
+                Ok((
+                    driver.name.clone(),
+                    Arc::new(NativePriceEstimator::new(
+                        Arc::new(self.sanitized(estimator)),
+                        self.network.native_token,
+                        native_token_price_estimation_amount,
+                    )),
+                ))
             }
             NativePriceEstimatorSource::OneInchSpotPriceApi => Ok((
                 "OneInchSpotPriceApi".into(),
@@ -324,7 +315,6 @@ impl<'a> PriceEstimatorFactory<'a> {
     pub fn native_price_estimator(
         &mut self,
         native: &[Vec<NativePriceEstimatorSource>],
-        external: &[PriceEstimatorSource],
         results_required: NonZeroUsize,
     ) -> Result<Arc<CachingNativePriceEstimator>> {
         anyhow::ensure!(
@@ -337,7 +327,7 @@ impl<'a> PriceEstimatorFactory<'a> {
             .map(|stage| {
                 stage
                     .iter()
-                    .map(|source| self.create_native_estimator(source.clone(), external))
+                    .map(|source| self.create_native_estimator(source))
                     .collect::<Result<Vec<_>>>()
             })
             .collect::<Result<Vec<Vec<_>>>>()?;

playground/docker-compose.fork.yml

@@ -76,8 +76,8 @@ services:
       - EIP1271_SKIP_CREATION_VALIDATION=true
       - ENABLE_EIP1271_ORDERS=true
       - PRICE_ESTIMATORS=None
-      - NATIVE_PRICE_ESTIMATORS=baseline
       - PRICE_ESTIMATION_DRIVERS=baseline|http://driver/baseline
+      - NATIVE_PRICE_ESTIMATORS=baseline|http://driver/baseline
       - DRIVERS=baseline|http://driver/baseline
       - BIND_ADDRESS=0.0.0.0:80
       - CHAIN_ID=$CHAIN
@@ -109,6 +109,7 @@ services:
       - NATIVE_PRICE_CACHE_MAX_AGE=20m
       - SOLVER_TIME_LIMIT=5
       - PRICE_ESTIMATION_DRIVERS=baseline|http://driver/baseline
+      - NATIVE_PRICE_ESTIMATORS=baseline|http://driver/baseline
       - DRIVERS=baseline|http://driver/baseline
       - SKIP_EVENT_SYNC=true
       - BASELINE_SOURCES=None