From 5b074786e563ea1c892640f60e85c0fbc63475fd Mon Sep 17 00:00:00 2001 From: Tinu Date: Thu, 24 Sep 2026 18:09:06 +0100 Subject: [PATCH] feat(escrow): enforce dispute state transitions and add dispute edge-case tests (#220) --- contracts/contracts/escrow/src/lib.rs | 16 +++- contracts/contracts/escrow/src/test.rs | 122 +++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 3 deletions(-) diff --git a/contracts/contracts/escrow/src/lib.rs b/contracts/contracts/escrow/src/lib.rs index 0980aad..9ea4207 100644 --- a/contracts/contracts/escrow/src/lib.rs +++ b/contracts/contracts/escrow/src/lib.rs @@ -392,6 +392,12 @@ impl EscrowContract { return Err(Error::InvalidMilestoneStatus); } + // While a dispute is open the escrowed funds are frozen: `release` must be + // blocked until the arbitrator resolves the dispute. + if milestone.status == MilestoneStatus::Disputed { + return Err(Error::InvalidMilestoneStatus); + } + if !milestone.client_approved { return Err(Error::InsufficientApprovals); } @@ -469,15 +475,19 @@ impl EscrowContract { let client: Address = env.storage().instance().get(&DataKey::Client).ok_or(Error::NotInitialized)?; let freelancer: Address = env.storage().instance().get(&DataKey::Freelancer).ok_or(Error::NotInitialized)?; + let arbiter: Address = env.storage().instance().get(&DataKey::Arbiter).ok_or(Error::NotInitialized)?; - if caller != client && caller != freelancer { + // Role-based access: only the escrow participants (client/freelancer) or the + // authorized arbitrator may raise a dispute. + if caller != client && caller != freelancer && caller != arbiter { return Err(Error::Unauthorized); } let mut milestone: Milestone = env.storage().instance().get(&DataKey::Milestone(milestone_id)).ok_or(Error::MilestoneNotFound)?; - if milestone.status != MilestoneStatus::Funded - && milestone.status != MilestoneStatus::Submitted + // A dispute can only be raised after the milestone has been funded and + // submitted (Submitted/Approved) - it must not bypass the normal lifecycle. + if milestone.status != MilestoneStatus::Submitted && milestone.status != MilestoneStatus::Approved { return Err(Error::InvalidMilestoneStatus); diff --git a/contracts/contracts/escrow/src/test.rs b/contracts/contracts/escrow/src/test.rs index e525e3c..c249fee 100644 --- a/contracts/contracts/escrow/src/test.rs +++ b/contracts/contracts/escrow/src/test.rs @@ -612,3 +612,125 @@ fn test_successful_security_events_are_emitted() { 1 ); } + +// --- Dispute escrow mechanism edge cases (issue #220) --- + +#[test] +#[should_panic(expected = "HostError: Error(Contract, #6)")] +fn test_dispute_without_funding_fails() { + let setup = setup_test(); + // Milestone is Pending (never funded): a dispute must not bypass escrow funding. + initialize_single_milestone(&setup, 150); + + setup.escrow_client.dispute(&1, &setup.client); +} + +#[test] +#[should_panic(expected = "HostError: Error(Contract, #6)")] +fn test_dispute_before_submission_fails() { + let setup = setup_test(); + // Funded but not submitted yet: a dispute must not bypass milestone submission. + initialize_single_milestone(&setup, 150); + setup.escrow_client.fund(); + + setup.escrow_client.dispute(&1, &setup.client); +} + +#[test] +fn test_arbiter_can_raise_dispute() { + let setup = setup_test(); + initialize_single_milestone(&setup, 150); + setup.escrow_client.fund(); + setup.escrow_client.submit_milestone(&1); + + // The authorized arbitrator is allowed to raise a dispute. + setup.escrow_client.dispute(&1, &setup.arbiter); + + assert_eq!( + setup.escrow_client.get_milestones().get(0).unwrap().status, + MilestoneStatus::Disputed + ); +} + +#[test] +#[should_panic(expected = "HostError: Error(Contract, #6)")] +fn test_release_blocked_while_disputed() { + let setup = setup_test(); + initialize_single_milestone(&setup, 150); + setup.escrow_client.fund(); + setup.escrow_client.submit_milestone(&1); + setup.escrow_client.approve(&1); + setup.escrow_client.dispute(&1, &setup.client); + + // Funds are locked while the dispute is open, even for the client. + setup.escrow_client.release(&1, &setup.client); +} + +#[test] +#[should_panic(expected = "HostError: Error(Contract, #6)")] +fn test_refund_blocked_while_disputed() { + let setup = setup_test(); + initialize_single_milestone(&setup, 150); + setup.escrow_client.fund(); + setup.escrow_client.submit_milestone(&1); + setup.escrow_client.dispute(&1, &setup.freelancer); + + // The freelancer cannot refund out from under an open dispute. + setup.escrow_client.refund(&1, &setup.freelancer); +} + +#[test] +#[should_panic(expected = "HostError: Error(Contract, #6)")] +fn test_multiple_disputes_rejected() { + let setup = setup_test(); + initialize_single_milestone(&setup, 150); + setup.escrow_client.fund(); + setup.escrow_client.submit_milestone(&1); + + setup.escrow_client.dispute(&1, &setup.client); + setup.escrow_client.resolve_dispute(&1, &true); + + // Once resolved, the milestone is terminal: a second dispute is rejected. + setup.escrow_client.dispute(&1, &setup.client); +} + +#[test] +#[should_panic(expected = "HostError: Error(Contract, #6)")] +fn test_resolve_without_dispute_fails() { + let setup = setup_test(); + initialize_single_milestone(&setup, 150); + setup.escrow_client.fund(); + setup.escrow_client.submit_milestone(&1); + + setup.escrow_client.resolve_dispute(&1, &true); +} + +#[test] +fn test_dispute_events_are_emitted() { + let setup = setup_test(); + let env = setup.env.clone(); + + initialize_single_milestone(&setup, 150); + setup.escrow_client.fund(); + setup.escrow_client.submit_milestone(&1); + + setup.escrow_client.dispute(&1, &setup.client); + assert_eq!( + env.events() + .all() + .filter_by_contract(&setup.escrow_client.address) + .events() + .len(), + 1 + ); + + setup.escrow_client.resolve_dispute(&1, &true); + assert_eq!( + env.events() + .all() + .filter_by_contract(&setup.escrow_client.address) + .events() + .len(), + 1 + ); +}