Skip to content

Commit

Permalink
Fix quote fee handling in market price (#2572)
Browse files Browse the repository at this point in the history
# Description
Fixes the way we scale quote sell and quote buy amounts to consider
quote fee.

Quoting @fhenneke and his example: 

> Suppose there is a quote for an order selling 100 DAI to USDC. The
quote might be something like sell amount: 100 DAI, buy amount 100 USDC,
network fee 50 DAI.
> The logic currently used by the solver team: the reference buy amount
for computing price improvement is 50 USDC. This is because if a user
sells 100 DAI and 50 DAI are for network fees and the exchange rate is
100->100, i.e., 1 -> 1, the expected buy amount is 50 DAI.
> The logic currently used by the driver: the reference buy amount for
computing price improvement is 66 USDC. This is because if the user
payed additional 50DAI on top of the 100DAI they want to sell, they
could get 100USDC. Scaling down this trade from 150DAI -> 100USDC gives
100DAI -> 66USDC.

Note that this is affecting only sell orders.

## How to test
Fixed existing tests to be aligned with new logic.
  • Loading branch information
sunce86 authored Mar 29, 2024
1 parent 51cea79 commit e005371
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 47 deletions.
2 changes: 1 addition & 1 deletion crates/autopilot/src/domain/fee/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_, &quote_);
let outside_market_price = boundary::is_order_outside_market_price(&order_, &quote_, order.data.kind);
match (outside_market_price, &fee_policy.order_class) {
(_, OrderClass::Any) => Some(&fee_policy.policy),
(true, OrderClass::Limit) => Some(&fee_policy.policy),
Expand Down
4 changes: 2 additions & 2 deletions crates/database/src/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 *",
Expand Down
49 changes: 34 additions & 15 deletions crates/driver/src/domain/competition/solution/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PriceLimits, Math> {
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 {
Expand All @@ -245,6 +250,10 @@ pub fn adjust_quote_to_order_limits(order: Order, quote: Quote) -> Result<PriceL
})
}
Side::Buy => {
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)?
Expand Down Expand Up @@ -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]
Expand All @@ -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();
Expand All @@ -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."
);
}

Expand All @@ -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,
Expand Down
18 changes: 10 additions & 8 deletions crates/driver/src/tests/cases/protocol_fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(),
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
Expand Down
28 changes: 14 additions & 14 deletions crates/e2e/tests/e2e/protocol_fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion crates/orderbook/src/database/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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()
Expand Down
Loading

0 comments on commit e005371

Please sign in to comment.