From 961ebdca891c1fdef66347c84bc529e02e530650 Mon Sep 17 00:00:00 2001 From: JamesVictor-O Date: Sat, 25 Apr 2026 14:09:46 +0100 Subject: [PATCH 1/3] Fix admin transfer safety and add regression tests for voting, refunds, and revenue deposits. This switches admin updates to a two-step transfer to prevent accidental lockout, clears stale revenue-claimed state on refunds, and adds coverage for deadline/withdrawn vote gating plus repeated revenue deposits. Made-with: Cursor --- src/lib.rs | 64 +++++++++++++++++++-- src/storage.rs | 25 ++++++++ src/test.rs | 121 +++++++++++++++++++++++++++++++++++++++ src/update_admin_test.rs | 20 +++++++ src/voting.rs | 5 +- 5 files changed, 230 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 025ec79..afeea23 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -118,6 +118,7 @@ impl ProofOfHeart { bump_instance_ttl(&env); set_admin(&env, &admin); + remove_pending_admin(&env); set_token(&env, &token); set_initialized(&env); @@ -528,6 +529,7 @@ impl ProofOfHeart { bump_instance_ttl(&env); set_contribution(&env, campaign_id, &contributor, 0); + remove_revenue_claimed(&env, campaign_id, &contributor); let total_raised = get_total_raised_global(&env); set_total_raised_global(&env, total_raised - amount); @@ -910,11 +912,15 @@ impl ProofOfHeart { get_personal_cap(&env, campaign_id, &contributor).unwrap_or(0) } - /// Transfers admin privileges to a new address. + /// Initiates transfer of admin privileges to a new address. /// /// # Authorization /// Requires the current admin to authorize the call. - pub fn update_admin(env: Env, admin: Address, new_admin: Address) -> Result<(), Error> { + pub fn initiate_admin_transfer( + env: Env, + admin: Address, + new_admin: Address, + ) -> Result<(), Error> { admin.require_auth(); Self::require_not_paused(&env)?; @@ -922,15 +928,65 @@ impl ProofOfHeart { if admin != current_admin { return Err(Error::NotAuthorized); } + if new_admin == current_admin { + return Err(Error::InvalidNewOwner); + } + + bump_instance_ttl(&env); + set_pending_admin(&env, &new_admin); + env.events() + .publish(("admin_transfer_initiated",), (current_admin, new_admin)); + + Ok(()) + } + + /// Accepts a pending admin transfer. Must be called by the pending admin. + pub fn accept_admin_transfer(env: Env) -> Result<(), Error> { + Self::require_not_paused(&env)?; + + let pending_admin = get_pending_admin(&env).ok_or(Error::NoTransferPending)?; + pending_admin.require_auth(); bump_instance_ttl(&env); - set_admin(&env, &new_admin); + let old_admin = get_admin(&env); + set_admin(&env, &pending_admin); + remove_pending_admin(&env); env.events() - .publish(("admin_updated",), (current_admin, new_admin)); + .publish(("admin_updated",), (old_admin, pending_admin)); Ok(()) } + /// Cancels a pending admin transfer. + pub fn cancel_admin_transfer(env: Env, admin: Address) -> Result<(), Error> { + admin.require_auth(); + Self::require_not_paused(&env)?; + + let current_admin = get_admin(&env); + if admin != current_admin { + return Err(Error::NotAuthorized); + } + if get_pending_admin(&env).is_none() { + return Err(Error::NoTransferPending); + } + + bump_instance_ttl(&env); + remove_pending_admin(&env); + env.events().publish(("admin_transfer_cancelled",), current_admin); + + Ok(()) + } + + /// Backwards-compatible wrapper that initiates two-step admin transfer. + pub fn update_admin(env: Env, admin: Address, new_admin: Address) -> Result<(), Error> { + Self::initiate_admin_transfer(env, admin, new_admin) + } + + /// Returns the pending admin address if transfer is in progress. + pub fn get_pending_admin(env: Env) -> Option
{ + get_pending_admin(&env) + } + /// Gets the number of recorded approval votes for a campaign. pub fn get_approve_votes(env: Env, campaign_id: u32) -> u32 { get_approve_votes(&env, campaign_id) diff --git a/src/storage.rs b/src/storage.rs index becf08f..5e7ed95 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -17,6 +17,8 @@ pub fn bump_instance_ttl(env: &Env) { pub enum DataKey { /// The global admin address. Admin, + /// Pending admin during two-step admin transfer. + PendingAdmin, /// The contract's accepted token address. Token, /// Platform fee in basis points (e.g. 300 = 3%). @@ -127,6 +129,23 @@ pub fn set_admin(env: &Env, admin: &Address) { env.storage().instance().set(&DataKey::Admin, admin); } +/// Returns the pending admin address if an admin transfer is in progress. +pub fn get_pending_admin(env: &Env) -> Option
{ + env.storage().instance().get(&DataKey::PendingAdmin) +} + +/// Stores the pending admin address for two-step admin transfer. +pub fn set_pending_admin(env: &Env, pending_admin: &Address) { + env.storage() + .instance() + .set(&DataKey::PendingAdmin, pending_admin); +} + +/// Clears any pending admin transfer. +pub fn remove_pending_admin(env: &Env) { + env.storage().instance().remove(&DataKey::PendingAdmin); +} + /// Returns the accepted token address. Panics if not yet initialized. pub fn get_token(env: &Env) -> Address { env.storage().instance().get(&DataKey::Token).unwrap() @@ -217,6 +236,12 @@ pub fn set_revenue_claimed(env: &Env, campaign_id: u32, contributor: &Address, a .extend_ttl(&key, BUMP_THRESHOLD, BUMP_AMOUNT); } +/// Removes the revenue claimed record for a contributor in a campaign. +pub fn remove_revenue_claimed(env: &Env, campaign_id: u32, contributor: &Address) { + let key = DataKey::RevenueClaimed(campaign_id, contributor.clone()); + env.storage().persistent().remove(&key); +} + /// Returns the creator's total claimed revenue for a campaign, extending TTL if non-zero. pub fn get_creator_revenue_claimed(env: &Env, campaign_id: u32) -> i128 { let key = DataKey::CreatorRevenueClaimed(campaign_id); diff --git a/src/test.rs b/src/test.rs index 396a155..3c9e771 100644 --- a/src/test.rs +++ b/src/test.rs @@ -931,6 +931,7 @@ fn test_campaign_count_cannot_reset_after_deployment() { // Admin flows that must NOT reset the counter let new_admin = Address::generate(&env); client.update_admin(&admin, &new_admin); + client.accept_admin_transfer(); assert_eq!(client.get_campaign_count(), 3); client.set_voting_params(&new_admin, &5, &7000); @@ -2280,6 +2281,37 @@ fn test_deposit_revenue_non_existent_campaign() { assert_eq!(res.unwrap_err().unwrap(), Error::CampaignNotFound); } +#[test] +fn test_deposit_revenue_repeated_calls_accumulate_and_emit_events() { + 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( + &creator, + &String::from_str(&env, "Repeated Deposits"), + &String::from_str(&env, "Deposit idempotency"), + &1000, + &30, + &Category::EducationalStartup, + &true, + &2000, + &0i128, + ); + client.verify_campaign(&campaign_id); + client.contribute(&campaign_id, &contributor1, &1000); + client.withdraw_funds(&campaign_id); + + let events_before = env.events().all().len(); + for _ in 0..10 { + client.deposit_revenue(&campaign_id, &100); + } + let events_after = env.events().all().len(); + assert_eq!(client.get_revenue_pool(&campaign_id), 1000); + assert_eq!(events_after - events_before, 20); +} + // ── Issue 1: Validate refund state mutation order ──────────────────────────── #[test] @@ -2426,6 +2458,38 @@ fn test_claim_refund_expired_campaign() { client.claim_refund(&campaign_id, &contributor1); assert_eq!(client.get_contribution(&campaign_id, &contributor1), 0); assert_eq!(token.balance(&contributor1), 5000); + 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( + &creator, + &String::from_str(&env, "Refund Cleans Revenue Claim"), + &String::from_str(&env, "Ensure RevenueClaimed key is removed"), + &5000, + &30, + &Category::EducationalStartup, + &true, + &2000, + &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); } // ── Issue 3: Fuzz/Integration tests for vote_on_campaign ───────────────────── @@ -2599,6 +2663,63 @@ fn test_vote_on_cancelled_campaign_fails() { 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(); + token_admin.mint(&contributor1, &1000); + + let campaign_id = client.create_campaign( + &creator, + &String::from_str(&env, "Deadline Vote"), + &String::from_str(&env, "Voting deadline gate"), + &1000, + &1, + &Category::Learner, + &false, + &0, + &0i128, + ); + + let deadline = client.get_campaign(&campaign_id).deadline; + env.ledger().set(soroban_sdk::testutils::LedgerInfo { + timestamp: deadline + 1, + protocol_version: 22, + sequence_number: env.ledger().sequence(), + network_id: [0; 32], + base_reserve: 10, + min_temp_entry_ttl: 10, + min_persistent_entry_ttl: 10, + max_entry_ttl: 10, + }); + + let res = client.try_vote_on_campaign(&campaign_id, &contributor1, &true); + assert_eq!(res.unwrap_err().unwrap(), Error::CampaignNotActive); +} + +#[test] +fn test_vote_on_campaign_after_withdraw_fails() { + let (env, _admin, creator, contributor1, _, _token, token_admin, client) = setup_env(); + token_admin.mint(&contributor1, &2000); + + let campaign_id = client.create_campaign( + &creator, + &String::from_str(&env, "Withdrawn Vote"), + &String::from_str(&env, "Voting withdrawn gate"), + &1000, + &30, + &Category::Learner, + &false, + &0, + &0i128, + ); + client.verify_campaign(&campaign_id); + client.contribute(&campaign_id, &contributor1, &1000); + client.withdraw_funds(&campaign_id); + + let res = client.try_vote_on_campaign(&campaign_id, &contributor1, &true); + assert_eq!(res.unwrap_err().unwrap(), Error::CampaignNotActive); +} + #[test] fn test_vote_on_campaign_token_weighted() { let (env, _admin, creator, contributor1, contributor2, _token, token_admin, client) = diff --git a/src/update_admin_test.rs b/src/update_admin_test.rs index 1a970e5..d7c13f1 100644 --- a/src/update_admin_test.rs +++ b/src/update_admin_test.rs @@ -24,7 +24,13 @@ fn test_update_admin_success() { let res = client.try_update_admin(&admin, &new_admin); assert!(res.is_ok()); + assert_eq!(client.get_admin(), admin); + assert_eq!(client.get_pending_admin(), Some(new_admin.clone())); + + let accept_res = client.try_accept_admin_transfer(); + assert!(accept_res.is_ok()); assert_eq!(client.get_admin(), new_admin); + assert_eq!(client.get_pending_admin(), None); } #[test] @@ -35,3 +41,17 @@ fn test_update_admin_rejects_non_admin() { let res = client.try_update_admin(&creator, &new_admin); assert_eq!(res.unwrap_err().unwrap(), Error::NotAuthorized); } + +#[test] +fn test_cancel_admin_transfer() { + let (env, admin, _creator, client) = setup_env(); + let new_admin = Address::generate(&env); + + client.update_admin(&admin, &new_admin); + assert_eq!(client.get_pending_admin(), Some(new_admin)); + + let cancel_res = client.try_cancel_admin_transfer(&admin); + assert!(cancel_res.is_ok()); + assert_eq!(client.get_pending_admin(), None); + assert_eq!(client.get_admin(), admin); +} diff --git a/src/voting.rs b/src/voting.rs index e18d6ff..e0c4648 100644 --- a/src/voting.rs +++ b/src/voting.rs @@ -51,8 +51,11 @@ 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)?; - require_unverified_campaign(&campaign)?; require_active_campaign(&campaign)?; + if env.ledger().timestamp() > campaign.deadline { + return Err(Error::CampaignNotActive); + } + require_unverified_campaign(&campaign)?; let balance = token::Client::new(env, &get_token(env)).balance(&voter); if balance <= 0 { From e252b0da7dd2e82aab6aa3de79404c5d6db79455 Mon Sep 17 00:00:00 2001 From: JamesVictor-O Date: Sat, 25 Apr 2026 14:43:19 +0100 Subject: [PATCH 2/3] Format admin transfer cancellation event for CI rustfmt compliance. This applies rustfmt output so the build-and-test workflow passes cargo fmt --check on the PR branch. Made-with: Cursor --- src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index afeea23..c3fa95a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -972,7 +972,8 @@ impl ProofOfHeart { bump_instance_ttl(&env); remove_pending_admin(&env); - env.events().publish(("admin_transfer_cancelled",), current_admin); + env.events() + .publish(("admin_transfer_cancelled",), current_admin); Ok(()) } From d2852640a07fed286ac54d9e5c75ebb17c8ba87d Mon Sep 17 00:00:00 2001 From: JamesVictor-O Date: Sat, 25 Apr 2026 14:47:56 +0100 Subject: [PATCH 3/3] Update new regression tests to use CreateCampaignParams API. This aligns the four newly added tests with the upstream create_campaign signature used in CI. Made-with: Cursor --- src/test.rs | 88 ++++++++++++++++++++++++++--------------------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/src/test.rs b/src/test.rs index 3c9e771..0497601 100644 --- a/src/test.rs +++ b/src/test.rs @@ -2288,17 +2288,17 @@ fn test_deposit_revenue_repeated_calls_accumulate_and_emit_events() { token_admin.mint(&contributor1, &5000); token_admin.mint(&creator, &10_000); - let campaign_id = client.create_campaign( - &creator, - &String::from_str(&env, "Repeated Deposits"), - &String::from_str(&env, "Deposit idempotency"), - &1000, - &30, - &Category::EducationalStartup, - &true, - &2000, - &0i128, - ); + let campaign_id = client.create_campaign(&CreateCampaignParams { + creator: creator.clone(), + title: String::from_str(&env, "Repeated Deposits"), + description: String::from_str(&env, "Deposit idempotency"), + funding_goal: 1000, + 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.withdraw_funds(&campaign_id); @@ -2467,17 +2467,17 @@ fn test_claim_refund_clears_existing_revenue_claimed_key() { token_admin.mint(&contributor1, &5000); token_admin.mint(&creator, &10_000); - let campaign_id = client.create_campaign( - &creator, - &String::from_str(&env, "Refund Cleans Revenue Claim"), - &String::from_str(&env, "Ensure RevenueClaimed key is removed"), - &5000, - &30, - &Category::EducationalStartup, - &true, - &2000, - &0i128, - ); + 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); @@ -2668,17 +2668,17 @@ fn test_vote_on_campaign_past_deadline_fails() { let (env, _admin, creator, contributor1, _, _token, token_admin, client) = setup_env(); token_admin.mint(&contributor1, &1000); - let campaign_id = client.create_campaign( - &creator, - &String::from_str(&env, "Deadline Vote"), - &String::from_str(&env, "Voting deadline gate"), - &1000, - &1, - &Category::Learner, - &false, - &0, - &0i128, - ); + let campaign_id = client.create_campaign(&CreateCampaignParams { + creator: creator.clone(), + title: String::from_str(&env, "Deadline Vote"), + description: String::from_str(&env, "Voting deadline gate"), + funding_goal: 1000, + duration_days: 1, + category: Category::Learner, + has_revenue_sharing: false, + revenue_share_percentage: 0, + max_contribution_per_user: 0i128, + }); let deadline = client.get_campaign(&campaign_id).deadline; env.ledger().set(soroban_sdk::testutils::LedgerInfo { @@ -2701,17 +2701,17 @@ fn test_vote_on_campaign_after_withdraw_fails() { let (env, _admin, creator, contributor1, _, _token, token_admin, client) = setup_env(); token_admin.mint(&contributor1, &2000); - let campaign_id = client.create_campaign( - &creator, - &String::from_str(&env, "Withdrawn Vote"), - &String::from_str(&env, "Voting withdrawn gate"), - &1000, - &30, - &Category::Learner, - &false, - &0, - &0i128, - ); + let campaign_id = client.create_campaign(&CreateCampaignParams { + creator: creator.clone(), + title: String::from_str(&env, "Withdrawn Vote"), + description: String::from_str(&env, "Voting withdrawn gate"), + funding_goal: 1000, + duration_days: 30, + category: Category::Learner, + has_revenue_sharing: false, + revenue_share_percentage: 0, + max_contribution_per_user: 0i128, + }); client.verify_campaign(&campaign_id); client.contribute(&campaign_id, &contributor1, &1000); client.withdraw_funds(&campaign_id);