diff --git a/contracts/campaign-escrow/src/dispute.rs b/contracts/campaign-escrow/src/dispute.rs new file mode 100644 index 0000000..86135d5 --- /dev/null +++ b/contracts/campaign-escrow/src/dispute.rs @@ -0,0 +1,30 @@ +//! Client for the narrow slice of `dispute-resolution` that this contract calls. +//! +//! Declared locally with `#[contractclient]` rather than depending on the +//! `ads-bazaar-dispute-resolution` crate: linking that crate into this one would +//! pull its `#[contractimpl]` exports into this contract's wasm, so both +//! contracts' entry points would ship in a single binary. Keep these +//! signatures in sync with `dispute-resolution/src/lib.rs`. This mirrors the +//! local `escrow` client in `dispute-resolution/src/escrow.rs`. +//! +//! `close_dispute` is declared infallible even though the dispute-resolution +//! contract returns `Result<_, Error>`. The encoding is identical on success, +//! and an error from the callee traps the whole invocation. Callers that need +//! to survive a broken/unset dispute-resolution contract should use the +//! auto-generated `try_close_dispute` wrapper instead, which recovers from +//! the trap and lets the admin settlement path proceed. +#![allow(dead_code)] + +use ads_bazaar_shared::{CampaignId, DisputeOutcome}; +use soroban_sdk::{contractclient, Address, Env}; + +#[contractclient(name = "DisputeResolutionClient")] +pub trait DisputeContract { + fn close_dispute( + env: Env, + caller: Address, + campaign_id: CampaignId, + creator: Address, + outcome: DisputeOutcome, + ); +} diff --git a/contracts/campaign-escrow/src/events.rs b/contracts/campaign-escrow/src/events.rs index 4999d2a..30bfaf0 100644 --- a/contracts/campaign-escrow/src/events.rs +++ b/contracts/campaign-escrow/src/events.rs @@ -7,7 +7,7 @@ //! matching point in `lib.rs` as each `todo!()` handler is implemented. #![allow(dead_code)] -use ads_bazaar_shared::CampaignId; +use ads_bazaar_shared::{CampaignId, DisputeOutcome}; use soroban_sdk::{contractevent, Address, BytesN, String}; #[contractevent] @@ -174,6 +174,7 @@ pub struct DisputeResolved { pub campaign_id: CampaignId, #[topic] pub creator: Address, + pub dispute_outcome: DisputeOutcome, pub creator_amount: i128, pub business_amount: i128, } diff --git a/contracts/campaign-escrow/src/lib.rs b/contracts/campaign-escrow/src/lib.rs index e620be8..b4e6de7 100644 --- a/contracts/campaign-escrow/src/lib.rs +++ b/contracts/campaign-escrow/src/lib.rs @@ -12,6 +12,7 @@ //! USDC, etc. without per-asset special-casing. #![no_std] +mod dispute; mod error; mod events; mod storage; @@ -20,7 +21,9 @@ mod types; pub use error::Error; pub use types::{Application, Campaign, DisputeResolution, ProtocolConfig}; -use ads_bazaar_shared::{ApplicationStatus, CampaignId, CampaignStatus, PayoutAsset}; +use ads_bazaar_shared::{ + ApplicationStatus, CampaignId, CampaignStatus, DisputeOutcome, PayoutAsset, +}; use soroban_sdk::{contract, contractimpl, token, Address, BytesN, Env, String}; /// Version string stored at `initialize` time. `upgrade` swaps the WASM @@ -881,11 +884,7 @@ impl CampaignEscrowContract { } /// Admin-resolved settlement for a single creator's committed-but-not- - /// yet-paid application, as a simplified interim path alongside the - /// arbiter-resolved `dispute-resolution` contract (`resolve_dispute_payout` - /// above is the intended integration point for that contract once it's - /// implemented; this is a separate admin-only shortcut that works today - /// without it). Admin-only. + /// yet-paid application. Admin-only. /// /// Requires an application with a nonzero `payout_amount` that hasn't /// already been paid — i.e. one that was approved via `approve_creator` @@ -898,13 +897,25 @@ impl CampaignEscrowContract { /// /// - The application must have been frozen by `freeze_for_dispute` /// (`Error::NoDisputeOpen` otherwise). Admin can call that themselves, - /// so this is not a dependency on the `dispute-resolution` contract — - /// but it does mean every admin settlement is preceded by a public - /// `events::DisputeFrozen`, which is what gives the counterparty + /// so this is not a hard dependency on the `dispute-resolution` + /// contract — but it does mean every admin settlement is preceded by a + /// public `events::DisputeFrozen`, which is what gives the counterparty /// something to notice. /// - At least `MIN_EVIDENCE_WINDOW` must have elapsed since that freeze /// (`Error::EvidenceWindowOpen` otherwise), so neither party's payout /// can be reallocated out from under them without warning. + /// + /// Reconciliation with `dispute-resolution`: when the frozen payout was + /// frozen via `raise_dispute` (the normal flow), this contract closes the + /// corresponding open dispute record out by calling + /// `dispute-resolution::close_dispute` with the `DisputeOutcome` that + /// matches `resolution`, so `get_dispute` never reports a permanently + /// open dispute over money that has moved. When the freeze came from the + /// admin's direct `freeze_for_dispute` path there is no record to close, + /// and the call is a no-op. Uses `try_close_dispute` (the fallible + /// wrapper) so that a broken or unset dispute-resolution contract cannot + /// brick the admin settlement path — state writes and token transfers + /// are committed first, and the close-out is best-effort. pub fn resolve_dispute( env: Env, admin: Address, @@ -939,16 +950,22 @@ impl CampaignEscrowContract { let payout_amount = application.payout_amount; let fee_bps = campaign.fee_bps; - let (creator_gross, business_amount) = match resolution { - DisputeResolution::PayCreator => (payout_amount, 0), - DisputeResolution::RefundBusiness => (0, payout_amount), + let (creator_gross, business_amount, dispute_outcome) = match resolution { + DisputeResolution::PayCreator => (payout_amount, 0, DisputeOutcome::CreatorFavored), + DisputeResolution::RefundBusiness => { + (0, payout_amount, DisputeOutcome::BusinessFavored) + } DisputeResolution::Split(bps) => { if !(0..=ads_bazaar_shared::BASIS_POINTS_DENOMINATOR).contains(&bps) { return Err(Error::InvalidAmount); } let creator_gross = payout_amount.checked_mul(bps).ok_or(Error::InvalidAmount)? / ads_bazaar_shared::BASIS_POINTS_DENOMINATOR; - (creator_gross, payout_amount - creator_gross) + ( + creator_gross, + payout_amount - creator_gross, + DisputeOutcome::Split(bps), + ) } }; @@ -998,6 +1015,7 @@ impl CampaignEscrowContract { events::DisputeResolved { campaign_id, creator, + dispute_outcome, creator_amount: creator_net, business_amount, } diff --git a/contracts/campaign-escrow/src/test.rs b/contracts/campaign-escrow/src/test.rs index c2a80fc..126443c 100644 --- a/contracts/campaign-escrow/src/test.rs +++ b/contracts/campaign-escrow/src/test.rs @@ -55,6 +55,13 @@ mod test_helpers { /// Initialize the contract (admin + dispute contract + fee_bps) and mint /// `BUSINESS_FUNDS` to a freshly generated business address. Returns the /// client plus the generated identities. + /// + /// The dispute slot is a real, initialized `dispute-resolution` contract + /// rather than a bare generated address: `resolve_dispute` closes out an + /// open dispute there with a cross-contract call, so a placeholder + /// address would trap. No dispute-resolution record is created unless a + /// test explicitly goes through `raise_dispute`, so the close-out call is + /// a no-op for most tests here. pub fn bootstrap<'a>( env: &'a Env, contract_id: &Address, @@ -68,9 +75,11 @@ mod test_helpers { ) { let client = CampaignEscrowContractClient::new(env, contract_id); let admin = Address::generate(env); - let dispute = Address::generate(env); + let dispute = env.register(ads_bazaar_dispute_resolution::DisputeResolutionContract, ()); let business = Address::generate(env); client.initialize(&admin, &dispute, &fee_bps); + ads_bazaar_dispute_resolution::DisputeResolutionContractClient::new(env, &dispute) + .initialize(&admin, contract_id); let token = setup_token(env, &business, BUSINESS_FUNDS); (client, admin, dispute, business, token) } diff --git a/contracts/campaign-escrow/tests/integration.rs b/contracts/campaign-escrow/tests/integration.rs index d740d67..85f3d8d 100644 --- a/contracts/campaign-escrow/tests/integration.rs +++ b/contracts/campaign-escrow/tests/integration.rs @@ -29,6 +29,7 @@ //! | 14 | Raise in auto-approval window still blocks claim | PayoutFrozen | //! | 15 | Two creators on same campaign can have independent disputes | separate dispute ids | //! | 16 | raise → immediate admin resolve rejected | EvidenceWindowOpen; no funds moved | +//! | 17 | raise → admin resolve closes dispute-resolution record | record `Resolved`, matching outcome | //! //! Note that tests 6–8 advance the ledger past `MIN_EVIDENCE_WINDOW` before //! resolving. That is the real flow, not test scaffolding: a dispute raised @@ -612,3 +613,75 @@ fn admin_cannot_resolve_dispute_immediately_after_cross_contract_raise() { ApplicationStatus::Paid ); } + +// ── 17. Admin resolve after raise closes the dispute-resolution record ──────── + +/// The data-integrity fix: `resolve_dispute` settles the payout *and* closes +/// the corresponding dispute-resolution record, so `get_dispute` never +/// reports a permanently-open dispute over money that has already moved. +#[test] +fn admin_resolve_after_cross_contract_raise_closes_dispute_record() { + let f = Fixture::setup(); + let campaign_id = f.create_funded_campaign(); + let creator = f.add_creator_with_proof(campaign_id); + + let dispute_id = f.disputes().raise_dispute( + &creator, + &campaign_id, + &creator, + &String::from_str(&f.env, "ipfs://evidence"), + ); + assert_eq!( + f.disputes().get_dispute(&dispute_id).status, + DisputeStatus::Raised + ); + + f.wait_out_evidence_window(); + f.escrow().resolve_dispute( + &f.admin, + &campaign_id, + &creator, + &ads_bazaar_campaign_escrow::DisputeResolution::PayCreator, + ); + + let d = f.disputes().get_dispute(&dispute_id); + assert_eq!(d.status, DisputeStatus::Resolved); + // The admin's PayCreator maps to CreatorFavored in the dispute record. + assert_eq!(d.outcome, DisputeOutcome::CreatorFavored); + assert_eq!(d.resolved_at, Some(BASE_TIME + MIN_EVIDENCE_WINDOW)); + + // The application agrees with the record: paid and no longer frozen. + let app = f.escrow().get_application(&campaign_id, &creator); + assert_eq!(app.status, ApplicationStatus::Paid); + assert!(!app.frozen); +} + +/// Each admin resolution maps to the matching `DisputeOutcome` on the +/// record: RefundBusiness → BusinessFavored, Split → Split. +#[test] +fn admin_resolve_outcome_mapping_reaches_dispute_record() { + let f = Fixture::setup(); + let campaign_id = f.create_funded_campaign(); + + // Business raises against the creator, admin settles in the business's + // favor; assert the record reflects that rather than the default pending. + let creator = f.add_creator_with_proof(campaign_id); + let dispute_id = f.disputes().raise_dispute( + &f.business, + &campaign_id, + &creator, + &String::from_str(&f.env, "ipfs://no-work"), + ); + f.wait_out_evidence_window(); + + f.escrow().resolve_dispute( + &f.admin, + &campaign_id, + &creator, + &ads_bazaar_campaign_escrow::DisputeResolution::RefundBusiness, + ); + + let d = f.disputes().get_dispute(&dispute_id); + assert_eq!(d.status, DisputeStatus::Resolved); + assert_eq!(d.outcome, DisputeOutcome::BusinessFavored); +} diff --git a/contracts/dispute-resolution/src/events.rs b/contracts/dispute-resolution/src/events.rs index d6ccf9d..7c62995 100644 --- a/contracts/dispute-resolution/src/events.rs +++ b/contracts/dispute-resolution/src/events.rs @@ -1,7 +1,9 @@ //! Event definitions for the dispute-resolution contract. See the //! campaign-escrow crate's `events.rs` for more detail on the -//! `#[contractevent]` pattern used here. None of these are published yet — -//! wire up `.publish(&env)` calls as each `todo!()` handler is implemented. +//! `#[contractevent]` pattern used here. `DisputeResolved` is published by +//! `close_dispute` (the escrow-side admin bypass closing out a raised +//! dispute); the arbiter-resolved `resolve_dispute` will publish it too once +//! that `todo!()` is implemented. #![allow(dead_code)] use ads_bazaar_shared::DisputeId; diff --git a/contracts/dispute-resolution/src/lib.rs b/contracts/dispute-resolution/src/lib.rs index 8da32ee..d29c45e 100644 --- a/contracts/dispute-resolution/src/lib.rs +++ b/contracts/dispute-resolution/src/lib.rs @@ -178,7 +178,61 @@ impl DisputeResolutionContract { todo!("design + implement dispute resolution — see doc comment above") } + /// Close out the open dispute over a `(campaign_id, creator)` payout that + /// `campaign-escrow::resolve_dispute` (the admin bypass) just settled. + /// Only the `campaign-escrow` contract set at `initialize` may call this — + /// it is the party that moved the funds, and the only one that knows the + /// admin's chosen resolution, which it records here as the matching + /// `DisputeOutcome` (see `escrow.rs`). + /// + /// Idempotent: if there is no open dispute record for that payout — the + /// freeze came from the admin's direct `freeze_for_dispute` path rather + /// than `raise_dispute`, or the dispute was already closed — this is a + /// no-op rather than an error. Without that, `resolve_dispute` settling + /// an admin-frozen (never-`raise_dispute`d) payout would trap on a + /// missing record. + pub fn close_dispute( + env: Env, + caller: Address, + campaign_id: CampaignId, + creator: Address, + outcome: DisputeOutcome, + ) -> Result<(), Error> { + caller.require_auth(); + if caller != storage::get_escrow_contract(&env)? { + return Err(Error::Unauthorized); + } + if outcome == DisputeOutcome::Pending { + return Err(Error::InvalidStatus); + } + + let Some(dispute_id) = storage::get_open_dispute(&env, campaign_id, &creator) else { + return Ok(()); + }; + + let mut dispute = storage::get_dispute(&env, dispute_id)?; + dispute.status = DisputeStatus::Resolved; + dispute.outcome = outcome; + dispute.resolved_at = Some(env.ledger().timestamp()); + storage::set_dispute(&env, dispute_id, &dispute); + storage::clear_open_dispute(&env, campaign_id, &creator); + + events::DisputeResolved { dispute_id }.publish(&env); + Ok(()) + } + /// Read-only lookup of a dispute's current state. + /// + /// Note: a dispute raised via `raise_dispute` may also be settled by the + /// admin directly through `campaign-escrow::resolve_dispute`, which + /// closes the record out via `close_dispute`. A `Raised`/`Pending` status + /// here therefore only means "still open in *this* contract" — it is not + /// authoritative over whether the escrowed payout is still held. To tell + /// the two apart, cross-reference + /// `campaign-escrow::get_application(campaign_id, creator)`: a dispute + /// still reporting `Raised` while the application is `frozen == false` + /// (or `status == Paid`) means the admin bypass settled it without a + /// dispute-resolution record to close. pub fn get_dispute(env: Env, dispute_id: DisputeId) -> Result { storage::get_dispute(&env, dispute_id) } diff --git a/contracts/dispute-resolution/src/storage.rs b/contracts/dispute-resolution/src/storage.rs index 55e32cc..b53ef1b 100644 --- a/contracts/dispute-resolution/src/storage.rs +++ b/contracts/dispute-resolution/src/storage.rs @@ -111,7 +111,8 @@ pub fn set_open_dispute(env: &Env, campaign_id: CampaignId, creator: &Address, i } /// Clear the open-dispute marker for a payout so a fresh dispute can be -/// raised over it later. Called once `resolve_dispute` is implemented. +/// raised over it later. Called by `close_dispute`, which the +/// `campaign-escrow` admin bypass invokes after settling a payout. pub fn clear_open_dispute(env: &Env, campaign_id: CampaignId, creator: &Address) { env.storage() .persistent() diff --git a/contracts/dispute-resolution/src/test.rs b/contracts/dispute-resolution/src/test.rs index e3e9cdb..3fba6f2 100644 --- a/contracts/dispute-resolution/src/test.rs +++ b/contracts/dispute-resolution/src/test.rs @@ -476,3 +476,186 @@ mod test_assign_arbiter { assert_eq!(result, Err(Ok(Error::DisputeNotFound))); } } + +mod test_close_dispute { + use super::test_helpers::*; + use crate::Error; + use ads_bazaar_shared::{DisputeOutcome, DisputeStatus}; + use soroban_sdk::testutils::{Address as _, Ledger as _}; + use soroban_sdk::{Address, String}; + + fn raise_over(env: &soroban_sdk::Env, f: &super::test_helpers::Fixture<'_>) -> u64 { + f.disputes.raise_dispute( + &f.creator, + &f.campaign_id, + &f.creator, + &String::from_str(env, "ipfs://evidence"), + ) + } + + /// The escrow contract closes a dispute raised through `raise_dispute`: + /// the record reflects `Resolved` with the escrow-supplied outcome. + #[test] + fn escrow_close_dispute_marks_record_resolved() { + let (env, escrow_id, dispute_id) = setup_env(); + let f = bootstrap(&env, &escrow_id, &dispute_id); + let id = raise_over(&env, &f); + + f.disputes.close_dispute( + &escrow_id, + &f.campaign_id, + &f.creator, + &DisputeOutcome::CreatorFavored, + ); + + let dispute = f.disputes.get_dispute(&id); + assert_eq!(dispute.status, DisputeStatus::Resolved); + assert_eq!(dispute.outcome, DisputeOutcome::CreatorFavored); + assert_eq!(dispute.resolved_at, Some(BASE_TIME)); + } + + /// The open marker is cleared, so a fresh dispute over the same payout + /// can be raised again once escrow has unfrozen it (in the real flow + /// `resolve_dispute` closes the record *and* unfreezes in the same call). + #[test] + fn close_dispute_clears_open_marker() { + let (env, escrow_id, dispute_id) = setup_env(); + let f = bootstrap(&env, &escrow_id, &dispute_id); + raise_over(&env, &f); + + f.disputes.close_dispute( + &escrow_id, + &f.campaign_id, + &f.creator, + &DisputeOutcome::BusinessFavored, + ); + + let marker = env.as_contract(&f.disputes.address, || { + crate::storage::get_open_dispute(&env, f.campaign_id, &f.creator) + }); + assert_eq!(marker, None); + } + + /// Only the configured escrow contract may close a dispute — a stranger + /// must not be able to flip a record to `Resolved` without funds moving. + #[test] + fn non_escrow_caller_is_rejected() { + let (env, escrow_id, dispute_id) = setup_env(); + let f = bootstrap(&env, &escrow_id, &dispute_id); + let id = raise_over(&env, &f); + let stranger = Address::generate(&env); + + let result = f.disputes.try_close_dispute( + &stranger, + &f.campaign_id, + &f.creator, + &DisputeOutcome::CreatorFavored, + ); + + assert_eq!(result, Err(Ok(Error::Unauthorized))); + // The rejected close must not have touched the record. + let dispute = f.disputes.get_dispute(&id); + assert_eq!(dispute.status, DisputeStatus::Raised); + assert_eq!(dispute.outcome, DisputeOutcome::Pending); + assert_eq!(dispute.resolved_at, None); + } + + /// A payout frozen directly by the admin (never `raise_dispute`d) has no + /// record to close — `close_dispute` is a no-op, not an error, so the + /// escrow's `resolve_dispute` doesn't trap on the admin-direct path. + #[test] + fn close_dispute_is_noop_without_open_record() { + let (env, escrow_id, dispute_id) = setup_env(); + let f = bootstrap(&env, &escrow_id, &dispute_id); + + let result = f.disputes.try_close_dispute( + &escrow_id, + &f.campaign_id, + &f.creator, + &DisputeOutcome::CreatorFavored, + ); + + assert!(result.is_ok()); + // Nothing to look up afterward — no record was created by closing. + assert_eq!( + f.disputes.try_get_dispute(&0), + Err(Ok(Error::DisputeNotFound)) + ); + } + + /// A second close is harmless: the marker is already gone, so it no-ops. + #[test] + fn close_dispute_twice_is_idempotent() { + let (env, escrow_id, dispute_id) = setup_env(); + let f = bootstrap(&env, &escrow_id, &dispute_id); + let id = raise_over(&env, &f); + + f.disputes.close_dispute( + &escrow_id, + &f.campaign_id, + &f.creator, + &DisputeOutcome::Split(5_000), + ); + let result = f.disputes.try_close_dispute( + &escrow_id, + &f.campaign_id, + &f.creator, + &DisputeOutcome::Split(5_000), + ); + + assert!(result.is_ok()); + let dispute = f.disputes.get_dispute(&id); + assert_eq!(dispute.status, DisputeStatus::Resolved); + assert_eq!(dispute.outcome, DisputeOutcome::Split(5_000)); + // `resolved_at` keeps the first close's timestamp. + assert_eq!(dispute.resolved_at, Some(BASE_TIME)); + } + + /// `Pending` is not a settlement outcome — a resolved record must always + /// say which side won, or it would recreate the very ambiguity this + /// close-out exists to remove. + #[test] + fn pending_outcome_is_rejected() { + let (env, escrow_id, dispute_id) = setup_env(); + let f = bootstrap(&env, &escrow_id, &dispute_id); + let id = raise_over(&env, &f); + + let result = f.disputes.try_close_dispute( + &escrow_id, + &f.campaign_id, + &f.creator, + &DisputeOutcome::Pending, + ); + + assert_eq!(result, Err(Ok(Error::InvalidStatus))); + assert_eq!(f.disputes.get_dispute(&id).status, DisputeStatus::Raised); + } + + /// The acceptance flow end-to-end: a dispute raised through the real + /// `raise_dispute` path is settled by `campaign-escrow::resolve_dispute`, + /// and the dispute-resolution record reflects `Resolved`. + #[test] + fn escrow_admin_resolve_dispute_closes_open_record() { + let (env, escrow_id, dispute_id) = setup_env(); + let f = bootstrap(&env, &escrow_id, &dispute_id); + let id = raise_over(&env, &f); + + env.ledger().with_mut(|l| { + l.timestamp += ads_bazaar_campaign_escrow::MIN_EVIDENCE_WINDOW; + }); + f.escrow.resolve_dispute( + &f.admin, + &f.campaign_id, + &f.creator, + &ads_bazaar_campaign_escrow::DisputeResolution::PayCreator, + ); + + let dispute = f.disputes.get_dispute(&id); + assert_eq!(dispute.status, DisputeStatus::Resolved); + assert_eq!(dispute.outcome, DisputeOutcome::CreatorFavored); + assert_eq!( + dispute.resolved_at, + Some(BASE_TIME + ads_bazaar_campaign_escrow::MIN_EVIDENCE_WINDOW) + ); + } +}