fix: use checked creation fee math

contracts/commitment_core/src/fuzz_tests.rs

@@ -8,10 +8,8 @@ use crate::{
     CommitmentCoreContract, CommitmentCoreContractClient, CommitmentRules,
 };
 use soroban_sdk::{
-    contract, contractimpl,
-    testutils::Address as _,
-    token::StellarAssetClient,
-    Address, Env, String,
+    contract, contractimpl, testutils::Address as _, token::StellarAssetClient, Address, Env,
+    String,
 };
 
 #[contract]
@@ -121,13 +119,15 @@ fn test_create_commitment_rejects_fee_math_overflow() {
 
     let admin = Address::generate(&e);
     let owner = Address::generate(&e);
+    let second_owner = Address::generate(&e);
     let token_admin = Address::generate(&e);
     let amount = i128::MAX;
 
     let token_contract = e.register_stellar_asset_contract_v2(token_admin);
     let asset_address = token_contract.address();
     let token_admin_client = StellarAssetClient::new(&e, &asset_address);
     token_admin_client.mint(&owner, &amount);
+    token_admin_client.mint(&second_owner, &1_000);
 
     client.initialize(&admin, &nft_contract);
     client.set_creation_fee_bps(&admin, &2);
@@ -138,4 +138,10 @@ fn test_create_commitment_rejects_fee_math_overflow() {
     assert_eq!(client.get_total_commitments(), 0);
     assert_eq!(client.get_total_value_locked(), 0);
     assert_eq!(client.get_collected_fees(&asset_address), 0);
+
+    client.set_creation_fee_bps(&admin, &100);
+    assert!(client
+        .try_create_commitment(&second_owner, &1_000, &asset_address, &default_rules(&e))
+        .is_ok());
+    assert_eq!(client.get_total_commitments(), 1);
 }

contracts/commitment_core/src/lib.rs

@@ -317,7 +317,11 @@ fn require_authorized_updater(e: &Env, caller: &Address) {
         .get::<_, bool>(&DataKey::AuthorizedUpdater(caller.clone()))
         .unwrap_or(false)
     {
-        fail(e, CommitmentError::NotAuthorizedUpdater, "require_authorized_updater");
+        fail(
+            e,
+            CommitmentError::NotAuthorizedUpdater,
+            "require_authorized_updater",
+        );
     }
 }
 
@@ -504,11 +508,13 @@ 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
-        };
+        // Single source of truth for creation-fee math: use the checked helper so
+        // overflow routes through `ArithmeticOverflow` and clears the guard.
+        let creation_fee =
+            fuzzing::checked_fee_from_bps(amount, creation_fee_bps).unwrap_or_else(|| {
+                set_reentrancy_guard(&e, false);
+                fail(&e, CommitmentError::ArithmeticOverflow, "create");
+            });
         let net_amount = amount.checked_sub(creation_fee).unwrap_or_else(|| {
             set_reentrancy_guard(&e, false);
             fail(&e, CommitmentError::ArithmeticOverflow, "create");
@@ -585,7 +591,9 @@ impl CommitmentCoreContract {
             set_reentrancy_guard(&e, false);
             fail(&e, CommitmentError::ArithmeticOverflow, "create");
         });
-        e.storage().instance().set(&DataKey::TotalValueLocked, &updated_tvl);
+        e.storage()
+            .instance()
+            .set(&DataKey::TotalValueLocked, &updated_tvl);
 
         let mut all_ids = e
             .storage()
@@ -608,9 +616,7 @@ impl CommitmentCoreContract {
                 set_reentrancy_guard(&e, false);
                 fail(&e, CommitmentError::ArithmeticOverflow, "create");
             });
