From 26deb8135c0b34ae5983da65f49d1b512aa95506 Mon Sep 17 00:00:00 2001
From: D'Angelo Rodriguez <70290504+dangelo352@users.noreply.github.com>
Date: Sun, 21 Jun 2026 23:16:18 -0400
Subject: [PATCH] fix: use checked creation fee arithmetic

---
 contracts/commitment_core/src/fee_tests.rs | 22 +++++++++++++++++++++-
 contracts/commitment_core/src/lib.rs       | 12 +++++++-----
 docs/FEES.md                               |  2 +-
 docs/FEE_MODEL_CROSS_CHECK.md              |  3 ++-
 4 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/contracts/commitment_core/src/fee_tests.rs b/contracts/commitment_core/src/fee_tests.rs
index 6ad0fa7..4cb59d0 100644
--- a/contracts/commitment_core/src/fee_tests.rs
+++ b/contracts/commitment_core/src/fee_tests.rs
@@ -241,6 +241,26 @@ fn test_create_commitment_with_max_fee() {
     assert_eq!(client.get_collected_fees(&token_address), expected_fee);
 }
 
+#[test]
+fn test_create_commitment_fee_overflow_resets_reentrancy_guard() {
+    let (e, admin, contract_id, user, token_address, client) = setup_test();
+
+    client.set_creation_fee_bps(&admin, &10000);
+    let rules = default_rules(&e);
+
+    let overflow_result =
+        client.try_create_commitment(&user, &i128::MAX, &token_address, &rules);
+    assert!(overflow_result.is_err());
+
+    client.set_creation_fee_bps(&admin, &100);
+    let commitment_id =
+        create_commitment_direct(&e, &contract_id, &user, 1_000_000, &token_address, &rules);
+
+    let commitment = client.get_commitment(&commitment_id);
+    assert_eq!(commitment.amount, 990_000);
+    assert_eq!(client.get_collected_fees(&token_address), 10_000);
+}
+
 #[test]
 fn test_create_commitment_fee_rounds_down() {
     let (e, admin, contract_id, user, token_address, client) = setup_test();
@@ -628,4 +648,4 @@ fn test_complete_fee_lifecycle() {
 
     // 7. Verify no fees remaining
     assert_eq!(client.get_collected_fees(&token_address), 0);
-}
\ No newline at end of file
+}
diff --git a/contracts/commitment_core/src/lib.rs b/contracts/commitment_core/src/lib.rs
index 0269de6..064ab5d 100644
--- a/contracts/commitment_core/src/lib.rs
+++ b/contracts/commitment_core/src/lib.rs
@@ -504,11 +504,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 arithmetic: use the checked
+        // helper so overflow clears the reentrancy guard before failing.
+        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");
diff --git a/docs/FEES.md b/docs/FEES.md
index 9c3d0fb..6a7cf25 100644
--- a/docs/FEES.md
+++ b/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 fails as `ArithmeticOverflow` | 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
diff --git a/docs/FEE_MODEL_CROSS_CHECK.md b/docs/FEE_MODEL_CROSS_CHECK.md
index 5e7553e..764646b 100644
--- a/docs/FEE_MODEL_CROSS_CHECK.md
+++ b/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 bps helper, 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 | ✅ |
@@ -36,6 +36,7 @@ The previous version of this file listed commitment_core fee infrastructure as *
 
 - Storage keys exist.
 - `create_commitment` collects creation fees.
+- Creation-fee arithmetic uses a single checked helper and fails as `ArithmeticOverflow` before mutating fee state when multiplication overflows.
 - `early_exit` credits penalties to `CollectedFees`.
 - Admin functions and getters are implemented with tests in `fee_tests.rs`.