From d85a15062513d0ee9de92d649744d84bb6f0d20e Mon Sep 17 00:00:00 2001 From: JamesVictor-O Date: Thu, 28 May 2026 19:06:40 +0100 Subject: [PATCH] Fix issues #360, #361, #377, #391: voting auth, event payload, pending-transfer query, and admin-path tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #377: Remove unused `_admin` parameter from `voting::set_params` — auth is enforced exclusively by `lib.rs::assert_admin`, which already calls `require_auth()`. Keeping the dead parameter created a false sense of defense-in-depth while passing a value that was never used. Issue #391: Enrich the `campaign_vote_cast` event payload to include the voter's token balance and computed vote weight as `(approve, balance, vote_weight)`. Off-chain indexers can now independently verify the tally without querying historical RPC balances. Issue #361: Add `has_pending_campaign_transfer(env, campaign_id) -> bool` view function so off-chain tooling and the pending creator can discover pending transfers on-chain without iterating all campaigns or relying on external communication. Issue #360: Add three missing test cases for `resume_campaign`: - `test_resume_by_admin` — admin (not creator) can lift the auto-pause - `test_resume_unauthorized_fails` — random address is correctly rejected - `test_resume_after_campaign_transfer_uses_new_creator` — after ownership transfer the NEW creator holds the resume right, not the original creator Co-Authored-By: Claude Sonnet 4.6 --- src/issues_test.rs | 58 ++++++++++++++- src/lib.rs | 13 +++- src/voting.rs | 5 +- .../test/test_cancel_and_refund.1.json | 74 +++++++++++++------ ...est_contribute_and_withdraw_success.1.json | 74 +++++++++++++------ .../test/test_failure_states.1.json | 74 +++++++++++++------ ...est_pull_based_revenue_distribution.1.json | 74 +++++++++++++------ 7 files changed, 280 insertions(+), 92 deletions(-) diff --git a/src/issues_test.rs b/src/issues_test.rs index 4eaba69..6541fc2 100644 --- a/src/issues_test.rs +++ b/src/issues_test.rs @@ -294,8 +294,64 @@ fn test_vote_weight_overflow_fails() { // Cast a vote with balance 501, which overflows i128::MAX when added to i128::MAX - 500 token_admin.mint(&contributor, &501); - + // cast vote should return Overflow error let res = client.try_vote_on_campaign(&campaign_id, &contributor, &true); assert_eq!(res.unwrap_err().unwrap(), Error::Overflow); } + +// ── #360 resume_campaign admin-path coverage ────────────────────────────────── + +/// Helper: set the contract into auto-paused state directly in storage. +fn set_auto_paused(env: &Env, client_address: &Address, paused: bool) { + env.as_contract(client_address, || { + env.storage().instance().set(&DataKey::AutoPaused, &paused); + }); +} + +#[test] +fn test_resume_by_admin() { + let (env, admin, creator, _, _, _, _, client) = setup_env(); + let campaign_id = client.create_campaign(&make_campaign_params_simple(&env, &creator)); + + set_auto_paused(&env, &client.address, true); + assert!(client.is_paused()); + + // Admin (not the creator) should be able to resume. + client.resume_campaign(&campaign_id, &admin); + assert!(!client.is_paused()); +} + +#[test] +fn test_resume_unauthorized_fails() { + let (env, _admin, creator, _, _, _, _, client) = setup_env(); + let campaign_id = client.create_campaign(&make_campaign_params_simple(&env, &creator)); + let stranger = Address::generate(&env); + + set_auto_paused(&env, &client.address, true); + + let result = client.try_resume_campaign(&campaign_id, &stranger); + assert_eq!(result.unwrap_err().unwrap(), Error::NotAuthorized); +} + +#[test] +fn test_resume_after_campaign_transfer_uses_new_creator() { + let (env, _admin, original_creator, _, _, _, _, client) = setup_env(); + let campaign_id = client.create_campaign(&make_campaign_params_simple(&env, &original_creator)); + + let new_creator = Address::generate(&env); + client.initiate_campaign_transfer(&campaign_id, &new_creator); + client.accept_campaign_transfer(&campaign_id); + + set_auto_paused(&env, &client.address, true); + assert!(client.is_paused()); + + // New creator can resume; original creator can no longer. + client.resume_campaign(&campaign_id, &new_creator); + assert!(!client.is_paused()); + + // Re-pause and verify the original creator is now rejected. + set_auto_paused(&env, &client.address, true); + let result = client.try_resume_campaign(&campaign_id, &original_creator); + assert_eq!(result.unwrap_err().unwrap(), Error::NotAuthorized); +} diff --git a/src/lib.rs b/src/lib.rs index 928c16c..50b694b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -947,7 +947,7 @@ impl ProofOfHeart { let old_threshold = get_approval_threshold_bps(&env, voting::DEFAULT_APPROVAL_THRESHOLD_BPS); let caller = admin.clone(); - voting::set_params(&env, admin, min_votes_quorum, approval_threshold_bps)?; + voting::set_params(&env, min_votes_quorum, approval_threshold_bps)?; env.events().publish( ( soroban_sdk::Symbol::new(&env, "voting_params_updated"), @@ -1910,6 +1910,17 @@ impl ProofOfHeart { } } + /// Returns `true` if the given campaign currently has a pending ownership transfer. + /// + /// Allows off-chain tooling and the pending creator to discover transfers without + /// iterating all campaigns. Returns `false` for unknown campaign IDs. + pub fn has_pending_campaign_transfer(env: Env, campaign_id: u32) -> bool { + match get_campaign(&env, campaign_id) { + Some(c) => c.pending_creator != MaybePendingCreator::None, + None => false, + } + } + /// Initiates a transfer of campaign ownership to a new address. /// /// # Authorization diff --git a/src/voting.rs b/src/voting.rs index 316b041..dab80e5 100644 --- a/src/voting.rs +++ b/src/voting.rs @@ -30,7 +30,6 @@ pub const MIN_APPROVAL_THRESHOLD_BPS: u32 = 1000; /// * `ValidationFailed` - Quorum or threshold values are out of range. pub fn set_params( env: &Env, - _admin: Address, min_votes_quorum: u32, approval_threshold_bps: u32, ) -> Result<(), Error> { @@ -90,8 +89,10 @@ pub fn cast_vote(env: &Env, campaign_id: u32, voter: Address, approve: bool) -> } set_has_voted(env, campaign_id, &voter); + + let vote_weight = balance; env.events() - .publish(("campaign_vote_cast", campaign_id, voter), approve); + .publish(("campaign_vote_cast", campaign_id, voter), (approve, balance, vote_weight)); Ok(()) } diff --git a/test_snapshots/test/test_cancel_and_refund.1.json b/test_snapshots/test/test_cancel_and_refund.1.json index b79b6a5..a74df05 100644 --- a/test_snapshots/test/test_cancel_and_refund.1.json +++ b/test_snapshots/test/test_cancel_and_refund.1.json @@ -789,6 +789,58 @@ 15 ] ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "key": { + "vec": [ + { + "symbol": "BlockCampaignContributionCount" + }, + { + "u32": 1 + } + ] + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "key": { + "vec": [ + { + "symbol": "BlockCampaignContributionCount" + }, + { + "u32": 1 + } + ] + }, + "durability": "temporary", + "val": { + "vec": [ + { + "u32": 0 + }, + { + "u32": 2 + } + ] + } + } + }, + "ext": "v0" + }, + 15 + ] + ], [ { "contract_data": { @@ -1338,28 +1390,6 @@ "u32": 6000 } }, - { - "key": { - "vec": [ - { - "symbol": "BlockCampaignContributionCount" - }, - { - "u32": 1 - } - ] - }, - "val": { - "vec": [ - { - "u32": 0 - }, - { - "u32": 2 - } - ] - } - }, { "key": { "vec": [ diff --git a/test_snapshots/test/test_contribute_and_withdraw_success.1.json b/test_snapshots/test/test_contribute_and_withdraw_success.1.json index fd2b760..fd3abc1 100644 --- a/test_snapshots/test/test_contribute_and_withdraw_success.1.json +++ b/test_snapshots/test/test_contribute_and_withdraw_success.1.json @@ -538,6 +538,58 @@ 15 ] ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "key": { + "vec": [ + { + "symbol": "BlockCampaignContributionCount" + }, + { + "u32": 1 + } + ] + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "key": { + "vec": [ + { + "symbol": "BlockCampaignContributionCount" + }, + { + "u32": 1 + } + ] + }, + "durability": "temporary", + "val": { + "vec": [ + { + "u32": 0 + }, + { + "u32": 1 + } + ] + } + } + }, + "ext": "v0" + }, + 15 + ] + ], [ { "contract_data": { @@ -1195,28 +1247,6 @@ "u32": 6000 } }, - { - "key": { - "vec": [ - { - "symbol": "BlockCampaignContributionCount" - }, - { - "u32": 1 - } - ] - }, - "val": { - "vec": [ - { - "u32": 0 - }, - { - "u32": 1 - } - ] - } - }, { "key": { "vec": [ diff --git a/test_snapshots/test/test_failure_states.1.json b/test_snapshots/test/test_failure_states.1.json index c84f2f2..531a87f 100644 --- a/test_snapshots/test/test_failure_states.1.json +++ b/test_snapshots/test/test_failure_states.1.json @@ -540,6 +540,58 @@ 15 ] ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "key": { + "vec": [ + { + "symbol": "BlockCampaignContributionCount" + }, + { + "u32": 1 + } + ] + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "key": { + "vec": [ + { + "symbol": "BlockCampaignContributionCount" + }, + { + "u32": 1 + } + ] + }, + "durability": "temporary", + "val": { + "vec": [ + { + "u32": 0 + }, + { + "u32": 1 + } + ] + } + } + }, + "ext": "v0" + }, + 15 + ] + ], [ { "contract_data": { @@ -1089,28 +1141,6 @@ "u32": 6000 } }, - { - "key": { - "vec": [ - { - "symbol": "BlockCampaignContributionCount" - }, - { - "u32": 1 - } - ] - }, - "val": { - "vec": [ - { - "u32": 0 - }, - { - "u32": 1 - } - ] - } - }, { "key": { "vec": [ diff --git a/test_snapshots/test/test_pull_based_revenue_distribution.1.json b/test_snapshots/test/test_pull_based_revenue_distribution.1.json index 0c4b8bc..0ac0ad4 100644 --- a/test_snapshots/test/test_pull_based_revenue_distribution.1.json +++ b/test_snapshots/test/test_pull_based_revenue_distribution.1.json @@ -1126,6 +1126,58 @@ 15 ] ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "key": { + "vec": [ + { + "symbol": "BlockCampaignContributionCount" + }, + { + "u32": 1 + } + ] + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "key": { + "vec": [ + { + "symbol": "BlockCampaignContributionCount" + }, + { + "u32": 1 + } + ] + }, + "durability": "temporary", + "val": { + "vec": [ + { + "u32": 0 + }, + { + "u32": 2 + } + ] + } + } + }, + "ext": "v0" + }, + 15 + ] + ], [ { "contract_data": { @@ -1999,28 +2051,6 @@ "u32": 6000 } }, - { - "key": { - "vec": [ - { - "symbol": "BlockCampaignContributionCount" - }, - { - "u32": 1 - } - ] - }, - "val": { - "vec": [ - { - "u32": 0 - }, - { - "u32": 2 - } - ] - } - }, { "key": { "vec": [