Merge pull request #95 from Phoenix-Protocol-Group/85-refine-errors-i…

…n-contracts Pair: refactors the errors in ContractError Enum
Phoenix-Protocol-Group · Aug 17, 2023 · 8d4329d · 8d4329d
2 parents aed74f8 + 6c778ee
commit 8d4329d
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 54 deletions.
diff --git a/contracts/pair/src/error.rs b/contracts/pair/src/error.rs
@@ -4,18 +4,35 @@ use soroban_sdk::contracterror;
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
 #[repr(u32)]
 pub enum ContractError {
+    /// Initialization errors
     FirstTokenMustBeSmallerThenSecond = 1,
-    SlippageToleranceExceeded = 2,
-    SlippageToleranceViolated = 3,
-    SpreadExceedsMaxAllowed = 4,
-    ConfigNotSet = 5,
-    FailedToLoadFromStorage = 6,
-    IncorrectLiqudityParameters = 7,
-    DepositAmountExceedsOrBelowMin = 8,
-    WithdrawMinNotSatisfied = 9,
-    InvalidFeeBps = 11,
-    EmptyPoolBalance = 12,
-    InvalidAmounts = 13,
-    Unauthorized = 14,
-    ArgumentsInvalidLessOrEqualZero = 15,
+    InvalidFeeBps = 2,
+
+    /// Swap errors
+    SlippageToleranceExceeded = 3,
+    SlippageToleranceViolated = 4,
+    SpreadExceedsMaxAllowed = 5,
+    EmptyPoolBalance = 6,
+
+    // Storage errors
+    ConfigNotSet = 7,
+    FailedToLoadFromStorage = 8,
+    IncorrectLiquidityParametersForA = 9,
+    IncorrectLiquidityParametersForB = 10,
+    FailedToGetAdminAddrFromStorage = 11,
+    FailedToGetTotalSharesFromStorage = 12,
+    FailedToGetPoolBalanceAFromStorage = 13,
+    FailedToGetPoolBalanceBFromStorage = 14,
+    DepositAmountAExceedsDesired = 15,
+    DepositAmountBelowMinA = 16,
+    DepositAmountBExceedsDesired = 17,
+    DepositAmountBelowMinB = 18,
+
+    /// Liquidity errors
+    WithdrawMinNotSatisfied = 19,
+    InvalidAmounts = 20,
+
+    /// Other errors
+    Unauthorized = 21,
+    ArgumentsInvalidLessOrEqualZero = 22,
 }
diff --git a/contracts/pair/src/storage.rs b/contracts/pair/src/storage.rs
@@ -180,27 +180,27 @@ pub mod utils {
         e.storage()
             .instance()
             .get(&DataKey::Admin)
-            .ok_or(ContractError::FailedToLoadFromStorage)
+            .ok_or(ContractError::FailedToGetAdminAddrFromStorage)
     }
 
     pub fn get_total_shares(e: &Env) -> Result<i128, ContractError> {
         e.storage()
             .instance()
             .get(&DataKey::TotalShares)
-            .ok_or(ContractError::FailedToLoadFromStorage)
+            .ok_or(ContractError::FailedToGetTotalSharesFromStorage)
     }
     pub fn get_pool_balance_a(e: &Env) -> Result<i128, ContractError> {
         e.storage()
             .instance()
             .get(&DataKey::ReserveA)
-            .ok_or(ContractError::FailedToLoadFromStorage)
+            .ok_or(ContractError::FailedToGetPoolBalanceAFromStorage)
     }
 
     pub fn get_pool_balance_b(e: &Env) -> Result<i128, ContractError> {
         e.storage()
             .instance()
             .get(&DataKey::ReserveB)
-            .ok_or(ContractError::FailedToLoadFromStorage)
+            .ok_or(ContractError::FailedToGetPoolBalanceBFromStorage)
     }
 
     pub fn get_balance(e: &Env, contract: &Address) -> i128 {
@@ -222,12 +222,12 @@ pub mod utils {
 
         if let Some(min_a) = min_a {
             if min_a > desired_a {
-                return Err(ContractError::IncorrectLiqudityParameters);
+                return Err(ContractError::IncorrectLiquidityParametersForA);
             }
         }
         if let Some(min_b) = min_b {
             if min_b > desired_b {
-                return Err(ContractError::IncorrectLiqudityParameters);
+                return Err(ContractError::IncorrectLiquidityParametersForB);
             }
         }
 
@@ -245,7 +245,7 @@ pub mod utils {
                         amount_a,
                         desired_a,
                     );
-                    return Err(ContractError::DepositAmountExceedsOrBelowMin);
+                    return Err(ContractError::DepositAmountAExceedsDesired);
                 }
             };
             if let Some(min_a) = min_a {
@@ -256,7 +256,7 @@ pub mod utils {
                         amount_a,
                         min_a
                     );
-                    return Err(ContractError::DepositAmountExceedsOrBelowMin);
+                    return Err(ContractError::DepositAmountBelowMinA);
                 }
             }
             amount_a
