diff --git a/crates/autopilot/src/arguments.rs b/crates/autopilot/src/arguments.rs index 4b22afe3c7..98d8c886c4 100644 --- a/crates/autopilot/src/arguments.rs +++ b/crates/autopilot/src/arguments.rs @@ -1,5 +1,6 @@ use { crate::infra, + anyhow::Context, primitive_types::{H160, U256}, shared::{ arguments::{display_list, display_option, ExternalSolver}, @@ -357,7 +358,7 @@ pub struct FeePolicy { /// /// - Volume based: /// volume:0.1 - #[clap(long, env, default_value = "surplus:0.0:1.0")] + #[clap(long, env, default_value = "surplus:0.0:0.9")] pub fee_policy_kind: FeePolicyKind, /// Should protocol fees be collected or skipped for orders whose @@ -379,24 +380,34 @@ pub enum FeePolicyKind { Volume { factor: f64 }, } +fn validate_factor(factor: f64) -> anyhow::Result<()> { + anyhow::ensure!( + (0.0..1.0).contains(&factor), + "Factor must be in the range [0, 1)" + ); + Ok(()) +} + impl FromStr for FeePolicyKind { - type Err = String; + type Err = anyhow::Error; fn from_str(s: &str) -> Result { let mut parts = s.split(':'); - let kind = parts.next().ok_or("missing fee policy kind")?; + let kind = parts.next().context("missing fee policy kind")?; match kind { "surplus" => { let factor = parts .next() - .ok_or("missing surplus factor")? + .context("missing surplus factor")? .parse::() - .map_err(|e| format!("invalid surplus factor: {}", e))?; + .map_err(|e| anyhow::anyhow!("invalid surplus factor: {}", e))?; + validate_factor(factor)?; let max_volume_factor = parts .next() - .ok_or("missing max volume factor")? + .context("missing max volume factor")? .parse::() - .map_err(|e| format!("invalid max volume factor: {}", e))?; + .map_err(|e| anyhow::anyhow!("invalid max volume factor: {}", e))?; + validate_factor(max_volume_factor)?; Ok(Self::Surplus { factor, max_volume_factor, @@ -405,14 +416,18 @@ impl FromStr for FeePolicyKind { "priceImprovement" => { let factor = parts .next() - .ok_or("missing price improvement factor")? + .context("missing price improvement factor")? .parse::() - .map_err(|e| format!("invalid price improvement factor: {}", e))?; + .map_err(|e| anyhow::anyhow!("invalid price improvement factor: {}", e))?; + validate_factor(factor)?; let max_volume_factor = parts .next() - .ok_or("missing price improvement max volume factor")? + .context("missing price improvement max volume factor")? .parse::() - .map_err(|e| format!("invalid price improvement max volume factor: {}", e))?; + .map_err(|e| { + anyhow::anyhow!("invalid price improvement max volume factor: {}", e) + })?; + validate_factor(max_volume_factor)?; Ok(Self::PriceImprovement { factor, max_volume_factor, @@ -421,12 +436,42 @@ impl FromStr for FeePolicyKind { "volume" => { let factor = parts .next() - .ok_or("missing volume factor")? + .context("missing volume factor")? .parse::() - .map_err(|e| format!("invalid volume factor: {}", e))?; + .map_err(|e| anyhow::anyhow!("invalid volume factor: {}", e))?; + validate_factor(factor)?; Ok(Self::Volume { factor }) } - _ => Err(format!("invalid fee policy kind: {}", kind)), + _ => Err(anyhow::anyhow!("invalid fee policy kind: {}", kind)), + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_fee_factor_limits() { + let policies = vec![ + "volume:1.0", + "volume:-1.0", + "surplus:1.0:0.5", + "surplus:0.5:1.0", + "surplus:0.5:-1.0", + "surplus:-1.0:0.5", + "priceImprovement:1.0:0.5", + "priceImprovement:-1.0:0.5", + "priceImprovement:0.5:1.0", + "priceImprovement:0.5:-1.0", + ]; + + for policy in policies { + assert!(FeePolicyKind::from_str(policy) + .err() + .unwrap() + .to_string() + .contains("Factor must be in the range [0, 1)"),) } } } diff --git a/crates/e2e/tests/e2e/protocol_fee.rs b/crates/e2e/tests/e2e/protocol_fee.rs index 4824b8341b..508df7d39b 100644 --- a/crates/e2e/tests/e2e/protocol_fee.rs +++ b/crates/e2e/tests/e2e/protocol_fee.rs @@ -58,7 +58,7 @@ async fn local_node_volume_fee_buy_order() { async fn surplus_fee_sell_order_test(web3: Web3) { let fee_policy = FeePolicyKind::Surplus { factor: 0.3, - max_volume_factor: 1.0, + max_volume_factor: 0.9, }; // Without protocol fee: // Expected execution is 10000000000000000000 GNO for @@ -91,7 +91,7 @@ async fn surplus_fee_sell_order_test(web3: Web3) { async fn surplus_fee_sell_order_capped_test(web3: Web3) { let fee_policy = FeePolicyKind::Surplus { - factor: 1.0, + factor: 0.9, max_volume_factor: 0.1, }; // Without protocol fee: @@ -150,7 +150,7 @@ async fn partner_fee_sell_order_test(web3: Web3) { // Fee policy to be overwritten by the partner fee let fee_policy = FeePolicyKind::PriceImprovement { factor: 0.5, - max_volume_factor: 1.0, + max_volume_factor: 0.9, }; // Without protocol fee: // Expected execution is 10000000000000000000 GNO for @@ -182,7 +182,7 @@ async fn partner_fee_sell_order_test(web3: Web3) { async fn surplus_fee_buy_order_test(web3: Web3) { let fee_policy = FeePolicyKind::Surplus { factor: 0.3, - max_volume_factor: 1.0, + max_volume_factor: 0.9, }; // Without protocol fee: // Expected execution is 5040413426236634210 GNO for 5000000000000000000 DAI, @@ -211,7 +211,7 @@ async fn surplus_fee_buy_order_test(web3: Web3) { async fn surplus_fee_buy_order_capped_test(web3: Web3) { let fee_policy = FeePolicyKind::Surplus { - factor: 1.0, + factor: 0.9, max_volume_factor: 0.1, }; // Without protocol fee: