diff --git a/bettapay_common/src/constants.rs b/bettapay_common/src/constants.rs index d54ae9f3..6cfb3fbe 100644 --- a/bettapay_common/src/constants.rs +++ b/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; diff --git a/settlement_contract/src/admin.rs b/settlement_contract/src/admin.rs index b2aebcd9..ecfeb6ae 100644 --- a/settlement_contract/src/admin.rs +++ b/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); diff --git a/settlement_contract/src/lib.rs b/settlement_contract/src/lib.rs index 226e7842..357eb97c 100644 --- a/settlement_contract/src/lib.rs +++ b/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`. /// diff --git a/settlement_contract/src/tests/timelock_tests.rs b/settlement_contract/src/tests/timelock_tests.rs index b619dc60..28616cf0 100644 --- a/settlement_contract/src/tests/timelock_tests.rs +++ b/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();