fix: let admin call freeze_for_dispute directly - #51
Merged
JamesVictor-O merged 1 commit intoJul 27, 2026
Merged
Conversation
…olution claim_payment already rejects a frozen application (PayoutFrozen), and resolve_dispute already clears the flag when it settles — that race-closing logic landed with PR Ads-Bazaar#49's freeze_for_dispute/raise_dispute wiring, before this issue was picked up. What was still missing per Ads-Bazaar#39's "Rules" section: freeze_for_dispute was reachable only through the dispute-resolution contract's raise_dispute (itself only callable by the creator or business), so admin had no direct way to freeze a payout — unlike resolve_dispute, which admin can already call on its own. Give freeze_for_dispute an explicit caller argument, authorized as either the configured dispute_contract or admin, so an emergency freeze doesn't require routing through raise_dispute. Also fills a small test gap: verifies that after resolve_dispute clears a freeze and marks the application Paid, a further claim_payment attempt still correctly fails (via the ordinary already-Paid guard). Closes Ads-Bazaar#39
Closed
6 tasks
JamesVictor-O
pushed a commit
that referenced
this pull request
Jul 27, 2026
cancel_campaign, expire_campaign, and reclaim_surplus all require business.require_auth(), which is correct day-to-day (no hidden admin release path) but leaves escrow permanently stuck if a business's signing key is ever lost, rotated away from, or the account otherwise abandoned. Adds emergency_recover_campaign: admin-only, gated on completion_deadline plus a new EMERGENCY_RECOVERY_GRACE_PERIOD (~6 months) on top, so it's deliberately much harder to reach than expire_campaign and can never substitute for the normal exit paths. It only ever sweeps the unallocated remainder (escrow_balance - committed_payouts), mirroring the existing cancel/expire/reclaim pattern exactly — a payout already committed to an approved creator stays reserved and claimable regardless of how long the business has been unreachable. Recovered funds go to treasury rather than back to business, since the whole premise is that the business's address is unreachable. Emits a distinct EmergencyRecovery event so the path stays auditable on its own. Closes #48 Verified locally in an isolated worktree before merge: - cargo fmt --all -- --check - cargo build --workspace - cargo test --workspace (103 tests: 86 escrow + 17 dispute-resolution) - cargo clippy --workspace --all-targets -- -D warnings - cargo build --workspace --target wasm32v1-none --release - Confirmed via git merge-tree that this combines cleanly with #51 (merged just before it) — freeze_for_dispute's new caller argument and this PR's emergency_recover_campaign coexist correctly with no regression.
Contributor
|
Merged, thanks — good catch on the gap and a clean, minimal fix. The explicit-caller-plus-comparison pattern matches resolve_dispute exactly, and updating both call sites (the new admin path and dispute-resolution's raise_dispute) in the same PR avoided leaving anything broken in between. Verified before merging: fmt, full workspace build, all 99 tests, clippy -D warnings, and the wasm32v1-none release build. CI is green on main: https://github.com/Ads-Bazaar/ads-bazaar-contract/actions/runs/30230403166 |
4 tasks
JamesVictor-O
pushed a commit
that referenced
this pull request
Jul 29, 2026
…resolution (#54) Adds contracts/campaign-escrow/tests/integration.rs — 15 tests that register both CampaignEscrowContract and DisputeResolutionContract in the same Env and exercise real Soroban cross-contract call semantics (auth checks, balance correctness, and the freeze/claim lifecycle) that single-contract unit tests can't see. Covers: dual initialization wiring, raise_dispute freezing the payout across the contract boundary (both creator- and business-raised), frozen-payout claim rejection, full dispute-record field verification, balance-level assertions for all three admin resolve_dispute outcomes (PayCreator/RefundBusiness/Split), stranger rejection on freeze_for_dispute, the admin emergency-bypass path from #51, cross-creator isolation, duplicate-raise rejection, already-paid rejection, and independent per-creator disputes on the same campaign. Closes #45 Verified locally in an isolated worktree before merge: - cargo fmt --all -- --check — this initially failed on a few lines in the new integration.rs; applied rustfmt and pushed the fix directly to the PR branch (maintainer-edits were enabled) - cargo build --workspace - cargo test --workspace (87 escrow unit + 15 integration + 17 dispute-resolution = 119 tests) - cargo clippy --workspace --all-targets -- -D warnings - cargo build --workspace --target wasm32v1-none --release
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.
Problem
Closes #39.
claim_paymenthad no awareness of disputes, letting a creator front-run an unfavorableresolve_disputeoutcome by claiming first. Most of the fix for this had already landed by the time this issue was picked up, via PR #49'sfreeze_for_dispute/raise_disputewiring:freeze_for_disputesets a per-applicationfrozenflag (no longertodo!())claim_paymentrejects a frozen application withError::PayoutFrozen, regardless ofproof_approvedor the deadlineresolve_disputeclearsfrozenwhen it settles an applicationWhat was still missing, per this issue's "Rules" section:
In the current code,
freeze_for_disputeis reachable only throughdispute-resolution::raise_dispute, which in turn only lets thecreatoror campaignbusinesscall it. Admin had no direct path to freeze a payout — unlikeresolve_dispute, which admin can already call unilaterally. That's a real gap: an admin who spots a suspicious claim-in-flight (outside the normalraise_disputeflow) had no way to intervene before this PR.Fix
freeze_for_disputenow takes an explicitcaller: Addressargument,require_auth()'d and checked against either the configureddispute_contractoradmin(mirrors the explicit-address-plus-comparison pattern already used byrequire_admin/resolve_disputeelsewhere in this contract).dispute-resolution::raise_disputenow passes its own contract address (env.current_contract_address()) ascallerwhen it invokesescrow.freeze_for_dispute(...), preserving the existing behavior for the normal dispute-raising flow.freeze_for_dispute, theApplication.frozenfield, and theDisputeFrozenevent to describe the dual auth path instead of only the dispute-resolution contract.Test coverage added
admin_can_freeze_directly— admin freezes without going throughraise_dispute; the frozen application then correctly rejectsclaim_payment.freeze_rejects_uninvolved_stranger— replaces the oldfreeze_requires_dispute_contract_authmock-auth test now that the function takes an explicit caller; asserts a caller that's neitheradminnor the dispute contract getsError::Unauthorizedand the application stays unfrozen.admin_resolve_dispute_settles_and_clears_freezeextended to also assert that, afterresolve_disputeclears the freeze and marks the applicationPaid, a furtherclaim_paymentattempt still correctly fails (via the ordinary already-Paidguard, notPayoutFrozen) — this was one of the acceptance-criteria test cases not yet covered.All existing
freeze_for_disputetests updated to pass the newcallerargument (the bootstrap-configured dispute contract address, matching production usage).Test plan
cargo test --workspace— 82 + 17 tests pass incampaign-escrow/dispute-resolutioncargo clippy --workspace --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleancargo build --workspace --target wasm32v1-none --release— builds