Skip to content
Merged

Fixes #397

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
2 changes: 0 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,4 @@ issue*.json
pr_data.json
pr_response.json

# target files

fix.md
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
7 changes: 7 additions & 0 deletions docs/CAMPAIGN_LIFECYCLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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`
Expand All @@ -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.
15 changes: 15 additions & 0 deletions fix.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
19 changes: 16 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -844,15 +856,15 @@ impl ProofOfHeart {
if contribution == 0 {
return Err(Error::ValidationFailed);
}
if campaign.amount_raised == 0 {
if campaign.effective_amount_raised == 0 {
return Err(Error::AmountRaisedIsZero);
}

let total_pool = get_revenue_pool(&env, campaign_id);
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;
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 1 addition & 6 deletions src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions src/tests/test_admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
8 changes: 2 additions & 6 deletions src/tests/test_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
68 changes: 68 additions & 0 deletions src/tests/test_refund_edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 42 additions & 0 deletions src/tests/test_voting_verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ pub struct Campaign {
pub fee_override: Option<u32>,
/// 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.
Expand Down
5 changes: 5 additions & 0 deletions src/voting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Loading