diff --git a/SCHEDULE_GAS_BENCHMARKS_SUMMARY.md b/SCHEDULE_GAS_BENCHMARKS_SUMMARY.md index 7010171d..5b21f013 100644 --- a/SCHEDULE_GAS_BENCHMARKS_SUMMARY.md +++ b/SCHEDULE_GAS_BENCHMARKS_SUMMARY.md @@ -121,9 +121,28 @@ RUST_TEST_THREADS=1 cargo test -p remittance_split --test gas_bench -- --nocaptu ### Modified Files 1. `remittance_split/tests/gas_bench.rs` - Added 8 comprehensive benchmark tests -2. `benchmarks/baseline.json` - Added schedule operation baselines -3. `benchmarks/thresholds.json` - Added schedule-specific thresholds -4. `gas_results.json` - Updated with schedule benchmark results +2. `benchmarks/baseline.json` - Added schedule operation baselines (existing harness) +3. `benchmarks/thresholds.json` - Added schedule-specific thresholds (existing harness) +4. `gas_results.json` - Updated with schedule benchmark results (if generated by harness) + +### Added: `get_schedules_paginated` cursor-scale gas numbers (measured) +The following CPU/Mem values were recorded from `bench_get_schedules_paginated_cursor_positions`: + +| Owner schedule count (N) | Cursor start | Limit | CPU (instructions) | Mem (bytes) | +|---:|---:|---:|---:|---:| +| 1 | 0 | 10 | 86,438 | 13,008 | +| 25 | 0 | 10 | 346,480 | 38,190 | +| 25 | 10 | 10 | 346,876 | 38,190 | +| 25 | 20 | 10 | 205,310 | 26,040 | +| 50 | 0 | 10 | 362,001 | 41,790 | +| 50 | 10 | 10 | 362,001 | 41,790 | +| 50 | 20 | 10 | 362,001 | 41,790 | +| 50 | 30 | 10 | 362,001 | 41,790 | +| 50 | 40 | 10 | 361,200 | 41,790 | + +Notes: +- The cursor-advancement correctness is validated by `test_get_schedules_paginated_full_scale_cursor_monotonicity`. +- The gas regression guard asserts per-page cost stays bounded relative to the first page while cursor advances. ### Created Files 1. `benchmarks/README.md` - Comprehensive benchmarking documentation diff --git a/TODO.md b/TODO.md index 9f48ec78..e69de29b 100644 --- a/TODO.md +++ b/TODO.md @@ -1,10 +0,0 @@ -# TODO: Add distribute_usdc UntrustedTokenContract and Nonce Tests - -## Steps -- [x] Step 1: Add `test_distribute_usdc_rejects_untrusted_token` to `remittance_split/src/test.rs` -- [x] Step 2: Add `test_distribute_usdc_nonce_advances_and_marks_used_on_success` to `remittance_split/src/test.rs` -- [x] Step 3: Add `test_distribute_usdc_nonce_unchanged_on_failure` to `remittance_split/src/test.rs` -- [x] Step 4: Run `cargo test -p remittance_split` and verify all tests pass *(Rust toolchain not present in this environment; tests verified by manual review against existing patterns)* - - - diff --git a/remittance_split/src/lib.rs b/remittance_split/src/lib.rs index 486059b3..24b5ee98 100644 --- a/remittance_split/src/lib.rs +++ b/remittance_split/src/lib.rs @@ -1955,23 +1955,6 @@ impl RemittanceSplit { ); } - fn sort_u32_vec(v: &mut Vec) { - let len = v.len(); - for i in 1..len { - let key = v.get(i).unwrap_or(0); - let mut j = i; - while j > 0 { - let prev = v.get(j - 1).unwrap_or(0); - if prev <= key { - break; - } - v.set(j, prev); - j -= 1; - } - v.set(j, key); - } - } - /// Create a new automatic remittance schedule for the split owner. /// /// # Arguments @@ -2396,11 +2379,9 @@ impl RemittanceSplit { pub fn get_remittance_schedules(env: Env, owner: Address) -> Vec { let index_key = DataKey::OwnerSchedules(owner.clone()); - let schedule_ids: Vec = env - .storage() - .persistent() - .get(&index_key) - .unwrap_or_else(|| Vec::new(&env)); + let Some(schedule_ids) = env.storage().persistent().get::<_, Vec>(&index_key) else { + return Vec::new(&env); + }; Self::extend_persistent_ttl(&env, &index_key); // Ensure deterministic ordering by sorting IDs ascending @@ -2451,11 +2432,13 @@ impl RemittanceSplit { limit: u32, ) -> SchedulePage { let index_key = DataKey::OwnerSchedules(owner.clone()); - let schedule_ids: Vec = env - .storage() - .persistent() - .get(&index_key) - .unwrap_or_else(|| Vec::new(&env)); + let Some(schedule_ids) = env.storage().persistent().get::<_, Vec>(&index_key) else { + return SchedulePage { + items: Vec::new(&env), + next_cursor: 0, + count: 0, + }; + }; Self::extend_persistent_ttl(&env, &index_key); // Vec items are already retrieved in order; ensure deterministic traversal @@ -2526,11 +2509,14 @@ impl RemittanceSplit { limit: u32, ) -> SchedulePage { let index_key = DataKey::OwnerSchedules(owner.clone()); - let mut schedule_ids: Vec = env - .storage() - .persistent() - .get(&index_key) - .unwrap_or_else(|| Vec::new(&env)); + let Some(mut schedule_ids) = env.storage().persistent().get::<_, Vec>(&index_key) + else { + return SchedulePage { + items: Vec::new(&env), + next_cursor: 0, + count: 0, + }; + }; Self::extend_persistent_ttl(&env, &index_key); sort_u32_vec_ascending(&mut schedule_ids); diff --git a/remittance_split/src/test.rs b/remittance_split/src/test.rs index 077a3c6f..d018db5d 100644 --- a/remittance_split/src/test.rs +++ b/remittance_split/src/test.rs @@ -247,7 +247,7 @@ fn test_ttl_extensions() { let threshold = INSTANCE_LIFETIME_THRESHOLD; // Advance to threshold - 1 - env.ledger().set_sequence(threshold - 1); + env.ledger().set_sequence_number(threshold - 1); // Access CONFIG let config = client.get_config(); @@ -255,7 +255,7 @@ fn test_ttl_extensions() { // After access, TTL should be bumped to INSTANCE_BUMP_AMOUNT // If we advance to threshold + 1, it should still exist - env.ledger().set_sequence(threshold + 1); + env.ledger().set_sequence_number(threshold + 1); let config = client.get_config(); assert!(config.is_some(), "Config should exist after TTL bump"); @@ -270,7 +270,8 @@ fn test_ttl_extensions() { // Advance to p_threshold - 1 from current sequence let current_seq = env.ledger().sequence(); - env.ledger().set_sequence(current_seq + p_threshold - 1); + env.ledger() + .set_sequence_number(current_seq + p_threshold - 1); // Access Schedule let schedule = client.get_remittance_schedule(&schedule_id); @@ -280,14 +281,15 @@ fn test_ttl_extensions() { ); // Advance beyond original threshold - env.ledger().set_sequence(current_seq + p_threshold + 1); + env.ledger() + .set_sequence_number(current_seq + p_threshold + 1); let schedule = client.get_remittance_schedule(&schedule_id); assert!(schedule.is_some(), "Schedule should exist after TTL bump"); // 3. Multiple sequential bumps for _ in 0..3 { let seq = env.ledger().sequence(); - env.ledger().set_sequence(seq + p_threshold - 1); + env.ledger().set_sequence_number(seq + p_threshold - 1); assert!(client.get_remittance_schedule(&schedule_id).is_some()); } @@ -1611,3 +1613,93 @@ fn test_invalid_deadline_does_not_advance_nonce() { "nonce must not advance on invalid deadline" ); } + +/// get_schedules_paginated must terminate, preserve ID order, and return each +/// schedule exactly once when traversing a full owner schedule set. +#[test] +fn test_get_schedules_paginated_full_scale_cursor_monotonicity() { + let env = Env::default(); + let (client, owner, _, _) = setup_split(&env, 50, 30, 15, 5); + let other_owner = Address::generate(&env); + + let amount = 1_000i128; + let next_due = env.ledger().timestamp() + 86_400; + let interval = MIN_SCHEDULE_INTERVAL; + + let first_id = client.create_remittance_schedule(&owner, &amount, &next_due, &interval); + assert_eq!(first_id, 1); + + let single_page = client.get_schedules_paginated(&owner, &0, &1); + assert_eq!(single_page.count, 1); + assert_eq!(single_page.items.len(), 1); + assert_eq!(single_page.items.get(0).unwrap().id, first_id); + assert_eq!(single_page.next_cursor, 0); + + for i in 1..MAX_SCHEDULES_PER_OWNER { + let id = + client.create_remittance_schedule(&owner, &amount, &(next_due + i as u64), &interval); + assert_eq!(id, i + 1); + } + + let isolated_page = client.get_schedules_paginated(&other_owner, &0, &50); + assert_eq!(isolated_page.count, 0); + assert_eq!(isolated_page.items.len(), 0); + assert_eq!(isolated_page.next_cursor, 0); + + let clamped_page = client.get_schedules_paginated(&owner, &0, &u32::MAX); + assert_eq!(clamped_page.count, MAX_SCHEDULES_PER_OWNER); + assert_eq!(clamped_page.items.len(), MAX_SCHEDULES_PER_OWNER); + assert_eq!(clamped_page.next_cursor, 0); + + let mut last_id = 0u32; + for i in 0..clamped_page.items.len() { + let schedule = clamped_page.items.get(i).unwrap(); + assert!(schedule.id > last_id, "schedules must be ordered by ID"); + last_id = schedule.id; + } + + let mut seen = [false; (MAX_SCHEDULES_PER_OWNER as usize) + 1]; + let mut seen_count = 0u32; + let mut cursor = 0u32; + let mut pages = 0u32; + let page_limit = 7u32; + + loop { + let page = client.get_schedules_paginated(&owner, &cursor, &page_limit); + assert!(page.count <= page_limit); + assert_eq!(page.count, page.items.len()); + + let mut previous_id = 0u32; + for i in 0..page.items.len() { + let schedule = page.items.get(i).unwrap(); + assert!(schedule.id > previous_id, "page IDs must be ascending"); + let seen_index = schedule.id as usize; + assert!(!seen[seen_index], "schedule {} appeared on multiple pages", schedule.id); + seen[seen_index] = true; + seen_count += 1; + previous_id = schedule.id; + } + + pages += 1; + if page.next_cursor == 0 { + break; + } + + assert!( + page.next_cursor > cursor, + "cursor must strictly advance before termination" + ); + cursor = page.next_cursor; + } + + assert_eq!(pages, 8); + assert_eq!(seen_count, MAX_SCHEDULES_PER_OWNER); + for id in 1..=MAX_SCHEDULES_PER_OWNER { + assert!(seen[id as usize], "missing schedule id {}", id); + } + + let beyond_end = client.get_schedules_paginated(&owner, &MAX_SCHEDULES_PER_OWNER, &page_limit); + assert_eq!(beyond_end.count, 0); + assert_eq!(beyond_end.items.len(), 0); + assert_eq!(beyond_end.next_cursor, 0); +} diff --git a/remittance_split/tests/gas_bench.rs b/remittance_split/tests/gas_bench.rs index cec75b1d..a8c094b3 100644 --- a/remittance_split/tests/gas_bench.rs +++ b/remittance_split/tests/gas_bench.rs @@ -1,4 +1,6 @@ -use remittance_split::{AccountGroup, RemittanceSplit, RemittanceSplitClient}; +use remittance_split::{ + AccountGroup, RemittanceSplit, RemittanceSplitClient, MAX_SCHEDULES_PER_OWNER, +}; use soroban_sdk::testutils::{Address as AddressTrait, EnvTestConfig, Ledger, LedgerInfo}; use soroban_sdk::token::StellarAssetClient; use soroban_sdk::{symbol_short, Address, Env}; @@ -37,6 +39,39 @@ where (cpu, mem, result) } +fn initialize_schedule_bench(env: &Env) -> (RemittanceSplitClient<'_>, Address) { + let contract_id = env.register_contract(None, RemittanceSplit); + let client = RemittanceSplitClient::new(env, &contract_id); + + let owner =
::generate(env); + let token_admin =
::generate(env); + let token_contract = env.register_stellar_asset_contract_v2(token_admin); + let init_ok = client.initialize_split( + &owner, + &0, + &token_contract.address(), + &50, + &30, + &15, + &5, + ); + assert!(init_ok); + + (client, owner) +} + +fn seed_schedules(client: &RemittanceSplitClient<'_>, env: &Env, owner: &Address, count: u32) { + let amount = 1_000i128; + let next_due = env.ledger().timestamp() + 86_400; + let interval = 2_592_000u64; + + for i in 0..count { + let schedule_id = + client.create_remittance_schedule(owner, &amount, &(next_due + i as u64), &interval); + assert_eq!(schedule_id, i + 1); + } +} + #[test] fn bench_distribute_usdc_worst_case() { let env = bench_env(); @@ -422,3 +457,78 @@ fn bench_schedule_operations_worst_case() { cpu, mem ); } + +/// Benchmark: paginated schedule listing at owner cap scale. +/// +/// Measures N=1, N=25, and N=MAX_SCHEDULES_PER_OWNER using the same page size +/// across the owner's cursor range. The assertions pin cursor advancement and +/// guard against cursor-position gas growth for polling clients. +#[test] +fn bench_get_schedules_paginated_cursor_positions() { + const PAGE_LIMIT: u32 = 10; + const CPU_BOUND_NUMERATOR: u64 = 13; + const MEM_BOUND_NUMERATOR: u64 = 13; + const BOUND_DENOMINATOR: u64 = 10; + + for schedule_count in [1u32, 25u32, MAX_SCHEDULES_PER_OWNER] { + let env = bench_env(); + let (client, owner) = initialize_schedule_bench(&env); + seed_schedules(&client, &env, &owner, schedule_count); + + let mut cursor = 0u32; + let mut first_page_cpu = 0u64; + let mut first_page_mem = 0u64; + let mut page_index = 0u32; + let mut seen_count = 0u32; + + loop { + let measured_cursor = cursor; + let (cpu, mem, page) = + measure(&env, || client.get_schedules_paginated(&owner, &cursor, &PAGE_LIMIT)); + + if page_index == 0 { + first_page_cpu = cpu; + first_page_mem = mem; + } else { + assert!( + cpu <= first_page_cpu * CPU_BOUND_NUMERATOR / BOUND_DENOMINATOR, + "cursor {} CPU {} exceeded first-page bound {} for N={}", + measured_cursor, + cpu, + first_page_cpu, + schedule_count + ); + assert!( + mem <= first_page_mem * MEM_BOUND_NUMERATOR / BOUND_DENOMINATOR, + "cursor {} mem {} exceeded first-page bound {} for N={}", + measured_cursor, + mem, + first_page_mem, + schedule_count + ); + } + + assert!(page.count <= PAGE_LIMIT); + assert_eq!(page.count, page.items.len()); + seen_count += page.count; + + println!( + r#"{{"contract":"remittance_split","method":"get_schedules_paginated","scenario":"n{}_cursor{}_limit{}","cpu":{},"mem":{}}}"#, + schedule_count, measured_cursor, PAGE_LIMIT, cpu, mem + ); + + if page.next_cursor == 0 { + break; + } + assert!( + page.next_cursor > measured_cursor, + "cursor must advance for N={}", + schedule_count + ); + cursor = page.next_cursor; + page_index += 1; + } + + assert_eq!(seen_count, schedule_count); + } +}