From 6dc40ebac162bc95753c5e595013c064f0343cd7 Mon Sep 17 00:00:00 2001 From: Mateo Date: Fri, 1 Mar 2024 11:54:10 +0100 Subject: [PATCH] Do not count "in-market" orders for the order limit #2433 --- crates/e2e/tests/e2e/limit_orders.rs | 92 ++++++++++++++++++++++ crates/shared/src/order_validation.rs | 107 ++++++++++++++++---------- 2 files changed, 160 insertions(+), 39 deletions(-) diff --git a/crates/e2e/tests/e2e/limit_orders.rs b/crates/e2e/tests/e2e/limit_orders.rs index 98a7138018..9584dd27d2 100644 --- a/crates/e2e/tests/e2e/limit_orders.rs +++ b/crates/e2e/tests/e2e/limit_orders.rs @@ -1,5 +1,6 @@ use { contracts::ERC20, + driver::domain::eth::NonZeroU256, e2e::{nodes::forked_node::ForkedNodeApi, setup::*, tx}, ethcontract::{prelude::U256, H160}, model::{ @@ -30,6 +31,12 @@ async fn local_node_too_many_limit_orders() { run_test(too_many_limit_orders_test).await; } +#[tokio::test] +#[ignore] +async fn local_node_limit_does_not_apply_to_in_market_orders_test() { + run_test(limit_does_not_apply_to_in_market_orders_test).await; +} + #[tokio::test] #[ignore] async fn local_node_mixed_limit_and_market_orders() { @@ -483,11 +490,96 @@ async fn too_many_limit_orders_test(web3: Web3) { &onchain.contracts().domain_separator, SecretKeyRef::from(&SecretKey::from_slice(trader.private_key()).unwrap()), ); + let (status, body) = services.create_order(&order).await.unwrap_err(); assert_eq!(status, 400); assert!(body.contains("TooManyLimitOrders")); } +async fn limit_does_not_apply_to_in_market_orders_test(web3: Web3) { + let mut onchain = OnchainComponents::deploy(web3.clone()).await; + + let [solver] = onchain.make_solvers(to_wei(1)).await; + let [trader] = onchain.make_accounts(to_wei(1)).await; + let [token_a] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) + .await; + token_a.mint(trader.address(), to_wei(100)).await; + + // Approve GPv2 for trading + tx!( + trader.account(), + token_a.approve(onchain.contracts().allowance, to_wei(101)) + ); + + // Place Orders + let services = Services::new(onchain.contracts()).await; + let solver_endpoint = + colocation::start_baseline_solver(onchain.contracts().weth.address()).await; + colocation::start_driver( + onchain.contracts(), + vec![colocation::SolverEngine { + name: "test_solver".into(), + account: solver, + endpoint: solver_endpoint, + }], + ); + services + .start_api(vec![ + "--max-limit-orders-per-user=1".into(), + "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver".to_string(), + ]) + .await; + + let quote_request = OrderQuoteRequest { + from: trader.address(), + sell_token: token_a.address(), + buy_token: onchain.contracts().weth.address(), + side: OrderQuoteSide::Sell { + sell_amount: SellAmount::BeforeFee { + value: NonZeroU256::try_from(to_wei(5)).unwrap(), + }, + }, + ..Default::default() + }; + let quote = services.submit_quote("e_request).await.unwrap(); + + let order = OrderCreation { + sell_token: token_a.address(), + sell_amount: quote.quote.sell_amount, + buy_token: onchain.contracts().weth.address(), + buy_amount: quote.quote.buy_amount.saturating_sub(to_wei(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()), + ); + let order_id = services.create_order(&order).await.unwrap(); + let limit_order = services.get_order(&order_id).await.unwrap(); + assert!(limit_order.metadata.class.is_limit()); + + // Place another "in-market" order in order to check it is not limited + let order = OrderCreation { + sell_token: token_a.address(), + sell_amount: quote.quote.sell_amount, + buy_token: onchain.contracts().weth.address(), + buy_amount: quote.quote.buy_amount.saturating_sub(to_wei(2)), + 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()); +} + async fn forked_mainnet_single_limit_order_test(web3: Web3) { let mut onchain = OnchainComponents::deployed(web3.clone()).await; let forked_node_api = web3.api::>(); diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 28c75582f9..4edcc4793b 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -345,20 +345,14 @@ impl OrderValidator { self } - async fn check_max_limit_orders( - &self, - owner: H160, - class: &OrderClass, - ) -> Result<(), ValidationError> { - if class.is_limit() { - let num_limit_orders = self - .limit_order_counter - .count(owner) - .await - .map_err(ValidationError::Other)?; - if num_limit_orders >= self.max_limit_orders_per_user { - return Err(ValidationError::TooManyLimitOrders); - } + async fn check_max_limit_orders(&self, owner: H160) -> Result<(), ValidationError> { + let num_limit_orders = self + .limit_order_counter + .count(owner) + .await + .map_err(ValidationError::Other)?; + if num_limit_orders >= self.max_limit_orders_per_user { + return Err(ValidationError::TooManyLimitOrders); } Ok(()) } @@ -582,22 +576,6 @@ impl OrderValidating for OrderValidator { additional_gas: app_data.inner.protocol.hooks.gas_limit(), verification, }; - let quote = match class { - OrderClass::Market => { - let fee = Some(data.fee_amount); - let quote = - get_quote_and_check_fee(&*self.quoter, "e_parameters, order.quote_id, fee) - .await?; - Some(quote) - } - OrderClass::Limit => { - let quote = - get_quote_and_check_fee(&*self.quoter, "e_parameters, order.quote_id, None) - .await?; - Some(quote) - } - OrderClass::Liquidity => None, - }; let min_balance = minimum_balance(&data).ok_or(ValidationError::SellAmountOverflow)?; @@ -647,6 +625,22 @@ impl OrderValidating for OrderValidator { }, } + let quote = match class { + OrderClass::Market => { + let fee = Some(data.fee_amount); + let quote = + get_quote_and_check_fee(&*self.quoter, "e_parameters, order.quote_id, fee) + .await?; + Some(quote) + } + OrderClass::Limit => { + let quote = + get_quote_and_check_fee(&*self.quoter, "e_parameters, order.quote_id, None) + .await?; + Some(quote) + } + OrderClass::Liquidity => None, + }; tracing::debug!( ?uid, ?order, @@ -656,8 +650,22 @@ impl OrderValidating for OrderValidator { // Check if we need to re-classify the market order if it is outside the market // price. We consider out-of-price orders as liquidity orders. See // . - let class = match (class, "e) { - (OrderClass::Market, Some(quote)) + let (class, quote) = match class { + // This has to be here in order to keep the previous behaviour + OrderClass::Market => { + let quote = get_quote_and_check_fee( + &*self.quoter, + "e_parameters, + order.quote_id, + Some(data.fee_amount), + ) + .await?; + tracing::debug!( + ?uid, + ?order, + ?quote, + "checking if order is outside market price" + ); if is_order_outside_market_price( &Amounts { sell: data.sell_amount, @@ -669,16 +677,37 @@ impl OrderValidating for OrderValidator { buy: quote.buy_amount, fee: quote.fee_amount, }, - ) => - { - tracing::debug!(%uid, ?owner, ?class, "order being flagged as outside market price"); - OrderClass::Liquidity + ) { + tracing::debug!(%uid, ?owner, ?class, "order being flagged as outside market price"); + (OrderClass::Liquidity, Some(quote)) + } else { + (class, Some(quote)) + } + } + OrderClass::Limit => { + let quote = + get_quote_and_check_fee(&*self.quoter, "e_parameters, order.quote_id, None) + .await?; + // If the order is "In-Market", check for the limit orders + if is_order_outside_market_price( + &Amounts { + sell: data.sell_amount, + buy: data.buy_amount, + fee: data.fee_amount, + }, + &Amounts { + sell: quote.sell_amount, + buy: quote.buy_amount, + fee: quote.fee_amount, + }, + ) { + self.check_max_limit_orders(owner).await?; + } + (class, Some(quote)) } - (_, _) => class, + OrderClass::Liquidity => (class, None), }; - self.check_max_limit_orders(owner, &class).await?; - let order = Order { metadata: OrderMetadata { owner,