From 2fc6ca49588ab1ba20cb01ff98225d65b421fa19 Mon Sep 17 00:00:00 2001 From: sunnykid-02 <01sunnykid@gmail.com> Date: Wed, 30 Sep 2026 08:55:26 +0100 Subject: [PATCH 1/2] Paginate listing of registered recipients --- contracts/recipient/src/lib.rs | 137 +++++++++++++++++++++++-- contracts/recipient/src/test.rs | 174 +++++++++++++++++++++++++++++++- 2 files changed, 304 insertions(+), 7 deletions(-) diff --git a/contracts/recipient/src/lib.rs b/contracts/recipient/src/lib.rs index 91c354ad..71a03385 100644 --- a/contracts/recipient/src/lib.rs +++ b/contracts/recipient/src/lib.rs @@ -29,11 +29,14 @@ pub mod events; pub use errors::RecipientError; -use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, String}; +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, String, Vec}; /// Maximum byte length of a recipient name. pub const MAX_NAME_LEN: u32 = 64; +/// Maximum number of entries returned by a single [`CalloraRecipient::list_recipients`] call. +pub const MAX_PAGE_SIZE: u32 = 50; + // --------------------------------------------------------------------------- // Storage keys // --------------------------------------------------------------------------- @@ -44,10 +47,18 @@ pub const MAX_NAME_LEN: u32 = 64; pub enum StorageKey { /// Instance: the current admin [`Address`]. Admin, - /// Instance: total number of registered recipients. + /// Instance: total number of registered recipients (also the logical + /// length of the name index). RecipientCount, /// Persistent: a registered recipient entry, keyed by name. Recipient(String), + /// Persistent: position-keyed name index entry. Forms a dense array + /// `0 .. RecipientCount` that enables enumeration via + /// [`CalloraRecipient::list_recipients`]. + NameIndex(u32), + /// Persistent: reverse lookup — maps a recipient name to its current + /// position in the [`StorageKey::NameIndex`] array. + NamePosition(String), } // --------------------------------------------------------------------------- @@ -183,6 +194,16 @@ impl CalloraRecipient { .instance() .get(&StorageKey::RecipientCount) .ok_or(RecipientError::NotInitialized)?; + + // Append name to the position index. + let new_pos = count; + env.storage() + .persistent() + .set(&StorageKey::NameIndex(new_pos), &name); + env.storage() + .persistent() + .set(&StorageKey::NamePosition(name.clone()), &new_pos); + env.storage().instance().set( &StorageKey::RecipientCount, &count.checked_add(1).ok_or(RecipientError::Overflow)?, @@ -242,6 +263,11 @@ impl CalloraRecipient { /// The recipient must exist. The caller must be the admin and must /// authorize. The recipient count is decremented with overflow-safe math. /// + /// The name index is maintained with an O(1) swap-remove: the removed + /// entry's slot is filled with the last entry in the index, which is then + /// truncated. Enumeration order is therefore not guaranteed to be stable + /// across removals. + /// /// # Parameters /// * `caller` — Must be the current admin; must authorize. /// * `name` — Name of the recipient to remove. @@ -271,10 +297,46 @@ impl CalloraRecipient { .instance() .get(&StorageKey::RecipientCount) .ok_or(RecipientError::NotInitialized)?; - env.storage().instance().set( - &StorageKey::RecipientCount, - &count.checked_sub(1).ok_or(RecipientError::Overflow)?, - ); + let new_count = count.checked_sub(1).ok_or(RecipientError::Overflow)?; + + // Swap-remove from the name index: + // 1. Look up the slot of the name being removed. + // 2. If it is not the last slot, move the last name into that slot + // and update its reverse-lookup entry. + // 3. Remove the (now vacated) last slot and both reverse entries. + let removed_pos: u32 = env + .storage() + .persistent() + .get(&StorageKey::NamePosition(name.clone())) + .ok_or(RecipientError::NotFound)?; + + if removed_pos < new_count { + // The name at the tail must fill the vacated slot. + let tail_name: String = env + .storage() + .persistent() + .get(&StorageKey::NameIndex(new_count)) + .ok_or(RecipientError::NotFound)?; + + env.storage() + .persistent() + .set(&StorageKey::NameIndex(removed_pos), &tail_name); + env.storage() + .persistent() + .set(&StorageKey::NamePosition(tail_name), &removed_pos); + } + + // Truncate the tail slot and clean up the removed name's entries. + env.storage() + .persistent() + .remove(&StorageKey::NameIndex(new_count)); + env.storage() + .persistent() + .remove(&StorageKey::NamePosition(name.clone())); + + env.storage() + .instance() + .set(&StorageKey::RecipientCount, &new_count); env.events() .publish((events::event_recipient_removed(&env), name), ()); @@ -334,6 +396,69 @@ impl CalloraRecipient { .get(&StorageKey::RecipientCount) .ok_or(RecipientError::NotInitialized)?) } + + /// Return a page of registered recipient names. + /// + /// Names are returned in an unspecified but stable order within a single + /// page. Order may change when recipients are removed (swap-remove + /// semantics). Callers must paginate using `start` offsets to walk the + /// full set; combine with [`Self::get_recipient_count`] to determine the + /// total number of pages. + /// + /// # Parameters + /// * `start` — Zero-based index of the first entry to return. + /// * `limit` — Maximum number of entries to return, capped at + /// [`MAX_PAGE_SIZE`]. Passing `0` returns up to [`MAX_PAGE_SIZE`] + /// entries starting at `start`. + /// + /// # Returns + /// A [`Vec`] containing at most `min(limit, MAX_PAGE_SIZE)` names. + /// Returns an empty vec when `start >= get_recipient_count()`. + /// + /// # Errors + /// * [`RecipientError::NotInitialized`] — contract was never initialized. + /// + /// Pure view: no auth, no storage writes. + pub fn list_recipients( + env: Env, + start: u32, + limit: u32, + ) -> Result, RecipientError> { + if !env.storage().instance().has(&StorageKey::Admin) { + return Err(RecipientError::NotInitialized); + } + + let count: u32 = env + .storage() + .instance() + .get(&StorageKey::RecipientCount) + .ok_or(RecipientError::NotInitialized)?; + + // Cap the page size. + let effective_limit = if limit == 0 || limit > MAX_PAGE_SIZE { + MAX_PAGE_SIZE + } else { + limit + }; + + let mut result = Vec::new(&env); + + if start >= count { + return Ok(result); + } + + let end = (start + effective_limit).min(count); + for pos in start..end { + let name: String = env + .storage() + .persistent() + .get(&StorageKey::NameIndex(pos)) + .ok_or(RecipientError::NotFound)?; + result.push_back(name); + } + + Ok(result) + } } // --------------------------------------------------------------------------- diff --git a/contracts/recipient/src/test.rs b/contracts/recipient/src/test.rs index 74fefe15..c7eb64df 100644 --- a/contracts/recipient/src/test.rs +++ b/contracts/recipient/src/test.rs @@ -1,4 +1,4 @@ -use crate::{CalloraRecipient, CalloraRecipientClient, RecipientError}; +use crate::{CalloraRecipient, CalloraRecipientClient, RecipientError, MAX_PAGE_SIZE}; use soroban_sdk::testutils::Address as _; use soroban_sdk::{Address, Env, String as SorobanString}; @@ -219,6 +219,178 @@ fn uninitialized_contract_returns_not_initialized() { assert_eq!(err, Err(Ok(RecipientError::NotInitialized))); } +// --------------------------------------------------------------------------- +// list_recipients tests +// --------------------------------------------------------------------------- + +#[test] +fn list_recipients_empty() { + let (_env, admin, client) = setup(); + client.init(&admin); + + let page = client.list_recipients(&0, &10); + assert!(page.is_empty()); +} + +#[test] +fn list_recipients_single_page() { + let (env, admin, client) = setup(); + client.init(&admin); + + let n_a = name(&env, "alpha"); + let n_b = name(&env, "beta"); + let n_c = name(&env, "gamma"); + + for n in [&n_a, &n_b, &n_c] { + let addr = Address::generate(&env); + client.register_recipient(&admin, n, &addr); + } + + let page = client.list_recipients(&0, &10); + assert_eq!(page.len(), 3); + + // All registered names must appear somewhere in the page. + let contains = |needle: &SorobanString| { + (0..page.len()).any(|i| page.get(i).unwrap() == *needle) + }; + assert!(contains(&n_a), "alpha missing"); + assert!(contains(&n_b), "beta missing"); + assert!(contains(&n_c), "gamma missing"); +} + +#[test] +fn list_recipients_pagination() { + let (env, admin, client) = setup(); + client.init(&admin); + + let names = [ + name(&env, "r0"), + name(&env, "r1"), + name(&env, "r2"), + name(&env, "r3"), + name(&env, "r4"), + ]; + for n in &names { + let addr = Address::generate(&env); + client.register_recipient(&admin, n, &addr); + } + + // Page size 2, walk all pages. + let page0 = client.list_recipients(&0, &2); + let page1 = client.list_recipients(&2, &2); + let page2 = client.list_recipients(&4, &2); + + assert_eq!(page0.len(), 2); + assert_eq!(page1.len(), 2); + assert_eq!(page2.len(), 1); // only one entry left + + // Collect all results and verify no duplicates. + let mut seen: std::vec::Vec = std::vec::Vec::new(); + for page in [&page0, &page1, &page2] { + for i in 0..page.len() { + let entry = page.get(i).unwrap(); + assert!( + !seen.iter().any(|s| s == &entry), + "duplicate name across pages" + ); + seen.push(entry); + } + } + assert_eq!(seen.len(), 5); +} + +#[test] +fn list_recipients_page_size_capped() { + let (env, admin, client) = setup(); + client.init(&admin); + + // Register MAX_PAGE_SIZE + 5 entries. + for i in 0u32..(MAX_PAGE_SIZE + 5) { + // Build a unique name like "n000", "n001", ... + let s = std::format!("n{:03}", i); + let n = name(&env, &s); + let addr = Address::generate(&env); + client.register_recipient(&admin, &n, &addr); + } + + // Even with a huge limit, result must not exceed MAX_PAGE_SIZE. + let page = client.list_recipients(&0, &1000); + assert_eq!(page.len() as u32, MAX_PAGE_SIZE); +} + +#[test] +fn list_recipients_start_beyond_count_returns_empty() { + let (env, admin, client) = setup(); + client.init(&admin); + + let addr = Address::generate(&env); + client.register_recipient(&admin, &name(&env, "only"), &addr); + + let page = client.list_recipients(&99, &10); + assert!(page.is_empty()); +} + +#[test] +fn list_recipients_removed_names_absent() { + let (env, admin, client) = setup(); + client.init(&admin); + + let n_a = name(&env, "aaa"); + let n_b = name(&env, "bbb"); + let n_c = name(&env, "ccc"); + + for n in [&n_a, &n_b, &n_c] { + let addr = Address::generate(&env); + client.register_recipient(&admin, n, &addr); + } + + client.remove_recipient(&admin, &n_b); + + let page = client.list_recipients(&0, &10); + assert_eq!(page.len(), 2); + for i in 0..page.len() { + assert_ne!( + page.get(i).unwrap(), + n_b, + "removed name still appears in listing" + ); + } +} + +#[test] +fn list_recipients_no_auth_required() { + // list_recipients must be callable without any authentication. + let env = Env::default(); + let admin = Address::generate(&env); + let contract_addr = env.register(CalloraRecipient, ()); + let client = CalloraRecipientClient::new(&env, &contract_addr); + + env.mock_all_auths(); + client.init(&admin); + let addr = Address::generate(&env); + client.register_recipient(&admin, &name(&env, "pub"), &addr); + env.set_auths(&[]); + + // Must succeed without any auth. + let page = client.list_recipients(&0, &10); + assert_eq!(page.len(), 1); +} + +#[test] +fn list_recipients_zero_limit_uses_cap() { + let (env, admin, client) = setup(); + client.init(&admin); + + for n in ["x0", "x1", "x2", "x3", "x4"] { + let addr = Address::generate(&env); + client.register_recipient(&admin, &name(&env, n), &addr); + } + + // limit=0 should fall back to MAX_PAGE_SIZE, returning all 5. + let page = client.list_recipients(&0, &0); + assert_eq!(page.len(), 5); +} + // --------------------------------------------------------------------------- // Rustdoc coverage test // --------------------------------------------------------------------------- From 38c54b9c8ad4ce999d5c81a883753011a4c37011 Mon Sep 17 00:00:00 2001 From: sunnykid-02 <01sunnykid@gmail.com> Date: Wed, 30 Sep 2026 08:56:48 +0100 Subject: [PATCH 2/2] quick fix [ci skip]