fix: let recovery veto scheduled admin operations

bettapay_common/src/constants.rs

@@ -50,6 +50,8 @@ pub const TTL_THRESHOLD_LEDGERS: u32 = LEDGERS_PER_DAY * 14;
 pub const TTL_BUMP_LEDGERS: u32 = LEDGERS_PER_DAY * 30;
 
 /// Cooldown between `initiate_recovery` and `execute_recovery`: seven days,
-/// expressed in seconds. Both contracts use the same delay window so they can
-/// share a single definition.
+/// expressed in seconds. Scheduled settlement administrative operations use a
+/// delay of at least this long. This ordering is part of the threat model:
+/// recovery must be able to veto compromised-admin upgrades and admin
+/// transfers before they execute.
 pub const RECOVERY_DELAY_SECONDS: u64 = 7 * 24 * 60 * 60;

settlement_contract/src/admin.rs

@@ -101,6 +101,11 @@ impl SettlementContract {
         );
     }
 
+    /// Initiates recovery and vetoes every pending scheduled operation.
+    ///
+    /// Recovery is intentionally the emergency veto path: once the recovery
+    /// address authenticates, no operation scheduled under the compromised
+    /// admin can execute, including an upgrade or admin transfer.
     pub fn initiate_recovery(env: Env, new_admin: Address) {
         let recovery_address = read_recovery_address(&env);
         recovery_address.require_auth();
@@ -118,6 +123,7 @@ impl SettlementContract {
         env.storage()
             .instance()
             .set(&CommonDataKey::PendingRecovery, &pending);
+        // `PendingRecovery` itself is the veto marker checked by `execute`.
         events::emit_recovery_initiated(&env, &recovery_address, &new_admin, pending.execute_after);
     }
 
@@ -342,6 +348,13 @@ impl SettlementContract {
     /// * [`OperationNotScheduled`](SettlementError::OperationNotScheduled) — if the operation was not scheduled.
     /// * [`ExecutionNotReady`](SettlementError::ExecutionNotReady) — if the timelock delay has not elapsed.
     pub fn execute(env: Env, operation: Operation) {
+        if env.storage().instance().has(&CommonDataKey::PendingRecovery) {
+            // A pending recovery is an emergency veto over all scheduled ops.
+            // The recovery record remains until recovery execution/cancellation,
+            // so this also closes the race between the two transactions.
+            panic_with_error!(&env, SettlementError::ExecutionNotReady);
+        }
+        let op_hash: BytesN<32> = env.crypto().sha256(&operation.clone().to_xdr(&env)).into();
         assert_not_paused(&env);
 
         let operation_xdr = operation.clone().to_xdr(&env);

settlement_contract/src/lib.rs

@@ -222,7 +222,13 @@ pub(crate) const RULE_TTL_BUMP: u32 = LEDGERS_PER_DAY * 30;
 pub(crate) const MERCHANT_TTL_THRESHOLD: u32 = LEDGERS_PER_DAY * 14;
 pub(crate) const MERCHANT_TTL_BUMP: u32 = LEDGERS_PER_DAY * 30;
 
-pub(crate) const DEFAULT_TIMELOCK_DELAY_SECONDS: u64 = 2 * 24 * 60 * 60; // 48 hours
+/// Minimum delay for scheduled administrative operations.
+///
+/// This matches the seven-day recovery window so the recovery address has
+/// time to replace compromised admins before a scheduled upgrade or admin
+/// transfer can execute. The recovery path is the veto authority for pending
+/// schedules; ordinary admin cancellation remains available as well.
+pub(crate) const DEFAULT_TIMELOCK_DELAY_SECONDS: u64 = 7 * 24 * 60 * 60;
 
 /// The single interface version advertised by `supports_interface`.
 ///

settlement_contract/src/tests/timelock_tests.rs

@@ -1,5 +1,7 @@
 //! Regression coverage for the settlement administrative timelock.
 
+use crate::{Operation, DEFAULT_TIMELOCK_DELAY_SECONDS};
+use bettapay_common::constants::RECOVERY_DELAY_SECONDS;
 use crate::{Operation, SettlementContractClient, DEFAULT_TIMELOCK_DELAY_SECONDS};
 use soroban_sdk::testutils::{Address as _, Ledger};
 use soroban_sdk::{Address, Env};
@@ -26,6 +28,32 @@ fn scheduled_operation_executes_only_after_delay() {
     assert!(client.try_execute(&operation).is_err());
 }
 
+#[test]
+fn recovery_vetoes_scheduled_operation_before_timelock_expiry() {
+    let (env, client, admins, recovery) = setup();
+    let operation = Operation::TransferAdmin(
+        soroban_sdk::vec![&env, Address::generate(&env)],
+        1,
+    );
+    let admin = admins.get(0).unwrap();
+
+    client.schedule(&admin, &operation, &DEFAULT_TIMELOCK_DELAY_SECONDS);
+    env.ledger().with_mut(|ledger| {
+        ledger.timestamp += RECOVERY_DELAY_SECONDS;
+    });
+    client.initiate_recovery(Address::generate(&env));
+
+    // Recovery begins at the same boundary as the timelock and must win the
+    // transaction race: a scheduled operation cannot execute while recovery
+    // is pending, even when its nominal delay has elapsed.
+    env.ledger().with_mut(|ledger| {
+        ledger.timestamp += DEFAULT_TIMELOCK_DELAY_SECONDS;
+    });
+    assert!(client.try_execute(&operation).is_err());
+    assert_eq!(client.get_admin(), admins);
+    assert_eq!(client.get_recovery_address(), recovery);
+}
+
 #[test]
 fn schedule_rejects_non_admin_and_insufficient_delay() {
     let (env, client, admins, merchant) = setup();