Remove user fee (#2586)

# Description Fixes #2492 Related gnosis-solvers PR: gnosis/solvers#9 Will be merged next week - apparently we don't have enough solvers ready for this breaking change yet! # Changes Removed field "user_fee" from auction endpoint - breaking API change. Removed field "fee_amount" from solvers api - also breaking API change ## How to test Existing (updated) tests.
cowprotocol · Apr 9, 2024 · a940bc1 · a940bc1
1 parent 260ba3e
commit a940bc1
Show file tree

Hide file tree

Showing 44 changed files with 29 additions and 185 deletions.
diff --git a/crates/autopilot/src/boundary/order.rs b/crates/autopilot/src/boundary/order.rs
@@ -13,7 +13,6 @@ pub fn to_domain(
         buy_token: order.data.buy_token,
         sell_amount: order.data.sell_amount,
         buy_amount: order.data.buy_amount,
-        user_fee: order.data.fee_amount,
         protocol_fees,
         valid_to: order.data.valid_to,
         side: order.data.kind.into(),

diff --git a/crates/autopilot/src/domain/auction/order.rs b/crates/autopilot/src/domain/auction/order.rs
@@ -11,7 +11,6 @@ pub struct Order {
     pub buy_token: H160,
     pub sell_amount: U256,
     pub buy_amount: U256,
-    pub user_fee: U256,
     pub protocol_fees: Vec<fee::Policy>,
     pub side: Side,
     pub class: Class,

diff --git a/crates/autopilot/src/infra/persistence/dto/order.rs b/crates/autopilot/src/infra/persistence/dto/order.rs
@@ -21,8 +21,6 @@ pub struct Order {
     pub sell_amount: U256,
     #[serde_as(as = "HexOrDecimalU256")]
     pub buy_amount: U256,
-    #[serde_as(as = "HexOrDecimalU256")]
-    pub user_fee: U256,
     pub protocol_fees: Vec<FeePolicy>,
     pub valid_to: u32,
     pub kind: boundary::OrderKind,
@@ -49,7 +47,6 @@ pub fn from_domain(order: domain::Order) -> Order {
         buy_token: order.buy_token,
         sell_amount: order.sell_amount,
         buy_amount: order.buy_amount,
-        user_fee: order.user_fee,
         protocol_fees: order.protocol_fees.into_iter().map(Into::into).collect(),
         valid_to: order.valid_to,
         kind: order.side.into(),
@@ -78,7 +75,6 @@ pub fn to_domain(order: Order) -> domain::Order {
         buy_token: order.buy_token,
         sell_amount: order.sell_amount,
         buy_amount: order.buy_amount,
-        user_fee: order.user_fee,
         protocol_fees: order.protocol_fees.into_iter().map(Into::into).collect(),
         valid_to: order.valid_to,
         side: order.kind.into(),

diff --git a/crates/driver/openapi.yml b/crates/driver/openapi.yml
@@ -197,12 +197,6 @@ components:
         buyAmount:
           description: Amount to be bought
           $ref: "#/components/schemas/TokenAmount"
-        userFee:
-          description: |
-            The fee that the user signed to pay for getting their order included.
-
-            This fee may be different from solver fee as it includes subsidies.
-          $ref: "#/components/schemas/TokenAmount"
         validTo:
           description: The time until which the order is valid.
           type: integer

diff --git a/crates/driver/src/boundary/settlement.rs b/crates/driver/src/boundary/settlement.rs
@@ -208,7 +208,10 @@ fn to_boundary_order(order: &competition::Order) -> Order {
             buy_token: order.buy.token.into(),
             sell_amount: order.sell.amount.into(),
             buy_amount: order.buy.amount.into(),
-            fee_amount: order.user_fee.into(),
+            // The fee amount is guaranteed to be 0 and it no longer exists in the domain, but for
+            // the proper encoding of the order where the `model::OrderData` struct is used, we must
+            // set it to 0.
+            fee_amount: 0.into(),
             receiver: order.receiver.map(Into::into),
             valid_to: order.valid_to.into(),
             app_data: AppDataHash(order.app_data.into()),
@@ -229,7 +232,7 @@ fn to_boundary_order(order: &competition::Order) -> Order {
         },
         metadata: OrderMetadata {
             full_fee_amount: Default::default(),
-            solver_fee: order.user_fee.into(),
+            solver_fee: 0.into(),
             class: match order.kind {
                 competition::order::Kind::Market => OrderClass::Market,
                 competition::order::Kind::Liquidity => OrderClass::Liquidity,

diff --git a/crates/driver/src/domain/competition/auction.rs b/crates/driver/src/domain/competition/auction.rs
@@ -238,20 +238,7 @@ impl AuctionProcessor {
                 }
             };
 
-            let available_sell_amount = {
-                let available = order.available(weth);
-                available.sell.amount.0.checked_add(available.user_fee.0)
-            };
-            let max_sell = match available_sell_amount {
-                Some(amount) => order::SellAmount(amount),
-                None => {
-                    observe::order_excluded_from_auction(
-                        order,
-                        observe::OrderExcludedFromAuctionReason::CouldNotCalculateMaxSell,
-                    );
-                    return false;
-                }
-            };
+            let max_sell = order::SellAmount(order.available(weth).sell.amount.0);
 
             let allocated_balance = match order.partial {
                 order::Partial::Yes { .. } => max_sell.min(*remaining_balance),

diff --git a/crates/driver/src/domain/competition/order/mod.rs b/crates/driver/src/domain/competition/order/mod.rs
@@ -25,7 +25,6 @@ pub struct Order {
     /// The maximum amount this order is allowed to sell when completely filled.
     pub sell: eth::Asset,
     pub side: Side,
-    pub user_fee: SellAmount,
     pub kind: Kind,
     pub app_data: AppData,
     pub partial: Partial,
@@ -118,8 +117,6 @@ pub struct Available {
     /// The available minimum buy amount for an order that gets passed to a
     /// solver engine.
     pub buy: eth::Asset,
-    /// The available fee amount.
-    pub user_fee: SellAmount,
 }
 
 impl Order {
@@ -172,7 +169,6 @@ impl Order {
                 token: self.buy.token.wrap(weth),
                 amount: self.buy.amount,
             },
-            user_fee: self.user_fee,
         };
 
         let available = match self.partial {
@@ -181,9 +177,9 @@ impl Order {
         };
         let target = self.target();
 
-        for amount in [&mut amounts.sell.amount.0, &mut amounts.user_fee.0] {
-            *amount = util::math::mul_ratio(*amount, available.0, target.0).unwrap_or_default();
-        }
+        amounts.sell.amount = util::math::mul_ratio(amounts.sell.amount.0, available.0, target.0)
+            .unwrap_or_default()
+            .into();
 
         amounts.buy.amount =
             util::math::mul_ratio_ceil(amounts.buy.amount.0, available.0, target.0)
@@ -427,7 +423,6 @@ mod tests {
                 Some(executed) if executed.token == buy(0).token => Side::Buy,
                 _ => panic!(),
             },
-            user_fee: Default::default(),
             kind: Kind::Limit,
             app_data: Default::default(),
             partial: available

diff --git a/crates/driver/src/domain/competition/solution/trade.rs b/crates/driver/src/domain/competition/solution/trade.rs
@@ -88,7 +88,12 @@ impl Fulfillment {
     /// considering their signed order and the uniform clearing prices
     pub fn fee(&self) -> order::SellAmount {
         match self.fee {
-            Fee::Static => self.order.user_fee,
+            Fee::Static => {
+                // Orders with static fees are no longer used, except for quoting purposes, when
+                // the static fee is set to 0. This is expected to be resolved with https://github.com/cowprotocol/services/issues/2543
+                // Once resolved, this code will be simplified as part of https://github.com/cowprotocol/services/issues/2507
+                order::SellAmount(0.into())
+            }
             Fee::Dynamic(fee) => fee,
         }
     }

diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs
@@ -120,7 +120,6 @@ impl Order {
                 buy: self.buy(),
                 sell: self.sell(),
                 side: self.side,
-                user_fee: Default::default(),
                 kind: competition::order::Kind::Market,
                 app_data: Default::default(),
                 partial: competition::order::Partial::No,

diff --git a/crates/driver/src/infra/api/routes/solve/dto/auction.rs b/crates/driver/src/infra/api/routes/solve/dto/auction.rs
@@ -46,7 +46,6 @@ impl Auction {
                         Kind::Sell => competition::order::Side::Sell,
                         Kind::Buy => competition::order::Side::Buy,
                     },
-                    user_fee: order.user_fee.into(),
                     kind: match order.class {
                         Class::Market => competition::order::Kind::Market,
                         Class::Limit => competition::order::Kind::Limit,
@@ -227,8 +226,6 @@ struct Order {
     sell_amount: eth::U256,
     #[serde_as(as = "serialize::U256")]
     buy_amount: eth::U256,
-    #[serde_as(as = "serialize::U256")]
-    user_fee: eth::U256,
     protocol_fees: Vec<FeePolicy>,
     valid_to: u32,
     kind: Kind,

diff --git a/crates/driver/src/infra/observe/mod.rs b/crates/driver/src/infra/observe/mod.rs
@@ -371,7 +371,6 @@ pub fn deadline(deadline: &Deadline, timeouts: &Timeouts) {
 #[derive(Debug)]
 pub enum OrderExcludedFromAuctionReason {
     CouldNotFetchBalance,
-    CouldNotCalculateMaxSell,
     InsufficientBalance,
     OrderWithZeroAmountRemaining,
 }

diff --git a/crates/driver/src/infra/solver/dto/auction.rs b/crates/driver/src/infra/solver/dto/auction.rs
@@ -113,7 +113,6 @@ impl Auction {
                         buy_token: available.buy.token.into(),
                         sell_amount: available.sell.amount.into(),
                         buy_amount: available.buy.amount.into(),
-                        fee_amount: available.user_fee.into(),
                         kind: match order.side {
                             competition::order::Side::Buy => Kind::Buy,
                             competition::order::Side::Sell => Kind::Sell,
@@ -301,8 +300,6 @@ struct Order {
     sell_amount: eth::U256,
     #[serde_as(as = "serialize::U256")]
     buy_amount: eth::U256,
-    #[serde_as(as = "serialize::U256")]
-    fee_amount: eth::U256,
     kind: Kind,
     partially_fillable: bool,
     class: Class,

diff --git a/crates/driver/src/tests/cases/fees.rs b/crates/driver/src/tests/cases/fees.rs
@@ -7,25 +7,6 @@ use crate::{
     },
 };
 
-#[tokio::test]
-#[ignore]
-async fn rejects_unwarranted_solver_fee() {
-    let test = tests::setup()
-        .name("Solver fee on market order".to_string())
-        .pool(ab_pool())
-        .order(
-            // A solver reporting a fee on a swap order
-            ab_order()
-                .user_fee(1000.into())
-                .solver_fee(Some(500.into())),
-        )
-        .solution(ab_solution())
-        .done()
-        .await;
-
-    test.solve().await.status(hyper::StatusCode::BAD_REQUEST);
-}
-
 #[tokio::test]
 #[ignore]
 async fn solver_fee() {
@@ -46,20 +27,3 @@ async fn solver_fee() {
         test.solve().await.ok().orders(&[order]);
     }
 }
-
-#[tokio::test]
-#[ignore]
-async fn user_fee() {
-    for side in [order::Side::Buy, order::Side::Sell] {
-        let order = ab_order().side(side).user_fee(1000.into());
-        let test = tests::setup()
-            .name(format!("User Fee: {side:?}"))
-            .pool(ab_pool())
-            .order(order.clone())
-            .solution(ab_solution())
-            .done()
-            .await;
-
-        test.solve().await.ok().orders(&[order]);
-    }
-}
diff --git a/crates/driver/src/tests/setup/blockchain.rs b/crates/driver/src/tests/setup/blockchain.rs
@@ -147,7 +147,7 @@ impl QuotedOrder {
             sell_amount: self.sell_amount(),
             buy_amount: self.buy_amount(),
             valid_to: u32::try_from(time::now().timestamp()).unwrap() + self.order.valid_for.0,
-            user_fee: self.order.user_fee,
+            user_fee: 0.into(),
             side: self.order.side,
             secret_key: blockchain.trader_secret_key,
             domain_separator: blockchain.domain_separator,
@@ -631,7 +631,7 @@ impl Blockchain {
                         .unwrap()
                         .mint(
                             self.trader_address,
-                            "1e-7".ether().into_wei() * execution.sell + order.user_fee,
+                            "1e-7".ether().into_wei() * execution.sell,
                         )
                         .from(trader_account.clone())
                         .send(),

diff --git a/crates/driver/src/tests/setup/driver.rs b/crates/driver/src/tests/setup/driver.rs
@@ -68,7 +68,6 @@ pub fn solve_req(test: &Test) -> serde_json::Value {
             "buyToken": hex_address(test.blockchain.get_token(quote.order.buy_token)),
             "sellAmount": quote.sell_amount().to_string(),
             "buyAmount": quote.buy_amount().to_string(),
-            "userFee": quote.order.user_fee.to_string(),
             "protocolFees": match quote.order.kind {
                 order::Kind::Market => json!([]),
                 order::Kind::Liquidity => json!([]),

diff --git a/crates/driver/src/tests/setup/mod.rs b/crates/driver/src/tests/setup/mod.rs
@@ -100,7 +100,6 @@ pub struct Order {
     pub valid_for: util::Timestamp,
     pub kind: order::Kind,
 
-    pub user_fee: eth::U256,
     // Currently used for limit orders to represent the surplus_fee calculated by the solver.
     pub solver_fee: Option<eth::U256>,
 
@@ -145,13 +144,6 @@ impl Order {
         }
     }
 
-    pub fn user_fee(self, amount: eth::U256) -> Self {
-        Self {
-            user_fee: amount,
-            ..self
-        }
-    }
-
     /// Ensure that this order generates no surplus, and therefore most likely
     /// has a negative score.
     pub fn no_surplus(self) -> Self {
@@ -268,7 +260,6 @@ impl Default for Order {
             partial: Default::default(),
             valid_for: 100.into(),
             kind: order::Kind::Market,
-            user_fee: Default::default(),
             solver_fee: Default::default(),
             name: Default::default(),
             surplus_factor: DEFAULT_SURPLUS_FACTOR.ether().into_wei(),
@@ -1014,10 +1005,6 @@ impl<'a> Solve<'a> {
             blockchain: self.blockchain,
         }
     }
-
-    pub fn status(self, code: hyper::StatusCode) {
-        assert_eq!(self.status, code);
-    }
 }
 
 impl<'a> SolveOk<'a> {
@@ -1088,10 +1075,7 @@ impl<'a> SolveOk<'a> {
 
             let (expected_sell, expected_buy) = match &expected.expected_amounts {
                 Some(executed_amounts) => (executed_amounts.sell, executed_amounts.buy),
-                None => (
-                    fulfillment.quoted_order.sell + fulfillment.quoted_order.order.user_fee,
-                    fulfillment.quoted_order.buy,
-                ),
+                None => (fulfillment.quoted_order.sell, fulfillment.quoted_order.buy),
             };
             assert!(u256(trade.get("sellAmount").unwrap()) == expected_sell);
             assert!(u256(trade.get("buyAmount").unwrap()) == expected_buy);

diff --git a/crates/driver/src/tests/setup/solver.rs b/crates/driver/src/tests/setup/solver.rs
@@ -103,7 +103,6 @@ impl Solver {
                 "buyToken": hex_address(config.blockchain.get_token(buy_token)),
                 "sellAmount": sell_amount,
                 "buyAmount": buy_amount,
-                "feeAmount": quote.order.user_fee.to_string(),
                 "kind": match quote.order.side {
                     order::Side::Sell => "sell",
                     order::Side::Buy => "buy",

diff --git a/crates/e2e/tests/e2e/partially_fillable_balance.rs b/crates/e2e/tests/e2e/partially_fillable_balance.rs
@@ -97,7 +97,6 @@ async fn test(web3: Web3) {
     let order = auction.orders.into_iter().next().unwrap();
     assert!(order.partially_fillable);
     assert!(matches!(order.class, OrderClass::Limit));
-    assert_eq!(order.user_fee, 0.into());
 
     tracing::info!("Waiting for trade.");
     let trade_happened =

diff --git a/crates/e2e/tests/e2e/partially_fillable_pool.rs b/crates/e2e/tests/e2e/partially_fillable_pool.rs
@@ -97,7 +97,6 @@ async fn test(web3: Web3) {
     let order = auction.orders.into_iter().next().unwrap();
     assert!(order.partially_fillable);
     assert!(matches!(order.class, OrderClass::Limit));
-    assert_eq!(order.user_fee, 0.into());
 
     tracing::info!("Waiting for trade.");
     let trade_happened =

diff --git a/crates/orderbook/openapi.yml b/crates/orderbook/openapi.yml
@@ -914,10 +914,6 @@ components:
           description: see `OrderParameters::buyAmount`
           allOf:
             - $ref: "#/components/schemas/TokenAmount"
-        userFee:
-          description: see `OrderParameters::feeAmount`
-          allOf:
-            - $ref: "#/components/schemas/TokenAmount"
         validTo:
           description: see `OrderParameters::validTo`
           type: integer
@@ -980,7 +976,6 @@ components:
         - buyToken
         - sellAmount
         - buyAmount
-        - userFee
         - validTo
         - kind
         - receiver

diff --git a/crates/orderbook/src/dto/order.rs b/crates/orderbook/src/dto/order.rs
@@ -22,8 +22,6 @@ pub struct Order {
     pub sell_amount: U256,
     #[serde_as(as = "HexOrDecimalU256")]
     pub buy_amount: U256,
-    #[serde_as(as = "HexOrDecimalU256")]
-    pub user_fee: U256,
     pub protocol_fees: Vec<FeePolicy>,
     pub valid_to: u32,
     pub kind: OrderKind,

diff --git a/crates/solvers-dto/src/auction.rs b/crates/solvers-dto/src/auction.rs
@@ -34,8 +34,6 @@ pub struct Order {
     pub sell_amount: U256,
     #[serde_as(as = "HexOrDecimalU256")]
     pub buy_amount: U256,
-    #[serde_as(as = "HexOrDecimalU256")]
-    pub fee_amount: U256,
     pub kind: Kind,
     pub partially_fillable: bool,
     pub class: Class,