Fix quote verification for sell=buy

m-sz · m-sz · commit 6fe22d19c668 · 2025-12-05T15:24:44.000+01:00
diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs
@@ -9,8 +9,10 @@ use {
     futures::{Stream, StreamExt},
     model::{
         order::{OrderCreation, OrderKind},
+        quote::{OrderQuoteRequest, OrderQuoteSide, SellAmount},
         signature::EcdsaSigningScheme,
     },
+    number::nonzero::U256 as NonZeroU256,
     secp256k1::SecretKey,
     shared::ethrpc::Web3,
     std::time::Duration,
@@ -280,19 +282,53 @@ async fn test_execute_same_sell_and_buy_token(web3: Web3) {
         .await
         .expect("Must be able to disable automine");
 
+    tracing::info!("Quoting");
+    let quote_sell_amount = to_wei(1);
+    let quote_request = OrderQuoteRequest {
+        from: trader.address(),
+        sell_token: *token.address(),
+        buy_token: *token.address(),
+        side: OrderQuoteSide::Sell {
+            sell_amount: SellAmount::BeforeFee {
+                value: NonZeroU256::try_from(quote_sell_amount).unwrap(),
+            },
+        },
+        ..Default::default()
+    };
+    let quote_response = services.submit_quote(&quote_request).await.unwrap();
+    tracing::info!(?quote_response);
+    assert!(quote_response.id.is_some());
+    assert!(quote_response.verified);
+    assert!(quote_response.quote.buy_amount.into_legacy() < quote_sell_amount);
+
+    let quote_metadata =
+        crate::database::quote_metadata(services.db(), quote_response.id.unwrap()).await;
+    assert!(quote_metadata.is_some());
+    tracing::debug!(?quote_metadata);
+
     tracing::info!("Placing order");
     let initial_balance = token.balanceOf(trader.address()).call().await.unwrap();
     assert_eq!(initial_balance, eth(10));
 
-    let order_sell_amount = to_wei(2);
-    let order_buy_amount = to_wei(1);
+    // let quote_sell_amount = to_wei(2);
+    // let order_buy_amount = to_wei(1);
+    // let order = OrderCreation {
+    //     sell_token: token.address().into_legacy(),
+    //     sell_amount: order_sell_amount,
+    //     buy_token: token.address().into_legacy(),
+    //     buy_amount: order_buy_amount,
+    //     valid_to: model::time::now_in_epoch_seconds() + 300,
+    //     kind: OrderKind::Sell,
+    //     ..Default::default()
+    // }
     let order = OrderCreation {
+        kind: OrderKind::Sell,
         sell_token: token.address().into_legacy(),
-        sell_amount: order_sell_amount,
         buy_token: token.address().into_legacy(),
-        buy_amount: order_buy_amount,
+        quote_id: quote_response.id,
+        sell_amount: quote_sell_amount,
+        buy_amount: quote_response.quote.buy_amount.into_legacy(),
         valid_to: model::time::now_in_epoch_seconds() + 300,
-        kind: OrderKind::Sell,
         ..Default::default()
     }
     .sign(
@@ -314,7 +350,7 @@ async fn test_execute_same_sell_and_buy_token(web3: Web3) {
     onchain.mint_block().await;
 
     // Wait for settlement tx to appear in txpool
-    wait_for_condition(Duration::from_secs(2), || async {
+    wait_for_condition(TIMEOUT, || async {
         get_pending_tx(solver.account().address(), &web3)
             .await
             .is_some()
diff --git a/crates/shared/src/price_estimation/factory.rs b/crates/shared/src/price_estimation/factory.rs
@@ -194,7 +194,7 @@ impl<'a> PriceEstimatorFactory<'a> {
                     driver.name.clone(),
                     Arc::new(InstrumentedPriceEstimator::new(
                         NativePriceEstimator::new(
-                            Arc::new(self.sanitized(estimator)),
+                            Arc::new(self.sanitized_native_price(estimator)),
                             self.network.native_token,
                             native_token_price_estimation_amount,
                         ),
@@ -303,6 +303,21 @@ impl<'a> PriceEstimatorFactory<'a> {
             estimator,
             self.network.native_token,
             self.components.bad_token_detector.clone(),
+            false, // not estimating native price
+        )
+    }
+
+    /// Creates a SanitizedPRiceEstimator that is used for native price
+    /// estimations
+    fn sanitized_native_price(
+        &self,
+        estimator: Arc<dyn PriceEstimating>,
+    ) -> SanitizedPriceEstimator {
+        SanitizedPriceEstimator::new(
+            estimator,
+            self.network.native_token,
+            self.components.bad_token_detector.clone(),
+            true, // estimating native price
         )
     }
 
diff --git a/crates/shared/src/price_estimation/sanitized.rs b/crates/shared/src/price_estimation/sanitized.rs
@@ -24,18 +24,22 @@ pub struct SanitizedPriceEstimator {
     inner: Arc<dyn PriceEstimating>,
     bad_token_detector: Arc<dyn BadTokenDetecting>,
     native_token: Address,
+    /// Enables the short-circuiting logic in case of sell and buy is the same
+    is_estimating_native_price: bool,
 }
 
 impl SanitizedPriceEstimator {
     pub fn new(
         inner: Arc<dyn PriceEstimating>,
         native_token: Address,
         bad_token_detector: Arc<dyn BadTokenDetecting>,
+        is_estimating_native_price: bool,
     ) -> Self {
         Self {
             inner,
             native_token,
             bad_token_detector,
+            is_estimating_native_price,
         }
     }
 
@@ -64,7 +68,10 @@ impl PriceEstimating for SanitizedPriceEstimator {
             self.handle_bad_tokens(&query).await?;
 
             // buy_token == sell_token => 1 to 1 conversion
-            if query.buy_token == query.sell_token {
+            // Only in case of native price estimation.
+            // For regular price estimation, the sell and buy tokens can
+            // be the same and should be priced as usual
+            if self.is_estimating_native_price && query.buy_token == query.sell_token {
                 let estimation = Estimate {
                     out_amount: query.in_amount.get().into_alloy(),
                     gas: 0,
@@ -454,11 +461,12 @@ mod tests {
                 }
                 .boxed()
             });
-
+        let bad_token_detector = Arc::new(bad_token_detector);
         let sanitized_estimator = SanitizedPriceEstimator {
             inner: Arc::new(wrapped_estimator),
-            bad_token_detector: Arc::new(bad_token_detector),
+            bad_token_detector: bad_token_detector.clone(),
             native_token,
+            is_estimating_native_price: true,
         };
 
         for (query, expectation) in queries {
@@ -473,5 +481,110 @@ mod tests {
                 }
             }
         }
+
+        let queries = [
+            // Can be estimated by `sanitized_estimator` because `buy_token` and `sell_token` are
+            // identical.
+            (
+                Query {
+                    verification: Default::default(),
+                    sell_token: Address::with_last_byte(1),
+                    buy_token: Address::with_last_byte(1),
+                    in_amount: NonZeroU256::try_from(1).unwrap(),
+                    kind: OrderKind::Sell,
+                    block_dependent: false,
+                    timeout: HEALTHY_PRICE_ESTIMATION_TIME,
+                },
+                Ok(Estimate {
+                    out_amount: AlloyU256::ONE,
+                    gas: 100,
+                    solver: Default::default(),
+                    verified: true,
+                    execution: Default::default(),
+                }),
+            ),
+            (
+                Query {
+                    verification: Default::default(),
+                    sell_token: native_token,
+                    buy_token: native_token,
+                    in_amount: NonZeroU256::try_from(1).unwrap(),
+                    kind: OrderKind::Sell,
+                    block_dependent: false,
+                    timeout: HEALTHY_PRICE_ESTIMATION_TIME,
+                },
+                Ok(Estimate {
+                    out_amount: AlloyU256::ONE,
+                    gas: 100,
+                    solver: Default::default(),
+                    verified: true,
+                    execution: Default::default(),
+                }),
+            ),
+        ];
+
+        // SanitizedPriceEstimator will simply forward the Query in the sell=buy case
+        // if it is not calculating native price
+        let first_forwarded_query = queries[0].0.clone();
+
+        // SanitizedPriceEstimator will simply forward the Query if sell=buy of native
+        // token case if it is not calculating the native price
+        let second_forwarded_query = queries[1].0.clone();
+
+        let mut wrapped_estimator = MockPriceEstimating::new();
+        wrapped_estimator
+            .expect_estimate()
+            .times(1)
+            .withf(move |query| **query == first_forwarded_query)
+            .returning(|_| {
+                async {
+                    Ok(Estimate {
+                        out_amount: AlloyU256::ONE,
+                        gas: 100,
+                        solver: Default::default(),
+                        verified: true,
+                        execution: Default::default(),
+                    })
+                }
+                .boxed()
+            });
+        wrapped_estimator
+            .expect_estimate()
+            .times(1)
+            .withf(move |query| **query == second_forwarded_query)
+            .returning(|_| {
+                async {
+                    Ok(Estimate {
+                        out_amount: AlloyU256::ONE,
+                        gas: 100,
+                        solver: Default::default(),
+                        verified: true,
+                        execution: Default::default(),
+                    })
+                }
+                .boxed()
+            });
+
+        let sanitized_estimator_non_native = SanitizedPriceEstimator {
+            inner: Arc::new(wrapped_estimator),
+            bad_token_detector,
+            native_token,
+            is_estimating_native_price: false,
+        };
+
+        for (query, expectation) in queries {
+            let result = sanitized_estimator_non_native
+                .estimate(Arc::new(query))
+                .await;
+            match result {
+                Ok(estimate) => assert_eq!(estimate, expectation.unwrap()),
+                Err(err) => {
+                    // we only compare the error variant; everything else would be a PITA
+                    let reported_error = std::mem::discriminant(&err);
+                    let expected_error = std::mem::discriminant(&expectation.unwrap_err());
+                    assert_eq!(reported_error, expected_error);
+                }
+            }
+        }
     }
 }
diff --git a/crates/shared/src/price_estimation/trade_verifier/mod.rs b/crates/shared/src/price_estimation/trade_verifier/mod.rs
@@ -19,7 +19,7 @@ use {
     ::alloy::sol_types::SolCall,
     alloy::primitives::{Address, address},
     anyhow::{Context, Result, anyhow},
-    bigdecimal::BigDecimal,
+    bigdecimal::{BigDecimal, Zero},
     contracts::alloy::{
         GPv2Settlement,
         WETH9,
@@ -766,8 +766,11 @@ fn add_balance_queries(
         alloy::primitives::U256::ZERO,
         query_balance_call.into(),
     );
-    // query balance query at the end of pre-interactions
-    settlement.interactions[0].push(interaction.clone());
+
+    // query the balance as the first interaction, the sell token has already
+    // been transferred into the settlement contract since the calculation is
+    // based on the out amount
+    settlement.interactions[1].insert(0, interaction.clone());
     // query balance right after we payed out all `buy_token`
     settlement.interactions[2].insert(0, interaction);
     settlement
@@ -864,7 +867,7 @@ fn ensure_quote_accuracy(
         .get(&query.buy_token)
         .context("summary buy token is missing")?;
 
-    if *sell_token_lost >= sell_token_lost_limit || *buy_token_lost >= buy_token_lost_limit {
+    if (!sell_token_lost.is_zero() && *sell_token_lost >= sell_token_lost_limit) || (!buy_token_lost.is_zero() && *buy_token_lost >= buy_token_lost_limit) {
         return Err(Error::TooInaccurate);
     }
 
diff --git a/crates/solvers/src/boundary/baseline.rs b/crates/solvers/src/boundary/baseline.rs
@@ -46,10 +46,10 @@ impl<'a> Solver<'a> {
         &self,
         request: solver::Request,
         max_hops: usize,
-    ) -> Result<Option<solver::Route<'a>>, RouteComputationError> {
+    ) -> Option<solver::Route<'a>> {
         if request.sell.token == request.buy.token {
             tracing::info!("Sell=Buy, returning empty route");
-            return Ok(None);
+            return Some(solver::Route::new(vec![]));
         }
         let candidates = self.base_tokens.path_candidates_with_hops(
             request.sell.token.0.into_alloy(),
@@ -87,8 +87,7 @@ impl<'a> Solver<'a> {
                     .await
                     .into_iter()
                     .flatten()
-                    .min_by_key(|(_, sell)| sell.value)
-                    .ok_or(RouteComputationError)?
+                    .min_by_key(|(_, sell)| sell.value)?
             }
             order::Side::Sell => {
                 let futures = candidates.iter().map(|path| async {
@@ -119,12 +118,11 @@ impl<'a> Solver<'a> {
                     .await
                     .into_iter()
                     .flatten()
-                    .max_by_key(|(_, buy)| buy.value)
-                    .ok_or(RouteComputationError)?
+                    .max_by_key(|(_, buy)| buy.value)?
             }
         };
 
-        solver::Route::new(segments).map(Ok).transpose()
+        Some(solver::Route::new(segments))
     }
 
     async fn traverse_path(
diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs
