From 1a5203af93040fc8af9aaed6e1377807e0b7cd60 Mon Sep 17 00:00:00 2001 From: Arowolokehinde Date: Mon, 27 Apr 2026 12:21:01 +0100 Subject: [PATCH] feat(savings_goals): add cursor pagination for owner goal listing Add get_goals_for_owner(owner, cursor, limit) as a public contract method with deterministic cursor-based pagination backed by the owner-scoped goal ID index. Includes 13 comprehensive pagination tests covering limit clamping, full iteration, owner isolation, cross-owner cursor rejection, and edge cases. Also fixes pre-existing build issues: duplicate proptest key in remittance_split/Cargo.toml, String::from_slice type mismatch in tag validation, Val PartialEq errors in event_test.rs, and mismatched create_goal argument in stress_test_large_amounts.rs. --- remittance_split/Cargo.toml | 1 - savings_goals/src/event_test.rs | 60 ++-- savings_goals/src/lib.rs | 33 +- savings_goals/src/test.rs | 321 +++++++++++++++++- .../tests/stress_test_large_amounts.rs | 3 +- 5 files changed, 371 insertions(+), 47 deletions(-) diff --git a/remittance_split/Cargo.toml b/remittance_split/Cargo.toml index e39fdee2..395ca59a 100644 --- a/remittance_split/Cargo.toml +++ b/remittance_split/Cargo.toml @@ -14,4 +14,3 @@ remitwise-common = { path = "../remitwise-common" } proptest = "1.10.0" soroban-sdk = { version = "=21.7.7", features = ["testutils"] } testutils = { path = "../testutils" } -proptest = "1" diff --git a/savings_goals/src/event_test.rs b/savings_goals/src/event_test.rs index 9724110f..f5fb5fd9 100644 --- a/savings_goals/src/event_test.rs +++ b/savings_goals/src/event_test.rs @@ -3,10 +3,10 @@ use super::*; use soroban_sdk::{ testutils::{Address as _, Events, Ledger}, - Address, Env, IntoVal, String, Symbol, TryFromVal, Val, Vec as SorobanVec, + Address, Env, String, Symbol, TryFromVal, Val, Vec as SorobanVec, }; -fn setup_test(env: &Env) -> (SavingsGoalContractClient, Address) { +fn setup_test(env: &Env) -> (SavingsGoalContractClient<'_>, Address) { let contract_id = env.register_contract(None, SavingsGoalContract); let client = SavingsGoalContractClient::new(env, &contract_id); let user = Address::generate(env); @@ -15,10 +15,6 @@ fn setup_test(env: &Env) -> (SavingsGoalContractClient, Address) { (client, user) } -fn last_event(env: &Env) -> (Address, SorobanVec, Val) { - env.events().all().last().expect("No events emitted") -} - fn get_remitwise_events(env: &Env, action: Symbol) -> SorobanVec<(Address, SorobanVec, Val)> { let mut result = SorobanVec::new(env); let events = env.events().all(); @@ -58,19 +54,17 @@ fn test_goal_created_event_schema() { let event = remitwise_events.get(0).unwrap(); // Topic Schema: [Remitwise, State, Medium, created] - assert_eq!( - event.1.get(0).unwrap(), - symbol_short!("Remitwise").into_val(&env) - ); - assert_eq!( - event.1.get(1).unwrap(), - (EventCategory::State as u32).into_val(&env) - ); - assert_eq!( - event.1.get(2).unwrap(), - (EventPriority::Medium as u32).into_val(&env) - ); - assert_eq!(event.1.get(3).unwrap(), GOAL_CREATED.into_val(&env)); + let topic0: Symbol = Symbol::try_from_val(&env, &event.1.get(0).unwrap()).unwrap(); + assert_eq!(topic0, symbol_short!("Remitwise")); + + let topic1: u32 = u32::try_from_val(&env, &event.1.get(1).unwrap()).unwrap(); + assert_eq!(topic1, EventCategory::State as u32); + + let topic2: u32 = u32::try_from_val(&env, &event.1.get(2).unwrap()).unwrap(); + assert_eq!(topic2, EventPriority::Medium as u32); + + let topic3: Symbol = Symbol::try_from_val(&env, &event.1.get(3).unwrap()).unwrap(); + assert_eq!(topic3, GOAL_CREATED); // Payload Schema: GoalCreatedEvent let data: GoalCreatedEvent = GoalCreatedEvent::try_from_val(&env, &event.2).unwrap(); @@ -101,14 +95,11 @@ fn test_funds_added_event_schema() { let event = remitwise_events.get(0).unwrap(); // Topic Schema: [Remitwise, Transaction, Medium, funds_add] - assert_eq!( - event.1.get(1).unwrap(), - (EventCategory::Transaction as u32).into_val(&env) - ); - assert_eq!( - event.1.get(3).unwrap(), - symbol_short!("funds_add").into_val(&env) - ); + let topic1: u32 = u32::try_from_val(&env, &event.1.get(1).unwrap()).unwrap(); + assert_eq!(topic1, EventCategory::Transaction as u32); + + let topic3: Symbol = Symbol::try_from_val(&env, &event.1.get(3).unwrap()).unwrap(); + assert_eq!(topic3, symbol_short!("funds_add")); // Payload Schema: FundsAddedEvent let data: FundsAddedEvent = FundsAddedEvent::try_from_val(&env, &event.2).unwrap(); @@ -145,10 +136,8 @@ fn test_funds_withdrawn_event_schema() { let event = remitwise_events.get(0).unwrap(); // Topic Schema: [Remitwise, Transaction, Medium, funds_rem] - assert_eq!( - event.1.get(3).unwrap(), - symbol_short!("funds_rem").into_val(&env) - ); + let topic3: Symbol = Symbol::try_from_val(&env, &event.1.get(3).unwrap()).unwrap(); + assert_eq!(topic3, symbol_short!("funds_rem")); // Payload Schema: FundsWithdrawnEvent let data: FundsWithdrawnEvent = FundsWithdrawnEvent::try_from_val(&env, &event.2).unwrap(); @@ -174,12 +163,15 @@ fn test_goal_completed_event_schema() { client.add_to_goal(&user, &id, &1000); // GoalCompletedEvent is published with a single topic (GOAL_COMPLETED,) - // It's not using RemitwiseEvents::emit for this specific one in lib.rs currently - // Let's verify what it emits let events = env.events().all(); let completed_event = events .iter() - .find(|e| e.1.len() == 1 && e.1.get(0).unwrap() == GOAL_COMPLETED.into_val(&env)) + .find(|e| { + e.1.len() == 1 && { + let t: Symbol = Symbol::try_from_val(&env, &e.1.get(0).unwrap()).unwrap(); + t == GOAL_COMPLETED + } + }) .expect("GoalCompletedEvent not found"); let data: GoalCompletedEvent = diff --git a/savings_goals/src/lib.rs b/savings_goals/src/lib.rs index acad4c29..3a2b9dbc 100644 --- a/savings_goals/src/lib.rs +++ b/savings_goals/src/lib.rs @@ -464,18 +464,22 @@ impl SavingsGoalContract { } let mut buf = [0u8; 32]; tag.copy_into_slice(&mut buf[..len as usize]); - - for i in 0..(len as usize) { - let mut c = buf[i]; - if c >= b'A' && c <= b'Z' { - c = c + (b'a' - b'A'); - buf[i] = c; + + for c in buf[..len as usize].iter_mut() { + if c.is_ascii_uppercase() { + *c += b'a' - b'A'; } - if !((c >= b'a' && c <= b'z') || (c >= b'0' && c <= b'9') || c == b'-' || c == b'_') { + if !(c.is_ascii_lowercase() + || c.is_ascii_digit() + || *c == b'-' + || *c == b'_') + { soroban_sdk::panic_with_error!(env, SavingsGoalsError::InvalidTagContent); } } - normalized_tags.push_back(String::from_slice(env, &buf[..len as usize])); + let tag_str = core::str::from_utf8(&buf[..len as usize]) + .unwrap_or_else(|_| panic!("Invalid UTF-8 in tag")); + normalized_tags.push_back(String::from_str(env, tag_str)); } normalized_tags } @@ -1243,8 +1247,19 @@ impl SavingsGoalContract { } } + /// Named alias for `get_goals` matching the issue specification. + /// Returns a deterministic page of goals scoped to `owner`. + /// + /// # Arguments + /// * `owner` - whose goals to return + /// * `cursor` - start after this goal ID (pass 0 for the first page) + /// * `limit` - max items per page (0 -> DEFAULT, capped at MAX_PAGE_LIMIT) + pub fn get_goals_for_owner(env: Env, owner: Address, cursor: u32, limit: u32) -> GoalPage { + Self::get_goals(env, owner, cursor, limit) + } + /// Backward-compatible: returns ALL goals for owner in one Vec. - /// Prefer the paginated `get_goals` for production use. + /// Prefer the paginated `get_goals` / `get_goals_for_owner` for production use. pub fn get_all_goals(env: Env, owner: Address) -> Vec { let goals: Map = env .storage() diff --git a/savings_goals/src/test.rs b/savings_goals/src/test.rs index 690f5168..f1040986 100644 --- a/savings_goals/src/test.rs +++ b/savings_goals/src/test.rs @@ -3147,7 +3147,10 @@ fn test_add_tags_to_goal_normalization_success() { client.add_tags_to_goal(&user, &goal_id, &tags); let goal = client.get_goal(&goal_id).unwrap(); - assert_eq!(goal.tags.get(0).unwrap(), String::from_str(&env, "urgent-1_tag")); + assert_eq!( + goal.tags.get(0).unwrap(), + String::from_str(&env, "urgent-1_tag") + ); } #[test] @@ -4087,3 +4090,319 @@ mod migration_e2e_tests { } } } + +// ============================================================================ +// get_goals_for_owner – deterministic cursor pagination tests +// ============================================================================ + +/// get_goals_for_owner is an alias for get_goals – must produce identical results. +#[test] +fn test_get_goals_for_owner_matches_get_goals() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &owner, 7); + + let via_get_goals = client.get_goals(&owner, &0, &3); + let via_for_owner = client.get_goals_for_owner(&owner, &0, &3); + + assert_eq!(via_get_goals.count, via_for_owner.count); + assert_eq!(via_get_goals.next_cursor, via_for_owner.next_cursor); + assert_eq!(via_get_goals.items.len(), via_for_owner.items.len()); + for i in 0..via_get_goals.items.len() { + assert_eq!( + via_get_goals.items.get(i).unwrap().id, + via_for_owner.items.get(i).unwrap().id + ); + } +} + +/// get_goals_for_owner empty owner returns empty page. +#[test] +fn test_get_goals_for_owner_empty() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + let page = client.get_goals_for_owner(&owner, &0, &10); + assert_eq!(page.count, 0); + assert_eq!(page.next_cursor, 0); + assert_eq!(page.items.len(), 0); +} + +/// Limit exceeding MAX_PAGE_LIMIT is clamped to MAX_PAGE_LIMIT. +#[test] +fn test_get_goals_for_owner_limit_clamped_to_max() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + // Create exactly MAX_PAGE_LIMIT + 5 goals + let total = MAX_PAGE_LIMIT + 5; + setup_goals(&env, &client, &owner, total); + + // Request limit=200, should be clamped to MAX_PAGE_LIMIT + let page = client.get_goals_for_owner(&owner, &0, &200); + assert_eq!( + page.count, MAX_PAGE_LIMIT, + "count must be clamped to MAX_PAGE_LIMIT" + ); + assert!(page.next_cursor > 0, "there must be a next page"); + + // Verify the remaining goals are on the second page + let page2 = client.get_goals_for_owner(&owner, &page.next_cursor, &200); + assert_eq!(page2.count, 5); + assert_eq!(page2.next_cursor, 0, "no more pages"); +} + +/// Exact boundary: when goal count equals the limit, next_cursor must be 0. +#[test] +fn test_get_goals_for_owner_exact_page_boundary() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &owner, 4); + + let page = client.get_goals_for_owner(&owner, &0, &4); + assert_eq!(page.count, 4); + assert_eq!(page.next_cursor, 0, "exactly filling the page signals end"); +} + +/// Single goal pagination: page of 1 with 1 goal returns that goal and next_cursor=0. +#[test] +fn test_get_goals_for_owner_single_goal() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + let goal_id = client.create_goal( + &owner, + &String::from_str(&env, "Only"), + &500, + &2_000_000_000, + ); + + let page = client.get_goals_for_owner(&owner, &0, &1); + assert_eq!(page.count, 1); + assert_eq!(page.items.get(0).unwrap().id, goal_id); + assert_eq!(page.next_cursor, 0); +} + +/// next_cursor value equals the last returned item's ID. +#[test] +fn test_get_goals_for_owner_next_cursor_is_last_item_id() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &owner, 10); + + let page = client.get_goals_for_owner(&owner, &0, &3); + let last_item_id = page.items.get(page.count - 1).unwrap().id; + assert_eq!( + page.next_cursor, last_item_id, + "next_cursor must equal the last returned item's ID" + ); +} + +/// Full iteration: paginating through all goals collects every ID exactly once. +#[test] +fn test_get_goals_for_owner_full_iteration_no_gaps_no_dupes() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + let total = 23u32; + setup_goals(&env, &client, &owner, total); + + let mut collected_ids = std::vec::Vec::new(); + let mut cursor = 0u32; + let limit = 5u32; + loop { + let page = client.get_goals_for_owner(&owner, &cursor, &limit); + for goal in page.items.iter() { + collected_ids.push(goal.id); + } + if page.next_cursor == 0 { + break; + } + cursor = page.next_cursor; + } + + assert_eq!( + collected_ids.len(), + total as usize, + "must collect exactly {total} goals" + ); + // IDs must be strictly ascending (deterministic order) + for i in 1..collected_ids.len() { + assert!( + collected_ids[i] > collected_ids[i - 1], + "IDs must be strictly ascending: {} > {}", + collected_ids[i], + collected_ids[i - 1] + ); + } + // No duplicates (ascending guarantees this, but explicit check) + let mut deduped = collected_ids.clone(); + deduped.sort(); + deduped.dedup(); + assert_eq!(deduped.len(), collected_ids.len(), "no duplicates allowed"); +} + +/// Cursor pointing at the last goal returns an empty next page. +#[test] +fn test_get_goals_for_owner_cursor_at_last_returns_empty() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &owner, 3); + + // Get all goals to find the last ID + let all_page = client.get_goals_for_owner(&owner, &0, &50); + let last_id = all_page.items.get(all_page.count - 1).unwrap().id; + + // Use last ID as cursor + let next_page = client.get_goals_for_owner(&owner, &last_id, &10); + assert_eq!(next_page.count, 0); + assert_eq!(next_page.next_cursor, 0); + assert_eq!(next_page.items.len(), 0); +} + +/// Limit=0 defaults to DEFAULT_PAGE_LIMIT via get_goals_for_owner. +#[test] +fn test_get_goals_for_owner_limit_zero_uses_default() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &owner, 3); + + let page = client.get_goals_for_owner(&owner, &0, &0); + assert_eq!(page.count, 3, "3 < DEFAULT_PAGE_LIMIT so all returned"); + assert_eq!(page.next_cursor, 0); +} + +/// Invalid (non-existent) cursor panics via get_goals_for_owner. +#[test] +fn test_get_goals_for_owner_rejects_invalid_cursor() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &owner, 3); + + let res = client.try_get_goals_for_owner(&owner, &999, &10); + assert!(res.is_err(), "invalid cursor must be rejected"); +} + +/// Owner isolation: one owner's cursor is invalid for another owner. +#[test] +fn test_get_goals_for_owner_cross_owner_cursor_rejected() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let alice = Address::generate(&env); + let bob = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &alice, 5); + setup_goals(&env, &client, &bob, 3); + + let alice_page = client.get_goals_for_owner(&alice, &0, &2); + let alice_cursor = alice_page.next_cursor; + + // Bob using Alice's cursor must fail + let res = client.try_get_goals_for_owner(&bob, &alice_cursor, &10); + assert!(res.is_err(), "cross-owner cursor must be rejected"); +} + +/// Multi-owner isolation: each owner sees only their own goals. +#[test] +fn test_get_goals_for_owner_multi_owner_isolation() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let alice = Address::generate(&env); + let bob = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &alice, 4); + setup_goals(&env, &client, &bob, 6); + + let alice_page = client.get_goals_for_owner(&alice, &0, &50); + assert_eq!(alice_page.count, 4); + for g in alice_page.items.iter() { + assert_eq!(g.owner, alice); + } + + let bob_page = client.get_goals_for_owner(&bob, &0, &50); + assert_eq!(bob_page.count, 6); + for g in bob_page.items.iter() { + assert_eq!(g.owner, bob); + } +} + +/// Page size of 1 iterates one goal at a time. +#[test] +fn test_get_goals_for_owner_page_size_one() { + let env = Env::default(); + env.mock_all_auths(); + let id = env.register_contract(None, SavingsGoalContract); + let client = SavingsGoalContractClient::new(&env, &id); + let owner = Address::generate(&env); + + client.init(); + setup_goals(&env, &client, &owner, 3); + + let mut collected = std::vec::Vec::new(); + let mut cursor = 0u32; + loop { + let page = client.get_goals_for_owner(&owner, &cursor, &1); + assert!(page.count <= 1); + for g in page.items.iter() { + collected.push(g.id); + } + if page.next_cursor == 0 { + break; + } + cursor = page.next_cursor; + } + assert_eq!(collected.len(), 3); + assert_eq!(collected, std::vec![1, 2, 3]); +} diff --git a/savings_goals/tests/stress_test_large_amounts.rs b/savings_goals/tests/stress_test_large_amounts.rs index bfe9b353..c1f45b5f 100644 --- a/savings_goals/tests/stress_test_large_amounts.rs +++ b/savings_goals/tests/stress_test_large_amounts.rs @@ -722,7 +722,7 @@ fn test_withdraw_from_goal_overflow_protection() { let goal_id = client.create_goal( &owner, &String::from_str(&env, "Withdrawal Test"), - &i128::MAX / 2, + &(i128::MAX / 2), &2000000, ); @@ -861,4 +861,3 @@ fn test_error_codes_stable_across_repeated_operations() { let goal = client.get_goal(&goal_id).unwrap(); assert_eq!(goal.current_amount, 1000); } -