From 56e5bec0b4050586c801468f22cbe6dc7206d613 Mon Sep 17 00:00:00 2001
From: Kaloyan Gangov <gangov1@gmail.com>
Date: Thu, 18 Jan 2024 19:21:52 +0200
Subject: [PATCH 1/6] Phoenix: adds a new macro, that validates that the
 arguments which it takes are 0..10_000

---
 packages/phoenix/src/utils.rs | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/packages/phoenix/src/utils.rs b/packages/phoenix/src/utils.rs
index 7aad3c4d4..af06867e9 100644
--- a/packages/phoenix/src/utils.rs
+++ b/packages/phoenix/src/utils.rs
@@ -18,6 +18,19 @@ macro_rules! validate_int_parameters {
     };
 }
 
+// Validate all bps to be between the range 0..10_000
+macro_rules! validate_bps {
+    ($($value:expr),+) => {
+        const MIN_BPS: i64 = 0;
+        const MAX_BPS: i64 = 10_000;
+        $(
+            if $value < MIN_BPS || $value > MAX_BPS {
+                panic!("The value {} is out of range. Must be between {}‱ and {}‱", $value, MIN_BPS, MAX_BPS);
+            }
+        )+
+    }
+}
+
 pub fn is_approx_ratio(a: Decimal, b: Decimal, tolerance: Decimal) -> bool {
     let diff = (a - b).abs();
     diff <= tolerance
@@ -119,4 +132,21 @@ mod tests {
         let tolerance = Decimal::percent(3);
         assert!(!is_approx_ratio(a, b, tolerance));
     }
+
+    #[test]
+    #[should_panic(expected = "The value -1 is out of range. Must be between 0‱ and 10000‱")]
+    fn test_below_min() {
+        validate_bps!(-1);
+    }
+
+    #[test]
+    #[should_panic(expected = "The value 10001 is out of range. Must be between 0‱ and 10000‱")]
+    fn test_above_max() {
+        validate_bps!(10_001);
+    }
+
+    #[test]
+    fn test_valid_range() {
+        validate_bps!(0, 5000, 10_000);
+    }
 }

From c71d028114dc9e7a6f376fd70654c59549bf04c4 Mon Sep 17 00:00:00 2001
From: Kaloyan Gangov <gangov1@gmail.com>
Date: Thu, 18 Jan 2024 19:32:06 +0200
Subject: [PATCH 2/6] CHANGELOG.md: adds new entry

