Skip to content

Commit

Permalink
Remove solver_fee (#2272)
Browse files Browse the repository at this point in the history
# Description
Removes `solver_fee`.

I want to drop the `solver_fee` completely. It's hard to understand
whether it is a fee set by solver or it is a fee sent to solvers to do
something with it.

Also, we are currently not using subsidies (which are btw used only for
market orders that we want to deprecate) so `solver_fee` is equal to
`user_fee`.

@fleupold already touched this renaming before. After this change, only
`model::Order` will contain `solver_fee` but changing that is a major
breaking api change.

I expect shadow solver to fail again because of this. Alerter is not
deserializing this field so it should be ok.
  • Loading branch information
sunce86 authored Jan 18, 2024
1 parent d122b53 commit abac2c1
Show file tree
Hide file tree
Showing 29 changed files with 88 additions and 161 deletions.
1 change: 0 additions & 1 deletion crates/autopilot/src/boundary/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
solver_fee: order.metadata.full_fee_amount,
user_fee: order.data.fee_amount,
protocol_fees,
valid_to: order.data.valid_to,
Expand Down
1 change: 0 additions & 1 deletion crates/autopilot/src/domain/auction/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ pub struct Order {
pub buy_token: H160,
pub sell_amount: U256,
pub buy_amount: U256,
pub solver_fee: U256,
pub user_fee: U256,
pub protocol_fees: Vec<fee::Policy>,
pub kind: Kind,
Expand Down
4 changes: 0 additions & 4 deletions crates/autopilot/src/infra/persistence/dto/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ pub struct Order {
#[serde_as(as = "HexOrDecimalU256")]
pub buy_amount: U256,
#[serde_as(as = "HexOrDecimalU256")]
pub solver_fee: U256,
#[serde_as(as = "HexOrDecimalU256")]
pub user_fee: U256,
pub protocol_fees: Vec<FeePolicy>,
pub valid_to: u32,
Expand Down Expand Up @@ -50,7 +48,6 @@ pub fn from_domain(order: domain::Order) -> Order {
buy_token: order.buy_token,
sell_amount: order.sell_amount,
buy_amount: order.buy_amount,
solver_fee: order.solver_fee,
user_fee: order.user_fee,
protocol_fees: order.protocol_fees.into_iter().map(Into::into).collect(),
valid_to: order.valid_to,
Expand Down Expand Up @@ -80,7 +77,6 @@ pub fn to_domain(order: Order) -> domain::Order {
buy_token: order.buy_token,
sell_amount: order.sell_amount,
buy_amount: order.buy_amount,
solver_fee: order.solver_fee,
user_fee: order.user_fee,
protocol_fees: order.protocol_fees.into_iter().map(Into::into).collect(),
valid_to: order.valid_to,
Expand Down
6 changes: 0 additions & 6 deletions crates/driver/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,6 @@ components:
buyAmount:
description: Amount to be bought
$ref: "#/components/schemas/TokenAmount"
solverFee:
description: |
The fee that the solver should use to compute the objective value of the solution.
This fee may be different from user fee as it removes any subsidies.
$ref: "#/components/schemas/TokenAmount"
userFee:
description: |
The fee that the user signed to pay for getting their order included.
Expand Down
12 changes: 6 additions & 6 deletions crates/driver/src/boundary/settlement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,15 @@ impl Settlement {
to_boundary_order(trade.order()),
LimitOrderExecution {
filled: trade.executed().into(),
scoring_fee: trade.scoring_fee().into(),
fee: trade.fee().into(),
},
)
}
competition::solution::Trade::Jit(trade) => (
to_boundary_jit_order(&DomainSeparator(domain.0), trade.order()),
LimitOrderExecution {
filled: trade.executed().into(),
scoring_fee: 0.into(),
fee: 0.into(),
},
),
};
Expand Down Expand Up @@ -234,8 +234,8 @@ impl Settlement {
)?;

let surplus = self.inner.total_surplus(&prices);
let solver_fees = self.inner.total_scoring_fees(&prices);
let quality = surplus + solver_fees;
let scoring_fees = self.inner.total_scoring_fees(&prices);
let quality = surplus + scoring_fees;

Ok(eth::U256::from_big_rational(&quality)?.into())
}
Expand Down Expand Up @@ -267,7 +267,7 @@ 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.fee.user.into(),
fee_amount: order.user_fee.into(),
receiver: order.receiver.map(Into::into),
valid_to: order.valid_to.into(),
app_data: AppDataHash(order.app_data.into()),
Expand All @@ -288,7 +288,7 @@ fn to_boundary_order(order: &competition::Order) -> Order {
},
metadata: OrderMetadata {
full_fee_amount: Default::default(),
solver_fee: order.fee.solver.into(),
solver_fee: order.user_fee.into(),
class: match order.kind {
competition::order::Kind::Market => OrderClass::Market,
competition::order::Kind::Liquidity => OrderClass::Liquidity,
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/domain/competition/auction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ impl AuctionProcessor {

let max_sell = match {
let available = order.available(weth);
available.sell.amount.0.checked_add(available.fee.user.0)
available.sell.amount.0.checked_add(available.user_fee.0)
} {
Some(amount) => order::SellAmount(amount),
None => {
Expand Down
24 changes: 5 additions & 19 deletions crates/driver/src/domain/competition/order/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct Order {
/// The maximum amount this order is allowed to sell when completely filled.
pub sell: eth::Asset,
pub side: Side,
pub fee: Fee,
pub user_fee: SellAmount,
pub kind: Kind,
pub app_data: AppData,
pub partial: Partial,
Expand Down Expand Up @@ -97,16 +97,6 @@ impl From<TargetAmount> for eth::TokenAmount {
}
}

/// Order fee denominated in the sell token.
#[derive(Debug, Default, Clone, Copy)]
pub struct Fee {
/// The order fee that is actually paid by the user.
pub user: SellAmount,
/// The fee used for scoring. This is a scaled version of the user fee to
/// incentivize solvers to solve orders in batches.
pub solver: SellAmount,
}

/// The available amounts for a specific order that gets passed to the solver.
///
/// These amounts differ from the order buy/sell/fee amounts in two ways:
Expand All @@ -129,7 +119,7 @@ pub struct Available {
/// solver engine.
pub buy: eth::Asset,
/// The available fee amount.
pub fee: Fee,
pub user_fee: SellAmount,
}

impl Order {
Expand Down Expand Up @@ -182,7 +172,7 @@ impl Order {
token: self.buy.token.wrap(weth),
amount: self.buy.amount,
},
fee: self.fee,
user_fee: self.user_fee,
};

let available = match self.partial {
Expand All @@ -191,11 +181,7 @@ impl Order {
};
let target = self.target();

for amount in [
&mut amounts.sell.amount.0,
&mut amounts.fee.user.0,
&mut amounts.fee.solver.0,
] {
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();
}

Expand Down Expand Up @@ -441,7 +427,7 @@ mod tests {
Some(executed) if executed.token == buy(0).token => Side::Buy,
_ => panic!(),
},
fee: Default::default(),
user_fee: Default::default(),
kind: Kind::Limit,
app_data: Default::default(),
partial: available
Expand Down
11 changes: 1 addition & 10 deletions crates/driver/src/domain/competition/solution/trade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,20 +77,11 @@ impl Fulfillment {
self.executed
}

/// Returns the fee that should be considered as collected when
/// scoring a solution.
pub fn scoring_fee(&self) -> order::SellAmount {
match self.fee {
Fee::Static => self.order.fee.solver,
Fee::Dynamic(fee) => fee,
}
}

/// Returns the effectively paid fee from the user's perspective
/// considering their signed order and the uniform clearing prices
pub fn fee(&self) -> order::SellAmount {
match self.fee {
Fee::Static => self.order.fee.user,
Fee::Static => self.order.user_fee,
Fee::Dynamic(fee) => fee,
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/domain/quote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl Order {
buy: self.buy(),
sell: self.sell(),
side: self.side,
fee: Default::default(),
user_fee: Default::default(),
kind: competition::order::Kind::Market,
app_data: Default::default(),
partial: competition::order::Partial::No,
Expand Down
7 changes: 1 addition & 6 deletions crates/driver/src/infra/api/routes/solve/dto/auction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,7 @@ impl Auction {
Kind::Sell => competition::order::Side::Sell,
Kind::Buy => competition::order::Side::Buy,
},
fee: competition::order::Fee {
user: order.user_fee.into(),
solver: order.solver_fee.into(),
},
user_fee: order.user_fee.into(),
kind: match order.class {
Class::Market => competition::order::Kind::Market,
Class::Limit => competition::order::Kind::Limit,
Expand Down Expand Up @@ -227,8 +224,6 @@ struct Order {
#[serde_as(as = "serialize::U256")]
buy_amount: eth::U256,
#[serde_as(as = "serialize::U256")]
solver_fee: eth::U256,
#[serde_as(as = "serialize::U256")]
user_fee: eth::U256,
protocol_fees: Vec<FeePolicy>,
valid_to: u32,
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/infra/solver/dto/auction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl Auction {
buy_token: available.buy.token.into(),
sell_amount: available.sell.amount.into(),
buy_amount: available.buy.amount.into(),
fee_amount: available.fee.solver.into(),
fee_amount: available.user_fee.into(),
kind: match order.side {
competition::order::Side::Buy => Kind::Buy,
competition::order::Side::Sell => Kind::Sell,
Expand Down
1 change: 0 additions & 1 deletion crates/driver/src/tests/setup/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,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(),
"solverFee": quote.order.user_fee.to_string(),
"userFee": quote.order.user_fee.to_string(),
"protocolFees": match quote.order.kind {
order::Kind::Market => json!([]),
Expand Down
2 changes: 1 addition & 1 deletion crates/e2e/tests/e2e/partially_fillable_balance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ 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.solver_fee, 0.into());
assert_eq!(order.user_fee, 0.into());

tracing::info!("Waiting for trade.");
let trade_happened =
Expand Down
2 changes: 1 addition & 1 deletion crates/e2e/tests/e2e/partially_fillable_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ 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.solver_fee, 0.into());
assert_eq!(order.user_fee, 0.into());

tracing::info!("Waiting for trade.");
let trade_happened =
Expand Down
4 changes: 4 additions & 0 deletions crates/model/src/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,8 @@ pub struct OrderMetadata {
/// execution we could find while quoting converted to an equivalent
/// `sell_token` amount.
/// Does not take partial fill into account.
///
/// [TO BE DEPRECATED]
#[serde_as(as = "HexOrDecimalU256")]
pub full_fee_amount: U256,
/// The fee amount that should be used for objective value computations.
Expand All @@ -767,6 +769,8 @@ pub struct OrderMetadata {
/// factor to make matching orders more valuable from an objective value
/// perspective.
/// Does not take partial fill into account.
///
/// [TO BE DEPRECATED]
#[serde_as(as = "HexOrDecimalU256")]
pub solver_fee: U256,
#[serde(default, skip_serializing_if = "Option::is_none")]
Expand Down
5 changes: 0 additions & 5 deletions crates/orderbook/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -928,10 +928,6 @@ components:
description: see `OrderParameters::feeAmount`
allOf:
- $ref: "#/components/schemas/TokenAmount"
solverFee:
description: Amount that the signed fee would be without subsidies.
allOf:
- $ref: "#/components/schemas/TokenAmount"
validTo:
description: see `OrderParameters::validTo`
type: integer
Expand Down Expand Up @@ -995,7 +991,6 @@ components:
- sellAmount
- buyAmount
- userFee
- solverFee
- validTo
- kind
- receiver
Expand Down
2 changes: 0 additions & 2 deletions crates/orderbook/src/dto/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ pub struct Order {
#[serde_as(as = "HexOrDecimalU256")]
pub buy_amount: U256,
#[serde_as(as = "HexOrDecimalU256")]
pub solver_fee: U256,
#[serde_as(as = "HexOrDecimalU256")]
pub user_fee: U256,
pub protocol_fees: Vec<FeePolicy>,
pub valid_to: u32,
Expand Down
1 change: 1 addition & 0 deletions crates/shared/src/http_solver/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub struct OrderModel {
pub buy_amount: U256,
pub allow_partial_fill: bool,
pub is_sell_order: bool,
/// Represents user_fee. Which is 0 for limit orders.
pub fee: TokenAmount,
pub cost: TokenAmount,
pub is_liquidity_order: bool,
Expand Down
17 changes: 6 additions & 11 deletions crates/solver/src/liquidity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,8 @@ pub struct LimitOrder {
pub buy_amount: U256,
pub kind: OrderKind,
pub partially_fillable: bool,
/// The fee that should be used for objective value computations.
/// Takes partiall fill into account.
pub scoring_fee: U256,
pub user_fee: U256,
#[cfg_attr(test, derivative(PartialEq = "ignore"))]
pub settlement_handling: Arc<dyn SettlementHandling<Self>>,
pub exchange: Exchange,
Expand Down Expand Up @@ -223,20 +222,16 @@ pub struct LimitOrderExecution {
/// The amount that the order `side` (`buy`, `sell`) should be filled by
/// this trade.
pub filled: U256,
/// The fee (for the objective value) associated with this order.
/// For limit orders this value gets computed by the
/// solver already refers to the `filled` amount. In this case no
/// further scaling is necessary for partial fills. For market orders
/// this is unsubsidized fee amount.
pub scoring_fee: U256,
/// this is signed user fee amount.
pub fee: U256,
}

impl LimitOrderExecution {
pub fn new(filled: U256, scoring_fee: U256) -> Self {
Self {
filled,
scoring_fee,
}
pub fn new(filled: U256, fee: U256) -> Self {
Self { filled, fee }
}
}

Expand Down Expand Up @@ -267,7 +262,7 @@ impl Default for LimitOrder {
buy_amount: Default::default(),
kind: Default::default(),
partially_fillable: Default::default(),
scoring_fee: Default::default(),
user_fee: Default::default(),
settlement_handling: tests::CapturingSettlementHandler::arc(),
id: Default::default(),
exchange: Exchange::GnosisProtocol,
Expand Down
Loading

0 comments on commit abac2c1

Please sign in to comment.