diff --git a/.gitignore b/.gitignore index 51bb1a2..7821496 100644 --- a/.gitignore +++ b/.gitignore @@ -51,6 +51,4 @@ issue*.json pr_data.json pr_response.json -# target files - fix.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fdbdb2..c00b3ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,14 +7,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] ### Fixed + - `update_campaign_description` now blocks edits once `amount_raised > 0`, preventing bait-and-switch after contributions (#166). - `claim_creator_revenue` returns `ValidationFailed` when `revenue_share_percentage > 10000` instead of producing negative math or panicking (#167). +- `init` and `update_platform_fee` now reject `platform_fee` values above `1000` with `InvalidPlatformFee` instead of silently capping them. - `initiate_campaign_transfer` now rejects cancelled or withdrawn campaigns, keeping ownership transfers off terminal campaigns (#323). - `resume_campaign` now returns `ValidationFailed` when no pause is active, preventing spurious state writes and `campaign_resumed` events (#348). - `update_campaign` now emits both the updated title and description in `campaign_updated`, allowing full metadata indexing without extra reads (#349). ### Refactored + - Extracted `assert_admin(env, caller)` helper; used in `pause`, `unpause`, and `set_voting_params` to provide a single source of truth for admin authorization (#224). ### Documentation + - Added `CHANGELOG.md` and documented the Keep-a-Changelog convention in `CONTRIBUTING.md` (#227). diff --git a/docs/CAMPAIGN_LIFECYCLE.md b/docs/CAMPAIGN_LIFECYCLE.md index 87b7372..e487309 100644 --- a/docs/CAMPAIGN_LIFECYCLE.md +++ b/docs/CAMPAIGN_LIFECYCLE.md @@ -15,12 +15,14 @@ Additional derived conditions used by the contract: ## States ### 1) Active (unverified) + - Set on `create_campaign`: `is_active = true`, `is_cancelled = false`, `funds_withdrawn = false`, `is_verified = false`. - Contributions are blocked until verified: `contribute` returns `CampaignNotVerified` while `is_verified = false`. - Creator can still update/cancel while active (subject to each method's rules). - Full `update_campaign` edits are only available before verification and before any contributions. ### 2) Active + Verified + - Reached by either: - `verify_campaign` (admin verification), or - `verify_campaign_with_votes` (community verification after quorum + threshold). @@ -29,11 +31,13 @@ Additional derived conditions used by the contract: - After verification, `update_campaign` is blocked so the verified title/description cannot be changed without re-review. ### 3) Funded (derived) + - When `amount_raised >= funding_goal`, the campaign is considered funded. - The contract does not set a dedicated boolean for "funded"; it is checked when withdrawing. - The creator may call `withdraw_funds` once funded (and if not cancelled / not previously withdrawn). ### 4) Withdrawn / Closed + - Reached by `withdraw_funds`: - sets `funds_withdrawn = true` - sets `is_active = false` @@ -42,12 +46,15 @@ Additional derived conditions used by the contract: - `cancel_campaign` is blocked by `CancellationNotAllowed` ### 5) Cancelled + - Reached by `cancel_campaign` (creator only): - sets `is_cancelled = true` - sets `is_active = false` - Contributors can claim refunds via `claim_refund` after cancellation (if they contributed). - Successful refunds remove the contributor's stored contribution record instead of leaving a zero-value entry behind. +- Refunds also reduce the campaign's live contribution denominator used for revenue sharing, ensuring remaining contributors receive the correct pro-rata share of future revenue claims. ### 6) Expired / Failed (derived) + - If the deadline passes and the campaign did not reach its goal (`Expired/Failed` derived condition), contributors can claim refunds via `claim_refund`. - The contract does not currently toggle `is_active` automatically when a deadline passes; "expired" is computed at call time using the ledger timestamp. diff --git a/fix.md b/fix.md new file mode 100644 index 0000000..b04589e --- /dev/null +++ b/fix.md @@ -0,0 +1,15 @@ +#162 [BUG] admin_verify and verify_with_votes do not check is_active or is_cancelled +Repo Avatar +Iris-IV/ProofOfHeart-stellar +Summary +voting::admin_verify and voting::verify_with_votes flip is_verified=true regardless of the campaign's active/cancel state. A cancelled campaign can be marked verified — confusing for indexers and front-end consumers. + +Where +src/voting.rs admin_verify (line ~100) +src/voting.rs verify_with_votes (line ~125) +Fix +Before flipping the bit, return Error::CampaignNotActive if is_cancelled || !is_active. + +Acceptance criteria + Negative tests covering both functions on a cancelled campaign. + No state regression on the happy path. \ No newline at end of file diff --git a/src/errors.rs b/src/errors.rs index 91f3197..c45cd53 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -81,6 +81,8 @@ pub enum Error { ExtensionTooLong = 37, /// The funding goal exceeds the configured maximum (anti-spam cap). FundingGoalTooHigh = 38, + /// The provided platform fee exceeds the maximum allowed basis points. + InvalidPlatformFee = 39, /// A campaign transfer is already pending; cancel it before initiating a new one. TransferAlreadyPending = 39, } diff --git a/src/lib.rs b/src/lib.rs index b64a7c2..c26d29f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -161,6 +161,10 @@ impl ProofOfHeart { .map_err(|_| Error::InvalidTokenContract)? .map_err(|_| Error::InvalidTokenContract)?; + if platform_fee > PLATFORM_FEE_MAX_BPS { + return Err(Error::InvalidPlatformFee); + } + bump_instance_ttl(&env); set_admin(&env, &admin); remove_pending_admin(&env); @@ -301,6 +305,7 @@ impl ProofOfHeart { max_contribution_per_user, fee_override: None, deadline_extended: false, + effective_amount_raised: 0, }; set_campaign(&env, count, &campaign); @@ -417,6 +422,7 @@ impl ProofOfHeart { client.transfer(&contributor, &env.current_contract_address(), &amount); campaign.amount_raised += amount; + campaign.effective_amount_raised += amount; set_campaign(&env, campaign_id, &campaign); set_contribution(&env, campaign_id, &contributor, current + amount); set_lifetime_contribution(&env, campaign_id, &contributor, lifetime + amount); @@ -735,7 +741,7 @@ impl ProofOfHeart { contributor.require_auth(); Self::require_not_paused(&env)?; - let campaign = get_campaign_or_error(&env, campaign_id)?; + let mut campaign = get_campaign_or_error(&env, campaign_id)?; let failed_due_to_goal = env.ledger().timestamp() > campaign.deadline && campaign.amount_raised < campaign.funding_goal; @@ -759,6 +765,12 @@ impl ProofOfHeart { // (the contributor no longer has any contribution to this campaign) decrement_contributor_count(&env, campaign_id); + campaign.effective_amount_raised = campaign + .effective_amount_raised + .checked_sub(amount) + .ok_or(Error::Overflow)?; + set_campaign(&env, campaign_id, &campaign); + let total_raised = get_total_raised_global(&env); set_total_raised_global( &env, @@ -844,7 +856,7 @@ impl ProofOfHeart { if contribution == 0 { return Err(Error::ValidationFailed); } - if campaign.amount_raised == 0 { + if campaign.effective_amount_raised == 0 { return Err(Error::AmountRaisedIsZero); } @@ -852,7 +864,7 @@ impl ProofOfHeart { let contributor_pool = (total_pool * (campaign.revenue_share_percentage as i128)) / 10000; let total_due = contribution .checked_mul(contributor_pool) - .and_then(|n| n.checked_div(campaign.amount_raised)) + .and_then(|n| n.checked_div(campaign.effective_amount_raised)) .ok_or(Error::Overflow)?; let already_claimed = get_revenue_claimed(&env, campaign_id, &contributor); let claimable = total_due - already_claimed; @@ -1347,6 +1359,7 @@ impl ProofOfHeart { assert_admin(&env, &admin)?; Self::require_not_paused(&env)?; if new_fee > PLATFORM_FEE_MAX_BPS { + return Err(Error::InvalidPlatformFee); return Err(Error::ValidationFailed); } let old_fee = get_platform_fee(&env); diff --git a/src/test.rs b/src/test.rs index b003fa3..40681c3 100644 --- a/src/test.rs +++ b/src/test.rs @@ -102,13 +102,7 @@ fn test_platform_fee_cap_enforcement() { env.mock_all_auths(); let admin = Address::generate(&env); - let creator = Address::generate(&env); - let contributor = Address::generate(&env); - let token_address = env.register_stellar_asset_contract(admin.clone()); - let token = TokenClient::new(&env, &token_address); - let token_admin = TokenAdminClient::new(&env, &token_address); - let contract_id = env.register_contract(None, ProofOfHeart); let client = ProofOfHeartClient::new(&env, &contract_id); @@ -1065,6 +1059,7 @@ fn test_update_platform_fee() { // Issue #343: fees above the cap are rejected, not silently clamped. let result = client.try_update_platform_fee(&5000); + assert_eq!(result.unwrap_err().unwrap(), Error::InvalidPlatformFee); assert_eq!(result.unwrap_err().unwrap(), Error::ValidationFailed); } diff --git a/src/tests/test_admin.rs b/src/tests/test_admin.rs index 9a5483e..f51c503 100644 --- a/src/tests/test_admin.rs +++ b/src/tests/test_admin.rs @@ -21,6 +21,7 @@ fn test_update_platform_fee() { // Issue #343: fees above the cap are rejected, not silently clamped. let result = client.try_update_platform_fee(&5000); + assert_eq!(result.unwrap_err().unwrap(), Error::InvalidPlatformFee); assert_eq!(result.unwrap_err().unwrap(), Error::ValidationFailed); } diff --git a/src/tests/test_init.rs b/src/tests/test_init.rs index 79ee746..984ffa8 100644 --- a/src/tests/test_init.rs +++ b/src/tests/test_init.rs @@ -15,16 +15,12 @@ fn test_platform_fee_cap_enforcement() { env.mock_all_auths(); let admin = Address::generate(&env); - let creator = Address::generate(&env); - let contributor = Address::generate(&env); - let token_address = env.register_stellar_asset_contract(admin.clone()); - let token = soroban_sdk::token::Client::new(&env, &token_address); - let token_admin = soroban_sdk::token::StellarAssetClient::new(&env, &token_address); - let contract_id = env.register_contract(None, crate::ProofOfHeart); let client = crate::ProofOfHeartClient::new(&env, &contract_id); + let res = client.try_init(&admin, &token_address, &5000); + assert_eq!(res.unwrap_err().unwrap(), Error::InvalidPlatformFee); // Issue #343: init rejects fees above the cap rather than silently clamping. let res = client.try_init(&admin, &token_address, &5000); assert_eq!(res.unwrap_err().unwrap(), Error::ValidationFailed); diff --git a/src/tests/test_refund_edge.rs b/src/tests/test_refund_edge.rs index 1008108..ca1a836 100644 --- a/src/tests/test_refund_edge.rs +++ b/src/tests/test_refund_edge.rs @@ -93,6 +93,74 @@ fn test_claim_refund_expired_campaign() { assert_eq!(client.get_revenue_claimed(&campaign_id, &contributor1), 0); } +#[test] +fn test_claim_refund_clears_existing_revenue_claimed_key() { + let (env, _admin, creator, contributor1, _, _token, token_admin, client) = setup_env(); + token_admin.mint(&contributor1, &5000); + token_admin.mint(&creator, &10_000); + + let campaign_id = client.create_campaign(&CreateCampaignParams { + creator: creator.clone(), + title: String::from_str(&env, "Refund Cleans Revenue Claim"), + description: String::from_str(&env, "Ensure RevenueClaimed key is removed"), + funding_goal: 5000, + duration_days: 30, + category: Category::EducationalStartup, + has_revenue_sharing: true, + revenue_share_percentage: 2000, + max_contribution_per_user: 0i128, + }); + client.verify_campaign(&campaign_id); + client.contribute(&campaign_id, &contributor1, &1000); + client.deposit_revenue(&campaign_id, &1000); + client.claim_revenue(&campaign_id, &contributor1); + + let claimed_before_refund = client.get_revenue_claimed(&campaign_id, &contributor1); + assert!(claimed_before_refund > 0); + + client.cancel_campaign(&campaign_id); + client.claim_refund(&campaign_id, &contributor1); + + assert_eq!(client.get_revenue_claimed(&campaign_id, &contributor1), 0); +} + +#[test] +fn test_claim_revenue_after_single_refund_uses_live_raised() { + let (env, _admin, creator, contributor1, contributor2, token, token_admin, client) = + setup_env(); + + token_admin.mint(&contributor1, &5000); + token_admin.mint(&contributor2, &5000); + token_admin.mint(&creator, &10_000); + + let campaign_id = client.create_campaign(&make_params( + creator.clone(), + String::from_str(&env, "Revenue Refund Denominator"), + String::from_str(&env, "Remaining contributor receives full share after refund"), + 2000, + 10, + Category::EducationalStartup, + true, + 5000, + 0i128, + )); + + client.verify_campaign(&campaign_id); + client.contribute(&campaign_id, &contributor1, &1000); + client.contribute(&campaign_id, &contributor2, &1000); + client.deposit_revenue(&campaign_id, &1000); + + client.cancel_campaign(&campaign_id); + client.claim_refund(&campaign_id, &contributor1); + + assert_eq!(client.get_contribution(&campaign_id, &contributor1), 0); + assert_eq!(token.balance(&contributor1), 5000); + + client.claim_revenue(&campaign_id, &contributor2); + + assert_eq!(token.balance(&contributor2), 4500); + assert_eq!(client.get_revenue_claimed(&campaign_id, &contributor2), 500); +} // Issue #341: claim_revenue is gated on funds_withdrawn. The prior "claim // then cancel then refund" flow this test exercised is now structurally // impossible (cancel is blocked once funds are withdrawn). Covered by diff --git a/src/tests/test_voting_verify.rs b/src/tests/test_voting_verify.rs index 7c86e44..531166a 100644 --- a/src/tests/test_voting_verify.rs +++ b/src/tests/test_voting_verify.rs @@ -19,6 +19,48 @@ fn test_vote_on_cancelled_campaign_fails() { assert_eq!(res.unwrap_err().unwrap(), Error::CampaignNotActive); } +#[test] +fn test_admin_verify_cancelled_campaign_fails() { + let (env, _admin, creator, _, _, _token, _token_admin, client) = setup_env(); + + let campaign_id = client.create_campaign(&make_params( + creator.clone(), String::from_str(&env, "Cancelled Admin Verify"), + String::from_str(&env, "Test admin verification on cancelled campaign"), 1000, 30, + Category::Learner, false, 0, 0i128, + )); + + client.cancel_campaign(&campaign_id); + + let res = client.try_verify_campaign(&campaign_id); + assert_eq!(res.unwrap_err().unwrap(), Error::CampaignNotActive); +} + +#[test] +fn test_verify_campaign_with_votes_cancelled_campaign_fails() { + let (env, admin, creator, contributor1, contributor2, _token, token_admin, client) = setup_env(); + token_admin.mint(&contributor1, &1000); + token_admin.mint(&contributor2, &1000); + let voter3 = Address::generate(&env); + token_admin.mint(&voter3, &1000); + + client.set_voting_params(&admin, &3, &6000); + + let campaign_id = client.create_campaign(&make_params( + creator.clone(), String::from_str(&env, "Cancelled Vote Verify"), + String::from_str(&env, "Test vote-based verification on cancelled campaign"), 1000, 30, + Category::Learner, false, 0, 0i128, + )); + + client.vote_on_campaign(&campaign_id, &contributor1, &true); + client.vote_on_campaign(&campaign_id, &contributor2, &true); + client.vote_on_campaign(&campaign_id, &voter3, &false); + + client.cancel_campaign(&campaign_id); + + let res = client.try_verify_campaign_with_votes(&campaign_id); + assert_eq!(res.unwrap_err().unwrap(), Error::CampaignNotActive); +} + #[test] fn test_vote_on_campaign_past_deadline_fails() { let (env, _admin, creator, contributor1, _, _token, token_admin, client) = setup_env(); diff --git a/src/types.rs b/src/types.rs index 9825d8d..ab13635 100644 --- a/src/types.rs +++ b/src/types.rs @@ -66,6 +66,8 @@ pub struct Campaign { pub fee_override: Option, /// Whether the deadline has already been extended once. pub deadline_extended: bool, + /// Total live contributions remaining after refunds, used for revenue-sharing pro-rata. + pub effective_amount_raised: i128, } /// Aggregate platform metrics for dashboard and indexer consumers. diff --git a/src/voting.rs b/src/voting.rs index dab80e5..d8a2184 100644 --- a/src/voting.rs +++ b/src/voting.rs @@ -54,6 +54,9 @@ pub fn cast_vote(env: &Env, campaign_id: u32, voter: Address, approve: bool) -> voter.require_auth(); let campaign = get_campaign_or_error(env, campaign_id)?; + if campaign.funds_withdrawn { + return Err(Error::CampaignNotActive); + } require_active_campaign(&campaign)?; if env.ledger().timestamp() > campaign.deadline { return Err(Error::DeadlinePassed); @@ -111,6 +114,7 @@ pub fn admin_verify(env: &Env, campaign_id: u32) -> Result<(), Error> { if campaign.is_verified { return Err(Error::AdminVerificationConflict); } + require_active_campaign(&campaign)?; campaign.is_verified = true; set_campaign(env, campaign_id, &campaign); @@ -136,6 +140,7 @@ pub fn verify_with_votes(env: &Env, campaign_id: u32) -> Result<(), Error> { if campaign.is_verified { return Err(Error::CommunityVerificationConflict); } + require_active_campaign(&campaign)?; let approve_votes = get_approve_votes(env, campaign_id); let reject_votes = get_reject_votes(env, campaign_id);