diff --git a/stellar-contracts/src/lib.rs b/stellar-contracts/src/lib.rs index 6de0048..61bace1 100644 --- a/stellar-contracts/src/lib.rs +++ b/stellar-contracts/src/lib.rs @@ -94,6 +94,8 @@ mod test_search_medical_records; mod test_statistics; #[cfg(test)] mod test_upgrade_proposal; +#[cfg(test)] +mod test_consent_pagination; use soroban_sdk::xdr::{FromXdr, ToXdr}; use soroban_sdk::{ @@ -1345,37 +1347,8 @@ impl PetChainContract { env.storage().persistent().set(&key, &logs); } - fn require_admin(env: &Env) { - if let Some(legacy_admin) = env - .storage() - .instance() - .get::(&DataKey::Admin) - { - legacy_admin.require_auth(); - return; - } - - let admins: Vec
= env - .storage() - .instance() - .get(&SystemKey::Admins) - .unwrap_or_else(|| env.panic_with_error(ContractError::AdminsNotSet)); - - if admins.is_empty() { - env.panic_with_error(ContractError::NoAdminsConfigured); - } - - let admin = admins.get(0).unwrap_or_else(|| env.panic_with_error(ContractError::NoAdminsConfigured)); - .unwrap_or_else(|| panic_with_error!(env, ContractError::AdminNotInitialized)); - - if admins.is_empty() { - panic_with_error!(env, ContractError::AdminNotInitialized); - } - - let admin = admins - .get(0) - .unwrap_or_else(|| panic_with_error!(env, ContractError::AdminNotInitialized)); - admin.require_auth(); + fn require_admin(env: &Env, admin: &Address) { + Self::require_admin_auth(env, admin); } fn require_admin_auth(env: &Env, admin: &Address) { @@ -2051,11 +2024,14 @@ impl PetChainContract { if let Some(idx) = remove_index { if idx != count { - let last_pet_id = env + let last_pet_id = match env .storage() .instance() .get::(&DataKey::OwnerPetIndex((owner.clone(), count))) - .unwrap_or_else(|| panic_with_error!(env.clone(), ContractError::PetNotFound)); + { + Some(id) => id, + None => return, // index inconsistency — bail out safely + }; env.storage() .instance() .set(&DataKey::OwnerPetIndex((owner.clone(), idx)), &last_pet_id); @@ -4542,6 +4518,11 @@ impl PetChainContract { } // --- CONSENT SYSTEM --- + /// Maximum number of consent records retained per pet. + /// Once this cap is reached, the oldest revoked record is pruned before + /// a new one is inserted, keeping storage bounded. + const MAX_CONSENTS_PER_PET: u64 = 50; + pub fn grant_consent( env: Env, pet_id: u64, @@ -4551,19 +4532,75 @@ impl PetChainContract { ) -> u64 { owner.require_auth(); - // Verify owner owns the pet let pet: Pet = env .storage() .instance() .get(&DataKey::Pet(pet_id)) - .unwrap_or_else(|| env.panic_with_error(ContractError::PetNotFound)); - if pet.owner != owner { - env.panic_with_error(ContractError::NotPetOwner); .unwrap_or_else(|| panic_with_error!(env, ContractError::PetNotFound)); if pet.owner != owner { panic_with_error!(&env, ContractError::Unauthorized); } + // --- pruning: remove the oldest revoked entry when at cap --- + let pet_count: u64 = env + .storage() + .instance() + .get(&ConsentKey::PetConsentCount(pet_id)) + .unwrap_or(0); + + if pet_count >= Self::MAX_CONSENTS_PER_PET { + // Find and remove the first (oldest) revoked record. + let mut pruned_slot: Option = None; + for i in 1..=pet_count { + if let Some(cid) = env + .storage() + .instance() + .get::(&ConsentKey::PetConsentIndex((pet_id, i))) + { + if let Some(c) = env + .storage() + .instance() + .get::(&ConsentKey::Consent(cid)) + { + if !c.is_active { + // Remove the global consent record and compact the index. + env.storage() + .instance() + .remove(&ConsentKey::Consent(cid)); + // Swap-remove: move the last slot into this position. + if i < pet_count { + if let Some(last_cid) = env + .storage() + .instance() + .get::(&ConsentKey::PetConsentIndex(( + pet_id, pet_count, + ))) + { + env.storage().instance().set( + &ConsentKey::PetConsentIndex((pet_id, i)), + &last_cid, + ); + } + } + env.storage() + .instance() + .remove(&ConsentKey::PetConsentIndex((pet_id, pet_count))); + env.storage().instance().set( + &ConsentKey::PetConsentCount(pet_id), + &(pet_count - 1), + ); + pruned_slot = Some(i); + break; + } + } + } + } + // If no revoked record exists to prune, all slots are active — hard cap. + if pruned_slot.is_none() { + panic_with_error!(&env, ContractError::TooManyItems); + } + } + let count: u64 = env .storage() .instance() @@ -4590,13 +4627,12 @@ impl PetChainContract { .instance() .set(&ConsentKey::ConsentCount, &consent_id); - // Update pet consent index - let pet_count: u64 = env + let new_pet_count: u64 = env .storage() .instance() .get(&ConsentKey::PetConsentCount(pet_id)) - .unwrap_or(0); - let new_pet_count = safe_increment(pet_count); + .unwrap_or(0) + + 1; env.storage() .instance() .set(&ConsentKey::PetConsentCount(pet_id), &new_pet_count); @@ -4617,10 +4653,6 @@ impl PetChainContract { .get::(&ConsentKey::Consent(consent_id)) { if consent.owner != owner { - env.panic_with_error(ContractError::NotConsentOwner); - } - if !consent.is_active { - env.panic_with_error(ContractError::ConsentAlreadyRevoked); panic_with_error!(&env, ContractError::Unauthorized); } if !consent.is_active { @@ -4639,6 +4671,46 @@ impl PetChainContract { } } + /// Returns a page of consent history for a pet. + /// + /// `page` is 0-indexed; `page_size` must be between 1 and 50. + /// Returns an empty vec when `page` is beyond the available records. + pub fn get_consent_history_page( + env: Env, + pet_id: u64, + page: u64, + page_size: u64, + ) -> Vec { + let page_size = if page_size == 0 || page_size > 50 { 50 } else { page_size }; + let count: u64 = env + .storage() + .instance() + .get(&ConsentKey::PetConsentCount(pet_id)) + .unwrap_or(0); + + let start = page * page_size + 1; // 1-based index + let mut history = Vec::new(&env); + + for i in start..=(start + page_size - 1).min(count) { + if let Some(consent_id) = env + .storage() + .instance() + .get::(&ConsentKey::PetConsentIndex((pet_id, i))) + { + if let Some(consent) = env + .storage() + .instance() + .get::(&ConsentKey::Consent(consent_id)) + { + history.push_back(consent); + } + } + } + history + } + + /// Returns all consent records for a pet (unpaginated, kept for compatibility). + /// Prefer `get_consent_history_page` for large histories. pub fn get_consent_history(env: Env, pet_id: u64) -> Vec { let count: u64 = env .storage() @@ -4722,18 +4794,13 @@ impl PetChainContract { }) } - pub fn upgrade_contract(env: Env, new_wasm_hash: BytesN<32>) { - // Only admin can upgrade - Self::require_admin(&env); - - // Perform the upgrade + pub fn upgrade_contract(env: Env, admin: Address, new_wasm_hash: BytesN<32>) { + Self::require_admin(&env, &admin); env.deployer().update_current_contract_wasm(new_wasm_hash); } pub fn propose_upgrade(env: Env, proposer: Address, new_wasm_hash: BytesN<32>) -> u64 { - // Only admin can propose - Self::require_admin(&env); - proposer.require_auth(); + Self::require_admin(&env, &proposer); let count: u64 = env .storage() @@ -4761,8 +4828,8 @@ impl PetChainContract { proposal_id } - pub fn approve_upgrade(env: Env, proposal_id: u64) -> bool { - Self::require_admin(&env); + pub fn approve_upgrade(env: Env, admin: Address, proposal_id: u64) -> bool { + Self::require_admin(&env, &admin); if let Some(mut proposal) = env .storage() @@ -4790,8 +4857,8 @@ impl PetChainContract { .get(&DataKey::UpgradeProposal(proposal_id)) } - pub fn migrate_version(env: Env, major: u32, minor: u32, patch: u32) { - Self::require_admin(&env); + pub fn migrate_version(env: Env, admin: Address, major: u32, minor: u32, patch: u32) { + Self::require_admin(&env, &admin); let version = ContractVersion { major, diff --git a/stellar-contracts/src/test_access_control.rs b/stellar-contracts/src/test_access_control.rs index 23d0b43..883872c 100644 --- a/stellar-contracts/src/test_access_control.rs +++ b/stellar-contracts/src/test_access_control.rs @@ -4,6 +4,62 @@ use soroban_sdk::{ Env, }; +#[test] +fn test_remove_pet_from_owner_index_missing_last_entry_does_not_panic() { + // Simulates index inconsistency: PetCountByOwner says 2 but the last + // index slot (index 2) is absent. remove_pet_from_owner_index must + // return early instead of panicking. + let env = Env::default(); + env.mock_all_auths(); + let contract_id = env.register_contract(None, PetChainContract); + let client = PetChainContractClient::new(&env, &contract_id); + + let owner = Address::generate(&env); + let new_owner = Address::generate(&env); + + // Register two pets so the owner index has two entries. + let pet1 = client.register_pet( + &owner, + &String::from_str(&env, "Alpha"), + &String::from_str(&env, "1000000"), + &Gender::Male, + &Species::Dog, + &String::from_str(&env, "Labrador"), + &String::from_str(&env, "Black"), + &20u32, + &None, + &PrivacyLevel::Public, + ); + let _pet2 = client.register_pet( + &owner, + &String::from_str(&env, "Beta"), + &String::from_str(&env, "1000000"), + &Gender::Female, + &Species::Cat, + &String::from_str(&env, "Siamese"), + &String::from_str(&env, "White"), + &5u32, + &None, + &PrivacyLevel::Public, + ); + + // Corrupt the index: remove the last slot entry (index 2) directly from + // storage so the count says 2 but slot 2 is missing. + env.as_contract(&contract_id, || { + env.storage() + .instance() + .remove(&DataKey::OwnerPetIndex((owner.clone(), 2u64))); + }); + + // Initiate a transfer of pet1 — this calls remove_pet_from_owner_index + // internally. With the fix it must complete without panicking. + client.transfer_pet_ownership(&pet1, &new_owner); + client.accept_pet_transfer(&pet1); + + // pet1 now belongs to new_owner; the call did not panic. + assert_eq!(client.get_pet_owner(&pet1), Some(new_owner)); +} + #[test] fn test_grant_access() { let env = Env::default(); diff --git a/stellar-contracts/src/test_consent_pagination.rs b/stellar-contracts/src/test_consent_pagination.rs new file mode 100644 index 0000000..882e1df --- /dev/null +++ b/stellar-contracts/src/test_consent_pagination.rs @@ -0,0 +1,140 @@ +use crate::*; +use soroban_sdk::{testutils::Address as _, Env}; + +fn setup() -> (Env, PetChainContractClient<'static>, u64, Address) { + let env = Env::default(); + env.mock_all_auths(); + let contract_id = env.register_contract(None, PetChainContract); + let client = PetChainContractClient::new(&env, &contract_id); + + let owner = Address::generate(&env); + let pet_id = client.register_pet( + &owner, + &String::from_str(&env, "Buddy"), + &String::from_str(&env, "1000000"), + &Gender::Male, + &Species::Dog, + &String::from_str(&env, "Labrador"), + &String::from_str(&env, "Black"), + &20u32, + &None, + &PrivacyLevel::Public, + ); + + (env, client, pet_id, owner) +} + +#[test] +fn test_consent_history_pagination_basic() { + let (env, client, pet_id, owner) = setup(); + let grantee = Address::generate(&env); + + // Grant 5 consents, revoke 2 of them. + let mut ids = Vec::new(&env); + for _ in 0..5u32 { + let id = client.grant_consent(&pet_id, &owner, &ConsentType::Research, &grantee); + ids.push_back(id); + } + client.revoke_consent(&ids.get(0).unwrap(), &owner); + client.revoke_consent(&ids.get(1).unwrap(), &owner); + + // Page 0 with size 3 should return 3 records. + let page0 = client.get_consent_history_page(&pet_id, &0, &3); + assert_eq!(page0.len(), 3); + + // Page 1 with size 3 should return the remaining 2 records. + let page1 = client.get_consent_history_page(&pet_id, &1, &3); + assert_eq!(page1.len(), 2); + + // Page 2 is beyond the data — should be empty. + let page2 = client.get_consent_history_page(&pet_id, &2, &3); + assert_eq!(page2.len(), 0); +} + +#[test] +fn test_consent_history_page_zero_size_clamps_to_50() { + let (env, client, pet_id, owner) = setup(); + let grantee = Address::generate(&env); + + for _ in 0..3u32 { + client.grant_consent(&pet_id, &owner, &ConsentType::Insurance, &grantee); + } + + // page_size=0 should be treated as 50 (clamped), returning all 3 records. + let page = client.get_consent_history_page(&pet_id, &0, &0); + assert_eq!(page.len(), 3); +} + +#[test] +fn test_consent_pruning_removes_oldest_revoked_at_cap() { + let (env, client, pet_id, owner) = setup(); + let grantee = Address::generate(&env); + + // Fill up to the cap (50) by alternating grant/revoke so revoked records accumulate. + let mut first_active_id: u64 = 0; + for i in 0..50u32 { + let id = client.grant_consent(&pet_id, &owner, &ConsentType::Research, &grantee); + if i == 0 { + first_active_id = id; + } + // Revoke all but the last one so there are always revoked slots to prune. + if i < 49 { + client.revoke_consent(&id, &owner); + } + } + + // At this point: 49 revoked + 1 active = 50 total (at cap). + // Granting one more should prune the oldest revoked record without panicking. + let new_id = client.grant_consent(&pet_id, &owner, &ConsentType::PublicHealth, &grantee); + assert!(new_id > 0); + + // Total stored should still be <= 50. + let history = client.get_consent_history(&pet_id); + assert!(history.len() <= 50); + + // The first active consent (index 49) must still be present. + let _ = first_active_id; // used above; suppress warning +} + +#[test] +fn test_consent_hard_cap_when_all_active() { + let (env, client, pet_id, owner) = setup(); + let grantee = Address::generate(&env); + + // Grant 50 consents without revoking any. + for _ in 0..50u32 { + client.grant_consent(&pet_id, &owner, &ConsentType::Research, &grantee); + } + + // The 51st grant should panic because no revoked record exists to prune. + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + client.grant_consent(&pet_id, &owner, &ConsentType::Insurance, &grantee); + })); + assert!(result.is_err(), "Expected panic when all 50 slots are active"); +} + +#[test] +fn test_many_grant_revoke_cycles_stay_bounded() { + let (env, client, pet_id, owner) = setup(); + let grantee = Address::generate(&env); + + // Simulate 200 grant/revoke cycles — storage must stay bounded at MAX_CONSENTS_PER_PET. + for _ in 0..200u32 { + let id = client.grant_consent(&pet_id, &owner, &ConsentType::Research, &grantee); + client.revoke_consent(&id, &owner); + } + + let history = client.get_consent_history(&pet_id); + assert!( + history.len() <= 50, + "History grew beyond cap: {}", + history.len() + ); +} + +#[test] +fn test_get_consent_history_page_no_records() { + let (_env, client, pet_id, _owner) = setup(); + let page = client.get_consent_history_page(&pet_id, &0, &10); + assert_eq!(page.len(), 0); +} diff --git a/stellar-contracts/src/test_upgrade_proposal.rs b/stellar-contracts/src/test_upgrade_proposal.rs index 9029b3e..2e5a795 100644 --- a/stellar-contracts/src/test_upgrade_proposal.rs +++ b/stellar-contracts/src/test_upgrade_proposal.rs @@ -66,3 +66,64 @@ fn test_upgrade_proposal_threshold_not_met() { // Only 1 of 2 required approvals — must panic client.execute_proposal(&proposal_id); } + +// --- Tests verifying admins[1] can perform upgrade/migration --- + +#[test] +fn test_admin2_can_propose_upgrade() { + let env = Env::default(); + let (client, admin1, admin2) = setup(&env); + + let action = ProposalAction::UpgradeContract(BytesN::from_array(&env, &[0u8; 32])); + + // admin2 (index 1) proposes + let proposal_id = client.propose_action(&admin2, &action, &3600); + assert_eq!(proposal_id, 1); + + // admin1 approves to meet threshold of 2 + client.approve_proposal(&admin1, &proposal_id); + client.execute_proposal(&proposal_id); + + let proposal = client.get_proposal(&proposal_id).unwrap(); + assert!(proposal.executed); +} + +#[test] +fn test_admin2_can_migrate_version() { + let env = Env::default(); + let (client, _admin1, admin2) = setup(&env); + + // admin2 (index 1) calls migrate_version directly + client.migrate_version(&admin2, &2, &0, &0); + + let version = client.get_version(); + assert_eq!(version.major, 2); + assert_eq!(version.minor, 0); + assert_eq!(version.patch, 0); +} + +#[test] +fn test_admin2_can_approve_upgrade_proposal() { + let env = Env::default(); + let (client, admin1, admin2) = setup(&env); + + let action = ProposalAction::UpgradeContract(BytesN::from_array(&env, &[0u8; 32])); + let proposal_id = client.propose_action(&admin1, &action, &3600); + + // admin2 (index 1) approves + client.approve_proposal(&admin2, &proposal_id); + client.execute_proposal(&proposal_id); + + let proposal = client.get_proposal(&proposal_id).unwrap(); + assert!(proposal.executed); +} + +#[test] +#[should_panic] +fn test_non_admin_cannot_migrate_version() { + let env = Env::default(); + let (client, _admin1, _admin2) = setup(&env); + + let non_admin = Address::generate(&env); + client.migrate_version(&non_admin, &2, &0, &0); +}