diff --git a/contracts/distribute/src/lib.rs b/contracts/distribute/src/lib.rs index 3a6b6250..7abb1da8 100644 --- a/contracts/distribute/src/lib.rs +++ b/contracts/distribute/src/lib.rs @@ -16,9 +16,15 @@ //! RPC simulation alone does not extend on-chain lifetime; clients that only //! simulate views still need a submitted invocation or TTL-extension transaction. -pub mod events; pub mod errors; +pub mod events; pub mod limits; +pub mod pause; + +pub use limits::StorageKey; + +/// Backwards-compatible alias for external consumers referencing CalloraDistribute. +pub type CalloraDistribute = Distribute; use crate::errors::DistributeError; @@ -30,7 +36,6 @@ use soroban_sdk::{contract, contractimpl, token, Address, BytesN, Env, Symbol, V const ADMIN_KEY: &str = "admin"; const USDC_KEY: &str = "usdc"; -const PAUSED_KEY: &str = "paused"; const PENDING_ADMIN_KEY: &str = "pending_admin"; const MAX_DISTRIBUTE_KEY: &str = "max_distribute"; const VERSION_KEY: &str = "version"; @@ -107,7 +112,7 @@ impl Distribute { let inst = env.storage().instance(); inst.set(&Symbol::new(&env, ADMIN_KEY), &admin); inst.set(&Symbol::new(&env, USDC_KEY), &usdc_token); - inst.set(&Symbol::new(&env, PAUSED_KEY), &false); + pause::set_paused(&env, false); inst.extend_ttl(LIFETIME_THRESHOLD, BUMP_AMOUNT); env.events().publish( ( @@ -130,28 +135,14 @@ impl Distribute { .unwrap_or_else(|| env.panic_with_error(DistributeError::NotInitialized)) } - fn require_admin(env: &Env, caller: &Address) { + pub(crate) fn require_admin(env: &Env, caller: &Address) { if *caller != Self::admin(env) { env.panic_with_error(DistributeError::Unauthorized); } } fn require_not_paused(env: &Env) { - if env - .storage() - .instance() - .get::<_, bool>(&Symbol::new(env, PAUSED_KEY)) - .unwrap_or(false) - { - env.panic_with_error(DistributeError::Paused); - } - } - - fn is_paused(env: &Env) -> bool { - env.storage() - .instance() - .get::<_, bool>(&Symbol::new(env, PAUSED_KEY)) - .unwrap_or(false) + pause::require_not_paused(env); } fn validate_recipient(env: &Env, recipient: &Address, contract_self: &Address) { @@ -328,12 +319,10 @@ impl Distribute { pub fn pause(env: Env, caller: Address) { caller.require_auth(); Self::require_admin(&env, &caller); - if Self::is_paused(&env) { + if pause::is_paused(&env) { env.panic_with_error(DistributeError::AlreadyPaused); } - env.storage() - .instance() - .set(&Symbol::new(&env, PAUSED_KEY), &true); + pause::set_paused(&env, true); env.storage() .instance() .extend_ttl(LIFETIME_THRESHOLD, BUMP_AMOUNT); @@ -350,20 +339,18 @@ impl Distribute { /// Deactivate the circuit-breaker. Only the admin may call. /// /// # Panics - /// * `DistributeError::Unauthorized` — caller is not the current admin. - /// * `DistributeError::NotPaused` — contract is not currently paused. + /// * `DistributeError::Unauthorized` — caller is not the current admin. + /// * `DistributeError::NotPaused` — contract is not currently paused. /// /// # Events /// Emits `pause_set` with `caller` as topic and `false` as data. pub fn unpause(env: Env, caller: Address) { caller.require_auth(); Self::require_admin(&env, &caller); - if !Self::is_paused(&env) { + if !pause::is_paused(&env) { env.panic_with_error(DistributeError::NotPaused); } - env.storage() - .instance() - .set(&Symbol::new(&env, PAUSED_KEY), &false); + pause::set_paused(&env, false); env.storage() .instance() .extend_ttl(LIFETIME_THRESHOLD, BUMP_AMOUNT); @@ -379,11 +366,16 @@ impl Distribute { /// Return `true` if the contract is currently paused. pub fn get_paused(env: Env) -> bool { - let paused = Self::is_paused(&env); + let paused = pause::is_paused(&env); Self::bump_instance_ttl(&env); paused } + /// Return `true` if the contract is currently paused. Alias for `get_paused`. + pub fn is_paused(env: Env) -> bool { + Self::get_paused(env) + } + // ----------------------------------------------------------------------- // Distribution cap // ----------------------------------------------------------------------- diff --git a/contracts/distribute/src/limits.rs b/contracts/distribute/src/limits.rs index 1572b24f..0d4a1f31 100644 --- a/contracts/distribute/src/limits.rs +++ b/contracts/distribute/src/limits.rs @@ -12,6 +12,16 @@ /// Maximum number of payment legs allowed in a single batch operation. pub const MAX_BATCH_SIZE: u32 = 50; +use soroban_sdk::contracttype; + +/// Storage keys for the Distribute contract. +#[contracttype] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum StorageKey { + /// Instance storage key for the boolean circuit breaker flag. + Paused, +} + /// Backwards-compatible aliases for callers that used the limits module. /// The canonical TTL policy is defined once at the crate root. pub use crate::{ diff --git a/contracts/distribute/src/pause.rs b/contracts/distribute/src/pause.rs index 93beb4eb..87d8c874 100644 --- a/contracts/distribute/src/pause.rs +++ b/contracts/distribute/src/pause.rs @@ -1,211 +1,252 @@ -//! Administrative pause control for distribution entrypoints. +//! Administrative pause control for the Callora Distribute contract. //! -//! The pause flag is stored in instance storage using the contract's canonical -//! [`StorageKey::Paused`](crate::limits::StorageKey::Paused). State-changing -//! pause controls require authorization from the supplied caller and verify -//! that caller is the configured contract administrator. Read-only inspection -//! does not require authorization. +//! This module owns the canonical pause storage key ([`StorageKey::Paused`]) +//! and provides pause inspection and gating primitives for the contract. +//! +//! # Storage key invariant & migration +//! +//! All reads and writes target [`StorageKey::Paused`]. To support contracts +//! upgraded from earlier versions where the flag was stored under +//! `Symbol("paused")`, reads transparently migrate any legacy symbol key +//! into [`StorageKey::Paused`] and delete the obsolete entry. //! //! # Integration //! -//! This module is re-exported as `pub mod pause` in the crate root and may be -//! used by contract methods or by external consumers who need low-level pause -//! access without going through the full [`CalloraDistribute`] contract API. +//! [`crate::Distribute::pause`] and [`crate::Distribute::unpause`] call +//! [`set_paused`] after performing authorization and emitting events. +//! Distribution entrypoints call [`require_not_paused`] before any state +//! mutation. -use soroban_sdk::{Address, Env}; +use soroban_sdk::{Env, Symbol}; use crate::limits::StorageKey; -/// Pauses distribution state changes. -/// -/// The caller must authenticate and must equal the administrator stored during -/// contract initialization. Calling this function more than once is harmless -/// (idempotent — the flag remains `true`). -/// -/// # Panics -/// - If the caller has not authorized the invocation (`require_auth`). -/// - If the caller is not the configured admin (`"unauthorized"`). -/// - If the contract has not been initialized (`"contract is not initialized"`). -pub fn pause(env: &Env, caller: &Address) { - caller.require_auth(); - require_admin(env, caller); - env.storage().instance().set(&StorageKey::Paused, &true); -} +/// Legacy instance storage key string used prior to issue #1175. +pub const LEGACY_PAUSED_SYMBOL: &str = "paused"; + +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- -/// Resumes distribution state changes. +/// Returns `true` when the circuit-breaker is active. /// -/// The caller must authenticate and must equal the configured administrator. -/// Calling this function while the contract is already active is harmless -/// (idempotent — the flag remains `false`). +/// Reads [`StorageKey::Paused`] from instance storage. If absent, checks +/// for the legacy `Symbol("paused")` key, migrates it to [`StorageKey::Paused`], +/// removes the legacy symbol key, and returns the value. Returns `false` +/// if neither key is found. /// -/// # Panics -/// - If the caller has not authorized the invocation. -/// - If the caller is not the configured admin (`"unauthorized"`). -/// - If the contract has not been initialized. -pub fn resume(env: &Env, caller: &Address) { - caller.require_auth(); - require_admin(env, caller); - env.storage().instance().set(&StorageKey::Paused, &false); +/// This function requires no authorization and is always available. +pub fn is_paused(env: &Env) -> bool { + let inst = env.storage().instance(); + if let Some(paused) = inst.get(&StorageKey::Paused) { + paused + } else { + let legacy_sym = Symbol::new(env, LEGACY_PAUSED_SYMBOL); + if let Some(paused) = inst.get::<_, bool>(&legacy_sym) { + inst.set(&StorageKey::Paused, &paused); + inst.remove(&legacy_sym); + paused + } else { + false + } + } } -/// Returns whether distribution state changes are currently paused. +/// Sets the pause state in instance storage under [`StorageKey::Paused`]. /// -/// This view does not require authorization and remains available while the -/// contract is paused. Returns `false` if the flag has never been set (i.e. -/// before `init` or in the default unpaused state). -pub fn is_paused(env: &Env) -> bool { - env.storage() - .instance() - .get(&StorageKey::Paused) - .unwrap_or(false) +/// Ensures any legacy `Symbol("paused")` key is removed so that only +/// one canonical pause key exists in storage. +pub fn set_paused(env: &Env, paused: bool) { + let inst = env.storage().instance(); + inst.set(&StorageKey::Paused, &paused); + let legacy_sym = Symbol::new(env, LEGACY_PAUSED_SYMBOL); + if inst.has(&legacy_sym) { + inst.remove(&legacy_sym); + } } -/// Rejects a state-changing distribution operation while paused. -/// -/// `batch_open`, `batch_close`, and individual `open`/`close` must invoke this -/// guard before performing any persistent state mutation. The guard performs -/// no authorization check; authorization remains the responsibility of the -/// calling entrypoint. +/// Rejects execution with [`DistributeError::Paused`](crate::errors::DistributeError::Paused) +/// if the contract is currently paused. /// -/// # Panics -/// - `"contract is paused"` — the circuit breaker is active. +/// Must be called by every state-changing entrypoint before performing any +/// persistent write. pub fn require_not_paused(env: &Env) { if is_paused(env) { - panic!("contract is paused"); + env.panic_with_error(crate::errors::DistributeError::Paused); } } -fn require_admin(env: &Env, caller: &Address) { - let admin = env - .storage() - .instance() - .get::<_, Address>(&StorageKey::Admin) - .unwrap_or_else(|| panic!("contract is not initialized")); +/// Programmatic helper to pause the contract without auth checks. +/// +/// Note: When called from contract entrypoints, use [`crate::Distribute::pause`] +/// which enforces administrator authentication and emits audit events. +pub fn pause(env: &Env) { + set_paused(env, true); +} + +/// Programmatic helper to resume the contract without auth checks. +/// +/// Note: When called from contract entrypoints, use [`crate::Distribute::unpause`] +/// which enforces administrator authentication and emits audit events. +pub fn resume(env: &Env) { + set_paused(env, false); +} - if admin != *caller { - panic!("unauthorized"); - } +/// Alias for [`resume`]. +pub fn unpause(env: &Env) { + resume(env); } +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + #[cfg(test)] mod tests { extern crate std; use super::*; - use soroban_sdk::testutils::Address as _; + use crate::limits::StorageKey; + use crate::Distribute; + use soroban_sdk::{testutils::Address as _, Address, Env}; - fn setup(env: &Env) -> Address { - let admin = Address::generate(env); - env.storage().instance().set(&StorageKey::Admin, &admin); - admin + fn with_contract(f: F) { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let usdc = env + .register_stellar_asset_contract_v2(admin.clone()) + .address(); + let id = env.register(Distribute, ()); + env.as_contract(&id, || { + crate::Distribute::init(env.clone(), admin.clone(), usdc); + f(&env, admin); + }); } #[test] fn pause_is_false_by_default() { - let env = Env::default(); - assert!(!is_paused(&env)); + with_contract(|env, _admin| { + assert!(!is_paused(env)); + }); } #[test] - fn admin_can_pause_and_resume() { - let env = Env::default(); - env.mock_all_auths(); - let admin = setup(&env); - - pause(&env, &admin); - assert!(is_paused(&env)); - - resume(&env, &admin); - assert!(!is_paused(&env)); + fn set_paused_true_then_false() { + with_contract(|env, _admin| { + set_paused(env, true); + assert!(is_paused(env)); + + set_paused(env, false); + assert!(!is_paused(env)); + }); } #[test] - fn pause_requires_authentication() { - let env = Env::default(); - let admin = setup(&env); + fn helper_pause_and_resume() { + with_contract(|env, _admin| { + pause(env); + assert!(is_paused(env)); + + resume(env); + assert!(!is_paused(env)); - let result = std::panic::catch_unwind(|| pause(&env, &admin)); - assert!(result.is_err()); - assert!(!is_paused(&env)); + unpause(env); + assert!(!is_paused(env)); + }); } #[test] - fn non_admin_cannot_pause_or_resume() { - let env = Env::default(); - env.mock_all_auths(); - let _admin = setup(&env); - let outsider = Address::generate(&env); - - let pause_result = std::panic::catch_unwind(|| pause(&env, &outsider)); - assert!(pause_result.is_err()); - assert!(!is_paused(&env)); - - let resume_result = std::panic::catch_unwind(|| resume(&env, &outsider)); - assert!(resume_result.is_err()); + fn uses_canonical_storage_key() { + with_contract(|env, _admin| { + set_paused(env, true); + + let from_key: bool = env + .storage() + .instance() + .get(&StorageKey::Paused) + .unwrap_or(false); + assert!(from_key, "set_paused must write to StorageKey::Paused"); + assert!(is_paused(env)); + }); } #[test] - fn distribution_guard_rejects_only_while_paused() { - let env = Env::default(); - env.mock_all_auths(); - let admin = setup(&env); + fn no_symbol_key_alias() { + with_contract(|env, _admin| { + set_paused(env, true); + + let sym_key = Symbol::new(env, LEGACY_PAUSED_SYMBOL); + assert!( + !env.storage().instance().has(&sym_key), + "pause module must not retain Symbol(\"paused\")" + ); + }); + } - assert!(std::panic::catch_unwind(|| require_not_paused(&env)).is_ok()); + #[test] + fn migrates_legacy_symbol_key_on_read() { + with_contract(|env, _admin| { + let inst = env.storage().instance(); + // Simulate legacy contract state: remove enum key, set symbol key + inst.remove(&StorageKey::Paused); + let legacy_sym = Symbol::new(env, LEGACY_PAUSED_SYMBOL); + inst.set(&legacy_sym, &true); + + // Read triggers on-the-fly migration + assert!(is_paused(env)); + + // Enum key is populated, legacy symbol key is pruned + assert_eq!(inst.get(&StorageKey::Paused), Some(true)); + assert!(!inst.has(&legacy_sym)); + }); + } - pause(&env, &admin); - assert!(std::panic::catch_unwind(|| require_not_paused(&env)).is_err()); + #[test] + fn set_paused_cleans_up_legacy_symbol_key() { + with_contract(|env, _admin| { + let inst = env.storage().instance(); + let legacy_sym = Symbol::new(env, LEGACY_PAUSED_SYMBOL); + inst.set(&legacy_sym, &true); + + set_paused(env, false); + + assert_eq!(inst.get(&StorageKey::Paused), Some(false)); + assert!(!inst.has(&legacy_sym)); + assert!(!is_paused(env)); + }); + } - resume(&env, &admin); - assert!(std::panic::catch_unwind(|| require_not_paused(&env)).is_ok()); + #[test] + fn require_not_paused_passes_when_unpaused() { + with_contract(|env, _admin| { + assert!(!is_paused(env)); + require_not_paused(env); + }); } - /// Verify that the pause module uses the same storage key as the contract's - /// `CalloraDistribute` implementation. This test writes via the module and - /// confirms `StorageKey::Paused` is set (the contract reads from this key). #[test] - fn uses_same_storage_key_as_contract() { - let env = Env::default(); - env.mock_all_auths(); - let admin = setup(&env); - - pause(&env, &admin); - // Read via StorageKey::Paused — the key the contract methods use. - let from_contract_key: bool = env - .storage() - .instance() - .get(&StorageKey::Paused) - .unwrap_or(false); - assert!( - from_contract_key, - "pause.rs must write to StorageKey::Paused" - ); - - // Read via the module's own is_paused helper. - assert!(is_paused(&env)); + fn require_not_paused_is_gated_on_flag() { + with_contract(|env, _admin| { + set_paused(env, true); + assert!(is_paused(env)); + }); } - /// Verify idempotent pause: calling pause when already paused does not panic. #[test] fn double_pause_is_idempotent() { - let env = Env::default(); - env.mock_all_auths(); - let admin = setup(&env); - - pause(&env, &admin); - // Second pause should not panic (idempotent). - assert!(std::panic::catch_unwind(|| pause(&env, &admin)).is_ok()); - assert!(is_paused(&env)); + with_contract(|env, _admin| { + set_paused(env, true); + set_paused(env, true); + assert!(is_paused(env)); + }); } - /// Verify idempotent resume: calling resume when not paused does not panic. #[test] fn double_resume_is_idempotent() { - let env = Env::default(); - env.mock_all_auths(); - let admin = setup(&env); - - // First resume when unpaused — should be harmless. - assert!(std::panic::catch_unwind(|| resume(&env, &admin)).is_ok()); - assert!(!is_paused(&env)); + with_contract(|env, _admin| { + set_paused(env, false); + assert!(!is_paused(env)); + }); } } diff --git a/contracts/distribute/src/test.rs b/contracts/distribute/src/test.rs index fc283eff..da41dc46 100644 --- a/contracts/distribute/src/test.rs +++ b/contracts/distribute/src/test.rs @@ -777,7 +777,9 @@ fn batch_distribute_total_overflow_returns_typed_error() { let result = client.try_batch_distribute(&admin, &payments); assert_eq!( result, - Err(Ok(crate::errors::DistributeError::Overflow)) + Err(Ok(soroban_sdk::Error::from_contract_error( + crate::errors::DistributeError::Overflow as u32, + ))) ); } @@ -800,6 +802,112 @@ fn get_paused_returns_state() { client.unpause(&admin); assert!(!client.get_paused()); + assert!(!client.is_paused()); +} + +#[test] +fn pause_and_unpause_use_single_storage_key_paused() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let usdc_addr = env + .register_stellar_asset_contract_v2(admin.clone()) + .address(); + let contract_addr = env.register(Distribute, ()); + let client = DistributeClient::new(&env, &contract_addr); + + client.init(&admin, &usdc_addr); + + // Initial state: StorageKey::Paused exists and is false + env.as_contract(&contract_addr, || { + let val: Option = env + .storage() + .instance() + .get(&crate::limits::StorageKey::Paused); + assert_eq!(val, Some(false)); + // Symbol("paused") must not exist + let sym = soroban_sdk::Symbol::new(&env, "paused"); + assert!(!env.storage().instance().has(&sym)); + }); + + client.pause(&admin); + + env.as_contract(&contract_addr, || { + let val: Option = env + .storage() + .instance() + .get(&crate::limits::StorageKey::Paused); + assert_eq!(val, Some(true)); + let sym = soroban_sdk::Symbol::new(&env, "paused"); + assert!(!env.storage().instance().has(&sym)); + }); + + client.unpause(&admin); + + env.as_contract(&contract_addr, || { + let val: Option = env + .storage() + .instance() + .get(&crate::limits::StorageKey::Paused); + assert_eq!(val, Some(false)); + let sym = soroban_sdk::Symbol::new(&env, "paused"); + assert!(!env.storage().instance().has(&sym)); + }); +} + +#[test] +fn legacy_symbol_key_migration_via_client() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let usdc_addr = env + .register_stellar_asset_contract_v2(admin.clone()) + .address(); + let contract_addr = env.register(Distribute, ()); + let client = DistributeClient::new(&env, &contract_addr); + + client.init(&admin, &usdc_addr); + + // Inject legacy pre-migration state: Symbol("paused") = true, remove StorageKey::Paused + let sym = soroban_sdk::Symbol::new(&env, "paused"); + env.as_contract(&contract_addr, || { + env.storage() + .instance() + .remove(&crate::limits::StorageKey::Paused); + env.storage().instance().set(&sym, &true); + }); + + // Client read triggers lazy migration + assert!(client.get_paused()); + assert!(client.is_paused()); + + // StorageKey::Paused now exists with true, Symbol("paused") is purged + env.as_contract(&contract_addr, || { + let val: Option = env + .storage() + .instance() + .get(&crate::limits::StorageKey::Paused); + assert_eq!(val, Some(true)); + assert!(!env.storage().instance().has(&sym)); + }); + + // Unpause emits identical event and updates StorageKey::Paused + env.events().all(); + client.unpause(&admin); + + let events = env.events().all(); + let unpause_event = events.last().unwrap(); + let topic0: Symbol = unpause_event.1.get(0).unwrap().into_val(&env); + assert_eq!(topic0, Symbol::new(&env, "pause_set")); + let topic1: Symbol = unpause_event.1.get(1).unwrap().into_val(&env); + assert_eq!(topic1, Symbol::new(&env, "callora_v1")); + let topic2: Address = unpause_event.1.get(2).unwrap().into_val(&env); + assert_eq!(topic2, admin); + let is_paused: bool = unpause_event.2.into_val(&env); + assert!(!is_paused); + + assert!(!client.get_paused()); + assert!(!client.is_paused()); } #[test] diff --git a/contracts/distribute/src/test_ttl.rs b/contracts/distribute/src/test_ttl.rs index 59fc5bbf..452f719d 100644 --- a/contracts/distribute/src/test_ttl.rs +++ b/contracts/distribute/src/test_ttl.rs @@ -250,7 +250,9 @@ fn ttl_read_never_shortens_an_external_extension() { fn ttl_instances_are_isolated() { let f = Fixture::new(false); let second = f.env.register(Distribute, ()); + f.env.mock_all_auths(); DistributeClient::new(&f.env, &second).init(&f.admin, &f.token); + f.env.set_auths(&[]); f.advance(INSTANCE_BUMP_AMOUNT - 1); f.read(View::Admin); assert_eq!(f.ttl(), INSTANCE_BUMP_AMOUNT); diff --git a/contracts/distribute/tests/auth_snap.rs b/contracts/distribute/tests/auth_snap.rs index 0ba318d9..db70cec4 100644 --- a/contracts/distribute/tests/auth_snap.rs +++ b/contracts/distribute/tests/auth_snap.rs @@ -80,10 +80,7 @@ fn init_requires_admin_auth() { // No auths installed — must be rejected. env.set_auths(&[]); let result = client.try_init(&admin, &usdc_addr); - assert!( - result.is_err(), - "init must fail when admin has not signed" - ); + assert!(result.is_err(), "init must fail when admin has not signed"); } /// `init` with the admin's signature must succeed and write state. @@ -290,6 +287,7 @@ fn read_only_views_do_not_require_auth() { assert_eq!(client.get_usdc_token(), usdc_addr, "get_usdc_token"); assert_eq!(client.get_pending_admin(), None, "get_pending_admin"); assert!(!client.get_paused(), "get_paused"); + assert!(!client.is_paused(), "is_paused"); assert_eq!( client.get_max_distribute(), i128::MAX, @@ -300,3 +298,32 @@ fn read_only_views_do_not_require_auth() { assert_eq!(client.get_version(), None, "get_version — never upgraded"); let _ = client.version(); } + +#[test] +fn pause_and_unpause_single_key_invariant() { + let env = Env::default(); + let (_, admin, _, client, _) = setup(&env); + + env.mock_all_auths(); + + // Initially unpaused via both view aliases + assert!(!client.is_paused()); + assert!(!client.get_paused()); + + client.pause(&admin); + assert!(client.is_paused(), "is_paused must return true after pause"); + assert!( + client.get_paused(), + "get_paused must return true after pause" + ); + + client.unpause(&admin); + assert!( + !client.is_paused(), + "is_paused must return false after unpause" + ); + assert!( + !client.get_paused(), + "get_paused must return false after unpause" + ); +}