Feat/389 refund admin rotation

.github/ISSUE_TEMPLATE/feature_request.md

@@ -0,0 +1,29 @@
+---
+name: Feature Request
+about: Propose a new feature or enhancement for the smart contracts
+title: "[Feature]: "
+labels: enhancement
+assignees: ""
+---
+
+## Contract affected
+
+Please specify the contract or module this feature applies to.
+
+Example: `token`, `escrow`, `payment`, `refund`, `staking`, `governance`
+
+## Motivating use case
+
+Describe the problem or use case that motivates this feature. Who needs it, and why?
+
+## Proposed solution
+
+Describe the feature or change you'd like to see. Include relevant function signatures, storage changes, or events if applicable.
+
+## Alternatives considered
+
+Describe any alternative solutions or workarounds you've considered, and why they fall short.
+
+## Additional context
+
+Add any other context, references, or screenshots that may help evaluate this proposal.

contracts/escrow/README.md

@@ -11,6 +11,61 @@ Multi-party escrows can require a minimum approval weight before funds are relea
 - The minimum safe threshold is `1` bps, but in practice you should use a higher value for any escrow that holds meaningful funds. Setting the threshold too low can let a small group of signers release funds with weak consensus, which increases the risk of misuse or compromise.
 - Recommended defaults are `10000` for high-value or sensitive escrows, and `5000` or higher for majority-based approval policies.
 
+## Dispute lifecycle
+
+An escrow moves through dispute states via `dispute_escrow`, `escalate_dispute`, `file_dispute_appeal`, `resolve_appeal`, `resolve_dispute`/`auto_resolve_dispute`, and the timeout paths `trigger_timeout_resolution` / `process_escalation_timeouts`.
+
+```mermaid
+stateDiagram-v2
+    [*] --> Locked: create_escrow_with_multisig / create_conditional_escrow
+    Locked --> Released: release_escrow
+    Locked --> Disputed: dispute_escrow (customer or merchant)
+    Locked --> Cancelled: cancel_escrow
+
+    Disputed --> Resolved: resolve_dispute / auto_resolve_dispute (favors customer)
+    Disputed --> Released: auto_resolve_dispute (favors merchant)
+    Disputed --> Escalated: escalate_dispute (customer or merchant)
+
+    Escalated --> Resolved: trigger_timeout_resolution (favor: Customer)
+    Escalated --> Released: trigger_timeout_resolution (favor: Merchant)
+    Escalated --> Escalated: process_escalation_timeouts (batch sweep, no state change until deadline hit)
+
+    Resolved --> Appealed: file_dispute_appeal (within 72h appeal window, Initial round only)
+    Released --> Appealed: file_dispute_appeal (within 72h appeal window, Initial round only)
+    Appealed --> Resolved: resolve_appeal
+    Appealed --> Released: resolve_appeal
+
+    Released --> [*]
+    Resolved --> [*]
+    Cancelled --> [*]
+```
+
+Notes:
+- Only the escrow's `customer` or `merchant` can open a dispute, escalate it, or file an appeal.
+- Each escrow allows at most one appeal round (`DisputeRound::Initial` → `Appeal` → `Final`); a third round is rejected.
+- The appeal window is fixed at 72 hours (259200 seconds) from `dispute_started_at`; filing after that window returns `InvalidStatus`.
+- Escalation timeouts are configured per-contract via `set_escalation_config` (timeout duration + which party auto-resolution favors) and enforced by `trigger_timeout_resolution` (single escrow) or `process_escalation_timeouts` (batch sweep of the escalation queue).
+
+## Observer role
+
+Observers get time-limited, read-only visibility into a single escrow — useful for auditors, support staff, or dispute mediators who need to inspect an escrow without being a party to it.
+
+- **Granting access** — `add_observer(granter, escrow_id, observer, duration_seconds)` grants `observer` read access to `escrow_id` until `now + duration_seconds`. `granter` must be the escrow's `customer`, its `merchant`, or an address in the admin multisig (`AdminMultiSig`).
+- **Revoking access** — `remove_observer(granter, escrow_id, observer)` removes an observer entry early. Same caller restriction as granting.
+- **Checking access** — `verify_observer_access(escrow_id, observer)` returns `true` only if the address was granted observer status and `expires_at` is still in the future; expired grants are not deleted automatically but read as inactive.
+- **Listing** — `get_observers(escrow_id)` returns all observer entries for an escrow, including expired ones (callers should check `expires_at` themselves).
+
+Access split:
+
+| Function | Observer access |
+|----------|------------------|
+| `get_escrow_details(caller, escrow_id)` | Readable — succeeds for the customer, merchant, or any active (non-expired) observer. |
+| `get_observers`, `verify_observer_access`, `get_dispute_round`, `get_appeal`, `is_escrow_disputed` | Public/unauthenticated reads, not gated by observer status. |
+| `add_observer`, `remove_observer` | Restricted — customer, merchant, or admin only. Observers cannot grant or revoke observer access, including their own. |
+| `dispute_escrow`, `escalate_dispute`, `file_dispute_appeal`, `release_escrow`, `resolve_dispute`, and all fund-moving or state-changing calls | Restricted — observers have no write access; these remain limited to the customer, merchant, arbitrators, or admins as applicable. |
+
+An observer is purely a read-only role: it never satisfies `require_auth` checks for customer/merchant/admin-gated functions, so granting observer access cannot be used to bypass dispute or fund-release authorization.
+
 ## Events
 
 All events are emitted via `#[contractevent]` structs published through the Soroban event system. Each event's **topic** is a `Symbol` matching the struct name. Off-chain consumers (Horizon, indexers) can subscribe to these topics.

