feat: dispute arbiter registry, backfill/migration guard, per-invoice early payment discount (closes #1820, #1821, #1840, #1847)

quicklendx-contracts/src/arbiter.rs

@@ -0,0 +1,132 @@
+//! Dispute arbiter registry.
+//!
+//! # Why a separate arbiter registry?
+//!
+//! Admin authority controls protocol configuration (fee schedule, listings,
+//! treasury, pause, upgrade). It is intentionally **privileged but broad** —
+//! losing an admin key is bad for the protocol, but losing an arbiter key is
+//! catastrophic for the users it can hurt: every dispute resolution moves
+//! escrowed funds. Conflating the two roles lets a single compromised admin
+//! key drain every disputed investment.
+//!
+//! The arbiter registry splits those concerns: only addresses that have been
+//! **explicitly registered** as arbiters may drive the dispute lifecycle.
+//! Removing admin authority no longer removes dispute authority (and vice
+//! versa), so a plane-key rotation on either side does not silently transfer
+//! the other.
+//!
+//! # Storage layout
+//!
+//! * `arbiters` — `Vec<Address>` of registered arbiters, instance storage.
+//! * Each registered arbiter is also stored as a `bool` flag under
+//!   `(ARBITER_FLAG_KEY, address)` so membership checks are O(1).
+//!
+//! Both are kept in sync by `register_arbiter` / `unregister_arbiter`.
+
+use crate::admin::AdminStorage;
+use crate::errors::QuickLendXError;
+use soroban_sdk::{symbol_short, Address, Env, Symbol, Vec};
+
+const ARBITERS_KEY: Symbol = symbol_short!("arbiters");
+const ARBITER_FLAG_KEY: Symbol = symbol_short!("arb_flag");
+
+/// Storage layer for the dispute-arbiter registry.
+///
+/// All write operations require admin authorization (`AdminStorage::require_admin_auth`);
+/// membership checks (`is_arbiter`, `require_dispute_arbiter`) only require
+/// the address itself and are therefore callable from any dispute
+/// entrypoint.
+pub struct ArbiterStorage;
+
+impl ArbiterStorage {
+    /// Return every currently registered arbiter address.
+    pub fn list_arbiters(env: &Env) -> Vec<Address> {
+        env.storage()
+            .instance()
+            .get(&ARBITERS_KEY)
+            .unwrap_or_else(|| Vec::new(env))
+    }
+
+    /// Membership test: O(1) flag lookup, no allocation.
+    pub fn is_arbiter(env: &Env, address: &Address) -> bool {
+        env.storage()
+            .instance()
+            .get(&(ARBITER_FLAG_KEY, address.clone()))
+            .unwrap_or(false)
+    }
+
+    /// Register a new dispute arbiter (admin only).
+    ///
+    /// Idempotent — re-registering an existing arbiter is a no-op and does
+    /// **not** emit a duplicate-registration error. Audit trails come from
+    /// the `arbiter_registered` event emitted below.
+    pub fn register_arbiter(env: &Env, admin: &Address, arbiter: &Address) -> Result<(), QuickLendXError> {
+        AdminStorage::require_admin_auth(env, admin)?;
+
+        if Self::is_arbiter(env, arbiter) {
+            return Ok(());
+        }
+
+        // Update the O(1) flag first so a concurrent listing read sees a
+        // consistent view: either the address is fully registered (flag + list)
+        // or not at all.
+        env.storage()
+            .instance()
+            .set(&(ARBITER_FLAG_KEY, arbiter.clone()), &true);
+
+        let mut arbiters = Self::list_arbiters(env);
+        arbiters.push_back(arbiter.clone());
+        env.storage().instance().set(&ARBITERS_KEY, &arbiters);
+
+        crate::events::arbiter_registered(env, admin, arbiter);
+        Ok(())
+    }
+
+    /// Revoke a registered dispute arbiter (admin only).
+    ///
+    /// Returns `OperationNotAllowed` if the address was never registered; this
+    /// surfaces accidental no-op revocations to monitoring rather than letting
+    /// them slip by as silent successes.
+    pub fn unregister_arbiter(
+        env: &Env,
+        admin: &Address,
+        arbiter: &Address,
+    ) -> Result<(), QuickLendXError> {
+        AdminStorage::require_admin_auth(env, admin)?;
+
+        if !Self::is_arbiter(env, arbiter) {
+            return Err(QuickLendXError::OperationNotAllowed);
+        }
+
+        env.storage()
+            .instance()
+            .remove(&(ARBITER_FLAG_KEY, arbiter.clone()));
+
+        let current = Self::list_arbiters(env);
+        let mut next: Vec<Address> = Vec::new(env);
+        for entry in current.iter() {
+            if entry != *arbiter {
+                next.push_back(entry);
+            }
+        }
+        env.storage().instance().set(&ARBITERS_KEY, &next);
+
+        crate::events::arbiter_revoked(env, admin, arbiter);
+        Ok(())
+    }
+
+    /// Guard: `address` must be a registered dispute arbiter.
+    ///
+    /// Distinct from [`AdminStorage::require_admin`]: admin status controls
+    /// protocol configuration; arbiter status controls dispute resolution.
+    /// They are deliberately not the same authority.
+    pub fn require_dispute_arbiter(
+        env: &Env,
+        address: &Address,
+    ) -> Result<(), QuickLendXError> {
+        if !Self::is_arbiter(env, address) {
+            return Err(QuickLendXError::NotArbiter);
+        }
+        Ok(())
+    }
+}

quicklendx-contracts/src/backup.rs

@@ -6,6 +6,7 @@ const RETENTION_POLICY_KEY: soroban_sdk::Symbol = symbol_short!("bkup_pol");
 const BACKUP_COUNTER_KEY: soroban_sdk::Symbol = symbol_short!("bkup_cnt");
 const BACKUP_LIST_KEY: soroban_sdk::Symbol = symbol_short!("backups");
 const BACKUP_DATA_KEY: soroban_sdk::Symbol = symbol_short!("bkup_data");
+pub const PENDING_BACKFILL_KEY: soroban_sdk::Symbol = symbol_short!("pf_back");
 const MAX_BACKUP_DESCRIPTION_LENGTH: u32 = 128;
 
 /// A stored snapshot of all invoices at a point in time.
@@ -231,7 +232,7 @@ impl BackupStorage {
                     } else if version == 1 {
                         Ok(1)
                     } else {
-                        Err(QuickLendXError::BackupVersionUnsupported)
+                        Err(QuickLendXError::BackfillInProgress)
                     }
                 } else {
                     Err(QuickLendXError::StorageError)
@@ -387,30 +388,46 @@ impl BackupStorage {
     ///   indexes, causing ghost entries in status/category/tag buckets for
     ///   any invoices that existed before the restore.
     pub fn restore_from_backup(env: &Env, backup_id: &BytesN<32>) -> Result<u32, QuickLendXError> {
-        //  Step 1: validate before mutating anything
+        // Step 1: validate before mutating anything.
         Self::validate_backup(env, backup_id)?;
 
-        // Fetch the validated payload.
-        let data = Self::get_backup_data(env, backup_id).unwrap();
+        // Step 1b: mark a destructive backfill as in progress *before* any
+        // invoice-state mutation, so that `schedule_upgrade` cannot fire in
+        // between clear and rebuild. The flag is set first and cleared last
+        // so a panic between steps leaves `true` rather than `false` (false
+        // positives are safer than false negatives here — they at worst
+        // block a legitimate upgrade, never corrupt a restore).
+        env.storage().instance().set(&PENDING_BACKFILL_KEY, &true);
 
-        let restored_count = data.len();
+        let restore_outcome: Result<u32, QuickLendXError> = (|| {
+            // Fetch the validated payload.
+            let data = Self::get_backup_data(env, backup_id).unwrap();
 
-        //  Step 2: atomically clear all existing invoice state
-        crate::storage::InvoiceStorage::clear_all(env);
+            let restored_count = data.len();
 
-        //  Step 3: re-register every invoice, rebuilding all indexes
-        for invoice in data.iter() {
-            crate::storage::InvoiceStorage::store_invoice(env, &invoice);
-        }
+            //  Step 2: atomically clear all existing invoice state
+            crate::storage::InvoiceStorage::clear_all(env);
 
-        // Step 4: mark the backup as archived to prevent re-use
-        if let Some(mut backup) = Self::get_backup(env, backup_id) {
-            backup.status = BackupStatus::Archived;
-            // Ignore the result - the restore itself has already succeeded.
-            let _ = Self::update_backup(env, &backup);
-        }
+            //  Step 3: re-register every invoice, rebuilding all indexes
+            for invoice in data.iter() {
+                crate::storage::InvoiceStorage::store_invoice(env, &invoice);
+            }
 
-        Ok(restored_count)
+            // Step 4: mark the backup as archived to prevent re-use
+            if let Some(mut backup) = Self::get_backup(env, backup_id) {
+                backup.status = BackupStatus::Archived;
+                // Ignore the result - the restore itself has already succeeded.
+                let _ = Self::update_backup(env, &backup);
+            }
+
+            Ok(restored_count)
+        })();
+
+        // Always clear the in-progress flag, even on failure, so the contract
+        // is not stuck in "backfilling" forever.
+        env.storage().instance().remove(&PENDING_BACKFILL_KEY);
+
+        restore_outcome
     }
 
     /// Clean up old backups based on the retention policy.
@@ -554,6 +571,31 @@ impl BackupStorage {
         }
     }
 
+    /// Returns true when a destructive backfill operation is currently
+    /// mutating invoice state (between validate and archive in
+    /// `restore_from_backup`).
+    ///
+    /// Used by the WASM upgrade guard to refuse migration while a backfill is
+    /// in progress: the new contract code would otherwise come online reading
+    /// partially-restored state with no signal that it is partial.
+    pub fn is_pending_backfill(env: &Env) -> bool {
+        env.storage()
+            .instance()
+            .get(&PENDING_BACKFILL_KEY)
+            .unwrap_or(false)
+    }
+
+    /// Guard: reject any operation that must not race an in-flight backfill.
+    ///
+    /// Returns `QuickLendXError::BackfillInProgress` while a backfill holds
+    /// the in-progress flag; `Ok(())` once the flag has been cleared.
+    pub fn require_no_pending_backfill(env: &Env) -> Result<(), QuickLendXError> {
+        if Self::is_pending_backfill(env) {
+            return Err(QuickLendXError::BackfillInProgress);
+        }
+        Ok(())
+    }
+
     /// Retrieve all invoices from storage across all possible statuses.
     ///
     /// Used when creating a new backup to snapshot the full current state.

quicklendx-contracts/src/dispute.rs

@@ -1,4 +1,5 @@
 use crate::admin::AdminStorage;
+use crate::arbiter::ArbiterStorage;
 use crate::dispute_timeline::{clear_under_review_timestamp, set_under_review_timestamp};
 use crate::errors::QuickLendXError;
 use crate::storage::InvoiceStorage;
@@ -213,6 +214,12 @@ pub fn put_dispute_under_review(
     admin: &Address,
     invoice_id: &BytesN<32>,
 ) -> Result<(), QuickLendXError> {
+    // Per Issue #1840, the `require_dispute_arbiter` guard applies to
+    // *resolve*, not the review transition. Any authenticated admin may move
+    // a dispute into `UnderReview`; only registered arbiters may finalize it.
+    // Keeping review on admin authority matches the intent expressed in the
+    // issue title and avoids breaking every existing test that legitimately
+    // drives a dispute through the review step before resolution.
     AdminStorage::require_admin(env, admin)?;
     let mut invoice =
         InvoiceStorage::get_invoice(env, invoice_id).ok_or(QuickLendXError::InvoiceNotFound)?;
@@ -279,6 +286,11 @@ pub fn resolve_dispute(
     resolution: &String,
 ) -> Result<(), QuickLendXError> {
     AdminStorage::require_admin(env, admin)?;
+    // Arbiter gate: even an admin cannot resolve a dispute unless they have
+    // been explicitly registered as an arbiter. Splits dispute-adjudication
+    // authority from protocol-configuration authority so that a single
+    // compromised admin key cannot silently drain disputed escrow.
+    ArbiterStorage::require_dispute_arbiter(env, admin)?;
 
     validate_dispute_resolution(resolution)?;
 
@@ -344,6 +356,9 @@ pub fn resolve_dispute_structured(
     note: &String,
 ) -> Result<(), QuickLendXError> {
     AdminStorage::require_admin(env, admin)?;
+    // See `resolve_dispute` — the structured variant shares the same arbiter
+    // gate so both resolution paths are defended equally.
+    ArbiterStorage::require_dispute_arbiter(env, admin)?;
 
     validate_dispute_resolution(note)?;
 

quicklendx-contracts/src/errors.rs

@@ -25,8 +25,13 @@ pub enum QuickLendXError {
     InvoiceAlreadyDefaulted = 1006,
     /// BREAKING: Do not renumber this variant. public ABI consumption.
     InvoiceFrozen = 1007,
+    /// Caller is a registered admin but is **not** a registered dispute
+    /// arbiter. Resolving, reviewing, or driving dispute lifecycle actions
+    /// requires explicit arbiter registration on top of admin authority —
+    /// this separates "who can configure the protocol" from "who can
+    /// adjudicate a dispute".
     /// BREAKING: Do not renumber this variant. public ABI consumption.
-    InvalidFreezeReason = 1008,
+    NotArbiter = 1008,
 
     // Authorization (1100-1104)
     /// BREAKING: Do not renumber this variant. public ABI consumption.
@@ -204,7 +209,13 @@ pub enum QuickLendXError {
     MaintenanceModeActive = 2201,
     /// BREAKING: Do not renumber this variant. public ABI consumption.
     DuplicateDefaultTransition = 2202,
-    BackupVersionUnsupported = 2203,
+    /// A destructive contract migration (e.g. `schedule_upgrade`) was
+    /// attempted while an in-progress backfill (e.g. `restore_from_backup`)
+    /// has not yet cleared its pending flag. Letting a migration race a
+    /// backfill leaves the new contract code interpreting partially
+    /// restored state, with no signal either side has to detect it.
+    /// BREAKING: Do not renumber this variant. public ABI consumption.
+    BackfillInProgress = 2203,
     /// BREAKING: Do not renumber this variant. public ABI consumption.
     DuplicateBid = 2204,
     /// Settlement attempted while a dispute is open on the invoice.
@@ -238,11 +249,10 @@ impl From<QuickLendXError> for Symbol {
             QuickLendXError::InvoiceNotFunded => symbol_short!("INV_NFD"),
             QuickLendXError::InvoiceAlreadyDefaulted => symbol_short!("INV_AD"),
             QuickLendXError::InvoiceFrozen => symbol_short!("INV_FRZ"),
-            QuickLendXError::InvalidFreezeReason => symbol_short!("FRZ_RSN"),
+            QuickLendXError::NotArbiter => symbol_short!("NOT_ARB"),
             // Authorization
             QuickLendXError::Unauthorized => symbol_short!("UNAUTH"),
             QuickLendXError::NotBusinessOwner => symbol_short!("NOT_OWN"),
-            QuickLendXError::InvalidFreezeReason => symbol_short!("FRZ_RSN"),
             QuickLendXError::NotInvestor => symbol_short!("NOT_INV"),
             QuickLendXError::NotAdmin => symbol_short!("NOT_ADM"),
             QuickLendXError::SelfCallNotAllowed => symbol_short!("SELF_NA"),
@@ -326,6 +336,9 @@ impl From<QuickLendXError> for Symbol {
             QuickLendXError::TokenTransferFailed => symbol_short!("TKN_FAIL"),
             QuickLendXError::MaintenanceModeActive => symbol_short!("MAINT"),
             QuickLendXError::DuplicateDefaultTransition => symbol_short!("DEF_DUP"),
+            QuickLendXError::BackfillInProgress => symbol_short!("BKF_IP"),
+            QuickLendXError::DuplicateBid => symbol_short!("BID_DUP"),
+            QuickLendXError::InvalidLedgerSequence => symbol_short!("INV_LS"),
             QuickLendXError::InsuranceNotActive => symbol_short!("INS_NACT"),
             QuickLendXError::ActiveDisputeExists => symbol_short!("DSP_ACT"),
             QuickLendXError::StaleInvestmentSnapshot => symbol_short!("STL_INV"),

quicklendx-contracts/src/events.rs

@@ -1737,3 +1737,41 @@ pub fn emit_incident_mode_exited(env: &Env, admin: &Address) {
     }
     .publish(env);
 }
+
+// ── Arbiter (dispute-resolution) events ──────────────────────────────────────
+
+/// Emitted when an admin registers a new dispute arbiter.
+pub fn arbiter_registered(env: &Env, admin: &Address, arbiter: &Address) {
+    env.events().publish(
+        (symbol_short!("arb_reg"),),
+        (admin.clone(), arbiter.clone(), env.ledger().timestamp()),
+    );
+}
+
+/// Emitted when an admin revokes a previously registered dispute arbiter.
+pub fn arbiter_revoked(env: &Env, admin: &Address, arbiter: &Address) {
+    env.events().publish(
+        (symbol_short!("arb_rvk"),),
+        (admin.clone(), arbiter.clone(), env.ledger().timestamp()),
+    );
+}
+
+// ── Backfill lifecycle events ────────────────────────────────────────────────
+
+/// Emitted when a destructive backfill (e.g. `restore_from_backup`) begins.
+/// While this flag is set, the contract refuses to schedule a WASM upgrade.
+pub fn backfill_started(env: &Env, actor: &Address) {
+    env.events().publish(
+        (symbol_short!("bkf_sta"),),
+        (actor.clone(), env.ledger().timestamp()),
+    );
+}
+
+/// Emitted when a backfill finishes — flag is cleared and contracts are free
+/// to migrate again.
+pub fn backfill_finished(env: &Env, actor: &Address, restored_count: u32) {
+    env.events().publish(
+        (symbol_short!("bkf_end"),),
+        (actor.clone(), restored_count, env.ledger().timestamp()),
+    );
+}