Skip to content

Implement Expert Directory Pagination #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Bosun-Josh121 merged 1 commit into from
Mar 25, 2026
Merged

Implement Expert Directory Pagination #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions contracts/identity-registry-contract/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,25 @@ pub fn update_profile(env: &Env, expert: &Address, new_uri: String) -> Result<()
events::emit_profile_updated(env, expert.clone(), new_uri);
Ok(())
}

/// Get a paginated list of experts
/// Returns a vector of expert addresses from start_index to start_index + limit
pub fn get_experts_paginated(env: &Env, start_index: u64, limit: u64) -> Vec<Address> {
let total = storage::get_total_experts(env);
let mut experts = Vec::new(env);

// Calculate the actual end index (don't exceed total)
let end_index = if start_index + limit > total {
total
} else {
start_index + limit
};
Comment on lines +168 to +172
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cd contracts/identity-registry-contract && head -n 180 src/contract.rs | tail -n 30

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 1031

🏁 Script executed:

cd contracts/identity-registry-contract && cat -n src/contract.rs | sed -n '150,180p'

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 1306

🏁 Script executed:

rg -n "get_experts_paginated" contracts/identity-registry-contract/src/contract.rs -A 25

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 708

🏁 Script executed:

rg -n "get_experts_paginated" contracts/identity-registry-contract/src/ -B 2 -A 2

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 2085

🏁 Script executed:

rg -n "#\[contract\]|#\[derive_contract\]|pub fn get_experts" contracts/identity-registry-contract/src/contract.rs -B 1 -A 5

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 474

🏁 Script executed:

fd "contract.rs|lib.rs|mod.rs" contracts/identity-registry-contract/src/ -type f

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 249

🏁 Script executed:

cat -n contracts/identity-registry-contract/src/lib.rs | head -n 80

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 3309

🏁 Script executed:

rg -n "#\[contract\]" contracts/identity-registry-contract/src/

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 141

Prevent u64 overflow in pagination bound calculation.

get_experts_paginated is a public contract method that accepts arbitrary start_index and limit values from external callers. On lines 168 and 171, start_index + limit can overflow; in Soroban release builds, this wraps to an unexpected value and breaks the bounds check. Use saturating arithmetic to ensure predictable, safe behavior.

🔧 Proposed fix 
-    let end_index = if start_index + limit > total {
-        total
-    } else {
-        start_index + limit
-    };
+    let end_index = core::cmp::min(start_index.saturating_add(limit), total);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let end_index = if start_index + limit > total {
total
} else {
start_index + limit
};
let end_index = core::cmp::min(start_index.saturating_add(limit), total);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/identity-registry-contract/src/contract.rs` around lines 168 - 172,
The pagination end bound computation in get_experts_paginated can overflow when
computing start_index + limit; replace the raw addition with saturating
arithmetic (e.g., start_index.saturating_add(limit)) when computing end_index so
the sum never wraps, then compare that saturated sum to total to pick the min
for end_index (retain the existing variables end_index, start_index, limit,
total and the same branch logic but use saturating_add).


// Fetch experts from start_index to end_index
for i in start_index..end_index {
let expert = storage::get_expert_by_index(env, i);
experts.push_back(expert);
}

experts
}
6 changes: 6 additions & 0 deletions contracts/identity-registry-contract/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,10 @@ impl IdentityRegistryContract {
pub fn update_profile(env: Env, expert: Address, new_uri: String) -> Result<(), RegistryError> {
contract::update_profile(&env, &expert, new_uri)
}

/// Get a paginated list of experts
/// Returns a vector of expert addresses from start_index to start_index + limit
pub fn get_experts_paginated(env: Env, start_index: u64, limit: u64) -> Vec<Address> {
contract::get_experts_paginated(&env, start_index, limit)
}
}
42 changes: 42 additions & 0 deletions contracts/identity-registry-contract/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,3 +581,45 @@ fn test_expert_directory_via_batch_add() {
assert_eq!(client.get_expert_by_index(&1u64), expert2);
assert_eq!(client.get_expert_by_index(&2u64), expert3);
}

#[test]
fn test_expert_pagination() {
let env = Env::default();
env.mock_all_auths();

let contract_id = env.register(IdentityRegistryContract, ());
let client = IdentityRegistryContractClient::new(&env, &contract_id);

let admin = Address::generate(&env);
client.init(&admin);

// Add 15 experts
let mut experts = vec![&env];
for _ in 0..15 {
let expert = Address::generate(&env);
let uri = String::from_str(&env, "ipfs://expert");
client.add_expert(&expert, &uri);
experts.push_back(expert);
}

// Verify total count
assert_eq!(client.get_total_experts(), 15u64);

// Fetch start: 0, limit: 10 (should return 10)
let page1 = client.get_experts_paginated(&0u64, &10u64);
assert_eq!(page1.len(), 10);

// Verify the first 10 experts match
for i in 0..10 {
assert_eq!(page1.get(i as u32).unwrap(), experts.get(i as u32).unwrap());
}

// Fetch start: 10, limit: 10 (should return 5)
let page2 = client.get_experts_paginated(&10u64, &10u64);
assert_eq!(page2.len(), 5);

// Verify the last 5 experts match
for i in 0..5 {
assert_eq!(page2.get(i as u32).unwrap(), experts.get((i + 10) as u32).unwrap());
}
}
Loading