diff --git a/contracts/commitment_core/src/fuzz_tests.rs b/contracts/commitment_core/src/fuzz_tests.rs index e3a00ef..28bd7ef 100644 --- a/contracts/commitment_core/src/fuzz_tests.rs +++ b/contracts/commitment_core/src/fuzz_tests.rs @@ -2,16 +2,14 @@ use crate::{ fuzzing::{ - classify_generated_commitment_id_bytes, observe_amount, observe_commitment_input, - AmountShape, CommitmentIdShape, + checked_fee_and_net_from_bps, checked_fee_from_bps, classify_generated_commitment_id_bytes, + observe_amount, observe_commitment_input, AmountShape, CommitmentIdShape, }, 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] @@ -88,7 +86,6 @@ fn test_fuzz_amount_seed_shapes() { assert_eq!(observe_amount(0, 0).shape, AmountShape::NonPositive); assert_eq!(observe_amount(-1, 0).shape, AmountShape::NonPositive); assert_eq!(observe_amount(1, 10_001).shape, AmountShape::InvalidFeeBps); - assert_eq!(observe_amount(i128::MAX, 2).shape, AmountShape::FeeOverflow); let max_fee = observe_amount(1, 10_000); assert_eq!(max_fee.shape, AmountShape::Valid); @@ -101,6 +98,78 @@ fn test_fuzz_amount_seed_shapes() { assert_eq!(normal.net, Some(990)); } +#[test] +fn test_fuzz_checked_fee_conservation_seed_grid() { + let amounts = [ + 0i128, + 1, + 9_999, + 10_000, + 10_001, + i128::MAX / 10_000 - 1, + i128::MAX / 10_000, + i128::MAX / 10_000 + 1, + i128::MAX - 10_000, + i128::MAX - 1, + i128::MAX, + ]; + let bps_values = [0u32, 1, 2, 99, 100, 5_000, 9_999, 10_000]; + + for amount in amounts { + for fee_bps in bps_values { + let fee = checked_fee_from_bps(amount, fee_bps) + .expect("valid non-negative amount and bps should not overflow"); + let (fee_from_pair, net) = checked_fee_and_net_from_bps(amount, fee_bps) + .expect("valid non-negative amount and bps should produce fee and net"); + + assert_eq!(fee, fee_from_pair); + assert!(fee >= 0, "fee must never be negative"); + assert!(fee <= amount, "fee must never exceed amount"); + assert!(net >= 0, "net amount must never be negative"); + assert_eq!( + net.checked_add(fee), + Some(amount), + "net_amount + fee must conserve the original amount" + ); + + if fee_bps == 0 { + assert_eq!(fee, 0); + assert_eq!(net, amount); + } + if fee_bps == 10_000 { + assert_eq!(fee, amount); + assert_eq!(net, 0); + } + } + } +} + +#[test] +fn test_fuzz_checked_fee_rejects_invalid_domain_seed_grid() { + assert_eq!(checked_fee_from_bps(-1, 0), None); + assert_eq!(checked_fee_from_bps(1, 10_001), None); + assert_eq!(checked_fee_and_net_from_bps(-1, 10_000), None); + assert_eq!(checked_fee_and_net_from_bps(1, 10_001), None); +} + +#[test] +fn test_fuzz_amount_observation_handles_max_adjacent_boundaries() { + for amount in [i128::MAX - 1, i128::MAX] { + let full_fee = observe_amount(amount, 10_000); + assert_eq!(full_fee.shape, AmountShape::Valid); + assert_eq!(full_fee.fee, Some(amount)); + assert_eq!(full_fee.net, Some(0)); + + let tiny_fee = observe_amount(amount, 1); + assert_eq!(tiny_fee.shape, AmountShape::Valid); + assert!(tiny_fee.fee.unwrap() <= amount); + assert_eq!( + tiny_fee.net.unwrap().checked_add(tiny_fee.fee.unwrap()), + Some(amount) + ); + } +} + #[test] fn test_fuzz_observation_combines_id_and_amount_seed() { let observation = observe_commitment_input(b"c_42", 1_000, 100); diff --git a/contracts/commitment_core/src/fuzzing.rs b/contracts/commitment_core/src/fuzzing.rs index 79f44c2..b0b0ebb 100644 --- a/contracts/commitment_core/src/fuzzing.rs +++ b/contracts/commitment_core/src/fuzzing.rs @@ -68,13 +68,29 @@ pub fn classify_generated_commitment_id_bytes(bytes: &[u8]) -> CommitmentIdShape } pub fn checked_fee_from_bps(amount: i128, fee_bps: u32) -> Option { + if amount < 0 { + return None; + } + if fee_bps > BPS_MAX { return None; } - amount - .checked_mul(fee_bps as i128)? - .checked_div(BPS_SCALE as i128) + let scale = BPS_SCALE as i128; + let bps = fee_bps as i128; + let quotient_fee = amount.checked_div(scale)?.checked_mul(bps)?; + let remainder_fee = amount + .checked_rem(scale)? + .checked_mul(bps)? + .checked_div(scale)?; + + quotient_fee.checked_add(remainder_fee) +} + +pub fn checked_fee_and_net_from_bps(amount: i128, fee_bps: u32) -> Option<(i128, i128)> { + let fee = checked_fee_from_bps(amount, fee_bps)?; + let net = amount.checked_sub(fee)?; + Some((fee, net)) } pub fn observe_amount(amount: i128, fee_bps: u32) -> AmountObservation { @@ -94,7 +110,7 @@ pub fn observe_amount(amount: i128, fee_bps: u32) -> AmountObservation { }; } - let Some(fee) = checked_fee_from_bps(amount, fee_bps) else { + let Some((fee, net)) = checked_fee_and_net_from_bps(amount, fee_bps) else { return AmountObservation { shape: AmountShape::FeeOverflow, fee: None, @@ -102,14 +118,6 @@ pub fn observe_amount(amount: i128, fee_bps: u32) -> AmountObservation { }; }; - let Some(net) = amount.checked_sub(fee) else { - return AmountObservation { - shape: AmountShape::NetUnderflow, - fee: Some(fee), - net: None, - }; - }; - AmountObservation { shape: AmountShape::Valid, fee: Some(fee), @@ -117,7 +125,11 @@ pub fn observe_amount(amount: i128, fee_bps: u32) -> AmountObservation { } } -pub fn observe_commitment_input(commitment_id: &[u8], amount: i128, fee_bps: u32) -> CommitmentInputObservation { +pub fn observe_commitment_input( + commitment_id: &[u8], + amount: i128, + fee_bps: u32, +) -> CommitmentInputObservation { CommitmentInputObservation { id_shape: classify_generated_commitment_id_bytes(commitment_id), amount: observe_amount(amount, fee_bps), diff --git a/docs/SECURITY_AUDIT_PREP.md b/docs/SECURITY_AUDIT_PREP.md index 716645a..5bccc91 100644 --- a/docs/SECURITY_AUDIT_PREP.md +++ b/docs/SECURITY_AUDIT_PREP.md @@ -30,6 +30,17 @@ - Cross-contract calls (token transfer, NFT mint/settle, commitment_core reads) - Storage growth and data consistency for vectors and registries +## Fee Arithmetic Fuzz Invariants + +`commitment_core` includes deterministic fuzz-seed tests for basis-point fee arithmetic. For every non-negative amount in the seed grid and every valid `bps` value in `0..=10000`, the checked helper must satisfy: + +- `0 <= fee <= amount` +- `net_amount + fee == amount` +- `bps == 0` yields `fee == 0` and `net_amount == amount` +- `bps == 10000` yields `fee == amount` and `net_amount == 0` + +The seed grid includes `i128::MAX`-adjacent amounts so the helper computes mathematically valid fees without first overflowing `amount * bps`. Invalid domains, including negative amounts and `bps > 10000`, return `None` instead of producing fee observations. + ## Open items before audit - Capture a coverage report and attach to TEST_COVERAGE.md - Decide on authorization model for mint/allocate/settle flows