Forbit fee policies with factor 1.0 (#2545)

# Description
We need to add a protection against sending the fee policies with factor
= 1.0.

# Changes
- When parsing the fee policy configuration, check for the validity of
the factor
- Fails to parse the configuration if the factor value is out of the
range [0, 1)

## How to test
1. e2e
2. unit test

## Related Issues
Fixes #2501

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<Self, Self::Err> {
         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::<f64>()
-                    .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::<f64>()
-                    .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::<f64>()
-                    .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::<f64>()
-                    .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::<f64>()
-                    .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)"),)
         }
     }
 }

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: