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

Jump to bottom

Remove Custom Price Order Sell Price from UCP #878

Merged
merged 6 commits into from
Dec 1, 2022
Merged

Remove Custom Price Order Sell Price from UCP #878

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3002ba8
Remove Custom Price Order Sell Price from UCP
sunce86 Nov 30, 2022
7ac16c1
cr fixes
sunce86 Dec 1, 2022
7bf675b
fixed unit tests
sunce86 Dec 1, 2022
b02c9af
fmt fix
sunce86 Dec 1, 2022
3785972
fix unit tests
sunce86 Dec 1, 2022
789c7d3
Merge branch 'main' into fix-liquidity-orders-price-bug
sunce86 Dec 1, 2022
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
150 changes: 63 additions & 87 deletions crates/solver/src/settlement/settlement_encoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ enum TokenReference {
},

/// Token reference for orders with a price that can be different from the
/// uniform clearing price. This is required for liquidity orders and limit
/// orders. Liquidity orders are not allowed to get surplus and therefore
/// uniform clearing price. This means that these prices need to be stored
/// outside of the uniform clearing price vector.
/// This is required for liquidity orders and limit orders.
/// Liquidity orders are not allowed to get surplus and therefore
/// have to be settled at their limit price. Prices for limit orders have to
/// be adjusted slightly to account for the `surplus_fee` mark up.
CustomPrice {
sell_token_index: usize,
sell_token_price: U256,
buy_token_price: U256,
},
}
Expand Down Expand Up @@ -174,12 +176,9 @@ impl SettlementEncoder {
self.clearing_prices[&self.tokens[buy_token_index]],
),
TokenReference::CustomPrice {
sell_token_index,
buy_token_price,
} => (
self.clearing_prices[&self.tokens[sell_token_index]],
sell_token_price,
buy_token_price,
),
} => (sell_token_price, buy_token_price),
};