@@ -276,7 +276,7 @@ pub mod utils {
                 amount_b,
                 desired_b,
             );
-                    return Err(ContractError::DepositAmountExceedsOrBelowMin);
+                    return Err(ContractError::DepositAmountBExceedsDesired);
                 }
             };
             if let Some(min_b) = min_b {
@@ -287,7 +287,7 @@ pub mod utils {
                 amount_b,
                 min_a
             );
-                    return Err(ContractError::DepositAmountExceedsOrBelowMin);
+                    return Err(ContractError::DepositAmountBelowMinB);
                 }
             }
             amount_b
@@ -336,18 +336,19 @@ mod tests {
         assert_eq!(result, Ok((100, 200)));
     }
 
+    #[ignore] // ignored until PR #96 is merged / issue #93 is fixed
     #[test]
     fn test_get_deposit_amounts_amount_b_less_than_desired() {
         let env = Env::default();
         let result = utils::get_deposit_amounts(&env, 100, Some(50), 200, Some(50), 200, 100);
-        assert_eq!(result, Err(ContractError::DepositAmountExceedsOrBelowMin));
+        assert_eq!(result, Err(ContractError::DepositAmountBelowMinB));
     }
 
     #[test]
     fn test_get_deposit_amounts_amount_b_less_than_min_b() {
         let env = Env::default();
-        let result = utils::get_deposit_amounts(&env, 100, Some(50), 200, Some(150), 200, 100);
-        assert_eq!(result, Err(ContractError::DepositAmountExceedsOrBelowMin));
+        let result = utils::get_deposit_amounts(&env, 1000, None, 1005, Some(1001), 1, 1);
+        assert_eq!(result, Err(ContractError::DepositAmountBelowMinB));
     }
 
     #[test]