contracts/refund/src/lib.rs

@@ -85,6 +85,8 @@ pub enum DataKey {
     CustomerTierPolicy(Address, u32),
     StrictTierPolicy(Address),
     AppealWindowSeconds,
+    // Issue #389: two-step admin rotation
+    PendingAdmin,
 }
 
 #[derive(Clone, Debug, PartialEq)]
@@ -285,6 +287,9 @@ pub enum ExtError {
     // Issue #370: Customer tier policy errors
     TierPolicyNotFound = 57,
     SchemaAlreadyAtTarget = 58,
+    // Issue #389: two-step admin rotation errors
+    NoPendingAdmin = 59,
+    NotPendingAdmin = 60,
 }
 
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -1308,6 +1313,22 @@ pub struct RateLimitUpdated {
     pub effective_at: u64,
 }
 
+/// Event emitted when the current admin proposes a new admin.
+#[contractevent]
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct AdminRotationProposed {
+    pub current_admin: Address,
+    pub pending_admin: Address,
+}
+
+/// Event emitted when a proposed admin accepts the role, completing rotation.
+#[contractevent]
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct AdminRotationAccepted {
+    pub previous_admin: Address,
+    pub new_admin: Address,
+}
+
 #[contract]
 pub struct RefundContract;
 
@@ -1401,6 +1422,86 @@ impl RefundContract {
         Ok(())
     }
 
+    /// Propose a new admin, starting a two-step rotation (Issue #389).
+    ///
+    /// The current admin designates `new_admin` as pending. The rotation only
+    /// completes once `new_admin` calls [`Self::accept_admin`], so a typo'd or
+    /// unreachable address can never brick admin control of the contract.
+    ///
+    /// # Arguments
+    /// * `admin` - The current admin (must be authorized and match stored admin).
+    /// * `new_admin` - The address to propose as the next admin.
+    ///
+    /// # Errors
+    /// Returns `Unauthorized` if the caller is not the current admin.
+    pub fn propose_admin(env: Env, admin: Address, new_admin: Address) -> Result<(), Error> {
+        admin.require_auth();
+        let stored_admin: Address = env
+            .storage()
+            .instance()
+            .get(&DataKey::Admin)
+            .ok_or(Error::Core(CoreError::Unauthorized))?;
+        if admin != stored_admin {
+            return Err(Error::Core(CoreError::Unauthorized));
+        }
+
+        env.storage()
+            .instance()
+            .set(&DataKey::PendingAdmin, &new_admin);
+
+        (AdminRotationProposed {
+            current_admin: admin,
+            pending_admin: new_admin,
+        })
+        .publish(&env);
+
+        Ok(())
+    }
+
+    /// Accept a pending admin rotation, finalizing the transition (Issue #389).
+    ///
+    /// Must be called by the address previously proposed via
+    /// [`Self::propose_admin`]. Replaces `DataKey::Admin` with the caller and
+    /// clears the pending slot.
+    ///
+    /// # Errors
+    /// Returns `NoPendingAdmin` if no rotation has been proposed.
+    /// Returns `NotPendingAdmin` if the caller is not the proposed admin.
+    pub fn accept_admin(env: Env, new_admin: Address) -> Result<(), Error> {
+        new_admin.require_auth();
+
+        let pending: Address = env
+            .storage()
+            .instance()
+            .get(&DataKey::PendingAdmin)
+            .ok_or(Error::Ext(ExtError::NoPendingAdmin))?;
+        if pending != new_admin {
+            return Err(Error::Ext(ExtError::NotPendingAdmin));
+        }
+
+        let previous_admin: Address = env
+            .storage()
+            .instance()
+            .get(&DataKey::Admin)
+            .ok_or(Error::Core(CoreError::Unauthorized))?;
+
+        env.storage().instance().set(&DataKey::Admin, &new_admin);
+        env.storage().instance().remove(&DataKey::PendingAdmin);
+
+        (AdminRotationAccepted {
+            previous_admin,
+            new_admin,
+        })
+        .publish(&env);
+
+        Ok(())
+    }
+
+    /// Get the address currently proposed as the next admin, if any.
+    pub fn get_pending_admin(env: Env) -> Option<Address> {
+        env.storage().instance().get(&DataKey::PendingAdmin)
+    }
+
     /// Request a refund for a payment.
     ///
     /// Creates a new refund request with status `Requested` (or `Approved` if auto-approval
