Skip to content

Commit

Permalink
Rename solver_fee when referring to unsubsidised scoring fee (#2152)
Browse files Browse the repository at this point in the history
# Description
Follow up in #2151 this is an attempt to rename most of the
driver/solver relevant use of "solver_fee" when meaning the
"unsubsidised fee that solvers should consider when evaluating the
object value for this order" since it's confusing when we also refer to
the fee that solvers chose e.g. for limit orders as "solver fee"

Notable, it doesn't change the field on the `order.metadata` struct, as
I'm afraid this may be used by the frontend or other dependencies.

# Changes
- [ ] Rename `solver_fee` to `scoring_fee` on `Trade`
- [ ] Cascading renames from there on
- [ ] Stop at the `order.metadata` struct (there we keep surplus_fee for
backwards compatibility reasons). It's sufficiently documented on this
struct

## How to test
Just™️  a rename 🤞 , CI still passes.

<!--
## Related Issues

Fixes #
-->

---------

Co-authored-by: narcis96 <[email protected]>
  • Loading branch information
fleupold and narcis96 authored Dec 13, 2023
1 parent 486e836 commit ebe4716
Show file tree
Hide file tree
Showing 16 changed files with 80 additions and 87 deletions.
4 changes: 2 additions & 2 deletions crates/database/src/order_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub async fn save(
order: &OrderUid,
auction: AuctionId,
surplus_fee: Option<&BigDecimal>,
solver_fee: Option<&BigDecimal>,
scoring_fee: Option<&BigDecimal>,
) -> Result<(), sqlx::Error> {
const QUERY: &str = r#"
INSERT INTO order_execution (order_uid, auction_id, reward, surplus_fee, solver_fee)
Expand All @@ -21,7 +21,7 @@ VALUES ($1, $2, $3, $4, $5)
.bind(auction)
.bind(0.) // reward is deprecated but saved for historical analysis
.bind(surplus_fee)
.bind(solver_fee)
.bind(scoring_fee)
.execute(ex)
.await?;
Ok(())
Expand Down
6 changes: 3 additions & 3 deletions crates/driver/src/boundary/settlement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,15 @@ impl Settlement {
to_boundary_order(trade.order()),
LimitOrderExecution {
filled: trade.executed().into(),
solver_fee: trade.scoring_fee().into(),
scoring_fee: trade.scoring_fee().into(),
},
)
}
competition::solution::Trade::Jit(trade) => (
to_boundary_jit_order(&DomainSeparator(domain.0), trade.order()),
LimitOrderExecution {
filled: trade.executed().into(),
solver_fee: 0.into(),
scoring_fee: 0.into(),
},
),
};
Expand Down Expand Up @@ -233,7 +233,7 @@ impl Settlement {
)?;

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

Ok(eth::U256::from_big_rational(&quality)?.into())
Expand Down
2 changes: 1 addition & 1 deletion crates/model/src/solver_competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub struct Execution {
#[serde_as(as = "Option<HexOrDecimalU256>")]
pub surplus_fee: Option<U256>,
#[serde_as(as = "HexOrDecimalU256")]
pub solver_fee: U256,
pub scoring_fee: U256,
}