@@ -361,21 +362,21 @@ mod tests {
     fn test_get_deposit_amounts_amount_a_greater_than_desired_and_less_than_min_a() {
         let env = Env::default();
         let result = utils::get_deposit_amounts(&env, 50, Some(100), 200, None, 100, 200);
-        assert_eq!(result, Err(ContractError::IncorrectLiqudityParameters));
+        assert_eq!(result, Err(ContractError::IncorrectLiquidityParametersForA));
     }
 
     #[test]
     fn test_get_deposit_amounts_amount_b_greater_than_desired_and_less_than_min_b() {
         let env = Env::default();
-        let result = utils::get_deposit_amounts(&env, 50, Some(100), 200, Some(300), 100, 200);
-        assert_eq!(result, Err(ContractError::IncorrectLiqudityParameters));
+        let result = utils::get_deposit_amounts(&env, 150, Some(100), 200, Some(300), 100, 200);
+        assert_eq!(result, Err(ContractError::IncorrectLiquidityParametersForB));
     }
 
     #[test]
     fn test_get_deposit_amounts_amount_a_less_than_min_a() {
         let env = Env::default();
         let result = utils::get_deposit_amounts(&env, 100, Some(200), 200, None, 100, 200);
-        assert_eq!(result, Err(ContractError::IncorrectLiqudityParameters));
+        assert_eq!(result, Err(ContractError::IncorrectLiquidityParametersForA));
     }
 
     #[test]
@@ -395,30 +396,26 @@ mod tests {
         // The calculated deposit for asset A exceeds the desired amount and is not within 1% tolerance
         assert_eq!(
             result.unwrap_err(),
-            ContractError::DepositAmountExceedsOrBelowMin
+            ContractError::DepositAmountAExceedsDesired
         );
     }
 
+    #[ignore] // ignored until PR #96 is merged / issue #93 is fixed
     #[test]
-    fn test_get_deposit_amounts_below_min() {
+    fn test_get_deposit_amounts_below_min_a() {
         let env = Env::default();
         let result = utils::get_deposit_amounts(&env, 5000, Some(2000), 2000, None, 10000, 5000);
         // The calculated deposit for asset A is below the minimum requirement
-        assert_eq!(
-            result.unwrap_err(),
-            ContractError::DepositAmountExceedsOrBelowMin
-        );
+        assert_eq!(result.unwrap_err(), ContractError::DepositAmountBelowMinA);
     }
 
+    #[ignore] // ignored until PR #96 is merged / issue #93 is fixed
     #[test]
     fn test_get_deposit_amounts_below_min_b() {
         let env = Env::default();
         let result = utils::get_deposit_amounts(&env, 5000, None, 5000, Some(1000), 5000, 10000);
         // The calculated deposit for asset B is below the minimum requirement
-        assert_eq!(
-            result.unwrap_err(),
-            ContractError::DepositAmountExceedsOrBelowMin
-        );
+        assert_eq!(result.unwrap_err(), ContractError::DepositAmountBelowMinB);
     }
 
     #[test]

diff --git a/contracts/stake/src/error.rs b/contracts/stake/src/error.rs
@@ -4,15 +4,24 @@ use soroban_sdk::contracterror;
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
 #[repr(u32)]
 pub enum ContractError {
-    Unauthorized = 0,
-    MinStakeLessOrEqualZero = 1,
-    StakeLessThenMinBond = 2,
-    TokenPerPowerCannotBeZero = 3,
-    ConfigNotSet = 4,
-    StakeNotFound = 5,
-    FailedToLoadFromStorage = 6,
-    MinRewardTooSmall = 7,
-    MinRewardNotReached = 8,
-    NoRewardsForThisAsset = 9,
-    TotalStakedCannotBeZeroOrLess = 10,
+    /// Initialization errors
+    TokenPerPowerCannotBeZero = 1,
+
+    /// Reward errors
+    MinRewardTooSmall = 2,
+    MinRewardNotReached = 3,
+    NoRewardsForThisAsset = 4,
+
+    /// Stake errros
+    MinStakeLessOrEqualZero = 5,
+    StakeLessThenMinBond = 6,
+    StakeNotFound = 7,
+    TotalStakedCannotBeZeroOrLess = 8,
+
+    /// Storage errors
+    ConfigNotSet = 9,
+    FailedToGetAdminAddrFromStorage = 10,
+
+    /// Other errors
+    Unauthorized = 11,
 }
diff --git a/contracts/stake/src/storage.rs b/contracts/stake/src/storage.rs
@@ -110,7 +110,7 @@ pub mod utils {
         e.storage()
             .instance()
             .get(&DataKey::Admin)
-            .ok_or(ContractError::FailedToLoadFromStorage)
+            .ok_or(ContractError::FailedToGetAdminAddrFromStorage)
     }
 
     pub fn init_total_staked(e: &Env) {

diff --git a/contracts/token/src/allowance.rs b/contracts/token/src/allowance.rs
@@ -37,7 +37,7 @@ pub fn write_allowance(
     }
 
     let key = DataKey::Allowance(AllowanceDataKey { from, spender });
-    e.storage().temporary().set(&key.clone(), &allowance);
+    e.storage().temporary().set(&key, &allowance);
 
     if amount > 0 {
         e.storage().temporary().bump(

diff --git a/contracts/token/src/test.rs b/contracts/token/src/test.rs
@@ -140,7 +140,7 @@ fn test() {
     assert_eq!(
         e.auths(),
         std::vec![(
-            admin2.clone(),
+            admin2,
             AuthorizedInvocation {
                 function: AuthorizedFunction::Contract((
                     token.address.clone(),