@@ -8472,3 +8573,6 @@ mod schema_version_test;
 
 #[cfg(test)]
 mod test_merchant_override_and_error_codes;
+
+#[cfg(test)]
+mod test_admin_rotation;

contracts/refund/src/test_admin_rotation.rs

@@ -0,0 +1,105 @@
+#![cfg(test)]
+
+// Issue #389: two-step admin rotation for the refund contract.
+
+use super::*;
+use soroban_sdk::testutils::Address as _;
+use soroban_sdk::Env;
+
+#[test]
+fn test_propose_and_accept_admin_rotates_admin() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(RefundContract, ());
+    let client = RefundContractClient::new(&env, &contract_id);
+
+    let admin = Address::generate(&env);
+    let new_admin = Address::generate(&env);
+    client.initialize(&admin);
+
+    assert_eq!(client.get_pending_admin(), None);
+
+    client.propose_admin(&admin, &new_admin);
+    assert_eq!(client.get_pending_admin(), Some(new_admin.clone()));
+
+    client.accept_admin(&new_admin);
+    assert_eq!(client.get_pending_admin(), None);
+
+    // The rotated-in admin can now perform an admin-gated action.
+    let target_version = client.get_schema_version() + 1;
+    client.migrate_schema(&new_admin, &target_version);
+    assert_eq!(client.get_schema_version(), target_version);
+}
+
+#[test]
+fn test_propose_admin_rejects_non_admin_caller() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(RefundContract, ());
+    let client = RefundContractClient::new(&env, &contract_id);
+
+    let admin = Address::generate(&env);
+    let not_admin = Address::generate(&env);
+    let new_admin = Address::generate(&env);
+    client.initialize(&admin);
+
+    let result = client.try_propose_admin(&not_admin, &new_admin);
+    assert_eq!(result, Err(Ok(Error::Core(CoreError::Unauthorized))));
+}
+
+#[test]
+fn test_accept_admin_rejects_wrong_caller() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(RefundContract, ());
+    let client = RefundContractClient::new(&env, &contract_id);
+
+    let admin = Address::generate(&env);
+    let new_admin = Address::generate(&env);
+    let imposter = Address::generate(&env);
+    client.initialize(&admin);
+
+    client.propose_admin(&admin, &new_admin);
+
+    let result = client.try_accept_admin(&imposter);
+    assert_eq!(result, Err(Ok(Error::Ext(ExtError::NotPendingAdmin))));
+
+    // Old admin still retains control since rotation never completed.
+    let target_version = client.get_schema_version() + 1;
+    client.migrate_schema(&admin, &target_version);
+    assert_eq!(client.get_schema_version(), target_version);
+}
+
+#[test]
+fn test_accept_admin_without_proposal_fails() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(RefundContract, ());
+    let client = RefundContractClient::new(&env, &contract_id);
+
+    let admin = Address::generate(&env);
+    let stranger = Address::generate(&env);
+    client.initialize(&admin);
+
+    let result = client.try_accept_admin(&stranger);
+    assert_eq!(result, Err(Ok(Error::Ext(ExtError::NoPendingAdmin))));
+}
+
+#[test]
+fn test_compromised_admin_key_cannot_be_used_after_rotation() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(RefundContract, ());
+    let client = RefundContractClient::new(&env, &contract_id);
+
+    let compromised_admin = Address::generate(&env);
+    let safe_admin = Address::generate(&env);
+    client.initialize(&compromised_admin);
+
+    client.propose_admin(&compromised_admin, &safe_admin);
+    client.accept_admin(&safe_admin);
+
+    // The old (compromised) admin address can no longer perform admin actions.
+    let result = client.try_migrate_schema(&compromised_admin, &2);
+    assert_eq!(result, Err(Ok(Error::Core(CoreError::Unauthorized))));
+}