Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion contracts/commitment_core/src/fee_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -628,4 +648,4 @@ fn test_complete_fee_lifecycle() {

// 7. Verify no fees remaining
assert_eq!(client.get_collected_fees(&token_address), 0);
}
}
12 changes: 7 additions & 5 deletions contracts/commitment_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion docs/FEES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion docs/FEE_MODEL_CROSS_CHECK.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | ✅ |
Expand All @@ -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`.

Expand Down
Loading