fix(campaign-escrow): finalize state writes before token transfers - #72
Merged
JamesVictor-O merged 6 commits intoAug 22, 2026
Conversation
… claim_payment Reorders claim_payment so application.status, campaign.escrow_balance and campaign.committed_payouts are all persisted before the fee and net token.transfer calls are issued, following checks-effects- interactions instead of interactions-then-effects.
…cancel_campaign Persists campaign.escrow_balance and campaign.status before issuing the refund transfer, matching checks-effects-interactions.
…expire_campaign Persists campaign.escrow_balance and campaign.status before issuing the refund transfer, matching checks-effects-interactions.
…emergency_recover_campaign Persists campaign.escrow_balance and campaign.status before sweeping the unallocated remainder to treasury, matching checks-effects-interactions.
…reclaim_surplus Persists campaign.escrow_balance and campaign.status before issuing the surplus transfer, matching checks-effects-interactions.
… resolve_dispute Persists application.status/frozen/dispute_opened_at and campaign.escrow_balance/committed_payouts/status before issuing the fee, creator and business transfers, matching checks-effects-interactions.
JamesVictor-O
approved these changes
Aug 22, 2026
JamesVictor-O
left a comment
JamesVictor-O
left a comment
Contributor
There was a problem hiding this comment.
Automated review: verified all six reordered functions (claim_payment, resolve_dispute, cancel_campaign, expire_campaign, reclaim_surplus, emergency_recover_campaign) function by function — every transfer amount is snapshotted before the moved storage writes, no mutated field is read stale, and no transfer return value is consumed. Confirmed against Soroban host source (soroban-env-host) that a trapping transfer rolls back all prior storage writes in the same invocation, so this reordering is behaviorally pure — no double-payout or stuck-fund window either way. Existing tests re-validate real token balances for all six paths; CI clean (build, clippy, format, test). This is solid checks-effects-interactions hygiene. Approving.
This was referenced Aug 22, 2026
Promise278
added a commit
to Promise278/ads-bazaar-contract
that referenced
this pull request
Aug 24, 2026
…dispute - Use try_close_dispute (fallible) instead of close_dispute (infallible) so a broken/unset dispute-resolution contract cannot brick the admin settlement path. The close-out is now best-effort: state writes and token transfers are committed first atomically. - Reorder resolve_dispute to finalize state writes before token transfers (aligns with PR Ads-Bazaar#72 pattern): if a transfer traps, the whole invocation reverts cleanly — no partial-failure window. - Add DisputeOutcome to the DisputeResolved event so indexers/frontends can distinguish PayCreator/RefundBusiness/Split without ratio inference. - Rewrite stale doc comment that described the admin path as working 'without' dispute-resolution (it now calls try_close_dispute on every resolution, best-effort).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every fund-moving function in
campaign-escrowissued itstoken::Client::transfercall(s) before finalizing the corresponding storage writes, inverting checks-effects-interactions. This reorders all six affected functions so storage writes land first and transfers are issued last:claim_paymentresolve_disputecancel_campaignexpire_campaignreclaim_surplusemergency_recover_campaignIn each case, amounts and addresses needed for the transfer (fee, net, refund, surplus, recovered, treasury, business) are computed/read before the storage write, then the
token::Clientand transfer calls are moved afterstorage::set_campaign/storage::set_application. This is a pure reordering — no balances, statuses, or event payloads change as a result.Test plan
campaign-escrow/src/test.rs,campaign-escrow/tests/integration.rs) passes unchangedcloses #63