diff --git a/crates/autopilot/src/domain/fee/mod.rs b/crates/autopilot/src/domain/fee/mod.rs index 2b214bb72e..38ab745eb4 100644 --- a/crates/autopilot/src/domain/fee/mod.rs +++ b/crates/autopilot/src/domain/fee/mod.rs @@ -115,7 +115,7 @@ impl ProtocolFees { .iter() // TODO: support multiple fee policies .find_map(|fee_policy| { - let outside_market_price = boundary::is_order_outside_market_price(&order_, "e_); + let outside_market_price = boundary::is_order_outside_market_price(&order_, "e_, order.data.kind); match (outside_market_price, &fee_policy.order_class) { (_, OrderClass::Any) => Some(&fee_policy.policy), (true, OrderClass::Limit) => Some(&fee_policy.policy), diff --git a/crates/database/src/orders.rs b/crates/database/src/orders.rs index 09804e43e2..b6ea8d4506 100644 --- a/crates/database/src/orders.rs +++ b/crates/database/src/orders.rs @@ -720,7 +720,7 @@ pub async fn count_limit_orders_by_owner( pub struct OrderWithQuote { pub order_buy_amount: BigDecimal, pub order_sell_amount: BigDecimal, - pub order_fee_amount: BigDecimal, + pub order_kind: OrderKind, pub quote_buy_amount: BigDecimal, pub quote_sell_amount: BigDecimal, pub quote_gas_amount: f64, @@ -737,7 +737,7 @@ pub async fn user_orders_with_quote( const QUERY: &str = const_format::concatcp!( "SELECT o_quotes.sell_amount as quote_sell_amount, o.sell_amount as order_sell_amount,", " o_quotes.buy_amount as quote_buy_amount, o.buy_amount as order_buy_amount,", - " o.fee_amount as order_fee_amount, o_quotes.gas_amount as quote_gas_amount,", + " o.kind as order_kind, o_quotes.gas_amount as quote_gas_amount,", " o_quotes.gas_price as quote_gas_price, o_quotes.sell_token_price as quote_sell_token_price", " FROM (", " SELECT *", diff --git a/crates/driver/src/domain/competition/solution/fee.rs b/crates/driver/src/domain/competition/solution/fee.rs index 83ee4b6bcd..34d35707dc 100644 --- a/crates/driver/src/domain/competition/solution/fee.rs +++ b/crates/driver/src/domain/competition/solution/fee.rs @@ -225,18 +225,23 @@ pub struct Quote { /// - test_adjust_quote_to_in_market_sell_order_limits /// - test_adjust_quote_to_in_market_buy_order_limits pub fn adjust_quote_to_order_limits(order: Order, quote: Quote) -> Result { - let quote_sell_amount = quote - .sell_amount - .checked_add(quote.fee_amount) - .ok_or(Math::Overflow)?; - match order.side { Side::Sell => { - let scaled_buy_amount = quote + let quote_buy_amount = quote .buy_amount + .checked_sub( + quote + .fee_amount + .checked_mul(quote.buy_amount) + .ok_or(Math::Overflow)? + .checked_div(quote.sell_amount) + .ok_or(Math::DivisionByZero)?, + ) + .ok_or(Math::Negative)?; + let scaled_buy_amount = quote_buy_amount .checked_mul(order.sell_amount) .ok_or(Math::Overflow)? - .checked_div(quote_sell_amount) + .checked_div(quote.sell_amount) .ok_or(Math::DivisionByZero)?; let buy_amount = order.buy_amount.max(scaled_buy_amount); Ok(PriceLimits { @@ -245,6 +250,10 @@ pub fn adjust_quote_to_order_limits(order: Order, quote: Quote) -> Result { + let quote_sell_amount = quote + .sell_amount + .checked_add(quote.fee_amount) + .ok_or(Math::Overflow)?; let scaled_sell_amount = quote_sell_amount .checked_mul(order.buy_amount) .ok_or(Math::Overflow)? @@ -294,6 +303,11 @@ mod tests { limit.sell.0, order.sell_amount, "Sell amount should match order sell amount for sell orders." ); + assert_eq!( + limit.buy.0, + to_wei(19), + "Buy amount should be equal to order buy amount for out of market orders" + ); } #[test] @@ -315,19 +329,24 @@ mod tests { limit.buy.0, order.buy_amount, "Buy amount should match order buy amount for buy orders." ); + assert_eq!( + limit.sell.0, + to_wei(20), + "Sell amount should be equal to order sell amount for out of market orders." + ); } #[test] fn test_adjust_quote_to_in_market_sell_order_limits() { let order = Order { sell_amount: to_wei(10), - buy_amount: to_wei(20), + buy_amount: to_wei(10), side: Side::Sell, }; let quote = Quote { - sell_amount: to_wei(9), + sell_amount: to_wei(10), buy_amount: to_wei(25), - fee_amount: to_wei(1), + fee_amount: to_wei(2), }; let limit = adjust_quote_to_order_limits(order.clone(), quote.clone()).unwrap(); @@ -337,8 +356,9 @@ mod tests { "Sell amount should be taken from the order for sell orders in market price." ); assert_eq!( - limit.buy.0, quote.buy_amount, - "Buy amount should reflect the improved market condition from the quote." + limit.buy.0, + to_wei(20), + "Buy amount should be equal to quoted buy amount but reduced by fee." ); } @@ -359,9 +379,8 @@ mod tests { assert_eq!( limit.sell.0, - quote.sell_amount + quote.fee_amount, - "Sell amount should reflect the improved market condition from the quote for buy \ - orders." + to_wei(18), + "Sell amount should match quoted buy amount increased by fee" ); assert_eq!( limit.buy.0, order.buy_amount, diff --git a/crates/driver/src/tests/cases/protocol_fees.rs b/crates/driver/src/tests/cases/protocol_fees.rs index 97c697c302..9d028bbf87 100644 --- a/crates/driver/src/tests/cases/protocol_fees.rs +++ b/crates/driver/src/tests/cases/protocol_fees.rs @@ -687,9 +687,9 @@ async fn price_improvement_fee_sell_in_market_order_not_capped() { // high enough so we don't get capped by volume fee max_volume_factor: 0.9, quote: Quote { - sell: 49.ether().into_wei(), + sell: 50.ether().into_wei(), buy: 50.ether().into_wei(), - network_fee: 1.ether().into_wei(), + network_fee: 4.ether().into_wei(), // 50 sell for 46 buy }, }; let test_case = TestCase { @@ -701,14 +701,14 @@ async fn price_improvement_fee_sell_in_market_order_not_capped() { side: order::Side::Sell, }, execution: Execution { - // Receive 10 ETH more than quoted, half of which gets captured by the protocol + // Receive 14 ETH more than quoted, half of which gets captured by the protocol solver: Amounts { sell: 50.ether().into_wei(), buy: 60.ether().into_wei(), }, driver: Amounts { sell: 50.ether().into_wei(), - buy: 55.ether().into_wei(), + buy: 53.ether().into_wei(), }, }, expected_score: 20.ether().into_wei(), @@ -991,9 +991,9 @@ async fn price_improvement_fee_partial_sell_in_market_order_not_capped() { // high enough so we don't get capped by volume fee max_volume_factor: 0.9, quote: Quote { - sell: 49.ether().into_wei(), + sell: 50.ether().into_wei(), buy: 50.ether().into_wei(), - network_fee: 1.ether().into_wei(), + network_fee: 5.ether().into_wei(), // 50 sell for 45 buy }, }; let test_case = TestCase { @@ -1005,14 +1005,16 @@ async fn price_improvement_fee_partial_sell_in_market_order_not_capped() { side: order::Side::Sell, }, execution: Execution { - // Receive 10 ETH more than quoted, half of which gets captured by the protocol + // Quote is 50 sell for 45 buy, which is equal to 20 sell for 18 buy + // Solver returns 20 sell for 30 buy, so the price improvement is 12 in buy token + // Receive 12 ETH more than quoted, half of which gets captured by the protocol solver: Amounts { sell: 20.ether().into_wei(), buy: 30.ether().into_wei(), }, driver: Amounts { sell: 20.ether().into_wei(), - buy: 25.ether().into_wei(), + buy: 24.ether().into_wei(), }, }, expected_score: 20.ether().into_wei(), diff --git a/crates/e2e/tests/e2e/protocol_fee.rs b/crates/e2e/tests/e2e/protocol_fee.rs index d578510a38..5792773824 100644 --- a/crates/e2e/tests/e2e/protocol_fee.rs +++ b/crates/e2e/tests/e2e/protocol_fee.rs @@ -379,30 +379,30 @@ async fn price_improvement_fee_sell_order_test(web3: Web3) { // 9871415430342266811 DAI, with executed_surplus_fee = 167058994203399 GNO // // Quote: 10000000000000000000 GNO for 9871580343970612988 DAI with - // 294580438010728 GNO fee. Equivalent to: (10000000000000000000 + - // 294580438010728) GNO for 9871580343970612988 DAI, then scaled to sell amount - // gives 10000000000000000000 GNO for 9871289555090525964 DAI + // 294580438010728 GNO fee. Equivalent to: 10000000000000000000 GNO for + // 9871580343970612988 * ((10000000000000000000 - 294580438010728) / + // 10000000000000000000) = 9871289546524454223 DAI // - // Price improvement over quote: 9871415430342266811 - 9871289555090525964 = - // 125875251741847 DAI. Protocol fee = 0.3 * 125875251741847 DAI = - // 37762575522554 DAI + // Price improvement over quote: 9871415430342266811 - 9871289546524454223 = + // 125883817812588 DAI. Protocol fee = 0.3 * 125883817812588 DAI = + // 37765145343776 DAI // - // Protocol fee in sell token: 37762575522554 DAI / 9871415430342266811 * - // (10000000000000000000 - 167058994203399) = 38253829890184 GNO + // Protocol fee in sell token: 37765145343776 DAI / 9871415430342266811 * + // (10000000000000000000 - 167058994203399) = 38256433142280 GNO // // Final execution is 10000000000000000000 GNO for (9871415430342266811 - - // 37762575522554) = 9871377667766744257 DAI, with 205312824093583 GNO fee + // 37765145343776) = 9871377665196923035 DAI, with 205315427345679 GNO fee // - // Settlement contract balance after execution = 205312824093583 GNO = - // 205312824093583 GNO * 9871377667766744257 / (10000000000000000000 - - // 205312824093583) = 202676203868731 DAI + // Settlement contract balance after execution = 205315427345679 GNO = + // 205315427345679 GNO * 9871377665196923035 / (10000000000000000000 - + // 205315427345679) = 202678773689953 DAI execute_test( web3.clone(), vec![ProtocolFeesConfig(vec![protocol_fee])], OrderKind::Sell, None, - 205312824093583u128.into(), - 202676203868731u128.into(), + 205315427345679u128.into(), + 202678773689953u128.into(), ) .await; } diff --git a/crates/orderbook/src/database/orders.rs b/crates/orderbook/src/database/orders.rs index dd52d877c5..00cf02a1d1 100644 --- a/crates/orderbook/src/database/orders.rs +++ b/crates/orderbook/src/database/orders.rs @@ -379,7 +379,7 @@ impl LimitOrderCounting for Postgres { &Amounts { sell: big_decimal_to_u256(&order_with_quote.order_sell_amount).unwrap(), buy: big_decimal_to_u256(&order_with_quote.order_buy_amount).unwrap(), - fee: big_decimal_to_u256(&order_with_quote.order_fee_amount).unwrap(), + fee: 0.into(), }, &Amounts { sell: big_decimal_to_u256(&order_with_quote.quote_sell_amount).unwrap(), @@ -391,6 +391,10 @@ impl LimitOrderCounting for Postgres { } .fee(), }, + match order_with_quote.order_kind { + DbOrderKind::Buy => model::order::OrderKind::Buy, + DbOrderKind::Sell => model::order::OrderKind::Sell, + }, ) }) .count() diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 2d21a9e9d6..3bf3209858 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -667,6 +667,7 @@ impl OrderValidating for OrderValidator { buy: quote.buy_amount, fee: quote.fee_amount, }, + data.kind, ) { tracing::debug!(%uid, ?owner, ?class, "order being flagged as outside market price"); (OrderClass::Limit, Some(quote)) @@ -695,6 +696,7 @@ impl OrderValidating for OrderValidator { buy: quote.buy_amount, fee: quote.fee_amount, }, + data.kind, ) { self.check_max_limit_orders(owner).await?; } @@ -721,6 +723,7 @@ impl OrderValidating for OrderValidator { buy: quote.buy_amount, fee: quote.fee_amount, }, + data.kind, ) { self.check_max_limit_orders(owner).await?; } @@ -925,11 +928,31 @@ pub struct Amounts { /// Checks whether or not an order's limit price is outside the market price /// specified by the quote. -/// -/// Note that this check only looks at the order's limit price and the market -/// price and is independent of amounts or trade direction. -pub fn is_order_outside_market_price(order: &Amounts, quote: &Amounts) -> bool { - (order.sell + order.fee).full_mul(quote.buy) < (quote.sell + quote.fee).full_mul(order.buy) +pub fn is_order_outside_market_price( + order: &Amounts, + quote: &Amounts, + kind: model::order::OrderKind, +) -> bool { + let check = move || match kind { + OrderKind::Buy => { + Some(order.sell.full_mul(quote.buy) < (quote.sell + quote.fee).full_mul(order.buy)) + } + OrderKind::Sell => { + let quote_buy = quote + .buy + .checked_sub(quote.fee.checked_mul(quote.buy)?.checked_div(quote.sell)?)?; + Some(order.sell.full_mul(quote_buy) < quote.sell.full_mul(order.buy)) + } + }; + + check().unwrap_or_else(|| { + tracing::warn!( + ?order, + ?quote, + "failed to check if order is outside market price" + ); + true + }) } pub fn convert_signing_scheme_into_quote_signing_scheme( @@ -2339,7 +2362,7 @@ mod tests { } #[test] - fn detects_market_orders() { + fn detects_market_orders_buy() { let quote = Quote { sell_amount: 90.into(), buy_amount: 100.into(), @@ -2359,6 +2382,7 @@ mod tests { buy: quote.buy_amount, fee: quote.fee_amount, }, + model::order::OrderKind::Buy, )); // willing to buy less than market price assert!(!is_order_outside_market_price( @@ -2372,6 +2396,61 @@ mod tests { buy: quote.buy_amount, fee: quote.fee_amount, }, + model::order::OrderKind::Buy, + )); + // wanting to buy more than market price + assert!(is_order_outside_market_price( + &Amounts { + sell: 100.into(), + buy: 1000.into(), + fee: 0.into(), + }, + &Amounts { + sell: quote.sell_amount, + buy: quote.buy_amount, + fee: quote.fee_amount, + }, + model::order::OrderKind::Buy, + )); + } + + #[test] + fn detects_market_orders_sell() { + // 1 to 1 conversion + let quote = Quote { + sell_amount: 100.into(), + buy_amount: 100.into(), + fee_amount: 10.into(), + ..Default::default() + }; + + // at market price + assert!(!is_order_outside_market_price( + &Amounts { + sell: 100.into(), + buy: 90.into(), + fee: 0.into(), + }, + &Amounts { + sell: quote.sell_amount, + buy: quote.buy_amount, + fee: quote.fee_amount, + }, + model::order::OrderKind::Sell, + )); + // willing to buy less than market price + assert!(!is_order_outside_market_price( + &Amounts { + sell: 100.into(), + buy: 80.into(), + fee: 0.into(), + }, + &Amounts { + sell: quote.sell_amount, + buy: quote.buy_amount, + fee: quote.fee_amount, + }, + model::order::OrderKind::Sell, )); // wanting to buy more than market price assert!(is_order_outside_market_price( @@ -2385,6 +2464,7 @@ mod tests { buy: quote.buy_amount, fee: quote.fee_amount, }, + model::order::OrderKind::Sell, )); } }