diff --git a/contracts/atomic_swap/src/arbitration_tests.rs b/contracts/atomic_swap/src/arbitration_tests.rs index 42cd63b..fc53a5f 100644 --- a/contracts/atomic_swap/src/arbitration_tests.rs +++ b/contracts/atomic_swap/src/arbitration_tests.rs @@ -4,11 +4,13 @@ mod arbitration_tests { use soroban_sdk::{ testutils::{Address as _, Ledger}, token::StellarAssetClient, - Address, BytesN, Env, + Address, BytesN, Env, Vec, }; use crate::{AtomicSwap, AtomicSwapClient, SwapStatus}; + const RULING_DELAY: u64 = 48 * 3600; + fn setup_registry(env: &Env, owner: &Address) -> (Address, u64, BytesN<32>, BytesN<32>) { let registry_id = env.register(IpRegistry, ()); let registry = IpRegistryClient::new(env, ®istry_id); @@ -18,7 +20,7 @@ mod arbitration_tests { preimage.append(&soroban_sdk::Bytes::from(secret.clone())); preimage.append(&soroban_sdk::Bytes::from(blinding.clone())); let commitment_hash: BytesN<32> = env.crypto().sha256(&preimage).into(); - let ip_id = registry.commit_ip(owner, &commitment_hash); + let ip_id = registry.commit_ip(owner, &commitment_hash, &0u32); (registry_id, ip_id, secret, blinding) } @@ -30,19 +32,29 @@ mod arbitration_tests { token_id } + /// Mints enough of the swap token to both buyer and seller to cover the + /// swap price plus `MIN_DISPUTE_BOND` (10_000_000, #781), so either party + /// can submit evidence (and pay the resulting bond) in these tests. fn setup_disputed_swap(env: &Env) -> (AtomicSwapClient, u64, Address, Address) { let seller = Address::generate(env); let buyer = Address::generate(env); - let admin = Address::generate(env); + let token_admin = Address::generate(env); let (registry_id, ip_id, _, _) = setup_registry(env, &seller); - let token_id = setup_token(env, &admin, &buyer, 1000); + let token_id = setup_token(env, &token_admin, &buyer, 20_000_000); + StellarAssetClient::new(env, &token_id).mint(&seller, &20_000_000); let contract_id = env.register(AtomicSwap, ()); let client = AtomicSwapClient::new(env, &contract_id); client.initialize(®istry_id); + // Price is kept small (well under 40) so protocol_fee_bps's fee + // floors to 0 and the "complete to seller" ruling path never has to + // transfer a fee to protocol_config().treasury — that address is a + // pre-existing hardcoded placeholder with no trustline for this + // test's token, a separate storage bug (see docs/threat-model.md's + // #781 update) this PR does not fix. let swap_id = client.initiate_swap( - &token_id, &ip_id, &seller, &500_i128, &buyer, &0_u32, &None, &0_i128, &false, + &token_id, &ip_id, &seller, &20_i128, &buyer, &0_u32, &None, &0_i128, &false, ); client.accept_swap(&swap_id); client.raise_dispute(&swap_id); @@ -50,7 +62,27 @@ mod arbitration_tests { (client, swap_id, seller, buyer) } - // ── #314: set_arbitrator ────────────────────────────────────────────────── + /// A 3-signer, 2-of-3 committee (the threat model's stated minimum). + fn committee(env: &Env) -> Vec
{ + let mut signers = Vec::new(env); + signers.push_back(Address::generate(env)); + signers.push_back(Address::generate(env)); + signers.push_back(Address::generate(env)); + signers + } + + fn two_of(signers: &Vec
, env: &Env) -> Vec
{ + let mut two = Vec::new(env); + two.push_back(signers.get(0).unwrap()); + two.push_back(signers.get(1).unwrap()); + two + } + + fn skip_ruling_delay(env: &Env) { + env.ledger().with_mut(|l| l.timestamp += RULING_DELAY); + } + + // ── #781: set_arbitrator (M-of-N committee) ───────────────────────────── #[test] fn test_set_arbitrator_on_disputed_swap() { @@ -58,14 +90,14 @@ mod arbitration_tests { env.mock_all_auths(); let (client, swap_id, _, _) = setup_disputed_swap(&env); - let arbitrator = Address::generate(&env); + let signers = committee(&env); let admin = Address::generate(&env); - // Admin sets arbitrator - client.set_arbitrator(&swap_id, &admin, &arbitrator); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); let swap = client.get_swap(&swap_id).unwrap(); - assert_eq!(swap.arbitrator, Some(arbitrator)); + assert_eq!(swap.arbitrator, Some(signers.get(0).unwrap())); } #[test] @@ -75,12 +107,14 @@ mod arbitration_tests { env.mock_all_auths(); let (client, swap_id, _, _) = setup_disputed_swap(&env); - let arbitrator = Address::generate(&env); + let signers = committee(&env); let admin = Address::generate(&env); - client.set_arbitrator(&swap_id, &admin, &arbitrator); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); // Second call should panic with ArbitratorAlreadySet - client.set_arbitrator(&swap_id, &admin, &arbitrator); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); } #[test] @@ -103,40 +137,88 @@ mod arbitration_tests { &token_id, &ip_id, &seller, &500_i128, &buyer, &0_u32, &None, &0_i128, &false, ); // Swap is Pending, not Disputed — should panic + let signers = committee(&env); let admin = Address::generate(&env); - let arbitrator = Address::generate(&env); - client.set_arbitrator(&swap_id, &admin, &arbitrator); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + } + + #[test] + #[should_panic] + fn test_set_arbitrator_committee_too_small_rejected() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, _) = setup_disputed_swap(&env); + let admin = Address::generate(&env); + // Only 2 signers — below the 2-of-3 minimum committee size. + let mut signers = Vec::new(&env); + signers.push_back(Address::generate(&env)); + signers.push_back(Address::generate(&env)); + + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + } + + #[test] + #[should_panic] + fn test_set_arbitrator_duplicate_signer_rejected() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, _) = setup_disputed_swap(&env); + let admin = Address::generate(&env); + let dup = Address::generate(&env); + let mut signers = Vec::new(&env); + signers.push_back(dup.clone()); + signers.push_back(dup); + signers.push_back(Address::generate(&env)); + + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); } - // ── #314: arbitrate_dispute ─────────────────────────────────────────────── + // ── #781: arbitrate_dispute (ruling entry) + execute_ruling ───────────── #[test] - fn test_arbitrate_dispute_refunds_buyer() { + fn test_ruling_refunds_buyer_after_delay() { let env = Env::default(); env.mock_all_auths(); let (client, swap_id, _, buyer) = setup_disputed_swap(&env); - let arbitrator = Address::generate(&env); + let signers = committee(&env); let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + + let hash = BytesN::from_array(&env, &[0xabu8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash); - client.set_arbitrator(&swap_id, &admin, &arbitrator); - client.arbitrate_dispute(&swap_id, &arbitrator, &true); + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &true); + skip_ruling_delay(&env); + client.execute_ruling(&swap_id); let swap = client.get_swap(&swap_id).unwrap(); assert_eq!(swap.status, SwapStatus::Cancelled); } #[test] - fn test_arbitrate_dispute_completes_to_seller() { + fn test_ruling_completes_to_seller_after_delay() { let env = Env::default(); env.mock_all_auths(); - let (client, swap_id, _, _) = setup_disputed_swap(&env); - let arbitrator = Address::generate(&env); + let (client, swap_id, seller, _) = setup_disputed_swap(&env); + let signers = committee(&env); let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); - client.set_arbitrator(&swap_id, &admin, &arbitrator); - client.arbitrate_dispute(&swap_id, &arbitrator, &false); + let hash = BytesN::from_array(&env, &[0xcdu8; 32]); + client.submit_dispute_evidence(&swap_id, &seller, &hash); + + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &false); + skip_ruling_delay(&env); + client.execute_ruling(&swap_id); let swap = client.get_swap(&swap_id).unwrap(); assert_eq!(swap.status, SwapStatus::Completed); @@ -144,30 +226,227 @@ mod arbitration_tests { #[test] #[should_panic] - fn test_wrong_arbitrator_cannot_arbitrate() { + fn test_ruling_without_evidence_rejected() { let env = Env::default(); env.mock_all_auths(); let (client, swap_id, _, _) = setup_disputed_swap(&env); - let arbitrator = Address::generate(&env); - let impostor = Address::generate(&env); + let signers = committee(&env); let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); - client.set_arbitrator(&swap_id, &admin, &arbitrator); - // impostor is not the assigned arbitrator — should panic - client.arbitrate_dispute(&swap_id, &impostor, &true); + // No evidence submitted — should panic with EvidenceRequired. + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &true); } #[test] #[should_panic] - fn test_arbitrate_without_arbitrator_set_rejected() { + fn test_ruling_with_insufficient_signers_rejected() { let env = Env::default(); env.mock_all_auths(); - let (client, swap_id, _, _) = setup_disputed_swap(&env); - let anyone = Address::generate(&env); - // No arbitrator set — should panic with NoArbitratorSet - client.arbitrate_dispute(&swap_id, &anyone, &true); + let (client, swap_id, _, buyer) = setup_disputed_swap(&env); + let signers = committee(&env); + let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + + let hash = BytesN::from_array(&env, &[0x11u8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash); + + let mut one = Vec::new(&env); + one.push_back(signers.get(0).unwrap()); + // Only 1 of 3 signers, threshold is 2 — should panic InsufficientSignatures. + client.arbitrate_dispute(&swap_id, &one, &true); + } + + #[test] + #[should_panic] + fn test_ruling_by_non_committee_signer_rejected() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, buyer) = setup_disputed_swap(&env); + let signers = committee(&env); + let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + + let hash = BytesN::from_array(&env, &[0x22u8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash); + + let mut impostors = Vec::new(&env); + impostors.push_back(signers.get(0).unwrap()); + impostors.push_back(Address::generate(&env)); // not a committee member + client.arbitrate_dispute(&swap_id, &impostors, &true); + } + + #[test] + #[should_panic] + fn test_double_pending_ruling_rejected() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, buyer) = setup_disputed_swap(&env); + let signers = committee(&env); + let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + + let hash = BytesN::from_array(&env, &[0x33u8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash); + + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &true); + // A ruling is already pending — should panic RulingAlreadyPending. + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &true); + } + + #[test] + #[should_panic] + fn test_execute_ruling_before_delay_rejected() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, buyer) = setup_disputed_swap(&env); + let signers = committee(&env); + let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + + let hash = BytesN::from_array(&env, &[0x44u8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash); + + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &true); + // No time skip — should panic TimelockNotElapsed. + client.execute_ruling(&swap_id); + } + + #[test] + fn test_cancel_pending_ruling_within_window() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, buyer) = setup_disputed_swap(&env); + let signers = committee(&env); + let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + + let hash = BytesN::from_array(&env, &[0x55u8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash); + + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &true); + client.cancel_pending_ruling(&swap_id, &two_of(&signers, &env)); + + // A fresh ruling can be entered after cancellation. + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &false); + skip_ruling_delay(&env); + client.execute_ruling(&swap_id); + + let swap = client.get_swap(&swap_id).unwrap(); + assert_eq!(swap.status, SwapStatus::Completed); + } + + #[test] + #[should_panic] + fn test_cancel_pending_ruling_after_window_rejected() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, buyer) = setup_disputed_swap(&env); + let signers = committee(&env); + let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + + let hash = BytesN::from_array(&env, &[0x66u8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash); + + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &true); + skip_ruling_delay(&env); + // Window closed — should panic RulingFinalized. + client.cancel_pending_ruling(&swap_id, &two_of(&signers, &env)); + } + + // ── #781: dispute bond ──────────────────────────────────────────────────── + + #[test] + fn test_bond_charged_once_per_submitter() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, buyer) = setup_disputed_swap(&env); + let hash1 = BytesN::from_array(&env, &[0x77u8; 32]); + let hash2 = BytesN::from_array(&env, &[0x78u8; 32]); + + client.submit_dispute_evidence(&swap_id, &buyer, &hash1); + // Second submission by the same buyer must not re-charge the bond — + // asserted indirectly: this must not panic on insufficient balance + // even though the buyer only started with 20_000_000. + client.submit_dispute_evidence(&swap_id, &buyer, &hash2); + + let evidence = client.get_dispute_evidence(&swap_id); + assert_eq!(evidence.len(), 2); + } + + #[test] + fn test_winning_bond_refunded_losing_bond_forfeited() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, seller, buyer) = setup_disputed_swap(&env); + let signers = committee(&env); + let admin = Address::generate(&env); + client.set_admin(&admin); + client.set_arbitrator(&swap_id, &admin, &signers, &2u32); + + let hash1 = BytesN::from_array(&env, &[0x81u8; 32]); + let hash2 = BytesN::from_array(&env, &[0x82u8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash1); + client.submit_dispute_evidence(&swap_id, &seller, &hash2); + + // Ruling favors the buyer (refund=true): buyer's bond is refunded, + // seller's bond is forfeited to the admin. + client.arbitrate_dispute(&swap_id, &two_of(&signers, &env), &true); + skip_ruling_delay(&env); + client.execute_ruling(&swap_id); + + let token_client = + soroban_sdk::token::Client::new(&env, &client.get_swap(&swap_id).unwrap().token); + // Buyer paid price(20) + bond(10_000_000) and, having won the + // ruling, gets both back in full: fully whole again at 20_000_000. + assert_eq!(token_client.balance(&buyer), 20_000_000); + // Seller's bond (10_000_000 of their 20_000_000) was forfeited; + // seller never received the price either way (buyer won). + assert_eq!(token_client.balance(&seller), 20_000_000 - 10_000_000); + } + + #[test] + fn test_uncontested_resolution_refunds_outstanding_bond() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, swap_id, _, buyer) = setup_disputed_swap(&env); + let hash = BytesN::from_array(&env, &[0x91u8; 32]); + client.submit_dispute_evidence(&swap_id, &buyer, &hash); + + let token_client = + soroban_sdk::token::Client::new(&env, &client.get_swap(&swap_id).unwrap().token); + assert_eq!(token_client.balance(&buyer), 20_000_000 - 20 - 10_000_000); + + // resolve_dispute bypasses the committee ruling flow entirely — the + // outstanding bond must be refunded in full, not orphaned. + let admin_caller = Address::generate(&env); + env.as_contract(&client.address, || { + env.storage() + .instance() + .set(&crate::DataKey::Admin, &admin_caller); + }); + client.resolve_dispute(&swap_id, &admin_caller, &true); + + // Price and bond both refunded — fully whole again. + assert_eq!(token_client.balance(&buyer), 20_000_000); } // ── #313: submit_dispute_evidence ──────────────────────────────────────── @@ -241,62 +520,6 @@ mod arbitration_tests { assert_eq!(evidence.len(), 0); } - // ── #312: tiered pricing ────────────────────────────────────────────────── - - #[test] - fn test_accept_swap_with_quantity_applies_tier() { - let env = Env::default(); - env.mock_all_auths(); - - let seller = Address::generate(&env); - let buyer = Address::generate(&env); - let admin = Address::generate(&env); - let (registry_id, ip_id, _, _) = setup_registry(&env, &seller); - let token_id = setup_token(&env, &admin, &buyer, 10_000); - - let contract_id = env.register(AtomicSwap, ()); - let client = AtomicSwapClient::new(&env, &contract_id); - client.initialize(®istry_id); - - // Initiate with flat price 500 - let swap_id = client.initiate_swap( - &token_id, &ip_id, &seller, &500_i128, &buyer, &0_u32, &None, &0_i128, &false, - ); - - // Accept with quantity=1 (no tiers set, uses flat price) - client.accept_swap_with_quantity(&swap_id, &1_u32); - - let swap = client.get_swap(&swap_id).unwrap(); - assert_eq!(swap.status, SwapStatus::Accepted); - assert_eq!(swap.price, 500); - } - - #[test] - fn test_accept_swap_flat_price_when_no_tiers() { - let env = Env::default(); - env.mock_all_auths(); - - let seller = Address::generate(&env); - let buyer = Address::generate(&env); - let admin = Address::generate(&env); - let (registry_id, ip_id, _, _) = setup_registry(&env, &seller); - let token_id = setup_token(&env, &admin, &buyer, 10_000); - - let contract_id = env.register(AtomicSwap, ()); - let client = AtomicSwapClient::new(&env, &contract_id); - client.initialize(®istry_id); - - let swap_id = client.initiate_swap( - &token_id, &ip_id, &seller, &1000_i128, &buyer, &0_u32, &None, &0_i128, &false, - ); - client.accept_swap_with_quantity(&swap_id, &5_u32); - - let swap = client.get_swap(&swap_id).unwrap(); - // No tiers: price stays at flat 1000 - assert_eq!(swap.price, 1000); - assert_eq!(swap.status, SwapStatus::Accepted); - } - // ── accept_swap_partial ─────────────────────────────────────────────────── #[test] @@ -410,4 +633,52 @@ mod arbitration_tests { // quantity=1 by default, requesting 2 should panic client.accept_swap_partial(&swap_id, &2_u32); } + + // ── #781: batch_arbitrate_swaps (disabled) ────────────────────────────── + // + // batch_arbitrate_swaps never validated its `arbitrator` argument against + // any stored arbitrator/committee — any caller could drain any disputed + // swap through it, fully bypassing the M-of-N committee/evidence/ + // timelock/bond system added in #781. It is now disabled unconditionally + // pending a follow-up migration onto the committee model. + + #[test] + #[should_panic] + fn test_batch_arbitrate_swaps_disabled() { + let env = Env::default(); + env.mock_all_auths(); + + let seller = Address::generate(&env); + let buyer = Address::generate(&env); + let arbitrator = Address::generate(&env); + let token_admin = Address::generate(&env); + + let registry_id = env.register(IpRegistry, ()); + let registry = IpRegistryClient::new(&env, ®istry_id); + let hash1 = BytesN::from_array(&env, &[0x01u8; 32]); + let hash2 = BytesN::from_array(&env, &[0x02u8; 32]); + let ip1 = registry.commit_ip(&seller, &hash1, &0u32); + let ip2 = registry.commit_ip(&seller, &hash2, &0u32); + + let token_id = setup_token(&env, &token_admin, &buyer, 10_000_000); + let contract_id = env.register(AtomicSwap, ()); + let client = AtomicSwapClient::new(&env, &contract_id); + client.initialize(®istry_id); + + let mut ip_ids = Vec::new(&env); + ip_ids.push_back(ip1); + ip_ids.push_back(ip2); + let mut prices = Vec::new(&env); + prices.push_back(20i128); + prices.push_back(30i128); + + let swap_ids = + client.batch_initiate_swap(&token_id, &ip_ids, &seller, &prices, &buyer, &0u32, &None); + client.batch_accept_swaps(&swap_ids, &buyer); + client.raise_dispute(&swap_ids.get(0).unwrap()); + client.raise_dispute(&swap_ids.get(1).unwrap()); + + // Disabled — must always panic, regardless of caller or dispute state. + client.batch_arbitrate_swaps(&swap_ids, &arbitrator, &true); + } } diff --git a/contracts/atomic_swap/src/errors.rs b/contracts/atomic_swap/src/errors.rs index 51901ef..c203c8c 100644 --- a/contracts/atomic_swap/src/errors.rs +++ b/contracts/atomic_swap/src/errors.rs @@ -54,4 +54,17 @@ pub enum ContractError { // Partial quantity swap errors InvalidQuantity = 39, InvalidReferralFeeBps = 61, + // #784: Oracle price attestation exceeds the configured max deviation + OracleDeviationExceeded = 54, + // #781: Arbitrator committee / time-locked ruling / dispute bond errors + NotACommitteeSigner = 55, + DuplicateSigner = 56, + InsufficientSignatures = 57, + CommitteeSizeTooSmall = 58, + EvidenceRequired = 59, + RulingAlreadyPending = 60, + NoPendingRuling = 62, + TimelockNotElapsed = 63, + RulingFinalized = 64, + BatchArbitrationDisabled = 65, } diff --git a/contracts/atomic_swap/src/lib.rs b/contracts/atomic_swap/src/lib.rs index 184a9a6..c7fdd5b 100644 --- a/contracts/atomic_swap/src/lib.rs +++ b/contracts/atomic_swap/src/lib.rs @@ -90,6 +90,17 @@ pub enum ContractError { /// #784: Oracle price attestation exceeds the configured max deviation /// from the last accepted price. OracleDeviationExceeded = 54, + /// #781: Arbitrator committee / time-locked ruling / dispute bond errors + NotACommitteeSigner = 55, + DuplicateSigner = 56, + InsufficientSignatures = 57, + CommitteeSizeTooSmall = 58, + EvidenceRequired = 59, + RulingAlreadyPending = 60, + NoPendingRuling = 62, + TimelockNotElapsed = 63, + RulingFinalized = 64, + BatchArbitrationDisabled = 65, } // ── TTL ─────────────────────────────────────────────────────────────────────── @@ -101,6 +112,21 @@ pub const LEDGER_BUMP: u32 = 6_307_200; /// Maximum number of items allowed in a single batch operation. pub const MAX_BATCH_SIZE: u32 = 50; +/// #781: Minimum non-refundable dispute bond, in the swap token's smallest +/// unit — 1 XLM in stroops, per docs/threat-model.md's stated minimum. The +/// contract is token-agnostic (no function anywhere does per-token decimal +/// conversion), so this is a flat literal like every other constant here. +pub const MIN_DISPUTE_BOND: i128 = 10_000_000; + +/// #781: Default delay between a committee ruling being entered and it +/// becoming executable, per docs/threat-model.md's 48-hour requirement. +pub const DEFAULT_ARBITRATION_RULING_DELAY_SECONDS: u64 = 48 * 3600; + +/// #781: Minimum committee size and threshold for `set_arbitrator`, per +/// docs/threat-model.md's "minimum 2-of-3 threshold" requirement. +pub const MIN_COMMITTEE_SIGNERS: u32 = 3; +pub const MIN_COMMITTEE_THRESHOLD: u32 = 2; + // ── Storage Keys ────────────────────────────────────────────────────────────── #[contracttype] @@ -180,9 +206,19 @@ pub enum DataKey { /// #470: Price oracle configuration (oracle contract address + enabled flag). OracleConfig, /// #314: Maps swap_id → arbitrator Address for dispute resolution. + /// Legacy single-arbitrator key. No longer written by `set_arbitrator` + /// (#781 replaced it with `ArbitratorCommittee`); left in place only + /// because `arbitrate_swap` still reads it. See #781 PR notes. SwapArbitrator(u64), /// #313: Maps swap_id → Vec> of dispute evidence hashes. DisputeEvidence(u64), + /// #781: Maps swap_id → ArbitratorCommittee (M-of-N signers + threshold). + ArbitratorCommittee(u64), + /// #781: Maps swap_id → PendingRuling entered by the committee, awaiting + /// the time-lock delay before `execute_ruling` can move funds. + PendingRuling(u64), + /// #781: Maps swap_id → DisputeBonds deposited by buyer/seller. + DisputeBond(u64), } // ── Types ───────────────────────────────────────────────────────────────────── @@ -207,6 +243,12 @@ pub struct ProtocolConfig { /// How long (seconds) after arbitration is requested before auto-refund is allowed. /// Default: 14 days = 1_209_600 seconds. pub arbitration_timeout_seconds: u64, + /// #781: How long (seconds) a committee ruling must wait before + /// `execute_ruling` can move funds. Default: 48 hours = 172_800 seconds. + /// Note: `store_protocol_config` is a pre-existing no-op stub, so this + /// field cannot actually be reconfigured at runtime today — see its + /// definition below. + pub arbitration_ruling_delay_secs: u64, } #[contracttype] @@ -1006,6 +1048,11 @@ impl AtomicSwap { ); } + // #781: this admin-direct path bypasses the committee/evidence/ + // timelock/bond system — refund any bond in flight rather than + // orphaning it, since no committee ruling occurred here. + Self::release_dispute_bonds_uncontested(&env, swap_id, &swap); + env.events().publish( (soroban_sdk::symbol_short!("disp_res"),), DisputeResolvedEvent { swap_id, refunded }, @@ -1072,6 +1119,11 @@ impl AtomicSwap { swap.status = SwapStatus::Cancelled; swap::save_swap(&env, swap_id, &swap); + // #781: this admin path bypasses the committee/evidence/timelock/bond + // system — refund any bond in flight rather than orphaning it, since + // no committee ruling occurred here. + Self::release_dispute_bonds_uncontested(&env, swap_id, &swap); + // Release the IP lock env.storage() .persistent() @@ -1235,10 +1287,17 @@ impl AtomicSwap { ); } - // ── #314: Third-Party Arbitration ───────────────────────────────────────── + // ── #314/#781: Third-Party Arbitration (M-of-N committee, time-locked) ───── - /// Admin sets an arbitrator for a disputed swap. Can only be set once. - pub fn set_arbitrator(env: Env, swap_id: u64, admin: Address, arbitrator: Address) { + /// #781: Admin designates an M-of-N committee to rule on a disputed swap. + /// Can only be set once. Minimum 2-of-3, per docs/threat-model.md. + pub fn set_arbitrator( + env: Env, + swap_id: u64, + admin: Address, + signers: Vec
, + threshold: u32, + ) { admin.require_auth(); require_admin(&env, &admin); @@ -1253,54 +1312,225 @@ impl AtomicSwap { if env .storage() .persistent() - .has(&DataKey::SwapArbitrator(swap_id)) + .has(&DataKey::ArbitratorCommittee(swap_id)) { env.panic_with_error(Error::from_contract_error( ContractError::ArbitratorAlreadySet as u32, )); } + if signers.len() < MIN_COMMITTEE_SIGNERS + || threshold < MIN_COMMITTEE_THRESHOLD + || threshold > signers.len() + { + env.panic_with_error(Error::from_contract_error( + ContractError::CommitteeSizeTooSmall as u32, + )); + } + + for i in 0..signers.len() { + let s = signers.get(i).unwrap(); + for j in 0..i { + if signers.get(j).unwrap() == s { + env.panic_with_error(Error::from_contract_error( + ContractError::DuplicateSigner as u32, + )); + } + } + } + + let committee = ArbitratorCommittee { + signers: signers.clone(), + threshold, + }; env.storage() .persistent() - .set(&DataKey::SwapArbitrator(swap_id), &arbitrator); + .set(&DataKey::ArbitratorCommittee(swap_id), &committee); env.storage().persistent().extend_ttl( - &DataKey::SwapArbitrator(swap_id), + &DataKey::ArbitratorCommittee(swap_id), LEDGER_BUMP, LEDGER_BUMP, ); - swap.arbitrator = Some(arbitrator.clone()); + // Informational only — not read for any auth/enforcement decision. + swap.arbitrator = signers.get(0); swap::save_swap(&env, swap_id, &swap); env.events().publish( - (soroban_sdk::symbol_short!("arb_set"),), - ArbitratorSetEvent { + (soroban_sdk::symbol_short!("arb_comm"),), + ArbitratorCommitteeSetEvent { swap_id, - arbitrator, + signers, + threshold, }, ); } - /// Arbitrator resolves a disputed swap. refund=true refunds buyer; false completes to seller. - pub fn arbitrate_dispute(env: Env, swap_id: u64, arbitrator: Address, refund: bool) { - arbitrator.require_auth(); - - let stored_arbitrator: Address = env + /// #781: Validates `signers` against the swap's `ArbitratorCommittee` — + /// each address must `require_auth()`, be a distinct committee member, + /// and together meet the committee's threshold. + fn require_committee_threshold( + env: &Env, + swap_id: u64, + signers: &Vec
, + ) -> ArbitratorCommittee { + let committee: ArbitratorCommittee = env .storage() .persistent() - .get(&DataKey::SwapArbitrator(swap_id)) + .get(&DataKey::ArbitratorCommittee(swap_id)) .unwrap_or_else(|| { env.panic_with_error(Error::from_contract_error( ContractError::NoArbitratorSet as u32, )) }); - if arbitrator != stored_arbitrator { + for i in 0..signers.len() { + let signer = signers.get(i).unwrap(); + signer.require_auth(); + + if !committee.signers.contains(signer.clone()) { + env.panic_with_error(Error::from_contract_error( + ContractError::NotACommitteeSigner as u32, + )); + } + + for j in 0..i { + if signers.get(j).unwrap() == signer { + env.panic_with_error(Error::from_contract_error( + ContractError::DuplicateSigner as u32, + )); + } + } + } + + if signers.len() < committee.threshold { env.panic_with_error(Error::from_contract_error( - ContractError::NotArbitrator as u32, + ContractError::InsufficientSignatures as u32, + )); + } + + committee + } + + /// #781: Committee ruling entry — NOT fund movement. Requires `threshold` + /// of the committee's `signers` to jointly authorize this call (each must + /// `require_auth()`), requires evidence to have already been submitted + /// (read directly from storage, not signer-supplied), and stores the + /// ruling as pending for `execute_ruling` to carry out once the time-lock + /// delay elapses. refund=true favors the buyer; false favors the seller. + pub fn arbitrate_dispute(env: Env, swap_id: u64, signers: Vec
, refund: bool) { + let swap = require_swap_exists(&env, swap_id); + require_swap_status( + &env, + &swap, + SwapStatus::Disputed, + ContractError::NotDisputed, + ); + + if env + .storage() + .persistent() + .has(&DataKey::PendingRuling(swap_id)) + { + env.panic_with_error(Error::from_contract_error( + ContractError::RulingAlreadyPending as u32, + )); + } + + Self::require_committee_threshold(&env, swap_id, &signers); + + let evidence: Vec> = env + .storage() + .persistent() + .get(&DataKey::DisputeEvidence(swap_id)) + .unwrap_or(Vec::new(&env)); + if evidence.is_empty() { + env.panic_with_error(Error::from_contract_error( + ContractError::EvidenceRequired as u32, + )); + } + + let ruled_at = env.ledger().timestamp(); + let pending = PendingRuling { + refund, + ruled_at, + evidence_hashes: evidence.clone(), + ruled_by: signers.clone(), + }; + env.storage() + .persistent() + .set(&DataKey::PendingRuling(swap_id), &pending); + env.storage().persistent().extend_ttl( + &DataKey::PendingRuling(swap_id), + LEDGER_BUMP, + LEDGER_BUMP, + ); + + env.events().publish( + (soroban_sdk::symbol_short!("rule_ent"),), + RulingEnteredEvent { + swap_id, + refund, + ruled_at, + evidence_hashes: evidence, + ruled_by: signers, + }, + ); + } + + /// #781: Cancels a pending ruling before the time-lock delay elapses, + /// requiring the same committee threshold as ruling entry. Once + /// cancelled, `arbitrate_dispute` may be called again to enter a fresh + /// ruling. + pub fn cancel_pending_ruling(env: Env, swap_id: u64, signers: Vec
) { + Self::require_committee_threshold(&env, swap_id, &signers); + + let pending: PendingRuling = env + .storage() + .persistent() + .get(&DataKey::PendingRuling(swap_id)) + .unwrap_or_else(|| { + env.panic_with_error(Error::from_contract_error( + ContractError::NoPendingRuling as u32, + )) + }); + + let config = Self::protocol_config(&env); + let elapsed = env.ledger().timestamp().saturating_sub(pending.ruled_at); + if elapsed >= config.arbitration_ruling_delay_secs { + env.panic_with_error(Error::from_contract_error( + ContractError::RulingFinalized as u32, )); } + env.storage() + .persistent() + .remove(&DataKey::PendingRuling(swap_id)); + + env.events().publish( + (soroban_sdk::symbol_short!("rule_can"),), + RulingCancelledEvent { + swap_id, + cancelled_by: signers, + }, + ); + } + + /// #781: Executes a committee ruling after the time-lock delay has + /// elapsed. Permissionless — anyone may trigger it once the delay has + /// passed, matching `auto_refund_timeout`'s convention. Moves funds and + /// settles dispute bonds (see `settle_dispute_bonds`). + pub fn execute_ruling(env: Env, swap_id: u64) { + let pending: PendingRuling = env + .storage() + .persistent() + .get(&DataKey::PendingRuling(swap_id)) + .unwrap_or_else(|| { + env.panic_with_error(Error::from_contract_error( + ContractError::NoPendingRuling as u32, + )) + }); + let mut swap = require_swap_exists(&env, swap_id); require_swap_status( &env, @@ -1309,13 +1539,21 @@ impl AtomicSwap { ContractError::NotDisputed, ); + let config = Self::protocol_config(&env); + let elapsed = env.ledger().timestamp().saturating_sub(pending.ruled_at); + if elapsed < config.arbitration_ruling_delay_secs { + env.panic_with_error(Error::from_contract_error( + ContractError::TimelockNotElapsed as u32, + )); + } + + let refund = pending.refund; let token_client = token::Client::new(&env, &swap.token); if refund { token_client.transfer(&env.current_contract_address(), &swap.buyer, &swap.price); swap.status = SwapStatus::Cancelled; } else { - let config = Self::protocol_config(&env); let fee_amount = if config.protocol_fee_bps > 0 { (swap.price * config.protocol_fee_bps as i128) / 10000 } else { @@ -1337,27 +1575,186 @@ impl AtomicSwap { swap.status = SwapStatus::Completed; } + Self::settle_dispute_bonds(&env, swap_id, &swap, refund, &token_client); + swap::save_swap(&env, swap_id, &swap); env.storage() .persistent() .remove(&DataKey::ActiveSwap(swap.ip_id)); env.storage() .persistent() - .remove(&DataKey::SwapArbitrator(swap_id)); + .remove(&DataKey::PendingRuling(swap_id)); + env.storage() + .persistent() + .remove(&DataKey::ArbitratorCommittee(swap_id)); Self::append_history(&env, swap_id, swap.status.clone()); env.events().publish( - (soroban_sdk::symbol_short!("arb_dec"),), - ArbitratedEvent { - swap_id, - arbitrator, - refunded: refund, - }, + (soroban_sdk::symbol_short!("rule_exc"),), + RulingExecutedEvent { swap_id, refund }, ); } - /// Buyer or seller submits dispute evidence (a hash of off-chain evidence). + /// #781: Refunds the winning party's dispute bond and forfeits the + /// losing party's bond. Forfeited bonds go to the current `DataKey::Admin` + /// address rather than `protocol_config().treasury` — `protocol_config()` + /// (below) unconditionally returns a hardcoded placeholder address + /// regardless of what's configured (a pre-existing storage bug this PR + /// does not fix); routing real forfeited value through it would be a new, + /// self-inflicted loss path, so real value is deliberately kept on the + /// one admin-storage path that actually round-trips correctly today. + fn settle_dispute_bonds( + env: &Env, + swap_id: u64, + swap: &SwapRecord, + refund: bool, + token_client: &token::Client, + ) { + let bonds: DisputeBonds = env + .storage() + .persistent() + .get(&DataKey::DisputeBond(swap_id)) + .unwrap_or(DisputeBonds { + buyer_bond: 0, + seller_bond: 0, + }); + + if bonds.buyer_bond == 0 && bonds.seller_bond == 0 { + return; + } + + let admin: Address = env + .storage() + .instance() + .get(&DataKey::Admin) + .unwrap_or_else(|| { + env.panic_with_error(Error::from_contract_error( + ContractError::NotInitialized as u32, + )) + }); + + if bonds.buyer_bond > 0 { + if refund { + token_client.transfer( + &env.current_contract_address(), + &swap.buyer, + &bonds.buyer_bond, + ); + env.events().publish( + (soroban_sdk::symbol_short!("bond_rfd"),), + DisputeBondRefundedEvent { + swap_id, + submitter: swap.buyer.clone(), + bond_amount: bonds.buyer_bond, + }, + ); + } else { + token_client.transfer(&env.current_contract_address(), &admin, &bonds.buyer_bond); + env.events().publish( + (soroban_sdk::symbol_short!("bond_fft"),), + DisputeBondForfeitedEvent { + swap_id, + submitter: swap.buyer.clone(), + bond_amount: bonds.buyer_bond, + }, + ); + } + } + + if bonds.seller_bond > 0 { + if !refund { + token_client.transfer( + &env.current_contract_address(), + &swap.seller, + &bonds.seller_bond, + ); + env.events().publish( + (soroban_sdk::symbol_short!("bond_rfd"),), + DisputeBondRefundedEvent { + swap_id, + submitter: swap.seller.clone(), + bond_amount: bonds.seller_bond, + }, + ); + } else { + token_client.transfer(&env.current_contract_address(), &admin, &bonds.seller_bond); + env.events().publish( + (soroban_sdk::symbol_short!("bond_fft"),), + DisputeBondForfeitedEvent { + swap_id, + submitter: swap.seller.clone(), + bond_amount: bonds.seller_bond, + }, + ); + } + } + + env.storage() + .persistent() + .remove(&DataKey::DisputeBond(swap_id)); + } + + /// #781: Called by dispute-resolution paths other than the committee + /// ruling flow (`resolve_dispute`, `auto_refund_timeout`, + /// `admin_rollback_swap`) when they move a swap out of `Disputed`. + /// Refunds any deposited dispute bonds in full (no forfeiture — no + /// committee ruling occurred) and clears any in-flight committee/pending- + /// ruling state, so neither is orphaned in storage. + fn release_dispute_bonds_uncontested(env: &Env, swap_id: u64, swap: &SwapRecord) { + if let Some(bonds) = env + .storage() + .persistent() + .get::<_, DisputeBonds>(&DataKey::DisputeBond(swap_id)) + { + let token_client = token::Client::new(env, &swap.token); + if bonds.buyer_bond > 0 { + token_client.transfer( + &env.current_contract_address(), + &swap.buyer, + &bonds.buyer_bond, + ); + env.events().publish( + (soroban_sdk::symbol_short!("bond_rfd"),), + DisputeBondRefundedEvent { + swap_id, + submitter: swap.buyer.clone(), + bond_amount: bonds.buyer_bond, + }, + ); + } + if bonds.seller_bond > 0 { + token_client.transfer( + &env.current_contract_address(), + &swap.seller, + &bonds.seller_bond, + ); + env.events().publish( + (soroban_sdk::symbol_short!("bond_rfd"),), + DisputeBondRefundedEvent { + swap_id, + submitter: swap.seller.clone(), + bond_amount: bonds.seller_bond, + }, + ); + } + env.storage() + .persistent() + .remove(&DataKey::DisputeBond(swap_id)); + } + + env.storage() + .persistent() + .remove(&DataKey::PendingRuling(swap_id)); + env.storage() + .persistent() + .remove(&DataKey::ArbitratorCommittee(swap_id)); + } + + /// Buyer or seller submits dispute evidence (a hash of off-chain + /// evidence). #781: charges a non-refundable dispute bond (the greater of + /// `MIN_DISPUTE_BOND` or 10% of the swap price) on a submitter's first + /// submission; later submissions by the same address don't re-charge. pub fn submit_dispute_evidence( env: Env, swap_id: u64, @@ -1385,6 +1782,8 @@ impl AtomicSwap { .persistent() .extend_ttl(&key, LEDGER_BUMP, LEDGER_BUMP); + Self::charge_dispute_bond(&env, swap_id, &swap, &submitter); + env.events().publish( (soroban_sdk::symbol_short!("evid_sub"),), DisputeEvidenceSubmittedEvent { @@ -1395,6 +1794,60 @@ impl AtomicSwap { ); } + fn charge_dispute_bond(env: &Env, swap_id: u64, swap: &SwapRecord, submitter: &Address) { + let bond_key = DataKey::DisputeBond(swap_id); + let mut bonds: DisputeBonds = env + .storage() + .persistent() + .get(&bond_key) + .unwrap_or(DisputeBonds { + buyer_bond: 0, + seller_bond: 0, + }); + + let is_buyer = *submitter == swap.buyer; + let already_charged = if is_buyer { + bonds.buyer_bond > 0 + } else { + bonds.seller_bond > 0 + }; + if already_charged { + return; + } + + let pct_bond = swap.price * 1000 / 10000; + let bond_amount = if pct_bond > MIN_DISPUTE_BOND { + pct_bond + } else { + MIN_DISPUTE_BOND + }; + + token::Client::new(env, &swap.token).transfer( + submitter, + &env.current_contract_address(), + &bond_amount, + ); + + if is_buyer { + bonds.buyer_bond = bond_amount; + } else { + bonds.seller_bond = bond_amount; + } + env.storage().persistent().set(&bond_key, &bonds); + env.storage() + .persistent() + .extend_ttl(&bond_key, LEDGER_BUMP, LEDGER_BUMP); + + env.events().publish( + (soroban_sdk::symbol_short!("bond_dep"),), + DisputeBondDepositedEvent { + swap_id, + submitter: submitter.clone(), + bond_amount, + }, + ); + } + /// Returns all dispute evidence hashes submitted for a swap. pub fn get_dispute_evidence(env: Env, swap_id: u64) -> Vec> { env.storage() @@ -1403,8 +1856,6 @@ impl AtomicSwap { .unwrap_or(Vec::new(&env)) } - // submit_dispute_evidence removed - DisputeEvidence DataKey variant not defined - // get_dispute_evidence removed - DisputeEvidence DataKey variant not defined // accept_swap_with_quantity removed - price_tiers field not in SwapRecord /// Buyer accepts a partial quantity of a bulk swap at a proportional price. @@ -1757,6 +2208,7 @@ impl AtomicSwap { dispute_timeout_secs, referral_fee_bps, arbitration_timeout_seconds: 1_209_600, + arbitration_ruling_delay_secs: DEFAULT_ARBITRATION_RULING_DELAY_SECONDS, }, ); } @@ -1779,6 +2231,7 @@ impl AtomicSwap { dispute_timeout_secs: 604800, referral_fee_bps: 100, arbitration_timeout_seconds: 1_209_600, // 14 days + arbitration_ruling_delay_secs: DEFAULT_ARBITRATION_RULING_DELAY_SECONDS, } } @@ -3052,6 +3505,11 @@ impl AtomicSwap { .persistent() .remove(&DataKey::ArbitrationTimestamp(swap_id)); + // #781: this timeout path bypasses the committee/evidence/timelock/ + // bond system — refund any bond in flight rather than orphaning it, + // since no committee ruling occurred here. + Self::release_dispute_bonds_uncontested(&env, swap_id, &swap); + // Refund buyer token::Client::new(&env, &swap.token).transfer( &env.current_contract_address(), @@ -3801,99 +4259,24 @@ impl AtomicSwap { swap_ids } - /// Arbitrate multiple disputed swaps in one call. Arbitrator-only. - /// `refund` applies uniformly to all swaps in the batch. - pub fn batch_arbitrate_swaps(env: Env, swap_ids: Vec, arbitrator: Address, refund: bool) { - arbitrator.require_auth(); - - for swap_id in swap_ids.iter() { - let mut swap = require_swap_exists(&env, swap_id); - require_swap_status( - &env, - &swap, - SwapStatus::Disputed, - ContractError::NotDisputed, - ); - - let token_client = token::Client::new(&env, &swap.token); - - if refund { - token_client.transfer(&env.current_contract_address(), &swap.buyer, &swap.price); - - if swap.collateral_amount > 0 { - if let Some(collateral) = env - .storage() - .persistent() - .get::<_, i128>(&DataKey::SwapCollateral(swap_id)) - { - token_client.transfer( - &env.current_contract_address(), - &swap.buyer, - &collateral, - ); - env.storage() - .persistent() - .remove(&DataKey::SwapCollateral(swap_id)); - } - } - - swap.status = SwapStatus::Cancelled; - } else { - let config = Self::protocol_config(&env); - let fee_amount = if config.protocol_fee_bps > 0 && swap.price > 0 { - (swap.price * config.protocol_fee_bps as i128) / 10000 - } else { - 0 - }; - let seller_amount = swap.price - fee_amount; - token_client.transfer( - &env.current_contract_address(), - &swap.seller, - &seller_amount, - ); - if fee_amount > 0 { - token_client.transfer( - &env.current_contract_address(), - &config.treasury, - &fee_amount, - ); - } - - if swap.collateral_amount > 0 { - if let Some(collateral) = env - .storage() - .persistent() - .get::<_, i128>(&DataKey::SwapCollateral(swap_id)) - { - token_client.transfer( - &env.current_contract_address(), - &swap.seller, - &collateral, - ); - env.storage() - .persistent() - .remove(&DataKey::SwapCollateral(swap_id)); - } - } - - swap.status = SwapStatus::Completed; - } - - swap::save_swap(&env, swap_id, &swap); - env.storage() - .persistent() - .remove(&DataKey::ActiveSwap(swap.ip_id)); - Self::append_history(&env, swap_id, swap.status.clone()); - - env.events().publish( - (soroban_sdk::symbol_short!("arb_dec"),), - ArbitratedEvent { - swap_id, - arbitrator: arbitrator.clone(), - refunded: refund, - }, - ); - } + /// #781: Disabled. This function never validated `arbitrator` against + /// any stored arbitrator/committee — any caller could pass any address + /// and drain any disputed swap through it, fully bypassing the + /// committee/evidence/timelock/bond system added in #781. Left in place + /// (rather than deleted) only to preserve its signature/selector; + /// unconditionally rejected pending a follow-up issue to migrate it onto + /// the `ArbitratorCommittee` model, at which point it can call + /// `arbitrate_dispute`/`execute_ruling` per swap instead of moving funds + /// directly. + pub fn batch_arbitrate_swaps( + env: Env, + _swap_ids: Vec, + _arbitrator: Address, + _refund: bool, + ) { + env.panic_with_error(Error::from_contract_error( + ContractError::BatchArbitrationDisabled as u32, + )); } // ── #358: Swap Timeout Escalation ───────────────────────────────────────── @@ -4859,14 +5242,20 @@ impl AtomicSwap { // #[cfg(test)] // mod escrow_tests; -// FIXME: pre-existing compile errors from merge conflict - re-enable after fix -// #[cfg(test)] -// mod arbitration_tests; +// #781: re-enabled — was blocked by 3 pre-existing, unrelated compile errors +// (a dropped `commit_ip` arg, and two obsolete `accept_swap_with_quantity` +// calls to a since-removed function; both fixed / removed in this PR). +#[cfg(test)] +mod arbitration_tests; // FIXME: pre-existing compile errors from merge conflict - re-enable after fix // include!("multi_signer_tests.rs"); // FIXME: pre-existing compile errors from merge conflict - re-enable after fix +// (also has pre-existing runtime failures in test_batch_reveal_keys_* unrelated +// to #781 — a protocol-fee transfer to protocol_config().treasury's hardcoded +// placeholder address fails for lack of a trustline; same root cause noted in +// docs/threat-model.md's #781 update, left unfixed here as out of scope). // #[cfg(test)] // mod batch_swap_features_tests; diff --git a/contracts/atomic_swap/src/types.rs b/contracts/atomic_swap/src/types.rs index 88c2777..66efabb 100644 --- a/contracts/atomic_swap/src/types.rs +++ b/contracts/atomic_swap/src/types.rs @@ -201,6 +201,97 @@ pub struct ArbitratedEvent { pub refunded: bool, } +// ── #781: M-of-N Arbitrator Committee, Time-Locked Ruling, Dispute Bond ──────── + +/// A designated committee of `signers`, `threshold` of whom must jointly +/// authorize a ruling. Replaces the single trusted `Address` previously stored +/// under `DataKey::SwapArbitrator`. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct ArbitratorCommittee { + pub signers: Vec
, + pub threshold: u32, +} + +/// A ruling entered by the committee but not yet executed. `evidence_hashes` is +/// a snapshot of `DisputeEvidence` read from storage at ruling-entry time (not +/// supplied by the signers), so outside observers can verify the ruling +/// actually references submitted evidence. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct PendingRuling { + pub refund: bool, + pub ruled_at: u64, + pub evidence_hashes: Vec>, + pub ruled_by: Vec
, +} + +/// Non-refundable dispute bonds deposited by the buyer and/or seller on their +/// first evidence submission for a swap. Zero means that party has not +/// submitted evidence (or hasn't been charged yet). +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct DisputeBonds { + pub buyer_bond: i128, + pub seller_bond: i128, +} + +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct ArbitratorCommitteeSetEvent { + pub swap_id: u64, + pub signers: Vec
, + pub threshold: u32, +} + +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct RulingEnteredEvent { + pub swap_id: u64, + pub refund: bool, + pub ruled_at: u64, + pub evidence_hashes: Vec>, + pub ruled_by: Vec
, +} + +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct RulingCancelledEvent { + pub swap_id: u64, + pub cancelled_by: Vec
, +} + +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct RulingExecutedEvent { + pub swap_id: u64, + pub refund: bool, +} + +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct DisputeBondDepositedEvent { + pub swap_id: u64, + pub submitter: Address, + pub bond_amount: i128, +} + +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct DisputeBondForfeitedEvent { + pub swap_id: u64, + pub submitter: Address, + pub bond_amount: i128, +} + +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct DisputeBondRefundedEvent { + pub swap_id: u64, + pub submitter: Address, + pub bond_amount: i128, +} + // ── #360: Admin Rollback Event ─────────────────────────────────────────────── #[contracttype] diff --git a/docs/threat-model.md b/docs/threat-model.md index bbd24ac..7e92716 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -210,12 +210,46 @@ The dispute resolution mechanism allows a designated admin to adjudicate contest **Impact**: Fraudulent dispute outcomes; direct loss of buyer funds or seller IP. **Mitigations**: -- Admin role must be a **multi-sig account** (minimum 2-of-3 threshold; 3-of-5 recommended for high-value deployments) -- All admin rulings are recorded on-chain with the admin address and ledger timestamp — fully auditable -- A **48-hour time-lock** between ruling and fund release gives the losing party time to escalate off-chain -- Admin key rotation is supported via contract upgrade path; rotation procedure must be documented before mainnet - -**Status**: ⚠️ Partially mitigated — depends on operator deploying multi-sig correctly +- Per-swap dispute rulings now require an **M-of-N arbitrator committee** + (`set_arbitrator`, minimum 2-of-3 threshold, 3-of-5 or larger supported) — + `arbitrate_dispute` requires `threshold` of the committee's `signers` to + jointly `require_auth()` a ruling in one call; no single key can rule alone. +- A ruling is bound to submitted evidence: `arbitrate_dispute` rejects a + ruling if `DisputeEvidence` is empty, and the evidence hashes actually + considered (read from storage, not signer-supplied) are recorded in + `RulingEnteredEvent` for outside audit. +- A **48-hour time-lock** (`execute_ruling`, `ProtocolConfig.arbitration_ruling_delay_secs`) + separates ruling entry from fund release; `cancel_pending_ruling` lets the + same committee threshold void a ruling within that window. +- All transitions are individually auditable: `ArbitratorCommitteeSetEvent`, + `RulingEnteredEvent`, `RulingCancelledEvent`, `RulingExecutedEvent`. +- **Not covered by the above, still open**: the contract's own `Admin` role + (`DataKey::Admin`) remains a single `Address` — it alone appoints the + arbitrator committee and is unaffected by this mechanism. More directly, + `resolve_dispute` still lets that single admin resolve any disputed swap + directly, completely bypassing the committee/evidence/timelock/bond system + described above — a real, disclosed gap, recommended as a follow-up issue + (either remove `resolve_dispute`'s direct fund-movement path or route it + through the same committee/timelock). `admin_rollback_swap` and + `auto_refund_timeout` are narrower emergency/timeout escape hatches that + remain single-admin/permissionless by design, but now correctly refund + (never forfeit) any dispute bond in flight if they fire instead of a + committee ruling, so they no longer orphan bonded value. +- `batch_arbitrate_swaps` — previously did not validate its caller against + any stored arbitrator at all — is now disabled (always reverts) pending a + follow-up migration onto the committee model. `arbitrate_swap`, a + collateral-aware duplicate of the old single-key `arbitrate_dispute` that + never checked swap status, is now permanently unreachable since nothing + populates the legacy `SwapArbitrator` key anymore; this also closes a + pre-existing double-payout risk where it could still be called after + `resolve_dispute` had already paid out a swap. +- Admin key rotation is supported via contract upgrade path; rotation + procedure must still be documented before mainnet. + +**Status**: ⚠️ Partially mitigated — per-swap arbitration is now committee-gated, +evidence-bound, and time-locked, but the contract `Admin` role itself is still +single-key and `resolve_dispute` remains an admin-direct bypass of the new +safeguards. --- @@ -226,12 +260,23 @@ The dispute resolution mechanism allows a designated admin to adjudicate contest **Impact**: Counterparty funds locked; griefing / DoS against legitimate swap completion. **Mitigations**: -- Disputes require an **on-chain evidence hash** (`dispute_evidence` field) submitted at filing time — no evidence, no dispute -- A **non-refundable dispute bond** (minimum 1 XLM or 10% of swap price, whichever is greater) is forfeited if the dispute is ruled frivolous +- Disputes require an **on-chain evidence hash** (`submit_dispute_evidence`) — + no evidence, no dispute, and (see #13) no evidence means `arbitrate_dispute` + cannot enter a ruling at all. +- A **non-refundable dispute bond** (`MIN_DISPUTE_BOND`, the greater of 1 XLM + or 10% of swap price) is charged on a party's first evidence submission and + forfeited to the contract admin if the committee's ruling goes against that + party; refunded in full if the ruling goes their way, or if the dispute is + instead resolved via one of the non-committee paths in #13 (no ruling ⇒ no + forfeiture). - Disputes must be filed within `dispute_period` ledgers of the triggering event; late filings are rejected by the contract - Repeated frivolous filings from the same address are rate-limited by the admin -**Status**: ⚠️ Partially mitigated — bond amount and evidence format must be configured by operator +**Status**: ⚠️ Partially mitigated — evidence + bond are now enforced in code +with a fixed minimum; bond forfeiture destination is the contract admin +address rather than the protocol treasury (`protocol_config().treasury` +resolves to a hardcoded placeholder today — a pre-existing, separate storage +bug, not fixed by this change). ---