Merge pull request #245 from Phoenix-Protocol-Group/229-v-phx-vul-035…

…-lack-of-validation-on-total_shares Pool: adds a validation and panic for when shares are equal to zero.
Phoenix-Protocol-Group · Feb 13, 2024 · 2f55fdc · 2f55fdc
2 parents dab13c2 + f6f5798
commit 2f55fdc
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,8 +32,10 @@ and this project adheres to
 ## Bug fixes
 
 - Pool stable: Fixes an error in the compute_swap function, where commission isn't deducted ([233])
+- Pool: Adds a validation for when shares can be zero during withdrawal ([#245])
 
 [#233]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/233
+[#245]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/245
 
 ## [0.8.0] - 2024-01-17
 

diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs
@@ -415,12 +415,15 @@ impl LiquidityPoolTrait for LiquidityPool {
         let pool_balance_a = utils::get_pool_balance_a(&env);
         let pool_balance_b = utils::get_pool_balance_b(&env);
 
-        let mut share_ratio = Decimal::zero();
         let total_shares = utils::get_total_shares(&env);
-        if total_shares != 0i128 {
-            share_ratio = Decimal::from_ratio(share_amount, total_shares);
+
+        if total_shares == 0i128 {
+            log!(&env, "Pool: WithdrawLiquidity: Critical error - Total shares are equal to zero before withdrawal!");
+            panic_with_error!(env, ContractError::TotalSharesEqualZero);
         }
 
+        let share_ratio = Decimal::from_ratio(share_amount, total_shares);
+
         let return_amount_a = pool_balance_a * share_ratio;
         let return_amount_b = pool_balance_b * share_ratio;
 

diff --git a/contracts/pool/src/error.rs b/contracts/pool/src/error.rs
@@ -19,4 +19,5 @@ pub enum ContractError {
     GetDepositAmountsAmountALessThenMinA = 10,
     GetDepositAmountsAmountBBiggerThenDesiredB = 11,
     GetDepositAmountsAmountBLessThenMinB = 12,
+    TotalSharesEqualZero = 13,
 }
diff --git a/contracts/pool/src/storage.rs b/contracts/pool/src/storage.rs
@@ -253,12 +253,14 @@ pub mod utils {
 
         if let Some(min_a) = min_a {
             if min_a > desired_a {
+                log!(&env, "Pool: GetDepositAmounts: Critical error - minimumA is bigger than desiredA");
                 panic_with_error!(env, ContractError::GetDepositAmountsMinABiggerThenDesiredA);
             }
         }
         if let Some(min_b) = min_b {
             if min_b > desired_b {
-                panic_with_error!(env, ContractError::GetDepositAmountsMinABiggerThenDesiredA);
+                log!(&env, "Pool: GetDepositAmounts: Critical error - minimumB is bigger than desiredB");
+                panic_with_error!(env, ContractError::GetDepositAmountsMinBBiggerThenDesiredB);
             }
         }
 
@@ -421,7 +423,7 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "Error(Contract, #7)")]
+    #[should_panic(expected = "Error(Contract, #8)")]
     fn test_get_deposit_amounts_amount_b_greater_than_desired_and_less_than_min_b() {
         let env = Env::default();
         utils::get_deposit_amounts(