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

Price improvemenet fee policy autopilot domain #2378

Merged
merged 16 commits into from
Feb 12, 2024
38 changes: 35 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_policy_builder(&self) -> domain::fee::PolicyBuilder {
match self.fee_policy_kind {
FeePolicyKind::Surplus {
factor,
max_volume_factor,
} => domain::fee::Policy::Surplus {
} => domain::fee::PolicyBuilder::Surplus {
factor,
max_volume_factor,
},
FeePolicyKind::Volume { factor } => domain::fee::Policy::Volume { factor },
FeePolicyKind::PriceImprovement {
factor,
max_volume_factor,
} => domain::fee::PolicyBuilder::PriceImprovement {
factor,
max_volume_factor,
},
FeePolicyKind::Volume { factor } => domain::fee::PolicyBuilder::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 Expand Up @@ -407,6 +423,22 @@ impl FromStr for FeePolicyKind {
max_volume_factor,
})
}
"price-improvement" => {
let factor = parts
.next()
.ok_or("missing price-improvement factor")?
.parse::<f64>()
.map_err(|e| format!("invalid price-improvement factor: {}", e))?;
let max_volume_factor = parts
.next()
.ok_or("missing price-improvement max volume factor")?
.parse::<f64>()
.map_err(|e| format!("invalid price-improvement max volume factor: {}", e))?;
Ok(Self::PriceImprovement {
factor,
max_volume_factor,
})
}
"volume" => {
let factor = parts
.next()
Expand Down
118 changes: 85 additions & 33 deletions crates/autopilot/src/domain/fee/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,65 +4,64 @@
//! 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_builder: PolicyBuilder,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a private type now, if we move it to a policy submodule we could even call it Policy without having a name collision (the way it turned out now it's not really following the Builder pattern anymore)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved

fee_policy_skip_market_orders: bool,
}

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

/// Get policies for order.
pub fn get(&self, order: &boundary::Order, quote: Option<&domain::Quote>) -> Vec<Policy> {
match order.metadata.class {
/// Converts an order from the boundary layer to the domain layer, applying
/// protocol fees if necessary.
pub fn to_order(&self, order: boundary::Order, quote: &domain::Quote) -> domain::Order {
squadgazzz marked this conversation as resolved.
Show resolved Hide resolved
let protocol_fees = match order.metadata.class {
boundary::OrderClass::Market => {
if self.fee_policy_skip_market_orders {
vec![]
} else {
vec![self.policy]
vec![self.policy_builder.build_with(quote)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the information whether or not to apply a fee for market order be part of the fee policy (builder) and not part of this component?

}
}
boundary::OrderClass::Liquidity => vec![],
boundary::OrderClass::Limit => {
if !self.fee_policy_skip_market_orders {
return vec![self.policy];
}

// 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![];
};

let order_ = boundary::Amounts {
sell: order.data.sell_amount,
buy: order.data.buy_amount,
fee: order.data.fee_amount,
};
let quote = boundary::Amounts {
sell: quote.sell_amount,
buy: quote.buy_amount,
fee: quote.fee,
};
if boundary::is_order_outside_market_price(&order_, &quote) {
vec![self.policy]
vec![self.policy_builder.build_with(quote)]
} else {
vec![]
let order_ = boundary::Amounts {
sell: order.data.sell_amount,
buy: order.data.buy_amount,
fee: order.data.fee_amount,
};
let quote_ = boundary::Amounts {
sell: quote.sell_amount,
buy: quote.buy_amount,
fee: quote.fee,
};
if boundary::is_order_outside_market_price(&order_, &quote_) {
vec![self.policy_builder.build_with(quote)]
} else {
vec![]
}
}
}
}
};
boundary::order::to_domain(order, protocol_fees)
}
}

Expand All @@ -83,6 +82,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,
},
/// 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 @@ -92,3 +99,48 @@ pub enum Policy {
factor: f64,
},
}

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

Choose a reason for hiding this comment

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