---
 CHANGELOG.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ad96433d5..eb02eb195 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,12 @@ and this project adheres to
 
 [#200]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/200
 
+## Added
+
+- Adds a new macro that validates the bps arguments value ([#199])
+
+[#199]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/199
+
 ## [0.8.0] - 2024-01-17
 
 ## Changed

From 61ff81b476ca899b15243fc0d55c9774acab433a Mon Sep 17 00:00:00 2001
From: Kaloyan Gangov <gangov1@gmail.com>
Date: Fri, 19 Jan 2024 22:19:34 +0200
Subject: [PATCH 3/6] Phoenix: tests

---
 packages/phoenix/src/utils.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/phoenix/src/utils.rs b/packages/phoenix/src/utils.rs
index af06867e9..b08375d31 100644
--- a/packages/phoenix/src/utils.rs
+++ b/packages/phoenix/src/utils.rs
@@ -135,18 +135,18 @@ mod tests {
 
     #[test]
     #[should_panic(expected = "The value -1 is out of range. Must be between 0‱ and 10000‱")]
-    fn test_below_min() {
-        validate_bps!(-1);
+    fn validate_bps_below_min() {
+        validate_bps!(-1, 300, 5_000, 8_534);
     }
 
     #[test]
     #[should_panic(expected = "The value 10001 is out of range. Must be between 0‱ and 10000‱")]
-    fn test_above_max() {
-        validate_bps!(10_001);
+    fn validate_bps_above_max() {
+        validate_bps!(100, 10_001, 31_3134, 348);
     }
 
     #[test]
-    fn test_valid_range() {
-        validate_bps!(0, 5000, 10_000);
+    fn bps_valid_range() {
+        validate_bps!(0, 5_000, 7_500, 10_000);
     }
 }

From 9a7723d78c429967e1dcc1c1c3b7ebcab6b2b8a1 Mon Sep 17 00:00:00 2001
From: Kaloyan Gangov <gangov1@gmail.com>
Date: Fri, 19 Jan 2024 22:27:11 +0200
Subject: [PATCH 4/6] Phoenix, Factory: makes validate_bps available for export
 and uses it in factory contract

---
 contracts/factory/src/contract.rs | 8 ++++++++
 packages/phoenix/src/utils.rs     | 1 +
 2 files changed, 9 insertions(+)

diff --git a/contracts/factory/src/contract.rs b/contracts/factory/src/contract.rs
index 10f6fb74b..94d63884c 100644
--- a/contracts/factory/src/contract.rs
+++ b/contracts/factory/src/contract.rs
@@ -8,6 +8,7 @@ use crate::{
     utils::deploy_lp_contract,
 };
 use phoenix::utils::{LiquidityPoolInitInfo, StakeInitInfo, TokenInitInfo};
+use phoenix::validate_bps;
 use soroban_sdk::{
     contract, contractimpl, contractmeta, log, Address, BytesN, Env, IntoVal, Symbol, Val, Vec,
 };
@@ -120,6 +121,13 @@ impl FactoryTrait for Factory {
             &lp_init_info.token_init_info.token_b,
         );
 
+        validate_bps!(
+            lp_init_info.swap_fee_bps,
+            lp_init_info.max_allowed_slippage_bps,
+            lp_init_info.max_allowed_spread_bps,
+            lp_init_info.max_referral_bps
+        );
+
         let init_fn: Symbol = Symbol::new(&env, "initialize");
         let init_fn_args: Vec<Val> =
             (stake_wasm_hash, token_wasm_hash, lp_init_info.clone()).into_val(&env);
diff --git a/packages/phoenix/src/utils.rs b/packages/phoenix/src/utils.rs
index b08375d31..46beae7c2 100644
--- a/packages/phoenix/src/utils.rs
+++ b/packages/phoenix/src/utils.rs
@@ -19,6 +19,7 @@ macro_rules! validate_int_parameters {
 }
 
 // Validate all bps to be between the range 0..10_000
+#[macro_export]
 macro_rules! validate_bps {
     ($($value:expr),+) => {
         const MIN_BPS: i64 = 0;

From 25e9734ea95cf6a11682815c6a66c9f7e5d3a4ad Mon Sep 17 00:00:00 2001
From: Kaloyan Gangov <gangov1@gmail.com>
Date: Fri, 19 Jan 2024 22:36:27 +0200
Subject: [PATCH 5/6] Phoenix, Factory: makes test for the usecase in factory

---
 packages/phoenix/src/utils.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/phoenix/src/utils.rs b/packages/phoenix/src/utils.rs
index 46beae7c2..f6fcff2c2 100644
--- a/packages/phoenix/src/utils.rs
+++ b/packages/phoenix/src/utils.rs
@@ -26,7 +26,7 @@ macro_rules! validate_bps {
         const MAX_BPS: i64 = 10_000;
         $(
             if $value < MIN_BPS || $value > MAX_BPS {
-                panic!("The value {} is out of range. Must be between {}‱ and {}‱", $value, MIN_BPS, MAX_BPS);
+                panic!("The value {} is out of range. Must be between {} and {} bps.", $value, MIN_BPS, MAX_BPS);
             }
         )+
     }
@@ -135,13 +135,13 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "The value -1 is out of range. Must be between 0‱ and 10000‱")]
+    #[should_panic(expected = "The value -1 is out of range. Must be between 0 and 10000 bps.")]
     fn validate_bps_below_min() {
         validate_bps!(-1, 300, 5_000, 8_534);
     }
 
     #[test]
-    #[should_panic(expected = "The value 10001 is out of range. Must be between 0‱ and 10000‱")]
+    #[should_panic(expected = "The value 10001 is out of range. Must be between 0 and 10000 bps.")]
     fn validate_bps_above_max() {
         validate_bps!(100, 10_001, 31_3134, 348);
     }

From 59ae7636bc3824b09b0ee265b3d27ac4fe1d6dd4 Mon Sep 17 00:00:00 2001
From: Kaloyan Gangov <gangov1@gmail.com>
Date: Thu, 25 Jan 2024 21:37:13 +0200
Subject: [PATCH 6/6] Phoenix: updates validate_bps macro to check the range in
 a more Rust-native way; Pool, Pool-Stable: uses validate_bps macro in the
 init call and updates tests.

---
 contracts/pool/src/contract.rs               | 9 ++++++++-
 contracts/pool/src/tests/liquidity.rs        | 2 +-
 contracts/pool/src/tests/setup.rs            | 6 +-----
 contracts/pool_stable/src/contract.rs        | 7 ++++++-
 contracts/pool_stable/src/tests/liquidity.rs | 2 +-
 packages/phoenix/src/utils.rs                | 7 ++++---
 6 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs
index 8b6672396..1b711a1f3 100644
--- a/contracts/pool/src/contract.rs
+++ b/contracts/pool/src/contract.rs
@@ -17,7 +17,7 @@ use crate::{
     token_contract,
 };
 use decimal::Decimal;
-use phoenix::{utils::is_approx_ratio, validate_int_parameters};
+use phoenix::{utils::is_approx_ratio, validate_bps, validate_int_parameters};
 
 // Metadata that is added on to the WASM custom section
 contractmeta!(
@@ -144,6 +144,13 @@ impl LiquidityPoolTrait for LiquidityPool {
         let token_init_info = lp_init_info.token_init_info;
         let stake_init_info = lp_init_info.stake_init_info;
 
+        validate_bps!(
+            swap_fee_bps,
+            max_allowed_slippage_bps,
+            max_allowed_spread_bps,
+            max_referral_bps
+        );
+
         set_initialized(&env);
 
         // Token info
diff --git a/contracts/pool/src/tests/liquidity.rs b/contracts/pool/src/tests/liquidity.rs
index 13516ac5a..d48cbc6d4 100644
--- a/contracts/pool/src/tests/liquidity.rs
+++ b/contracts/pool/src/tests/liquidity.rs
@@ -542,7 +542,7 @@ fn provide_liqudity_single_asset_one_third_with_fees() {
 }
 
 #[test]
-#[should_panic(expected = "Pool: Initialize: Fees must be between 0 and 100%")]
+#[should_panic(expected = "The value 10001 is out of range. Must be between 0 and 10000 bps.")]
 fn provide_liqudity_too_high_fees() {
     let env = Env::default();
     env.mock_all_auths();
diff --git a/contracts/pool/src/tests/setup.rs b/contracts/pool/src/tests/setup.rs
index 3e1c639fc..d229b0c64 100644
--- a/contracts/pool/src/tests/setup.rs
+++ b/contracts/pool/src/tests/setup.rs
@@ -65,10 +65,6 @@ pub fn deploy_liquidity_pool_contract<'a>(
         stake_init_info,
     };
 
-    pool.initialize(
-        &stake_wasm_hash,
-        &token_wasm_hash,
-        &lp_init_info,
-    );
+    pool.initialize(&stake_wasm_hash, &token_wasm_hash, &lp_init_info);
     pool
 }
diff --git a/contracts/pool_stable/src/contract.rs b/contracts/pool_stable/src/contract.rs
index 4aa72de3a..dc73b6507 100644
--- a/contracts/pool_stable/src/contract.rs
+++ b/contracts/pool_stable/src/contract.rs
@@ -14,7 +14,7 @@ use crate::{
     token_contract,
 };
 use decimal::Decimal;
-use phoenix::validate_int_parameters;
+use phoenix::{validate_bps, validate_int_parameters};
 
 // Minimum amount of initial LP shares to mint
 const MINIMUM_LIQUIDITY_AMOUNT: i128 = 1000;
@@ -140,6 +140,11 @@ impl StableLiquidityPoolTrait for StableLiquidityPool {
         let token_init_info = lp_init_info.token_init_info;
         let stake_init_info = lp_init_info.stake_init_info;
 
+        validate_bps!(
+            swap_fee_bps,
+            max_allowed_slippage_bps,
+            max_allowed_spread_bps
+        );
         set_initialized(&env);
 
         // Token info
diff --git a/contracts/pool_stable/src/tests/liquidity.rs b/contracts/pool_stable/src/tests/liquidity.rs
index 53ecf19e7..80210183a 100644
--- a/contracts/pool_stable/src/tests/liquidity.rs
+++ b/contracts/pool_stable/src/tests/liquidity.rs
@@ -542,7 +542,7 @@ fn provide_liqudity_single_asset_one_third_with_fees() {
 }
 
 #[test]
-#[should_panic(expected = "Pool: Initialize: Fees must be between 0 and 100%")]
+#[should_panic(expected = "The value 10001 is out of range. Must be between 0 and 10000 bps.")]
 fn provide_liqudity_too_high_fees() {
     let env = Env::default();
     env.mock_all_auths();
diff --git a/packages/phoenix/src/utils.rs b/packages/phoenix/src/utils.rs
index f6fcff2c2..1395f51d2 100644
--- a/packages/phoenix/src/utils.rs
+++ b/packages/phoenix/src/utils.rs
@@ -25,9 +25,10 @@ macro_rules! validate_bps {
         const MIN_BPS: i64 = 0;
         const MAX_BPS: i64 = 10_000;
         $(
-            if $value < MIN_BPS || $value > MAX_BPS {
-                panic!("The value {} is out of range. Must be between {} and {} bps.", $value, MIN_BPS, MAX_BPS);
-            }
+            // if $value < MIN_BPS || $value > MAX_BPS {
+            //     panic!("The value {} is out of range. Must be between {} and {} bps.", $value, MIN_BPS, MAX_BPS);
+            // }
+            assert!((MIN_BPS..=MAX_BPS).contains(&$value), "The value {} is out of range. Must be between {} and {} bps.", $value, MIN_BPS, MAX_BPS);
         )+
     }
 }