/// Stored directly in the database and turned into SolverCompetitionAPI for the
Expand Down
2 changes: 1 addition & 1 deletion crates/orderbook/src/database/solver_competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl SolverCompetitionStoring for Postgres {
&ByteArray(order.0),
request.auction,
surplus_fee.as_ref(),
Some(&u256_to_big_decimal(&execution.solver_fee)),
Some(&u256_to_big_decimal(&execution.scoring_fee)),
)
.await
.context("order_execution::save")?;
Expand Down
2 changes: 1 addition & 1 deletion crates/solver/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ impl Driver {
.map(|trade| {
let execution = Execution {
surplus_fee: trade.surplus_fee(),
solver_fee: trade.solver_fee,
scoring_fee: trade.scoring_fee,
};
(trade.order.metadata.uid, execution)
})
Expand Down
20 changes: 11 additions & 9 deletions crates/solver/src/liquidity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pub struct LimitOrder {
pub partially_fillable: bool,
/// The fee that should be used for objective value computations.
/// Takes partiall fill into account.
pub solver_fee: U256,
pub scoring_fee: U256,
#[cfg_attr(test, derivative(PartialEq = "ignore"))]
pub settlement_handling: Arc<dyn SettlementHandling<Self>>,
pub exchange: Exchange,
Expand Down Expand Up @@ -224,17 +224,19 @@ pub struct LimitOrderExecution {
/// this trade.
pub filled: U256,
/// The fee (for the objective value) associated with this order.
/// For partially fillable limit orders this value gets computed by the
/// 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 all other orders
/// this is the `solver_fee` for the entire order and will get scaled
/// correctly by the [`SettlementEncoder`].
pub solver_fee: U256,
/// further scaling is necessary for partial fills. For market orders
/// this is unsubsidized fee amount.
pub scoring_fee: U256,
}

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

Expand Down Expand Up @@ -265,7 +267,7 @@ impl Default for LimitOrder {
buy_amount: Default::default(),
kind: Default::default(),
partially_fillable: Default::default(),
solver_fee: Default::default(),
scoring_fee: Default::default(),
settlement_handling: tests::CapturingSettlementHandler::arc(),
id: Default::default(),
exchange: Exchange::GnosisProtocol,
Expand Down
16 changes: 8 additions & 8 deletions crates/solver/src/liquidity/order_converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl OrderConverter {

// The reported fee amount that is used for objective computation is the
// order's full full amount scaled by a constant factor.
let solver_fee = match order.solver_determines_fee() {
let scoring_fee = match order.solver_determines_fee() {
true => 0.into(),
false => remaining.remaining(order.metadata.solver_fee)?,
};
Expand All @@ -85,7 +85,7 @@ impl OrderConverter {
buy_amount,
kind: order.data.kind,
partially_fillable: order.data.partially_fillable,
solver_fee,
scoring_fee,
settlement_handling: Arc::new(OrderSettlementHandler {
order,
native_token: self.native_token.clone(),
Expand Down Expand Up @@ -116,7 +116,7 @@ impl SettlementHandling<LimitOrder> for OrderSettlementHandler {
}

let trade =
encoder.add_trade(self.order.clone(), execution.filled, execution.solver_fee)?;
encoder.add_trade(self.order.clone(), execution.filled, execution.scoring_fee)?;

if is_native_token_buy_order {
encoder.add_unwrap(UnwrapWethInteraction {
Expand Down Expand Up @@ -195,7 +195,7 @@ pub mod tests {

let execution = LimitOrderExecution::new(1337.into(), 0.into());
let executed_buy_amount = U256::from(2 * 1337);
let solver_fee = U256::from(1234);
let scoring_fee = U256::from(1234);

let prices = hashmap! {
native_token.address() => U256::from(100),
Expand Down Expand Up @@ -231,7 +231,7 @@ pub mod tests {
amount: executed_buy_amount,
});
assert!(encoder
.add_trade(order, execution.filled, solver_fee)
.add_trade(order, execution.filled, scoring_fee)
.is_ok());
},
);
Expand Down Expand Up @@ -359,7 +359,7 @@ pub mod tests {
// Amounts are halved because the order is half executed.
assert_eq!(order_.sell_amount, 10.into());
assert_eq!(order_.buy_amount, 20.into());
assert_eq!(order_.solver_fee, 100.into());
assert_eq!(order_.scoring_fee, 100.into());

let order_ = converter
.normalize_limit_order(BalancedOrder {
Expand All @@ -370,7 +370,7 @@ pub mod tests {
// Amounts are quartered because of balance.
assert_eq!(order_.sell_amount, 5.into());
assert_eq!(order_.buy_amount, 10.into());
assert_eq!(order_.solver_fee, 50.into());
assert_eq!(order_.scoring_fee, 50.into());

order.metadata.executed_sell_amount_before_fees = 0.into();
let order_ = converter
Expand All @@ -382,7 +382,7 @@ pub mod tests {
// Amounts are still quartered because of balance.
assert_eq!(order_.sell_amount, 5.into());
assert_eq!(order_.buy_amount, 10.into());
assert_eq!(order_.solver_fee, 50.into());
assert_eq!(order_.scoring_fee, 50.into());
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion crates/solver/src/liquidity/zeroex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl ZeroExLiquidity {
buy_amount: record.metadata.remaining_fillable_taker_amount.into(),
kind: OrderKind::Buy,
partially_fillable: true,
solver_fee: U256::zero(),
scoring_fee: U256::zero(),
settlement_handling: Arc::new(OrderSettlementHandler {
order: record.order,
zeroex: self.zeroex.clone(),
Expand Down
2 changes: 1 addition & 1 deletion crates/solver/src/objective_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl Inputs {

Self {
surplus_given: settlement.total_surplus(prices),
solver_fees: settlement.total_solver_fees(prices),
solver_fees: settlement.total_scoring_fees(prices),
gas_price,
gas_amount,
}
Expand Down
51 changes: 21 additions & 30 deletions crates/solver/src/settlement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ pub struct Trade {
pub order: Order,
pub executed_amount: U256,
/// The fee amount used for objective value computations.
pub solver_fee: U256,
pub scoring_fee: U256,
}

impl Trade {
/// Returns the fee taken from the surplus.
pub fn surplus_fee(&self) -> Option<U256> {
match self.order.solver_determines_fee() {
true => Some(self.solver_fee),
true => Some(self.scoring_fee),
false => None,
}
}
Expand Down Expand Up @@ -146,22 +146,13 @@ impl Trade {
self.scale_amount(self.order.data.fee_amount)
}

/// Returns the solver fee used for computing the objective value adjusted
/// for partial fills.
pub fn executed_solver_fee(&self) -> Option<U256> {
match self.order.solver_determines_fee() {
true => Some(self.solver_fee),
false => self.scale_amount(self.solver_fee),
}
}

/// Returns the actual fees taken by the protocol.
pub fn executed_earned_fee(&self) -> Option<U256> {
let user_fee = self.order.data.fee_amount;
match self.order.solver_determines_fee() {
true => {
// Solvers already scale the `solver_fee` for these orders.
self.scale_amount(user_fee)?.checked_add(self.solver_fee)
// Solvers already scale the `scoring_fee` for these orders.
self.scale_amount(user_fee)?.checked_add(self.scoring_fee)
}
false => self.scale_amount(user_fee),
}
Expand Down Expand Up @@ -442,12 +433,12 @@ impl Settlement {

/// Computes the total solver fees (in wei) used to compute the objective
/// value.
pub fn total_solver_fees(&self, external_prices: &ExternalPrices) -> BigRational {
pub fn total_scoring_fees(&self, external_prices: &ExternalPrices) -> BigRational {
self.user_trades()
.filter_map(|trade| {
external_prices.try_get_native_amount(
trade.order.data.sell_token,
trade.executed_solver_fee()?.to_big_rational(),
trade.scoring_fee.to_big_rational(),
)
})
.sum()
Expand Down Expand Up @@ -1335,7 +1326,7 @@ pub mod tests {
// Note that the scaled fee amount is different than the order's
// signed fee amount. This happens for subsidized orders, and when
// a fee objective scaling factor is configured.
solver_fee: 5.into(),
scoring_fee: 5.into(),
};
let trade1 = Trade {
order: Order {
Expand All @@ -1349,7 +1340,7 @@ pub mod tests {
..Default::default()
},
executed_amount: 10.into(),
solver_fee: 2.into(),
scoring_fee: 2.into(),
};

let clearing_prices = hashmap! {token0 => 5.into(), token1 => 10.into()};
Expand All @@ -1361,14 +1352,14 @@ pub mod tests {

// Fee in sell tokens
assert_eq!(trade0.executed_fee().unwrap(), 1.into());
assert_eq!(trade0.executed_solver_fee().unwrap(), 5.into());
assert_eq!(trade0.scoring_fee, 5.into());
assert_eq!(trade1.executed_fee().unwrap(), 2.into());
assert_eq!(trade1.executed_solver_fee().unwrap(), 2.into());
assert_eq!(trade1.scoring_fee, 2.into());

// Fee in wei of ETH
let settlement = test_settlement(clearing_prices, vec![trade0, trade1]);
assert_eq!(
settlement.total_solver_fees(&external_prices),
settlement.total_scoring_fees(&external_prices),
BigRational::from_integer(45.into())
);
}
Expand All @@ -1395,7 +1386,7 @@ pub mod tests {
},
executed_amount: 1.into(),
// This is what matters for the objective value
solver_fee: 42.into(),
scoring_fee: 42.into(),
},
Trade {
order: Order {
Expand All @@ -1415,13 +1406,13 @@ pub mod tests {
},
executed_amount: 1.into(),
// Doesn't count because it is a "liquidity order"
solver_fee: 1337.into(),
scoring_fee: 1337.into(),
},
],
);

assert_eq!(
settlement.total_solver_fees(&externalprices! { native_token: token0 }),
settlement.total_scoring_fees(&externalprices! { native_token: token0 }),
r(42),
);
}
Expand All @@ -1447,7 +1438,7 @@ pub mod tests {
..Default::default()
},
executed_amount: 99760667014_u128.into(),
solver_fee: 239332986_u128.into(),
scoring_fee: 239332986_u128.into(),
}],
);

Expand All @@ -1472,7 +1463,7 @@ pub mod tests {
..Default::default()
},
executed_amount: 99760667014_u128.into(),
solver_fee: 239332986_u128.into(),
scoring_fee: 239332986_u128.into(),
},
Trade {
order: Order {
Expand All @@ -1492,7 +1483,7 @@ pub mod tests {
..Default::default()
},
executed_amount: 99760667014_u128.into(),
solver_fee: 77577144_u128.into(),
scoring_fee: 77577144_u128.into(),
},
],
);
Expand All @@ -1509,7 +1500,7 @@ pub mod tests {
let gas_price = 105386573044;
let objective_value = |settlement: &Settlement, gas: u128| {
settlement.total_surplus(&external_prices)
+ settlement.total_solver_fees(&external_prices)
+ settlement.total_scoring_fees(&external_prices)
- r(gas * gas_price)
};

Expand Down Expand Up @@ -1555,7 +1546,7 @@ pub mod tests {
..Default::default()
},
executed_amount: 99_000_u128.into(),
solver_fee: 1_000_u128.into(),
scoring_fee: 1_000_u128.into(),
}],
);

Expand Down Expand Up @@ -1590,7 +1581,7 @@ pub mod tests {
OrderKind::Sell => 99_000_u128,
}
.into(),
solver_fee: 1_000_u128.into(),
scoring_fee: 1_000_u128.into(),
}],
);

Expand Down Expand Up @@ -1628,7 +1619,7 @@ pub mod tests {
..Default::default()
},
executed_amount: 99_000_u128.into(),
solver_fee: 1_000_u128.into(),
scoring_fee: 1_000_u128.into(),
}],
)
.encode(InternalizationStrategy::SkipInternalizableInteraction);
Expand Down
Loading

0 comments on commit ebe4716

Please sign in to comment.