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.

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

PriceImprovement fee policy #2277

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
c1a60c0
PriceImprovement policy in autopilot
squadgazzz Jan 12, 2024
d68cfbd
PriceImprovement policy in driver
squadgazzz Jan 12, 2024
ce00a5b
todo
squadgazzz Jan 12, 2024
e901812
Update crates/autopilot/src/arguments.rs
squadgazzz Jan 15, 2024
5406807
Update crates/driver/openapi.yml
squadgazzz Jan 15, 2024
ab00441
Minor review fixes
squadgazzz Jan 15, 2024
ebf8dd9
Formatting
squadgazzz Jan 15, 2024
4a0e5ff
Implement price improvement fee calculation
squadgazzz Jan 15, 2024
5a153aa
Merge branch 'main' into price-improvement-policy
squadgazzz Jan 16, 2024
0cbbb00
DB persistence
squadgazzz Jan 16, 2024
6bc3de6
Orderbook OpenAPI
squadgazzz Jan 16, 2024
5d4e32d
Missing files
squadgazzz Jan 16, 2024
b4e4b5a
Test fix
squadgazzz Jan 16, 2024
69a2297
Warn about skipped order
squadgazzz Jan 17, 2024
a1444b1
PolicyRaw dto
squadgazzz Jan 17, 2024
7f1a7c5
Make quote mandatory
squadgazzz Jan 23, 2024
50520ae
Naming and docs
squadgazzz Jan 23, 2024
2376f79
Merge branch 'main' into price-improvement-policy
squadgazzz Jan 23, 2024
eb08cf4
PolicyRaw -> PolicyBuilder
squadgazzz Jan 24, 2024
f629e61
Fee calculation fix
squadgazzz Jan 24, 2024
e3a27a0
Construct domain::Order inside the ProtocolFee
squadgazzz Jan 24, 2024
ed9ff89
Merge branch 'main' into price-improvement-policy
Jan 29, 2024
b48ecdc
Imports
squadgazzz Jan 29, 2024
8fd95e4
Merge branch 'main' into price-improvement-policy
squadgazzz Feb 6, 2024
c582f74
Fix after merge
squadgazzz Feb 6, 2024
8ca5b05
Update migration version
squadgazzz Feb 6, 2024
ba9a399
e2e
squadgazzz Feb 6, 2024
603eb8b
Docs
squadgazzz Feb 6, 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
22 changes: 19 additions & 3 deletions crates/autopilot/src/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,12 @@ pub struct FeePolicy {
/// - Surplus with cap:
/// surplus:0.5:0.06
///
/// - Price improvement without cap:
/// price_improvement:0.5:1.0
///
/// - Price improvement with cap:
/// price_improvement:0.5:0.06
///
/// - Volume based:
/// volume:0.1
#[clap(long, env, default_value = "surplus:0.0:1.0")]
Expand All @@ -362,16 +368,23 @@ pub struct FeePolicy {
}

impl FeePolicy {
pub fn to_domain(self) -> domain::fee::Policy {
pub fn to_domain_raw(&self) -> domain::fee::PolicyRaw {
match self.fee_policy_kind {
FeePolicyKind::Surplus {
factor,
max_volume_factor,
} => domain::fee::Policy::Surplus {
} => domain::fee::PolicyRaw::Surplus {
factor,
max_volume_factor,
},
FeePolicyKind::PriceImprovement {
factor,
max_volume_factor,
} => domain::fee::PolicyRaw::PriceImprovement {
factor,
max_volume_factor,
},
FeePolicyKind::Volume { factor } => domain::fee::Policy::Volume { factor },
FeePolicyKind::Volume { factor } => domain::fee::PolicyRaw::Volume { factor },
}
}
}
Expand All @@ -380,6 +393,9 @@ impl FeePolicy {
pub enum FeePolicyKind {
/// How much of the order's surplus should be taken as a protocol fee.
Surplus { factor: f64, max_volume_factor: f64 },
/// How much of the order's price improvement should be taken as a protocol
/// fee.
PriceImprovement { factor: f64, max_volume_factor: f64 },
/// How much of the order's volume should be taken as a protocol fee.
Volume { factor: f64 },
}
Expand Down
31 changes: 27 additions & 4 deletions crates/autopilot/src/database/fee_policies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub async fn insert_batch(
) -> Result<(), sqlx::Error> {
let mut query_builder = QueryBuilder::new(
"INSERT INTO fee_policies (auction_id, order_uid, kind, surplus_factor, \
max_volume_factor, volume_factor) ",
max_volume_factor, volume_factor, quote_sell_amount, quote_buy_amount) ",
);

query_builder.push_values(fee_policies, |mut b, fee_policy| {
Expand All @@ -18,7 +18,9 @@ pub async fn insert_batch(
.push_bind(fee_policy.kind)
.push_bind(fee_policy.surplus_factor)
.push_bind(fee_policy.max_volume_factor)
.push_bind(fee_policy.volume_factor);
.push_bind(fee_policy.volume_factor)
.push_bind(fee_policy.quote_sell_amount)
.push_bind(fee_policy.quote_buy_amount);
});

query_builder.build().execute(ex).await.map(|_| ())
Expand Down Expand Up @@ -46,7 +48,7 @@ pub async fn fetch(

#[cfg(test)]
mod tests {
use {super::*, database::byte_array::ByteArray, sqlx::Connection};
use {super::*, bigdecimal::BigDecimal, database::byte_array::ByteArray, sqlx::Connection};

#[tokio::test]
#[ignore]
Expand All @@ -66,6 +68,8 @@ mod tests {
surplus_factor: Some(0.1),
max_volume_factor: Some(1.0),
volume_factor: None,
quote_sell_amount: None,
quote_buy_amount: None,
};
// surplus fee policy with caps
let fee_policy_2 = dto::FeePolicy {
Expand All @@ -75,6 +79,8 @@ mod tests {
surplus_factor: Some(0.2),
max_volume_factor: Some(0.05),
volume_factor: None,
quote_sell_amount: None,
quote_buy_amount: None,
};
// volume based fee policy
let fee_policy_3 = dto::FeePolicy {
Expand All @@ -84,19 +90,36 @@ mod tests {
surplus_factor: None,
max_volume_factor: None,
volume_factor: Some(0.06),
quote_sell_amount: None,
quote_buy_amount: None,
};
// price improvement fee policy
let fee_policy_4 = dto::FeePolicy {
auction_id,
order_uid,
kind: dto::fee_policy::FeePolicyKind::PriceImprovement,
surplus_factor: Some(0.3),
max_volume_factor: Some(0.07),
volume_factor: None,
quote_sell_amount: Some(BigDecimal::new(100.into(), 1)),
quote_buy_amount: Some(BigDecimal::new(200.into(), 1)),
};
insert_batch(
&mut db,
vec![
fee_policy_1.clone(),
fee_policy_2.clone(),
fee_policy_3.clone(),
fee_policy_4.clone(),
],
)
.await
.unwrap();

let output = fetch(&mut db, 1, order_uid).await.unwrap();
assert_eq!(output, vec![fee_policy_1, fee_policy_2, fee_policy_3]);
assert_eq!(
output,
vec![fee_policy_1, fee_policy_2, fee_policy_3, fee_policy_4]
);
}
}
80 changes: 65 additions & 15 deletions crates/autopilot/src/domain/fee/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,48 +4,45 @@
//! we define the way to calculate the protocol fee based on the configuration
//! parameters.

use crate::{
boundary::{self},
domain,
use {
crate::{
boundary::{self},
domain,
},
primitive_types::U256,
};

/// Constructs fee policies based on the current configuration.
#[derive(Debug)]
pub struct ProtocolFee {
policy: Policy,
policy: PolicyRaw,
fee_policy_skip_market_orders: bool,
}

impl ProtocolFee {
pub fn new(policy: Policy, fee_policy_skip_market_orders: bool) -> Self {
pub fn new(policy: PolicyRaw, fee_policy_skip_market_orders: bool) -> Self {
Self {
policy,
fee_policy_skip_market_orders,
}
}

/// Get policies for order.
pub fn get(&self, order: &boundary::Order, quote: Option<&domain::Quote>) -> Vec<Policy> {
pub fn get(&self, order: &boundary::Order, quote: &domain::Quote) -> Vec<Policy> {
match order.metadata.class {
boundary::OrderClass::Market => {
if self.fee_policy_skip_market_orders {
vec![]
} else {
vec![self.policy]
vec![self.policy.to_domain(quote)]
}
}
boundary::OrderClass::Liquidity => vec![],
boundary::OrderClass::Limit => {
if !self.fee_policy_skip_market_orders {
return vec![self.policy];
return vec![self.policy.to_domain(quote)];
}

// if the quote is missing, we can't determine if the order is outside the
// market price so we protect the user and not charge a fee
let Some(quote) = quote else {
return vec![];
};

tracing::debug!(?order.metadata.uid, ?self.policy, ?order.data.sell_amount, ?order.data.buy_amount, ?quote, "checking if order is outside market price");
if boundary::is_order_outside_market_price(
&order.data.sell_amount,
Expand All @@ -55,7 +52,7 @@ impl ProtocolFee {
&quote.sell_amount,
&quote.fee,
) {
vec![self.policy]
vec![self.policy.to_domain(quote)]
} else {
vec![]
}
Expand All @@ -81,6 +78,14 @@ pub enum Policy {
/// Cap protocol fee with a percentage of the order's volume.
max_volume_factor: f64,
},
/// A price improvement corresponds to a situation where the order is
/// executed at a better price than the top quote. The protocol fee in such
/// case is calculated from a cut of this price improvement.
PriceImprovement {
factor: f64,
max_volume_factor: f64,
quote: Quote,
},
squadgazzz marked this conversation as resolved.
Show resolved Hide resolved
/// How much of the order's volume should be taken as a protocol fee.
/// The fee is taken in `sell` token for `sell` orders and in `buy`
/// token for `buy` orders.
Expand All @@ -90,3 +95,48 @@ pub enum Policy {
factor: f64,
},
}

#[derive(Debug)]
pub enum PolicyRaw {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit surprised by this type. I don't find the word "raw" very descriptive nor appealing and it's missing documentation for why it exists (on top of FeePolicy). It looks like the only difference is the quote (which in only part of the other struct).

Is there any way we can get around defining two almost identical domain types or can we differentiate better between the two?

Maybe PolicyRaw is more like an incomplete Policy (maybe a PolicyBuilder), that can be converted into a full policy .with(quote)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, totally makes sense. Renamed both the struct and the function.

Surplus { factor: f64, max_volume_factor: f64 },
PriceImprovement { factor: f64, max_volume_factor: f64 },
Volume { factor: f64 },
}

impl PolicyRaw {
pub fn to_domain(&self, quote: &domain::Quote) -> Policy {
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't PolicyRaw already a domain object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

match self {
PolicyRaw::Surplus {
factor,
max_volume_factor,
} => Policy::Surplus {
factor: *factor,
max_volume_factor: *max_volume_factor,
},
PolicyRaw::PriceImprovement {
factor,
max_volume_factor,
} => Policy::PriceImprovement {
factor: *factor,
max_volume_factor: *max_volume_factor,
quote: quote.clone().into(),
},
PolicyRaw::Volume { factor } => Policy::Volume { factor: *factor },
}
}
}

#[derive(Debug, Copy, Clone, PartialEq)]
pub struct Quote {
pub sell_amount: U256,
pub buy_amount: U256,
}

impl From<domain::Quote> for Quote {
fn from(value: domain::Quote) -> Self {
Self {
sell_amount: value.sell_amount,
buy_amount: value.buy_amount,
}
}
}
41 changes: 40 additions & 1 deletion crates/autopilot/src/infra/persistence/dto/fee_policy.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use crate::{boundary, domain};
use {
crate::{boundary, domain},
bigdecimal::BigDecimal,
number::conversions::{big_decimal_to_u256, u256_to_big_decimal},
};

#[derive(Debug, Clone, PartialEq, sqlx::FromRow)]
pub struct FeePolicy {
Expand All @@ -8,6 +12,8 @@ pub struct FeePolicy {
pub surplus_factor: Option<f64>,
pub max_volume_factor: Option<f64>,
pub volume_factor: Option<f64>,
pub quote_sell_amount: Option<BigDecimal>,
pub quote_buy_amount: Option<BigDecimal>,
}

impl FeePolicy {
Expand All @@ -27,6 +33,8 @@ impl FeePolicy {
surplus_factor: Some(factor),
max_volume_factor: Some(max_volume_factor),
volume_factor: None,
quote_sell_amount: None,
quote_buy_amount: None,
},
domain::fee::Policy::Volume { factor } => Self {
auction_id,
Expand All @@ -35,6 +43,22 @@ impl FeePolicy {
surplus_factor: None,
max_volume_factor: None,
volume_factor: Some(factor),
quote_sell_amount: None,
quote_buy_amount: None,
},
domain::fee::Policy::PriceImprovement {
factor,
max_volume_factor,
quote,
} => Self {
auction_id,
order_uid: boundary::database::byte_array::ByteArray(order_uid.0),
kind: FeePolicyKind::Surplus,
surplus_factor: Some(factor),
max_volume_factor: Some(max_volume_factor),
volume_factor: None,
quote_sell_amount: Some(u256_to_big_decimal(&quote.sell_amount)),
quote_buy_amount: Some(u256_to_big_decimal(&quote.buy_amount)),
},
}
}
Expand All @@ -50,6 +74,20 @@ impl From<FeePolicy> for domain::fee::Policy {
FeePolicyKind::Volume => domain::fee::Policy::Volume {
factor: row.volume_factor.expect("missing volume factor"),
},
FeePolicyKind::PriceImprovement => domain::fee::Policy::PriceImprovement {
factor: row.surplus_factor.expect("missing surplus factor"),
max_volume_factor: row.max_volume_factor.expect("missing max volume factor"),
quote: domain::fee::Quote {
sell_amount: big_decimal_to_u256(
&row.quote_sell_amount.expect("missing sell amount"),
)
.expect("sell amount is not a valid eth::U256"),
buy_amount: big_decimal_to_u256(
&row.quote_buy_amount.expect("missing buy amount"),
)
.expect("buy amount is not a valid eth::U256"),
},
},
}
}
}
Expand All @@ -59,4 +97,5 @@ impl From<FeePolicy> for domain::fee::Policy {
pub enum FeePolicyKind {
Surplus,
Volume,
PriceImprovement,
}
Loading
Loading