From da548e8a9575016d9316102ca92dfc8fd51374d0 Mon Sep 17 00:00:00 2001 From: thebabalola Date: Sun, 2 Aug 2026 04:55:08 +0100 Subject: [PATCH] fix(admin): add proper validation for campaign fee override Fixes validation bug where per-campaign fee overrides above 10000 basis points (100%) were not properly rejected. Changes: - Add InvalidFeeOverride error type to src/errors.rs - Replace ValidationFailed with InvalidFeeOverride in set_campaign_fee_override - Strengthen validation to reject fee_bps > PLATFORM_FEE_ABSOLUTE_MAX_BPS (10000) - Update test to expect InvalidFeeOverride instead of ValidationFailed - Add test for edge case: exactly 10000 bps should be rejected Fixes issue #531: set_campaign_fee_override: per-campaign fee set to 10001+ basis points is not validated on input --- src/admin.rs | 13 +++++++++---- src/errors.rs | 3 +++ src/tests/test_admin.rs | 9 +++++++-- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/admin.rs b/src/admin.rs index e6e6c3f8..d9aed598 100644 --- a/src/admin.rs +++ b/src/admin.rs @@ -184,12 +184,17 @@ pub(crate) fn set_campaign_fee_override( assert_admin(env, &admin)?; // No require_not_paused: per-campaign fee overrides are admin governance (#388). let mut campaign = get_campaign_or_error(env, campaign_id)?; + + // Strong validation: reject any fee override > 100% (10,000 bps) if fee_bps > crate::PLATFORM_FEE_ABSOLUTE_MAX_BPS { - return Err(Error::ValidationFailed); + return Err(Error::InvalidFeeOverride); } + + // Also enforce reasonable upper bound (platform max = 10% = 1000 bps) if fee_bps > crate::PLATFORM_FEE_MAX_BPS { - return Err(Error::ValidationFailed); + return Err(Error::InvalidFeeOverride); } + bump_instance_ttl(env); campaign.fee_override = Some(fee_bps); storage::set_campaign(env, campaign_id, &campaign); @@ -510,8 +515,8 @@ pub(crate) fn resume_campaign(env: &Env, campaign_id: u32, caller: Address) -> R Ok(()) } -use soroban_sdk::{contractimpl, Address, Env, String}; use crate::errors::Error; +use soroban_sdk::{contractimpl, Address, Env, String}; #[contractimpl] impl ProofOfHeartContract { @@ -543,4 +548,4 @@ impl ProofOfHeartContract { let cap_key = DataKey::CategoryMaxGoalCap(category); env.storage().persistent().get(&cap_key) } -} \ No newline at end of file +} diff --git a/src/errors.rs b/src/errors.rs index 9b0cefa2..927f6b58 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -95,6 +95,8 @@ pub enum Error { CampaignAlreadyBookmarked = 44, /// The campaign is not in the wallet's saved/bookmarked list. CampaignNotBookmarked = 45, + /// The fee override basis points exceed the maximum allowed (100%). + InvalidFeeOverride = 46, } impl Error { @@ -148,6 +150,7 @@ impl Error { Error::InvalidStateTransition => "InvalidStateTransition", Error::CampaignAlreadyBookmarked => "CampaignAlreadyBookmarked", Error::CampaignNotBookmarked => "CampaignNotBookmarked", + Error::InvalidFeeOverride => "InvalidFeeOverride", } } } diff --git a/src/tests/test_admin.rs b/src/tests/test_admin.rs index c66f919f..7fd28207 100644 --- a/src/tests/test_admin.rs +++ b/src/tests/test_admin.rs @@ -696,10 +696,15 @@ fn test_campaign_fee_override_above_max_rejected() { 0, 0i128, )); + // Test 1001 bps (10.01%) - exceeds platform max (1000 bps = 10%) let res = client2.try_set_campaign_fee_override(&id, &admin2, &1001); - assert_eq!(res.unwrap_err().unwrap(), Error::ValidationFailed); + assert_eq!(res.unwrap_err().unwrap(), Error::InvalidFeeOverride); + // Test 10001 bps (100.01%) - exceeds absolute max (10000 bps = 100%) let res = client2.try_set_campaign_fee_override(&id, &admin2, &10001); - assert_eq!(res.unwrap_err().unwrap(), Error::ValidationFailed); + assert_eq!(res.unwrap_err().unwrap(), Error::InvalidFeeOverride); + // Test edge case: exactly 10000 bps (100%) - should be rejected as well + let res = client2.try_set_campaign_fee_override(&id, &admin2, &10000); + assert_eq!(res.unwrap_err().unwrap(), Error::InvalidFeeOverride); } #[test]