Skip to content
Open
53 changes: 39 additions & 14 deletions crates/autopilot/src/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use {
crate::{domain::fee::FeeFactor, infra},
alloy::primitives::Address,
anyhow::{Context, anyhow, ensure},
chrono::{DateTime, Utc},
clap::ValueEnum,
primitive_types::{H160, U256},
shared::{
Expand Down Expand Up @@ -194,13 +195,8 @@ pub struct Arguments {
pub solve_deadline: Duration,

/// Describes how the protocol fees should be calculated.
#[clap(long, env, use_value_delimiter = true)]
pub fee_policies: Vec<FeePolicy>,

/// Maximum partner fee allow. If the partner fee specified is greater than
/// this maximum, the partner fee will be capped
#[clap(long, env, default_value = "0.01")]
pub fee_policy_max_partner_fee: FeeFactor,
#[clap(flatten)]
pub fee_policies_config: FeePoliciesConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

nice refactoring!


/// Arguments for uploading information to S3.
#[clap(flatten)]
Expand Down Expand Up @@ -389,8 +385,7 @@ impl std::fmt::Display for Arguments {
submission_deadline,
shadow,
solve_deadline,
fee_policies,
fee_policy_max_partner_fee,
fee_policies_config,
order_events_cleanup_interval,
order_events_cleanup_threshold,
db_write_url,
Expand Down Expand Up @@ -448,11 +443,7 @@ impl std::fmt::Display for Arguments {
writeln!(f, "submission_deadline: {submission_deadline}")?;
display_option(f, "shadow", shadow)?;
writeln!(f, "solve_deadline: {solve_deadline:?}")?;
writeln!(f, "fee_policies: {fee_policies:?}")?;
writeln!(
f,
"fee_policy_max_partner_fee: {fee_policy_max_partner_fee:?}"
)?;
writeln!(f, "fee_policies_config: {fee_policies_config:?}")?;
writeln!(
f,
"order_events_cleanup_interval: {order_events_cleanup_interval:?}"
Expand Down Expand Up @@ -585,6 +576,22 @@ impl FromStr for Solver {
}
}

#[derive(clap::Parser, Debug, Clone)]
pub struct FeePoliciesConfig {
/// Describes how the protocol fees should be calculated.
#[clap(long, env, use_value_delimiter = true)]
pub fee_policies: Vec<FeePolicy>,

/// Maximum partner fee allowed. If the partner fee specified is greater
/// than this maximum, the partner fee will be capped
#[clap(long, env, default_value = "0.01")]
pub fee_policy_max_partner_fee: FeeFactor,

/// Volume fee policies that will become effective at a future timestamp.
#[clap(flatten)]
pub upcoming_fee_policies: UpcomingFeePolicies,
}

/// A fee policy to be used for orders base on it's class.
/// Examples:
/// - Surplus with a high enough cap for limit orders: surplus:0.5:0.9:limit
Expand All @@ -604,6 +611,24 @@ pub struct FeePolicy {
pub fee_policy_order_class: FeePolicyOrderClass,
}

/// Fee policies that will become effective at a future timestamp.
#[derive(clap::Parser, Debug, Clone)]
pub struct UpcomingFeePolicies {
#[clap(
id = "upcoming_fee_policies",
long = "upcoming-fee-policies",
env = "UPCOMING_FEE_POLICIES",
use_value_delimiter = true
)]
pub fee_policies: Vec<FeePolicy>,

#[clap(
long = "upcoming-fee-policies-timestamp",
env = "UPCOMING_FEE_POLICIES_TIMESTAMP"
)]
pub effective_from_timestamp: Option<DateTime<Utc>>,
}

#[derive(clap::Parser, Debug, Clone)]
pub enum FeePolicyKind {
/// How much of the order's surplus should be taken as a protocol fee.
Expand Down
48 changes: 40 additions & 8 deletions crates/autopilot/src/domain/fee/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use {
},
alloy::primitives::{Address, U256},
app_data::Validator,
chrono::{DateTime, Utc},
derive_more::Into,
ethrpc::alloy::conversions::{IntoAlloy, IntoLegacy},
primitive_types::H160,
Expand Down Expand Up @@ -53,25 +54,47 @@ impl From<arguments::FeePolicy> for ProtocolFee {
}
}

pub struct UpcomingProtocolFees {
fee_policies: Vec<ProtocolFee>,
effective_from_timestamp: DateTime<Utc>,
}
Comment on lines +57 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we handle the upcoming/general protocol fees uniformly, by having the "always present" fees just start at UNIX epoch? This would simplify the approach, although will add timestamp checking to all fees in general. Let's discuss :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't get your comment. Could you rephrase?

Copy link
Contributor

Choose a reason for hiding this comment

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

The ProtocolFees struct:

pub struct ProtocolFees {
    fee_policies: Vec<ProtocolFee>,
    max_partner_fee: FeeFactor,
    upcoming_fee_policies: Option<UpcomingProtocolFees>,
}

contains fee_policies that are always applied unless superseded by upcoming_fee_policies which are essentially the same, but with an activation timestamp.

I was suggesting to unify these two, and instead support the generally applicable and late activated protocol fees in the same way. This would require the fee_policies to be a Vec of UpcomingProtocolFees.

But thinking about it the second time, I think it would unnecessarily complicate the solution as currently we support only 1 instance of upcoming protocol fees, whereas the generalization would have to support an arbitrary number of upcoming protocol fees. Then the evaluation would require making sure We apply the correct protocol fee based on the current time.

I am happy to approve the PR as-is.


impl From<arguments::UpcomingFeePolicies> for Option<UpcomingProtocolFees> {
fn from(value: arguments::UpcomingFeePolicies) -> Self {
value
// both config fields must be non-empty
.effective_from_timestamp
.filter(|_| !value.fee_policies.is_empty())
.map(|effective_from_timestamp| UpcomingProtocolFees {
fee_policies: value
.fee_policies
.into_iter()
.map(ProtocolFee::from)
.collect::<Vec<_>>(),
effective_from_timestamp,
})
}
}

pub type ProtocolFeeExemptAddresses = HashSet<H160>;

pub struct ProtocolFees {
fee_policies: Vec<ProtocolFee>,
max_partner_fee: FeeFactor,
upcoming_fee_policies: Option<UpcomingProtocolFees>,
}

impl ProtocolFees {
pub fn new(
fee_policies: &[arguments::FeePolicy],
fee_policy_max_partner_fee: FeeFactor,
) -> Self {
pub fn new(config: &arguments::FeePoliciesConfig) -> Self {
Self {
fee_policies: fee_policies
fee_policies: config
.fee_policies
.iter()
.cloned()
.map(ProtocolFee::from)
.collect(),
max_partner_fee: fee_policy_max_partner_fee,
max_partner_fee: config.fee_policy_max_partner_fee,
upcoming_fee_policies: config.upcoming_fee_policies.clone().into(),
}
}

Expand Down Expand Up @@ -230,13 +253,22 @@ impl ProtocolFees {
quote: domain::Quote,
partner_fees: Vec<Policy>,
) -> domain::Order {
let protocol_fees = self
.fee_policies
// Use new fee policies if the order creation date is after their effective
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the desired logic. As of the target date, all orders that are settled should be settled according to the new fee policies (as these are what decides the rewards that solvers will receive for these trades0.

In particular, a limit order that has been placed in the past should also carry the new fee policy going forward.

I'm not sure what the best way to achieve this is, but don't we build the fee policies from scratch on every auction? In this case I'd apply the logic like in the orderbook - as of a certain timestamp we start adding them unconditionally on when the order was placed.

// timestamp.
let fee_policies = self
.upcoming_fee_policies
.as_ref()
.filter(|upcoming| order.metadata.creation_date >= upcoming.effective_from_timestamp)
.map(|upcoming| &upcoming.fee_policies)
.unwrap_or(&self.fee_policies);

let protocol_fees = fee_policies
.iter()
.filter_map(|fee_policy| Self::protocol_fee_into_policy(&order, &quote, fee_policy))
.flat_map(|policy| Self::variant_fee_apply(&order, &quote, policy))
.chain(partner_fees)
.collect::<Vec<_>>();

boundary::order::to_domain(order, protocol_fees, Some(quote))
}

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 @@ -538,7 +538,7 @@ pub async fn run(args: Arguments, shutdown_controller: ShutdownController) {
args.limit_order_price_factor
.try_into()
.expect("limit order price factor can't be converted to BigDecimal"),
domain::ProtocolFees::new(&args.fee_policies, args.fee_policy_max_partner_fee),
domain::ProtocolFees::new(&args.fee_policies_config),
cow_amm_registry.clone(),
args.run_loop_native_price_timeout,
eth.contracts().settlement().address().into_legacy(),
Expand Down
39 changes: 34 additions & 5 deletions crates/e2e/src/setup/fee.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
pub struct ProtocolFeesConfig(pub Vec<ProtocolFee>);
use chrono::{DateTime, Utc};

#[derive(Default)]
pub struct ProtocolFeesConfig {
pub protocol_fees: Vec<ProtocolFee>,
pub upcoming_protocol_fees: Option<UpcomingProtocolFees>,
}

#[derive(Clone)]
pub struct UpcomingProtocolFees {
pub fee_policies: Vec<ProtocolFee>,
pub effective_from_timestamp: DateTime<Utc>,
}

#[derive(Clone)]
pub struct ProtocolFee {
Expand Down Expand Up @@ -57,14 +69,31 @@ impl std::fmt::Display for ProtocolFee {
}
}

impl std::fmt::Display for ProtocolFeesConfig {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
impl ProtocolFeesConfig {
pub fn into_args(self) -> Vec<String> {
let mut args = Vec::new();
let fees_str = self
.0
.protocol_fees
.iter()
.map(|fee| fee.to_string())
.collect::<Vec<_>>()
.join(",");
write!(f, "--fee-policies={fees_str}")
args.push(format!("--fee-policies={fees_str}"));

if let Some(upcoming_protocol_fees) = &self.upcoming_protocol_fees {
let upcoming_fees_str = upcoming_protocol_fees
.fee_policies
.iter()
.map(|fee| fee.to_string())
.collect::<Vec<_>>()
.join(",");
args.push(format!("--upcoming-fee-policies={}", upcoming_fees_str));
args.push(format!(
"--upcoming-fee-policies-timestamp={}",
upcoming_protocol_fees.effective_from_timestamp.to_rfc3339()
));
}

args
}
}
42 changes: 23 additions & 19 deletions crates/e2e/tests/e2e/limit_orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1066,32 +1066,36 @@ async fn no_liquidity_limit_order(web3: Web3) {
.unwrap();

// Setup services
let protocol_fees_config = ProtocolFeesConfig(vec![
ProtocolFee {
policy: fee::FeePolicyKind::Surplus {
factor: 0.5,
max_volume_factor: 0.01,
let protocol_fee_args = ProtocolFeesConfig {
protocol_fees: vec![
ProtocolFee {
policy: fee::FeePolicyKind::Surplus {
factor: 0.5,
max_volume_factor: 0.01,
},
policy_order_class: FeePolicyOrderClass::Limit,
},
policy_order_class: FeePolicyOrderClass::Limit,
},
ProtocolFee {
policy: fee::FeePolicyKind::PriceImprovement {
factor: 0.5,
max_volume_factor: 0.01,
ProtocolFee {
policy: fee::FeePolicyKind::PriceImprovement {
factor: 0.5,
max_volume_factor: 0.01,
},
policy_order_class: FeePolicyOrderClass::Market,
},
policy_order_class: FeePolicyOrderClass::Market,
},
])
.to_string();
],
..Default::default()
}
.into_args();

let services = Services::new(&onchain).await;
services
.start_protocol_with_args(
ExtraServiceArgs {
autopilot: vec![
protocol_fees_config,
format!("--unsupported-tokens={:#x}", unsupported.address()),
],
autopilot: [
protocol_fee_args,
vec![format!("--unsupported-tokens={:#x}", unsupported.address())],
]
.concat(),
..Default::default()
},
solver,
Expand Down
Loading