Skip to content

feat(identity-registry): implement Expert Directory enumeration (#25) #28

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 4 commits into from
Feb 24, 2026
Merged

feat(identity-registry): implement Expert Directory enumeration (#25) #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1fb2717
feat(identity-registry): add enumerable expert directory storage
KevinMB0220 Feb 24, 2026
4dace81
feat(identity-registry): index experts on verification
KevinMB0220 Feb 24, 2026
c7252ba
feat(identity-registry): expose get_total_experts and get_expert_by_i…
KevinMB0220 Feb 24, 2026
c7590d4
test(identity-registry): add expert directory enumeration tests
KevinMB0220 Feb 24, 2026
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
18 changes: 15 additions & 3 deletions contracts/identity-registry-contract/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use soroban_sdk::{Address, Env, Vec, String};
use crate::storage;
use crate::events;
use crate::storage;
use crate::{error::RegistryError, types::ExpertStatus};
use soroban_sdk::{Address, Env, String, Vec};

/// Initialize the registry with an admin address
pub fn initialize_registry(env: &Env, admin: &Address) -> Result<(), RegistryError> {
Expand All @@ -16,7 +16,7 @@ pub fn initialize_registry(env: &Env, admin: &Address) -> Result<(), RegistryErr

/// Verify an expert by setting their status to Verified (Admin only)
/// Batch Verification
pub fn batch_add_experts(env:Env, experts: Vec<Address>) -> Result<(), RegistryError> {
pub fn batch_add_experts(env: Env, experts: Vec<Address>) -> Result<(), RegistryError> {
if experts.len() > 20 {
return Err(RegistryError::ExpertVecMax);
}
Expand All @@ -32,6 +32,7 @@ pub fn batch_add_experts(env:Env, experts: Vec<Address>) -> Result<(), RegistryE
// Default empty URI for batch adds
let empty_uri = String::from_str(&env, "");
storage::set_expert_record(&env, &expert, ExpertStatus::Verified, empty_uri);
storage::add_expert_to_index(&env, &expert);
events::emit_status_change(&env, expert, status, ExpertStatus::Verified, admin.clone());
}

Expand Down Expand Up @@ -77,6 +78,7 @@ pub fn verify_expert(env: &Env, expert: &Address, data_uri: String) -> Result<()
}

storage::set_expert_record(env, expert, ExpertStatus::Verified, data_uri);
storage::add_expert_to_index(env, expert);

events::emit_status_change(
env,
Expand Down Expand Up @@ -115,6 +117,16 @@ pub fn ban_expert(env: &Env, expert: &Address) -> Result<(), RegistryError> {
Ok(())
}

/// Get the total number of verified experts ever indexed
pub fn get_total_experts(env: &Env) -> u64 {
storage::get_total_experts(env)
}

/// Get the expert address at the given index
pub fn get_expert_by_index(env: &Env, index: u64) -> Address {
storage::get_expert_by_index(env, index)
}

/// Get the current status of an expert
pub fn get_expert_status(env: &Env, expert: &Address) -> ExpertStatus {
storage::get_expert_status(env, expert)
Expand Down
14 changes: 12 additions & 2 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 @@ -10,7 +10,7 @@ mod types;

use crate::error::RegistryError;
use crate::types::ExpertStatus;
use soroban_sdk::{contract, contractimpl, Address, Env, Vec, String};
use soroban_sdk::{contract, contractimpl, Address, Env, String, Vec};

#[contract]
pub struct IdentityRegistryContract;
Expand All @@ -22,7 +22,7 @@ impl IdentityRegistryContract {
contract::initialize_registry(&env, &admin)
}

/// Batch Add an expert to the whitelist (Admin only)
/// Batch Add an expert to the whitelist (Admin only)
pub fn batch_add_experts(env: Env, experts: Vec<Address>) -> Result<(), RegistryError> {
contract::batch_add_experts(env, experts)
}
Expand All @@ -43,6 +43,16 @@ impl IdentityRegistryContract {
contract::ban_expert(&env, &expert)
}

/// Get the total number of verified experts ever added to the directory
pub fn get_total_experts(env: Env) -> u64 {
contract::get_total_experts(&env)
}

/// Get the expert address at the given index in the directory
pub fn get_expert_by_index(env: Env, index: u64) -> Address {
contract::get_expert_by_index(&env, index)
}

/// Get the current status of an expert
pub fn get_status(env: Env, expert: Address) -> ExpertStatus {
contract::get_expert_status(&env, &expert)
Expand Down
42 changes: 42 additions & 0 deletions contracts/identity-registry-contract/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use soroban_sdk::{contracttype, Address, Env, String};
pub enum DataKey {
Admin,
Expert(Address),
VerifiedExpertIndex(u64),
TotalVerifiedCount,
}

// Constants for TTL (Time To Live)
Expand Down Expand Up @@ -87,3 +89,43 @@ pub fn get_expert_record(env: &Env, expert: &Address) -> ExpertRecord {
pub fn get_expert_status(env: &Env, expert: &Address) -> ExpertStatus {
get_expert_record(env, expert).status
}

// ... [Expert Directory Index Helpers] ...

/// Add an expert address to the enumerable index and increment the count
pub fn add_expert_to_index(env: &Env, expert: &Address) {
let count: u64 = env
.storage()
.instance()
.get(&DataKey::TotalVerifiedCount)
.unwrap_or(0u64);

env.storage()
.persistent()
.set(&DataKey::VerifiedExpertIndex(count), expert);
env.storage().persistent().extend_ttl(
&DataKey::VerifiedExpertIndex(count),
LEDGERS_THRESHOLD,
LEDGERS_EXTEND_TO,
);

env.storage()
.instance()
.set(&DataKey::TotalVerifiedCount, &(count + 1));
}
Comment on lines +96 to +115
Copy link

@coderabbitai coderabbitai bot Feb 24, 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:

#!/bin/bash
# Check for relevant state transition and indexing logic
rg -n 'Banned|add_expert_to_index|AlreadyVerified' contracts/identity-registry-contract/src/contract.rs

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 965

🏁 Script executed:

#!/bin/bash
# Get the verify_expert function with context around line 30-35
sed -n '20,45p' contracts/identity-registry-contract/src/contract.rs

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 1061

🏁 Script executed:

#!/bin/bash
# Get the verify_expert function
rg -B15 -A10 'pub fn verify_expert' contracts/identity-registry-contract/src/contract.rs

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 961

🏁 Script executed:

#!/bin/bash
# Get the complete verify_expert function
rg -A25 'pub fn verify_expert' contracts/identity-registry-contract/src/contract.rs

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 862

🏁 Script executed:

#!/bin/bash
# Check if there's any removal/deletion logic for the index in storage.rs
rg -n 'remove|delete|pop|clear' contracts/identity-registry-contract/src/storage.rs

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 61

🏁 Script executed:

#!/bin/bash
# Double-check by looking at the complete ExpertStatus enum to confirm Banned exists
rg -A5 'enum ExpertStatus' contracts/identity-registry-contract/src/

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 459

Append-only index creates duplicate entries when banned experts are re-verified.

The index has no removal path and is purely append-only. More critically, the verification logic allows the state transition Banned → Verified without preventing re-indexing. Both verify_expert and batch_add_experts only guard against Verified → Verified (checking if current_status == ExpertStatus::Verified), but they do not block Banned → Verified. This means a banned expert can be re-verified, causing add_expert_to_index to append a duplicate entry.

Additionally, get_total_experts returns the total index count (including duplicates), not the count of currently-verified experts, which is misleading.

Frontends iterating the index will need to filter both banned experts and duplicate entries client-side.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/identity-registry-contract/src/storage.rs` around lines 96 - 115,
The index is append-only and allows duplicates because re-verifying a previously
Banned expert will call add_expert_to_index unconditionally and
get_total_experts returns the append-only count; to fix, add an indexed-flag or
live-count and guard re-indexing: modify add_expert_to_index to first check a
per-address marker (e.g., a new DataKey like VerifiedIndexed(Address) or change
semantics to check current status via get(&DataKey::ExpertStatus(Address))) and
skip appending if already indexed, and update verify_expert and
batch_add_experts to only call add_expert_to_index when the previous status was
Unverified (not when Banned→Verified); also maintain a separate
DataKey::CurrentVerifiedCount (increment on Verified transition, decrement on
Ban transition) and change get_total_experts to return that live count instead
of the append-only TotalVerifiedCount so frontends won’t see duplicates.


/// Get the total number of verified experts ever indexed
pub fn get_total_experts(env: &Env) -> u64 {
env.storage()
.instance()
.get(&DataKey::TotalVerifiedCount)
.unwrap_or(0u64)
}

/// Get the expert address at the given index
pub fn get_expert_by_index(env: &Env, index: u64) -> Address {
env.storage()
.persistent()
.get(&DataKey::VerifiedExpertIndex(index))
.expect("Index out of bounds")
}
Comment on lines +126 to +131
Copy link

@coderabbitai coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing TTL extension on index entry reads — inconsistent with get_expert_record pattern.

get_expert_record (Line 72-76) extends TTL on reads to keep frequently-accessed entries alive. get_expert_by_index does not, meaning index entries can expire while the count still references them, leading to panics on valid indices.

♻️ Proposed fix 
 pub fn get_expert_by_index(env: &Env, index: u64) -> Address {
+    let key = DataKey::VerifiedExpertIndex(index);
+    if env.storage().persistent().has(&key) {
+        env.storage()
+            .persistent()
+            .extend_ttl(&key, LEDGERS_THRESHOLD, LEDGERS_EXTEND_TO);
+    }
     env.storage()
         .persistent()
-        .get(&DataKey::VerifiedExpertIndex(index))
+        .get(&key)
         .expect("Index out of bounds")
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/identity-registry-contract/src/storage.rs` around lines 126 - 131,
get_expert_by_index omits the TTL extension performed in get_expert_record, so
index entries can expire while still referenced; to fix, after retrieving the
Address in get_expert_by_index (DataKey::VerifiedExpertIndex(index)), re-apply
the same TTL-extension logic used by get_expert_record (i.e., use the same
persistent storage update/helper that refreshes the TTL for the key) so the
index entry's TTL is extended on each read and prevents out-of-bounds panics.

167 changes: 151 additions & 16 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 @@ -3,10 +3,12 @@
extern crate std;

use crate::error::RegistryError;
use crate::{IdentityRegistryContract, IdentityRegistryContractClient};
use crate::{storage, types::ExpertStatus};
use soroban_sdk::{Env, testutils::Address as _, Symbol, Address, IntoVal, TryIntoVal, vec, String};
use crate::{IdentityRegistryContract, IdentityRegistryContractClient};
use soroban_sdk::testutils::{AuthorizedFunction, AuthorizedInvocation, Events};
use soroban_sdk::{
testutils::Address as _, vec, Address, Env, IntoVal, String, Symbol, TryIntoVal,
};

#[test]
fn test_initialization() {
Expand Down Expand Up @@ -113,7 +115,12 @@ fn test_batch_verification_no_admin() {
let contract_id = env.register(IdentityRegistryContract, ());
let client = IdentityRegistryContractClient::new(&env, &contract_id);

let experts = vec![&env, Address::generate(&env), Address::generate(&env), Address::generate(&env)];
let experts = vec![
&env,
Address::generate(&env),
Address::generate(&env),
Address::generate(&env),
];

client.batch_add_experts(&experts);
}
Expand All @@ -135,16 +142,38 @@ fn test_batch_verification_check_status() {
let e4 = Address::generate(&env);
let e5 = Address::generate(&env);

let experts = vec![&env, e1.clone(), e2.clone(), e3.clone(), e4.clone(), e5.clone()];
let experts = vec![
&env,
e1.clone(),
e2.clone(),
e3.clone(),
e4.clone(),
e5.clone(),
];

client.batch_add_experts(&experts);

env.as_contract(&contract_id, ||{
assert_eq!(storage::get_expert_status(&env, &e1), ExpertStatus::Verified);
assert_eq!(storage::get_expert_status(&env, &e2), ExpertStatus::Verified);
assert_eq!(storage::get_expert_status(&env, &e3), ExpertStatus::Verified);
assert_eq!(storage::get_expert_status(&env, &e4), ExpertStatus::Verified);
assert_eq!(storage::get_expert_status(&env, &e5), ExpertStatus::Verified);
env.as_contract(&contract_id, || {
assert_eq!(
storage::get_expert_status(&env, &e1),
ExpertStatus::Verified
);
assert_eq!(
storage::get_expert_status(&env, &e2),
ExpertStatus::Verified
);
assert_eq!(
storage::get_expert_status(&env, &e3),
ExpertStatus::Verified
);
assert_eq!(
storage::get_expert_status(&env, &e4),
ExpertStatus::Verified
);
assert_eq!(
storage::get_expert_status(&env, &e5),
ExpertStatus::Verified
);
})
}

Expand All @@ -165,12 +194,32 @@ fn test_batch_verification_max_vec() {
let e3 = Address::generate(&env);
let e4 = Address::generate(&env);

let experts = vec![&env, e1.clone(), e2.clone(), e3.clone(), e4.clone(),
e1.clone(), e2.clone(), e3.clone(), e4.clone(),
e1.clone(), e2.clone(), e3.clone(), e4.clone(),
e1.clone(), e2.clone(), e3.clone(), e4.clone(),
e1.clone(), e2.clone(), e3.clone(), e4.clone(),
e1.clone(), e2.clone(), e3.clone(), e4.clone()
let experts = vec![
&env,
e1.clone(),
e2.clone(),
e3.clone(),
e4.clone(),
e1.clone(),
e2.clone(),
e3.clone(),
e4.clone(),
e1.clone(),
e2.clone(),
e3.clone(),
e4.clone(),
e1.clone(),
e2.clone(),
e3.clone(),
e4.clone(),
e1.clone(),
e2.clone(),
e3.clone(),
e4.clone(),
e1.clone(),
e2.clone(),
e3.clone(),
e4.clone(),
];

client.batch_add_experts(&experts);
Expand Down Expand Up @@ -446,3 +495,89 @@ fn test_getters() {
assert_eq!(client.is_verified(&expert), false);
assert_eq!(client.get_status(&expert), ExpertStatus::Banned);
}

#[test]
fn test_expert_directory_enumeration() {
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);
let expert1 = Address::generate(&env);
let expert2 = Address::generate(&env);
let expert3 = Address::generate(&env);

client.init(&admin);

// Verify 3 separate experts
let uri1 = String::from_str(&env, "ipfs://e1");
let uri2 = String::from_str(&env, "ipfs://e2");
let uri3 = String::from_str(&env, "ipfs://e3");
client.add_expert(&expert1, &uri1);
client.add_expert(&expert2, &uri2);
client.add_expert(&expert3, &uri3);

// Total should be 3
assert_eq!(client.get_total_experts(), 3u64);

// Indices 0, 1, 2 should return experts in chronological order
assert_eq!(client.get_expert_by_index(&0u64), expert1);
assert_eq!(client.get_expert_by_index(&1u64), expert2);
assert_eq!(client.get_expert_by_index(&2u64), expert3);
}

#[test]
fn test_expert_directory_no_duplicates_on_reverify() {
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);
let expert = Address::generate(&env);

client.init(&admin);

let uri = String::from_str(&env, "ipfs://expert");
client.add_expert(&expert, &uri);

// Total is 1
assert_eq!(client.get_total_experts(), 1u64);

// Re-verifying an already verified expert returns AlreadyVerified
let result = client.try_add_expert(&expert, &uri);
assert_eq!(result, Err(Ok(RegistryError::AlreadyVerified)));

// Total remains 1 — no duplicate in the index
assert_eq!(client.get_total_experts(), 1u64);
}
Comment on lines +531 to +556
Copy link

@coderabbitai coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing test for Banned→Verified re-verification — the scenario that triggers the duplicate index bug.

This test only covers the Verified→Verified path. Add a test that exercises the full lifecycle: verify → ban → re-verify, and asserts that the index count does not increase on re-verification. This is exactly the scenario where the duplicate-index bug identified in contract.rs would surface.

Suggested test 
#[test]
fn test_expert_directory_no_duplicates_on_reverify_after_ban() {
    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);
    let expert = Address::generate(&env);

    client.init(&admin);

    let uri = String::from_str(&env, "ipfs://expert");
    client.add_expert(&expert, &uri);
    assert_eq!(client.get_total_experts(), 1u64);

    // Ban the expert
    client.ban_expert(&expert);

    // Re-verify — should NOT create a duplicate index entry
    client.add_expert(&expert, &uri);
    assert_eq!(client.get_total_experts(), 1u64);
    assert_eq!(client.get_expert_by_index(&0u64), expert);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/identity-registry-contract/src/test.rs` around lines 531 - 556, Add
a new unit test that reproduces the Banned→Verified lifecycle to catch the
duplicate-index bug: create env, register IdentityRegistryContract and client,
init with admin, add_expert(&expert, &uri) and assert get_total_experts() == 1,
call ban_expert(&expert), then re-add via add_expert(&expert, &uri) and assert
get_total_experts() remains 1 and that get_expert_by_index(&0u64) returns the
original expert; name the test e.g.
test_expert_directory_no_duplicates_on_reverify_after_ban to mirror the existing
test and exercise the verify→ban→re-verify path.


#[test]
fn test_expert_directory_via_batch_add() {
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);
let expert1 = Address::generate(&env);
let expert2 = Address::generate(&env);
let expert3 = Address::generate(&env);

client.init(&admin);

let experts = vec![&env, expert1.clone(), expert2.clone(), expert3.clone()];
client.batch_add_experts(&experts);

// Total should be 3
assert_eq!(client.get_total_experts(), 3u64);

// Indices should map correctly
assert_eq!(client.get_expert_by_index(&0u64), expert1);
assert_eq!(client.get_expert_by_index(&1u64), expert2);
assert_eq!(client.get_expert_by_index(&2u64), expert3);
}