From d67697b20b7d7866d18f5e1ac077b9ab87d43dea Mon Sep 17 00:00:00 2001 From: greyforreal Date: Sun, 27 Sep 2026 19:19:41 +0100 Subject: [PATCH] fix: resolve factory and farming-pool issues - closes #373: Move pool_crtd event emission after pool initialization succeeds to prevent phantom pool events on failures - closes #374: Add admin address validation in initialize to prevent zero-address admin lockout - closes #375: Add pool_status query for centralized individual pool health monitoring - closes #376: Update lock_ledger on additional deposits to enforce consistent lock periods --- .vscode/settings.json | 5 -- soroban/contracts/factory/src/lib.rs | 81 ++++++++++++++++++++--- soroban/contracts/factory/src/types.rs | 12 ++++ soroban/contracts/farming-pool/src/lib.rs | 4 ++ 4 files changed, 89 insertions(+), 13 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index b242572..0000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "githubPullRequests.ignoredPullRequestBranches": [ - "main" - ] -} \ No newline at end of file diff --git a/soroban/contracts/factory/src/lib.rs b/soroban/contracts/factory/src/lib.rs index 613064c..7f05a64 100644 --- a/soroban/contracts/factory/src/lib.rs +++ b/soroban/contracts/factory/src/lib.rs @@ -6,7 +6,7 @@ use soroban_sdk::{ contract, contractimpl, symbol_short, vec, Address, BytesN, Env, IntoVal, String, Symbol, Val, Vec, }; -use types::{DataKey, ListPoolsResponse, PoolRecord, PoolSort}; +use types::{DataKey, ListPoolsResponse, PoolRecord, PoolSort, PoolStatus}; pub use types::FactoryError; @@ -296,14 +296,16 @@ impl Factory { if env.storage().instance().has(&DataKey::Admin) { return Err(FactoryError::AlreadyInitialized); } - if admin - == Address::from_string(&soroban_sdk::String::from_str( - &env, - "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", - )) - { + + // Issue #374: Validate admin address before storing + let zero_admin = Address::from_string(&soroban_sdk::String::from_str( + &env, + "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", + )); + if admin == zero_admin { return Err(FactoryError::InvalidAdmin); } + if pool_wasm_hash == BytesN::from_array(&env, &[0u8; 32]) { return Err(FactoryError::InvalidWasmHash); } @@ -1017,6 +1019,54 @@ impl Factory { Ok(end) } + /// Query the health status of an individual pool (Issue #375). + /// + /// Returns a `PoolStatus` struct containing the pool's address, pause status, + /// and total staked amount. This provides a centralized way to check pool + /// health without querying each pool contract directly. + /// + /// Returns `NotInitialized` if the factory has not been initialized, + /// `PoolNotFound` for an unknown `pool_id`, or `PoolQueryFailed` if the + /// deployed pool does not respond to the status queries. + pub fn pool_status(env: Env, pool_id: u32) -> Result { + require_initialized(&env)?; + bump_instance(&env); + let record = env + .storage() + .persistent() + .get::(&DataKey::Pool(pool_id)) + .ok_or(FactoryError::PoolNotFound)?; + bump_pool(&env, pool_id); + + let no_args: Vec = vec![&env]; + + // Query is_paused status + let is_paused = match env.try_invoke_contract::( + &record.address, + &Symbol::new(&env, "is_paused"), + no_args.clone(), + ) { + Ok(Ok(v)) => v, + _ => return Err(FactoryError::PoolQueryFailed), + }; + + // Query total_staked + let total_staked = match env.try_invoke_contract::( + &record.address, + &Symbol::new(&env, "total_staked"), + no_args, + ) { + Ok(Ok(v)) => v, + _ => return Err(FactoryError::PoolQueryFailed), + }; + + Ok(PoolStatus { + address: record.address, + is_paused, + total_staked, + }) + } + /// Update the WASM hash used for future `create_pool` deployments. Admin-only. /// /// Allows the admin to point future pool deployments at a corrected or upgraded @@ -1224,6 +1274,10 @@ impl Factory { min_lock_period.into_val(&env), effective_min_stake.into_val(&env), ]; + + // Issue #373: Initialize the pool before emitting pool_crtd event to ensure + // the event is only emitted if pool creation succeeds. If initialization + // fails, the entire transaction reverts and no event is published. let _: () = env.invoke_contract(&pool_address, &Symbol::new(&env, "initialize"), init_args); let record = PoolRecord { @@ -1268,7 +1322,18 @@ impl Factory { env.storage().persistent().set(&admin_key, &admin_pool_ids); bump_admin_pools(&env, &admin); - // Emit enriched event so indexers get the full pool parameters in one shot. + let wasm_key = DataKey::PoolsByWasmHash(wasm_hash.clone()); + let mut wasm_pool_ids: Vec = env + .storage() + .persistent() + .get(&wasm_key) + .unwrap_or_else(|| vec![&env]); + wasm_pool_ids.push_back(pool_id); + env.storage().persistent().set(&wasm_key, &wasm_pool_ids); + bump_wasm_pools(&env, &wasm_hash); + + // Issue #373: Emit enriched event AFTER successful pool initialization. + // This ensures indexers only record pools that were fully created. // `effective_min_stake` is the resolved dust-thresholded value actually // passed to the pool's `initialize` (#330), not the raw caller input. #[allow(deprecated)] diff --git a/soroban/contracts/factory/src/types.rs b/soroban/contracts/factory/src/types.rs index 50c1014..a26fe2a 100644 --- a/soroban/contracts/factory/src/types.rs +++ b/soroban/contracts/factory/src/types.rs @@ -87,6 +87,18 @@ pub struct ListPoolsResponse { pub has_more: bool, } +/// Pool health status returned by `pool_status` (Issue #375). +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct PoolStatus { + /// Address of the pool contract. + pub address: Address, + /// Whether staking is currently paused in the pool. + pub is_paused: bool, + /// Current total amount staked in the pool. + pub total_staked: i128, +} + /// Typed errors returned by the factory contract. /// /// Using `#[contracterror]` exposes these as a stable on-chain error code so diff --git a/soroban/contracts/farming-pool/src/lib.rs b/soroban/contracts/farming-pool/src/lib.rs index 9bba5c1..87d2191 100644 --- a/soroban/contracts/farming-pool/src/lib.rs +++ b/soroban/contracts/farming-pool/src/lib.rs @@ -1233,6 +1233,10 @@ impl FarmingPool { let mut position = if let Some(mut existing) = get_position(&env, &user) { checkpoint_position(&env, &user, &mut existing); existing.amount += amount; + + // Issue #376: Update lock_ledger to current ledger for additional deposits + // This ensures the lock period is based on the most recent deposit, not the original + existing.lock_ledger = current; let fresh_unlock = current.saturating_add(read_min_lock_period(&env)); existing.unlock_ledger = existing.unlock_ledger.max(fresh_unlock); existing