fix: cancel_campaign must decrement total_raised_global upfront (#439) - #615
Conversation
…l_campaign Track unclaimed refunds across cancelled campaigns via PendingRefundTotal, so accept_token_update can gate on both zero raised and zero pending refunds. - Add PendingRefundTotal to AdminKey storage - Subtract campaign.amount_raised from total_raised_global at cancel time - Increment pending_refund_total at cancel time - Decrement pending_refund_total in claim_refund for cancelled campaigns - Update accept_token_update gate to also check pending_refund_total == 0 - Update claim_refund to skip total_raised_global decrement for cancelled campaigns (already decremented at cancel) Co-authored-by: Promise Olubudo <254755326+baedboibidex-cmyk@users.noreply.github.com>
|
@baedboibidex-cmyk Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…ransfer (Iris-IV#453) Prevents users from wasting a Freighter signature when the contract is paused. The paused check now happens before pending.require_auth().
…ed payload The update_campaign function now emits (old_title, old_description, title, event_description) as a 4-tuple, but the tests were still unpacking as a 2-tuple (String, String), causing HostError(UnexpectedSize). Updated test_update_campaign_emits_title_and_description and test_update_campaign_event_tracks_latest_description to unpack the 4-tuple and assert against the correct indices (2 and 3 for new values).
…sonal_cap (Iris-IV#503) Reverts the broken duplicate contract and stray files shipped by two bad merges, and properly re-implements the feature they attempted: - Remove the stray `#[contract] ProofOfHeartContract` duplicate contract (list_active_campaigns with tag_filter, category max-goal caps) from lib.rs/admin.rs, leaving the real ProofOfHeart contract as the only one. - Delete orphaned files from Iris-IV#616/Iris-IV#618 that referenced the removed contract or were stray: src/proof_of_heart/, src/tests/voting_tests.rs, src/events.ts, src/types.ts, src/campaigns.rs, src/clients.rs, src/test.rs, and the frontend/ directory (no build setup; README references a separate frontend repo). - Re-implement issue Iris-IV#503 properly on the real contract: public remove_personal_cap(campaign_id, contributor) entrypoint that removes a contributor's personal contribution cap (requires contributor auth and an active campaign), new Error::PersonalCapNotFound (46), and a personal_cap_removed event. - Update docs: AUTHORIZATION.md, EVENT_PAYLOADS.md (count 49), CHANGELOG.md, and merge the duplicated sections of CAMPAIGN_LIFECYCLE.md into one coherent document (DataKey -> AdminKey renames). - Add tests covering restore/remove flow, event shape, not-found and inactive-campaign errors, and contributor auth recording. All 405 tests pass; cargo check, fmt, clippy clean.
…n_decrement_total_raised_global
…ront refund escrow
davidmaronio
left a comment
There was a problem hiding this comment.
the core design here is sound and thoughtful: decrementing total_raised_global upfront in cancel_campaign / admin_cancel_campaign (src/campaigns/cancel.rs), moving the escrow into a new PendingRefundTotal counter, decrementing that per-claimant in claim_refund (src/contributions.rs:285), and widening the accept_token_update gate in src/admin.rs:365 to include the pending refund total keeps the token-swap guard honest without double-counting. updating the get_platform_stats expectation with a comment explaining why is also the right move.
things to fix:
- scope: the diff carries a lot that has nothing to do with #439 - the whole
remove_personal_capentrypoint (#503) with its tests, error variant, CHANGELOG/EVENT_PAYLOADS/AUTHORIZATION doc entries, the duplicate-contract revert (src/proof_of_heart deletions, voting_tests.rs, frontend file deletions), and theaccept_campaign_transferpause reorder. some of this may be base drift since the branch is conflicted, but as it stands the PR is unreviewable as one unit. please rebase onto current main so the diff is only the #439 change. - migration hazard worth addressing (or at least documenting): a campaign cancelled before this change deploys has its escrow still counted in
total_raised_globaland nothing inPendingRefundTotal. after upgrade,claim_refundfor that campaign takes theis_cancelledbranch and doespending_refund.checked_sub(amount)on a zero counter, which returnsErr(Overflow)... actually checked_sub on i128 only errors on true overflow, so it would drivePendingRefundTotalnegative and leavetotal_raised_globalpermanently inflated, blocking token swaps forever. amigratestep or a floor-at-zero with an explicit comment is needed. - src/campaigns/cancel.rs - the same two-counter block is pasted into both
cancel_campaignandadmin_cancel_campaign. pull it into a small helper so the two paths can't drift. - overlap: #714, #604 and #718 touch the same accounting. worth agreeing with the maintainers which approach wins before polishing further.
gate: this branch has merge conflicts with main. please resolve them (which should also shed the unrelated changes), address point 2, and push.
closes #439
Decrement total_raised_global immediately in cancel_campaign and admin_cancel_campaign so cancelled campaigns with unclaimed refunds no longer inflate global stats.
Also introduces PendingRefundTotal storage counter to track unclaimed refunds across cancelled campaigns, so accept_token_update can reliably block when escrowed refunds remain (even after total_raised_global is zeroed at cancel time).
Changes:
Co-authored-by: Promise Olubudo 254755326+baedboibidex-cmyk@users.noreply.github.com