From 0addb360ec3d4a21c5728e01de95ce9bf7a60c6d Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Mon, 10 Nov 2025 17:21:29 +0100 Subject: [PATCH 01/39] Reimplement sell=buy PoC PR --- crates/driver/src/domain/quote.rs | 28 ++--- .../src/infra/api/routes/quote/dto/order.rs | 9 +- .../driver/src/infra/api/routes/quote/mod.rs | 4 +- .../e2e/tests/e2e/place_order_with_quote.rs | 104 +++++++++++++++++- crates/orderbook/src/api/post_order.rs | 7 -- crates/shared/src/order_validation.rs | 15 --- crates/solvers/src/boundary/baseline.rs | 5 +- crates/solvers/src/domain/solver.rs | 31 ++++-- 8 files changed, 141 insertions(+), 62 deletions(-) diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs index 73015a7ff7..cbdb499958 100644 --- a/crates/driver/src/domain/quote.rs +++ b/crates/driver/src/domain/quote.rs @@ -88,13 +88,13 @@ impl Order { liquidity: &infra::liquidity::Fetcher, tokens: &infra::tokens::Fetcher, ) -> Result { - let liquidity = match solver.liquidity() { - solver::Liquidity::Fetch => { + let liquidity = match (solver.liquidity(), self.liquidity_pairs()) { + (solver::Liquidity::Fetch, Some(pairs)) => { liquidity - .fetch(&self.liquidity_pairs(), infra::liquidity::AtBlock::Recent) + .fetch(&pairs, infra::liquidity::AtBlock::Recent) .await - } - solver::Liquidity::Skip => Default::default(), + }, + _ => Default::default(), }; let auction = self @@ -225,15 +225,12 @@ impl Order { } /// Returns the token pairs to fetch liquidity for. - fn liquidity_pairs(&self) -> HashSet { - let pair = liquidity::TokenPair::try_new(self.tokens.sell(), self.tokens.buy()) - .expect("sell != buy by construction"); - iter::once(pair).collect() + fn liquidity_pairs(&self) -> Option> { + liquidity::TokenPair::try_new(self.tokens.sell(), self.tokens.buy()) + .map(|pair| iter::once(pair).collect()).ok() } } -/// The sell and buy tokens to quote for. This type maintains the invariant that -/// the sell and buy tokens are distinct. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub struct Tokens { sell: eth::TokenAddress, @@ -241,13 +238,8 @@ pub struct Tokens { } impl Tokens { - /// Creates a new instance of [`Tokens`], verifying that the input buy and - /// sell tokens are distinct. - pub fn try_new(sell: eth::TokenAddress, buy: eth::TokenAddress) -> Result { - if sell == buy { - return Err(SameTokens); - } - Ok(Self { sell, buy }) + pub fn new(sell: eth::TokenAddress, buy: eth::TokenAddress) -> Self { + Self { sell, buy } } pub fn sell(&self) -> eth::TokenAddress { diff --git a/crates/driver/src/infra/api/routes/quote/dto/order.rs b/crates/driver/src/infra/api/routes/quote/dto/order.rs index b157954bf0..c893e0f901 100644 --- a/crates/driver/src/infra/api/routes/quote/dto/order.rs +++ b/crates/driver/src/infra/api/routes/quote/dto/order.rs @@ -8,17 +8,16 @@ use { }; impl Order { - pub fn into_domain(self) -> Result { - Ok(quote::Order { - tokens: quote::Tokens::try_new(self.sell_token.into(), self.buy_token.into()) - .map_err(|quote::SameTokens| Error::SameTokens)?, + pub fn into_domain(self) -> quote::Order { + quote::Order { + tokens: quote::Tokens::new(self.sell_token.into(), self.buy_token.into()), amount: self.amount.into(), side: match self.kind { Kind::Sell => competition::order::Side::Sell, Kind::Buy => competition::order::Side::Buy, }, deadline: self.deadline, - }) + } } } diff --git a/crates/driver/src/infra/api/routes/quote/mod.rs b/crates/driver/src/infra/api/routes/quote/mod.rs index 77de817213..a55e7490cb 100644 --- a/crates/driver/src/infra/api/routes/quote/mod.rs +++ b/crates/driver/src/infra/api/routes/quote/mod.rs @@ -19,9 +19,7 @@ async fn route( order: axum::extract::Query, ) -> Result, (hyper::StatusCode, axum::Json)> { let handle_request = async { - let order = order.0.into_domain().inspect_err(|err| { - observe::invalid_dto(err, "order"); - })?; + let order = order.0.into_domain(); observe::quoting(&order); let quote = order .quote( diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index f375426587..02ee7d29a1 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -7,19 +7,21 @@ use { order::{OrderCreation, OrderKind}, quote::{OrderQuoteRequest, OrderQuoteSide, SellAmount}, signature::EcdsaSigningScheme, - }, - secp256k1::SecretKey, - shared::ethrpc::Web3, - std::ops::DerefMut, - web3::signing::SecretKeyRef, + }, secp256k1::SecretKey, shared::ethrpc::Web3, std::ops::DerefMut, web3::signing::SecretKeyRef }; #[tokio::test] #[ignore] -async fn local_node_test() { +async fn local_node_place_order_with_quote() { run_test(place_order_with_quote).await; } +#[tokio::test] +#[ignore] +async fn local_node_place_order_with_quote_same_token_pair() { + run_test(place_order_with_quote_same_token_pair).await; +} + async fn place_order_with_quote(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3.clone()).await; @@ -111,3 +113,93 @@ async fn place_order_with_quote(web3: Web3) { assert_eq!(quote_response.verified, order_quote.verified); assert_eq!(quote_metadata.unwrap().0, order_quote.metadata); } + +async fn place_order_with_quote_same_token_pair(web3: Web3) { + let mut onchain = OnchainComponents::deploy(web3.clone()).await; + + let [solver] = onchain.make_solvers(to_wei(10).into_alloy()).await; + let [trader] = onchain.make_accounts(to_wei(10).into_alloy()).await; + let [token] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) + .await; + + token.mint(trader.address(), to_wei(10).into_alloy()).await; + + token + .approve(onchain.contracts().allowance.into_alloy(), eth(3)) + .from(trader.address()) + .send_and_watch() + .await + .unwrap(); + onchain + .contracts() + .weth + .deposit() + .from(trader.address()) + .value(eth(3)) + .send_and_watch() + .await + .unwrap(); + + tracing::info!("Starting services."); + let services = Services::new(&onchain).await; + services.start_protocol(solver.clone()).await; + + // Disable auto-mine so we don't accidentally mine a settlement + web3.api::>() + .set_automine_enabled(false) + .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("e_request).await.unwrap(); + tracing::debug!(?quote_response); + assert!(quote_response.id.is_some()); + assert!(quote_response.verified); + + 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 order = OrderCreation { + quote_id: quote_response.id, + sell_token: *token.address(), + sell_amount: quote_sell_amount.into_alloy(), + buy_token: *token.address(), + buy_amount: quote_response.quote.buy_amount, + 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.private_key()).unwrap()), + ); + let order_uid = services.create_order(&order).await.unwrap(); + + tracing::info!("Order quote verification"); + let order_quote = database::orders::read_quote( + services.db().acquire().await.unwrap().deref_mut(), + &database::byte_array::ByteArray(order_uid.0), + ) + .await + .unwrap() + .unwrap(); + assert_eq!(quote_response.verified, order_quote.verified); + assert_eq!(quote_metadata.unwrap().0, order_quote.metadata); +} diff --git a/crates/orderbook/src/api/post_order.rs b/crates/orderbook/src/api/post_order.rs index 7a57ac4ed4..d278b009e8 100644 --- a/crates/orderbook/src/api/post_order.rs +++ b/crates/orderbook/src/api/post_order.rs @@ -72,13 +72,6 @@ impl IntoWarpReply for PartialValidationErrorWrapper { ), StatusCode::BAD_REQUEST, ), - PartialValidationError::SameBuyAndSellToken => with_status( - error( - "SameBuyAndSellToken", - "Buy token is the same as the sell token.", - ), - StatusCode::BAD_REQUEST, - ), PartialValidationError::UnsupportedToken { token, reason } => with_status( error( "UnsupportedToken", diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index efc7845d25..1441bc73b4 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -109,7 +109,6 @@ pub enum PartialValidationError { Forbidden, ValidTo(OrderValidToError), InvalidNativeSellToken, - SameBuyAndSellToken, UnsupportedBuyTokenDestination(BuyTokenDestination), UnsupportedSellTokenSource(SellTokenSource), UnsupportedOrderType, @@ -481,9 +480,6 @@ impl OrderValidating for OrderValidator { self.validity_configuration.validate_period(&order)?; - if has_same_buy_and_sell_token(&order, self.native_token.address()) { - return Err(PartialValidationError::SameBuyAndSellToken); - } if order.sell_token == BUY_ETH_ADDRESS { return Err(PartialValidationError::InvalidNativeSellToken); } @@ -1201,17 +1197,6 @@ mod tests { OrderValidToError::Excessive, )) )); - assert!(matches!( - validator - .partial_validate(PreOrderData { - valid_to: legit_valid_to, - buy_token: Address::with_last_byte(2), - sell_token: Address::with_last_byte(2), - ..Default::default() - }) - .await, - Err(PartialValidationError::SameBuyAndSellToken) - )); assert!(matches!( validator .partial_validate(PreOrderData { diff --git a/crates/solvers/src/boundary/baseline.rs b/crates/solvers/src/boundary/baseline.rs index 412a10f752..0f6e7fb89a 100644 --- a/crates/solvers/src/boundary/baseline.rs +++ b/crates/solvers/src/boundary/baseline.rs @@ -43,6 +43,9 @@ impl<'a> Solver<'a> { request: solver::Request, max_hops: usize, ) -> Option> { + if request.sell.token == request.buy.token { + return Some(solver::Route::new(vec![])); + } let candidates = self.base_tokens.path_candidates_with_hops( request.sell.token.0, request.buy.token.0, @@ -112,7 +115,7 @@ impl<'a> Solver<'a> { } }; - solver::Route::new(segments) + 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 index 4dda735318..ad0458bb91 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -213,7 +213,17 @@ impl Inner { // can buy slightly more than intended. Fix this by // capping the output amount to the order's buy amount // for buy orders. - let mut output = route.output(); + // let mut output = route.output(); + let mut output = if route.is_empty() { + order.sell + } else { + route.output() + }; + let input = if route.is_empty() { + order.sell + } else { + route.input() + }; if let order::Side::Buy = order.side { output.amount = cmp::min(output.amount, order.buy.amount); } @@ -226,7 +236,7 @@ impl Inner { Some( solution::Single { order: order.clone(), - input: route.input(), + input, output, interactions, gas, @@ -355,18 +365,25 @@ pub struct Segment<'a> { } impl<'a> Route<'a> { - pub fn new(segments: Vec>) -> Option { - if segments.is_empty() { - return None; - } - Some(Self { segments }) + pub fn new(segments: Vec>) -> Self { + Self { segments } + } + + fn is_empty(&self) -> bool { + self.segments.is_empty() } fn input(&self) -> eth::Asset { + if self.is_empty() { + unreachable!("Output empty segment"); + } self.segments[0].input } fn output(&self) -> eth::Asset { + if self.is_empty() { + unreachable!("Output empty segment"); + } self.segments .last() .expect("route has at least one segment by construction") From 19941ce390c339f70784a1a06f3452ceb15f0d77 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 12 Nov 2025 12:27:25 +0100 Subject: [PATCH 02/39] Fix failing sell=buy test --- Justfile | 2 +- crates/e2e/tests/e2e/place_order_with_quote.rs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Justfile b/Justfile index c5a33ac10c..41f0c271e6 100644 --- a/Justfile +++ b/Justfile @@ -23,7 +23,7 @@ test-e2e-forked: (test-e2e "forked_node") # Run End-to-end tests with custom filters test-e2e *filters: - cargo nextest run -p e2e {{filters}} --test-threads 1 --failure-output final --run-ignored ignored-only + cargo nextest run -p e2e '{{filters}}' --test-threads 1 --failure-output final --run-ignored ignored-only test-driver: RUST_MIN_STACK=3145728 cargo nextest run -p driver --test-threads 1 --run-ignored ignored-only diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index 02ee7d29a1..5484cfe986 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -12,7 +12,7 @@ use { #[tokio::test] #[ignore] -async fn local_node_place_order_with_quote() { +async fn local_node_place_order_with_quote_basic() { run_test(place_order_with_quote).await; } @@ -140,6 +140,11 @@ async fn place_order_with_quote_same_token_pair(web3: Web3) { .send_and_watch() .await .unwrap(); + token.approve(onchain.contracts().allowance.into_alloy(), eth(10)) + .from(trader.address()) + .send_and_watch() + .await + .unwrap(); tracing::info!("Starting services."); let services = Services::new(&onchain).await; From 9b44cfdede186ab4f1df26d9630901194316ce1a Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 12 Nov 2025 16:51:26 +0100 Subject: [PATCH 03/39] cleanup --- Justfile | 4 +- crates/driver/src/domain/quote.rs | 7 +- .../e2e/tests/e2e/place_order_with_quote.rs | 28 ++---- crates/orderbook/src/run.rs | 1 - crates/shared/src/order_validation.rs | 93 +------------------ 5 files changed, 18 insertions(+), 115 deletions(-) diff --git a/Justfile b/Justfile index 41f0c271e6..763a7552f4 100644 --- a/Justfile +++ b/Justfile @@ -22,8 +22,8 @@ test-e2e-local: (test-e2e "local_node") test-e2e-forked: (test-e2e "forked_node") # Run End-to-end tests with custom filters -test-e2e *filters: - cargo nextest run -p e2e '{{filters}}' --test-threads 1 --failure-output final --run-ignored ignored-only +test-e2e filters="" *extra="": + cargo nextest run -p e2e '{{filters}}' --test-threads 1 --failure-output final --run-ignored ignored-only {{extra}} test-driver: RUST_MIN_STACK=3145728 cargo nextest run -p driver --test-threads 1 --run-ignored ignored-only diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs index cbdb499958..283020425c 100644 --- a/crates/driver/src/domain/quote.rs +++ b/crates/driver/src/domain/quote.rs @@ -93,8 +93,8 @@ impl Order { liquidity .fetch(&pairs, infra::liquidity::AtBlock::Recent) .await - }, - _ => Default::default(), + } + _ => Default::default(), }; let auction = self @@ -227,7 +227,8 @@ impl Order { /// Returns the token pairs to fetch liquidity for. fn liquidity_pairs(&self) -> Option> { liquidity::TokenPair::try_new(self.tokens.sell(), self.tokens.buy()) - .map(|pair| iter::once(pair).collect()).ok() + .map(|pair| iter::once(pair).collect()) + .ok() } } diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index 5484cfe986..cf8437ebea 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -2,12 +2,19 @@ use { ::alloy::primitives::U256, driver::domain::eth::NonZeroU256, e2e::{nodes::local_node::TestNodeApi, setup::*}, - ethrpc::alloy::{CallBuilderExt, conversions::IntoAlloy}, + ethrpc::alloy::{ + CallBuilderExt, + conversions::{IntoAlloy, IntoLegacy}, + }, model::{ order::{OrderCreation, OrderKind}, quote::{OrderQuoteRequest, OrderQuoteSide, SellAmount}, signature::EcdsaSigningScheme, - }, secp256k1::SecretKey, shared::ethrpc::Web3, std::ops::DerefMut, web3::signing::SecretKeyRef + }, + secp256k1::SecretKey, + shared::ethrpc::Web3, + std::ops::DerefMut, + web3::signing::SecretKeyRef, }; #[tokio::test] @@ -124,23 +131,8 @@ async fn place_order_with_quote_same_token_pair(web3: Web3) { .await; token.mint(trader.address(), to_wei(10).into_alloy()).await; - token - .approve(onchain.contracts().allowance.into_alloy(), eth(3)) - .from(trader.address()) - .send_and_watch() - .await - .unwrap(); - onchain - .contracts() - .weth - .deposit() - .from(trader.address()) - .value(eth(3)) - .send_and_watch() - .await - .unwrap(); - token.approve(onchain.contracts().allowance.into_alloy(), eth(10)) + .approve(onchain.contracts().allowance.into_alloy(), eth(10)) .from(trader.address()) .send_and_watch() .await diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index cc9fd2f761..d273c48f4b 100644 --- a/crates/orderbook/src/run.rs +++ b/crates/orderbook/src/run.rs @@ -402,7 +402,6 @@ pub async fn run(args: Arguments) { .await .ok(); let order_validator = Arc::new(OrderValidator::new( - native_token.clone(), Arc::new(order_validation::banned::Users::new( chainalysis_oracle, args.banned_users, diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 1441bc73b4..01bc86f096 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -17,15 +17,7 @@ use { }, signature_validator::{SignatureCheck, SignatureValidating, SignatureValidationError}, trade_finding, - }, - alloy::primitives::Address, - anyhow::{Result, anyhow}, - app_data::{AppDataHash, Hook, Hooks, ValidatedAppData, Validator}, - async_trait::async_trait, - contracts::alloy::{HooksTrampoline, WETH9}, - ethcontract::{H160, H256, U256}, - ethrpc::alloy::conversions::{IntoAlloy, IntoLegacy}, - model::{ + }, alloy::primitives::Address, anyhow::{Result, anyhow}, app_data::{AppDataHash, Hook, Hooks, ValidatedAppData, Validator}, async_trait::async_trait, contracts::alloy::HooksTrampoline, ethcontract::{H160, H256, U256}, ethrpc::alloy::conversions::{IntoAlloy, IntoLegacy}, model::{ DomainSeparator, interaction::InteractionData, order::{ @@ -46,9 +38,7 @@ use { quote::{OrderQuoteSide, QuoteSigningScheme, SellAmount}, signature::{self, Signature, SigningScheme, hashed_eip712_message}, time, - }, - std::{sync::Arc, time::Duration}, - tracing::instrument, + }, std::{sync::Arc, time::Duration}, tracing::instrument }; #[cfg_attr(any(test, feature = "test-util"), mockall::automock)] @@ -215,9 +205,6 @@ pub trait LimitOrderCounting: Send + Sync { #[derive(Clone)] pub struct OrderValidator { - /// For Pre/Partial-Validation: performed during fee & quote phase - /// when only part of the order data is available - native_token: WETH9::Instance, banned_users: Arc, validity_configuration: OrderValidPeriodConfiguration, eip1271_skip_creation_validation: bool, @@ -285,7 +272,6 @@ pub struct OrderAppData { impl OrderValidator { #[expect(clippy::too_many_arguments)] pub fn new( - native_token: WETH9::Instance, banned_users: Arc, validity_configuration: OrderValidPeriodConfiguration, eip1271_skip_creation_validation: bool, @@ -301,7 +287,6 @@ impl OrderValidator { max_gas_per_order: u64, ) -> Self { Self { - native_token, banned_users, validity_configuration, eip1271_skip_creation_validation, @@ -845,14 +830,6 @@ pub enum OrderValidToError { Excessive, } -/// Returns true if the orders have same buy and sell tokens. -/// -/// This also checks for orders selling wrapped native token for native token. -fn has_same_buy_and_sell_token(order: &PreOrderData, native_token: &Address) -> bool { - order.sell_token == order.buy_token - || (order.sell_token == *native_token && order.buy_token == BUY_ETH_ADDRESS) -} - /// Retrieves the quote for an order that is being created and verify that its /// fee is sufficient. /// @@ -1035,49 +1012,8 @@ mod tests { std::str::FromStr, }; - #[test] - fn detects_orders_with_same_buy_and_sell_token() { - let native_token = Address::repeat_byte(0xef); - assert!(has_same_buy_and_sell_token( - &PreOrderData { - sell_token: Address::repeat_byte(0x01), - buy_token: Address::repeat_byte(0x01), - ..Default::default() - }, - &native_token, - )); - assert!(has_same_buy_and_sell_token( - &PreOrderData { - sell_token: native_token, - buy_token: BUY_ETH_ADDRESS, - ..Default::default() - }, - &native_token, - )); - - assert!(!has_same_buy_and_sell_token( - &PreOrderData { - sell_token: Address::repeat_byte(0x01), - buy_token: Address::repeat_byte(0x02), - ..Default::default() - }, - &native_token, - )); - // Sell token set to 0xeee...eee has no special meaning, so it isn't - // considered buying and selling the same token. - assert!(!has_same_buy_and_sell_token( - &PreOrderData { - sell_token: BUY_ETH_ADDRESS, - buy_token: native_token, - ..Default::default() - }, - &native_token, - )); - } - #[tokio::test] async fn pre_validate_err() { - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validity_configuration = OrderValidPeriodConfiguration { min: Duration::from_secs(1), max_market: Duration::from_secs(100), @@ -1089,7 +1025,6 @@ mod tests { let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::from_set(banned_users)), validity_configuration, false, @@ -1229,9 +1164,7 @@ mod tests { let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), validity_configuration, false, @@ -1330,9 +1263,7 @@ mod tests { let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration { min: Duration::from_secs(1), @@ -1541,9 +1472,7 @@ mod tests { .expect_count() .returning(|_| Ok(MAX_LIMIT_ORDERS_PER_USER)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration { min: Duration::from_secs(1), @@ -1622,9 +1551,7 @@ mod tests { .expect_count() .returning(|_| Ok(MAX_LIMIT_ORDERS_PER_USER)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1686,9 +1613,7 @@ mod tests { .returning(|_, _| Ok(())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1743,9 +1668,7 @@ mod tests { .returning(|_, _| Ok(())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1803,9 +1726,7 @@ mod tests { .returning(|_, _| Ok(())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1866,9 +1787,7 @@ mod tests { .returning(|_, _| Err(TransferSimulationError::InsufficientBalance)); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1928,9 +1847,7 @@ mod tests { .returning(|_| Err(SignatureValidationError::Invalid)); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1997,9 +1914,7 @@ mod tests { .returning(move |_, _| Err(create_error())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -2088,9 +2003,7 @@ mod tests { .returning(|_, _| Err(TransferSimulationError::InsufficientBalance)); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -2502,9 +2415,7 @@ mod tests { .returning(|_| Ok(default_verification_gas_limit())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( - native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration { min: Duration::from_secs(1), From ae7d14b423df8136891fca4772bb0b738635b7ef Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 12 Nov 2025 16:52:07 +0100 Subject: [PATCH 04/39] cleanup comments --- crates/e2e/tests/e2e/place_order_with_quote.rs | 3 ++- crates/solvers/src/domain/solver.rs | 14 +++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index cf8437ebea..115a086040 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -130,7 +130,8 @@ async fn place_order_with_quote_same_token_pair(web3: Web3) { .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) .await; - token.mint(trader.address(), to_wei(10).into_alloy()).await; + token.mint(trader.address(), to_wei(10)).await; + token .approve(onchain.contracts().allowance.into_alloy(), eth(10)) .from(trader.address()) diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs index ad0458bb91..4426f88554 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -207,13 +207,6 @@ impl Inner { }) .collect(); - // The baseline solver generates a path with swapping - // for exact output token amounts. This leads to - // potential rounding errors for buy orders, where we - // can buy slightly more than intended. Fix this by - // capping the output amount to the order's buy amount - // for buy orders. - // let mut output = route.output(); let mut output = if route.is_empty() { order.sell } else { @@ -224,6 +217,13 @@ impl Inner { } else { route.input() }; + + // The baseline solver generates a path with swapping + // for exact output token amounts. This leads to + // potential rounding errors for buy orders, where we + // can buy slightly more than intended. Fix this by + // capping the output amount to the order's buy amount + // for buy orders. if let order::Side::Buy = order.side { output.amount = cmp::min(output.amount, order.buy.amount); } From ff4e678df6cb313937e18226d409761a0be2fdf3 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 14 Nov 2025 16:09:53 +0100 Subject: [PATCH 05/39] Feature flag for sell=buy --- .../e2e/tests/e2e/place_order_with_quote.rs | 62 ++++++++++- crates/orderbook/src/api/post_order.rs | 7 ++ crates/orderbook/src/arguments.rs | 9 ++ crates/orderbook/src/run.rs | 43 ++++---- crates/shared/src/order_validation.rs | 104 +++++++++++++++--- 5 files changed, 187 insertions(+), 38 deletions(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index 115a086040..57e9594dd6 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -29,6 +29,12 @@ async fn local_node_place_order_with_quote_same_token_pair() { run_test(place_order_with_quote_same_token_pair).await; } +#[tokio::test] +#[ignore] +async fn local_node_place_order_with_quote_same_token_pair_error() { + run_test(place_order_with_quote_same_token_pair_error).await; +} + async fn place_order_with_quote(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3.clone()).await; @@ -121,7 +127,7 @@ async fn place_order_with_quote(web3: Web3) { assert_eq!(quote_metadata.unwrap().0, order_quote.metadata); } -async fn place_order_with_quote_same_token_pair(web3: Web3) { +async fn place_order_with_quote_same_token_pair_error(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3.clone()).await; let [solver] = onchain.make_solvers(to_wei(10).into_alloy()).await; @@ -130,7 +136,7 @@ async fn place_order_with_quote_same_token_pair(web3: Web3) { .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) .await; - token.mint(trader.address(), to_wei(10)).await; + token.mint(trader.address(), eth(10)).await; token .approve(onchain.contracts().allowance.into_alloy(), eth(10)) @@ -143,6 +149,58 @@ async fn place_order_with_quote_same_token_pair(web3: Web3) { let services = Services::new(&onchain).await; services.start_protocol(solver.clone()).await; + // Disable auto-mine so we don't accidentally mine a settlement + web3.api::>() + .set_automine_enabled(false) + .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() + }; + assert!(services.submit_quote("e_request).await.is_err()); +} + +async fn place_order_with_quote_same_token_pair(web3: Web3) { + let mut onchain = OnchainComponents::deploy(web3.clone()).await; + + let [solver] = onchain.make_solvers(eth(10)).await; + let [trader] = onchain.make_accounts(eth(10)).await; + let [token] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) + .await; + + token.mint(trader.address(), eth(10)).await; + + token + .approve(onchain.contracts().allowance.into_alloy(), eth(10)) + .from(trader.address()) + .send_and_watch() + .await + .unwrap(); + + tracing::info!("Starting services."); + let services = Services::new(&onchain).await; + services + .start_protocol_with_args( + ExtraServiceArgs { + api: vec!["--allow-same-sell-and-buy-token=true".to_string()], + ..Default::default() + }, + solver.clone(), + ) + .await; + // Disable auto-mine so we don't accidentally mine a settlement web3.api::>() .set_automine_enabled(false) diff --git a/crates/orderbook/src/api/post_order.rs b/crates/orderbook/src/api/post_order.rs index d278b009e8..7a57ac4ed4 100644 --- a/crates/orderbook/src/api/post_order.rs +++ b/crates/orderbook/src/api/post_order.rs @@ -72,6 +72,13 @@ impl IntoWarpReply for PartialValidationErrorWrapper { ), StatusCode::BAD_REQUEST, ), + PartialValidationError::SameBuyAndSellToken => with_status( + error( + "SameBuyAndSellToken", + "Buy token is the same as the sell token.", + ), + StatusCode::BAD_REQUEST, + ), PartialValidationError::UnsupportedToken { token, reason } => with_status( error( "UnsupportedToken", diff --git a/crates/orderbook/src/arguments.rs b/crates/orderbook/src/arguments.rs index e47ba3eb6d..fc88e8c5ae 100644 --- a/crates/orderbook/src/arguments.rs +++ b/crates/orderbook/src/arguments.rs @@ -145,6 +145,10 @@ pub struct Arguments { #[clap(flatten)] pub volume_fee_config: Option, + + /// Allow same sell and buy token + #[clap(long, env, action = clap::ArgAction::Set, default_value = "false")] + pub allow_same_sell_and_buy_token: bool, } /// Volume-based protocol fee factor to be applied to quotes. @@ -234,6 +238,7 @@ impl std::fmt::Display for Arguments { max_gas_per_order, active_order_competition_threshold, volume_fee_config, + allow_same_sell_and_buy_token, } = self; write!(f, "{shared}")?; @@ -288,6 +293,10 @@ impl std::fmt::Display for Arguments { "active_order_competition_threshold: {active_order_competition_threshold}" )?; writeln!(f, "volume_fee_config: {volume_fee_config:?}")?; + writeln!( + f, + "allow_same_sell_and_buy_token: {allow_same_sell_and_buy_token}" + )?; Ok(()) } diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index d273c48f4b..64ddcfd3ce 100644 --- a/crates/orderbook/src/run.rs +++ b/crates/orderbook/src/run.rs @@ -401,25 +401,30 @@ pub async fn run(args: Arguments) { let chainalysis_oracle = ChainalysisOracle::Instance::deployed(&web3.alloy) .await .ok(); - let order_validator = Arc::new(OrderValidator::new( - Arc::new(order_validation::banned::Users::new( - chainalysis_oracle, - args.banned_users, - args.banned_users_max_cache_size.get().to_u64().unwrap(), - )), - validity_configuration, - args.eip1271_skip_creation_validation, - bad_token_detector.clone(), - hooks_contract, - optimal_quoter.clone(), - balance_fetcher, - signature_validator, - Arc::new(postgres_write.clone()), - args.max_limit_orders_per_user, - code_fetcher, - app_data_validator.clone(), - args.max_gas_per_order, - )); + let order_validator = Arc::new( + OrderValidator::new( + Arc::new(order_validation::banned::Users::new( + chainalysis_oracle, + args.banned_users, + args.banned_users_max_cache_size.get().to_u64().unwrap(), + )), + validity_configuration, + args.eip1271_skip_creation_validation, + bad_token_detector.clone(), + hooks_contract, + optimal_quoter.clone(), + balance_fetcher, + signature_validator, + Arc::new(postgres_write.clone()), + args.max_limit_orders_per_user, + code_fetcher, + app_data_validator.clone(), + args.max_gas_per_order, + ) + .with_sell_and_buy_validation( + (!args.allow_same_sell_and_buy_token).then(|| native_token.clone()), + ), + ); let ipfs = args .ipfs_gateway .map(|url| { diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 01bc86f096..d808958da0 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -17,7 +17,15 @@ use { }, signature_validator::{SignatureCheck, SignatureValidating, SignatureValidationError}, trade_finding, - }, alloy::primitives::Address, anyhow::{Result, anyhow}, app_data::{AppDataHash, Hook, Hooks, ValidatedAppData, Validator}, async_trait::async_trait, contracts::alloy::HooksTrampoline, ethcontract::{H160, H256, U256}, ethrpc::alloy::conversions::{IntoAlloy, IntoLegacy}, model::{ + }, + alloy::primitives::Address, + anyhow::{Result, anyhow}, + app_data::{AppDataHash, Hook, Hooks, ValidatedAppData, Validator}, + async_trait::async_trait, + contracts::alloy::{HooksTrampoline, WETH9}, + ethcontract::{H160, H256, U256}, + ethrpc::alloy::conversions::{IntoAlloy, IntoLegacy}, + model::{ DomainSeparator, interaction::InteractionData, order::{ @@ -99,6 +107,7 @@ pub enum PartialValidationError { Forbidden, ValidTo(OrderValidToError), InvalidNativeSellToken, + SameBuyAndSellToken, UnsupportedBuyTokenDestination(BuyTokenDestination), UnsupportedSellTokenSource(SellTokenSource), UnsupportedOrderType, @@ -205,6 +214,9 @@ pub trait LimitOrderCounting: Send + Sync { #[derive(Clone)] pub struct OrderValidator { + /// For Pre/Partial-Validation: performed during fee & quote phase + /// when only part of the order data is available + native_token: Option, banned_users: Arc, validity_configuration: OrderValidPeriodConfiguration, eip1271_skip_creation_validation: bool, @@ -287,6 +299,7 @@ impl OrderValidator { max_gas_per_order: u64, ) -> Self { Self { + native_token: None, banned_users, validity_configuration, eip1271_skip_creation_validation, @@ -303,6 +316,13 @@ impl OrderValidator { } } + /// Enables sell and buy validation if Some is provided + /// Disables it if None + pub fn with_sell_and_buy_validation(mut self, native_token: Option) -> Self { + self.native_token = native_token; + self + } + async fn check_max_limit_orders(&self, owner: Address) -> Result<(), ValidationError> { let num_limit_orders = self .limit_order_counter @@ -464,7 +484,11 @@ impl OrderValidating for OrderValidator { } self.validity_configuration.validate_period(&order)?; - + if let Some(native_token) = &self.native_token + && has_same_buy_and_sell_token(&order, native_token.address()) + { + return Err(PartialValidationError::SameBuyAndSellToken); + } if order.sell_token == BUY_ETH_ADDRESS { return Err(PartialValidationError::InvalidNativeSellToken); } @@ -830,6 +854,15 @@ pub enum OrderValidToError { Excessive, } +/// Returns true if the orders have same buy and sell tokens. +/// +/// This also checks for orders selling wrapped native token for native token. +fn has_same_buy_and_sell_token(order: &PreOrderData, native_token: &Address) -> bool { + order.sell_token == order.buy_token + || (order.sell_token == *native_token + && order.buy_token == BUY_ETH_ADDRESS) +} + /// Retrieves the quote for an order that is being created and verify that its /// fee is sufficient. /// @@ -996,7 +1029,7 @@ mod tests { signature_validator::MockSignatureValidating, }, alloy::{ - primitives::{Address, U160, address}, + primitives::{Address, B160, U160, address}, providers::{Provider, ProviderBuilder, mock::Asserter}, }, ethcontract::web3::signing::SecretKeyRef, @@ -1014,6 +1047,7 @@ mod tests { #[tokio::test] async fn pre_validate_err() { + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validity_configuration = OrderValidPeriodConfiguration { min: Duration::from_secs(1), max_market: Duration::from_secs(100), @@ -1043,7 +1077,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let result = validator .partial_validate(PreOrderData { partially_fillable: true, @@ -1132,6 +1167,17 @@ mod tests { OrderValidToError::Excessive, )) )); + assert!(matches!( + validator + .partial_validate(PreOrderData { + valid_to: legit_valid_to, + buy_token: Address::with_last_byte(2), + sell_token: Address::with_last_byte(2), + ..Default::default() + }) + .await, + Err(PartialValidationError::SameBuyAndSellToken) + )); assert!(matches!( validator .partial_validate(PreOrderData { @@ -1146,6 +1192,7 @@ mod tests { #[tokio::test] async fn pre_validate_ok() { + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validity_configuration = OrderValidPeriodConfiguration { min: Duration::from_secs(1), max_market: Duration::from_secs(100), @@ -1183,7 +1230,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let order = || PreOrderData { valid_to: time::now_in_epoch_seconds() + validity_configuration.min.as_secs() as u32 @@ -1263,6 +1311,7 @@ mod tests { let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration { @@ -1281,7 +1330,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -1472,6 +1522,7 @@ mod tests { .expect_count() .returning(|_| Ok(MAX_LIMIT_ORDERS_PER_USER)); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration { @@ -1495,7 +1546,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let creation = OrderCreation { valid_to: model::time::now_in_epoch_seconds() + 2, @@ -1550,7 +1602,7 @@ mod tests { limit_order_counter .expect_count() .returning(|_| Ok(MAX_LIMIT_ORDERS_PER_USER)); - + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), @@ -1570,7 +1622,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let creation = OrderCreation { valid_to: model::time::now_in_epoch_seconds() + 2, @@ -1613,6 +1666,7 @@ mod tests { .returning(|_, _| Ok(())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), @@ -1632,7 +1686,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1668,6 +1723,7 @@ mod tests { .returning(|_, _| Ok(())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), @@ -1687,7 +1743,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1726,6 +1783,8 @@ mod tests { .returning(|_, _| Ok(())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); + + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), @@ -1745,7 +1804,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1787,6 +1847,7 @@ mod tests { .returning(|_, _| Err(TransferSimulationError::InsufficientBalance)); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), @@ -1806,7 +1867,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1847,6 +1909,7 @@ mod tests { .returning(|_| Err(SignatureValidationError::Invalid)); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), @@ -1866,7 +1929,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -1914,6 +1978,7 @@ mod tests { .returning(move |_, _| Err(create_error())); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), @@ -1933,7 +1998,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let order = OrderCreation { valid_to: u32::MAX, @@ -2003,6 +2069,7 @@ mod tests { .returning(|_, _| Err(TransferSimulationError::InsufficientBalance)); let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), @@ -2022,7 +2089,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); // Test with flashloan hint that covers the sell amount let order_with_sufficient_flashloan = OrderCreation { @@ -2414,6 +2482,7 @@ mod tests { .expect_validate_signature_and_get_additional_gas() .returning(|_| Ok(default_verification_gas_limit())); let mut limit_order_counter = MockLimitOrderCounting::new(); + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); limit_order_counter.expect_count().returning(|_| Ok(0u64)); let validator = OrderValidator::new( Arc::new(order_validation::banned::Users::none()), @@ -2438,7 +2507,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ); + ) + .with_sell_and_buy_validation(Some(native_token)); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 10, From 8893b86f68540eca93571eebd56debfd63756c50 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 2 Dec 2025 11:40:44 +0100 Subject: [PATCH 06/39] Rename liquidity_pairs to token_liquidity --- crates/driver/src/domain/quote.rs | 4 ++-- crates/shared/src/order_validation.rs | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs index 283020425c..f23cd6dede 100644 --- a/crates/driver/src/domain/quote.rs +++ b/crates/driver/src/domain/quote.rs @@ -88,7 +88,7 @@ impl Order { liquidity: &infra::liquidity::Fetcher, tokens: &infra::tokens::Fetcher, ) -> Result { - let liquidity = match (solver.liquidity(), self.liquidity_pairs()) { + let liquidity = match (solver.liquidity(), self.token_liquidity()) { (solver::Liquidity::Fetch, Some(pairs)) => { liquidity .fetch(&pairs, infra::liquidity::AtBlock::Recent) @@ -225,7 +225,7 @@ impl Order { } /// Returns the token pairs to fetch liquidity for. - fn liquidity_pairs(&self) -> Option> { + fn token_liquidity(&self) -> Option> { liquidity::TokenPair::try_new(self.tokens.sell(), self.tokens.buy()) .map(|pair| iter::once(pair).collect()) .ok() diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index d808958da0..d476cdeb0a 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -46,7 +46,9 @@ use { quote::{OrderQuoteSide, QuoteSigningScheme, SellAmount}, signature::{self, Signature, SigningScheme, hashed_eip712_message}, time, - }, std::{sync::Arc, time::Duration}, tracing::instrument + }, + std::{sync::Arc, time::Duration}, + tracing::instrument, }; #[cfg_attr(any(test, feature = "test-util"), mockall::automock)] @@ -859,8 +861,7 @@ pub enum OrderValidToError { /// This also checks for orders selling wrapped native token for native token. fn has_same_buy_and_sell_token(order: &PreOrderData, native_token: &Address) -> bool { order.sell_token == order.buy_token - || (order.sell_token == *native_token - && order.buy_token == BUY_ETH_ADDRESS) + || (order.sell_token == *native_token && order.buy_token == BUY_ETH_ADDRESS) } /// Retrieves the quote for an order that is being created and verify that its @@ -1029,7 +1030,7 @@ mod tests { signature_validator::MockSignatureValidating, }, alloy::{ - primitives::{Address, B160, U160, address}, + primitives::{Address, U160, address}, providers::{Provider, ProviderBuilder, mock::Asserter}, }, ethcontract::web3::signing::SecretKeyRef, From bb1fb95024216d0459e17e5136e03d58dc80c1d0 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 3 Dec 2025 13:41:59 +0100 Subject: [PATCH 07/39] Refactor Route and sell=buy validation control --- crates/driver/src/domain/quote.rs | 12 +- .../e2e/tests/e2e/place_order_with_quote.rs | 6 +- crates/e2e/tests/e2e/submission.rs | 238 +++++++++++++++++- crates/orderbook/src/run.rs | 5 +- crates/shared/src/order_validation.rs | 153 ++++++++--- crates/solvers/src/boundary/baseline.rs | 17 +- crates/solvers/src/domain/solver.rs | 92 ++++--- 7 files changed, 437 insertions(+), 86 deletions(-) diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs index f23cd6dede..18a74a283e 100644 --- a/crates/driver/src/domain/quote.rs +++ b/crates/driver/src/domain/quote.rs @@ -16,10 +16,7 @@ use { util, }, chrono::Utc, - std::{ - collections::{HashMap, HashSet}, - iter, - }, + std::collections::{HashMap, HashSet}, }; /// A quote describing the expected outcome of an order. @@ -89,7 +86,7 @@ impl Order { tokens: &infra::tokens::Fetcher, ) -> Result { let liquidity = match (solver.liquidity(), self.token_liquidity()) { - (solver::Liquidity::Fetch, Some(pairs)) => { + (solver::Liquidity::Fetch, pairs) if !pairs.is_empty() => { liquidity .fetch(&pairs, infra::liquidity::AtBlock::Recent) .await @@ -225,10 +222,11 @@ impl Order { } /// Returns the token pairs to fetch liquidity for. - fn token_liquidity(&self) -> Option> { + fn token_liquidity(&self) -> HashSet { liquidity::TokenPair::try_new(self.tokens.sell(), self.tokens.buy()) - .map(|pair| iter::once(pair).collect()) .ok() + .into_iter() + .collect() } } diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index 57e9594dd6..2b96a2613c 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -4,7 +4,7 @@ use { e2e::{nodes::local_node::TestNodeApi, setup::*}, ethrpc::alloy::{ CallBuilderExt, - conversions::{IntoAlloy, IntoLegacy}, + conversions::IntoAlloy, }, model::{ order::{OrderCreation, OrderKind}, @@ -168,7 +168,9 @@ async fn place_order_with_quote_same_token_pair_error(web3: Web3) { }, ..Default::default() }; - assert!(services.submit_quote("e_request).await.is_err()); + assert!( + matches!(services.submit_quote("e_request).await, Err((reqwest::StatusCode::BAD_REQUEST, response)) if response.contains("SameBuyAndSellToken")) + ); } async fn place_order_with_quote_same_token_pair(web3: Web3) { diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index 41d6f18acf..e12686df17 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -2,7 +2,7 @@ use { ::alloy::primitives::U256, e2e::{nodes::local_node::TestNodeApi, setup::*}, ethcontract::{BlockId, H160, H256}, - ethrpc::alloy::{CallBuilderExt, conversions::IntoAlloy}, + ethrpc::alloy::{CallBuilderExt, conversions::{IntoAlloy, IntoLegacy}}, futures::{Stream, StreamExt}, model::{ order::{OrderCreation, OrderKind}, @@ -16,10 +16,28 @@ use { #[tokio::test] #[ignore] -async fn local_node_test() { +async fn local_node_on_expiry() { run_test(test_cancel_on_expiry).await; } +#[tokio::test] +#[ignore] +async fn local_node_execute_same_sell_and_buy_token() { + run_test(test_execute_same_sell_and_buy_token).await; +} + +#[tokio::test] +#[ignore] +async fn local_node_submit_same_sell_and_buy_token_order_without_quote() { + run_test(test_submit_same_sell_and_buy_token_order_without_quote).await; +} + +#[tokio::test] +#[ignore] +async fn local_node_submit_same_sell_and_buy_token_order_without_quote_fail() { + run_test(test_submit_same_sell_and_buy_token_order_without_quote_fail).await; +} + async fn test_cancel_on_expiry(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3.clone()).await; @@ -120,6 +138,222 @@ async fn test_cancel_on_expiry(web3: Web3) { assert_eq!(tx.to, Some(solver.account().address())) } +async fn test_submit_same_sell_and_buy_token_order_without_quote(web3: Web3) { + let mut onchain = OnchainComponents::deploy(web3.clone()).await; + + let [solver] = onchain.make_solvers(eth(10)).await; + let [trader] = onchain.make_accounts(eth(10)).await; + let [token] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) + .await; + + token.mint(trader.address(), eth(10)).await; + + token + .approve(onchain.contracts().allowance.into_alloy(), eth(10)) + .from(trader.address()) + .send_and_watch() + .await + .unwrap(); + + tracing::info!("Starting services."); + let services = Services::new(&onchain).await; + services + .start_protocol_with_args( + ExtraServiceArgs { + api: vec!["--allow-same-sell-and-buy-token=true".to_string()], + ..Default::default() + }, + solver.clone(), + ) + .await; + + // Disable auto-mine so we don't accidentally mine a settlement + web3.api::>() + .set_automine_enabled(false) + .await + .expect("Must be able to disable automine"); + + tracing::info!("Placing order"); + let order = OrderCreation { + sell_token: *token.address(), + sell_amount: eth(1), + buy_token: *token.address(), + buy_amount: eth(1), + 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.private_key()).unwrap()), + ); + services.create_order(&order).await.unwrap(); +} + +async fn test_submit_same_sell_and_buy_token_order_without_quote_fail(web3: Web3) { + let mut onchain = OnchainComponents::deploy(web3.clone()).await; + + let [solver] = onchain.make_solvers(eth(10)).await; + let [trader] = onchain.make_accounts(eth(10)).await; + let [token] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) + .await; + + token.mint(trader.address(), eth(10)).await; + + token + .approve(onchain.contracts().allowance.into_alloy(), eth(10)) + .from(trader.address()) + .send_and_watch() + .await + .unwrap(); + + tracing::info!("Starting services."); + let services = Services::new(&onchain).await; + services.start_protocol(solver.clone()).await; + + // Disable auto-mine so we don't accidentally mine a settlement + web3.api::>() + .set_automine_enabled(false) + .await + .expect("Must be able to disable automine"); + + tracing::info!("Placing order"); + let order = OrderCreation { + sell_token: *token.address(), + sell_amount: eth(1), + buy_token: *token.address(), + buy_amount: eth(1), + 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.private_key()).unwrap()), + ); + // services.create_order(&order).await.unwrap(); + assert!( + matches!(services.create_order(&order).await, Err((reqwest::StatusCode::BAD_REQUEST, response)) if response.contains("SameBuyAndSellToken")) + ); +} + +async fn test_execute_same_sell_and_buy_token(web3: Web3) { + let mut onchain = OnchainComponents::deploy(web3.clone()).await; + + let [solver] = onchain.make_solvers(eth(10)).await; + let [trader] = onchain.make_accounts(eth(10)).await; + let [token] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) + .await; + + token.mint(trader.address(), eth(10)).await; + + token + .approve(onchain.contracts().allowance.into_alloy(), eth(10)) + .from(trader.address()) + .send_and_watch() + .await + .unwrap(); + + tracing::info!("Starting services."); + let services = Services::new(&onchain).await; + services + .start_protocol_with_args( + ExtraServiceArgs { + api: vec!["--allow-same-sell-and-buy-token=true".to_string()], + ..Default::default() + }, + solver.clone(), + ) + .await; + + // Disable auto-mine so we don't accidentally mine a settlement + web3.api::>() + .set_automine_enabled(false) + .await + .expect("Must be able to disable automine"); + + tracing::info!("Placing order"); + let initial_balance = token.balanceOf(trader.address()).call().await.unwrap(); + assert_eq!(initial_balance, eth(10)); + + let order_sell_amount: ::alloy::primitives::Uint<256, 4> = eth(2); + let order_buy_amount = eth(1); + let order = OrderCreation { + sell_token: *token.address(), + sell_amount: order_sell_amount, + buy_token: *token.address(), + buy_amount: order_buy_amount, + 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.private_key()).unwrap()), + ); + assert!(services.create_order(&order).await.is_ok()); + + // Start tracking confirmed blocks so we can find the transaction later + let block_stream = web3 + .eth_filter() + .create_blocks_filter() + .await + .expect("must be able to create blocks filter") + .stream(Duration::from_millis(50)); + + tracing::info!("Waiting for trade."); + onchain.mint_block().await; + + // Wait for settlement tx to appear in txpool + wait_for_condition(Duration::from_secs(2), || async { + get_pending_tx(solver.account().address(), &web3) + .await + .is_some() + }) + .await + .unwrap(); + + // Continue mining to confirm the settlement + web3.api::>() + .set_automine_enabled(true) + .await + .expect("Must be able to enable automine"); + + // Wait for the settlement to be confirmed on chain + let tx = tokio::time::timeout( + Duration::from_secs(5), + get_confirmed_transaction(solver.account().address(), &web3, block_stream), + ) + .await + .unwrap(); + + // Verify the transaction is to the settlement contract (not a cancellation) + assert_eq!( + tx.to, + Some(onchain.contracts().gp_settlement.address().into_legacy()) + ); + + // Verify that the balance changed (settlement happened on chain) + let trade_happened = || async { + let balance = token.balanceOf(trader.address()).call().await.unwrap(); + // Balance should change due to fees even if sell token == buy token + balance != initial_balance + }; + wait_for_condition(TIMEOUT, trade_happened).await.unwrap(); + + let final_balance = token.balanceOf(trader.address()).call().await.unwrap(); + tracing::info!(?initial_balance, ?final_balance, "Trade completed"); + + // Verify that the balance changed (settlement happened on chain) + assert_ne!(final_balance, initial_balance); +} + async fn get_pending_tx(account: H160, web3: &Web3) -> Option { let txpool = web3 .txpool() diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index 64ddcfd3ce..f209111813 100644 --- a/crates/orderbook/src/run.rs +++ b/crates/orderbook/src/run.rs @@ -403,6 +403,7 @@ pub async fn run(args: Arguments) { .ok(); let order_validator = Arc::new( OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::new( chainalysis_oracle, args.banned_users, @@ -421,9 +422,7 @@ pub async fn run(args: Arguments) { app_data_validator.clone(), args.max_gas_per_order, ) - .with_sell_and_buy_validation( - (!args.allow_same_sell_and_buy_token).then(|| native_token.clone()), - ), + .with_allow_same_sell_and_buy_token(args.allow_same_sell_and_buy_token), ); let ipfs = args .ipfs_gateway diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index d476cdeb0a..8c19b27b57 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -218,7 +218,7 @@ pub trait LimitOrderCounting: Send + Sync { pub struct OrderValidator { /// For Pre/Partial-Validation: performed during fee & quote phase /// when only part of the order data is available - native_token: Option, + native_token: WETH9::Instance, banned_users: Arc, validity_configuration: OrderValidPeriodConfiguration, eip1271_skip_creation_validation: bool, @@ -233,6 +233,7 @@ pub struct OrderValidator { pub code_fetcher: Arc, app_data_validator: Validator, max_gas_per_order: u64, + allow_same_sell_and_buy_token: bool, } #[derive(Debug, Eq, PartialEq, Default)] @@ -286,6 +287,7 @@ pub struct OrderAppData { impl OrderValidator { #[expect(clippy::too_many_arguments)] pub fn new( + native_token: WETH9::Instance, banned_users: Arc, validity_configuration: OrderValidPeriodConfiguration, eip1271_skip_creation_validation: bool, @@ -301,7 +303,7 @@ impl OrderValidator { max_gas_per_order: u64, ) -> Self { Self { - native_token: None, + native_token, banned_users, validity_configuration, eip1271_skip_creation_validation, @@ -315,13 +317,17 @@ impl OrderValidator { code_fetcher, app_data_validator, max_gas_per_order, + allow_same_sell_and_buy_token: false, } } /// Enables sell and buy validation if Some is provided /// Disables it if None - pub fn with_sell_and_buy_validation(mut self, native_token: Option) -> Self { - self.native_token = native_token; + pub fn with_allow_same_sell_and_buy_token( + mut self, + allow_same_sell_and_buy_token: bool, + ) -> Self { + self.allow_same_sell_and_buy_token = allow_same_sell_and_buy_token; self } @@ -486,11 +492,16 @@ impl OrderValidating for OrderValidator { } self.validity_configuration.validate_period(&order)?; - if let Some(native_token) = &self.native_token - && has_same_buy_and_sell_token(&order, native_token.address()) - { + + if !self.allow_same_sell_and_buy_token && order.sell_token == order.buy_token { + return Err(PartialValidationError::SameBuyAndSellToken); + } + + // Check for orders selling wrapped native token for native token. + if &order.sell_token == self.native_token.address() && order.buy_token == BUY_ETH_ADDRESS { return Err(PartialValidationError::SameBuyAndSellToken); } + if order.sell_token == BUY_ETH_ADDRESS { return Err(PartialValidationError::InvalidNativeSellToken); } @@ -856,14 +867,6 @@ pub enum OrderValidToError { Excessive, } -/// Returns true if the orders have same buy and sell tokens. -/// -/// This also checks for orders selling wrapped native token for native token. -fn has_same_buy_and_sell_token(order: &PreOrderData, native_token: &Address) -> bool { - order.sell_token == order.buy_token - || (order.sell_token == *native_token && order.buy_token == BUY_ETH_ADDRESS) -} - /// Retrieves the quote for an order that is being created and verify that its /// fee is sufficient. /// @@ -1060,6 +1063,7 @@ mod tests { let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::from_set(banned_users)), validity_configuration, false, @@ -1079,7 +1083,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let result = validator .partial_validate(PreOrderData { partially_fillable: true, @@ -1213,6 +1217,7 @@ mod tests { let mut limit_order_counter = MockLimitOrderCounting::new(); limit_order_counter.expect_count().returning(|_| Ok(0u64)); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), validity_configuration, false, @@ -1232,7 +1237,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let order = || PreOrderData { valid_to: time::now_in_epoch_seconds() + validity_configuration.min.as_secs() as u32 @@ -1280,6 +1285,84 @@ mod tests { ); } + #[tokio::test] + async fn pre_validate_allow_same_sell_and_buy_token() { + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); + let validity_configuration = OrderValidPeriodConfiguration { + min: Duration::from_secs(1), + max_market: Duration::from_secs(100), + max_limit: Duration::from_secs(200), + }; + + let mut bad_token_detector = MockBadTokenDetecting::new(); + bad_token_detector + .expect_detect() + .with(eq(Address::with_last_byte(1))) + .returning(|_| Ok(TokenQuality::Good)); + bad_token_detector + .expect_detect() + .with(eq(Address::with_last_byte(2))) + .returning(|_| Ok(TokenQuality::Good)); + + let mut limit_order_counter = MockLimitOrderCounting::new(); + limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let validator = OrderValidator::new( + native_token.clone(), + Arc::new(order_validation::banned::Users::none()), + validity_configuration, + false, + Arc::new(bad_token_detector), + HooksTrampoline::Instance::new( + Address::from([0xcf; 20]), + ProviderBuilder::new() + .connect_mocked_client(Asserter::new()) + .erased(), + ), + Arc::new(MockOrderQuoting::new()), + Arc::new(MockBalanceFetching::new()), + Arc::new(MockSignatureValidating::new()), + Arc::new(limit_order_counter), + 0, + Arc::new(MockCodeFetching::new()), + Default::default(), + u64::MAX, + ) + .with_allow_same_sell_and_buy_token(true); + + let order = || PreOrderData { + valid_to: time::now_in_epoch_seconds() + + validity_configuration.min.as_secs() as u32 + + 2, + ..Default::default() + }; + + assert!(matches!( + dbg!( + validator + .partial_validate(PreOrderData { + buy_token: BUY_ETH_ADDRESS, + sell_token: *native_token.address(), + ..order() + }) + .await + ), + Err(PartialValidationError::SameBuyAndSellToken) + )); + + assert!( + dbg!( + validator + .partial_validate(PreOrderData { + buy_token: Address::with_last_byte(2), + sell_token: Address::with_last_byte(2), + ..order() + }) + .await + ) + .is_ok() + ); + } + #[tokio::test] async fn post_validate_ok() { let mut order_quoter = MockOrderQuoting::new(); @@ -1314,6 +1397,7 @@ mod tests { limit_order_counter.expect_count().returning(|_| Ok(0u64)); let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration { min: Duration::from_secs(1), @@ -1332,7 +1416,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -1525,6 +1609,7 @@ mod tests { let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration { min: Duration::from_secs(1), @@ -1548,7 +1633,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let creation = OrderCreation { valid_to: model::time::now_in_epoch_seconds() + 2, @@ -1605,6 +1690,7 @@ mod tests { .returning(|_| Ok(MAX_LIMIT_ORDERS_PER_USER)); let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1624,7 +1710,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let creation = OrderCreation { valid_to: model::time::now_in_epoch_seconds() + 2, @@ -1669,6 +1755,7 @@ mod tests { limit_order_counter.expect_count().returning(|_| Ok(0u64)); let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1688,7 +1775,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1726,6 +1813,7 @@ mod tests { limit_order_counter.expect_count().returning(|_| Ok(0u64)); let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1745,7 +1833,8 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); + let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1787,6 +1876,7 @@ mod tests { let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1806,7 +1896,8 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); + let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1850,6 +1941,7 @@ mod tests { limit_order_counter.expect_count().returning(|_| Ok(0u64)); let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1869,7 +1961,8 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); + let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1912,6 +2005,7 @@ mod tests { limit_order_counter.expect_count().returning(|_| Ok(0u64)); let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -1931,7 +2025,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -1981,6 +2075,7 @@ mod tests { limit_order_counter.expect_count().returning(|_| Ok(0u64)); let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -2000,7 +2095,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let order = OrderCreation { valid_to: u32::MAX, @@ -2072,6 +2167,7 @@ mod tests { limit_order_counter.expect_count().returning(|_| Ok(0u64)); let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration::any(), false, @@ -2091,7 +2187,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); // Test with flashloan hint that covers the sell amount let order_with_sufficient_flashloan = OrderCreation { @@ -2486,6 +2582,7 @@ mod tests { let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); limit_order_counter.expect_count().returning(|_| Ok(0u64)); let validator = OrderValidator::new( + native_token, Arc::new(order_validation::banned::Users::none()), OrderValidPeriodConfiguration { min: Duration::from_secs(1), @@ -2509,7 +2606,7 @@ mod tests { Default::default(), u64::MAX, ) - .with_sell_and_buy_validation(Some(native_token)); + .with_allow_same_sell_and_buy_token(false); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 10, diff --git a/crates/solvers/src/boundary/baseline.rs b/crates/solvers/src/boundary/baseline.rs index 0f6e7fb89a..699619baaf 100644 --- a/crates/solvers/src/boundary/baseline.rs +++ b/crates/solvers/src/boundary/baseline.rs @@ -21,6 +21,8 @@ pub struct Solver<'a> { liquidity: HashMap, } +pub struct RouteComputationError; + impl<'a> Solver<'a> { pub fn new( weth: ð::WethAddress, @@ -42,9 +44,10 @@ impl<'a> Solver<'a> { &self, request: solver::Request, max_hops: usize, - ) -> Option> { + ) -> Result>, RouteComputationError> { if request.sell.token == request.buy.token { - return Some(solver::Route::new(vec![])); + tracing::info!("Sell=Buy, returning empty route"); + return Ok(None); } let candidates = self.base_tokens.path_candidates_with_hops( request.sell.token.0, @@ -61,6 +64,7 @@ impl<'a> Solver<'a> { &self.onchain_liquidity, ) .await?; + let segments = self .traverse_path(&sell.path, request.sell.token.0, sell.value) .await?; @@ -81,7 +85,8 @@ impl<'a> Solver<'a> { .await .into_iter() .flatten() - .min_by_key(|(_, sell)| sell.value)? + .min_by_key(|(_, sell)| sell.value) + .ok_or(RouteComputationError)? } order::Side::Sell => { let futures = candidates.iter().map(|path| async { @@ -91,6 +96,7 @@ impl<'a> Solver<'a> { &self.onchain_liquidity, ) .await?; + let segments = self .traverse_path(&buy.path, request.sell.token.0, request.sell.amount) .await?; @@ -111,11 +117,12 @@ impl<'a> Solver<'a> { .await .into_iter() .flatten() - .max_by_key(|(_, buy)| buy.value)? + .max_by_key(|(_, buy)| buy.value) + .ok_or(RouteComputationError)? } }; - Some(solver::Route::new(segments)) + solver::Route::new(segments).map(Ok).transpose() } async fn traverse_path( diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs index 4426f88554..58de9495fd 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -170,7 +170,7 @@ impl Inner { .route(native_price_request, self.max_hops) .await { - Some(route) => { + Ok(Some(route)) => { // how many units of buy_token are bought for one unit of sell_token // (buy_amount / sell_amount). let price = f64::from(self.native_token_price_estimation_amount) @@ -181,6 +181,10 @@ impl Inner { 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 @@ -192,31 +196,49 @@ impl Inner { let compute_solution = async |request: Request| -> Option { let wrappers = request.wrappers.clone(); - let route = boundary_solver.route(request, self.max_hops).await?; - let 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(); - - let mut output = if route.is_empty() { - order.sell - } else { - route.output() - }; - let input = if route.is_empty() { - order.sell - } else { - route.input() - }; + let route = boundary_solver.route(request, self.max_hops).await.ok()?; + 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:?}" + ); + } + } // The baseline solver generates a path with swapping // for exact output token amounts. This leads to @@ -228,7 +250,6 @@ impl Inner { output.amount = cmp::min(output.amount, order.buy.amount); } - let gas = route.gas() + self.solution_gas_offset; let fee = sell_token_price .ether_value(eth::Ether(gas.0.checked_mul(auction.gas_price.0.0)?))? .into(); @@ -365,25 +386,18 @@ pub struct Segment<'a> { } impl<'a> Route<'a> { - pub fn new(segments: Vec>) -> Self { - Self { segments } - } - - fn is_empty(&self) -> bool { - self.segments.is_empty() + pub fn new(segments: Vec>) -> Option { + if segments.is_empty() { + return None; + } + Some(Self { segments }) } fn input(&self) -> eth::Asset { - if self.is_empty() { - unreachable!("Output empty segment"); - } self.segments[0].input } fn output(&self) -> eth::Asset { - if self.is_empty() { - unreachable!("Output empty segment"); - } self.segments .last() .expect("route has at least one segment by construction") From d7304b0028b244d94d7c79e2069cb6093797cbff Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 5 Dec 2025 14:24:22 +0100 Subject: [PATCH 08/39] Fix quote verification for sell=buy --- crates/e2e/tests/e2e/submission.rs | 37 +++++- crates/shared/src/price_estimation/factory.rs | 17 ++- .../shared/src/price_estimation/sanitized.rs | 119 +++++++++++++++++- .../price_estimation/trade_verifier/mod.rs | 11 +- crates/solvers/src/boundary/baseline.rs | 12 +- crates/solvers/src/domain/solver.rs | 101 +++++++-------- 6 files changed, 220 insertions(+), 77 deletions(-) diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index e12686df17..2c3233eed3 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -6,8 +6,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, @@ -277,19 +279,42 @@ 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 = eth(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.into_legacy()).unwrap(), + }, + }, + ..Default::default() + }; + let quote_response = services.submit_quote("e_request).await.unwrap(); + tracing::info!(?quote_response); + assert!(quote_response.id.is_some()); + assert!(quote_response.verified); + assert!(quote_response.quote.buy_amount < 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: ::alloy::primitives::Uint<256, 4> = eth(2); - let order_buy_amount = eth(1); let order = OrderCreation { + kind: OrderKind::Sell, sell_token: *token.address(), - sell_amount: order_sell_amount, buy_token: *token.address(), - buy_amount: order_buy_amount, + quote_id: quote_response.id, + sell_amount: quote_sell_amount, + buy_amount: quote_response.quote.buy_amount, valid_to: model::time::now_in_epoch_seconds() + 300, - kind: OrderKind::Sell, ..Default::default() } .sign( @@ -311,7 +336,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 index 3e99ae703b..9f4579b0e2 100644 --- 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, + ) -> 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 index 6144340bb9..5dd39f2cf0 100644 --- a/crates/shared/src/price_estimation/sanitized.rs +++ b/crates/shared/src/price_estimation/sanitized.rs @@ -24,6 +24,8 @@ pub struct SanitizedPriceEstimator { inner: Arc, bad_token_detector: Arc, native_token: Address, + /// Enables the short-circuiting logic in case of sell and buy is the same + is_estimating_native_price: bool, } impl SanitizedPriceEstimator { @@ -31,11 +33,13 @@ impl SanitizedPriceEstimator { inner: Arc, native_token: Address, bad_token_detector: Arc, + 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 index f3582b97e5..e58362f6fb 100644 --- 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, @@ -771,8 +771,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 @@ -869,7 +872,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 index 699619baaf..9d1351061b 100644 --- a/crates/solvers/src/boundary/baseline.rs +++ b/crates/solvers/src/boundary/baseline.rs @@ -44,10 +44,10 @@ impl<'a> Solver<'a> { &self, request: solver::Request, max_hops: usize, - ) -> Result>, RouteComputationError> { + ) -> Option> { 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, @@ -85,8 +85,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 { @@ -117,12 +116,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 index 58de9495fd..597f36a5cb 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -170,21 +170,20 @@ 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 = f64::from(self.native_token_price_estimation_amount) - / f64::from(route.input().amount); + / f64::from(route + .input() + .expect("route is not empty") + .amount); 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 +195,40 @@ impl Inner { let compute_solution = async |request: Request| -> Option { 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 +377,20 @@ pub struct Segment<'a> { } impl<'a> Route<'a> { - pub fn new(segments: Vec>) -> Option { - if segments.is_empty() { - return None; - } - Some(Self { segments }) + pub fn new(segments: Vec>) -> Self { + Self { segments } + } + + fn input(&self) -> Option { + self.segments.first().map(|segment| segment.input) } - fn input(&self) -> eth::Asset { - self.segments[0].input + fn output(&self) -> Option { + 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 { From 44ec35ec25505836f82d3854c60fbaeeca59453b Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 5 Dec 2025 15:26:03 +0100 Subject: [PATCH 09/39] fmt --- crates/shared/src/price_estimation/trade_verifier/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/shared/src/price_estimation/trade_verifier/mod.rs b/crates/shared/src/price_estimation/trade_verifier/mod.rs index e58362f6fb..b9eaad2a3e 100644 --- a/crates/shared/src/price_estimation/trade_verifier/mod.rs +++ b/crates/shared/src/price_estimation/trade_verifier/mod.rs @@ -872,7 +872,9 @@ fn ensure_quote_accuracy( .get(&query.buy_token) .context("summary buy token is missing")?; - 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) { + 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); } From 61725f952c8e91fda5d5c51e80709259786be9ba Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 5 Dec 2025 16:10:46 +0100 Subject: [PATCH 10/39] Addressed comments --- crates/solvers/src/boundary/baseline.rs | 1 - crates/solvers/src/domain/solver.rs | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/solvers/src/boundary/baseline.rs b/crates/solvers/src/boundary/baseline.rs index 9d1351061b..06fa9bc310 100644 --- a/crates/solvers/src/boundary/baseline.rs +++ b/crates/solvers/src/boundary/baseline.rs @@ -46,7 +46,6 @@ impl<'a> Solver<'a> { max_hops: usize, ) -> Option> { if request.sell.token == request.buy.token { - tracing::info!("Sell=Buy, returning empty route"); return Some(solver::Route::new(vec![])); } let candidates = self.base_tokens.path_candidates_with_hops( diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs index 597f36a5cb..2e6d033aa0 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -221,6 +221,11 @@ impl Inner { input = route.input().expect("route is not empty"); output = route.output().expect("route is not empty"); } else { + // Route is empty in case of sell and buy tokens being the same, as there + // is no need to figure out the liquidity for such pair. + // + // The input and output of the solution can be set directly to the + // respective sell and buy tokens 1 to 1. interactions = Vec::default(); gas = eth::Gas(U256::ZERO) + self.solution_gas_offset; From 47ffd26324dd7a4be07f994bcceafbf73bdd9841 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 5 Dec 2025 16:30:21 +0100 Subject: [PATCH 11/39] Remove sell=buy test case already covered by onchain verification --- .../e2e/tests/e2e/place_order_with_quote.rs | 97 +------------------ 1 file changed, 1 insertion(+), 96 deletions(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index 2b96a2613c..771b14649d 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -23,12 +23,6 @@ async fn local_node_place_order_with_quote_basic() { run_test(place_order_with_quote).await; } -#[tokio::test] -#[ignore] -async fn local_node_place_order_with_quote_same_token_pair() { - run_test(place_order_with_quote_same_token_pair).await; -} - #[tokio::test] #[ignore] async fn local_node_place_order_with_quote_same_token_pair_error() { @@ -171,93 +165,4 @@ async fn place_order_with_quote_same_token_pair_error(web3: Web3) { assert!( matches!(services.submit_quote("e_request).await, Err((reqwest::StatusCode::BAD_REQUEST, response)) if response.contains("SameBuyAndSellToken")) ); -} - -async fn place_order_with_quote_same_token_pair(web3: Web3) { - let mut onchain = OnchainComponents::deploy(web3.clone()).await; - - let [solver] = onchain.make_solvers(eth(10)).await; - let [trader] = onchain.make_accounts(eth(10)).await; - let [token] = onchain - .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) - .await; - - token.mint(trader.address(), eth(10)).await; - - token - .approve(onchain.contracts().allowance.into_alloy(), eth(10)) - .from(trader.address()) - .send_and_watch() - .await - .unwrap(); - - tracing::info!("Starting services."); - let services = Services::new(&onchain).await; - services - .start_protocol_with_args( - ExtraServiceArgs { - api: vec!["--allow-same-sell-and-buy-token=true".to_string()], - ..Default::default() - }, - solver.clone(), - ) - .await; - - // Disable auto-mine so we don't accidentally mine a settlement - web3.api::>() - .set_automine_enabled(false) - .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("e_request).await.unwrap(); - tracing::debug!(?quote_response); - assert!(quote_response.id.is_some()); - assert!(quote_response.verified); - - 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 order = OrderCreation { - quote_id: quote_response.id, - sell_token: *token.address(), - sell_amount: quote_sell_amount.into_alloy(), - buy_token: *token.address(), - buy_amount: quote_response.quote.buy_amount, - 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.private_key()).unwrap()), - ); - let order_uid = services.create_order(&order).await.unwrap(); - - tracing::info!("Order quote verification"); - let order_quote = database::orders::read_quote( - services.db().acquire().await.unwrap().deref_mut(), - &database::byte_array::ByteArray(order_uid.0), - ) - .await - .unwrap() - .unwrap(); - assert_eq!(quote_response.verified, order_quote.verified); - assert_eq!(quote_metadata.unwrap().0, order_quote.metadata); -} +} \ No newline at end of file From 6994ce451617e4d8fe32d2a7e01f6c636d1b8210 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 5 Dec 2025 17:11:05 +0100 Subject: [PATCH 12/39] Remove the builder method for allowing same sell and buy token --- crates/orderbook/src/run.rs | 44 ++++++++--------- crates/shared/src/order_validation.rs | 69 ++++++++++++--------------- 2 files changed, 51 insertions(+), 62 deletions(-) diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index f209111813..7562574249 100644 --- a/crates/orderbook/src/run.rs +++ b/crates/orderbook/src/run.rs @@ -401,29 +401,27 @@ pub async fn run(args: Arguments) { let chainalysis_oracle = ChainalysisOracle::Instance::deployed(&web3.alloy) .await .ok(); - let order_validator = Arc::new( - OrderValidator::new( - native_token, - Arc::new(order_validation::banned::Users::new( - chainalysis_oracle, - args.banned_users, - args.banned_users_max_cache_size.get().to_u64().unwrap(), - )), - validity_configuration, - args.eip1271_skip_creation_validation, - bad_token_detector.clone(), - hooks_contract, - optimal_quoter.clone(), - balance_fetcher, - signature_validator, - Arc::new(postgres_write.clone()), - args.max_limit_orders_per_user, - code_fetcher, - app_data_validator.clone(), - args.max_gas_per_order, - ) - .with_allow_same_sell_and_buy_token(args.allow_same_sell_and_buy_token), - ); + let order_validator = Arc::new(OrderValidator::new( + native_token, + Arc::new(order_validation::banned::Users::new( + chainalysis_oracle, + args.banned_users, + args.banned_users_max_cache_size.get().to_u64().unwrap(), + )), + validity_configuration, + args.eip1271_skip_creation_validation, + bad_token_detector.clone(), + hooks_contract, + optimal_quoter.clone(), + balance_fetcher, + signature_validator, + Arc::new(postgres_write.clone()), + args.max_limit_orders_per_user, + code_fetcher, + app_data_validator.clone(), + args.max_gas_per_order, + args.allow_same_sell_and_buy_token, + )); let ipfs = args .ipfs_gateway .map(|url| { diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 8c19b27b57..e9fdf6b94d 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -301,6 +301,7 @@ impl OrderValidator { code_fetcher: Arc, app_data_validator: Validator, max_gas_per_order: u64, + allow_same_sell_and_buy_token: bool, ) -> Self { Self { native_token, @@ -317,20 +318,10 @@ impl OrderValidator { code_fetcher, app_data_validator, max_gas_per_order, - allow_same_sell_and_buy_token: false, + allow_same_sell_and_buy_token, } } - /// Enables sell and buy validation if Some is provided - /// Disables it if None - pub fn with_allow_same_sell_and_buy_token( - mut self, - allow_same_sell_and_buy_token: bool, - ) -> Self { - self.allow_same_sell_and_buy_token = allow_same_sell_and_buy_token; - self - } - async fn check_max_limit_orders(&self, owner: Address) -> Result<(), ValidationError> { let num_limit_orders = self .limit_order_counter @@ -1082,8 +1073,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let result = validator .partial_validate(PreOrderData { partially_fillable: true, @@ -1236,8 +1227,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let order = || PreOrderData { valid_to: time::now_in_epoch_seconds() + validity_configuration.min.as_secs() as u32 @@ -1326,8 +1317,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(true); + true, + ); let order = || PreOrderData { valid_to: time::now_in_epoch_seconds() @@ -1415,8 +1406,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -1632,8 +1623,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let creation = OrderCreation { valid_to: model::time::now_in_epoch_seconds() + 2, @@ -1709,8 +1700,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let creation = OrderCreation { valid_to: model::time::now_in_epoch_seconds() + 2, @@ -1774,8 +1765,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, sell_token: Address::with_last_byte(1), @@ -1832,8 +1823,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -1895,8 +1886,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -1960,8 +1951,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -2024,8 +2015,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -2094,8 +2085,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let order = OrderCreation { valid_to: u32::MAX, @@ -2186,8 +2177,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); // Test with flashloan hint that covers the sell amount let order_with_sufficient_flashloan = OrderCreation { @@ -2605,8 +2596,8 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - ) - .with_allow_same_sell_and_buy_token(false); + false, + ); let creation = OrderCreation { valid_to: time::now_in_epoch_seconds() + 10, From 7d3bf9bbeb1956b493a9c3091224fdb48aba4e2b Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 9 Dec 2025 12:08:51 +0100 Subject: [PATCH 13/39] Update crates/orderbook/src/arguments.rs Co-authored-by: ilya --- crates/orderbook/src/arguments.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/orderbook/src/arguments.rs b/crates/orderbook/src/arguments.rs index fc88e8c5ae..2bc46de554 100644 --- a/crates/orderbook/src/arguments.rs +++ b/crates/orderbook/src/arguments.rs @@ -146,9 +146,9 @@ pub struct Arguments { #[clap(flatten)] pub volume_fee_config: Option, - /// Allow same sell and buy token + /// Disable ability to place orders with the same sell and buy tokens #[clap(long, env, action = clap::ArgAction::Set, default_value = "false")] - pub allow_same_sell_and_buy_token: bool, + pub disable_same_sell_and_buy_token_orders: bool, } /// Volume-based protocol fee factor to be applied to quotes. From 59fcc641a868ae67f159a3bc989bd3652b63c741 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 9 Dec 2025 12:09:48 +0100 Subject: [PATCH 14/39] Update crates/shared/src/price_estimation/sanitized.rs Co-authored-by: ilya --- crates/shared/src/price_estimation/sanitized.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/shared/src/price_estimation/sanitized.rs b/crates/shared/src/price_estimation/sanitized.rs index 5dd39f2cf0..d78980c693 100644 --- a/crates/shared/src/price_estimation/sanitized.rs +++ b/crates/shared/src/price_estimation/sanitized.rs @@ -24,7 +24,7 @@ pub struct SanitizedPriceEstimator { inner: Arc, bad_token_detector: Arc, native_token: Address, - /// Enables the short-circuiting logic in case of sell and buy is the same + /// Enables the short-circuiting logic in case the sell and buy tokens are the same is_estimating_native_price: bool, } From e59f4217267dc63e0f2f811b4628aa644479f666 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 9 Dec 2025 13:27:53 +0100 Subject: [PATCH 15/39] Update crates/e2e/tests/e2e/place_order_with_quote.rs Co-authored-by: ilya --- crates/e2e/tests/e2e/place_order_with_quote.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index 771b14649d..ba4d654649 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -121,7 +121,7 @@ async fn place_order_with_quote(web3: Web3) { assert_eq!(quote_metadata.unwrap().0, order_quote.metadata); } -async fn place_order_with_quote_same_token_pair_error(web3: Web3) { +async fn disabled_same_sell_and_buy_token_order_feature(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3.clone()).await; let [solver] = onchain.make_solvers(to_wei(10).into_alloy()).await; From 6ebc5aacf586c5ceca04ee76118238a5b2e324ed Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 9 Dec 2025 14:25:49 +0100 Subject: [PATCH 16/39] Apply suggestion from @MartinquaXD Co-authored-by: Martin Magnus --- crates/shared/src/price_estimation/trade_verifier/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/shared/src/price_estimation/trade_verifier/mod.rs b/crates/shared/src/price_estimation/trade_verifier/mod.rs index b9eaad2a3e..7ada1338bd 100644 --- a/crates/shared/src/price_estimation/trade_verifier/mod.rs +++ b/crates/shared/src/price_estimation/trade_verifier/mod.rs @@ -772,7 +772,7 @@ fn add_balance_queries( query_balance_call.into(), ); - // query the balance as the first interaction, the sell token has already + // query the balance as the first middle 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()); From a1d568f0c8e982ddbae15184ec2ca35ee00c6bfd Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 9 Dec 2025 13:29:53 +0100 Subject: [PATCH 17/39] Rename allow_same_sell_and_buy_token to disallow_... --- crates/e2e/tests/e2e/place_order_with_quote.rs | 4 ++-- crates/orderbook/src/arguments.rs | 4 ++-- crates/orderbook/src/run.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index ba4d654649..ffa629f9c2 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -25,8 +25,8 @@ async fn local_node_place_order_with_quote_basic() { #[tokio::test] #[ignore] -async fn local_node_place_order_with_quote_same_token_pair_error() { - run_test(place_order_with_quote_same_token_pair_error).await; +async fn local_node_disabled_same_sell_and_buy_token_order_feature() { + run_test(disabled_same_sell_and_buy_token_order_feature).await; } async fn place_order_with_quote(web3: Web3) { diff --git a/crates/orderbook/src/arguments.rs b/crates/orderbook/src/arguments.rs index 2bc46de554..781ac1b733 100644 --- a/crates/orderbook/src/arguments.rs +++ b/crates/orderbook/src/arguments.rs @@ -238,7 +238,7 @@ impl std::fmt::Display for Arguments { max_gas_per_order, active_order_competition_threshold, volume_fee_config, - allow_same_sell_and_buy_token, + disable_same_sell_and_buy_token_orders, } = self; write!(f, "{shared}")?; @@ -295,7 +295,7 @@ impl std::fmt::Display for Arguments { writeln!(f, "volume_fee_config: {volume_fee_config:?}")?; writeln!( f, - "allow_same_sell_and_buy_token: {allow_same_sell_and_buy_token}" + "disable_same_sell_and_buy_token_orders: {disable_same_sell_and_buy_token_orders}" )?; Ok(()) diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index 7562574249..4adcd1ec93 100644 --- a/crates/orderbook/src/run.rs +++ b/crates/orderbook/src/run.rs @@ -420,7 +420,7 @@ pub async fn run(args: Arguments) { code_fetcher, app_data_validator.clone(), args.max_gas_per_order, - args.allow_same_sell_and_buy_token, + args.disable_same_sell_and_buy_token_orders, )); let ipfs = args .ipfs_gateway From 0dd67549d36b23498a7d15f2c89036a9cbf8ac04 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 9 Dec 2025 16:18:40 +0100 Subject: [PATCH 18/39] Address comments --- crates/driver/src/boundary/liquidity/mod.rs | 4 + crates/e2e/tests/e2e/submission.rs | 64 +------- crates/orderbook/src/arguments.rs | 14 +- crates/orderbook/src/run.rs | 2 +- crates/shared/src/order_quoting.rs | 4 + crates/shared/src/order_validation.rs | 155 +++++++++++++++--- .../shared/src/price_estimation/sanitized.rs | 3 +- crates/solvers/src/boundary/baseline.rs | 5 +- crates/solvers/src/domain/solver.rs | 107 ++++++------ 9 files changed, 211 insertions(+), 147 deletions(-) diff --git a/crates/driver/src/boundary/liquidity/mod.rs b/crates/driver/src/boundary/liquidity/mod.rs index 545fd8d51a..f34ec43781 100644 --- a/crates/driver/src/boundary/liquidity/mod.rs +++ b/crates/driver/src/boundary/liquidity/mod.rs @@ -128,6 +128,10 @@ impl Fetcher { pairs: &HashSet, block: infra::liquidity::AtBlock, ) -> Result> { + if pairs.is_empty() { + return Ok(vec![]); + } + let pairs = pairs .iter() .map(|pair| { diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index 2c3233eed3..0bc34dfa17 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -34,12 +34,6 @@ async fn local_node_submit_same_sell_and_buy_token_order_without_quote() { run_test(test_submit_same_sell_and_buy_token_order_without_quote).await; } -#[tokio::test] -#[ignore] -async fn local_node_submit_same_sell_and_buy_token_order_without_quote_fail() { - run_test(test_submit_same_sell_and_buy_token_order_without_quote_fail).await; -} - async fn test_cancel_on_expiry(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3.clone()).await; @@ -163,7 +157,7 @@ async fn test_submit_same_sell_and_buy_token_order_without_quote(web3: Web3) { services .start_protocol_with_args( ExtraServiceArgs { - api: vec!["--allow-same-sell-and-buy-token=true".to_string()], + api: vec!["--same-tokens-policy=allowSell".to_string()], ..Default::default() }, solver.clone(), @@ -194,55 +188,6 @@ async fn test_submit_same_sell_and_buy_token_order_without_quote(web3: Web3) { services.create_order(&order).await.unwrap(); } -async fn test_submit_same_sell_and_buy_token_order_without_quote_fail(web3: Web3) { - let mut onchain = OnchainComponents::deploy(web3.clone()).await; - - let [solver] = onchain.make_solvers(eth(10)).await; - let [trader] = onchain.make_accounts(eth(10)).await; - let [token] = onchain - .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) - .await; - - token.mint(trader.address(), eth(10)).await; - - token - .approve(onchain.contracts().allowance.into_alloy(), eth(10)) - .from(trader.address()) - .send_and_watch() - .await - .unwrap(); - - tracing::info!("Starting services."); - let services = Services::new(&onchain).await; - services.start_protocol(solver.clone()).await; - - // Disable auto-mine so we don't accidentally mine a settlement - web3.api::>() - .set_automine_enabled(false) - .await - .expect("Must be able to disable automine"); - - tracing::info!("Placing order"); - let order = OrderCreation { - sell_token: *token.address(), - sell_amount: eth(1), - buy_token: *token.address(), - buy_amount: eth(1), - 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.private_key()).unwrap()), - ); - // services.create_order(&order).await.unwrap(); - assert!( - matches!(services.create_order(&order).await, Err((reqwest::StatusCode::BAD_REQUEST, response)) if response.contains("SameBuyAndSellToken")) - ); -} - async fn test_execute_same_sell_and_buy_token(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3.clone()).await; @@ -266,7 +211,7 @@ async fn test_execute_same_sell_and_buy_token(web3: Web3) { services .start_protocol_with_args( ExtraServiceArgs { - api: vec!["--allow-same-sell-and-buy-token=true".to_string()], + api: vec!["--same-tokens-policy=allowSell".to_string()], ..Default::default() }, solver.clone(), @@ -376,7 +321,10 @@ async fn test_execute_same_sell_and_buy_token(web3: Web3) { tracing::info!(?initial_balance, ?final_balance, "Trade completed"); // Verify that the balance changed (settlement happened on chain) - assert_ne!(final_balance, initial_balance); + assert!( + final_balance < initial_balance, + "Final balance should be smaller than initial balance due to fees" + ); } async fn get_pending_tx(account: H160, web3: &Web3) -> Option { diff --git a/crates/orderbook/src/arguments.rs b/crates/orderbook/src/arguments.rs index 781ac1b733..5c4ebd240f 100644 --- a/crates/orderbook/src/arguments.rs +++ b/crates/orderbook/src/arguments.rs @@ -146,9 +146,10 @@ pub struct Arguments { #[clap(flatten)] pub volume_fee_config: Option, - /// Disable ability to place orders with the same sell and buy tokens - #[clap(long, env, action = clap::ArgAction::Set, default_value = "false")] - pub disable_same_sell_and_buy_token_orders: bool, + /// Controls if same sell and buy token orders are allowed + /// Disallowed by default + #[clap(long, env, default_value = "disallow")] + pub same_tokens_policy: shared::order_validation::SameTokensPolicy, } /// Volume-based protocol fee factor to be applied to quotes. @@ -238,7 +239,7 @@ impl std::fmt::Display for Arguments { max_gas_per_order, active_order_competition_threshold, volume_fee_config, - disable_same_sell_and_buy_token_orders, + same_tokens_policy, } = self; write!(f, "{shared}")?; @@ -293,10 +294,7 @@ impl std::fmt::Display for Arguments { "active_order_competition_threshold: {active_order_competition_threshold}" )?; writeln!(f, "volume_fee_config: {volume_fee_config:?}")?; - writeln!( - f, - "disable_same_sell_and_buy_token_orders: {disable_same_sell_and_buy_token_orders}" - )?; + writeln!(f, "same_tokens_policy {same_tokens_policy:?}")?; Ok(()) } diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index 4adcd1ec93..95307143ce 100644 --- a/crates/orderbook/src/run.rs +++ b/crates/orderbook/src/run.rs @@ -420,7 +420,7 @@ pub async fn run(args: Arguments) { code_fetcher, app_data_validator.clone(), args.max_gas_per_order, - args.disable_same_sell_and_buy_token_orders, + args.same_tokens_policy, )); let ipfs = args .ipfs_gateway diff --git a/crates/shared/src/order_quoting.rs b/crates/shared/src/order_quoting.rs index 2f63570e5f..c7be74b11e 100644 --- a/crates/shared/src/order_quoting.rs +++ b/crates/shared/src/order_quoting.rs @@ -708,6 +708,10 @@ impl From<&OrderQuoteRequest> for PreOrderData { sell_token_balance: quote_request.sell_token_balance, signing_scheme: quote_request.signing_scheme.into(), class: OrderClass::Market, + kind: match quote_request.side { + OrderQuoteSide::Buy { .. } => OrderKind::Buy, + OrderQuoteSide::Sell { .. } => OrderKind::Sell, + }, } } } diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index e9fdf6b94d..a3ea3d2942 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -47,7 +47,7 @@ use { signature::{self, Signature, SigningScheme, hashed_eip712_message}, time, }, - std::{sync::Arc, time::Duration}, + std::{str::FromStr, sync::Arc, time::Duration}, tracing::instrument, }; @@ -214,6 +214,38 @@ pub trait LimitOrderCounting: Send + Sync { async fn count(&self, owner: Address) -> Result; } +#[derive(Clone, Debug)] +pub enum SameTokensPolicy { + Disallow, + AllowSell, + Allow, +} + +impl FromStr for SameTokensPolicy { + type Err = anyhow::Error; + + fn from_str(value: &str) -> Result { + Ok(match value { + "disallow" => Self::Disallow, + "allowSell" => Self::AllowSell, + "allow" => Self::Allow, + _ => anyhow::bail!("Invalid same token policy: {value}"), + }) + } +} + +impl SameTokensPolicy { + fn validate_same_sell_and_buy_token(&self, order: &PreOrderData) -> bool { + if order.sell_token != order.buy_token { + return true; + } + matches!( + (self, order.kind), + (Self::Allow, _) | (Self::AllowSell, OrderKind::Sell) + ) + } +} + #[derive(Clone)] pub struct OrderValidator { /// For Pre/Partial-Validation: performed during fee & quote phase @@ -233,7 +265,7 @@ pub struct OrderValidator { pub code_fetcher: Arc, app_data_validator: Validator, max_gas_per_order: u64, - allow_same_sell_and_buy_token: bool, + same_tokens_policy: SameTokensPolicy, } #[derive(Debug, Eq, PartialEq, Default)] @@ -248,6 +280,7 @@ pub struct PreOrderData { pub sell_token_balance: SellTokenSource, pub signing_scheme: SigningScheme, pub class: OrderClass, + pub kind: OrderKind, } fn actual_receiver(owner: Address, order: &OrderData) -> Address { @@ -275,6 +308,7 @@ impl PreOrderData { true => OrderClass::Limit, false => OrderClass::Market, }, + kind: order.kind, } } } @@ -301,7 +335,7 @@ impl OrderValidator { code_fetcher: Arc, app_data_validator: Validator, max_gas_per_order: u64, - allow_same_sell_and_buy_token: bool, + same_tokens_policy: SameTokensPolicy, ) -> Self { Self { native_token, @@ -318,7 +352,7 @@ impl OrderValidator { code_fetcher, app_data_validator, max_gas_per_order, - allow_same_sell_and_buy_token, + same_tokens_policy, } } @@ -484,7 +518,10 @@ impl OrderValidating for OrderValidator { self.validity_configuration.validate_period(&order)?; - if !self.allow_same_sell_and_buy_token && order.sell_token == order.buy_token { + if !self + .same_tokens_policy + .validate_same_sell_and_buy_token(&order) + { return Err(PartialValidationError::SameBuyAndSellToken); } @@ -1073,7 +1110,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let result = validator .partial_validate(PreOrderData { @@ -1227,7 +1264,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let order = || PreOrderData { valid_to: time::now_in_epoch_seconds() @@ -1277,7 +1314,7 @@ mod tests { } #[tokio::test] - async fn pre_validate_allow_same_sell_and_buy_token() { + async fn pre_validate_allow_same_tokens() { let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); let validity_configuration = OrderValidPeriodConfiguration { min: Duration::from_secs(1), @@ -1317,7 +1354,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - true, + SameTokensPolicy::Allow, ); let order = || PreOrderData { @@ -1354,6 +1391,84 @@ mod tests { ); } + #[tokio::test] + async fn pre_validate_same_tokens_allow_sell() { + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); + let validity_configuration = OrderValidPeriodConfiguration { + min: Duration::from_secs(1), + max_market: Duration::from_secs(100), + max_limit: Duration::from_secs(200), + }; + + let mut bad_token_detector = MockBadTokenDetecting::new(); + bad_token_detector + .expect_detect() + .with(eq(Address::with_last_byte(1))) + .returning(|_| Ok(TokenQuality::Good)); + bad_token_detector + .expect_detect() + .with(eq(Address::with_last_byte(2))) + .returning(|_| Ok(TokenQuality::Good)); + + let mut limit_order_counter = MockLimitOrderCounting::new(); + limit_order_counter.expect_count().returning(|_| Ok(0u64)); + let validator = OrderValidator::new( + native_token.clone(), + Arc::new(order_validation::banned::Users::none()), + validity_configuration, + false, + Arc::new(bad_token_detector), + HooksTrampoline::Instance::new( + Address::from([0xcf; 20]), + ProviderBuilder::new() + .connect_mocked_client(Asserter::new()) + .erased(), + ), + Arc::new(MockOrderQuoting::new()), + Arc::new(MockBalanceFetching::new()), + Arc::new(MockSignatureValidating::new()), + Arc::new(limit_order_counter), + 0, + Arc::new(MockCodeFetching::new()), + Default::default(), + u64::MAX, + SameTokensPolicy::AllowSell, + ); + + let order = || PreOrderData { + buy_token: Address::with_last_byte(2), + sell_token: Address::with_last_byte(2), + valid_to: time::now_in_epoch_seconds() + + validity_configuration.min.as_secs() as u32 + + 2, + ..Default::default() + }; + + assert!(matches!( + dbg!( + validator + .partial_validate(PreOrderData { + kind: OrderKind::Buy, + ..order() + }) + .await + ), + Err(PartialValidationError::SameBuyAndSellToken) + )); + + assert!( + dbg!( + validator + .partial_validate(PreOrderData { + kind: OrderKind::Sell, + ..order() + }) + .await + ) + .is_ok() + ); + } + #[tokio::test] async fn post_validate_ok() { let mut order_quoter = MockOrderQuoting::new(); @@ -1406,7 +1521,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let creation = OrderCreation { @@ -1623,7 +1738,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let creation = OrderCreation { @@ -1700,7 +1815,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let creation = OrderCreation { @@ -1765,7 +1880,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let order = OrderCreation { valid_to: time::now_in_epoch_seconds() + 2, @@ -1823,7 +1938,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let order = OrderCreation { @@ -1886,7 +2001,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let order = OrderCreation { @@ -1951,7 +2066,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let order = OrderCreation { @@ -2015,7 +2130,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let creation = OrderCreation { @@ -2085,7 +2200,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let order = OrderCreation { @@ -2177,7 +2292,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); // Test with flashloan hint that covers the sell amount @@ -2596,7 +2711,7 @@ mod tests { Arc::new(MockCodeFetching::new()), Default::default(), u64::MAX, - false, + SameTokensPolicy::Disallow, ); let creation = OrderCreation { diff --git a/crates/shared/src/price_estimation/sanitized.rs b/crates/shared/src/price_estimation/sanitized.rs index d78980c693..08546218a8 100644 --- a/crates/shared/src/price_estimation/sanitized.rs +++ b/crates/shared/src/price_estimation/sanitized.rs @@ -24,7 +24,8 @@ pub struct SanitizedPriceEstimator { inner: Arc, bad_token_detector: Arc, native_token: Address, - /// Enables the short-circuiting logic in case the sell and buy tokens are the same + /// Enables the short-circuiting logic in case the sell and buy tokens are + /// the same is_estimating_native_price: bool, } diff --git a/crates/solvers/src/boundary/baseline.rs b/crates/solvers/src/boundary/baseline.rs index 06fa9bc310..77e577512e 100644 --- a/crates/solvers/src/boundary/baseline.rs +++ b/crates/solvers/src/boundary/baseline.rs @@ -45,9 +45,6 @@ impl<'a> Solver<'a> { request: solver::Request, max_hops: usize, ) -> Option> { - if request.sell.token == request.buy.token { - return Some(solver::Route::new(vec![])); - } let candidates = self.base_tokens.path_candidates_with_hops( request.sell.token.0, request.buy.token.0, @@ -119,7 +116,7 @@ impl<'a> Solver<'a> { } }; - Some(solver::Route::new(segments)) + solver::Route::new(segments) } async fn traverse_path( diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs index 2e6d033aa0..59f75197f0 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -13,7 +13,7 @@ use { auction, eth, liquidity, - order::{self, Order}, + order::{self, Order, Side}, solution, }, infra::metrics, @@ -170,13 +170,12 @@ impl Inner { .route(native_price_request, self.max_hops) .await { - Some(route) if !route.is_empty() => { + Some(route) => { // how many units of buy_token are bought for one unit of sell_token // (buy_amount / sell_amount). let price = f64::from(self.native_token_price_estimation_amount) / f64::from(route .input() - .expect("route is not empty") .amount); let Some(price) = to_normalized_price(price) else { continue; @@ -195,13 +194,23 @@ impl Inner { let compute_solution = async |request: Request| -> Option { let wrappers = request.wrappers.clone(); - let route = boundary_solver.route(request, self.max_hops).await?; - let interactions; - let gas; - let (input, mut output); - - if !route.is_empty() { - interactions = route + let solution = if order.sell.token == order.buy.token { + let (input, mut output) = match order.side { + Side::Sell => (order.sell, order.buy), + Side::Buy => (order.buy, order.sell), + }; + output.amount = input.amount; + solution::Single { + order: order.clone(), + input, + output, + interactions: Vec::default(), + gas: eth::Gas(U256::ZERO) + self.solution_gas_offset, + wrappers, + } + } else { + let route = boundary_solver.route(request, self.max_hops).await?; + let interactions = route .segments .iter() .map(|segment| { @@ -217,51 +226,40 @@ impl Inner { )) }) .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 { - // Route is empty in case of sell and buy tokens being the same, as there - // is no need to figure out the liquidity for such pair. - // - // The input and output of the solution can be set directly to the - // respective sell and buy tokens 1 to 1. - 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 - // for exact output token amounts. This leads to - // potential rounding errors for buy orders, where we - // can buy slightly more than intended. Fix this by - // capping the output amount to the order's buy amount - // for buy orders. - if let order::Side::Buy = order.side { - output.amount = cmp::min(output.amount, order.buy.amount); - } - - let fee = sell_token_price - .ether_value(eth::Ether(gas.0.checked_mul(auction.gas_price.0.0)?))? - .into(); + let gas = route.gas() + self.solution_gas_offset; + let mut output = route.output().expect("route is not empty"); + + // The baseline solver generates a path with swapping + // for exact output token amounts. This leads to + // potential rounding errors for buy orders, where we + // can buy slightly more than intended. Fix this by + // capping the output amount to the order's buy amount + // for buy orders. + if let order::Side::Buy = order.side { + output.amount = cmp::min(output.amount, order.buy.amount); + } - Some( solution::Single { order: order.clone(), - input, + input: route.input(), output, interactions, gas, wrappers, } - .into_solution(fee)? - .with_id(solution::Id(i as u64)) - .with_buffers_internalizations(&auction.tokens), + }; + + let fee = sell_token_price + .ether_value(eth::Ether( + solution.gas.0.checked_mul(auction.gas_price.0.0)?, + ))? + .into(); + + Some( + solution + .into_solution(fee)? + .with_id(solution::Id(i as u64)) + .with_buffers_internalizations(&auction.tokens), ) }; @@ -382,22 +380,21 @@ pub struct Segment<'a> { } impl<'a> Route<'a> { - pub fn new(segments: Vec>) -> Self { - Self { segments } + pub fn new(segments: Vec>) -> Option { + if segments.is_empty() { + return None; + } + Some(Self { segments }) } - fn input(&self) -> Option { - self.segments.first().map(|segment| segment.input) + fn input(&self) -> eth::Asset { + self.segments[0].input } fn output(&self) -> Option { self.segments.last().map(|segment| segment.output) } - fn is_empty(&self) -> bool { - self.segments.is_empty() - } - fn gas(&self) -> eth::Gas { eth::Gas( self.segments From eaec342ba0da514d5479b3c8539f1f2eb13f0f5d Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 10 Dec 2025 12:24:29 +0100 Subject: [PATCH 19/39] Update crates/shared/src/order_validation.rs Use clap::ValueEnum instead of manual parsing. Co-authored-by: ilya --- crates/e2e/tests/e2e/submission.rs | 4 ++-- crates/shared/src/order_validation.rs | 15 +-------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index 0bc34dfa17..f832a41e12 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -157,7 +157,7 @@ async fn test_submit_same_sell_and_buy_token_order_without_quote(web3: Web3) { services .start_protocol_with_args( ExtraServiceArgs { - api: vec!["--same-tokens-policy=allowSell".to_string()], + api: vec!["--same-tokens-policy=allow-sell".to_string()], ..Default::default() }, solver.clone(), @@ -211,7 +211,7 @@ async fn test_execute_same_sell_and_buy_token(web3: Web3) { services .start_protocol_with_args( ExtraServiceArgs { - api: vec!["--same-tokens-policy=allowSell".to_string()], + api: vec!["--same-tokens-policy=allow-sell".to_string()], ..Default::default() }, solver.clone(), diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index a3ea3d2942..e65a475b3a 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -214,26 +214,13 @@ pub trait LimitOrderCounting: Send + Sync { async fn count(&self, owner: Address) -> Result; } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, clap::ValueEnum)] pub enum SameTokensPolicy { Disallow, AllowSell, Allow, } -impl FromStr for SameTokensPolicy { - type Err = anyhow::Error; - - fn from_str(value: &str) -> Result { - Ok(match value { - "disallow" => Self::Disallow, - "allowSell" => Self::AllowSell, - "allow" => Self::Allow, - _ => anyhow::bail!("Invalid same token policy: {value}"), - }) - } -} - impl SameTokensPolicy { fn validate_same_sell_and_buy_token(&self, order: &PreOrderData) -> bool { if order.sell_token != order.buy_token { From 265d17e274a3c024ace4e1702cdd66b48fa51026 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 10 Dec 2025 15:54:32 +0100 Subject: [PATCH 20/39] Remove buy orders support for sell=buy Retain placeholder for SameTokensPolicy referencing task to implement support for buy orders. --- .../e2e/tests/e2e/place_order_with_quote.rs | 7 +- crates/e2e/tests/e2e/submission.rs | 5 +- crates/shared/src/order_validation.rs | 85 +------------------ 3 files changed, 10 insertions(+), 87 deletions(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index ffa629f9c2..237750e09b 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -2,10 +2,7 @@ use { ::alloy::primitives::U256, driver::domain::eth::NonZeroU256, e2e::{nodes::local_node::TestNodeApi, setup::*}, - ethrpc::alloy::{ - CallBuilderExt, - conversions::IntoAlloy, - }, + ethrpc::alloy::{CallBuilderExt, conversions::IntoAlloy}, model::{ order::{OrderCreation, OrderKind}, quote::{OrderQuoteRequest, OrderQuoteSide, SellAmount}, @@ -165,4 +162,4 @@ async fn disabled_same_sell_and_buy_token_order_feature(web3: Web3) { assert!( matches!(services.submit_quote("e_request).await, Err((reqwest::StatusCode::BAD_REQUEST, response)) if response.contains("SameBuyAndSellToken")) ); -} \ No newline at end of file +} diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index f832a41e12..5b5ed8a72a 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -2,7 +2,10 @@ use { ::alloy::primitives::U256, e2e::{nodes::local_node::TestNodeApi, setup::*}, ethcontract::{BlockId, H160, H256}, - ethrpc::alloy::{CallBuilderExt, conversions::{IntoAlloy, IntoLegacy}}, + ethrpc::alloy::{ + CallBuilderExt, + conversions::{IntoAlloy, IntoLegacy}, + }, futures::{Stream, StreamExt}, model::{ order::{OrderCreation, OrderKind}, diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index e65a475b3a..f9df2bda07 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -47,7 +47,7 @@ use { signature::{self, Signature, SigningScheme, hashed_eip712_message}, time, }, - std::{str::FromStr, sync::Arc, time::Duration}, + std::{sync::Arc, time::Duration}, tracing::instrument, }; @@ -218,7 +218,7 @@ pub trait LimitOrderCounting: Send + Sync { pub enum SameTokensPolicy { Disallow, AllowSell, - Allow, + // Allow, TODO: Allow sell and buy orders with the same tokens (https://github.com/cowprotocol/services/issues/3963) } impl SameTokensPolicy { @@ -228,7 +228,8 @@ impl SameTokensPolicy { } matches!( (self, order.kind), - (Self::Allow, _) | (Self::AllowSell, OrderKind::Sell) + /* (Self::Allow, _) | To be implemented in https://github.com/cowprotocol/services/issues/3963 */ + (Self::AllowSell, OrderKind::Sell) ) } } @@ -1300,84 +1301,6 @@ mod tests { ); } - #[tokio::test] - async fn pre_validate_allow_same_tokens() { - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); - let validity_configuration = OrderValidPeriodConfiguration { - min: Duration::from_secs(1), - max_market: Duration::from_secs(100), - max_limit: Duration::from_secs(200), - }; - - let mut bad_token_detector = MockBadTokenDetecting::new(); - bad_token_detector - .expect_detect() - .with(eq(Address::with_last_byte(1))) - .returning(|_| Ok(TokenQuality::Good)); - bad_token_detector - .expect_detect() - .with(eq(Address::with_last_byte(2))) - .returning(|_| Ok(TokenQuality::Good)); - - let mut limit_order_counter = MockLimitOrderCounting::new(); - limit_order_counter.expect_count().returning(|_| Ok(0u64)); - let validator = OrderValidator::new( - native_token.clone(), - Arc::new(order_validation::banned::Users::none()), - validity_configuration, - false, - Arc::new(bad_token_detector), - HooksTrampoline::Instance::new( - Address::from([0xcf; 20]), - ProviderBuilder::new() - .connect_mocked_client(Asserter::new()) - .erased(), - ), - Arc::new(MockOrderQuoting::new()), - Arc::new(MockBalanceFetching::new()), - Arc::new(MockSignatureValidating::new()), - Arc::new(limit_order_counter), - 0, - Arc::new(MockCodeFetching::new()), - Default::default(), - u64::MAX, - SameTokensPolicy::Allow, - ); - - let order = || PreOrderData { - valid_to: time::now_in_epoch_seconds() - + validity_configuration.min.as_secs() as u32 - + 2, - ..Default::default() - }; - - assert!(matches!( - dbg!( - validator - .partial_validate(PreOrderData { - buy_token: BUY_ETH_ADDRESS, - sell_token: *native_token.address(), - ..order() - }) - .await - ), - Err(PartialValidationError::SameBuyAndSellToken) - )); - - assert!( - dbg!( - validator - .partial_validate(PreOrderData { - buy_token: Address::with_last_byte(2), - sell_token: Address::with_last_byte(2), - ..order() - }) - .await - ) - .is_ok() - ); - } - #[tokio::test] async fn pre_validate_same_tokens_allow_sell() { let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); From 73682d54acbd253ef32ffc7ec900cc8e892a2ca8 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 10 Dec 2025 16:33:08 +0100 Subject: [PATCH 21/39] fmt --- crates/solvers/src/domain/solver.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs index 59f75197f0..0cdfd39042 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -174,9 +174,7 @@ impl Inner { // how many units of buy_token are bought for one unit of sell_token // (buy_amount / sell_amount). let price = f64::from(self.native_token_price_estimation_amount) - / f64::from(route - .input() - .amount); + / f64::from(route.input().amount); let Some(price) = to_normalized_price(price) else { continue; }; From 2bcb29eb7a9e5f822d6bd19d977286f3dc459fd7 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 10 Dec 2025 16:35:42 +0100 Subject: [PATCH 22/39] Remove unused QuoteComputationError It was an artifact left out after rebasing --- crates/solvers/src/boundary/baseline.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/solvers/src/boundary/baseline.rs b/crates/solvers/src/boundary/baseline.rs index 77e577512e..69b4282a30 100644 --- a/crates/solvers/src/boundary/baseline.rs +++ b/crates/solvers/src/boundary/baseline.rs @@ -21,8 +21,6 @@ pub struct Solver<'a> { liquidity: HashMap, } -pub struct RouteComputationError; - impl<'a> Solver<'a> { pub fn new( weth: ð::WethAddress, From b7837cbefb992c52d8825b470976b96cdb7512a5 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 11:15:04 +0100 Subject: [PATCH 23/39] Change same token validation to return Result Cover placing order without quote with onchain verification Add helpful comments --- crates/driver/src/domain/quote.rs | 6 +-- crates/e2e/tests/e2e/submission.rs | 66 ++++++++++++++++++++++++++- crates/shared/src/order_validation.rs | 38 +++++++-------- crates/solvers/src/domain/solver.rs | 2 + 4 files changed, 88 insertions(+), 24 deletions(-) diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs index 18a74a283e..19c1dea42e 100644 --- a/crates/driver/src/domain/quote.rs +++ b/crates/driver/src/domain/quote.rs @@ -85,10 +85,10 @@ impl Order { liquidity: &infra::liquidity::Fetcher, tokens: &infra::tokens::Fetcher, ) -> Result { - let liquidity = match (solver.liquidity(), self.token_liquidity()) { - (solver::Liquidity::Fetch, pairs) if !pairs.is_empty() => { + let liquidity = match solver.liquidity() { + solver::Liquidity::Fetch => { liquidity - .fetch(&pairs, infra::liquidity::AtBlock::Recent) + .fetch(&self.token_liquidity(), infra::liquidity::AtBlock::Recent) .await } _ => Default::default(), diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index 5b5ed8a72a..aa11c85cf6 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -173,12 +173,18 @@ async fn test_submit_same_sell_and_buy_token_order_without_quote(web3: Web3) { .await .expect("Must be able to disable automine"); + let initial_balance = token.balanceOf(trader.address()).call().await.unwrap(); + assert_eq!(initial_balance, eth(10)); + + let sell_amount = eth(1); // Sell 1 wei + let buy_amount = eth(1) - U256::from(10).pow(U256::from(16)); // For 0.99 wei, for order to execute + tracing::info!("Placing order"); let order = OrderCreation { sell_token: *token.address(), - sell_amount: eth(1), + sell_amount, buy_token: *token.address(), - buy_amount: eth(1), + buy_amount, valid_to: model::time::now_in_epoch_seconds() + 300, kind: OrderKind::Sell, ..Default::default() @@ -189,6 +195,62 @@ async fn test_submit_same_sell_and_buy_token_order_without_quote(web3: Web3) { SecretKeyRef::from(&SecretKey::from_slice(trader.private_key()).unwrap()), ); services.create_order(&order).await.unwrap(); + // Start tracking confirmed blocks so we can find the transaction later + let block_stream = web3 + .eth_filter() + .create_blocks_filter() + .await + .expect("must be able to create blocks filter") + .stream(Duration::from_millis(50)); + + tracing::info!("Waiting for trade."); + onchain.mint_block().await; + + // Wait for settlement tx to appear in txpool + wait_for_condition(TIMEOUT, || async { + get_pending_tx(solver.account().address(), &web3) + .await + .is_some() + }) + .await + .unwrap(); + + // Continue mining to confirm the settlement + web3.api::>() + .set_automine_enabled(true) + .await + .expect("Must be able to enable automine"); + + // Wait for the settlement to be confirmed on chain + let tx = tokio::time::timeout( + Duration::from_secs(5), + get_confirmed_transaction(solver.account().address(), &web3, block_stream), + ) + .await + .unwrap(); + + // Verify the transaction is to the settlement contract (not a cancellation) + assert_eq!( + tx.to, + Some(onchain.contracts().gp_settlement.address().into_legacy()) + ); + + // Verify that the balance changed (settlement happened on chain) + let trade_happened = || async { + let balance = token.balanceOf(trader.address()).call().await.unwrap(); + // Balance should change due to fees even if sell token == buy token + balance != initial_balance + }; + wait_for_condition(TIMEOUT, trade_happened).await.unwrap(); + + let final_balance = token.balanceOf(trader.address()).call().await.unwrap(); + tracing::info!(?initial_balance, ?final_balance, "Trade completed"); + + // Verify that the balance changed (settlement happened on chain) + assert!( + final_balance < initial_balance, + "Final balance should be smaller than initial balance due to fees" + ); } async fn test_execute_same_sell_and_buy_token(web3: Web3) { diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index f9df2bda07..103ead8517 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -222,15 +222,25 @@ pub enum SameTokensPolicy { } impl SameTokensPolicy { - fn validate_same_sell_and_buy_token(&self, order: &PreOrderData) -> bool { + fn validate_same_sell_and_buy_token( + &self, + order: &PreOrderData, + native_token: &Address, + ) -> Result<(), PartialValidationError> { + // Check for orders selling wrapped native token for native token. + if &order.sell_token == native_token && order.buy_token == BUY_ETH_ADDRESS { + return Err(PartialValidationError::SameBuyAndSellToken); + } + if order.sell_token != order.buy_token { - return true; + return Ok(()); + } + + match (self, order.kind) { + // (Self::Allow, _) | To be implemented in https://github.com/cowprotocol/services/issues/3963 + (Self::AllowSell, OrderKind::Sell) => Ok(()), + _ => Err(PartialValidationError::SameBuyAndSellToken), } - matches!( - (self, order.kind), - /* (Self::Allow, _) | To be implemented in https://github.com/cowprotocol/services/issues/3963 */ - (Self::AllowSell, OrderKind::Sell) - ) } } @@ -505,18 +515,8 @@ impl OrderValidating for OrderValidator { } self.validity_configuration.validate_period(&order)?; - - if !self - .same_tokens_policy - .validate_same_sell_and_buy_token(&order) - { - return Err(PartialValidationError::SameBuyAndSellToken); - } - - // Check for orders selling wrapped native token for native token. - if &order.sell_token == self.native_token.address() && order.buy_token == BUY_ETH_ADDRESS { - return Err(PartialValidationError::SameBuyAndSellToken); - } + self.same_tokens_policy + .validate_same_sell_and_buy_token(&order, self.native_token.address())?; if order.sell_token == BUY_ETH_ADDRESS { return Err(PartialValidationError::InvalidNativeSellToken); diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs index 0cdfd39042..16c2da4b8e 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -193,6 +193,8 @@ impl Inner { let compute_solution = async |request: Request| -> Option { let wrappers = request.wrappers.clone(); let solution = if order.sell.token == order.buy.token { + // When sell and buy tokens are the same, the solution does not require routing + // and incurs no additional gas since the liquidity comes from the user let (input, mut output) = match order.side { Side::Sell => (order.sell, order.buy), Side::Buy => (order.buy, order.sell), From c97d196e336acc8f1169bce052d5e3a84128c736 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 12:48:29 +0100 Subject: [PATCH 24/39] Update crates/driver/src/domain/quote.rs Co-authored-by: ilya --- crates/driver/src/domain/quote.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs index 19c1dea42e..055a9c63d4 100644 --- a/crates/driver/src/domain/quote.rs +++ b/crates/driver/src/domain/quote.rs @@ -91,7 +91,7 @@ impl Order { .fetch(&self.token_liquidity(), infra::liquidity::AtBlock::Recent) .await } - _ => Default::default(), + solver::Liquidity::Skip => Default::default(), }; let auction = self From a955c22402d673f4ed6cb87062fef001b5c19ce3 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 15:12:41 +0100 Subject: [PATCH 25/39] Update crates/orderbook/src/arguments.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Duarte --- crates/orderbook/src/arguments.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/orderbook/src/arguments.rs b/crates/orderbook/src/arguments.rs index 5c4ebd240f..5fcfa08b03 100644 --- a/crates/orderbook/src/arguments.rs +++ b/crates/orderbook/src/arguments.rs @@ -146,8 +146,8 @@ pub struct Arguments { #[clap(flatten)] pub volume_fee_config: Option, - /// Controls if same sell and buy token orders are allowed - /// Disallowed by default + /// Controls if same sell and buy token orders are allowed. + /// Disallowed by default. #[clap(long, env, default_value = "disallow")] pub same_tokens_policy: shared::order_validation::SameTokensPolicy, } From 307e7fb6530337c406eb71dcd2199245adfba16f Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 15:13:56 +0100 Subject: [PATCH 26/39] Update crates/shared/src/price_estimation/factory.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Duarte --- crates/shared/src/price_estimation/factory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/shared/src/price_estimation/factory.rs b/crates/shared/src/price_estimation/factory.rs index 9f4579b0e2..a35edcbc09 100644 --- a/crates/shared/src/price_estimation/factory.rs +++ b/crates/shared/src/price_estimation/factory.rs @@ -307,7 +307,7 @@ impl<'a> PriceEstimatorFactory<'a> { ) } - /// Creates a SanitizedPRiceEstimator that is used for native price + /// Creates a SanitizedPriceEstimator that is used for native price /// estimations fn sanitized_native_price( &self, From d2cbf6e3666dbb681a8b58a3292424a96420eff2 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 15:14:14 +0100 Subject: [PATCH 27/39] Update crates/shared/src/price_estimation/sanitized.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Duarte --- crates/shared/src/price_estimation/sanitized.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/shared/src/price_estimation/sanitized.rs b/crates/shared/src/price_estimation/sanitized.rs index 08546218a8..a7f8827e64 100644 --- a/crates/shared/src/price_estimation/sanitized.rs +++ b/crates/shared/src/price_estimation/sanitized.rs @@ -491,7 +491,7 @@ mod tests { verification: Default::default(), sell_token: Address::with_last_byte(1), buy_token: Address::with_last_byte(1), - in_amount: NonZeroU256::try_from(1).unwrap(), + in_amount: Default::default(), kind: OrderKind::Sell, block_dependent: false, timeout: HEALTHY_PRICE_ESTIMATION_TIME, From f690b2962273e9e20dae98677a0cd5d30143d64b Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 15:14:33 +0100 Subject: [PATCH 28/39] Update crates/shared/src/order_validation.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Duarte --- crates/shared/src/order_validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 103ead8517..35d8670d39 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -1213,7 +1213,7 @@ mod tests { #[tokio::test] async fn pre_validate_ok() { - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); + let native_token = WETH9::Instance::new(Address::repeat_byte(0xef), ethrpc::mock::web3().alloy); let validity_configuration = OrderValidPeriodConfiguration { min: Duration::from_secs(1), max_market: Duration::from_secs(100), From 75273a245af08cfc628f403c1a919ae1fe83db35 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 15:14:41 +0100 Subject: [PATCH 29/39] Update crates/shared/src/order_validation.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Duarte --- crates/shared/src/order_validation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 35d8670d39..05719bddb9 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -1303,7 +1303,7 @@ mod tests { #[tokio::test] async fn pre_validate_same_tokens_allow_sell() { - let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().alloy); + let native_token = WETH9::Instance::new(Address::repeat_byte(0xef), ethrpc::mock::web3().alloy); let validity_configuration = OrderValidPeriodConfiguration { min: Duration::from_secs(1), max_market: Duration::from_secs(100), From ec5a05537a625f5a6e26222bdeba83543bd824a0 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 15:15:42 +0100 Subject: [PATCH 30/39] Update crates/solvers/src/domain/solver.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Duarte --- crates/solvers/src/domain/solver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs index 16c2da4b8e..bfe83d1e7b 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -219,7 +219,7 @@ impl Inner { liquidity: segment.liquidity.clone(), input: segment.input, output: segment.output, - // TODO does the baseline solver know about this + // NOTE(m-sz): does the baseline solver know about this // optimization? internalize: false, }, From 348f58422b6607416ad0561d81cb4715f04e22fc Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 15:23:40 +0100 Subject: [PATCH 31/39] fmt --- crates/shared/src/order_validation.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 05719bddb9..5395ba1219 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -1213,7 +1213,8 @@ mod tests { #[tokio::test] async fn pre_validate_ok() { - let native_token = WETH9::Instance::new(Address::repeat_byte(0xef), ethrpc::mock::web3().alloy); + let native_token = + WETH9::Instance::new(Address::repeat_byte(0xef), ethrpc::mock::web3().alloy); let validity_configuration = OrderValidPeriodConfiguration { min: Duration::from_secs(1), max_market: Duration::from_secs(100), @@ -1303,7 +1304,8 @@ mod tests { #[tokio::test] async fn pre_validate_same_tokens_allow_sell() { - let native_token = WETH9::Instance::new(Address::repeat_byte(0xef), ethrpc::mock::web3().alloy); + let native_token = + WETH9::Instance::new(Address::repeat_byte(0xef), ethrpc::mock::web3().alloy); let validity_configuration = OrderValidPeriodConfiguration { min: Duration::from_secs(1), max_market: Duration::from_secs(100), From 2656b984ec6065da1344fb7d517e4acfb0523b81 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Thu, 11 Dec 2025 15:52:16 +0100 Subject: [PATCH 32/39] NonZeroU256 usage fix in tests after merging --- crates/e2e/tests/e2e/place_order_with_quote.rs | 2 +- crates/e2e/tests/e2e/submission.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index b0b3200e5b..aa47a62b10 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -147,7 +147,7 @@ async fn disabled_same_sell_and_buy_token_order_feature(web3: Web3) { .expect("Must be able to disable automine"); tracing::info!("Quoting"); - let quote_sell_amount = to_wei(1); + let quote_sell_amount = eth(1); let quote_request = OrderQuoteRequest { from: trader.address(), sell_token: *token.address(), diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index aa11c85cf6..8ab21b96fa 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -12,7 +12,7 @@ use { quote::{OrderQuoteRequest, OrderQuoteSide, SellAmount}, signature::EcdsaSigningScheme, }, - number::nonzero::U256 as NonZeroU256, + number::nonzero::NonZeroU256, secp256k1::SecretKey, shared::ethrpc::Web3, std::time::Duration, @@ -297,7 +297,7 @@ async fn test_execute_same_sell_and_buy_token(web3: Web3) { buy_token: *token.address(), side: OrderQuoteSide::Sell { sell_amount: SellAmount::BeforeFee { - value: NonZeroU256::try_from(quote_sell_amount.into_legacy()).unwrap(), + value: NonZeroU256::try_from(quote_sell_amount).unwrap(), }, }, ..Default::default() From 295e56bd7ef59c7e711e6265eb5a5e5d4d3a1339 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 12 Dec 2025 12:33:45 +0100 Subject: [PATCH 33/39] Update crates/orderbook/src/arguments.rs Co-authored-by: Martin Magnus --- crates/orderbook/src/arguments.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/orderbook/src/arguments.rs b/crates/orderbook/src/arguments.rs index 5fcfa08b03..8ca891cde8 100644 --- a/crates/orderbook/src/arguments.rs +++ b/crates/orderbook/src/arguments.rs @@ -294,7 +294,7 @@ impl std::fmt::Display for Arguments { "active_order_competition_threshold: {active_order_competition_threshold}" )?; writeln!(f, "volume_fee_config: {volume_fee_config:?}")?; - writeln!(f, "same_tokens_policy {same_tokens_policy:?}")?; + writeln!(f, "same_tokens_policy: {same_tokens_policy:?}")?; Ok(()) } From 1476ff9038dbcbc4f40d7d211247151ce364602d Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 12 Dec 2025 12:34:00 +0100 Subject: [PATCH 34/39] Update crates/e2e/tests/e2e/submission.rs Co-authored-by: Martin Magnus --- crates/e2e/tests/e2e/submission.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index 8ab21b96fa..8a7cbcbb26 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -176,7 +176,7 @@ async fn test_submit_same_sell_and_buy_token_order_without_quote(web3: Web3) { let initial_balance = token.balanceOf(trader.address()).call().await.unwrap(); assert_eq!(initial_balance, eth(10)); - let sell_amount = eth(1); // Sell 1 wei + let sell_amount = eth(1); // Sell 1 eth let buy_amount = eth(1) - U256::from(10).pow(U256::from(16)); // For 0.99 wei, for order to execute tracing::info!("Placing order"); From 0ec429115245169e107ae00bd9fb5053503707b2 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Fri, 12 Dec 2025 15:16:13 +0100 Subject: [PATCH 35/39] Address comments --- crates/shared/src/price_estimation/sanitized.rs | 8 +++----- crates/solvers/src/domain/solver.rs | 9 ++++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/shared/src/price_estimation/sanitized.rs b/crates/shared/src/price_estimation/sanitized.rs index 6a259bba51..96b036cf30 100644 --- a/crates/shared/src/price_estimation/sanitized.rs +++ b/crates/shared/src/price_estimation/sanitized.rs @@ -66,11 +66,9 @@ impl PriceEstimating for SanitizedPriceEstimator { ) -> futures::future::BoxFuture<'_, super::PriceEstimateResult> { async move { self.handle_bad_tokens(&query).await?; - - // buy_token == sell_token => 1 to 1 conversion - // 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 + // When estimating native price the sell token is substituted by + // native one. In that case, the output amount of the price + // estimation can be trivially computed as the same amount as input if self.is_estimating_native_price && query.buy_token == query.sell_token { let estimation = Estimate { out_amount: query.in_amount.get(), diff --git a/crates/solvers/src/domain/solver.rs b/crates/solvers/src/domain/solver.rs index bfe83d1e7b..fc58155fe8 100644 --- a/crates/solvers/src/domain/solver.rs +++ b/crates/solvers/src/domain/solver.rs @@ -227,7 +227,7 @@ impl Inner { }) .collect(); let gas = route.gas() + self.solution_gas_offset; - let mut output = route.output().expect("route is not empty"); + let mut output = route.output(); // The baseline solver generates a path with swapping // for exact output token amounts. This leads to @@ -391,8 +391,11 @@ impl<'a> Route<'a> { self.segments[0].input } - fn output(&self) -> Option { - 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 gas(&self) -> eth::Gas { From f42b7ae1e496d0aa3d58f4a3d48bf99f7fe58b95 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 16 Dec 2025 15:56:08 +0100 Subject: [PATCH 36/39] Revert before/after balance calculation Introduce special case to handle sell=buy when calculating the out amounts. --- .../price_estimation/trade_verifier/mod.rs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/shared/src/price_estimation/trade_verifier/mod.rs b/crates/shared/src/price_estimation/trade_verifier/mod.rs index ee51494b71..f311a8aae2 100644 --- 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, Zero}, + bigdecimal::BigDecimal, contracts::alloy::{ GPv2Settlement, WETH9, @@ -265,6 +265,15 @@ impl TradeVerifier { *balance += number::conversions::alloy::u256_to_big_rational(&buy_amount) }); } + // It looks like the out_amount is 0 but only because the sell and buy tokens + // are the same. + if query.sell_token == query.buy_token { + summary.out_amount = query + .in_amount + .get() + .checked_sub(summary.out_amount) + .ok_or(Error::TooInaccurate)?; + } } tracing::debug!( @@ -768,10 +777,8 @@ fn add_balance_queries( query_balance_call.into(), ); - // query the balance as the first middle 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 query at the end of pre-interactions + settlement.interactions[0].push(interaction.clone()); // query balance right after we payed out all `buy_token` settlement.interactions[2].insert(0, interaction); settlement @@ -868,9 +875,7 @@ fn ensure_quote_accuracy( .get(&query.buy_token) .context("summary buy token is missing")?; - 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) - { + if (*sell_token_lost >= sell_token_lost_limit) || (*buy_token_lost >= buy_token_lost_limit) { return Err(Error::TooInaccurate); } From f46c77d308960b1b1abc7b5e0db71ecf44444456 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Tue, 16 Dec 2025 16:57:17 +0100 Subject: [PATCH 37/39] clippy --- crates/e2e/tests/e2e/place_order_with_quote.rs | 5 +---- crates/e2e/tests/e2e/submission.rs | 15 ++++++--------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index 98f5dae924..a199f3a7bd 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -125,10 +125,7 @@ async fn disabled_same_sell_and_buy_token_order_feature(web3: Web3) { let [solver] = onchain.make_solvers(10u64.eth()).await; let [trader] = onchain.make_accounts(10u64.eth()).await; let [token] = onchain - .deploy_tokens_with_weth_uni_v2_pools( - 1_000u64.eth().into_legacy(), - 1_000u64.eth().into_legacy(), - ) + .deploy_tokens_with_weth_uni_v2_pools(1_000u64.eth(), 1_000u64.eth()) .await; token.mint(trader.address(), 10u64.eth()).await; diff --git a/crates/e2e/tests/e2e/submission.rs b/crates/e2e/tests/e2e/submission.rs index 067a70f196..afce7daff0 100644 --- a/crates/e2e/tests/e2e/submission.rs +++ b/crates/e2e/tests/e2e/submission.rs @@ -2,7 +2,10 @@ use { ::alloy::primitives::U256, e2e::{nodes::local_node::TestNodeApi, setup::*}, ethcontract::{BlockId, H160, H256}, - ethrpc::alloy::{CallBuilderExt, conversions::IntoAlloy}, + ethrpc::alloy::{ + CallBuilderExt, + conversions::{IntoAlloy, IntoLegacy}, + }, futures::{Stream, StreamExt}, model::{ order::{OrderCreation, OrderKind}, @@ -140,10 +143,7 @@ async fn test_submit_same_sell_and_buy_token_order_without_quote(web3: Web3) { let [solver] = onchain.make_solvers(10u64.eth()).await; let [trader] = onchain.make_accounts(10u64.eth()).await; let [token] = onchain - .deploy_tokens_with_weth_uni_v2_pools( - 1_000u64.eth().into_legacy(), - 1_000u64.eth().into_legacy(), - ) + .deploy_tokens_with_weth_uni_v2_pools(1_000u64.eth(), 1_000u64.eth()) .await; token.mint(trader.address(), 10u64.eth()).await; @@ -259,10 +259,7 @@ async fn test_execute_same_sell_and_buy_token(web3: Web3) { let [solver] = onchain.make_solvers(10u64.eth()).await; let [trader] = onchain.make_accounts(10u64.eth()).await; let [token] = onchain - .deploy_tokens_with_weth_uni_v2_pools( - 1_000u64.eth().into_legacy(), - 1_000u64.eth().into_legacy(), - ) + .deploy_tokens_with_weth_uni_v2_pools(1_000u64.eth(), 1_000u64.eth()) .await; token.mint(trader.address(), 10u64.eth()).await; From 6a55fd91efeb105250b09027ca6fadb80affbb78 Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 17 Dec 2025 13:14:18 +0100 Subject: [PATCH 38/39] Fix out_amount calculation for sell=buy The out_amount had to be extended to I512, to account for negative outcomes of the previous calculation --- crates/number/src/conversions.rs | 25 ++++- .../price_estimation/trade_verifier/mod.rs | 93 +++++++++++++------ 2 files changed, 87 insertions(+), 31 deletions(-) diff --git a/crates/number/src/conversions.rs b/crates/number/src/conversions.rs index e9327b42c9..1318a8dd92 100644 --- a/crates/number/src/conversions.rs +++ b/crates/number/src/conversions.rs @@ -58,7 +58,7 @@ pub fn big_decimal_to_u256(big_decimal: &BigDecimal) -> Option { pub mod alloy { use { - alloy::primitives::U256, + alloy::primitives::{U256, aliases::I512}, anyhow::{Result, ensure}, bigdecimal::{BigDecimal, num_bigint::ToBigInt}, num::{BigInt, BigRational, BigUint, Zero, bigint::Sign}, @@ -104,6 +104,29 @@ pub mod alloy { let big_uint = u256_to_big_uint(u256); BigDecimal::from(BigInt::from(big_uint)) } + + pub fn i512_to_big_int(i512: &I512) -> BigInt { + BigInt::from_bytes_be( + match i512.sign() { + alloy::primitives::Sign::Positive => Sign::Plus, + alloy::primitives::Sign::Negative => Sign::Minus, + }, + &i512.abs().to_be_bytes::<64>(), + ) + } + + pub fn i512_to_big_rational(input: &I512) -> BigRational { + BigRational::new(i512_to_big_int(input), 1.into()) + } + + // TODO: Figure out a nicer way to convert I512 to U256, as a follow-up task + pub fn i512_to_u256(input: &I512) -> Result { + anyhow::ensure!(input >= &I512::ZERO, "input is not negative"); + anyhow::ensure!(input < &I512::from(U256::MAX), "U256::MAX fits into I512"); + Ok(alloy::primitives::U256::from_be_slice( + &input.to_be_bytes::<64>()[32..], + )) + } } pub fn rational_to_big_decimal(value: &Ratio) -> BigDecimal diff --git a/crates/shared/src/price_estimation/trade_verifier/mod.rs b/crates/shared/src/price_estimation/trade_verifier/mod.rs index f311a8aae2..849aca9211 100644 --- a/crates/shared/src/price_estimation/trade_verifier/mod.rs +++ b/crates/shared/src/price_estimation/trade_verifier/mod.rs @@ -17,7 +17,7 @@ use { }, }, ::alloy::sol_types::SolCall, - alloy::primitives::{Address, address}, + alloy::primitives::{Address, address, aliases::I512}, anyhow::{Context, Result, anyhow}, bigdecimal::BigDecimal, contracts::alloy::{ @@ -38,7 +38,10 @@ use { }, num::BigRational, number::{ - conversions::{alloy::u256_to_big_rational, big_decimal_to_big_rational}, + conversions::{ + alloy::{i512_to_u256, u256_to_big_rational}, + big_decimal_to_big_rational, + }, nonzero::NonZeroU256, }, std::{ @@ -240,8 +243,8 @@ impl TradeVerifier { // settlement buffers to make the quote happen. When the settlement contract // itself is the trader or receiver these values need to be adjusted slightly. let (sell_amount, buy_amount) = match query.kind { - OrderKind::Sell => (query.in_amount.get(), summary.out_amount), - OrderKind::Buy => (summary.out_amount, query.in_amount.get()), + OrderKind::Sell => (I512::from(query.in_amount.get()), summary.out_amount), + OrderKind::Buy => (summary.out_amount, I512::from(query.in_amount.get())), }; // It looks like the contract lost a lot of sell tokens but only because it was @@ -251,7 +254,7 @@ impl TradeVerifier { .tokens_lost .entry(query.sell_token) .and_modify(|balance| { - *balance -= number::conversions::alloy::u256_to_big_rational(&sell_amount) + *balance -= number::conversions::alloy::i512_to_big_rational(&sell_amount) }); } // It looks like the contract gained a lot of buy tokens (negative loss) but @@ -262,17 +265,48 @@ impl TradeVerifier { .tokens_lost .entry(query.buy_token) .and_modify(|balance| { - *balance += number::conversions::alloy::u256_to_big_rational(&buy_amount) + *balance += number::conversions::alloy::i512_to_big_rational(&buy_amount) }); } // It looks like the out_amount is 0 but only because the sell and buy tokens // are the same. + // + // The swap simulation computes the out_amount like this: + // sell order => trader_balance_before - trader_balance_after + // buy_order => trader_balance_after - trader_balance_before + // + // In case of sell=buy, the balance is only ever getting smaller, as the trader + // will always get less tokens out, which causes the above calculations to be + // wrong, and result in a negative number for sell order + // + // Example sell order: + // Trader having 1 ETH in their account, selling 0.3 ETH, with tx hooks cost of + // 0.1 ETH: in_amount = 0.3 ETH + // trader_balance_before = 1 ETH + // trader_balance_after = 0.9 ETH + // out_amount = 0.9 ETH - 1 ETH = -0.1 ETH + // The correct out_amount = 0.3 ETH (input) + (-0.1ETH) (out_amount) = 0.2 ETH + // + // Meaning they can sell 0.3 ETH for 0.2 ETH, considering the costs + // + // Example buy order: + // Trader having 1 ETH in their account, buying 1 wei, with tx hooks cost of 0.1 + // ETH in_amount = 1 wei + // trader_balance_before = 1 ETH + // trader_balance_after = 0.9 ETH + // out_amount = 1 ETH - 0.9 ETH = 0.1 ETH + // The correct out_amount = 1 wei (input) + 0.1 ETH (out_amount) = 0.1000...1 + // ETH + // + // Meaning they can buy 1 wei for 0.1ETH + 1 wei, considering the costs + // + // The general formula being: correct_out_amount = query.input + out_amount + // if query.sell_token == query.buy_token { - summary.out_amount = query - .in_amount - .get() - .checked_sub(summary.out_amount) - .ok_or(Error::TooInaccurate)?; + summary.out_amount = I512::from(query.in_amount.get()) + summary.out_amount; + } else if summary.out_amount < I512::ZERO { + tracing::error!("Trade out amount is negative"); + return Err(Error::TooInaccurate); } } @@ -284,7 +318,7 @@ impl TradeVerifier { verified_out_amount = ?summary.out_amount, promised_gas = trade.gas_estimate(), verified_gas = ?summary.gas_used, - out_diff = ?out_amount.abs_diff(summary.out_amount), + out_diff = ?(I512::from(*out_amount) - summary.out_amount).abs(), ?query, ?verification, "verified quote", @@ -791,7 +825,7 @@ struct SettleOutput { gas_used: alloy::primitives::U256, /// `out_amount` perceived by the trader (sell token for buy orders or buy /// token for sell order) - out_amount: alloy::primitives::U256, + out_amount: alloy::primitives::aliases::I512, /// Tokens difference of the settlement contract before and after the trade. tokens_lost: HashMap, } @@ -817,8 +851,8 @@ impl SettleOutput { i += 1; } - let trader_balance_before = queriedBalances[i]; - let trader_balance_after = queriedBalances[i + 1]; + let trader_balance_before = I512::from(queriedBalances[i]); + let trader_balance_after = I512::from(queriedBalances[i + 1]); i += 2; // Get settlement contract balances after the trade @@ -832,11 +866,10 @@ impl SettleOutput { let out_amount = match kind { // for sell orders we track the buy_token amount which increases during the settlement - OrderKind::Sell => trader_balance_after.checked_sub(trader_balance_before), + OrderKind::Sell => trader_balance_after - trader_balance_before, // for buy orders we track the sell_token amount which decreases during the settlement - OrderKind::Buy => trader_balance_before.checked_sub(trader_balance_after), + OrderKind::Buy => trader_balance_before - trader_balance_after, }; - let out_amount = out_amount.context("underflow during out_amount computation")?; Ok(SettleOutput { gas_used: gasUsed, @@ -856,12 +889,12 @@ fn ensure_quote_accuracy( ) -> std::result::Result { // amounts verified by the simulation let (sell_amount, buy_amount) = match query.kind { - OrderKind::Buy => (summary.out_amount, query.in_amount.get()), - OrderKind::Sell => (query.in_amount.get(), summary.out_amount), + OrderKind::Buy => (summary.out_amount, I512::from(query.in_amount.get())), + OrderKind::Sell => (I512::from(query.in_amount.get()), summary.out_amount), }; let (sell_amount, buy_amount) = ( - number::conversions::alloy::u256_to_big_rational(&sell_amount), - number::conversions::alloy::u256_to_big_rational(&buy_amount), + number::conversions::alloy::i512_to_big_rational(&sell_amount), + number::conversions::alloy::i512_to_big_rational(&buy_amount), ); let sell_token_lost_limit = inaccuracy_limit * &sell_amount; let buy_token_lost_limit = inaccuracy_limit * &buy_amount; @@ -880,7 +913,7 @@ fn ensure_quote_accuracy( } Ok(Estimate { - out_amount: summary.out_amount, + out_amount: i512_to_u256(&summary.out_amount)?, gas: summary.gas_used.saturating_to(), solver: trade.solver().into_legacy(), verified: true, @@ -939,7 +972,7 @@ mod tests { }; let summary = SettleOutput { gas_used: U256::ZERO, - out_amount: U256::from(2_000), + out_amount: I512::try_from(2_000).unwrap(), tokens_lost, }; let estimate = @@ -952,7 +985,7 @@ mod tests { }; let summary = SettleOutput { gas_used: U256::ZERO, - out_amount: U256::from(2_000), + out_amount: I512::try_from(2_000).unwrap(), tokens_lost, }; @@ -967,7 +1000,7 @@ mod tests { }; let summary = SettleOutput { gas_used: U256::ZERO, - out_amount: U256::from(2_000), + out_amount: I512::try_from(2_000).unwrap(), tokens_lost, }; let estimate = @@ -981,7 +1014,7 @@ mod tests { let sell_more = SettleOutput { gas_used: U256::ZERO, - out_amount: U256::from(2_000), + out_amount: I512::try_from(2_000).unwrap(), tokens_lost, }; @@ -1001,7 +1034,7 @@ mod tests { let pay_out_more = SettleOutput { gas_used: U256::ZERO, - out_amount: U256::from(2_000), + out_amount: I512::try_from(2_000).unwrap(), tokens_lost, }; @@ -1021,7 +1054,7 @@ mod tests { let sell_less = SettleOutput { gas_used: U256::ZERO, - out_amount: U256::from(2_000), + out_amount: I512::try_from(2_000).unwrap(), tokens_lost, }; // Ending up with surplus in the buffers is always fine @@ -1036,7 +1069,7 @@ mod tests { let pay_out_less = SettleOutput { gas_used: U256::ZERO, - out_amount: U256::from(2_000), + out_amount: I512::try_from(2_000).unwrap(), tokens_lost, }; // Ending up with surplus in the buffers is always fine From c99d3518b15843fe43f0776642563b25b8ad4c3f Mon Sep 17 00:00:00 2001 From: Marcin Szymczak Date: Wed, 17 Dec 2025 14:13:06 +0100 Subject: [PATCH 39/39] Minor changes: comments and log level --- crates/number/src/conversions.rs | 4 ++-- crates/shared/src/price_estimation/trade_verifier/mod.rs | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/number/src/conversions.rs b/crates/number/src/conversions.rs index 1318a8dd92..b61de44bb3 100644 --- a/crates/number/src/conversions.rs +++ b/crates/number/src/conversions.rs @@ -121,8 +121,8 @@ pub mod alloy { // TODO: Figure out a nicer way to convert I512 to U256, as a follow-up task pub fn i512_to_u256(input: &I512) -> Result { - anyhow::ensure!(input >= &I512::ZERO, "input is not negative"); - anyhow::ensure!(input < &I512::from(U256::MAX), "U256::MAX fits into I512"); + anyhow::ensure!(input >= &I512::ZERO, "Negative input value"); + anyhow::ensure!(input < &I512::from(U256::MAX), "Input exceeds U256::MAX"); Ok(alloy::primitives::U256::from_be_slice( &input.to_be_bytes::<64>()[32..], )) diff --git a/crates/shared/src/price_estimation/trade_verifier/mod.rs b/crates/shared/src/price_estimation/trade_verifier/mod.rs index 849aca9211..6c2ca1a891 100644 --- a/crates/shared/src/price_estimation/trade_verifier/mod.rs +++ b/crates/shared/src/price_estimation/trade_verifier/mod.rs @@ -268,9 +268,7 @@ impl TradeVerifier { *balance += number::conversions::alloy::i512_to_big_rational(&buy_amount) }); } - // It looks like the out_amount is 0 but only because the sell and buy tokens - // are the same. - // + // The swap simulation computes the out_amount like this: // sell order => trader_balance_before - trader_balance_after // buy_order => trader_balance_after - trader_balance_before @@ -301,11 +299,10 @@ impl TradeVerifier { // Meaning they can buy 1 wei for 0.1ETH + 1 wei, considering the costs // // The general formula being: correct_out_amount = query.input + out_amount - // if query.sell_token == query.buy_token { summary.out_amount = I512::from(query.in_amount.get()) + summary.out_amount; } else if summary.out_amount < I512::ZERO { - tracing::error!("Trade out amount is negative"); + tracing::debug!("Trade out amount is negative"); return Err(Error::TooInaccurate); } }