diff --git a/contracts/creditline-contract/src/lib.rs b/contracts/creditline-contract/src/lib.rs index c4dcaf2..436bee9 100644 --- a/contracts/creditline-contract/src/lib.rs +++ b/contracts/creditline-contract/src/lib.rs @@ -21,6 +21,7 @@ use vendor_registry::Client as VendorRegistryContractClient; mod access; mod errors; mod events; +mod safe_math; mod storage; mod types; @@ -69,13 +70,13 @@ impl CreditLineContract { guarantee_amount: i128, repayment_schedule: Vec, loan_type: LoanType, - ) -> u64 { + ) -> Result { user.require_auth(); - Self::validate_guarantee(&env, total_amount, guarantee_amount); - Self::validate_vendor(&env, &vendor); - let score = Self::validate_reputation(&env, &user); - Self::validate_liquidity(&env, total_amount, guarantee_amount); + Self::validate_guarantee(&env, total_amount, guarantee_amount)?; + Self::validate_vendor(&env, &vendor)?; + let score = Self::validate_reputation(&env, &user)?; + Self::validate_liquidity(&env, total_amount, guarantee_amount)?; Self::enter_non_reentrant(&env); let mut loan = Self::build_loan( @@ -88,7 +89,7 @@ impl CreditLineContract { score, LoanStatus::Active, loan_type, - ); + )?; loan.funded_at = env.ledger().timestamp(); storage::increase_user_active_debt(&env, &user, loan.remaining_balance) @@ -96,9 +97,7 @@ impl CreditLineContract { let loan_id = loan.loan_id; storage::write_loan(&env, &loan); - let pool_contribution = total_amount - .checked_sub(guarantee_amount) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); + let pool_contribution = safe_math::sub_i128(total_amount, guarantee_amount)?; Self::fund_loan_from_pool(&env, &user, &vendor, guarantee_amount, pool_contribution); events::emit_loan_created( @@ -112,7 +111,7 @@ impl CreditLineContract { ); Self::exit_non_reentrant(&env); - loan_id + Ok(loan_id) } pub fn request_loan( @@ -123,11 +122,11 @@ impl CreditLineContract { guarantee_amount: i128, repayment_schedule: Vec, loan_type: LoanType, - ) -> u64 { + ) -> Result { user.require_auth(); - Self::validate_guarantee(&env, total_amount, guarantee_amount); - let score = Self::validate_reputation(&env, &user); + Self::validate_guarantee(&env, total_amount, guarantee_amount)?; + let score = Self::validate_reputation(&env, &user)?; let loan = Self::build_loan( &env, user.clone(), @@ -138,11 +137,9 @@ impl CreditLineContract { score, LoanStatus::Pending, loan_type, - ); + )?; - let token_address = storage::get_token(&env) - .unwrap_or_else(|err| panic_with_error!(&env, err)) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::TokenNotConfigured)); + let token_address = storage::get_token(&env)?.ok_or(CreditLineError::TokenNotConfigured)?; let token_client = token::Client::new(&env, &token_address); token_client.transfer(&user, &env.current_contract_address(), &guarantee_amount); @@ -159,7 +156,7 @@ impl CreditLineContract { &repayment_schedule, ); - loan_id + Ok(loan_id) } pub fn get_user_loans(env: Env, borrower: Address, start: u64, limit: u32) -> Vec { @@ -223,30 +220,35 @@ impl CreditLineContract { storage::set_parameters_contract(&env, &address); } - fn validate_guarantee(env: &Env, total_amount: i128, guarantee_amount: i128) { + fn validate_guarantee( + env: &Env, + total_amount: i128, + guarantee_amount: i128, + ) -> Result<(), CreditLineError> { if total_amount <= 0 || guarantee_amount <= 0 { - panic_with_error!(env, CreditLineError::InvalidAmount); + return Err(CreditLineError::InvalidAmount); } if guarantee_amount > total_amount { - panic_with_error!(env, CreditLineError::InvalidAmount); + return Err(CreditLineError::InvalidAmount); } let params = Self::get_protocol_parameters(env); - let min_guarantee = total_amount - .checked_mul(params.min_guarantee_percent) - .and_then(|v| v.checked_div(100)) - .unwrap_or_else(|| panic_with_error!(env, CreditLineError::Overflow)); + let min_guarantee = safe_math::div_i128( + safe_math::mul_i128(total_amount, params.min_guarantee_percent)?, + 100, + )?; if guarantee_amount < min_guarantee { - panic_with_error!(env, CreditLineError::InsufficientGuarantee); + return Err(CreditLineError::InsufficientGuarantee); } + Ok(()) } - fn validate_vendor(env: &Env, vendor: &Address) { + fn validate_vendor(env: &Env, vendor: &Address) -> Result<(), CreditLineError> { let vendor_registry = storage::get_vendor_registry(env) .unwrap_or_else(|err| panic_with_error!(env, err)) - .unwrap_or_else(|| panic_with_error!(env, CreditLineError::InvalidVendor)); + .ok_or(CreditLineError::InvalidVendor)?; let registry_client = VendorRegistryContractClient::new(env, &vendor_registry); let is_active = env @@ -255,18 +257,19 @@ impl CreditLineContract { &symbol_short!("is_active"), (vendor,).into_val(env), ) - .unwrap_or_else(|_| panic_with_error!(env, CreditLineError::VendorValidationFailed)) - .unwrap_or_else(|_| panic_with_error!(env, CreditLineError::VendorValidationFailed)); + .map_err(|_| CreditLineError::VendorValidationFailed)? + .map_err(|_| CreditLineError::VendorValidationFailed)?; if !is_active { - panic_with_error!(env, CreditLineError::VendorNotActive); + return Err(CreditLineError::VendorNotActive); } + Ok(()) } - fn validate_reputation(env: &Env, user: &Address) -> u32 { + fn validate_reputation(env: &Env, user: &Address) -> Result { let reputation_contract = storage::get_reputation_contract(env) .unwrap_or_else(|err| panic_with_error!(env, err)) - .unwrap_or_else(|| panic_with_error!(env, CreditLineError::ParametersUnavailable)); + .ok_or(CreditLineError::ParametersUnavailable)?; let score: u32 = env.invoke_contract( &reputation_contract, @@ -276,31 +279,34 @@ impl CreditLineContract { let params = Self::get_protocol_parameters(env); if score < params.min_reputation_threshold { - panic_with_error!(env, CreditLineError::InsufficientReputation); + return Err(CreditLineError::InsufficientReputation); } - score + Ok(score) } - fn validate_liquidity(env: &Env, total_amount: i128, guarantee_amount: i128) { + fn validate_liquidity( + env: &Env, + total_amount: i128, + guarantee_amount: i128, + ) -> Result<(), CreditLineError> { let liquidity_pool = storage::get_liquidity_pool(env) .unwrap_or_else(|err| panic_with_error!(env, err)) - .unwrap_or_else(|| panic_with_error!(env, CreditLineError::InsufficientLiquidity)); + .ok_or(CreditLineError::InsufficientLiquidity)?; - let required_from_pool = total_amount - .checked_sub(guarantee_amount) - .unwrap_or_else(|| panic_with_error!(env, CreditLineError::Underflow)); + let required_from_pool = safe_math::sub_i128(total_amount, guarantee_amount)?; if required_from_pool == 0 { - return; + return Ok(()); } let lp_client = LiquidityPoolContractClient::new(env, &liquidity_pool); let stats = lp_client.get_pool_stats(); if stats.available_liquidity < required_from_pool { - panic_with_error!(env, CreditLineError::InsufficientLiquidity); + return Err(CreditLineError::InsufficientLiquidity); } + Ok(()) } fn fund_loan_from_pool( @@ -338,33 +344,31 @@ impl CreditLineContract { score: u32, status: LoanStatus, loan_type: LoanType, - ) -> Loan { - Self::validate_guarantee(env, total_amount, guarantee_amount); - Self::validate_vendor(env, &vendor); + ) -> Result { + Self::validate_guarantee(env, total_amount, guarantee_amount)?; + Self::validate_vendor(env, &vendor)?; let interest_rate_bps = Self::interest_rate_bps(env, score); let interest_amount = - Self::calculate_bps_amount(env, total_amount, interest_rate_bps as i128); + Self::calculate_bps_amount(env, total_amount, interest_rate_bps as i128)?; let service_fee_amount = - Self::calculate_bps_amount(env, total_amount, types::SERVICE_FEE_BPS); - let remaining_balance = total_amount - .checked_add(interest_amount) - .and_then(|v| v.checked_add(service_fee_amount)) - .unwrap_or_else(|| panic_with_error!(env, CreditLineError::Overflow)); + Self::calculate_bps_amount(env, total_amount, types::SERVICE_FEE_BPS)?; + let remaining_balance = safe_math::add_i128( + safe_math::add_i128(total_amount, interest_amount)?, + service_fee_amount, + )?; let credit_limit = Self::credit_limit(score); let active_debt = storage::get_user_active_debt(env, &user) .unwrap_or_else(|err| panic_with_error!(env, err)); - let next_debt = active_debt - .checked_add(remaining_balance) - .unwrap_or_else(|| panic_with_error!(env, CreditLineError::Overflow)); + let next_debt = safe_math::add_i128(active_debt, remaining_balance)?; if next_debt > credit_limit { - panic_with_error!(env, CreditLineError::ExposureLimitExceeded); + return Err(CreditLineError::ExposureLimitExceeded); } let loan_id = storage::increment_loan_counter(env).unwrap_or_else(|err| panic_with_error!(env, err)); - Loan { + Ok(Loan { loan_id, borrower: user, vendor, @@ -384,13 +388,11 @@ impl CreditLineContract { funded_at: 0, late_fees_outstanding: 0, late_fee_accrual_timestamp: 0, - } + }) } - fn calculate_bps_amount(env: &Env, base: i128, bps: i128) -> i128 { - base.checked_mul(bps) - .and_then(|v| v.checked_div(types::BPS_DENOMINATOR)) - .unwrap_or_else(|| panic_with_error!(env, CreditLineError::Overflow)) + fn calculate_bps_amount(_env: &Env, base: i128, bps: i128) -> Result { + safe_math::div_i128(safe_math::mul_i128(base, bps)?, types::BPS_DENOMINATOR) } fn interest_rate_bps(env: &Env, score: u32) -> u32 { @@ -604,23 +606,27 @@ impl CreditLineContract { events::emit_loan_cancelled(&env, &loan.borrower, loan_id, loan.guarantee_amount); } - pub fn repay_loan(env: Env, borrower: Address, loan_id: u64, amount: i128) -> i128 { + pub fn repay_loan( + env: Env, + borrower: Address, + loan_id: u64, + amount: i128, + ) -> Result { borrower.require_auth(); - let mut loan = - storage::read_loan(&env, loan_id).unwrap_or_else(|err| panic_with_error!(&env, err)); + let mut loan = storage::read_loan(&env, loan_id)?; if loan.borrower != borrower { - panic_with_error!(&env, CreditLineError::UnauthorizedRepayer); + return Err(CreditLineError::UnauthorizedRepayer); } if loan.status != LoanStatus::Active { - panic_with_error!(&env, CreditLineError::LoanNotActive); + return Err(CreditLineError::LoanNotActive); } // Accrue any outstanding late fees before validating the payment amount so // the borrower repays the true current balance (principal + interest + fees + late fees). - let accrued_fee = Self::accrue_late_fees_internal(&env, &mut loan); + let accrued_fee = Self::accrue_late_fees_internal(&env, &mut loan)?; if accrued_fee > 0 { storage::increase_user_active_debt(&env, &borrower, accrued_fee) .unwrap_or_else(|err| panic_with_error!(&env, err)); @@ -634,47 +640,28 @@ impl CreditLineContract { } if amount <= 0 || amount > loan.remaining_balance { - panic_with_error!(&env, CreditLineError::InvalidRepaymentAmount); + return Err(CreditLineError::InvalidRepaymentAmount); } Self::enter_non_reentrant(&env); // Payment priority: principal → interest → service fee → late fees let principal_paid = amount.min(loan.principal_outstanding); - let after_principal = amount - .checked_sub(principal_paid) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); + let after_principal = safe_math::sub_i128(amount, principal_paid)?; let interest_paid = after_principal.min(loan.interest_outstanding); - let after_interest = after_principal - .checked_sub(interest_paid) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); + let after_interest = safe_math::sub_i128(after_principal, interest_paid)?; let fee_paid = after_interest.min(loan.service_fee_outstanding); - let after_fee = after_interest - .checked_sub(fee_paid) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); + let after_fee = safe_math::sub_i128(after_interest, fee_paid)?; let late_fee_paid = after_fee.min(loan.late_fees_outstanding); - loan.principal_outstanding = loan - .principal_outstanding - .checked_sub(principal_paid) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); - loan.interest_outstanding = loan - .interest_outstanding - .checked_sub(interest_paid) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); - loan.service_fee_outstanding = loan - .service_fee_outstanding - .checked_sub(fee_paid) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); - loan.late_fees_outstanding = loan - .late_fees_outstanding - .checked_sub(late_fee_paid) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); - - let new_balance = loan - .remaining_balance - .checked_sub(amount) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); + loan.principal_outstanding = + safe_math::sub_i128(loan.principal_outstanding, principal_paid)?; + loan.interest_outstanding = safe_math::sub_i128(loan.interest_outstanding, interest_paid)?; + loan.service_fee_outstanding = safe_math::sub_i128(loan.service_fee_outstanding, fee_paid)?; + loan.late_fees_outstanding = + safe_math::sub_i128(loan.late_fees_outstanding, late_fee_paid)?; + + let new_balance = safe_math::sub_i128(loan.remaining_balance, amount)?; loan.remaining_balance = new_balance; let is_fully_repaid = new_balance == 0; @@ -686,25 +673,21 @@ impl CreditLineContract { .unwrap_or_else(|err| panic_with_error!(&env, err)); storage::write_loan(&env, &loan); - let lp_address = storage::get_liquidity_pool(&env) - .unwrap_or_else(|err| panic_with_error!(&env, err)) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::InsufficientLiquidity)); - let token_address = storage::get_token(&env) - .unwrap_or_else(|err| panic_with_error!(&env, err)) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::TokenNotConfigured)); + let lp_address = + storage::get_liquidity_pool(&env)?.ok_or(CreditLineError::InsufficientLiquidity)?; + let token_address = storage::get_token(&env)?.ok_or(CreditLineError::TokenNotConfigured)?; let token_client = token::Client::new(&env, &token_address); token_client.transfer(&borrower, &env.current_contract_address(), &amount); Self::authorize_token_transfer(&env, &token_address, &lp_address, amount); let lp_client = LiquidityPoolContractClient::new(&env, &lp_address); + let interest_fee_late = + safe_math::add_i128(safe_math::add_i128(interest_paid, fee_paid)?, late_fee_paid)?; lp_client.receive_repayment( &env.current_contract_address(), &principal_paid, - &interest_paid - .checked_add(fee_paid) - .and_then(|v| v.checked_add(late_fee_paid)) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Overflow)), + &interest_fee_late, ); if is_fully_repaid { @@ -725,9 +708,7 @@ impl CreditLineContract { ); if is_fully_repaid { - if let Some(reputation_contract) = storage::get_reputation_contract(&env) - .unwrap_or_else(|err| panic_with_error!(&env, err)) - { + if let Some(reputation_contract) = storage::get_reputation_contract(&env)? { let updater = env.current_contract_address(); let payment_date = env.ledger().timestamp(); let due_date = loan @@ -747,7 +728,7 @@ impl CreditLineContract { } Self::exit_non_reentrant(&env); - new_balance + Ok(new_balance) } /// Mark a single installment paid and reduce the loan's outstanding balance by `amount`. @@ -760,43 +741,39 @@ impl CreditLineContract { loan_id: u64, installment_index: u32, amount: i128, - ) -> i128 { + ) -> Result { borrower.require_auth(); - let mut loan = - storage::read_loan(&env, loan_id).unwrap_or_else(|err| panic_with_error!(&env, err)); + let mut loan = storage::read_loan(&env, loan_id)?; if loan.borrower != borrower { - panic_with_error!(&env, CreditLineError::UnauthorizedRepayer); + return Err(CreditLineError::UnauthorizedRepayer); } if loan.status != LoanStatus::Active { - panic_with_error!(&env, CreditLineError::LoanNotActive); + return Err(CreditLineError::LoanNotActive); } if installment_index >= loan.repayment_schedule.len() { - panic_with_error!(&env, CreditLineError::InvalidInstallmentIndex); + return Err(CreditLineError::InvalidInstallmentIndex); } let mut installment = loan .repayment_schedule .get(installment_index) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::InvalidInstallmentIndex)); + .ok_or(CreditLineError::InvalidInstallmentIndex)?; if installment.paid { - panic_with_error!(&env, CreditLineError::InstallmentAlreadyPaid); + return Err(CreditLineError::InstallmentAlreadyPaid); } if amount <= 0 || amount > loan.remaining_balance { - panic_with_error!(&env, CreditLineError::InvalidRepaymentAmount); + return Err(CreditLineError::InvalidRepaymentAmount); } Self::enter_non_reentrant(&env); - let new_balance = loan - .remaining_balance - .checked_sub(amount) - .unwrap_or_else(|| panic_with_error!(&env, CreditLineError::Underflow)); + let new_balance = safe_math::sub_i128(loan.remaining_balance, amount)?; loan.remaining_balance = new_balance; installment.paid = true; @@ -814,7 +791,7 @@ impl CreditLineContract { events::emit_installment_paid(&env, loan_id, installment_index, amount, new_balance); Self::exit_non_reentrant(&env); - new_balance + Ok(new_balance) } /// Accrue late fees for a loan and update the caller-supplied `loan` in place. @@ -825,7 +802,7 @@ impl CreditLineContract { /// carries over to the next accrual. /// /// Returns the newly accrued fee amount (0 if nothing was due). - fn accrue_late_fees_internal(env: &Env, loan: &mut Loan) -> i128 { + fn accrue_late_fees_internal(env: &Env, loan: &mut Loan) -> Result { let now = env.ledger().timestamp(); // Find the earliest overdue installment due date. @@ -847,7 +824,7 @@ impl CreditLineContract { let overdue_since = match overdue_since { Some(d) => d, - None => return 0, // no overdue installments + None => return Ok(0), // no overdue installments }; // Accrue from the later of (first overdue date, last accrual timestamp). @@ -860,41 +837,40 @@ impl CreditLineContract { }; if now <= accrual_start { - return 0; + return Ok(0); } - let seconds_elapsed = now - accrual_start; - let days_elapsed = (seconds_elapsed / types::SECONDS_PER_DAY) as i128; + let seconds_elapsed = safe_math::sub_u64(now, accrual_start)?; + let days_elapsed = + safe_math::div_i128(seconds_elapsed as i128, types::SECONDS_PER_DAY as i128)?; if days_elapsed == 0 { - return 0; // less than one full day has passed since last accrual + return Ok(0); // less than one full day has passed since last accrual } - let fee = loan - .remaining_balance - .checked_mul(types::LATE_FEE_BPS_PER_DAY) - .and_then(|v| v.checked_mul(days_elapsed)) - .and_then(|v| v.checked_div(types::BPS_DENOMINATOR)) - .unwrap_or(0); + let fee = safe_math::div_i128( + safe_math::mul_i128( + safe_math::mul_i128(loan.remaining_balance, types::LATE_FEE_BPS_PER_DAY)?, + days_elapsed, + )?, + types::BPS_DENOMINATOR, + ) + .unwrap_or(0); if fee == 0 { - return 0; + return Ok(0); } // Advance the accrual cursor by only complete days to avoid losing fractions. - loan.late_fee_accrual_timestamp = - accrual_start + (days_elapsed as u64) * types::SECONDS_PER_DAY; + loan.late_fee_accrual_timestamp = safe_math::add_u64( + accrual_start, + safe_math::mul_u64(days_elapsed as u64, types::SECONDS_PER_DAY)?, + )?; - loan.late_fees_outstanding = loan - .late_fees_outstanding - .checked_add(fee) - .unwrap_or(loan.late_fees_outstanding); - loan.remaining_balance = loan - .remaining_balance - .checked_add(fee) - .unwrap_or(loan.remaining_balance); + loan.late_fees_outstanding = safe_math::add_i128(loan.late_fees_outstanding, fee)?; + loan.remaining_balance = safe_math::add_i128(loan.remaining_balance, fee)?; - fee + Ok(fee) } /// Apply late fees to an active loan without requiring a repayment. @@ -902,18 +878,17 @@ impl CreditLineContract { /// Anyone may call this to trigger fee accrual on an overdue loan. Emits a /// `LOANLTFE` event when fees are accrued; is a no-op when no full day has /// elapsed since the last accrual or when no installment is overdue. - pub fn apply_late_fees(env: Env, loan_id: u64) { - let mut loan = - storage::read_loan(&env, loan_id).unwrap_or_else(|err| panic_with_error!(&env, err)); + pub fn apply_late_fees(env: Env, loan_id: u64) -> Result<(), CreditLineError> { + let mut loan = storage::read_loan(&env, loan_id)?; if loan.status != LoanStatus::Active { - panic_with_error!(&env, CreditLineError::LoanNotActive); + return Err(CreditLineError::LoanNotActive); } - let accrued_fee = Self::accrue_late_fees_internal(&env, &mut loan); + let accrued_fee = Self::accrue_late_fees_internal(&env, &mut loan)?; if accrued_fee == 0 { - return; + return Ok(()); } storage::increase_user_active_debt(&env, &loan.borrower, accrued_fee) @@ -926,6 +901,7 @@ impl CreditLineContract { accrued_fee, loan.remaining_balance, ); + Ok(()) } fn handle_reputation_increase( diff --git a/contracts/creditline-contract/src/safe_math.rs b/contracts/creditline-contract/src/safe_math.rs new file mode 100644 index 0000000..a40359b --- /dev/null +++ b/contracts/creditline-contract/src/safe_math.rs @@ -0,0 +1,33 @@ +use crate::errors::CreditLineError; + +pub fn add_i128(a: i128, b: i128) -> Result { + a.checked_add(b).ok_or(CreditLineError::Overflow) +} + +pub fn sub_i128(a: i128, b: i128) -> Result { + a.checked_sub(b).ok_or(CreditLineError::Underflow) +} + +pub fn mul_i128(a: i128, b: i128) -> Result { + a.checked_mul(b).ok_or(CreditLineError::Overflow) +} + +pub fn div_i128(a: i128, b: i128) -> Result { + a.checked_div(b).ok_or(CreditLineError::Overflow) +} + +pub fn add_u64(a: u64, b: u64) -> Result { + a.checked_add(b).ok_or(CreditLineError::Overflow) +} + +pub fn sub_u64(a: u64, b: u64) -> Result { + a.checked_sub(b).ok_or(CreditLineError::Underflow) +} + +pub fn mul_u64(a: u64, b: u64) -> Result { + a.checked_mul(b).ok_or(CreditLineError::Overflow) +} + +pub fn div_u64(a: u64, b: u64) -> Result { + a.checked_div(b).ok_or(CreditLineError::Overflow) +} diff --git a/contracts/creditline-contract/src/tests.rs b/contracts/creditline-contract/src/tests.rs index 5f5dd6b..f7d8327 100644 --- a/contracts/creditline-contract/src/tests.rs +++ b/contracts/creditline-contract/src/tests.rs @@ -3044,3 +3044,21 @@ fn test_repay_installment_zero_amount_rejected() { // Zero-amount payment must be rejected t.client.repay_installment(&user, &loan_id, &0, &0); } + +#[test] +fn test_safe_math_boundaries() { + use crate::safe_math; + // 0 cases + assert_eq!(safe_math::add_i128(0, 0), Ok(0)); + assert_eq!(safe_math::sub_i128(0, 0), Ok(0)); + assert_eq!(safe_math::mul_i128(0, 0), Ok(0)); + assert_eq!(safe_math::div_i128(0, 5), Ok(0)); + + // max cases + let max = i128::MAX; + let min = i128::MIN; + assert_eq!(safe_math::add_i128(max, 1), Err(CreditLineError::Overflow)); + assert_eq!(safe_math::sub_i128(min, 1), Err(CreditLineError::Underflow)); + assert_eq!(safe_math::mul_i128(max, 2), Err(CreditLineError::Overflow)); + assert_eq!(safe_math::div_i128(max, 0), Err(CreditLineError::Overflow)); +} diff --git a/contracts/liquidity-pool-contract/src/lib.rs b/contracts/liquidity-pool-contract/src/lib.rs index fb5d68a..5947cbd 100644 --- a/contracts/liquidity-pool-contract/src/lib.rs +++ b/contracts/liquidity-pool-contract/src/lib.rs @@ -3,6 +3,7 @@ use soroban_sdk::{contract, contractimpl, panic_with_error, token, Address, Env} mod errors; mod events; +mod safe_math; mod storage; mod types; @@ -85,17 +86,18 @@ impl LiquidityPoolContract { // LP Operations // ------------------------------------------------------------------------- + /// Deposit `amount` tokens and receive shares representing pool ownership. /// Deposit `amount` tokens and receive shares representing pool ownership. /// /// **First deposit**: shares issued == amount (1:1 ratio). /// **Subsequent deposits**: `shares = (amount × total_shares) / total_pool_value` /// /// Returns the number of shares issued. - pub fn deposit(env: Env, provider: Address, amount: i128) -> i128 { + pub fn deposit(env: Env, provider: Address, amount: i128) -> Result { provider.require_auth(); if amount < types::MIN_AMOUNT { - panic_with_error!(&env, LiquidityPoolError::InvalidAmount); + return Err(LiquidityPoolError::InvalidAmount); } Self::enter_non_reentrant(&env); @@ -112,31 +114,25 @@ impl LiquidityPoolContract { amount } else { // Subsequent deposits: proportional to current pool value - amount - .checked_mul(total_shares) - .and_then(|v| v.checked_div(total_liquidity)) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Overflow)) + safe_math::div_i128(safe_math::mul_i128(amount, total_shares)?, total_liquidity)? }; if shares_issued <= 0 { - panic_with_error!(&env, LiquidityPoolError::InvalidAmount); + return Err(LiquidityPoolError::InvalidAmount); } // Update state - let new_shares = storage::get_lp_shares(&env, &provider) - .unwrap_or_else(|err| panic_with_error!(&env, err)) - .checked_add(shares_issued) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Overflow)); + let new_shares = safe_math::add_i128( + storage::get_lp_shares(&env, &provider) + .unwrap_or_else(|err| panic_with_error!(&env, err)), + shares_issued, + )?; storage::set_lp_shares(&env, &provider, new_shares); - let new_total_shares = total_shares - .checked_add(shares_issued) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Overflow)); + let new_total_shares = safe_math::add_i128(total_shares, shares_issued)?; storage::set_total_shares(&env, new_total_shares); - let new_total_liquidity = total_liquidity - .checked_add(amount) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Overflow)); + let new_total_liquidity = safe_math::add_i128(total_liquidity, amount)?; storage::set_total_liquidity(&env, new_total_liquidity); // Transfer tokens from provider to pool contract after state effects. @@ -146,7 +142,7 @@ impl LiquidityPoolContract { events::emit_liquidity_deposited(&env, &provider, amount, shares_issued); Self::exit_non_reentrant(&env); - shares_issued + Ok(shares_issued) } /// Burn `shares` and return the proportional token amount to `provider`. @@ -154,11 +150,11 @@ impl LiquidityPoolContract { /// `amount = (shares × total_pool_value) / total_shares` /// /// Returns the number of tokens returned. - pub fn withdraw(env: Env, provider: Address, shares: i128) -> i128 { + pub fn withdraw(env: Env, provider: Address, shares: i128) -> Result { provider.require_auth(); if shares < types::MIN_AMOUNT { - panic_with_error!(&env, LiquidityPoolError::InvalidAmount); + return Err(LiquidityPoolError::InvalidAmount); } Self::enter_non_reentrant(&env); @@ -166,47 +162,37 @@ impl LiquidityPoolContract { let provider_shares = storage::get_lp_shares(&env, &provider) .unwrap_or_else(|err| panic_with_error!(&env, err)); if provider_shares < shares { - panic_with_error!(&env, LiquidityPoolError::InsufficientShares); + return Err(LiquidityPoolError::InsufficientShares); } let total_shares = storage::get_total_shares(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); if total_shares == 0 { - panic_with_error!(&env, LiquidityPoolError::ZeroTotalShares); + return Err(LiquidityPoolError::ZeroTotalShares); } let total_liquidity = storage::get_total_liquidity(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); let locked_liquidity = storage::get_locked_liquidity(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); - let available_liquidity = total_liquidity - .checked_sub(locked_liquidity) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Underflow)); + let available_liquidity = safe_math::sub_i128(total_liquidity, locked_liquidity)?; // Calculate withdrawal amount proportionally - let amount_returned = shares - .checked_mul(total_liquidity) - .and_then(|v| v.checked_div(total_shares)) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Overflow)); + let amount_returned = + safe_math::div_i128(safe_math::mul_i128(shares, total_liquidity)?, total_shares)?; if amount_returned > available_liquidity { - panic_with_error!(&env, LiquidityPoolError::InsufficientLiquidity); + return Err(LiquidityPoolError::InsufficientLiquidity); } // Burn shares - let new_provider_shares = provider_shares - .checked_sub(shares) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Underflow)); + let new_provider_shares = safe_math::sub_i128(provider_shares, shares)?; storage::set_lp_shares(&env, &provider, new_provider_shares); - let new_total_shares = total_shares - .checked_sub(shares) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Underflow)); + let new_total_shares = safe_math::sub_i128(total_shares, shares)?; storage::set_total_shares(&env, new_total_shares); - let new_total_liquidity = total_liquidity - .checked_sub(amount_returned) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Underflow)); + let new_total_liquidity = safe_math::sub_i128(total_liquidity, amount_returned)?; storage::set_total_liquidity(&env, new_total_liquidity); events::emit_liquidity_withdrawn(&env, &provider, shares, amount_returned); @@ -216,7 +202,7 @@ impl LiquidityPoolContract { token_client.transfer(&env.current_contract_address(), &provider, &amount_returned); Self::exit_non_reentrant(&env); - amount_returned + Ok(amount_returned) } // ------------------------------------------------------------------------- @@ -225,12 +211,17 @@ impl LiquidityPoolContract { /// Transfer `amount` tokens to `merchant` to fund a loan. /// Only the registered CreditLine contract may call this. - pub fn fund_loan(env: Env, creditline: Address, merchant: Address, amount: i128) { + pub fn fund_loan( + env: Env, + creditline: Address, + merchant: Address, + amount: i128, + ) -> Result<(), LiquidityPoolError> { creditline.require_auth(); Self::require_creditline(&env, &creditline); if amount <= 0 { - panic_with_error!(&env, LiquidityPoolError::InvalidAmount); + return Err(LiquidityPoolError::InvalidAmount); } Self::enter_non_reentrant(&env); @@ -239,17 +230,13 @@ impl LiquidityPoolContract { storage::get_total_liquidity(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); let locked_liquidity = storage::get_locked_liquidity(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); - let available = total_liquidity - .checked_sub(locked_liquidity) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Underflow)); + let available = safe_math::sub_i128(total_liquidity, locked_liquidity)?; if amount > available { - panic_with_error!(&env, LiquidityPoolError::InsufficientLiquidity); + return Err(LiquidityPoolError::InsufficientLiquidity); } - let new_locked = locked_liquidity - .checked_add(amount) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Overflow)); + let new_locked = safe_math::add_i128(locked_liquidity, amount)?; storage::set_locked_liquidity(&env, new_locked); // Transfer tokens from pool to merchant after accounting has been updated. @@ -259,35 +246,37 @@ impl LiquidityPoolContract { events::emit_loan_funded(&env, &creditline, amount); Self::exit_non_reentrant(&env); + Ok(()) } /// Receive a loan repayment (principal + interest) from CreditLine. /// /// `principal` reduces locked_liquidity (loan is repaid). /// `interest` is distributed via `distribute_interest` (increases pool value). - pub fn receive_repayment(env: Env, creditline: Address, principal: i128, interest: i128) { + pub fn receive_repayment( + env: Env, + creditline: Address, + principal: i128, + interest: i128, + ) -> Result<(), LiquidityPoolError> { creditline.require_auth(); Self::require_creditline(&env, &creditline); if principal < 0 || interest < 0 { - panic_with_error!(&env, LiquidityPoolError::InvalidAmount); + return Err(LiquidityPoolError::InvalidAmount); } - let total = principal - .checked_add(interest) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Overflow)); + let total = safe_math::add_i128(principal, interest)?; if total <= 0 { - panic_with_error!(&env, LiquidityPoolError::InvalidAmount); + return Err(LiquidityPoolError::InvalidAmount); } Self::enter_non_reentrant(&env); // Decrease locked liquidity by the principal let locked = storage::get_locked_liquidity(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); - let new_locked = locked - .checked_sub(principal) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Underflow)); + let new_locked = safe_math::sub_i128(locked, principal)?; storage::set_locked_liquidity(&env, new_locked); // Pull funds from CreditLine after accounting changes. @@ -298,20 +287,25 @@ impl LiquidityPoolContract { events::emit_repayment_received(&env, &creditline, principal, interest); if interest > 0 { - Self::distribute_interest_internal(&env, interest); + Self::distribute_interest_internal(&env, interest)?; } Self::exit_non_reentrant(&env); + Ok(()) } /// Receive a forfeited guarantee on loan default. /// The amount offsets the loss: it is added back to total_liquidity /// and reduces locked_liquidity by the same amount (partial recovery). - pub fn receive_guarantee(env: Env, creditline: Address, amount: i128) { + pub fn receive_guarantee( + env: Env, + creditline: Address, + amount: i128, + ) -> Result<(), LiquidityPoolError> { creditline.require_auth(); Self::require_creditline(&env, &creditline); if amount <= 0 { - panic_with_error!(&env, LiquidityPoolError::InvalidAmount); + return Err(LiquidityPoolError::InvalidAmount); } Self::enter_non_reentrant(&env); @@ -321,16 +315,12 @@ impl LiquidityPoolContract { let locked = storage::get_locked_liquidity(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); let recovered = amount.min(locked); // can't recover more than locked - let new_locked = locked - .checked_sub(recovered) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Underflow)); + let new_locked = safe_math::sub_i128(locked, recovered)?; storage::set_locked_liquidity(&env, new_locked); let total_liquidity = storage::get_total_liquidity(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); - let new_total = total_liquidity - .checked_add(recovered) - .unwrap_or_else(|| panic_with_error!(&env, LiquidityPoolError::Overflow)); + let new_total = safe_math::add_i128(total_liquidity, recovered)?; storage::set_total_liquidity(&env, new_total); let token = storage::get_token(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); @@ -339,12 +329,9 @@ impl LiquidityPoolContract { events::emit_guarantee_received(&env, &creditline, amount); Self::exit_non_reentrant(&env); + Ok(()) } - // ------------------------------------------------------------------------- - // Interest Distribution (SC-17 core feature) - // ------------------------------------------------------------------------- - /// Distribute `interest_amount` according to the protocol fee split: /// - 85 % → Liquidity Providers (increases share value by raising `total_liquidity`) /// - 10 % → Protocol Treasury @@ -355,15 +342,19 @@ impl LiquidityPoolContract { /// /// This function is called internally by `receive_repayment`, but it is also /// `pub` so that the CreditLine (or admin, in edge-case) can call it directly. - pub fn distribute_interest(env: &Env, interest_amount: i128) { - Self::enter_non_reentrant(env); - Self::distribute_interest_internal(env, interest_amount); - Self::exit_non_reentrant(env); + pub fn distribute_interest(env: Env, interest_amount: i128) -> Result<(), LiquidityPoolError> { + Self::enter_non_reentrant(&env); + let res = Self::distribute_interest_internal(&env, interest_amount); + Self::exit_non_reentrant(&env); + res } - fn distribute_interest_internal(env: &Env, interest_amount: i128) { + fn distribute_interest_internal( + env: &Env, + interest_amount: i128, + ) -> Result<(), LiquidityPoolError> { if interest_amount <= 0 { - panic_with_error!(env, LiquidityPoolError::InvalidAmount); + return Err(LiquidityPoolError::InvalidAmount); } debug_assert_eq!( types::LP_FEE_BPS + types::PROTOCOL_FEE_BPS + types::MERCHANT_FEE_BPS, @@ -371,22 +362,22 @@ impl LiquidityPoolContract { ); // 85% stays in the pool → increases share value - let lp_amount = interest_amount - .checked_mul(types::LP_FEE_BPS) - .and_then(|v| v.checked_div(types::TOTAL_BPS)) - .unwrap_or_else(|| panic_with_error!(env, LiquidityPoolError::Overflow)); + let lp_amount = safe_math::div_i128( + safe_math::mul_i128(interest_amount, types::LP_FEE_BPS)?, + types::TOTAL_BPS, + )?; // 10% → treasury - let protocol_amount = interest_amount - .checked_mul(types::PROTOCOL_FEE_BPS) - .and_then(|v| v.checked_div(types::TOTAL_BPS)) - .unwrap_or_else(|| panic_with_error!(env, LiquidityPoolError::Overflow)); + let protocol_amount = safe_math::div_i128( + safe_math::mul_i128(interest_amount, types::PROTOCOL_FEE_BPS)?, + types::TOTAL_BPS, + )?; // 5% → merchant fund (use remainder to avoid rounding dust) - let merchant_amount = interest_amount - .checked_sub(lp_amount) - .and_then(|v| v.checked_sub(protocol_amount)) - .unwrap_or_else(|| panic_with_error!(env, LiquidityPoolError::Underflow)); + let merchant_amount = safe_math::sub_i128( + safe_math::sub_i128(interest_amount, lp_amount)?, + protocol_amount, + )?; let token = storage::get_token(env).unwrap_or_else(|err| panic_with_error!(env, err)); let token_client = token::Client::new(env, &token); @@ -419,9 +410,7 @@ impl LiquidityPoolContract { // Update total_liquidity to reflect the added interest (raises share price). let total_liquidity = storage::get_total_liquidity(env).unwrap_or_else(|err| panic_with_error!(env, err)); - let new_total = total_liquidity - .checked_add(lp_amount) - .unwrap_or_else(|| panic_with_error!(env, LiquidityPoolError::Overflow)); + let new_total = safe_math::add_i128(total_liquidity, lp_amount)?; storage::set_total_liquidity(env, new_total); events::emit_interest_distributed( @@ -431,6 +420,7 @@ impl LiquidityPoolContract { protocol_amount, merchant_amount, ); + Ok(()) } // ------------------------------------------------------------------------- @@ -450,10 +440,11 @@ impl LiquidityPoolContract { let share_price = if total_shares == 0 { types::TOTAL_BPS // Default: 1.00 expressed as 10000 bps } else { - total_liquidity - .checked_mul(types::TOTAL_BPS) - .and_then(|v| v.checked_div(total_shares)) - .unwrap_or(types::TOTAL_BPS) + safe_math::div_i128( + safe_math::mul_i128(total_liquidity, types::TOTAL_BPS).unwrap_or(0), + total_shares, + ) + .unwrap_or(types::TOTAL_BPS) }; PoolStats { @@ -478,10 +469,11 @@ impl LiquidityPoolContract { } let total_liquidity = storage::get_total_liquidity(&env).unwrap_or_else(|err| panic_with_error!(&env, err)); - shares - .checked_mul(total_liquidity) - .and_then(|v| v.checked_div(total_shares)) - .unwrap_or(0) + safe_math::div_i128( + safe_math::mul_i128(shares, total_liquidity).unwrap_or(0), + total_shares, + ) + .unwrap_or(0) } // ------------------------------------------------------------------------- diff --git a/contracts/liquidity-pool-contract/src/safe_math.rs b/contracts/liquidity-pool-contract/src/safe_math.rs new file mode 100644 index 0000000..5673e4d --- /dev/null +++ b/contracts/liquidity-pool-contract/src/safe_math.rs @@ -0,0 +1,17 @@ +use crate::errors::LiquidityPoolError; + +pub fn add_i128(a: i128, b: i128) -> Result { + a.checked_add(b).ok_or(LiquidityPoolError::Overflow) +} + +pub fn sub_i128(a: i128, b: i128) -> Result { + a.checked_sub(b).ok_or(LiquidityPoolError::Underflow) +} + +pub fn mul_i128(a: i128, b: i128) -> Result { + a.checked_mul(b).ok_or(LiquidityPoolError::Overflow) +} + +pub fn div_i128(a: i128, b: i128) -> Result { + a.checked_div(b).ok_or(LiquidityPoolError::Overflow) +} diff --git a/contracts/parameters-contract/src/errors.rs b/contracts/parameters-contract/src/errors.rs index 8e34f1f..0ad7110 100644 --- a/contracts/parameters-contract/src/errors.rs +++ b/contracts/parameters-contract/src/errors.rs @@ -9,4 +9,6 @@ pub enum ParametersError { InvalidParameters = 3, NotInitialized = 4, ReentrancyDetected = 5, + Overflow = 6, + Underflow = 7, } diff --git a/contracts/parameters-contract/src/lib.rs b/contracts/parameters-contract/src/lib.rs index fa687c7..00ecf6e 100644 --- a/contracts/parameters-contract/src/lib.rs +++ b/contracts/parameters-contract/src/lib.rs @@ -3,6 +3,7 @@ mod access; mod errors; mod events; +mod safe_math; mod storage; mod types; @@ -81,9 +82,7 @@ impl ParametersContract { } fn enter_non_reentrant(env: &Env) { - if storage::is_reentrancy_locked(env) - .unwrap_or_else(|err| panic_with_error!(env, err)) - { + if storage::is_reentrancy_locked(env).unwrap_or_else(|err| panic_with_error!(env, err)) { panic_with_error!(env, ParametersError::ReentrancyDetected); } storage::set_reentrancy_locked(env, true); diff --git a/contracts/parameters-contract/src/safe_math.rs b/contracts/parameters-contract/src/safe_math.rs new file mode 100644 index 0000000..a53d229 --- /dev/null +++ b/contracts/parameters-contract/src/safe_math.rs @@ -0,0 +1,17 @@ +use crate::errors::ParametersError; + +pub fn add_i128(a: i128, b: i128) -> Result { + a.checked_add(b).ok_or(ParametersError::Overflow) +} + +pub fn sub_i128(a: i128, b: i128) -> Result { + a.checked_sub(b).ok_or(ParametersError::Underflow) +} + +pub fn mul_i128(a: i128, b: i128) -> Result { + a.checked_mul(b).ok_or(ParametersError::Overflow) +} + +pub fn div_i128(a: i128, b: i128) -> Result { + a.checked_div(b).ok_or(ParametersError::Overflow) +} diff --git a/contracts/parameters-contract/src/tests.rs b/contracts/parameters-contract/src/tests.rs index 6580f47..c31f68e 100644 --- a/contracts/parameters-contract/src/tests.rs +++ b/contracts/parameters-contract/src/tests.rs @@ -2,11 +2,7 @@ use crate::{ default_parameters, ParametersContract, ParametersContractClient, ParametersError, ProtocolParameters, }; -use soroban_sdk::{ - contract, contractimpl, - testutils::Address as _, - Address, Env, Symbol, -}; +use soroban_sdk::{testutils::Address as _, Address, Env}; fn setup() -> (Env, ParametersContractClient<'static>, Address) { let env = Env::default(); diff --git a/contracts/reputation-contract/src/lib.rs b/contracts/reputation-contract/src/lib.rs index 112e97d..0e6e33a 100644 --- a/contracts/reputation-contract/src/lib.rs +++ b/contracts/reputation-contract/src/lib.rs @@ -5,6 +5,7 @@ use soroban_sdk::{contract, contractimpl, symbol_short, Address, Env, Symbol}; mod access; mod errors; mod events; +mod safe_math; mod storage; mod types; @@ -31,7 +32,12 @@ impl ReputationContract { /// Increase a user's reputation score by a given amount /// Requires authorization from an updater - pub fn increase_score(env: Env, updater: Address, user: Address, amount: u32) { + pub fn increase_score( + env: Env, + updater: Address, + user: Address, + amount: u32, + ) -> Result<(), ReputationError> { updater.require_auth(); access::require_updater(&env, &updater); @@ -39,13 +45,10 @@ impl ReputationContract { let old_score = storage::read_score(&env, &user) .unwrap_or_else(|err| soroban_sdk::panic_with_error!(&env, err)); - let new_score = old_score - .checked_add(amount) - .ok_or(ReputationError::Overflow) - .unwrap(); + let new_score = safe_math::add_u32(old_score, amount)?; if new_score > types::MAX_SCORE { - soroban_sdk::panic_with_error!(&env, ReputationError::Overflow); + return Err(ReputationError::Overflow); } storage::write_score(&env, &user, new_score); @@ -54,11 +57,17 @@ impl ReputationContract { events::emit_score_changed(&env, &user, old_score, new_score, &reason); Self::exit_non_reentrant(&env); + Ok(()) } /// Decrease a user's reputation score by a given amount /// Requires authorization from an updater - pub fn decrease_score(env: Env, updater: Address, user: Address, amount: u32) { + pub fn decrease_score( + env: Env, + updater: Address, + user: Address, + amount: u32, + ) -> Result<(), ReputationError> { updater.require_auth(); access::require_updater(&env, &updater); @@ -66,10 +75,7 @@ impl ReputationContract { let old_score = storage::read_score(&env, &user) .unwrap_or_else(|err| soroban_sdk::panic_with_error!(&env, err)); - let new_score = match old_score.checked_sub(amount) { - Some(score) => score, - None => soroban_sdk::panic_with_error!(&env, ReputationError::Underflow), - }; + let new_score = safe_math::sub_u32(old_score, amount)?; storage::write_score(&env, &user, new_score); @@ -77,6 +83,7 @@ impl ReputationContract { events::emit_score_changed(&env, &user, old_score, new_score, &reason); Self::exit_non_reentrant(&env); + Ok(()) } /// Set a user's reputation score to a specific value @@ -103,7 +110,12 @@ impl ReputationContract { /// Add a mentor vouching boost to a user's reputation score. /// Requires authorization from an updater. - pub fn add_boost(env: Env, updater: Address, user: Address, amount: u32) { + pub fn add_boost( + env: Env, + updater: Address, + user: Address, + amount: u32, + ) -> Result<(), ReputationError> { updater.require_auth(); access::require_updater(&env, &updater); @@ -111,13 +123,10 @@ impl ReputationContract { let old_score = storage::read_score(&env, &user) .unwrap_or_else(|err| soroban_sdk::panic_with_error!(&env, err)); - let new_score = old_score - .checked_add(amount) - .ok_or(ReputationError::Overflow) - .unwrap(); + let new_score = safe_math::add_u32(old_score, amount)?; if new_score > types::MAX_SCORE { - soroban_sdk::panic_with_error!(&env, ReputationError::Overflow); + return Err(ReputationError::Overflow); } storage::write_score(&env, &user, new_score); @@ -126,11 +135,17 @@ impl ReputationContract { events::emit_score_changed(&env, &user, old_score, new_score, &reason); Self::exit_non_reentrant(&env); + Ok(()) } /// Remove a mentor vouching boost from a user's reputation score. /// Requires authorization from an updater. - pub fn remove_boost(env: Env, updater: Address, user: Address, amount: u32) { + pub fn remove_boost( + env: Env, + updater: Address, + user: Address, + amount: u32, + ) -> Result<(), ReputationError> { updater.require_auth(); access::require_updater(&env, &updater); @@ -138,10 +153,7 @@ impl ReputationContract { let old_score = storage::read_score(&env, &user) .unwrap_or_else(|err| soroban_sdk::panic_with_error!(&env, err)); - let new_score = match old_score.checked_sub(amount) { - Some(score) => score, - None => soroban_sdk::panic_with_error!(&env, ReputationError::Underflow), - }; + let new_score = safe_math::sub_u32(old_score, amount)?; storage::write_score(&env, &user, new_score); @@ -149,6 +161,7 @@ impl ReputationContract { events::emit_score_changed(&env, &user, old_score, new_score, &reason); Self::exit_non_reentrant(&env); + Ok(()) } /// Set or remove an address as an authorized updater diff --git a/contracts/reputation-contract/src/safe_math.rs b/contracts/reputation-contract/src/safe_math.rs new file mode 100644 index 0000000..fe5c159 --- /dev/null +++ b/contracts/reputation-contract/src/safe_math.rs @@ -0,0 +1,9 @@ +use crate::errors::ReputationError; + +pub fn add_u32(a: u32, b: u32) -> Result { + a.checked_add(b).ok_or(ReputationError::Overflow) +} + +pub fn sub_u32(a: u32, b: u32) -> Result { + a.checked_sub(b).ok_or(ReputationError::Underflow) +} diff --git a/contracts/vendor-registry-contract/src/errors.rs b/contracts/vendor-registry-contract/src/errors.rs index 330ad19..2366651 100644 --- a/contracts/vendor-registry-contract/src/errors.rs +++ b/contracts/vendor-registry-contract/src/errors.rs @@ -12,4 +12,5 @@ pub enum Error { Unauthorized = 6, Overflow = 7, ReentrancyDetected = 8, + Underflow = 9, } diff --git a/contracts/vendor-registry-contract/src/lib.rs b/contracts/vendor-registry-contract/src/lib.rs index 7e5056c..e88029a 100644 --- a/contracts/vendor-registry-contract/src/lib.rs +++ b/contracts/vendor-registry-contract/src/lib.rs @@ -3,6 +3,7 @@ mod access; mod errors; mod events; +mod safe_math; mod storage; mod types; diff --git a/contracts/vendor-registry-contract/src/safe_math.rs b/contracts/vendor-registry-contract/src/safe_math.rs new file mode 100644 index 0000000..176d97b --- /dev/null +++ b/contracts/vendor-registry-contract/src/safe_math.rs @@ -0,0 +1,9 @@ +use crate::errors::Error; + +pub fn add_u64(a: u64, b: u64) -> Result { + a.checked_add(b).ok_or(Error::Overflow) +} + +pub fn sub_u64(a: u64, b: u64) -> Result { + a.checked_sub(b).ok_or(Error::Underflow) +} diff --git a/contracts/vendor-registry-contract/src/tests.rs b/contracts/vendor-registry-contract/src/tests.rs index f607eac..f2c9265 100644 --- a/contracts/vendor-registry-contract/src/tests.rs +++ b/contracts/vendor-registry-contract/src/tests.rs @@ -1,5 +1,3 @@ -#![cfg(test)] - use super::*; use soroban_sdk::{ testutils::{Address as _, Ledger}, @@ -66,7 +64,7 @@ fn test_registration_flow() { let info = client.get_vendor_info(&vendor); assert_eq!(info.name, name); assert_eq!(info.registration_date, 1000000); - assert_eq!(info.active, true); + assert!(info.active); assert_eq!(info.total_sales, 0); assert_eq!(client.get_vendor_count(), 1); } @@ -114,12 +112,12 @@ fn test_activation_and_deactivation() { // Deactivate vendor client.deactivate_vendor(&admin, &vendor); assert!(!client.is_active(&vendor)); - assert_eq!(client.get_vendor_info(&vendor).active, false); + assert!(!client.get_vendor_info(&vendor).active); // Activate vendor client.activate_vendor(&admin, &vendor); assert!(client.is_active(&vendor)); - assert_eq!(client.get_vendor_info(&vendor).active, true); + assert!(client.get_vendor_info(&vendor).active); } #[test] @@ -165,9 +163,7 @@ fn test_reentrancy_guard_on_register_vendor() { // Lock the contract env.as_contract(&contract_id, || { - env.storage() - .instance() - .set(&types::DataKey::Locked, &true); + env.storage().instance().set(&types::DataKey::Locked, &true); }); let name = String::from_str(&env, "Test"); @@ -190,9 +186,7 @@ fn test_reentrancy_guard_on_deactivate_vendor() { client.register_vendor(&admin, &vendor, &name); env.as_contract(&contract_id, || { - env.storage() - .instance() - .set(&types::DataKey::Locked, &true); + env.storage().instance().set(&types::DataKey::Locked, &true); }); let res = client.try_deactivate_vendor(&admin, &vendor); @@ -215,9 +209,7 @@ fn test_reentrancy_guard_on_activate_vendor() { client.deactivate_vendor(&admin, &vendor); env.as_contract(&contract_id, || { - env.storage() - .instance() - .set(&types::DataKey::Locked, &true); + env.storage().instance().set(&types::DataKey::Locked, &true); }); let res = client.try_activate_vendor(&admin, &vendor); @@ -239,9 +231,7 @@ fn test_reentrancy_guard_on_set_vendor_status() { client.register_vendor(&admin, &vendor, &name); env.as_contract(&contract_id, || { - env.storage() - .instance() - .set(&types::DataKey::Locked, &true); + env.storage().instance().set(&types::DataKey::Locked, &true); }); let res = client.try_set_vendor_status(&admin, &vendor, &false); @@ -256,15 +246,13 @@ fn test_reentrancy_guard_allows_normal_operations() { // Normal operations should still succeed let name = String::from_str(&env, "Test"); - assert!(client.register_vendor(&admin, &vendor, &name).is_ok()); + client.register_vendor(&admin, &vendor, &name); - assert!(client.deactivate_vendor(&admin, &vendor).is_ok()); + client.deactivate_vendor(&admin, &vendor); - assert!(client.activate_vendor(&admin, &vendor).is_ok()); + client.activate_vendor(&admin, &vendor); - assert!(client - .set_vendor_status(&admin, &vendor, &false) - .is_ok()); + client.set_vendor_status(&admin, &vendor, &false); } #[test] @@ -275,9 +263,9 @@ fn test_reentrancy_guard_is_released_after_call() { // First call should succeed let name = String::from_str(&env, "Test"); - assert!(client.register_vendor(&admin, &vendor, &name).is_ok()); + client.register_vendor(&admin, &vendor, &name); // Lock should be released, second call should also succeed - assert!(client.deactivate_vendor(&admin, &vendor).is_ok()); - assert!(client.activate_vendor(&admin, &vendor).is_ok()); + client.deactivate_vendor(&admin, &vendor); + client.activate_vendor(&admin, &vendor); } diff --git a/contracts/vouching-contract/src/errors.rs b/contracts/vouching-contract/src/errors.rs index 9c19a7b..68ddac3 100644 --- a/contracts/vouching-contract/src/errors.rs +++ b/contracts/vouching-contract/src/errors.rs @@ -14,4 +14,6 @@ pub enum VouchingError { InvalidBoost = 8, ReputationCallFailed = 9, ReentrancyDetected = 10, + Overflow = 11, + Underflow = 12, } diff --git a/contracts/vouching-contract/src/lib.rs b/contracts/vouching-contract/src/lib.rs index 985e375..e2feb80 100644 --- a/contracts/vouching-contract/src/lib.rs +++ b/contracts/vouching-contract/src/lib.rs @@ -8,6 +8,7 @@ use soroban_sdk::{ mod errors; mod events; +mod safe_math; mod storage; mod types; @@ -188,9 +189,7 @@ impl VouchingContract { } fn enter_non_reentrant(env: &Env) { - if storage::is_reentrancy_locked(env) - .unwrap_or_else(|err| panic_with_error!(env, err)) - { + if storage::is_reentrancy_locked(env).unwrap_or_else(|err| panic_with_error!(env, err)) { panic_with_error!(env, VouchingError::ReentrancyDetected); } storage::set_reentrancy_locked(env, true); diff --git a/contracts/vouching-contract/src/safe_math.rs b/contracts/vouching-contract/src/safe_math.rs new file mode 100644 index 0000000..a8fb30f --- /dev/null +++ b/contracts/vouching-contract/src/safe_math.rs @@ -0,0 +1,9 @@ +use crate::errors::VouchingError; + +pub fn add_u32(a: u32, b: u32) -> Result { + a.checked_add(b).ok_or(VouchingError::Overflow) +} + +pub fn sub_u32(a: u32, b: u32) -> Result { + a.checked_sub(b).ok_or(VouchingError::Underflow) +}