From 01b9761786774feee9c95e8c1e02e4cfbcecec51 Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Wed, 7 Aug 2024 08:14:52 +0200 Subject: [PATCH] Use external prices for conversion of fees in autopilot (#2863) # Description Related to discussion in the issue: https://github.com/cowprotocol/services/issues/2862#issuecomment-2270980569 Use external price vector to convert "fee in surplus token" into "fee in sell token" (instead of uniform clearing prices). This is important so that we make sure that it's always true: `total_fee = executed_surplus_fee + protocol fees` If we use uniform clearing prices for `executed_surplus_fee` and external prices for `protocol fees` I think above equation won't stand. ## How to test Existing unit test --- crates/autopilot/src/domain/settlement/mod.rs | 2 +- .../src/domain/settlement/solution/mod.rs | 15 ++++- .../src/domain/settlement/solution/trade.rs | 56 ++++++++++++------- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/crates/autopilot/src/domain/settlement/mod.rs b/crates/autopilot/src/domain/settlement/mod.rs index 0fa37c3e27..97be55903a 100644 --- a/crates/autopilot/src/domain/settlement/mod.rs +++ b/crates/autopilot/src/domain/settlement/mod.rs @@ -99,7 +99,7 @@ impl Settlement { gas_price: self.transaction.effective_gas_price, surplus: self.solution.native_surplus(&self.auction), fee: self.solution.native_fee(&self.auction.prices), - order_fees: self.solution.fees(), + order_fees: self.solution.fees(&self.auction.prices), } } } diff --git a/crates/autopilot/src/domain/settlement/solution/mod.rs b/crates/autopilot/src/domain/settlement/solution/mod.rs index 5d46073f23..cb9295a8e4 100644 --- a/crates/autopilot/src/domain/settlement/solution/mod.rs +++ b/crates/autopilot/src/domain/settlement/solution/mod.rs @@ -86,10 +86,13 @@ impl Solution { } /// Returns fees denominated in sell token for each order in the solution. - pub fn fees(&self) -> HashMap> { + pub fn fees( + &self, + prices: &auction::Prices, + ) -> HashMap> { self.trades .iter() - .map(|trade| (*trade.order_uid(), trade.fee().ok())) + .map(|trade| (*trade.order_uid(), trade.fee_in_sell_token(prices).ok())) .collect() } @@ -307,9 +310,15 @@ mod tests { eth::U256::from(52937525819789126u128) ); // fee read from "executedSurplusFee" https://api.cow.fi/mainnet/api/v1/orders/0x10dab31217bb6cc2ace0fe601c15d342f7626a1ee5ef0495449800e73156998740a50cf069e992aa4536211b23f286ef88752187ffffffff + // "executedSurplusFee" and native fee are equal because the sell token is ETH assert_eq!( solution.native_fee(&auction.prices).0, - eth::U256::from(6890975030480504u128) + eth::U256::from(6752697350740628u128) + ); + // fee read from "executedSurplusFee" https://api.cow.fi/mainnet/api/v1/orders/0x10dab31217bb6cc2ace0fe601c15d342f7626a1ee5ef0495449800e73156998740a50cf069e992aa4536211b23f286ef88752187ffffffff + assert_eq!( + solution.fees(&auction.prices), + HashMap::from([(domain::OrderUid(hex!("10dab31217bb6cc2ace0fe601c15d342f7626a1ee5ef0495449800e73156998740a50cf069e992aa4536211b23f286ef88752187ffffffff")), Some(eth::SellTokenAmount(eth::U256::from(6752697350740628u128))))]) ); } } diff --git a/crates/autopilot/src/domain/settlement/solution/trade.rs b/crates/autopilot/src/domain/settlement/solution/trade.rs index 0261c406a2..b86a8bb9ad 100644 --- a/crates/autopilot/src/domain/settlement/solution/trade.rs +++ b/crates/autopilot/src/domain/settlement/solution/trade.rs @@ -136,36 +136,54 @@ impl Trade { pub fn native_fee(&self, prices: &auction::Prices) -> Result { let fee = self.fee()?; let price = prices - .get(&self.sell.token) - .ok_or(Error::MissingPrice(self.sell.token))?; - Ok(price.in_eth(fee.into())) + .get(&fee.token) + .ok_or(Error::MissingPrice(fee.token))?; + Ok(price.in_eth(fee.amount)) } /// Total fee (protocol fee + network fee). Equal to a surplus difference /// before and after applying the fees. /// /// Denominated in SELL token - pub fn fee(&self) -> Result { + pub fn fee_in_sell_token( + &self, + prices: &auction::Prices, + ) -> Result { + let fee = self.fee()?; + let fee_in_sell_token = match self.side { + order::Side::Buy => fee.amount, + order::Side::Sell => { + let buy_price = prices + .get(&self.buy.token) + .ok_or(Error::MissingPrice(self.buy.token))?; + let sell_price = prices + .get(&self.sell.token) + .ok_or(Error::MissingPrice(self.sell.token))?; + fee.amount + .checked_mul(&buy_price.get().0.into()) + .ok_or(error::Math::Overflow)? + .checked_div(&sell_price.get().0.into()) + .ok_or(error::Math::DivisionByZero)? + } + } + .into(); + Ok(fee_in_sell_token) + } + + /// Total fee (protocol fee + network fee). Equal to a surplus difference + /// before and after applying the fees. + /// + /// Denominated in SURPLUS token + fn fee(&self) -> Result { let fee = self .surplus_over_limit_price_before_fee()? .amount .checked_sub(&self.surplus_over_limit_price()?.amount) .ok_or(error::Math::Negative)?; - // We don't have to convert the fee to the sell token, we can just use the fee - // in surplus token. This is done just because it was done like this in - // the previous implementation - // - // https://github.com/cowprotocol/services/blob/main/crates/autopilot/src/decoded_settlement.rs#L345 - let fee_in_sell_token = match self.side { - order::Side::Buy => fee, - order::Side::Sell => fee - .checked_mul(&self.prices.uniform.buy.into()) - .ok_or(error::Math::Overflow)? - .checked_div(&self.prices.uniform.sell.into()) - .ok_or(error::Math::DivisionByZero)?, - } - .into(); - Ok(fee_in_sell_token) + Ok(eth::Asset { + token: self.surplus_token(), + amount: fee, + }) } /// Protocol fees is defined by fee policies attached to the order.