Merge pull request #37 from BigJohn-dev/LoanManager-uses-raw-panics-for-user-facing-failures-instead-of-typed-LoanError-returns

feat(loan_manager): add repayment below minimum error handling and up…

Author: ogazboiz

lending_pool/src/lib.rs

@@ -21,6 +21,7 @@ pub enum PoolError {
     NoProposedAdmin = 10,
     CooldownTooLong = 11,
     NotPaused = 12,
+    WithdrawalCooldownActive = 13,
 }
 
 /// Storage keys.
@@ -300,20 +301,26 @@ impl LendingPool {
             .ok_or(PoolError::InvalidAmount)
     }
 
-    fn assert_withdrawal_cooldown_elapsed(env: &Env, provider: &Address, token: &Address) {
+    fn assert_withdrawal_cooldown_elapsed(
+        env: &Env,
+        provider: &Address,
+        token: &Address,
+    ) -> Result<(), PoolError> {
         let cooldown = Self::withdrawal_cooldown(env);
         if cooldown == 0 {
-            return;
+            return Ok(());
         }
 
         let Some(deposit_ledger) = Self::read_deposit_timestamp(env, provider, token) else {
-            return;
+            return Ok(());
         };
 
         let current_ledger = env.ledger().sequence();
         if current_ledger < deposit_ledger.saturating_add(cooldown) {
-            panic!("withdrawal_cooldown_active");
+            return Err(PoolError::WithdrawalCooldownActive);
         }
+
+        Ok(())
     }
 
     /// Burns `shares` for `provider` and transfers out the proportional
@@ -726,7 +733,7 @@ impl LendingPool {
     ) -> Result<(), PoolError> {
         provider.require_auth();
         Self::assert_not_paused(&env)?;
-        Self::assert_withdrawal_cooldown_elapsed(&env, &provider, &token);
+        Self::assert_withdrawal_cooldown_elapsed(&env, &provider, &token)?;
         let assets = Self::redeem_shares(&env, &provider, &token, shares)?;
         withdraw(&env, provider, token, assets, shares);
         Ok(())

lending_pool/src/test.rs

@@ -194,8 +194,7 @@ fn test_insufficient_balance_withdraw_panic() {
 }
 
 #[test]
-#[should_panic(expected = "withdrawal_cooldown_active")]
-fn test_immediate_withdraw_panics_when_cooldown_active() {
+fn test_withdraw_returns_cooldown_error_when_cooldown_active() {
     let env = Env::default();
     env.mock_all_auths();
 
@@ -210,7 +209,8 @@ fn test_immediate_withdraw_panics_when_cooldown_active() {
     stellar_asset_client.mint(&provider, &5_000);
     pool_client.deposit(&provider, &token_id, &1_000);
 
-    pool_client.withdraw(&provider, &token_id, &1_000);
+    let result = pool_client.try_withdraw(&provider, &token_id, &1_000);
+    assert_eq!(result, Err(Ok(crate::PoolError::WithdrawalCooldownActive)));
 }
 
 #[test]

loan_manager/src/lib.rs

@@ -62,6 +62,7 @@ pub enum LoanError {
     InvalidExtension = 25,
     InsufficientCollateral = 26,
     LoanNotLiquidatable = 27,
+    RepaymentBelowMinimum = 28,
 }
 
 #[contracttype]
@@ -515,6 +516,10 @@ impl LoanManager {
             .checked_add(delta)
             .expect("total outstanding overflow");
 
+        // This can only become negative if the contract state is inconsistent:
+        // repayment logic only ever subtracts the exact principal amount of a
+        // fully repaid loan, and the corresponding loan-approval bookkeeping
+        // prevents a second subtraction from the same outstanding balance.
         if updated < 0 {
             panic!("total outstanding underflow");
         }
@@ -1270,7 +1275,7 @@ impl LoanManager {
         let is_rounding_dust_forgiveness = total_debt <= min_repayment_amount;
 
         if amount < total_debt && amount < min_repayment_amount && !is_rounding_dust_forgiveness {
-            panic!("repayment amount below minimum");
+            return Err(LoanError::RepaymentBelowMinimum);
         }
 
         let token: Address = env
@@ -2057,16 +2062,16 @@ impl LoanManager {
         Self::max_loan_amount(&env)
     }
 
-    pub fn set_min_repayment_amount(env: Env, amount: i128) {
+    pub fn set_min_repayment_amount(env: Env, amount: i128) -> Result<(), LoanError> {
         if amount < 0 {
-            panic!("min repayment amount cannot be negative");
+            return Err(LoanError::InvalidAmount);
         }
 
         let admin: Address = env
             .storage()
             .instance()
             .get(&DataKey::Admin)
-            .expect("not initialized");
+            .ok_or(LoanError::NotInitialized)?;
         admin.require_auth();
 
         let old_amount = Self::min_repayment_amount(&env);
@@ -2075,6 +2080,8 @@ impl LoanManager {
             .set(&DataKey::MinRepaymentAmount, &amount);
         Self::bump_instance_ttl(&env);
         events::min_repayment_updated(&env, admin, old_amount, amount);
+
+        Ok(())
     }
 
     pub fn get_min_repayment_amount(env: Env) -> i128 {

loan_manager/src/test.rs

@@ -710,7 +710,6 @@ fn test_partial_repayment_tracks_split_balances() {
 }
 
 #[test]
-#[should_panic(expected = "repayment amount below minimum")]
 fn test_minimum_repayment_amount_enforced() {
     let env = Env::default();
     env.mock_all_auths_allowing_non_root_auth();
@@ -737,7 +736,19 @@ fn test_minimum_repayment_amount_enforced() {
     manager.approve_loan(&loan_id);
 
     manager.set_min_repayment_amount(&150);
-    manager.repay(&borrower, &loan_id, &100);
+    let result = manager.try_repay(&borrower, &loan_id, &100);
+    assert_eq!(result, Err(Ok(LoanError::RepaymentBelowMinimum)));
+}
+
+#[test]
+fn test_set_min_repayment_amount_rejects_negative_values() {
+    let env = Env::default();
+    env.mock_all_auths();
+
+    let (manager, _nft_client, _pool_client, _token_id, _token_admin) = setup_test(&env);
+
+    let result = manager.try_set_min_repayment_amount(&-1);
+    assert_eq!(result, Err(Ok(LoanError::InvalidAmount)));
 }
 
 #[test]