Merge pull request #556 from ThatCodeBabe/fix/multi-sig-dispute-lifecycle-550

fix(escrow): allow multi-sig escrows to enter the Disputed state

Author: MaryammAli

app/contract/contracts/Folder/src/escrow.rs

@@ -1098,8 +1098,30 @@ pub fn dispute(env: &Env, commitment: BytesN<32>) -> Result<(),  RustAcademyErro
     let entry: EscrowEntry =
         get_escrow(env, &commitment_bytes).ok_or( RustAcademyError::CommitmentNotFound)?;
 
-    // Guard: must have an arbiter assigned
-    let arbiter = entry.arbiter.as_ref().ok_or( RustAcademyError::NoArbiter)?;
+    // Guard: must have an arbiter assigned.
+    //
+    // Two escrow shapes carry arbiters: single-arbiter escrows populate
+    // `arbiter`, while multi-sig escrows created by `deposit_with_arbiters`
+    // leave `arbiter` as `None` and populate `arbiters` + `arbiter_threshold`.
+    // Both must be able to reach `Disputed`, otherwise `vote_for_dispute` and
+    // `resolve_dispute_multi_sig` are unreachable for multi-sig escrows.
+    //
+    // The event carries a single representative arbiter. For multi-sig escrows
+    // the first listed arbiter is used, matching the deterministic fallback
+    // already applied in `dispute::resolve_expiry_recipient`.
+    let event_arbiter = match entry.arbiter.as_ref() {
+        Some(arbiter) => arbiter.clone(),
+        None => {
+            if entry.arbiter_threshold == 0 {
+                return Err( RustAcademyError::NoArbiter);
+            }
+            entry
+                .arbiters
+                .first()
+                .ok_or( RustAcademyError::NoArbiter)?
+                .clone()
+        }
+    };
 
     // Guard: escrow must be in Pending state
     if entry.status != EscrowStatus::Pending {
@@ -1113,7 +1135,7 @@ pub fn dispute(env: &Env, commitment: BytesN<32>) -> Result<(),  RustAcademyErro
     // Issue #49: snapshot timeout and default expiry action at dispute creation.
     dispute::record_dispute_expiry(env, commitment.clone());
 
-    events::publish_escrow_disputed(env, commitment.clone(), arbiter.clone());
+    events::publish_escrow_disputed(env, commitment.clone(), event_arbiter);
 
     Ok(())
 }

app/contract/contracts/Folder/src/lib.rs

@@ -33,8 +33,12 @@ pub mod nonce;
 mod nonce_test;
 mod oracle;
 mod privacy;
+#[cfg(test)]
+mod reentrancy_test;
 mod legacy_privacy;
 #[cfg(test)]
+mod lifecycle_test;
+#[cfg(test)]
 mod role_test;
 mod stealth;
 #[cfg(test)]

app/contract/contracts/Folder/src/lifecycle_test.rs

@@ -0,0 +1,95 @@
+//! Lifecycle transition tests for multi-arbiter (multi-sig) escrows.
+//!
+//! `deposit_with_arbiters` stores its arbiters in `EscrowEntry.arbiters` with
+//! `arbiter: None`, but `dispute()` reads only `EscrowEntry.arbiter`. These
+//! tests pin down that a multi-sig escrow can actually reach `Disputed` and be
+//! resolved, which is the precondition for `vote_for_dispute` and
+//! `resolve_dispute_multi_sig` being reachable at all.
+
+
+use soroban_sdk::{testutils::Address as _, token, Address, Bytes, Env, Vec};
+
+use crate::{types::EscrowStatus, RustAcademyContract, RustAcademyContractClient};
+
+fn setup<'a>() -> (Env, RustAcademyContractClient<'a>, Address) {
+    let env = Env::default();
+    env.mock_all_auths();
+    let id = env.register(RustAcademyContract, ());
+    let client = RustAcademyContractClient::new(&env, &id);
+    let admin = Address::generate(&env);
+    client.initialize(&admin);
+    let token = env
+        .register_stellar_asset_contract_v2(Address::generate(&env))
+        .address();
+    (env, client, token)
+}
+
+/// A multi-sig escrow must be able to enter `Disputed`.
+///
+/// Without this, `vote_for_dispute` and `resolve_dispute_multi_sig` are
+/// unreachable: both require `status == Disputed`, and `dispute()` is the only
+/// transition into that state.
+#[test]
+fn multi_sig_escrow_can_be_disputed() {
+    let (env, client, token) = setup();
+    let owner = Address::generate(&env);
+    let amount: i128 = 3_000;
+    let salt = Bytes::from_slice(&env, b"multisig_dispute_lifecycle");
+
+    token::StellarAssetClient::new(&env, &token).mint(&owner, &amount);
+
+    let mut arbiters = Vec::new(&env);
+    arbiters.push_back(Address::generate(&env));
+    arbiters.push_back(Address::generate(&env));
+    arbiters.push_back(Address::generate(&env));
+
+    let commitment =
+        client.deposit_with_arbiters(&token, &amount, &owner, &salt, &0u64, &arbiters, &2u32);
+
+    assert_eq!(
+        client.get_commitment_state(&commitment),
+        Some(EscrowStatus::Pending),
+        "escrow should start Pending"
+    );
+
+    let result = client.try_dispute(&commitment);
+    assert!(
+        result.is_ok(),
+        "a multi-sig escrow must be disputable; otherwise its arbiters can never act"
+    );
+
+    assert_eq!(
+        client.get_commitment_state(&commitment),
+        Some(EscrowStatus::Disputed),
+        "escrow should be Disputed after dispute()"
+    );
+}
+
+/// Once a multi-sig escrow is disputed, its assigned arbiters must be able to
+/// vote. This is the step that is dead code today.
+#[test]
+fn multi_sig_arbiters_can_vote_once_disputed() {
+    let (env, client, token) = setup();
+    let owner = Address::generate(&env);
+    let amount: i128 = 3_000;
+    let salt = Bytes::from_slice(&env, b"multisig_vote_lifecycle");
+
+    token::StellarAssetClient::new(&env, &token).mint(&owner, &amount);
+
+    let a1 = Address::generate(&env);
+    let a2 = Address::generate(&env);
+    let mut arbiters = Vec::new(&env);
+    arbiters.push_back(a1.clone());
+    arbiters.push_back(a2.clone());
+
+    let commitment =
+        client.deposit_with_arbiters(&token, &amount, &owner, &salt, &0u64, &arbiters, &2u32);
+
+    client.dispute(&commitment);
+
+    let vote = client.try_vote_for_dispute(&a1, &commitment, &true);
+    assert!(
+        vote.is_ok(),
+        "an assigned multi-sig arbiter must be able to vote on a disputed escrow"
+    );
+}