fix: subtract cancelled campaign amount from total_raised_global at cancellation time - #714
fix: subtract cancelled campaign amount from total_raised_global at cancellation time#714Nife-tanny wants to merge 1 commit into
Conversation
…ancellation time Closes Iris-IV#455 ## Root Cause When a campaign was cancelled, total_raised_global was only decremented on a per-contributor basis inside claim_refund. If contributors never called claim_refund, the cancelled campaign's refundable amount remained permanently included in the platform-wide total, inflating statistics and — more critically — blocking accept_token_update indefinitely. ## Fix 1. cancel_campaign and admin_cancel_campaign now subtract campaign.amount_raised from total_raised_global at cancellation time using checked arithmetic, removing the full claimable amount before any individual refund occurs. 2. claim_refund now skips the total_raised_global decrement when the campaign is cancelled (is_cancelled == true), because the amount was already removed at cancellation time. For expired/failed (non-cancelled) campaigns, the decrement still happens as before. ## Invariant After cancellation: total_raised_global == previous_global - cancelled_campaign.amount_raised A cancelled campaign's refund liability is no longer represented in total_raised_global. Individual refund claims do not subtract the same amount again, preventing double subtraction. ## Tests Added - Cancellation removes claimable amount from global total - Unclaimed refund does not remain globally raised (reproduces the bug) - Refund claim after cancel does not double-decrement - Multiple campaigns cancel accounting (A=100, B=200, cancel both) - Zero-value campaign cancel (no underflow) - Token migration no longer blocked by unclaimed cancelled refund
|
@Nife-tanny 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! 🚀 |
davidmaronio
left a comment
There was a problem hiding this comment.
really solid test coverage here, the five regression tests in test_queries.rs (double-decrement, multi-campaign accounting, zero-value underflow) are exactly the kind of coverage this area needs, and the checked_sub with Error::Overflow is the right arithmetic hygiene.
but there's a safety regression hiding in the semantics change:
-
src/campaigns/cancel.rs (both cancel paths) + src/contributions.rs claim_refund: main deliberately uses
total_raised_globalas the escrow-liveness gate for token migration.accept_token_updatein src/admin.rs (line 361-371 on main) blocks the swap whiletotal_raised_global != 0precisely because "cancel_campaign drops the active count immediately, but contributor refunds stay escrowed until each contributor calls claim_refund, which pays out in the current token". by subtracting at cancellation time and skipping the decrement in claim_refund for cancelled campaigns, this PR lets the admin swap tokens while refunds are still escrowed in the old token. after the swap, claim_refund transfers the NEW token, which the contract doesn't hold for those contributors. their refunds either trap or drain unrelated new-token balances. -
src/tests/test_admin.rs: the PR rewrites test_token_swap_blocked_with_unrefunded_cancelled_campaign into its opposite. that test existed to pin exactly this hazard (issue #407 follow-up), so flipping it is deleting the guard rather than satisfying it. fix: keep total_raised_global as the escrow gate, and solve the stats-inflation side of #455 differently, either a separate display counter for platform stats, or a dedicated outstanding-escrow counter that accept_token_update checks while the stats query subtracts cancelled amounts.
-
heads up that this overlaps #604 and #718 in the #438 family, worth coordinating with the maintainers on which approach wins before reworking, so effort isn't duplicated.
also note the CI red is partly stale-base: the two test_campaign_update failures are main's 4-tuple event payload change that your branch predates, a rebase clears those.
gate: rework the escrow gating as above, then resolve the conflicts with main and get CI green before this can move.
Closes #455
Root Cause
When a campaign was cancelled,
total_raised_globalwas only decremented on a per-contributor basis insideclaim_refund. If contributors never calledclaim_refund, the cancelled campaign's refundable amount remained permanently included in the platform-wide total — inflating statistics and, more critically, blockingaccept_token_updateindefinitely.Fix
cancel_campaignandadmin_cancel_campaign: subtractcampaign.amount_raisedfromtotal_raised_globalat cancellation time using checked arithmetic, removing the full claimable amount before any individual refund occurs.claim_refund: skips thetotal_raised_globaldecrement when the campaign is cancelled (is_cancelled == true), because the amount was already removed at cancellation time. For expired/failed (non-cancelled) campaigns, the decrement still happens as before — no double subtraction.Invariant
total_raised_globalafter cancellation = previous global total − cancelled campaign'samount_raisedA cancelled campaign's refund liability is no longer represented in
total_raised_global. Individual refund claims do not subtract the same amount again.Tests Added / Updated
test_get_platform_stats_returns_aggregates(total drops from 700→400 after cancelling campaign with 300)test_get_creator_stats_returns_aggregates—get_creator_statssumscampaign.amount_raiseddirectly, nottotal_raised_global, so it still reports all 700test_token_swap_blocked_with_unrefunded_cancelled_campaign→test_token_swap_succeeds_after_cancelled_campaign_with_unclaimed_refundValidation
Pre-existing Failures (unrelated to this change)
Two tests in
src/tests/test_campaign_update.rsfail withHostError: Error(Object, UnexpectedSize)— a Soroban SDK event-vector unpacking issue where an event payload's tuple size mismatches at runtime. These failures existed before this change and are not related to thetotal_raised_globalaccounting fix:tests::test_campaign_update::test_update_campaign_emits_title_and_descriptiontests::test_campaign_update::test_update_campaign_event_tracks_latest_descriptionChanged Files
src/campaigns/cancel.rssrc/contributions.rssrc/tests/test_admin.rssrc/tests/test_queries.rs4 files changed, 237 insertions, 21 deletions