Fix quote verification for sell=buy

m-sz · m-sz · commit c3a04a203de6 · 2025-12-05T14:39:37.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,
@@ -459,6 +466,7 @@ mod tests {
             inner: Arc::new(wrapped_estimator),
             bad_token_detector: Arc::new(bad_token_detector),
             native_token,
+            is_estimating_native_price: false,
         };
 
         for (query, expectation) in queries {
diff --git a/crates/shared/src/price_estimation/trade_verifier/mod.rs b/crates/shared/src/price_estimation/trade_verifier/mod.rs
@@ -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
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
@@ -170,21 +170,21 @@ impl Inner {
                         .route(native_price_request, self.max_hops)
                         .await
                     {
-                        Ok(Some(route)) => {
+                        Some(route) if !route.is_empty() => {
                             // how many units of buy_token are bought for one unit of sell_token
                             // (buy_amount / sell_amount).
                             let price = self.native_token_price_estimation_amount.to_f64_lossy()
-                                / route.input().amount.to_f64_lossy();
+                                / route
+                                    .input()
+                                    .expect("route is not empty")
+                                    .amount
+                                    .to_f64_lossy();
                             let Some(price) = to_normalized_price(price) else {
                                 continue;
                             };
 
                             auction::Price(eth::Ether(price))
                         }
-                        Ok(None) => {
-                            tracing::info!("Solving for sell=buy, price is 1");
-                            auction::Price(eth::Ether(U256::one()))
-                        }
                         _ => {
                             // This is to allow quotes to be generated for tokens for which the sell
                             // token price is not available, so we default to fee=0
@@ -196,48 +196,40 @@ impl Inner {
 
             let compute_solution = async |request: Request| -> Option<Solution> {
                 let wrappers = request.wrappers.clone();
-                let route = boundary_solver.route(request, self.max_hops).await.ok()?;
+                let route = boundary_solver.route(request, self.max_hops).await?;
                 let interactions;
                 let gas;
                 let (input, mut output);
 
-                match route {
-                    Some(route) => {
-                        interactions = route
-                            .segments
-                            .iter()
-                            .map(|segment| {
-                                solution::Interaction::Liquidity(Box::new(
-                                    solution::LiquidityInteraction {
-                                        liquidity: segment.liquidity.clone(),
-                                        input: segment.input,
-                                        output: segment.output,
-                                        // TODO does the baseline solver know about this
-                                        // optimization?
-                                        internalize: false,
-                                    },
-                                ))
-                            })
-                            .collect();
-                        gas = route.gas() + self.solution_gas_offset;
-                        input = route.input();
-                        output = route.output();
-                    }
-                    None => {
-                        interactions = Vec::default();
-                        gas = eth::Gas(U256::zero()) + self.solution_gas_offset;
-
-                        (input, output) = match order.side {
-                            order::Side::Sell => (order.buy, order.sell),
-                            order::Side::Buy => (order.sell, order.buy),
-                        };
-                        let sell = order.sell;
-                        let buy = order.buy;
-                        tracing::info!(
-                            "Computing solution for sell=buy. input: {input:?} output: \
-                             {output:?}, sell: {sell:?}, buy: {buy:?}"
-                        );
-                    }
+                if !route.is_empty() {
+                    interactions = route
+                        .segments
+                        .iter()
+                        .map(|segment| {
+                            solution::Interaction::Liquidity(Box::new(
+                                solution::LiquidityInteraction {
+                                    liquidity: segment.liquidity.clone(),
+                                    input: segment.input,
+                                    output: segment.output,
+                                    // TODO does the baseline solver know about this
+                                    // optimization?
+                                    internalize: false,
+                                },
+                            ))
+                        })
+                        .collect();
+                    gas = route.gas() + self.solution_gas_offset;
+                    input = route.input().expect("route is not empty");
+                    output = route.output().expect("route is not empty");
+                } else {
+                    interactions = Vec::default();
+                    gas = eth::Gas(U256::zero()) + self.solution_gas_offset;
+
+                    (input, output) = match order.side {
+                        order::Side::Sell => (order.sell, order.buy),
+                        order::Side::Buy => (order.buy, order.sell),
+                    };
+                    output.amount = input.amount;
                 }
 
                 // The baseline solver generates a path with swapping
@@ -386,22 +378,20 @@ pub struct Segment<'a> {
 }
 
 impl<'a> Route<'a> {
-    pub fn new(segments: Vec<Segment<'a>>) -> Option<Self> {
-        if segments.is_empty() {
-            return None;
-        }
-        Some(Self { segments })
+    pub fn new(segments: Vec<Segment<'a>>) -> Self {
+        Self { segments }
+    }
+
+    fn input(&self) -> Option<eth::Asset> {
+        self.segments.first().map(|segment| segment.input)
     }
 
-    fn input(&self) -> eth::Asset {
-        self.segments[0].input
+    fn output(&self) -> Option<eth::Asset> {
+        self.segments.last().map(|segment| segment.output)
     }
 
-    fn output(&self) -> eth::Asset {
-        self.segments
-            .last()
-            .expect("route has at least one segment by construction")
-            .output
+    fn is_empty(&self) -> bool {
+        self.segments.is_empty()
     }
 
     fn gas(&self) -> eth::Gas {
