diff --git a/soroban/contracts/factory/src/lib.rs b/soroban/contracts/factory/src/lib.rs index 9c885ad..4aea733 100644 --- a/soroban/contracts/factory/src/lib.rs +++ b/soroban/contracts/factory/src/lib.rs @@ -936,8 +936,17 @@ impl Factory { /// let their TTL lapse, or archive/white-list them off-chain rather than /// relying on attacker interference. /// + /// Returns a `RefreshPoolTtlsResponse` listing the IDs that were actually + /// refreshed (#393). The existence check was already in place; the sweep + /// previously returned `Ok(())`, so a caller could not tell which IDs had + /// a record and were kept alive versus which were skipped as missing. + /// /// Returns `NotInitialized` if the factory has not been initialized. - pub fn refresh_pool_ttls(env: Env, start_id: u32, limit: u32) -> Result<(), FactoryError> { + pub fn refresh_pool_ttls( + env: Env, + start_id: u32, + limit: u32, + ) -> Result { require_initialized(&env)?; bump_instance(&env); let count: u32 = env @@ -947,9 +956,14 @@ impl Factory { .unwrap_or(0); let capped_limit = limit.min(20); let end = start_id.saturating_add(capped_limit).min(count); + let mut refreshed: Vec = Vec::new(&env); + let mut missing = 0u32; for pool_id in start_id..end { if env.storage().persistent().has(&DataKey::Pool(pool_id)) { bump_pool(&env, pool_id); + refreshed.push_back(pool_id); + } else { + missing += 1; } } #[allow(deprecated)] @@ -957,7 +971,11 @@ impl Factory { (symbol_short!("factory"), symbol_short!("ttl_ref")), (start_id, end), ); - Ok(()) + Ok(RefreshPoolTtlsResponse { + refreshed, + end_id: end, + missing, + }) } /// Transfer admin rights to `new_admin`. Current admin must authorise. diff --git a/soroban/contracts/factory/src/test.rs b/soroban/contracts/factory/src/test.rs index 64c7474..845bc63 100644 --- a/soroban/contracts/factory/src/test.rs +++ b/soroban/contracts/factory/src/test.rs @@ -1325,8 +1325,15 @@ fn test_refresh_pool_ttls_restores_ttl_for_unqueried_pool() { advance_ledgers(&t.env, TTL_EXTEND_TO + 1); assert!(pool_record_ttl(&t.env, &t.factory_addr, id) < TTL_THRESHOLD); - // Call refresh_pool_ttls to restore TTL without a specific get_pool query - assert_eq!(t.client.try_refresh_pool_ttls(&id, &1u32), Ok(Ok(()))); + // Call refresh_pool_ttls to restore TTL without a specific get_pool query. + // #393: the sweep now reports which IDs were actually refreshed. + let refreshed = t.client.try_refresh_pool_ttls(&id, &1u32); + assert!(refreshed.is_ok()); + let sweep = refreshed.unwrap().unwrap(); + assert_eq!(sweep.refreshed.len(), 1); + assert_eq!(sweep.refreshed.get(0), Some(id)); + assert_eq!(sweep.end_id, id + 1); + assert_eq!(sweep.missing, 0); // Verify TTL is restored assert_eq!(pool_record_ttl(&t.env, &t.factory_addr, id), TTL_EXTEND_TO); @@ -1340,7 +1347,9 @@ fn test_refresh_pool_ttls_stays_permissionless_for_any_caller() { let t = setup_with_pool_records(3); let stranger = Address::generate(&t.env); // No admin auth provided at all: assert the refresh still succeeds. - assert_eq!(t.client.try_refresh_pool_ttls(&0u32, &3u32), Ok(Ok(()))); + let sweep = t.client.try_refresh_pool_ttls(&0u32, &3u32).unwrap().unwrap(); + assert_eq!(sweep.refreshed.len(), 3); + assert_eq!(sweep.missing, 0); assert_eq!( pool_record_ttl(&t.env, &t.factory_addr, 1), TTL_EXTEND_TO, @@ -1360,6 +1369,85 @@ fn test_refresh_pool_ttls_requires_initialized_factory() { assert!(matches!(result, Err(Ok(FactoryError::NotInitialized)))); } +#[test] +fn test_refresh_pool_ttls_reports_only_existing_pools_and_counts_gaps() { + // #393 / #394: pool IDs are handed out sequentially from PoolCount, so a + // hole in `start_id..end` means a record was removed from storage (TTL + // expiry / archival) — the same situation list_pools reports as a + // "pool_gap". The sweep must skip it, say so in the response, and leave + // the surviving records bumped. + let t = setup(); + + let p0 = t.client.create_pool( + &Address::generate(&t.env), + &1_728_000u128, + &2u32, + &50u64, + &0i128, + ); + let p1 = t.client.create_pool( + &Address::generate(&t.env), + &1_728_000u128, + &2u32, + &50u64, + &0i128, + ); + let p2 = t.client.create_pool( + &Address::generate(&t.env), + &1_728_000u128, + &2u32, + &50u64, + &0i128, + ); + + // Archive the middle record out from under the registry, as a TTL lapse + // would, leaving the count intact so the ID stays inside the sweep range. + t.env.as_contract(&t.factory_addr, || { + t.env.storage().persistent().remove(&DataKey::Pool(p1)); + }); + + // Age the surviving records past the refresh threshold. + advance_ledgers(&t.env, TTL_EXTEND_TO + 1); + for id in [p0, p2] { + assert!(pool_record_ttl(&t.env, &t.factory_addr, id) < TTL_THRESHOLD); + } + + let sweep = t.client.refresh_pool_ttls(&p0, &20u32); + + assert_eq!(sweep.refreshed.len(), 2); + assert_eq!(sweep.refreshed.get(0), Some(p0)); + assert_eq!(sweep.refreshed.get(1), Some(p2)); + assert_eq!(sweep.missing, 1, "the gap must be reported, not silently skipped"); + assert_eq!(sweep.end_id, p2 + 1); + + for id in [p0, p2] { + assert_eq!( + pool_record_ttl(&t.env, &t.factory_addr, id), + TTL_EXTEND_TO, + "existing pool {id} must have had its TTL extended" + ); + } + t.env.as_contract(&t.factory_addr, || { + assert!( + !t.env.storage().persistent().has(&DataKey::Pool(p1)), + "a TTL sweep must not materialise an archived pool record" + ); + }); +} + +#[test] +fn test_refresh_pool_ttls_reports_empty_sweep_past_the_registry() { + // #393: a start_id at or past the pool count sweeps nothing and says so, + // rather than returning a unit value the caller has to interpret. + let t = setup_with_pool_records(2); + + let sweep = t.client.refresh_pool_ttls(&5u32, &20u32); + + assert_eq!(sweep.refreshed.len(), 0); + assert_eq!(sweep.missing, 0); + assert_eq!(sweep.end_id, 2, "end is clamped to the registry count"); +} + #[test] fn test_create_pool_emits_pool_crtd_event_with_payload() { let t = setup(); diff --git a/soroban/contracts/farming-pool/src/lib.rs b/soroban/contracts/farming-pool/src/lib.rs index acb9bf0..8721831 100644 --- a/soroban/contracts/farming-pool/src/lib.rs +++ b/soroban/contracts/farming-pool/src/lib.rs @@ -8,11 +8,12 @@ mod types; use soroban_sdk::{ contract, contractimpl, symbol_short, token, Address, BytesN, Env, Executable, Vec, }; -pub use types::PoolError; +pub use types::{PoolError, PoolInfo}; use types::{ AdminActionEvent, AdminActionHistoryPage, BankedCreditTotals, BoostConfig, BoostEvent, BoostHistoryPage, CreditRateEvent, CreditRateHistoryPage, DataKey, GlobalMultiplierEvent, - GlobalMultiplierHistoryPage, ListWhitelistedResponse, Position, StakeEvent, StakeHistoryPage, + GlobalMultiplierHistoryPage, ListWhitelistedResponse, Position, PoolInfo, StakeEvent, + StakeHistoryPage, UserStake, WhitelistEvent, WhitelistHistoryPage, }; @@ -2377,6 +2378,49 @@ impl FarmingPool { Self::total_distributed_credits(env) } + /// Aggregate pool overview in a single invocation (Issue #395). + /// + /// A pool dashboard previously needed up to six separate reads + /// (`total_staked`, `credit_rate`, the global multiplier, the min lock + /// period, the min stake amount and the paused flag), each paying its own + /// invocation cost and TTL bump. This returns the same values together, + /// read straight from instance storage. + /// + /// Note on the issue's field list: it mentions "total credits" and + /// "number of stakers". Credits are covered by the two maintained + /// counters below. Staker count is not, because the pool keeps no + /// staker-count entry — it is derived by paging `get_positions`, so + /// inventing a field here would report a number nothing maintains. + /// + /// Returns `NotInitialized` if the pool has not been initialized. + pub fn get_pool_info(env: Env) -> Result { + require_initialized(&env)?; + bump_instance(&env); + + Ok(PoolInfo { + total_staked: env + .storage() + .instance() + .get(&DataKey::TotalStaked) + .unwrap_or(0), + total_banked_credits: read_total_banked_credits(&env), + total_distributed_credits: env + .storage() + .instance() + .get(&DataKey::TotalDistributedCredits) + .unwrap_or(0), + credit_rate: read_credit_rate(&env), + global_multiplier: read_global_multiplier(&env), + min_lock_period: read_min_lock_period(&env), + min_stake_amount: env + .storage() + .instance() + .get(&DataKey::MinStakeAmount) + .unwrap_or(1), + is_paused: pool_is_paused(&env), + }) + } + /// Return the total credits currently banked across all users. pub fn total_banked_credits(env: Env) -> Result { require_initialized(&env)?; diff --git a/soroban/contracts/farming-pool/src/test.rs b/soroban/contracts/farming-pool/src/test.rs index 32c674e..7b45976 100644 --- a/soroban/contracts/farming-pool/src/test.rs +++ b/soroban/contracts/farming-pool/src/test.rs @@ -626,6 +626,101 @@ fn test_admin_sets_global_multiplier() { assert_eq!(cfg.multiplier, 3); } +#[test] +fn test_get_pool_info_aggregates_pool_parameters() { + // #395: a pool overview is one invocation, and its values must agree with + // the individual getters so the aggregate is not a second source of truth. + let t = setup_with_lock_period(3, 42, 12); + + let info = t.client.get_pool_info(); + + assert_eq!(info.credit_rate, t.client.credit_rate().unwrap()); + assert_eq!(info.min_lock_period, t.client.min_lock_period().unwrap()); + assert_eq!( + info.min_stake_amount, + t.client.get_min_stake_amount().unwrap() + ); + assert_eq!(info.total_staked, t.client.total_staked().unwrap()); + assert_eq!( + info.total_distributed_credits, + t.client.total_distributed_credits().unwrap() + ); + assert_eq!( + info.total_banked_credits, + t.client.total_banked_credits().unwrap() + ); + assert_eq!(info.is_paused, t.client.is_paused().unwrap()); + + // Configured-at-initialize values are reflected. + assert_eq!(info.credit_rate, 42); + assert_eq!(info.global_multiplier, 3); + assert_eq!(info.min_lock_period, 12); + assert!(!info.is_paused); +} + +#[test] +fn test_get_pool_info_requires_initialized_pool() { + // #395: like every other getter, the aggregate must not answer for an + // uninitialized pool — a zeroed PoolInfo would be indistinguishable from + // a real pool that happens to hold nothing. + let (_env, client, _user) = setup_uninitialized(); + + let result = client.try_get_pool_info(); + assert!(matches!(result, Err(Ok(PoolError::NotInitialized)))); +} + +#[test] +fn test_set_credit_rate_event_carries_old_and_new_rate_across_changes() { + // #396: an indexer can only track rate history if `rate_set` carries the + // rate the pool is leaving as well as the one it is moving to. The event + // does publish (old_rate, new_rate, ledger), so this test pins that + // contract across a sequence of changes: each event's first payload value + // is the previous event's second, which is what makes deltas computable + // without reading storage. + let t = setup_with_lock_period(2, 10, 12); + + t.client.set_credit_rate(&25i128); + t.client.set_credit_rate(&40i128); + t.client.set_credit_rate(&5i128); + + assert_eq!( + t.env.events().all(), + soroban_sdk::vec![ + &t.env, + ( + t.contract_id.clone(), + soroban_sdk::vec![ + &t.env, + soroban_sdk::symbol_short!("pool").into_val(&t.env), + soroban_sdk::symbol_short!("rate_set").into_val(&t.env) + ], + (10i128, 25i128, 0u32).into_val(&t.env), + ), + ( + t.contract_id.clone(), + soroban_sdk::vec![ + &t.env, + soroban_sdk::symbol_short!("pool").into_val(&t.env), + soroban_sdk::symbol_short!("rate_set").into_val(&t.env) + ], + (25i128, 40i128, 0u32).into_val(&t.env), + ), + ( + t.contract_id.clone(), + soroban_sdk::vec![ + &t.env, + soroban_sdk::symbol_short!("pool").into_val(&t.env), + soroban_sdk::symbol_short!("rate_set").into_val(&t.env) + ], + (40i128, 5i128, 0u32).into_val(&t.env), + ) + ] + ); + + // Final value still matches the public getter. + assert_eq!(t.client.credit_rate().unwrap(), 5i128); +} + #[test] fn test_set_credit_rate_updates_public_getters() { let t = setup_with_lock_period(2, 1, 12); diff --git a/soroban/contracts/farming-pool/src/types.rs b/soroban/contracts/farming-pool/src/types.rs index 0e2cdde..fd065a3 100644 --- a/soroban/contracts/farming-pool/src/types.rs +++ b/soroban/contracts/farming-pool/src/types.rs @@ -193,6 +193,40 @@ pub struct ListWhitelistedResponse { pub total: u32, } +/// Aggregate pool parameters returned by `get_pool_info` (Issue #395). +/// +/// A pool overview previously needed several separate contract calls +/// (`total_staked`, `credit_rate`, the global multiplier, the min lock +/// period, the min stake amount and the paused flag), each paying its own +/// invocation and TTL bump. This bundles the read-only pool configuration +/// into one call for dashboards and analytics. +/// +/// The issue text also names "total credits" and "number of stakers"; those +/// are the maintained `total_distributed_credits` / `total_banked_credits` +/// counters, included here so a full overview is a single round trip. There +/// is no per-staker count kept in storage — staker counts are derived by +/// paging `get_positions`, so no field for it is invented here. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct PoolInfo { + /// Total value staked across all positions, in stroops. + pub total_staked: i128, + /// Credits earned banked by users but not yet withdrawn, in stroops. + pub total_banked_credits: i128, + /// Credits distributed to all users since initialization, in stroops. + pub total_distributed_credits: i128, + /// Current global credit rate. + pub credit_rate: i128, + /// Current global reward multiplier. + pub global_multiplier: u32, + /// Minimum lock period in ledgers. + pub min_lock_period: u32, + /// Minimum amount accepted by `stake`, in stroops. + pub min_stake_amount: i128, + /// Whether the pool is currently paused. + pub is_paused: bool, +} + // ─── History / audit trail types ───────────────────────────────────────────── /// A single whitelist change event (add or remove).