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

Remove executed_solver_fee #2178

Merged
merged 7 commits into from
Dec 19, 2023
Merged

Remove executed_solver_fee #2178

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
19f5bdf
Remove executed_solver_fee
sunce86 Dec 16, 2023
02d24e2
remove transpose
sunce86 Dec 16, 2023
2cb4e89
cleanup
sunce86 Dec 16, 2023
0cb891e
cr fixes
sunce86 Dec 18, 2023
68520cb
comment
sunce86 Dec 19, 2023
5ec89e9
cr fixes
sunce86 Dec 19, 2023
8743f40
Merge branch 'main' into replace-executed-solver-fee
sunce86 Dec 19, 2023
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
8 changes: 4 additions & 4 deletions crates/autopilot/src/database/competition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ use {

#[derive(Clone, Debug)]
pub enum ExecutedFee {
/// Fee is calculated by the solver and known upfront (before the settlement
/// is finalized).
Solver(U256),
/// Unsubsidized fee (full fee amount) that is taken from the signed order
/// and known upfront (before the settlement is finalized).
Order(U256),
/// Fee is unknown before the settlement is finalized and is calculated in
/// the postprocessing. Currently only used for limit orders.
Surplus,
Expand All @@ -28,7 +28,7 @@ pub enum ExecutedFee {
impl ExecutedFee {
pub fn fee(&self) -> Option<&U256> {
match self {
ExecutedFee::Solver(fee) => Some(fee),
ExecutedFee::Order(fee) => Some(fee),
ExecutedFee::Surplus => None,
}
}
Expand Down
32 changes: 23 additions & 9 deletions crates/autopilot/src/database/orders.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use {
super::Postgres,
crate::decoded_settlement::OrderExecution,
anyhow::Result,
anyhow::{Context, Result},
database::byte_array::ByteArray,
ethcontract::H256,
futures::{StreamExt, TryStreamExt},
futures::{TryFutureExt, TryStreamExt},
model::auction::AuctionId,
shared::db_order_conversions::full_order_into_model_order,
sqlx::PgConnection,
};

Expand All @@ -20,12 +21,25 @@ impl Postgres {
.with_label_values(&["orders_for_tx"])
.start_timer();

database::orders::order_executions_in_tx(ex, &ByteArray(tx_hash.0), auction_id)
.map(|result| match result {
Ok(execution) => execution.try_into().map_err(Into::into),
Err(err) => Err(anyhow::Error::from(err)),
})
.try_collect()
.await
let mut order_executions = Vec::new();

let executions =
sunce86 marked this conversation as resolved.
Show resolved Hide resolved
database::orders::order_executions_in_tx(ex, &ByteArray(tx_hash.0), auction_id)
.try_collect::<Vec<_>>()
.map_err(anyhow::Error::from)
.await?;

if let Some(execution) = executions.first() {
let order = database::orders::single_full_order(ex, &execution.order_uid)
.await?
.map(full_order_into_model_order)
.context("order not found")??;
sunce86 marked this conversation as resolved.
Show resolved Hide resolved

for execution in executions {
order_executions.push(OrderExecution::new(&order, execution)?);
}
}

Ok(order_executions)
}
}
140 changes: 42 additions & 98 deletions crates/autopilot/src/decoded_settlement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,19 @@
//! GPv2Settlement::settle function.

use {
crate::database::competition::ExecutedFee,
anyhow::{Context, Result},
bigdecimal::{Signed, Zero},
contracts::GPv2Settlement,
database::orders::OrderClass,
ethcontract::{common::FunctionExt, tokens::Tokenize, Address, Bytes, H160, U256},
model::{
app_data::AppDataHash,
order::{BuyTokenDestination, OrderData, OrderKind, OrderUid, SellTokenSource},
signature::Signature,
order::{BuyTokenDestination, Order, OrderData, OrderKind, OrderUid, SellTokenSource},
DomainSeparator,
},
num::BigRational,
number::conversions::{big_decimal_to_u256, big_rational_to_u256, u256_to_big_rational},
shared::{
conversions::U256Ext,
db_order_conversions::signing_scheme_from,
external_prices::ExternalPrices,
},
shared::{conversions::U256Ext, external_prices::ExternalPrices},
web3::ethabi::{Function, Token},
};

Expand Down Expand Up @@ -179,41 +174,23 @@ impl From<(Address, U256, Bytes<Vec<u8>>)> for DecodedInteraction {
#[derive(Debug, Clone)]
pub struct OrderExecution {
pub order_uid: OrderUid,
pub executed_solver_fee: Option<U256>,
pub sell_token: H160,
pub buy_token: H160,
pub sell_amount: U256,
pub buy_amount: U256,
pub executed_amount: U256,
pub signature: Vec<u8>, //encoded signature
// For limit orders the solver computes the fee
pub solver_determines_fee: bool,
pub owner: H160,
pub executed_amount: U256,
pub executed_fee: ExecutedFee,
}

impl TryFrom<database::orders::OrderExecution> for OrderExecution {
type Error = anyhow::Error;

fn try_from(order: database::orders::OrderExecution) -> std::result::Result<Self, Self::Error> {
let owner = H160(order.owner.0);
impl OrderExecution {
pub fn new(order: &Order, execution: database::orders::OrderExecution) -> Result<Self> {
sunce86 marked this conversation as resolved.
Show resolved Hide resolved
let owner = H160(execution.owner.0);
Ok(Self {
order_uid: OrderUid(order.order_uid.0),
executed_solver_fee: order
.executed_solver_fee
.as_ref()
.and_then(big_decimal_to_u256),
sell_token: H160(order.sell_token.0),
buy_token: H160(order.buy_token.0),
sell_amount: big_decimal_to_u256(&order.sell_amount).context("sell_amount")?,
buy_amount: big_decimal_to_u256(&order.buy_amount).context("buy_amount")?,
executed_amount: big_decimal_to_u256(&order.executed_amount).unwrap(),
signature: {
let signing_scheme = signing_scheme_from(order.signing_scheme);
let signature = Signature::from_bytes(signing_scheme, &order.signature)?;
signature.encode_for_settlement(owner).to_vec()
},
solver_determines_fee: order.class == OrderClass::Limit,
order_uid: OrderUid(execution.order_uid.0),
owner,
executed_amount: big_decimal_to_u256(&execution.executed_amount).unwrap(),
executed_fee: if order.metadata.solver_fee == U256::zero() {
sunce86 marked this conversation as resolved.
Show resolved Hide resolved
ExecutedFee::Surplus
} else {
ExecutedFee::Order(order.metadata.solver_fee)
},
})
}
}
Expand Down Expand Up @@ -359,8 +336,8 @@ impl DecodedSettlement {
// use every `OrderExecution` exactly once.
let order = orders.swap_remove(i);

// Update fee only for orders with solver computed fees (limit orders)
if !order.solver_determines_fee {
// Update fee only for orders where fee is taken from surplus
if !matches!(order.executed_fee, ExecutedFee::Surplus) {
return None;
}

Expand All @@ -380,27 +357,24 @@ impl DecodedSettlement {
order: &OrderExecution,
trade: &DecodedTrade,
) -> Option<Fees> {
let solver_fee = match &order.executed_solver_fee {
Some(solver_fee) => *solver_fee,
None => {
// get uniform prices
let sell_index = self
.tokens
.iter()
.position(|token| token == &order.sell_token)?;
let buy_index = self
.tokens
.iter()
.position(|token| token == &order.buy_token)?;
let uniform_sell_price = self.clearing_prices.get(sell_index).cloned()?;
let uniform_buy_price = self.clearing_prices.get(buy_index).cloned()?;

let sell_index = trade.sell_token_index.as_u64() as usize;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: buy_token and buy_token_index are only needed in the ExecutedFee::Surplus case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Together with the comment 👇 , I tried to find more readable code but failed. Note that most of the orders will go through the ExecutedFee::Surplus code path once swap with zero fees feature is enabled. Feel free to reorganize the code in another way in a separate PR, since I failed to make any improvements (or even in this one).

let buy_index = trade.buy_token_index.as_u64() as usize;
let sell_token = self.tokens.get(sell_index)?;
let buy_token = self.tokens.get(buy_index)?;

let solver_fee = match &order.executed_fee {
ExecutedFee::Order(fee) => *fee,
ExecutedFee::Surplus => {
// get executed(adjusted) prices
let sell_index = trade.sell_token_index.as_u64() as usize;
let buy_index = trade.buy_token_index.as_u64() as usize;
let adjusted_sell_price = self.clearing_prices.get(sell_index).cloned()?;
let adjusted_buy_price = self.clearing_prices.get(buy_index).cloned()?;

// get uniform prices
let sell_index = self.tokens.iter().position(|token| token == sell_token)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I found it pretty confusing that (sell|buy)_index was used once to refer to the adjusted price and once for the UCP.
It would help to either prefix them maybe ucp_sell_index or limit the scope where you change the meaning like:

                // get uniform prices
                let uniform_sell_price = {
                    let sell_index = self.tokens.iter().position(|token| token == sell_token)?;
                    self.clearing_prices.get(sell_index).cloned()?
                };
                let uniform_buy_price = {
                    let buy_index = self.tokens.iter().position(|token| token == buy_token)?;
                    self.clearing_prices.get(buy_index).cloned()?
                };

let buy_index = self.tokens.iter().position(|token| token == buy_token)?;
let uniform_sell_price = self.clearing_prices.get(sell_index).cloned()?;
let uniform_buy_price = self.clearing_prices.get(buy_index).cloned()?;

// the logic is opposite to the code in function `custom_price_for_limit_order`
match trade.flags.order_kind() {
OrderKind::Buy => {
Expand Down Expand Up @@ -434,7 +408,7 @@ impl DecodedSettlement {
// native token.
tracing::trace!(?solver_fee, "fee before conversion to native token");
let fee = external_prices
.try_get_native_amount(order.sell_token, u256_to_big_rational(&solver_fee))?;
.try_get_native_amount(*sell_token, u256_to_big_rational(&solver_fee))?;
tracing::trace!(?fee, "fee after conversion to native token");

Some(Fees {
Expand Down Expand Up @@ -781,27 +755,15 @@ mod tests {
let orders = vec![
OrderExecution {
order_uid: OrderUid::from_str("0xa8b0c9be7320d1314c6412e6557efd062bb9f97f2f4187f8b513f50ff63597cae995e2a9ae5210feb6dd07618af28ec38b2d7ce163f4d8c4").unwrap(),
executed_solver_fee: Some(48263037u128.into()),
buy_amount: 11446254517730382294118u128.into(),
sell_amount: 14955083027u128.into(),
sell_token: addr!("dac17f958d2ee523a2206206994597c13d831ec7"),
buy_token: Default::default(),
owner: addr!("E995E2A9Ae5210FEb6DD07618af28ec38B2D7ce1"),
executed_amount: 14955083027u128.into(),
signature: hex::decode("155ff208365bbf30585f5b18fc92d766e46121a1963f903bb6f3f77e5d0eaefb27abc4831ce1f837fcb70e11d4e4d97474c677469240849d69e17f7173aead841b").unwrap(),
solver_determines_fee: false,
owner: addr!("E995E2A9Ae5210FEb6DD07618af28ec38B2D7ce1")
executed_fee: ExecutedFee::Order(48263037u128.into())
},
OrderExecution {
order_uid: OrderUid::from_str("0x82582487739d1331572710a9283dc244c134d323f309eb0aac6c842ff5227e90f352bffb3e902d78166a79c9878e138a65022e1163f4d8bb").unwrap(),
executed_solver_fee: Some(127253135942751092736u128.into()),
buy_amount: 1236593080.into(),
sell_amount: 5701912712048588025933u128.into(),
sell_token: addr!("f4d2888d29d722226fafa5d9b24f9164c092421e"),
buy_token: Default::default(),
owner: addr!("f352bFFB3E902d78166a79C9878e138a65022e11"),
executed_amount: 5701912712048588025933u128.into(),
signature: hex::decode("882a1c875ff1316bb79bde0d0792869f784d58097d8489a722519e6417c577cf5cc745a2e353298dea6514036d5eb95563f8f7640e20ef0fd41b10ccbdfc87641b").unwrap(),
solver_determines_fee: false,
owner: addr!("f352bFFB3E902d78166a79C9878e138a65022e11")
executed_fee: ExecutedFee::Order(127253135942751092736u128.into())
}
];
let fees = settlement
Expand Down Expand Up @@ -898,15 +860,9 @@ mod tests {
let orders = vec![
OrderExecution {
order_uid: OrderUid::from_str("0xaa6ff3f3f755e804eefc023967be5d7f8267674d4bae053eaca01be5801854bf6c7f534c81dfedf90c9e42effb410a44e4f8ef1064690e05").unwrap(),
executed_solver_fee: None,
buy_amount: 11446254517730382294118u128.into(), // irrelevant
sell_amount: 14955083027u128.into(), // irrelevant
sell_token: addr!("ba386a4ca26b85fd057ab1ef86e3dc7bdeb5ce70"),
buy_token: addr!("c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2"),
owner: addr!("6c7f534c81dfedf90c9e42effb410a44e4f8ef10"),
executed_amount: 134069619089011499167823218927u128.into(),
signature: hex::decode("f8ad81db7333b891f88527d100a06f23ff4d7859c66ddd71514291379deb8ff660f4fb2a24173eaac5fad2a124823e968686e39467c7f3054c13c4b70980cc1a1c").unwrap(),
solver_determines_fee: true,
owner: addr!("6c7f534c81dfedf90c9e42effb410a44e4f8ef10")
executed_fee: ExecutedFee::Surplus
},
];
let fees = settlement
Expand Down Expand Up @@ -1013,15 +969,9 @@ mod tests {
let orders = vec![
OrderExecution {
order_uid: OrderUid::from_str("0x999d6ff17fb145220fd96c97493fd6013ecb7874dffc3b57837131a92a36dc02b70cd1ebd3b24aeeaf90c6041446630338536e7f643d6a39").unwrap(),
executed_solver_fee: Some(463182886014406361088u128.into()),
buy_amount: 89238894792574185u128.into(),
sell_amount: 3026871740084629982950u128.into(),
sell_token: addr!("f88baf18fab7e330fa0c4f83949e23f52fececce"),
buy_token: addr!("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"),
owner: addr!("b70cd1ebd3b24aeeaf90c6041446630338536e7f"),
executed_amount: 0.into(),
signature: hex::decode("4935ea3f24155f6757df94d8c0bc96665d46da51e1a8e39d935967c9216a60912fa50a5393a323d453c78d179d0199ddd58f6d787781e4584357d3e0205a76001c").unwrap(),
solver_determines_fee: false,
owner: addr!("b70cd1ebd3b24aeeaf90c6041446630338536e7f")
executed_fee: ExecutedFee::Order(463182886014406361088u128.into())
},
];
let fees = settlement
Expand Down Expand Up @@ -1392,15 +1342,9 @@ mod tests {

let orders = vec![OrderExecution {
order_uid: OrderUid::from_str("0x77425bd23d5fbb24d32229b1c343807bee572f0555429632161350a56811d263c001d00d425fa92c4f840baa8f1e0c27c4297a0b65782608").unwrap(),
executed_solver_fee: None,
buy_amount: 8854486397647516107245u128.into(),
sell_amount: 1558319022273364070254u128.into(),
sell_token: addr!("fbeb78a723b8087fd2ea7ef1afec93d35e8bed42"),
buy_token: addr!("da816459f1ab5631232fe5e97a05bbbb94970c95"),
executed_amount: 1558319022273364070254u128.into(),
signature: hex::decode("c001d00d425fa92c4f840baa8f1e0c27c4297a0b").unwrap(),
solver_determines_fee: true,
owner: addr!("c001d00d425fa92c4f840baa8f1e0c27c4297a0b"),
executed_amount: 1558319022273364070254u128.into(),
executed_fee: ExecutedFee::Surplus
}];

let fees = decoded.order_executions(&external_prices, orders);
Expand Down
2 changes: 1 addition & 1 deletion crates/autopilot/src/run_loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl RunLoop {
// we don't know the surplus fee in advance. will be populated
// after the transaction containing the order is mined
true => ExecutedFee::Surplus,
false => ExecutedFee::Solver(auction_order.metadata.solver_fee),
false => ExecutedFee::Order(auction_order.metadata.solver_fee),
};
order_executions.push(OrderExecution {
order_id: *order_id,
Expand Down
8 changes: 0 additions & 8 deletions crates/database/src/orders.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,6 @@ pub struct FullOrder {
pub onchain_user: Option<Address>,
pub onchain_placement_error: Option<OnchainOrderPlacementError>,
pub executed_surplus_fee: BigDecimal,
pub executed_solver_fee: BigDecimal,
pub full_app_data: Option<Vec<u8>>,
}

Expand Down Expand Up @@ -520,7 +519,6 @@ array(Select (p.target, p.value, p.data) from interactions p where p.order_uid =
(SELECT onchain_o.sender from onchain_placed_orders onchain_o where onchain_o.uid = o.uid limit 1) as onchain_user,
(SELECT onchain_o.placement_error from onchain_placed_orders onchain_o where onchain_o.uid = o.uid limit 1) as onchain_placement_error,
COALESCE((SELECT SUM(surplus_fee) FROM order_execution oe WHERE oe.order_uid = o.uid), 0) as executed_surplus_fee,
COALESCE((SELECT SUM(solver_fee) FROM order_execution oe WHERE oe.order_uid = o.uid), 0) as executed_solver_fee,
(SELECT full_app_data FROM app_data ad WHERE o.app_data = ad.contract_app_data LIMIT 1) as full_app_data
"#;

Expand Down Expand Up @@ -586,8 +584,6 @@ WHERE
#[derive(Debug, Clone, Default, PartialEq, sqlx::FromRow)]
pub struct OrderExecution {
pub order_uid: OrderUid,
/// The `solver_fee` that got executed for this specific fill.
pub executed_solver_fee: Option<BigDecimal>,
pub sell_token: Address,
pub buy_token: Address,
pub kind: OrderKind,
Expand All @@ -613,7 +609,6 @@ pub fn order_executions_in_tx<'a>(
{SETTLEMENT_LOG_INDICES}
SELECT
oe.order_uid AS order_uid,
oe.solver_fee AS executed_solver_fee,
o.sell_token,
o.buy_token,
o.sell_amount,
Expand Down Expand Up @@ -1835,7 +1830,6 @@ mod tests {
.unwrap()
.unwrap();
assert_eq!(order.executed_surplus_fee, fee);
assert_eq!(order.executed_solver_fee, solver_fee);
}

#[tokio::test]
Expand Down Expand Up @@ -1924,7 +1918,6 @@ mod tests {
executions,
vec![OrderExecution {
order_uid,
executed_solver_fee: Some(bigdecimal(463182886014406361088)),
sell_token: ByteArray(hex!("f88baf18fab7e330fa0c4f83949e23f52fececce")),
buy_token: ByteArray(hex!("eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee")),
kind: OrderKind::Sell,
Expand Down Expand Up @@ -2003,7 +1996,6 @@ mod tests {
assert_eq!(
executions,
vec![OrderExecution {
executed_solver_fee: Some(bigdecimal(42)),
kind: OrderKind::Sell,
class: OrderClass::Limit,
sell_amount: bigdecimal(1),
Expand Down
1 change: 0 additions & 1 deletion crates/orderbook/src/database/orders.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,6 @@ mod tests {
onchain_user: None,
onchain_placement_error: None,
executed_surplus_fee: Default::default(),
executed_solver_fee: Default::default(),
full_app_data: Default::default(),
};

Expand Down
Loading