From 63b617075967922af42e08528c484257e9877d4c Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Wed, 7 Aug 2024 13:53:50 +0200 Subject: [PATCH] Fix rounding of surplus/protocol_fee/score (#2857) # Description Related to https://github.com/cowprotocol/services/issues/2797#issuecomment-2264843339 Fixes issue in `driver` and new `autopilot` code. Old `autopilot` code is not fixed and I don't intend to do it. This means that settlement observations will still be wrong until new code is used. Dependent on https://github.com/cowprotocol/services/pull/2843 Note that this change affects the whole range of functionalities: surplus, protocol fee, score calculation. # Changes - [ ] Whenever we calculate `buy_amount` for `sell` orders, use `ceil_div` to be consistent with settlement contract logic ## How to test Added unit test that shows that new code calculates the expected circuit breaker surplus of `384509480572312` Expected score is 2 x surplus. --- .../src/domain/settlement/solution/mod.rs | 127 ++++++++++++++++++ .../src/domain/settlement/solution/trade.rs | 11 +- .../domain/competition/solution/scoring.rs | 11 +- .../src/domain/competition/solution/trade.rs | 13 +- 4 files changed, 156 insertions(+), 6 deletions(-) diff --git a/crates/autopilot/src/domain/settlement/solution/mod.rs b/crates/autopilot/src/domain/settlement/solution/mod.rs index cb9295a8e4..2347f428d5 100644 --- a/crates/autopilot/src/domain/settlement/solution/mod.rs +++ b/crates/autopilot/src/domain/settlement/solution/mod.rs @@ -321,4 +321,131 @@ mod tests { HashMap::from([(domain::OrderUid(hex!("10dab31217bb6cc2ace0fe601c15d342f7626a1ee5ef0495449800e73156998740a50cf069e992aa4536211b23f286ef88752187ffffffff")), Some(eth::SellTokenAmount(eth::U256::from(6752697350740628u128))))]) ); } + + // https://etherscan.io/tx/0x688508eb59bd20dc8c0d7c0c0b01200865822c889f0fcef10113e28202783243 + #[test] + fn settlement_with_protocol_fee() { + let calldata = hex!( + " + 13d79a0b + 0000000000000000000000000000000000000000000000000000000000000080 + 0000000000000000000000000000000000000000000000000000000000000120 + 00000000000000000000000000000000000000000000000000000000000001c0 + 00000000000000000000000000000000000000000000000000000000000003e0 + 0000000000000000000000000000000000000000000000000000000000000004 + 000000000000000000000000056fd409e1d7a124bd7017459dfea2f387b6d5cd + 000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec7 + 000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec7 + 000000000000000000000000056fd409e1d7a124bd7017459dfea2f387b6d5cd + 0000000000000000000000000000000000000000000000000000000000000004 + 00000000000000000000000000000000000000000000000000000019b743b945 + 0000000000000000000000000000000000000000000000000000000000a87cf3 + 0000000000000000000000000000000000000000000000000000000000a87c7c + 00000000000000000000000000000000000000000000000000000019b8b69873 + 0000000000000000000000000000000000000000000000000000000000000001 + 0000000000000000000000000000000000000000000000000000000000000020 + 0000000000000000000000000000000000000000000000000000000000000002 + 0000000000000000000000000000000000000000000000000000000000000003 + 000000000000000000000000f87da2093abee9b13a6f89671e4c3a3f80b42767 + 0000000000000000000000000000000000000000000000000000006d6e2edc00 + 0000000000000000000000000000000000000000000000000000000002cccdff + 000000000000000000000000000000000000000000000000000000006799c219 + 2d365e5affcfa62cf1067b845add9c01bedcb2fc5d7a37442d2177262af26a0c + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000002 + 00000000000000000000000000000000000000000000000000000019b8b69873 + 0000000000000000000000000000000000000000000000000000000000000160 + 0000000000000000000000000000000000000000000000000000000000000041 + e2ef661343676f9f4371ce809f728bb39a406f47835ee2b0104a8a1f340409ae + 742dfe47fe469c024dc2fb7f80b99878b35985d66312856a8b5dcf5de4b069ee + 1c00000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 0000000000000000000000000000000000000000000000000000000000000080 + 0000000000000000000000000000000000000000000000000000000000000520 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000003 + 0000000000000000000000000000000000000000000000000000000000000060 + 0000000000000000000000000000000000000000000000000000000000000140 + 00000000000000000000000000000000000000000000000000000000000002e0 + 000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 0000000000000000000000000000000000000000000000000000000000000044 + 095ea7b3000000000000000000000000e592427a0aece92de3edee1f18e0157c + 05861564ffffffffffffffffffffffffffffffffffffffffffffffffffffffff + ffffffff00000000000000000000000000000000000000000000000000000000 + 000000000000000000000000e592427a0aece92de3edee1f18e0157c05861564 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 0000000000000000000000000000000000000000000000000000000000000104 + db3e2198000000000000000000000000dac17f958d2ee523a2206206994597c1 + 3d831ec7000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce + 3606eb4800000000000000000000000000000000000000000000000000000000 + 000001f40000000000000000000000009008d19f58aabd9ed0d60971565aa851 + 0560ab4100000000000000000000000000000000000000000000000000000000 + 66abb94e00000000000000000000000000000000000000000000000000000019 + b4b64b9b00000000000000000000000000000000000000000000000000000019 + bdd90a1800000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 000000000000000000000000e592427a0aece92de3edee1f18e0157c05861564 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 0000000000000000000000000000000000000000000000000000000000000104 + db3e2198000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce + 3606eb48000000000000000000000000056fd409e1d7a124bd7017459dfea2f3 + 87b6d5cd00000000000000000000000000000000000000000000000000000000 + 000001f40000000000000000000000009008d19f58aabd9ed0d60971565aa851 + 0560ab4100000000000000000000000000000000000000000000000000000000 + 66abb94e00000000000000000000000000000000000000000000000000000000 + 00a87cf300000000000000000000000000000000000000000000000000000019 + bb4af52700000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 00000000008c912c" + ) + .to_vec(); + + let domain_separator = eth::DomainSeparator(hex!( + "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" + )); + let solution = super::Solution::new(&calldata.into(), &domain_separator).unwrap(); + assert_eq!(solution.trades.len(), 1); + + let prices: auction::Prices = From::from([ + ( + eth::TokenAddress(eth::H160::from_slice(&hex!( + "dac17f958d2ee523a2206206994597c13d831ec7" + ))), + auction::Price::new(eth::U256::from(321341140475275961528483840u128).into()) + .unwrap(), + ), + ( + eth::TokenAddress(eth::H160::from_slice(&hex!( + "056fd409e1d7a124bd7017459dfea2f387b6d5cd" + ))), + auction::Price::new(eth::U256::from(3177764302250520038326415654912u128).into()) + .unwrap(), + ), + ]); + + let auction = super::super::Auction { + prices, + surplus_capturing_jit_order_owners: vec![], + id: 0, + orders: HashMap::from([(domain::OrderUid(hex!("c6a81144bc822569a0752c7a537fa9cbbf6344cb187ce0ff15a534b571e277eaf87da2093abee9b13a6f89671e4c3a3f80b427676799c219")), vec![domain::fee::Policy::Surplus { + factor: 0.5f64.try_into().unwrap(), + max_volume_factor: 0.01.try_into().unwrap(), + }])]), + }; + + assert_eq!( + solution.native_surplus(&auction).0, + eth::U256::from(384509480572312u128) + ); + + assert_eq!( + solution.score(&auction).unwrap().get().0, + eth::U256::from(769018961144624u128) // 2 x surplus + ); + } } diff --git a/crates/autopilot/src/domain/settlement/solution/trade.rs b/crates/autopilot/src/domain/settlement/solution/trade.rs index b86a8bb9ad..f119b803ac 100644 --- a/crates/autopilot/src/domain/settlement/solution/trade.rs +++ b/crates/autopilot/src/domain/settlement/solution/trade.rs @@ -88,12 +88,17 @@ impl Trade { } order::Side::Sell => { // scale limit buy to support partially fillable orders + + // `checked_ceil_div`` to be consistent with how settlement contract calculates + // traded buy amounts + // smallest allowed executed_buy_amount per settlement contract is + // executed_sell_amount * ceil(price_limits.buy / price_limits.sell) let limit_buy = self .executed .0 .checked_mul(price_limits.buy.0) .ok_or(error::Math::Overflow)? - .checked_div(price_limits.sell.0) + .checked_ceil_div(&price_limits.sell.0) .ok_or(error::Math::DivisionByZero)?; let bought = self .executed @@ -228,6 +233,8 @@ impl Trade { /// The effective amount the user received after all fees. /// /// Note how the `executed` amount is used to build actual traded amounts. + /// + /// Settlement contract uses `ceil` division for buy amount calculation. fn buy_amount(&self) -> Result { Ok(match self.side { order::Side::Sell => self @@ -235,7 +242,7 @@ impl Trade { .0 .checked_mul(self.prices.custom.sell) .ok_or(error::Math::Overflow)? - .checked_div(self.prices.custom.buy) + .checked_ceil_div(&self.prices.custom.buy) .ok_or(error::Math::DivisionByZero)?, order::Side::Buy => self.executed.0, } diff --git a/crates/driver/src/domain/competition/solution/scoring.rs b/crates/driver/src/domain/competition/solution/scoring.rs index 4cd4eb5952..fe52e011cd 100644 --- a/crates/driver/src/domain/competition/solution/scoring.rs +++ b/crates/driver/src/domain/competition/solution/scoring.rs @@ -128,12 +128,17 @@ impl Trade { } Side::Sell => { // scale limit buy to support partially fillable orders + + // `checked_ceil_div`` to be consistent with how settlement contract calculates + // traded buy amounts + // smallest allowed executed_buy_amount per settlement contract is + // executed_sell_amount * ceil(price_limits.buy / price_limits.sell) let limit_buy = self .executed .0 .checked_mul(price_limits.buy.0) .ok_or(Math::Overflow)? - .checked_div(price_limits.sell.0) + .checked_ceil_div(&price_limits.sell.0) .ok_or(Math::DivisionByZero)?; let bought = self .executed @@ -206,6 +211,8 @@ impl Trade { /// The effective amount the user received after all fees. /// /// Note how the `executed` amount is used to build actual traded amounts. + /// + /// Settlement contract uses `ceil` division for buy amount calculation. fn buy_amount(&self) -> Result { Ok(match self.side { order::Side::Sell => self @@ -213,7 +220,7 @@ impl Trade { .0 .checked_mul(self.custom_price.sell) .ok_or(Math::Overflow)? - .checked_div(self.custom_price.buy) + .checked_ceil_div(&self.custom_price.buy) .ok_or(Math::DivisionByZero)?, order::Side::Buy => self.executed.0, } diff --git a/crates/driver/src/domain/competition/solution/trade.rs b/crates/driver/src/domain/competition/solution/trade.rs index 0cc9a7e475..c0c5d950d8 100644 --- a/crates/driver/src/domain/competition/solution/trade.rs +++ b/crates/driver/src/domain/competition/solution/trade.rs @@ -79,6 +79,8 @@ impl Trade { } /// The effective amount the user received after all fees. + /// + /// Settlement contract uses `ceil` division for buy amount calculation. fn buy_amount(&self, prices: &ClearingPrices) -> Result { let amount = match self.side() { order::Side::Buy => self.executed().0, @@ -215,6 +217,8 @@ impl Fulfillment { } /// The effective amount the user received after all fees. + /// + /// Settlement contract uses `ceil` division for buy amount calculation. pub fn buy_amount(&self, prices: &ClearingPrices) -> Result { let amount = match self.order.side { order::Side::Buy => self.executed.0, @@ -287,16 +291,21 @@ impl Fulfillment { } Side::Sell => { // Scale to support partially fillable orders + + // `checked_ceil_div`` to be consistent with how settlement contract calculates + // traded buy amounts + // smallest allowed executed_buy_amount per settlement contract is + // executed_sell_amount * ceil(price_limits.buy / price_limits.sell) let limit_buy_amount = limit_buy .checked_mul(executed_sell_amount_with_fee) .ok_or(Math::Overflow)? - .checked_div(limit_sell) + .checked_ceil_div(&limit_sell) .ok_or(Math::DivisionByZero)?; // How much `buy_token` we get for `executed` amount of `sell_token` let executed_buy_amount = executed .checked_mul(prices.sell) .ok_or(Math::Overflow)? - .checked_div(prices.buy) + .checked_ceil_div(&prices.buy) .ok_or(Math::DivisionByZero)?; // Remaining surplus after fees // Do not return error if `checked_sub` fails because violated limit prices will