Fix treasury/resolution CEI ordering, protocol-wide collateral ADR, remove legacy instant threshold setter

contracts/market/STORAGE_MIGRATION_GUIDE.md

@@ -651,7 +651,41 @@ pub fn new_storage_function(env: &Env) {
 
 ## Version History
 
-### Version 4 (Current)
+### Version 6 (Current)
+
+**Date:** 2026-08-25
+**Changes:**
+- Added `StorageKey::CollateralBalance(Address)` and
+  `StorageKey::TotalLockedCollateral(Address)` — a protocol-wide, user-scoped
+  collateral ledger (ADR-002, issue #685). `deposit_collateral` credits
+  `CollateralBalance(user)` in addition to the existing per-market
+  `Position.total_deposited`, and `update_position` checks a trade's
+  prospective lock against this shared balance (net of what is already
+  locked in the user's other markets) instead of the per-market field alone.
+  See `docs/adr-002-protocol-wide-collateral.md`.
+
+**Migration:** Fresh deployment required. No data migration available.
+Existing per-market `Position` records are untouched; a user's protocol-wide
+`CollateralBalance` and `TotalLockedCollateral` simply start at `0` and
+accrue from the next `deposit_collateral` / `update_position` call onward.
+
+**Breaking Changes:** None (additive only) — two new `StorageKey` variants.
+
+---
+
+### Version 5
+
+**Date:** 2026-Q2
+**Changes:**
+- Added `EmergencyMode` storage for coordinated emergency mode (#662)
+
+**Migration:** Fresh deployment required. No data migration available.
+
+**Breaking Changes:** None (additive only) — new `StorageKey` variant.
+
+---
+
+### Version 4
 
 **Date:** 2025-Q1
 **Changes:**

contracts/market/src/deposit.rs

@@ -125,10 +125,24 @@ pub fn deposit_collateral(
     // - Global user balance (deposit once, trade anywhere)
     // - Better capital efficiency
     //
-    // # Current Flow
-    // 1. User deposits USDC into specific market
-    // 2. Collateral locked to this market only
-    // 3. User must deposit separately for each market they want to trade
+    // Every deposit credits `storage::CollateralBalance(user)`, a balance
+    // scoped by *user only* (not by market — see `StorageKey::CollateralBalance`
+    // in `storage.rs`). `MarketContract::update_position` checks a trade's
+    // prospective lock against this shared balance (net of whatever is
+    // already locked in the user's *other* markets, tracked in
+    // `StorageKey::TotalLockedCollateral`), which is what lets a user deposit
+    // once and trade in any market without a second deposit.
+    //
+    // The legacy per-market `Position.total_deposited` field below is kept
+    // as-is for backward compatibility with `withdraw_unused_collateral` and
+    // settlement, which still refund/settle per market; migrating those to
+    // draw from the protocol-wide balance is tracked as follow-up work in
+    // the ADR.
+    let new_collateral_balance = storage::get_collateral_balance(&env, &user)
+        .checked_add(amount)
+        .ok_or(ContractError::ArithmeticOverflow)?;
+    storage::set_collateral_balance(&env, &user, new_collateral_balance);
+
     let mut position = storage::get_position(&env, market_id, &user)?.unwrap_or_else(|| Position {
         market_id,
         user: user.clone(),

contracts/market/src/lib.rs

@@ -1157,9 +1157,35 @@ impl MarketContract {
             if lock_increased && market.closed_to_deposits {
                 return Err(ContractError::MarketClosedToDeposits);
             }
-            if lock_increased && prospective_locked > position.total_deposited {
-                return Err(ContractError::InsufficientCollateral);
+            // Protocol-wide collateral check (ADR-002, issue #685): collateral
+            // is a single balance per user shared across every market
+            // (`storage::CollateralBalance`) rather than siloed per market.
+            // A trade is only rejected here when it would *increase* this
+            // market's lock beyond what the user's balance can cover once
+            // collateral already locked in every *other* market
+            // (`storage::TotalLockedCollateral` minus this market's current
+            // lock) is accounted for.
+            let locked_elsewhere = storage::get_total_locked_collateral(&env, &user)
+                .saturating_sub(position.locked_collateral);
+            if lock_increased {
+                let protocol_balance = storage::get_collateral_balance(&env, &user);
+                if positions::check_protocol_collateral(
+                    prospective_locked,
+                    protocol_balance,
+                    locked_elsewhere,
+                )
+                .is_err()
+                {
+                    return Err(ContractError::InsufficientCollateral);
+                }
             }
+            // Keep the protocol-wide aggregate in sync so other markets see
+            // this market's updated lock immediately.
+            storage::set_total_locked_collateral(
+                &env,
+                &user,
+                locked_elsewhere.saturating_add(prospective_locked),
+            );
         }
 
         // 5. Apply the share deltas (persists the position and emits an event)
@@ -1650,7 +1676,14 @@ impl MarketContract {
     /// # Errors
     /// - [`ContractError::InvalidThresholdQuorum`] — `quorum` exceeds
     ///   `signers.len()`; such a quorum could never be satisfied.
+    ///
     /// Propose a threshold signer set and quorum update, subject to timelock (#665).
+    ///
+    /// This is now the *only* production path to change the global threshold
+    /// signer set — the legacy instant `set_threshold_signers` entrypoint was
+    /// removed (#684) because it let an admin bypass the
+    /// [`FEE_RATE_TIMELOCK_SECONDS`] (172,800s / 48h) delay enforced here and
+    /// in [`Self::execute_threshold_signers`].
     pub fn propose_threshold_signers(
         env: Env,
         admin: Address,
@@ -1739,28 +1772,6 @@ impl MarketContract {
         Ok(())
     }
 
-    /// Legacy immediate set_threshold_signers retained for test backward compatibility.
-    pub fn set_threshold_signers(
-        env: Env,
-        admin: Address,
-        signers: soroban_sdk::Vec<BytesN<32>>,
-        quorum: u32,
-    ) -> Result<(), ContractError> {
-        validation::require_initialized(&env)?;
-        admin.require_auth();
-        let stored_admin = storage::get_admin(&env)?;
-        if admin != stored_admin {
-            return Err(ContractError::NotAdmin);
-        }
-        let signers_len = signers.len();
-        if quorum == 0 || signers_len == 0 || quorum > signers_len {
-            return Err(ContractError::InvalidThresholdQuorum);
-        }
-        storage::set_threshold_signers(&env, &signers);
-        storage::set_threshold_quorum(&env, quorum);
-        Ok(())
-    }
-
     /// Return the current threshold signer set.
     pub fn get_threshold_signers(env: Env) -> soroban_sdk::Vec<BytesN<32>> {
         storage::get_threshold_signers(&env)

contracts/market/src/positions.rs

@@ -15,6 +15,10 @@ pub enum PositionError {
     ShareBalanceBelowZero = 1,
     /// Market price is outside the valid basis-point range (0–10_000)
     InvalidMarketPrice = 2,
+    /// The user's protocol-wide collateral balance cannot cover this
+    /// market's prospective lock once collateral already locked in the
+    /// user's other markets is accounted for (ADR-002, issue #685).
+    InsufficientProtocolCollateral = 3,
 }
 
 /// Scale `amount` by `price_bps` basis points (i.e. `amount * price_bps / 10_000`).
@@ -81,6 +85,33 @@ pub fn validate_position_change(
     Ok(())
 }
 
+/// Check whether a user's protocol-wide collateral balance (ADR-002, issue
+/// #685) can cover a prospective locked-collateral amount for one market,
+/// once collateral already locked in the user's *other* markets is taken
+/// into account.
+///
+/// Replaces the old per-market check against `Position.total_deposited`:
+/// `collateral_balance` is the user's single balance shared across every
+/// market (see `storage::CollateralBalance`), and `locked_elsewhere` is the
+/// sum of `locked_collateral` across every *other* market the user holds a
+/// position in (see `storage::TotalLockedCollateral`). A trade that would
+/// only keep this market's lock flat or reduce it is never rejected by this
+/// check — callers should only invoke it when the lock is increasing.
+///
+/// # Errors
+/// Returns [`PositionError::InsufficientProtocolCollateral`] when
+/// `prospective_locked + locked_elsewhere > collateral_balance`.
+pub fn check_protocol_collateral(
+    prospective_locked: i128,
+    collateral_balance: i128,
+    locked_elsewhere: i128,
+) -> Result<(), PositionError> {
+    if prospective_locked.saturating_add(locked_elsewhere) > collateral_balance {
+        return Err(PositionError::InsufficientProtocolCollateral);
+    }
+    Ok(())
+}
+
 /// Determine which side exceeded the allowed position limits.
 ///
 /// Returns `true` when the YES side would underflow, or `false` when the NO

contracts/market/src/storage.rs

@@ -31,9 +31,13 @@ use soroban_sdk::{contracttype, Address, BytesN, Env, Vec};
 /// 5. Initialize: `stellar contract invoke ... -- initialize --admin <addr>`
 /// 6. Verify old deployment returns `UpgradeRequired` error
 ///
-/// ## Current version: 5
+/// ## Current version: 6
 ///
 /// ### Version history:
+/// - **v6:** Added `CollateralBalance(Address)` and
+///   `TotalLockedCollateral(Address)` — the protocol-wide, user-scoped
+///   collateral ledger introduced by ADR-002 (issue #685). See
+///   `docs/adr-002-protocol-wide-collateral.md`.
 /// - **v5:** Added `EmergencyMode` storage for coordinated emergency mode (#662)
 /// - **v4:** Added per-adapter-type `AdapterEnabled` flag for the Reflector/Pyth
 ///   Ed25519 fallback path (#488)
@@ -114,6 +118,21 @@ pub enum StorageKey {
     MarketThresholdSigners(u32),
     /// Per-market threshold quorum override (#665).
     MarketThresholdQuorum(u32),
+    /// Protocol-wide collateral balance for a user, scoped by user only —
+    /// **not** by market (ADR-002, issue #685). Deposits made via
+    /// `deposit_collateral` credit this balance regardless of which market
+    /// they were deposited against, and it is the shared pool checked
+    /// against when a trade in *any* market would increase that market's
+    /// locked collateral. This replaces the old per-market silo where a
+    /// user had to re-deposit collateral separately for every market.
+    CollateralBalance(Address),
+    /// Aggregate `locked_collateral` across every market for a user
+    /// (ADR-002, issue #685). Kept in sync by
+    /// `MarketContract::update_position` so the protocol-wide invariant
+    /// (`sum of locked_collateral across all markets <= CollateralBalance`)
+    /// can be checked in O(1) instead of iterating every market the user
+    /// has ever traded in.
+    TotalLockedCollateral(Address),
 }
 
 pub fn get_pending_threshold_signers(env: &Env) -> Option<crate::types::PendingThresholdSignersChange> {
@@ -251,6 +270,41 @@ pub fn has_position(env: &Env, market_id: u32, user: &Address) -> Result<bool, C
     Ok(env.storage().persistent().has(&StorageKey::Position(market_id, user.clone())))
 }
 
+// --- Protocol-wide Collateral (ADR-002, Issue #685) ---
+
+/// Return `user`'s protocol-wide collateral balance (the sum of everything
+/// they have deposited across every market via `deposit_collateral`).
+/// Defaults to `0` when the user has never deposited.
+pub fn get_collateral_balance(env: &Env, user: &Address) -> i128 {
+    env.storage()
+        .persistent()
+        .get(&StorageKey::CollateralBalance(user.clone()))
+        .unwrap_or(0)
+}
+
+/// Set `user`'s protocol-wide collateral balance.
+pub fn set_collateral_balance(env: &Env, user: &Address, balance: i128) {
+    env.storage()
+        .persistent()
+        .set(&StorageKey::CollateralBalance(user.clone()), &balance);
+}
+
+/// Return the aggregate `locked_collateral` across every market for `user`.
+/// Defaults to `0` when the user has no open positions.
+pub fn get_total_locked_collateral(env: &Env, user: &Address) -> i128 {
+    env.storage()
+        .persistent()
+        .get(&StorageKey::TotalLockedCollateral(user.clone()))
+        .unwrap_or(0)
+}
+
+/// Set the aggregate `locked_collateral` across every market for `user`.
+pub fn set_total_locked_collateral(env: &Env, user: &Address, locked: i128) {
+    env.storage()
+        .persistent()
+        .set(&StorageKey::TotalLockedCollateral(user.clone()), &locked);
+}
+
 // --- Market Participants (Issue #495) ---
 
 /// Return the ordered list of every address that has ever held a position

contracts/market/src/test.rs

@@ -1707,7 +1707,10 @@ mod test {
 
         let (signer, signature) = generate_test_keypair_and_sign(&env, market_id, true);
         let signers = soroban_sdk::vec![&env, signer];
-        client.set_threshold_signers(&admin, &signers, &1u32);
+        client.propose_threshold_signers(&admin, &signers, &1u32);
+        env.ledger()
+            .set_timestamp(env.ledger().timestamp() + crate::FEE_RATE_TIMELOCK_SECONDS);
+        client.execute_threshold_signers();
         let signatures = soroban_sdk::vec![&env, signature];
 
         // Registering the challenge-based resolution contract selects that
@@ -1746,7 +1749,10 @@ mod test {
 
         let (signer, signature) = generate_test_keypair_and_sign(&env, market_id, true);
         let signers = soroban_sdk::vec![&env, signer];
-        client.set_threshold_signers(&admin, &signers, &1u32);
+        client.propose_threshold_signers(&admin, &signers, &1u32);
+        env.ledger()
+            .set_timestamp(env.ledger().timestamp() + crate::FEE_RATE_TIMELOCK_SECONDS);
+        client.execute_threshold_signers();
         let signatures = soroban_sdk::vec![&env, signature];
 
         assert_eq!(
@@ -2407,7 +2413,7 @@ mod test {
             Err(Ok(ContractError::NotAdmin))
         );
         assert_eq!(
-            client.try_set_threshold_signers(&stranger, &soroban_sdk::Vec::new(&env), &1u32),
+            client.try_propose_threshold_signers(&stranger, &soroban_sdk::Vec::new(&env), &1u32),
             Err(Ok(ContractError::NotAdmin))
         );
         assert_eq!(

contracts/resolution/src/lib.rs

@@ -335,6 +335,12 @@ impl ResolutionContract {
             bond_amount,
         };
 
+        // Effects: persist the new candidate (status = Proposed) *before*
+        // making any external call (CEI — issue #686, see
+        // docs/reentrancy-cei-audit.md). Previously the bond transfer below
+        // ran first, leaving a window where a malicious/upgraded token
+        // contract's `transfer` callback could re-enter this contract while
+        // no candidate record existed yet for this market.
         storage::set_candidate(&env, &candidate);
         events::emit_candidate_proposed(&env, &candidate);
 
@@ -428,6 +434,12 @@ impl ResolutionContract {
             &challenge_uri,
             bond_amount,
         );
+
+        // Interactions: lock the challenger's bond only after state is
+        // committed.
+        let this = env.current_contract_address();
+        TokenClient::new(&env, &collateral_token).transfer(&challenger, &this, &bond_amount);
+
         Ok(())
     }
 

contracts/treasury/src/lib.rs

@@ -558,6 +558,25 @@ impl TreasuryContract {
     /// integer-division remainder (dust) stays in the treasury balance and
     /// rolls into the next distribution.
     ///
+    /// # Dust remainder (issue #688)
+    /// Because each share is floor-divided, `sum(amount)` across all
+    /// stakeholders can be up to `stakeholders.len() - 1` stroops less than
+    /// `balance`. That leftover is **not** dropped: `remaining = balance -
+    /// distributed` is written back to `TokenBalance(token)` before any
+    /// transfer is made, so it is simply carried forward and gets
+    /// distributed — proportionally, like any other collected fee — the
+    /// next time `distribute_fees` runs for this token.
+    ///
+    /// # CEI ordering (issue #688)
+    /// All per-stakeholder amounts are computed and the treasury's own
+    /// balance is persisted (`storage::set_token_balance`) *before* any
+    /// external `token_client.transfer` call is made, per
+    /// Checks-Effects-Interactions (see `docs/reentrancy-cei-audit.md`).
+    /// This closes a reentrancy window where a malicious/upgraded token
+    /// contract's `transfer` callback could otherwise re-enter
+    /// `distribute_fees` while the old (undecremented) balance was still
+    /// visible in storage.
+    ///
     /// # Errors
     /// - [`TreasuryError::NotInitialized`] – treasury not initialized.
     /// - [`TreasuryError::ContractPaused`] – treasury is paused.
@@ -617,9 +636,17 @@ impl TreasuryContract {
                 distributed = distributed
                     .checked_add(amount)
                     .ok_or(TreasuryError::ArithmeticOverflow)?;
+                payouts.push_back((stakeholder, amount));
             }
         }
 
+        // Floor-division dust remainder: `balance - distributed` is whatever
+        // is left after every stakeholder's basis-point share is rounded
+        // down. It is credited straight back into the treasury's own
+        // `TokenBalance(token)` below (not dropped, and not sent to any one
+        // stakeholder), so it simply rolls forward and is redistributed —
+        // proportionally, same as any other collected fee — the next time
+        // `distribute_fees` is called for this token.
         let remaining = balance - distributed;
         storage::set_token_balance(&env, &token, remaining);