[#544] Make fee accounting and rounding invariant-safe

contracts/commitment_core/src/lib.rs

@@ -19,7 +19,7 @@
 //! [`docs/COMMITMENT_CORE_FORMAL_VERIFICATION_SCOPE.md`](../../../docs/COMMITMENT_CORE_FORMAL_VERIFICATION_SCOPE.md)
 
 use shared_utils::{
-    emit_error_event, fees, EmergencyControl, Pausable, RateLimiter, SafeMath, TimeUtils,
+    emit_error_event, fee_invariants, fees, EmergencyControl, Pausable, RateLimiter, SafeMath, TimeUtils,
     Validation,
 };
 use soroban_sdk::{
@@ -509,15 +509,12 @@ impl CommitmentCoreContract {
             .instance()
             .get(&DataKey::CreationFeeBps)
             .unwrap_or(0);
-        let creation_fee = if creation_fee_bps > 0 {
-            fees::fee_from_bps(amount, creation_fee_bps)
-        } else {
-            0
-        };
-        let net_amount = amount.checked_sub(creation_fee).unwrap_or_else(|| {
+        let creation_split = fee_invariants::split_bps(amount, creation_fee_bps).unwrap_or_else(|_| {
             set_reentrancy_guard(&e, false);
             fail(&e, CommitmentError::ArithmeticOverflow, "create");
         });
+        let creation_fee = creation_split.fee;
+        let net_amount = creation_split.net;
 
         let expires_at = TimeUtils::checked_calculate_expiration(&e, rules.duration_days)
             .unwrap_or_else(|| {
@@ -1168,11 +1165,16 @@ impl CommitmentCoreContract {
             fail(&e, CommitmentError::NotActive, "exit");
         }
 
-        let penalty = SafeMath::penalty_amount(
+        let penalty_split = fee_invariants::split_percent(
             commitment.current_value,
             commitment.rules.early_exit_penalty,
-        );
-        let returned = SafeMath::sub(commitment.current_value, penalty);
+        )
+        .unwrap_or_else(|_| {
+            set_reentrancy_guard(&e, false);
+            fail(&e, CommitmentError::ArithmeticOverflow, "exit");
+        });
+        let penalty = penalty_split.fee;
+        let returned = penalty_split.net;
         let original_val = commitment.current_value;
 
         // Add penalty to collected fees (protocol revenue)

contracts/commitment_marketplace/src/lib.rs

@@ -20,11 +20,11 @@
 
 #![no_std]
 
-use shared_utils::math::SafeMath;
 use soroban_sdk::{
     contract, contracterror, contractimpl, contracttype, symbol_short, token, Address, Env, Symbol,
     Vec,
 };
+use shared_utils::fee_invariants;
 
 // ============================================================================
 // Error Types
@@ -79,6 +79,8 @@ pub enum MarketplaceError {
     TransferFailed = 21,
     /// Payment token is not allowlisted for marketplace settlement
     PaymentTokenNotAllowed = 22,
+    /// Configured fee cannot be represented as a safe accounting split.
+    InvalidFeeConfiguration = 34,
     /// No administrator handover is currently pending
     NoPendingAdmin = 31,
     /// Caller is not the nominated administrator
@@ -274,14 +276,20 @@ fn calculate_sale_payouts(
     if royalty_bps > MAX_ROYALTY_BASIS_POINTS {
         return Err(MarketplaceError::RoyaltyTooHigh);
     }
-    let fee = sale_amount
-        .checked_mul(fee_basis_points as i128)
-        .ok_or(MarketplaceError::PayoutOverflow)?
-        / 10_000;
-    let royalty_amount = sale_amount
-        .checked_mul(royalty_bps as i128)
-        .ok_or(MarketplaceError::PayoutOverflow)?
-        / 10_000;
+    let fee_split = fee_invariants::split_bps(sale_amount, fee_basis_points).map_err(|error| {
+        match error {
+            fee_invariants::FeeError::InvalidRate => MarketplaceError::PayoutExceedsSale,
+            _ => MarketplaceError::PayoutOverflow,
+        }
+    })?;
+    let royalty_split = fee_invariants::split_bps(sale_amount, royalty_bps).map_err(|error| {
+        match error {
+            fee_invariants::FeeError::InvalidRate => MarketplaceError::RoyaltyTooHigh,
+            _ => MarketplaceError::PayoutOverflow,
+        }
+    })?;
+    let fee = fee_split.fee;
+    let royalty_amount = royalty_split.fee;
     let deductions = fee
         .checked_add(royalty_amount)
         .ok_or(MarketplaceError::PayoutOverflow)?;