diff --git a/remittance_split/README.md b/remittance_split/README.md index 9351ecc4..a9ca279a 100644 --- a/remittance_split/README.md +++ b/remittance_split/README.md @@ -25,6 +25,7 @@ in strict order before any token interaction occurs: Returns `SelfTransferNotAllowed` if any match. 7. **Replay protection** — nonce must equal `get_nonce(from)` and is incremented after success. 8. **Audit + event** — a `DistributionCompleted` event is emitted on success for off-chain indexing. +9. **Schedule ID Sequencing** — `create_remittance_schedule` generates strictly monotonic IDs using a synchronized counter (`NEXT_RSCH`), ensuring no collisions across high-volume operations. ## Features diff --git a/remittance_split/src/lib.rs b/remittance_split/src/lib.rs index 04d082c9..f7eaf653 100644 --- a/remittance_split/src/lib.rs +++ b/remittance_split/src/lib.rs @@ -1341,8 +1341,16 @@ impl RemittanceSplit { .storage() .instance() .get(&symbol_short!("NEXT_RSCH")) - .unwrap_or(0u32) - + 1; + .unwrap_or(0u32); + + let next_schedule_id = current_max_id + .checked_add(1) + .ok_or(RemittanceSplitError::Overflow)?; + + // Explicit uniqueness check to prevent any potential storage collisions + if schedules.contains_key(next_schedule_id) { + return Err(RemittanceSplitError::Overflow); // Should be unreachable with monotonic counter + } let schedule = RemittanceSchedule { id: next_schedule_id, diff --git a/remittance_split/tests/stress_test_large_amounts.rs b/remittance_split/tests/stress_test_large_amounts.rs index 85a447e0..736b8b6f 100644 --- a/remittance_split/tests/stress_test_large_amounts.rs +++ b/remittance_split/tests/stress_test_large_amounts.rs @@ -254,3 +254,88 @@ fn test_insurance_remainder_calculation_with_large_values() { let total: i128 = amounts.iter().sum(); assert_eq!(total, large_amount); } +#[test] +fn test_schedule_id_sequencing_monotonicity() { + let env = Env::default(); + let contract_id = env.register_contract(None, RemittanceSplit); + let client = RemittanceSplitClient::new(&env, &contract_id); + let owner =
::generate(&env); + env.mock_all_auths(); + + let amount = 1000_i128; + let next_due = env.ledger().timestamp() + 86400; + let interval = 86400; + + let mut last_id = 0; + for _ in 0..100 { + let id = client.create_remittance_schedule(&owner, &amount, &next_due, &interval); + assert!(id > last_id, "Schedule IDs must be strictly monotonic"); + last_id = id; + } +} + +#[test] +fn test_schedule_id_uniqueness_across_operations() { + let env = Env::default(); + let contract_id = env.register_contract(None, RemittanceSplit); + let client = RemittanceSplitClient::new(&env, &contract_id); + let owner =
::generate(&env); + env.mock_all_auths(); + + let amount = 1000_i128; + let next_due = env.ledger().timestamp() + 86400; + let interval = 86400; + + // 1. Create several schedules + let id1 = client.create_remittance_schedule(&owner, &amount, &next_due, &interval); + let id2 = client.create_remittance_schedule(&owner, &amount, &next_due, &interval); + let id3 = client.create_remittance_schedule(&owner, &amount, &next_due, &interval); + + assert_ne!(id1, id2); + assert_ne!(id2, id3); + + // 2. Modify one + client.modify_remittance_schedule(&owner, &id1, &(amount * 2), &(next_due + 100), &interval); + let mod_schedule = client.get_remittance_schedule(&id1).unwrap(); + assert_eq!(mod_schedule.id, id1, "Schedule ID must remain stable after modification"); + + // 3. Cancel one + client.cancel_remittance_schedule(&owner, &id2); + let cancelled = client.get_remittance_schedule(&id2).unwrap(); + assert_eq!(cancelled.active, false); + + // 4. Create new one and verify it doesn't collide with ANY previous ID (including cancelled/modified) + let id4 = client.create_remittance_schedule(&owner, &amount, &next_due, &interval); + assert!(id4 > id3, "New ID must be greater than all previous IDs"); + assert_ne!(id4, id1); + assert_ne!(id4, id2); + assert_ne!(id4, id3); +} + +#[test] +fn test_high_volume_schedule_creation_no_collisions() { + let env = Env::default(); + env.budget().reset_unlimited(); + let contract_id = env.register_contract(None, RemittanceSplit); + let client = RemittanceSplitClient::new(&env, &contract_id); + let owner =
::generate(&env); + env.mock_all_auths(); + + let amount = 1000_i128; + let next_due = env.ledger().timestamp() + 86400; + + // Create 500 schedules and track IDs + let mut ids = soroban_sdk::Vec::new(&env); + for i in 0..500 { + let id = client.create_remittance_schedule(&owner, &amount, &(next_due + i as u64), &0); + ids.push_back(id); + } + + // Verify all IDs are unique (O(n^2) check if necessary, or sort) + // In soroban testing we can just use a Map for O(n) + let mut seen = soroban_sdk::Map::new(&env); + for id in ids.iter() { + assert!(seen.get(id).is_none(), "Collision detected for schedule ID: {}", id); + seen.set(id, true); + } +}