Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ON HOLD: Calculate score as per "rank by surplus" CIP #2406

Closed
wants to merge 20 commits into from
Closed

ON HOLD: Calculate score as per "rank by surplus" CIP #2406

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6030e2f
Refactor surplus calculation
sunce86 Feb 12, 2024
8a5e469
fix copy paste error
sunce86 Feb 12, 2024
6b08988
Calculate new score
sunce86 Feb 12, 2024
428ece6
Refactor surplus calculation
sunce86 Feb 12, 2024
3763f1b
Calculate new score
sunce86 Feb 12, 2024
fa12490
merge
sunce86 Feb 13, 2024
e15dda1
fixes
sunce86 Feb 13, 2024
486d235
conversion to token amounts
sunce86 Feb 13, 2024
e83ec4f
error handling
sunce86 Feb 13, 2024
f424e8b
small fix
sunce86 Feb 13, 2024
61840e0
fix comment
sunce86 Feb 13, 2024
b7923c8
renaming
sunce86 Feb 13, 2024
52c74f1
todo CIP38
sunce86 Feb 13, 2024
eecace7
settlement score
sunce86 Feb 19, 2024
3cc1e9c
self cr
sunce86 Feb 19, 2024
f3df0c5
comments
sunce86 Feb 19, 2024
d5981f0
fix bug
sunce86 Feb 19, 2024
05dd8a7
renaming
sunce86 Feb 20, 2024
4e36c36
added comment
sunce86 Feb 20, 2024
212c971
small fix
sunce86 Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 48 additions & 46 deletions crates/driver/src/domain/competition/solution/fee.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ impl Fulfillment {
factor,
max_volume_factor,
}) => self.calculate_fee(
self.order().sell.amount.0,
self.order().buy.amount.0,
self.order().sell.amount,
self.order().buy.amount,
prices,
*factor,
*max_volume_factor,
Expand All @@ -97,12 +97,12 @@ impl Fulfillment {
quote,
}) => {
let (sell_amount, buy_amount) = adjust_quote_to_order_limits(
self.order().sell.amount.0,
self.order().buy.amount.0,
self.order().sell.amount,
self.order().buy.amount,
self.order().side,
quote.sell.amount.0,
quote.buy.amount.0,
quote.fee.amount.0,
quote.sell.amount,
quote.buy.amount,
quote.fee.amount,
)?;
self.calculate_fee(sell_amount, buy_amount, prices, *factor, *max_volume_factor)
}
Expand All @@ -115,8 +115,8 @@ impl Fulfillment {
/// the order or a quote.
fn calculate_fee(
&self,
limit_sell_amount: eth::U256,
limit_buy_amount: eth::U256,
limit_sell_amount: eth::TokenAmount,
limit_buy_amount: eth::TokenAmount,
prices: ClearingPrices,
factor: f64,
max_volume_factor: f64,
Expand All @@ -132,8 +132,8 @@ impl Fulfillment {

fn fee_from_surplus(
&self,
sell_amount: eth::U256,
buy_amount: eth::U256,
sell_amount: eth::TokenAmount,
buy_amount: eth::TokenAmount,
prices: ClearingPrices,
factor: f64,
) -> Result<eth::U256, Error> {
Expand Down Expand Up @@ -195,35 +195,37 @@ fn apply_factor(amount: eth::U256, factor: f64) -> Result<eth::U256, Error> {
/// - test_adjust_quote_to_in_market_sell_order_limits
/// - test_adjust_quote_to_in_market_buy_order_limits
fn adjust_quote_to_order_limits(
order_sell_amount: eth::U256,
order_buy_amount: eth::U256,
order_sell_amount: eth::TokenAmount,
MartinquaXD marked this conversation as resolved.
Show resolved Hide resolved
order_buy_amount: eth::TokenAmount,
order_side: Side,
quote_sell_amount: eth::U256,
quote_buy_amount: eth::U256,
quote_fee_amount: eth::U256,
) -> Result<(eth::U256, eth::U256), Error> {
quote_sell_amount: eth::TokenAmount,
quote_buy_amount: eth::TokenAmount,
quote_fee_amount: eth::TokenAmount,
) -> Result<(eth::TokenAmount, eth::TokenAmount), Error> {
let quote_sell_amount = quote_sell_amount
.checked_add(quote_fee_amount)
.0
.checked_add(quote_fee_amount.0)
.ok_or(trade::Error::Overflow)?;

match order_side {
Side::Sell => {
let scaled_buy_amount = quote_buy_amount
.checked_mul(order_sell_amount)
.0
.checked_mul(order_sell_amount.0)
.ok_or(trade::Error::Overflow)?
.checked_div(quote_sell_amount)
.ok_or(trade::Error::DivisionByZero)?;
let buy_amount = order_buy_amount.max(scaled_buy_amount);
Ok((order_sell_amount, buy_amount))
let buy_amount = order_buy_amount.0.max(scaled_buy_amount);
Ok((order_sell_amount, buy_amount.into()))
}
Side::Buy => {
let scaled_sell_amount = quote_sell_amount
.checked_mul(order_buy_amount)
.checked_mul(order_buy_amount.0)
.ok_or(trade::Error::Overflow)?
.checked_div(quote_buy_amount)
.checked_div(quote_buy_amount.0)
.ok_or(trade::Error::DivisionByZero)?;
let sell_amount = order_sell_amount.min(scaled_sell_amount);
Ok((sell_amount, order_buy_amount))
let sell_amount = order_sell_amount.0.min(scaled_sell_amount);
Ok((sell_amount.into(), order_buy_amount))
}
}
}
Expand All @@ -243,11 +245,11 @@ mod tests {

#[test]
fn test_adjust_quote_to_out_market_sell_order_limits() {
let order_sell_amount = to_wei(20);
let order_buy_amount = to_wei(19);
let quote_sell_amount = to_wei(21);
let quote_buy_amount = to_wei(18);
let quote_fee_amount = to_wei(1);
let order_sell_amount = to_wei(20).into();
let order_buy_amount = to_wei(19).into();
let quote_sell_amount = to_wei(21).into();
let quote_buy_amount = to_wei(18).into();
let quote_fee_amount = to_wei(1).into();

let (sell_amount, _) = adjust_quote_to_order_limits(
order_sell_amount,
Expand All @@ -267,11 +269,11 @@ mod tests {

#[test]
fn test_adjust_quote_to_out_market_buy_order_limits() {
let order_sell_amount = to_wei(20);
let order_buy_amount = to_wei(19);
let quote_sell_amount = to_wei(21);
let quote_buy_amount = to_wei(18);
let quote_fee_amount = to_wei(1);
let order_sell_amount = to_wei(20).into();
let order_buy_amount = to_wei(19).into();
let quote_sell_amount = to_wei(21).into();
let quote_buy_amount = to_wei(18).into();
let quote_fee_amount = to_wei(1).into();

let (_, buy_amount) = adjust_quote_to_order_limits(
order_sell_amount,
Expand All @@ -291,11 +293,11 @@ mod tests {

#[test]
fn test_adjust_quote_to_in_market_sell_order_limits() {
let order_sell_amount = to_wei(10);
let order_buy_amount = to_wei(20);
let quote_sell_amount = to_wei(9);
let quote_buy_amount = to_wei(25);
let quote_fee_amount = to_wei(1);
let order_sell_amount = to_wei(10).into();
let order_buy_amount = to_wei(20).into();
let quote_sell_amount = to_wei(9).into();
let quote_buy_amount = to_wei(25).into();
let quote_fee_amount = to_wei(1).into();

let (sell_amount, buy_amount) = adjust_quote_to_order_limits(
order_sell_amount,
Expand All @@ -319,11 +321,11 @@ mod tests {

#[test]
fn test_adjust_quote_to_in_market_buy_order_limits() {
let order_sell_amount = to_wei(20);
let order_buy_amount = to_wei(10);
let quote_sell_amount = to_wei(17);
let quote_buy_amount = to_wei(10);
let quote_fee_amount = to_wei(1);
let order_sell_amount = to_wei(20).into();
let order_buy_amount = to_wei(10).into();
let quote_sell_amount = to_wei(17).into();
let quote_buy_amount = to_wei(10).into();
let quote_fee_amount = to_wei(1).into();

let (sell_amount, buy_amount) = adjust_quote_to_order_limits(
order_sell_amount,
Expand All @@ -337,7 +339,7 @@ mod tests {

assert_eq!(
sell_amount,
quote_sell_amount + quote_fee_amount,
(quote_sell_amount.0 + quote_fee_amount.0).into(),
"Sell amount should reflect the improved market condition from the quote for buy \
orders."
);
Expand Down
34 changes: 33 additions & 1 deletion crates/driver/src/domain/competition/solution/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ pub struct Solution {
prices: HashMap<eth::TokenAddress, eth::U256>,
interactions: Vec<Interaction>,
solver: Solver,
score: SolverScore,
score: SolverScore, // todo CIP38 remove
score_cip38: SolverScoreCIP38,
weth: eth::WethAddress,
}

Expand All @@ -52,13 +53,30 @@ impl Solution {
score: SolverScore,
weth: eth::WethAddress,
) -> Result<Self, SolutionError> {
let score_cip38 = SolverScoreCIP38 {
surplus: {
let mut surplus = vec![];
for trade in trades.iter() {
match trade.score(&prices, weth) {
Ok(score) => surplus.push(score),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As is the surplus vector will contain multiple entries for the same token and 0 surplus entries for JIT orders.
Should we aggregate those surpluses in the same token and filter out 0 amounts to remove noise or are the individual entries specifically needed for some bookkeeping?

Copy link
Contributor Author

@sunce86 sunce86 Feb 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we aggregate them at the solution level, I have to do it again at the settlement level, since score is calculated for settlement and settlement can have multiple solutions. So i figured I just do it once at the settlement level and keep vector here for potential debugging purposes.

Err(_err) => {
// todo CIP38 enable
// return Err(SolutionError::Scoring(err))
}
}
}
surplus
},
};

let solution = Self {
id,
trades,
prices,
interactions,
solver,
score,
score_cip38,
weth,
};

Expand Down Expand Up @@ -97,6 +115,11 @@ impl Solution {
&self.score
}

/// The score of a solution as per CIP38
pub fn score_cip38(&self) -> &SolverScoreCIP38 {
&self.score_cip38
}

/// Approval interactions necessary for encoding the settlement.
pub async fn approvals(
&self,
Expand Down Expand Up @@ -277,6 +300,13 @@ pub enum SolverScore {
Solver(eth::U256),
RiskAdjusted(f64),
}

#[derive(Debug, Clone)]
pub struct SolverScoreCIP38 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this struct need to be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's referenced in the boundary

// todo CIP38 rename to SolverScore
pub surplus: Vec<eth::Asset>,
}

/// A unique solution ID. This ID is generated by the solver and only needs to
/// be unique within a single round of competition. This ID is only important in
/// the communication between the driver and the solver, and it is not used by
Expand Down Expand Up @@ -318,6 +348,8 @@ pub enum Error {

#[derive(Debug, thiserror::Error)]
pub enum SolutionError {
#[error("scoring failed: {0:?}")]
Scoring(#[from] crate::domain::competition::solution::trade::Error),
#[error("invalid clearing prices")]
InvalidClearingPrices,
#[error(transparent)]
Expand Down
60 changes: 55 additions & 5 deletions crates/driver/src/domain/competition/solution/trade.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,30 @@ pub enum Trade {
Jit(Jit),
}

impl Trade {
pub fn score(
&self,
prices: &HashMap<eth::TokenAddress, eth::U256>,
weth: eth::WethAddress,
) -> Result<eth::Asset, Error> {
match self {
Self::Fulfillment(fulfillment) => {
let prices = ClearingPrices {
sell: prices[&fulfillment.order().sell.token.wrap(weth)],
buy: prices[&fulfillment.order().buy.token.wrap(weth)],
};

fulfillment.surplus(prices)
}
// JIT orders have a zero score
Self::Jit(jit) => Ok(eth::Asset {
token: jit.order().sell.token,
amount: 0.into(),
}),
}
}
}

/// A trade which fulfills an order from the auction.
#[derive(Debug, Clone)]
pub struct Fulfillment {
Expand Down Expand Up @@ -137,12 +161,15 @@ impl Fulfillment {

/// Returns the surplus denominated in the surplus token.
///
/// The surplus token is the buy token for a sell order and sell token for a
/// The surplus token is a buy token for a sell order and a sell token for a
/// buy order.
///
/// The surplus is defined as the improvement of price, i.e. the difference
/// between the executed price and the reference (limit) price.
pub fn surplus_over_reference_price(
&self,
limit_sell: eth::U256,
limit_buy: eth::U256,
limit_sell: eth::TokenAmount,
limit_buy: eth::TokenAmount,
prices: ClearingPrices,
) -> Result<eth::U256, Error> {
let executed = self.executed().0;
Expand Down Expand Up @@ -170,9 +197,10 @@ impl Fulfillment {
Side::Buy => {
// Scale to support partially fillable orders
let limit_sell_amount = limit_sell
.0
.checked_mul(executed)
.ok_or(Error::Overflow)?
.checked_div(limit_buy)
.checked_div(limit_buy.0)
.ok_or(Error::DivisionByZero)?;
// Remaining surplus after fees
// Do not return error if `checked_sub` fails because violated limit prices will
Expand All @@ -184,9 +212,10 @@ impl Fulfillment {
Side::Sell => {
// Scale to support partially fillable orders
let limit_buy_amount = limit_buy
.0
.checked_mul(executed_sell_amount_with_fee)
.ok_or(Error::Overflow)?
.checked_div(limit_sell)
.checked_div(limit_sell.0)
.ok_or(Error::DivisionByZero)?;
// How much `buy_token` we get for `executed` amount of `sell_token`
let executed_buy_amount = executed
Expand All @@ -205,6 +234,27 @@ impl Fulfillment {
Ok(surplus)
}

/// Returns the surplus denominated in the surplus token.
///
/// The surplus token is a buy token for a sell order and a sell token for a
/// buy order.
///
/// The surplus is defined as the difference between the executed price and
/// the order limit price.
pub fn surplus(&self, prices: ClearingPrices) -> Result<eth::Asset, Error> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn surplus(&self, prices: ClearingPrices) -> Result<eth::Asset, Error> {
pub fn surplus(&self, prices: &ClearingPrices) -> Result<eth::Asset, Error> {

Seems like we could only take a reference everywhere where we currently require ownership.

Also unrelated to this PR it's a bit awkward that the ClearingPrices are strictly related to a Trade but this is not enforce on a type level. Also the struct is pub in the crate so I think it's probably possible to skrew up and use ClearingPrices for a trade it's not related to.
Just wanted to bring it up in case we want to refactor the ClearingPrices at some point.

let limit_sell = self.order().sell.amount;
let limit_buy = self.order().buy.amount;

self.surplus_over_reference_price(limit_sell, limit_buy, prices)
.map(|surplus| eth::Asset {
token: match self.order().side {
Side::Sell => self.order().buy.token,
Side::Buy => self.order().sell.token,
},
amount: surplus.into(),
})
}

/// Returns the surplus denominated in the sell token.
pub fn surplus_in_sell_token(
&self,
Expand Down
3 changes: 3 additions & 0 deletions crates/driver/src/infra/solver/dto/solution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ impl Solutions {
competition::solution::SolutionError::ProtocolFee(err) => {
super::Error(format!("could not incorporate protocol fee: {err}"))
}
competition::solution::SolutionError::Scoring(err) => {
super::Error(format!("could not calculate score: {err}"))
}
})
})
.collect()
Expand Down
Loading