I think we should make this class private and create it in the main Fee component from the infra config. It might also be nicer to have every variant be there own sub-struct so that we can implement slightly different build behaviors (e.g. surplus and volume don't need the quote).

Then the main apply function would become something like

self.fee_variants.filter_map(|variant| {
  match variant {
    FeeVariant::Surplus(variant) => variant.apply(order),
    FeeVariant::PriceImprovement(variant) => variant.apply(order, 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.

Refactored

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

impl PolicyBuilder {
pub fn build_with(&self, quote: &domain::Quote) -> Policy {
match self {
PolicyBuilder::Surplus {
factor,
max_volume_factor,
} => Policy::Surplus {
factor: *factor,
max_volume_factor: *max_volume_factor,
},
PolicyBuilder::PriceImprovement {
factor,
max_volume_factor,
} => Policy::PriceImprovement {
factor: *factor,
max_volume_factor: *max_volume_factor,
quote: quote.clone().into(),
},
PolicyBuilder::Volume { factor } => Policy::Volume { factor: *factor },
}
}
}

#[derive(Debug, Copy, Clone, PartialEq)]
pub struct Quote {
pub sell_amount: U256,
pub buy_amount: U256,
sunce86 marked this conversation as resolved.
Show resolved Hide resolved
}

impl From<domain::Quote> for Quote {
fn from(value: domain::Quote) -> Self {
Self {
sell_amount: value.sell_amount,
buy_amount: value.buy_amount,
}
}
}
14 changes: 14 additions & 0 deletions crates/autopilot/src/infra/persistence/dto/fee_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ impl FeePolicy {
max_volume_factor: None,
volume_factor: Some(factor),
},
domain::fee::Policy::PriceImprovement {
factor,
max_volume_factor,
..
} => Self {
auction_id,
order_uid: boundary::database::byte_array::ByteArray(order_uid.0),
kind: FeePolicyKind::Surplus,
squadgazzz marked this conversation as resolved.
Show resolved Hide resolved
surplus_factor: Some(factor),
max_volume_factor: Some(max_volume_factor),
volume_factor: None,
},
}
}
}
Expand All @@ -50,6 +62,7 @@ impl From<FeePolicy> for domain::fee::Policy {
FeePolicyKind::Volume => domain::fee::Policy::Volume {
factor: row.volume_factor.expect("missing volume factor"),
},
FeePolicyKind::PriceImprovement => todo!(),
}
}
}
Expand All @@ -59,4 +72,5 @@ impl From<FeePolicy> for domain::fee::Policy {
pub enum FeePolicyKind {
Surplus,
Volume,
PriceImprovement,
}
38 changes: 38 additions & 0 deletions crates/autopilot/src/infra/persistence/dto/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,23 @@ pub enum FeePolicy {
#[serde(rename_all = "camelCase")]
Surplus { factor: f64, max_volume_factor: f64 },
#[serde(rename_all = "camelCase")]
PriceImprovement {
factor: f64,
max_volume_factor: f64,
quote: Quote,
},
#[serde(rename_all = "camelCase")]
Volume { factor: f64 },
}

#[serde_as]
#[derive(Clone, Debug, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct Quote {
pub sell_amount: U256,
pub buy_amount: U256,
}

impl From<domain::fee::Policy> for FeePolicy {
fn from(policy: domain::fee::Policy) -> Self {
match policy {
Expand All @@ -281,6 +295,18 @@ impl From<domain::fee::Policy> for FeePolicy {
factor,
max_volume_factor,
},
domain::fee::Policy::PriceImprovement {
factor,
max_volume_factor,
quote,
} => Self::PriceImprovement {
factor,
max_volume_factor,
quote: Quote {
sell_amount: quote.sell_amount,
buy_amount: quote.buy_amount,
},
},
domain::fee::Policy::Volume { factor } => Self::Volume { factor },
}
}
Expand All @@ -296,6 +322,18 @@ impl From<FeePolicy> for domain::fee::Policy {
factor,
max_volume_factor,
},
FeePolicy::PriceImprovement {
factor,
max_volume_factor,
quote,
} => Self::PriceImprovement {
factor,
max_volume_factor,
quote: domain::fee::Quote {
sell_amount: quote.sell_amount,
buy_amount: quote.buy_amount,
},
},
FeePolicy::Volume { factor } => Self::Volume { factor },
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/autopilot/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ pub async fn run(args: Arguments) {
.try_into()
.expect("limit order price factor can't be converted to BigDecimal"),
domain::ProtocolFee::new(
args.fee_policy.clone().to_domain(),
args.fee_policy.clone().to_policy_builder(),
args.fee_policy.fee_policy_skip_market_orders,
),
);
Expand Down
13 changes: 8 additions & 5 deletions crates/autopilot/src/solvable_orders.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use {
crate::{boundary, domain, infra},
crate::{domain, infra},
anyhow::Result,
bigdecimal::BigDecimal,
database::order_events::OrderEventLabel,
Expand Down Expand Up @@ -236,10 +236,13 @@ impl SolvableOrdersCache {
latest_settlement_block: db_solvable_orders.latest_settlement_block,
orders: orders
.into_iter()
.map(|order| {
let quote = db_solvable_orders.quotes.get(&order.metadata.uid.into());
let protocol_fees = self.protocol_fee.get(&order, quote);
boundary::order::to_domain(order, protocol_fees)
.filter_map(|order| {
if let Some(quote) = db_solvable_orders.quotes.get(&order.metadata.uid.into()) {
Some(self.protocol_fee.to_order(order, quote))
} else {
tracing::warn!(order_uid = %order.metadata.uid, "order is skipped, quote is missing");
None
}
})
.collect(),
prices,
Expand Down
Loading