PricedTrade {
Expand Down Expand Up @@ -254,11 +253,13 @@ impl SettlementEncoder {
self.add_market_trade(order, executed_amount, scaled_unsubsidized_fee)?
}
OrderClass::Liquidity => {
let buy_price = self.compute_limit_buy_price(&order)?;
let sell_price = order.data.buy_amount;
let buy_price = order.data.sell_amount;
self.add_custom_price_trade(
order,
executed_amount,
scaled_unsubsidized_fee,
sell_price,
buy_price,
)?
}
Expand All @@ -269,12 +270,13 @@ impl SettlementEncoder {
OrderKind::Sell => order.data.sell_amount,
OrderKind::Buy => order.data.buy_amount,
};
let buy_price = self.custom_price_for_limit_order(&order, limit)?;
let (sell_price, buy_price) = self.custom_price_for_limit_order(&order, limit)?;

self.add_custom_price_trade(
order,
executed_amount,
scaled_unsubsidized_fee,
sell_price,
buy_price,
)?
}
Expand All @@ -288,7 +290,11 @@ impl SettlementEncoder {
/// `compute_synthetic_order_amounts_if_limit_order()`).
/// Returns an error if the UCP doesn't contain the traded tokens or if under- or overflows
/// happen during the computation.
fn custom_price_for_limit_order(&self, order: &Order, limit: &LimitOrderClass) -> Result<U256> {
fn custom_price_for_limit_order(
&self,
order: &Order,
limit: &LimitOrderClass,
) -> Result<(U256, U256)> {
anyhow::ensure!(
order.metadata.class.is_limit(),
"this function should only be called for limit orders"
Expand Down Expand Up @@ -343,81 +349,39 @@ impl SettlementEncoder {
}
};

sell_amount
.checked_mul(uniform_sell_price)
.context("custom_buy_price computation failed")?
.checked_div(buy_amount)
.context("custom_buy_price computation failed")
}

/// Uses either the existing UCP for the sell token or the sell price within the order to
/// compute the price of the buy token right at the limit price of the passed order.
/// This is needed to settle liquidity orders in a way that gives them no surplus.
fn compute_limit_buy_price(&self, order: &Order) -> Result<U256> {
// Liquidity orders are settled at their limit price. We set:
// buy_price = sell_price * order.sellAmount / order.buyAmount, where sell_price is given from uniform clearing prices

// Rounding error checks:
// Following limit price constraint is checked in the smart contract:
// order.sellAmount.mul(sellPrice) >= order.buyAmount.mul(buyPrice),

// This equation will always be true, as the following equivalence is showing
// order.sellAmount.mul(sellPrice) >= order.sellAmount.mul(sellPrice) holds, as the left and ride side are equal.
// Dividing first and then multiplying by order.buyAmount can only make the right side smaller
// <=> order.sellAmount.mul(sellPrice) >= order.buyAmount.mul(sell_price * order.sellAmount / order.buyAmount)
// <=> order.sellAmount.mul(sellPrice) >= order.buyAmount.mul(buyPrice)
// <=> equation from smart contract

self.clearing_prices
.get(&order.data.sell_token)
.unwrap_or(&order.data.buy_amount)
.checked_mul(order.data.sell_amount)
.context("buy_price calculation failed")?
.checked_div(order.data.buy_amount)
.context("buy_price calculation failed")
Ok((
uniform_sell_price,
sell_amount
.checked_mul(uniform_sell_price)
.context("custom_buy_price computation failed")?
.checked_div(buy_amount)
.context("custom_buy_price computation failed")?,
))
sunce86 marked this conversation as resolved.
Show resolved Hide resolved
}

fn add_custom_price_trade(
&mut self,
order: Order,
executed_amount: U256,
scaled_unsubsidized_fee: U256,
sell_price: U256,
buy_price: U256,
) -> Result<TradeExecution> {
verify_executed_amount(&order, executed_amount)?;
// For the encoding strategy of liquidity and limit orders, the sell prices are taken from
// the uniform clearing price vector. Therefore, either there needs to be an existing price
// for the sell token in the uniform clearing prices or we have to create a new price entry beforehand,
// if the sell token price is not yet available
if self.token_index(order.data.sell_token).is_none() {
let sell_token = order.data.sell_token;
let sell_price = order.data.buy_amount;
self.tokens.push(sell_token);
self.clearing_prices.insert(sell_token, sell_price);
Copy link
Contributor

Choose a reason for hiding this comment

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

For context the assumption is that this line added the buy_amount of a foreign liquidity order contained in a solution to the clearing price vector of the Settlement if that order's sell_token was not already part of the clearing prices. Because this price is not denominated in ETH and actually only makes sense in relation to the custom price associated to that order it can make the Settlement fail the price deviation check.

self.sort_tokens_and_update_indices();
};
let sell_price = self
.clearing_prices
.get(&order.data.sell_token)
.context("settlement missing sell token")?;
let sell_token_index = self
.token_index(order.data.sell_token)
.context("settlement missing sell token")?;
sunce86 marked this conversation as resolved.
Show resolved Hide resolved

let trade = EncoderTrade {
data: Trade {
order: order.clone(),
executed_amount,
scaled_unsubsidized_fee,
},
tokens: TokenReference::CustomPrice {
sell_token_index,
sell_token_price: sell_price,
buy_token_price: buy_price,
},
};
let execution = trade
.data
.executed_amounts(*sell_price, buy_price)
.executed_amounts(sell_price, buy_price)
.context("impossible trade execution")?;

self.trades.push(trade);
Expand Down Expand Up @@ -483,21 +447,20 @@ impl SettlementEncoder {
self.tokens.sort();

for i in 0..self.trades.len() {
let sell_token_index = self
.token_index(self.trades[i].data.order.data.sell_token)
.expect("missing sell token for existing trade");

self.trades[i].tokens = match self.trades[i].tokens {
TokenReference::Indexed { .. } => TokenReference::Indexed {
sell_token_index,
sell_token_index: self
.token_index(self.trades[i].data.order.data.sell_token)
.expect("missing sell token for existing trade"),
buy_token_index: self
.token_index(self.trades[i].data.order.data.buy_token)
.expect("missing buy token for existing trade"),
},
TokenReference::CustomPrice {
sunce86 marked this conversation as resolved.
Show resolved Hide resolved
buy_token_price, ..
sell_token_price,
buy_token_price,
} => TokenReference::CustomPrice {
sell_token_index,
sell_token_price,
buy_token_price,
},
};
Expand Down Expand Up @@ -551,17 +514,6 @@ impl SettlementEncoder {
) -> EncodedSettlement {
self.drop_unnecessary_tokens_and_prices();

let (mut liquidity_order_buy_tokens, mut liquidity_order_prices): (Vec<H160>, Vec<U256>) =
self.trades
.iter()
.filter_map(|trade| match trade.tokens {
TokenReference::CustomPrice {
buy_token_price, ..
} => Some((trade.data.order.data.buy_token, buy_token_price)),
_ => None,
})
.unzip();

let uniform_clearing_price_vec_length = self.tokens.len();
let mut tokens = self.tokens.clone();
let mut clearing_prices: Vec<U256> = self
Expand All @@ -575,8 +527,30 @@ impl SettlementEncoder {
})
.collect();

tokens.append(&mut liquidity_order_buy_tokens);
clearing_prices.append(&mut liquidity_order_prices);
{
// add tokens/prices for custom price orders, since they are not contained in the UCP vector
let (mut custom_price_order_tokens, mut custom_price_order_prices): (
Vec<H160>,
Vec<U256>,
) = self
.trades
.iter()
.filter_map(|trade| match trade.tokens {
TokenReference::CustomPrice {
sell_token_price,
buy_token_price,
} => Some(vec![
(trade.data.order.data.sell_token, sell_token_price),
(trade.data.order.data.buy_token, buy_token_price),
]),
_ => None,
})
.flatten()
.unzip();

tokens.append(&mut custom_price_order_tokens);
clearing_prices.append(&mut custom_price_order_prices);
}

let (_, trades) = self.trades.into_iter().fold(
(uniform_clearing_price_vec_length, Vec::new()),
Expand All @@ -586,9 +560,11 @@ impl SettlementEncoder {
sell_token_index,
buy_token_index,
} => (sell_token_index, buy_token_index, custom_price_index),
TokenReference::CustomPrice {
sell_token_index, ..
} => (sell_token_index, custom_price_index, custom_price_index + 1),
TokenReference::CustomPrice { .. } => (
custom_price_index,
custom_price_index + 1,
custom_price_index + 2,
),
};

trades.push(trade.data.encode(sell_token_index, buy_token_index));
Expand Down