From ddf46462047803aee9da7701c08a2bcecc5652d6 Mon Sep 17 00:00:00 2001 From: afam <319766468+aurorabini@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:27:25 +0200 Subject: [PATCH 1/4] feat(factory): report refreshed pool IDs from refresh_pool_ttls (#393) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sweep already guarded each bump with a storage existence check, so non-existent IDs were skipped — but the entry point returned Ok(()), and callers had no way to learn which IDs were actually kept alive versus which were skipped. A keeper reconciling a sweep against a known registry had to guess. - New RefreshPoolTtlsResponse contract type: refreshed (ascending IDs that existed and were bumped), end_id (clamped resume point) and missing (count of IDs skipped for an absent record). - refresh_pool_ttls now returns it. The existence check and the ttl_ref event payload are unchanged, so the event contract is stable. - Existing refresh tests updated to the new return value. --- soroban/contracts/factory/src/lib.rs | 24 +++++++++++++++++++++--- soroban/contracts/factory/src/types.rs | 19 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/soroban/contracts/factory/src/lib.rs b/soroban/contracts/factory/src/lib.rs index f7f3750..e35b598 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, RefreshPoolTtlsResponse}; pub use types::FactoryError; pub use types::PoolParams; @@ -930,8 +930,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 @@ -941,9 +950,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)] @@ -951,7 +965,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/types.rs b/soroban/contracts/factory/src/types.rs index 367a7dd..bec2847 100644 --- a/soroban/contracts/factory/src/types.rs +++ b/soroban/contracts/factory/src/types.rs @@ -100,6 +100,25 @@ pub struct ListPoolsResponse { pub has_more: bool, } +/// Result of a `refresh_pool_ttls` sweep (Issue #393). +/// +/// The existence check already guards each `bump_pool` call, so IDs whose +/// record is missing are simply skipped — but the caller previously had no +/// way to learn which IDs were actually refreshed, which made verifying a +/// keep-alive sweep (and reconciling against a known registry) guesswork. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)] +pub struct RefreshPoolTtlsResponse { + /// IDs whose persistent record existed and had its TTL extended, ascending. + pub refreshed: Vec, + /// First ID not covered by the sweep (`start_id + capped_limit`, clamped to + /// the registry count) — the resume point for the next page. + pub end_id: u32, + /// Number of IDs in `start_id..end_id` whose record was missing from + /// storage and therefore skipped. + pub missing: u32, +} + /// Typed errors returned by the factory contract. /// /// Using `#[contracterror]` exposes these as a stable on-chain error code so From 52413c9fa308814cff28a35e484d155864828321 Mon Sep 17 00:00:00 2001 From: afam <319766468+aurorabini@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:27:25 +0200 Subject: [PATCH 2/4] test(factory): cover refresh_pool_ttls with missing pool records (#394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the coverage the TTL logic lacked: - test_refresh_pool_ttls_reports_only_existing_pools_and_counts_gaps: pool IDs are handed out sequentially, so a hole inside the sweep range means a record was removed from storage (TTL lapse / archival) — the same case list_pools reports as pool_gap. The test archives the middle record, ages the survivors, then asserts the sweep returns only the two surviving IDs, reports missing == 1, extends the survivors' TTLs to TTL_EXTEND_TO, and does not materialise the archived record. - test_refresh_pool_ttls_reports_empty_sweep_past_the_registry: a start_id beyond PoolCount sweeps nothing and reports an empty result with end_id clamped to the count, instead of an ambiguous unit return. --- soroban/contracts/factory/src/test.rs | 94 ++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 3 deletions(-) diff --git a/soroban/contracts/factory/src/test.rs b/soroban/contracts/factory/src/test.rs index 9173786..118d43d 100644 --- a/soroban/contracts/factory/src/test.rs +++ b/soroban/contracts/factory/src/test.rs @@ -1321,8 +1321,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); @@ -1336,7 +1343,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, @@ -1356,6 +1365,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(); From bbcb745f6b91366c3fd1e1c499ab8f44ccbd72cd Mon Sep 17 00:00:00 2001 From: afam <319766468+aurorabini@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:27:25 +0200 Subject: [PATCH 3/4] feat(farming-pool): add get_pool_info aggregate query (#395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pool overview needed up to six 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 cost and TTL bump, with no on-chain way to fetch the set together. - New PoolInfo contract type carrying total_staked, total_banked_credits, total_distributed_credits, credit_rate, global_multiplier, min_lock_period, min_stake_amount and is_paused. - get_pool_info() reads instance storage directly, guarded by require_initialized + bump_instance like every other getter. The issue text also names 'number of stakers', which is deliberately not a field: the pool keeps no staker-count entry (it is derived by paging get_positions), so the field would report a value nothing maintains. The two maintained credit counters are included instead so the overview is a single round trip. Tests: the aggregate agrees with every individual getter it replaces, and an uninitialized pool returns NotInitialized rather than a zeroed PoolInfo that would be indistinguishable from an empty pool. --- soroban/contracts/farming-pool/src/lib.rs | 48 ++++++++++++++++++++- soroban/contracts/farming-pool/src/test.rs | 43 ++++++++++++++++++ soroban/contracts/farming-pool/src/types.rs | 34 +++++++++++++++ 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/soroban/contracts/farming-pool/src/lib.rs b/soroban/contracts/farming-pool/src/lib.rs index 4e6b71e..4f1db2c 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, }; @@ -2346,6 +2347,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..24edec9 100644 --- a/soroban/contracts/farming-pool/src/test.rs +++ b/soroban/contracts/farming-pool/src/test.rs @@ -626,6 +626,49 @@ 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_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). From dfd04bad5a9801c1c08e36857143da2be6174c1e Mon Sep 17 00:00:00 2001 From: afam <319766468+aurorabini@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:27:25 +0200 Subject: [PATCH 4/4] test(farming-pool): verify set_credit_rate event carries old and new rate (#396) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The issue asks to verify the event already includes both rates. It does: set_credit_rate publishes (old_rate, new_rate, ledger_sequence). This adds the missing verification, which is a sequence rather than a single change — each event's old_rate must equal the previous event's new_rate, which is precisely the property an indexer needs to reconstruct rate history and compute deltas without reading storage. Pins three consecutive changes (10 -> 25 -> 40 -> 5) as three rate_set events with those payloads, and asserts the final value still matches the public credit_rate getter. --- soroban/contracts/farming-pool/src/test.rs | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/soroban/contracts/farming-pool/src/test.rs b/soroban/contracts/farming-pool/src/test.rs index 24edec9..7b45976 100644 --- a/soroban/contracts/farming-pool/src/test.rs +++ b/soroban/contracts/farming-pool/src/test.rs @@ -669,6 +669,58 @@ fn test_get_pool_info_requires_initialized_pool() { 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);