-            e.storage()
-                .instance()
-                .set(&fee_key, &updated_fees);
+            e.storage().instance().set(&fee_key, &updated_fees);
         }
 
         let nft_token_id = call_nft_mint(
@@ -775,10 +781,9 @@ impl CommitmentCoreContract {
     /// from commitments to target pools.
     pub fn add_allocator(e: Env, caller: Address, allocator: Address) {
         require_admin(&e, &caller);
-        e.storage().instance().set(
-            &DataKey::AuthorizedAllocator(allocator.clone()),
-            &true,
-        );
+        e.storage()
+            .instance()
+            .set(&DataKey::AuthorizedAllocator(allocator.clone()), &true);
         e.events().publish(
             (Symbol::new(&e, "AuthorizedAllocatorAdded"),),
             (allocator, e.ledger().timestamp()),
@@ -790,7 +795,9 @@ impl CommitmentCoreContract {
     /// Restricted to the Admin role.
     pub fn remove_allocator(e: Env, caller: Address, allocator: Address) {
         require_admin(&e, &caller);
-        e.storage().instance().remove(&DataKey::AuthorizedAllocator(allocator.clone()));
+        e.storage()
+            .instance()
+            .remove(&DataKey::AuthorizedAllocator(allocator.clone()));
         e.events().publish(
             (Symbol::new(&e, "AuthorizedAllocatorRemoved"),),
             (allocator, e.ledger().timestamp()),
@@ -814,10 +821,18 @@ impl CommitmentCoreContract {
     pub fn is_allocator(e: Env, address: Address) -> bool {
         let admin = e.storage().instance().get::<_, Address>(&DataKey::Admin);
         if let Some(a) = admin {
-            if address == a { return true; }
+            if address == a {
+                return true;
+            }
         }
-        if let Some(alloc_contract) = e.storage().instance().get::<_, Address>(&DataKey::AllocationContract) {
-            if address == alloc_contract { return true; }
+        if let Some(alloc_contract) = e
+            .storage()
+            .instance()
+            .get::<_, Address>(&DataKey::AllocationContract)
+        {
+            if address == alloc_contract {
+                return true;
+            }
         }
         e.storage()
             .instance()
@@ -852,7 +867,9 @@ impl CommitmentCoreContract {
     pub fn is_guardian(e: Env, address: Address) -> bool {
         let admin = e.storage().instance().get::<_, Address>(&DataKey::Admin);
         if let Some(a) = admin {
-            if address == a { return true; }
+            if address == a {
+                return true;
+            }
         }
         e.storage()
             .instance()
@@ -867,7 +884,9 @@ impl CommitmentCoreContract {
     pub fn is_treasurer(e: Env, address: Address) -> bool {
         let admin = e.storage().instance().get::<_, Address>(&DataKey::Admin);
         if let Some(a) = admin {
-            if address == a { return true; }
+            if address == a {
+                return true;
+            }
         }
         e.storage()
             .instance()
@@ -882,9 +901,14 @@ impl CommitmentCoreContract {
     pub fn is_operator(e: Env, address: Address) -> bool {
         let admin = e.storage().instance().get::<_, Address>(&DataKey::Admin);
         if let Some(a) = admin {
-            if address == a { return true; }
+            if address == a {
+                return true;
+            }
         }
-        e.storage().instance().get::<_, bool>(&DataKey::AuthorizedOperator(address)).unwrap_or(false)
+        e.storage()
+            .instance()
+            .get::<_, bool>(&DataKey::AuthorizedOperator(address))
+            .unwrap_or(false)
     }
 
     /// Update the current value of a commitment.
@@ -948,12 +972,18 @@ impl CommitmentCoreContract {
         set_commitment(&e, &commitment);
 
         // Update TVL by the delta so the aggregate stays consistent with the persisted value.
-        let tvl = e.storage().instance().get::<_, i128>(&DataKey::TotalValueLocked).unwrap_or(0);
+        let tvl = e
+            .storage()
+            .instance()
+            .get::<_, i128>(&DataKey::TotalValueLocked)
+            .unwrap_or(0);
         let updated_tvl = tvl
             .checked_sub(old_value)
             .and_then(|value| value.checked_add(new_value))
             .unwrap_or_else(|| fail(&e, CommitmentError::ArithmeticOverflow, "upd"));
-        e.storage().instance().set(&DataKey::TotalValueLocked, &updated_tvl);
+        e.storage()
+            .instance()
+            .set(&DataKey::TotalValueLocked, &updated_tvl);
     }
 
     pub fn check_violations(e: Env, commitment_id: String) -> bool {
@@ -1070,7 +1100,9 @@ impl CommitmentCoreContract {
         } else {
             0
         };
-        e.storage().instance().set(&DataKey::TotalValueLocked, &new_tvl);
+        e.storage()
+            .instance()
+            .set(&DataKey::TotalValueLocked, &new_tvl);
 
         transfer_assets(
             &e,
@@ -1230,33 +1262,45 @@ impl CommitmentCoreContract {
 
     pub fn add_guardian(e: Env, caller: Address, guardian: Address) {
         require_admin(&e, &caller);
-        e.storage().instance().set(&DataKey::AuthorizedGuardian(guardian), &true);
+        e.storage()
+            .instance()
+            .set(&DataKey::AuthorizedGuardian(guardian), &true);
     }
     pub fn remove_guardian(e: Env, caller: Address, guardian: Address) {
         require_admin(&e, &caller);
-        e.storage().instance().remove(&DataKey::AuthorizedGuardian(guardian));
+        e.storage()
+            .instance()
+            .remove(&DataKey::AuthorizedGuardian(guardian));
     }
     pub fn add_treasurer(e: Env, caller: Address, treasurer: Address) {
         require_admin(&e, &caller);
-        e.storage().instance().set(&DataKey::AuthorizedTreasurer(treasurer), &true);
+        e.storage()
+            .instance()
+            .set(&DataKey::AuthorizedTreasurer(treasurer), &true);
     }
     pub fn remove_treasurer(e: Env, caller: Address, treasurer: Address) {
         require_admin(&e, &caller);
-        e.storage().instance().remove(&DataKey::AuthorizedTreasurer(treasurer));
+        e.storage()
+            .instance()
+            .remove(&DataKey::AuthorizedTreasurer(treasurer));
     }
     pub fn add_operator(e: Env, caller: Address, operator: Address) {
         require_admin(&e, &caller);
-        e.storage().instance().set(&DataKey::AuthorizedOperator(operator), &true);
+        e.storage()
+            .instance()
+            .set(&DataKey::AuthorizedOperator(operator), &true);
     }
     pub fn remove_operator(e: Env, caller: Address, operator: Address) {
         require_admin(&e, &caller);
-        e.storage().instance().remove(&DataKey::AuthorizedOperator(operator));
+        e.storage()
+            .instance()
+            .remove(&DataKey::AuthorizedOperator(operator));
     }
 
     /// Allocates assets from a commitment to a target investment pool.
     ///
     /// This operation is restricted to the admin or an authorized allocator contract.
-    /// It reduces the commitment's internal `current_value` and transfers the 
+    /// It reduces the commitment's internal `current_value` and transfers the
     /// underlying tokens to the target address.
     ///
     /// ### Parameters
@@ -1508,7 +1552,9 @@ impl CommitmentCoreContract {
         }
 
         // Update collected fees
-        e.storage().instance().set(&fee_key, &SafeMath::sub(collected, amount));
+        e.storage()
+            .instance()
+            .set(&fee_key, &SafeMath::sub(collected, amount));
 
         // Transfer fees to recipient
         transfer_assets(

docs/FEES.md

@@ -68,7 +68,7 @@ Both round toward zero (floor for positive values). The sum of tranche amounts c
 
 | Fee | When | Calculation | Token flow |
 |-----|------|-------------|------------|
-| Creation | `create_commitment` | `fees::fee_from_bps(amount, CreationFeeBps)`; default bps `0` | Owner transfers full `amount` to contract; `creation_fee` credited to `CollectedFees(asset_address)`; NFT minted with `net_amount = amount - creation_fee`; TVL incremented by `net_amount` |
+| Creation | `create_commitment` | `fuzzing::checked_fee_from_bps(amount, CreationFeeBps)`; default bps `0`; overflow maps to `ArithmeticOverflow` with guard reset | Owner transfers full `amount` to contract; `creation_fee` credited to `CollectedFees(asset_address)`; NFT minted with `net_amount = amount - creation_fee`; TVL incremented by `net_amount` |
 | Early exit | `early_exit` | `SafeMath::penalty_amount(current_value, rules.early_exit_penalty)` | Penalty added to `CollectedFees(asset)`; `returned = current_value - penalty` transferred to owner when `returned > 0` |
 
 #### Storage keys

docs/FEE_MODEL_CROSS_CHECK.md

@@ -24,7 +24,7 @@ All four contracts are now documented in a single reconciled view in `docs/FEES.
 | `CreationFeeBps` storage | Yes | `DataKey::CreationFeeBps` | ✅ |
 | `CollectedFees(Address)` | Yes | `DataKey::CollectedFees(Address)` | ✅ |
 | `FeeRecipient` | Yes | `DataKey::FeeRecipient` | ✅ |
-| Creation fee on `create_commitment` | `fee_from_bps`, credit `CollectedFees` | Lines ~478–635 | ✅ |
+| Creation fee on `create_commitment` | `checked_fee_from_bps`, credit `CollectedFees` | Lines ~478–635 | ✅ |
 | Early exit penalty to `CollectedFees` | `SafeMath::penalty_amount` (percent / 100) | Lines ~1190–1204 | ✅ |
 | `set_creation_fee_bps` validates 0–10000 | Yes | `bps > fees::BPS_MAX` | ✅ |
 | `withdraw_fees` semantics | Treasurer/Admin, recipient required, cap by ledger | Lines ~1495–1537 | ✅ |
@@ -133,7 +133,8 @@ Prior `docs/FEES.md` listed marketplace fees as "TBD". They are now documented a
 | Item | `shared_utils::fees` | Used by |
 |------|---------------------|---------|
 | `BPS_SCALE` / `BPS_MAX` = 10_000 | `fees.rs` | `commitment_core`, `commitment_transformation` |
-| `fee_from_bps` — floor division | `fees.rs` | Creation, transformation fees |
+| `fee_from_bps` — floor division | `fees.rs` | Transformation fees |
+| `checked_fee_from_bps` — checked floor division with `Option` overflow signaling | `commitment_core::fuzzing` | Commitment creation fees |
 | `SafeMath::penalty_amount` — percent ÷ 100 | `math.rs` | `commitment_core::early_exit` |
 | `SafeMath::div(mul(price, bps), 10_000)` | `math.rs` | `commitment_marketplace` listings/auctions |