Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
53 changes: 45 additions & 8 deletions contracts/identity-registry-contract/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::storage;
use crate::events;
use crate::{error::RegistryError, types::ExpertStatus};

/// Initialize the registry with an admin address
pub fn initialize_registry(env: &Env, admin: &Address) -> Result<(), RegistryError> {
if storage::has_admin(env) {
return Err(RegistryError::AlreadyInitialized);
Expand All @@ -13,6 +14,7 @@ pub fn initialize_registry(env: &Env, admin: &Address) -> Result<(), RegistryErr
Ok(())
}

/// 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> {
Comment on lines +17 to 19
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟑 Minor

Fix the batch function doc comment.

The doc comment says β€œVerify an expert” but this function batch-verifies. Consider updating for accuracy.

πŸ€– Prompt for AI Agents
In `@contracts/identity-registry-contract/src/contract.rs` around lines 17 - 19,
Update the function doc comment to accurately describe batch_add_experts:
replace the singular "Verify an expert" line with a description indicating this
function batch-verifies multiple experts (admin-only), e.g., note that
batch_add_experts takes Env and Vec<Address> and sets multiple experts' status
to Verified, returning Result<(), RegistryError>; keep mention of admin-only and
Batch Verification.

if experts.len() > 20 {
Expand All @@ -36,18 +38,53 @@ pub fn batch_add_experts(env:Env, experts: Vec<Address>) -> Result<(), RegistryE

pub fn verify_expert(env: &Env, expert: &Address) -> Result<(), RegistryError> {
let admin = storage::get_admin(env).ok_or(RegistryError::NotInitialized)?;

admin.require_auth();

let current_status = storage::get_expert_status(env, expert);

if current_status == ExpertStatus::Verified {
return Err(RegistryError::AlreadyVerified);
}

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

events::emit_status_change(env, expert.clone(), current_status, ExpertStatus::Verified, admin);


events::emit_status_change(
env,
expert.clone(),
current_status,
ExpertStatus::Verified,
admin,
);

Ok(())
}
}

/// Ban an expert by setting their status to Banned (Admin only)
pub fn ban_expert(env: &Env, expert: &Address) -> Result<(), RegistryError> {
let admin = storage::get_admin(env).ok_or(RegistryError::NotInitialized)?;
admin.require_auth();

let current_status = storage::get_expert_status(env, expert);

if current_status == ExpertStatus::Banned {
return Err(RegistryError::AlreadyBanned);
}

storage::set_expert_record(env, expert, ExpertStatus::Banned);

events::emit_status_change(
env,
expert.clone(),
current_status,
ExpertStatus::Banned,
admin,
);

Ok(())
}
Comment on lines +64 to +85
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟑 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the verify_expert function implementation
cat -n contracts/identity-registry-contract/src/contract.rs | sed -n '1,80p'

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 2208


🏁 Script executed:

# Search for test files related to ban and verify operations
fd -e rs -path "*/test*" | head -20

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 247


🏁 Script executed:

# Look for any tests that mention ban_expert or the interaction between ban and verify
rg "ban_expert|Banned.*Verified|verify.*ban" -B 3 -A 3 --type rust

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 7289


🏁 Script executed:

# Check if there's a verify_expert function and see its implementation
ast-grep --pattern 'fn verify_expert($_) { $$$ }'

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 61


🏁 Script executed:

# Look for any comments, TODOs, or documentation about expert status handling
rg "TODO|FIXME|NOTE|preemptive|unbann" --type rust -i

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 61


🏁 Script executed:

# Search more thoroughly for tests that might cover re-verification of banned experts
cat -n contracts/identity-registry-contract/src/test.rs | head -300

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 9485


🏁 Script executed:

# Look for the complete test_ban_expert_workflow test
rg "test_ban_expert_workflow" -A 50 -B 5

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 4640


🏁 Script executed:

# Check ExpertStatus enum to understand all possible states
rg "enum ExpertStatus" -A 10

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 847


🏁 Script executed:

# Look for any documentation or comments about the intended behavior of ban/verify
rg "Ban|Verified|expert" contracts/identity-registry-contract/src/lib.rs -B 2 -A 2

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 600


🏁 Script executed:

# Check if there's any test for re-verifying a banned expert (verify after ban)
rg "ban_expert.*verify_expert|verify_expert.*ban_expert" -B 5 -A 5

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 61


🏁 Script executed:

# Search more comprehensively for any mention of re-verification after ban
rg "Verified.*Banned|Banned.*Verified" -B 3 -A 3

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 61


🏁 Script executed:

# Check if there's documentation or requirements about the design
find . -name "README*" -o -name "REQUIREMENTS*" -o -name "*.md" | head -10

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 125


🏁 Script executed:

# Look for any TODOs or FIXMEs related to ban/verify logic
rg "TODO|FIXME" contracts/identity-registry-contract/ -B 2 -A 2

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 61


🏁 Script executed:

# Check the README for design documentation or requirements
cat README.md

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 8315


🏁 Script executed:

# Look specifically for the PR template to understand requirements
cat ./.github/pull_request_template.md

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 1480


🏁 Script executed:

# Check if there's any test attempting to re-verify a banned expert
rg "ban_expert" -A 20 contracts/identity-registry-contract/src/test.rs | rg -A 15 "verify_expert|add_expert"

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 1605


🏁 Script executed:

# Look at complete_expert_lifecycle test to confirm it doesn't test re-verification after ban
cat -n contracts/identity-registry-contract/src/test.rs | sed -n '238,261p'

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 971


Add a check in verify_expert to prevent re-verifying banned experts.

The implementation currently allows re-verifying a Banned expert back to Verified status. The verify_expert function only guards against AlreadyVerified but does not check Banned status. This conflicts with the documented semantics that banning "revokes" verification. Add: if current_status == ExpertStatus::Banned { return Err(RegistryError::AlreadyBanned); } before the existing check, or introduce an explicit unban_expert function if unbanning should require separate authorization.

πŸ€– Prompt for AI Agents
In `@contracts/identity-registry-contract/src/contract.rs` around lines 41 - 62,
In verify_expert, add a guard to prevent re-verifying a banned expert: fetch
current_status via storage::get_expert_status(env, expert) (same as ban_expert
uses) and if current_status == ExpertStatus::Banned return
Err(RegistryError::AlreadyBanned) before the existing AlreadyVerified check;
this enforces that ban revokes verification (alternatively implement a separate
unban_expert with explicit authorization if unbanning is allowed).


/// Get the current status of an expert
pub fn get_expert_status(env: &Env, expert: &Address) -> ExpertStatus {
storage::get_expert_status(env, expert)
}
2 changes: 1 addition & 1 deletion contracts/identity-registry-contract/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ pub enum RegistryError {
AlreadyVerified = 5,
AlreadyBanned = 6,
ExpertVecMax = 7,
}
}
21 changes: 11 additions & 10 deletions contracts/identity-registry-contract/src/events.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use soroban_sdk::{contracttype, Address, Env, Symbol};
use crate::types::ExpertStatus;
use soroban_sdk::{contracttype, Address, Env, Symbol};

// The Event Data Structure
#[contracttype]
Expand All @@ -11,21 +11,22 @@ pub struct ExpertStatusChangedEvent {
pub admin: Address,
}

// Helper function to emit the event
// Helper function to emit the status change event
pub fn emit_status_change(
env: &Env,
expert: Address,
old_status: ExpertStatus,
new_status: ExpertStatus,
admin: Address
env: &Env,
expert: Address,
old_status: ExpertStatus,
new_status: ExpertStatus,
admin: Address,
) {
let event = ExpertStatusChangedEvent {
expert,
old_status,
new_status,
admin,
};

// We publish with the topic "status_change" so indexers can find it easily
env.events().publish((Symbol::new(env, "status_change"),), event);
}
env.events()
.publish((Symbol::new(env, "status_change"),), event);
}
17 changes: 14 additions & 3 deletions contracts/identity-registry-contract/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ mod contract;
mod error;
mod events;
mod storage;
mod types;
#[cfg(test)]
mod test;
mod types;

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

#[contract]
pub struct IdentityRegistryContract;
Expand All @@ -30,4 +31,14 @@ impl IdentityRegistryContract {
pub fn add_expert(env: Env, expert: Address) -> Result<(), RegistryError> {
contract::verify_expert(&env, &expert)
}

/// Ban an expert and revoke their verification status (Admin only)
pub fn ban_expert(env: Env, expert: Address) -> Result<(), RegistryError> {
contract::ban_expert(&env, &expert)
}

/// Get the current status of an expert
pub fn get_status(env: Env, expert: Address) -> ExpertStatus {
contract::get_expert_status(&env, &expert)
}
}
36 changes: 19 additions & 17 deletions contracts/identity-registry-contract/src/storage.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::types::{ExpertRecord, ExpertStatus};
use soroban_sdk::{contracttype, Address, Env};
use crate::types::{ExpertStatus, ExpertRecord};

// 1. Data Keys
#[contracttype]
#[derive(Clone)]
pub enum DataKey {
Admin,
Expert(Address),
Admin,
Expert(Address),
Comment on lines 7 to +9
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n contracts/identity-registry-contract/src/storage.rs | head -50

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 214


🏁 Script executed:

cat contracts/identity-registry-contract/src/storage.rs

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 2568


Extend admin instance TTL to prevent silent expiration.

Admin is stored in instance storage without TTL extension. Unlike expert records which correctly call extend_ttl() on both set and get operations, admin functions (has_admin, set_admin, get_admin) never bump the TTL. Once the admin expires, has_admin() silently returns false and the registry becomes unmanaged or can be re-initialized. Apply the same TTL extension pattern used for expert records:

TTL extension for admin storage
 pub fn has_admin(env: &Env) -> bool {
-    env.storage().instance().has(&DataKey::Admin)
+    let has = env.storage().instance().has(&DataKey::Admin);
+    if has {
+        env.storage().instance().extend_ttl(LEDGERS_THRESHOLD, LEDGERS_EXTEND_TO);
+    }
+    has
 }

 pub fn set_admin(env: &Env, admin: &Address) {
     env.storage().instance().set(&DataKey::Admin, admin);
+    env.storage().instance().extend_ttl(LEDGERS_THRESHOLD, LEDGERS_EXTEND_TO);
 }

 pub fn get_admin(env: &Env) -> Option<Address> {
-    env.storage().instance().get(&DataKey::Admin)
+    let admin = env.storage().instance().get(&DataKey::Admin);
+    if admin.is_some() {
+        env.storage().instance().extend_ttl(LEDGERS_THRESHOLD, LEDGERS_EXTEND_TO);
+    }
+    admin
 }
πŸ€– Prompt for AI Agents
In `@contracts/identity-registry-contract/src/storage.rs` around lines 7 - 9, The
admin record stored under DataKey::Admin currently never extends its instance
TTL, so modify has_admin, set_admin, and get_admin to call the same extend_ttl()
used for expert records: after successfully setting the admin in set_admin call
extend_ttl() to bump TTL; in get_admin and has_admin, if the admin is found
return the value and call extend_ttl() to refresh TTL (mirror the expert get/set
pattern). Locate DataKey::Admin and the functions has_admin, set_admin, and
get_admin and add the extend_ttl() invocation in the same spots and order as the
expert record code to ensure consistent TTL behavior.

}

// Constants for TTL (Time To Live)
Expand All @@ -23,23 +23,27 @@ const LEDGERS_EXTEND_TO: u32 = 6_300_000; // ~1 year

// ... [Admin Helpers] ...

/// Check if the admin has been set
pub fn has_admin(env: &Env) -> bool {
env.storage().instance().has(&DataKey::Admin)
}

/// Set the admin address
pub fn set_admin(env: &Env, admin: &Address) {
env.storage().instance().set(&DataKey::Admin, admin);
}

/// Get the admin address
pub fn get_admin(env: &Env) -> Option<Address> {
env.storage().instance().get(&DataKey::Admin)
}

// ... [Expert Helpers] ...

/// Set the expert record with status and timestamp
pub fn set_expert_record(env: &Env, expert: &Address, status: ExpertStatus) {
let key = DataKey::Expert(expert.clone());

let record = ExpertRecord {
status,
updated_at: env.ledger().timestamp(),
Expand All @@ -49,26 +53,23 @@ pub fn set_expert_record(env: &Env, expert: &Address, status: ExpertStatus) {
env.storage().persistent().set(&key, &record);

// 2. Extend the TTL
// This tells the network: "If this data is going to die in less than 2 months,
// This tells the network: "If this data is going to die in less than 2 months,
// extend its life to 1 full year from now."
env.storage().persistent().extend_ttl(
&key,
LEDGERS_THRESHOLD,
LEDGERS_EXTEND_TO
);
env.storage()
.persistent()
.extend_ttl(&key, LEDGERS_THRESHOLD, LEDGERS_EXTEND_TO);
}

/// Get the expert record, extending TTL if exists
pub fn get_expert_record(env: &Env, expert: &Address) -> ExpertRecord {
let key = DataKey::Expert(expert.clone());

// We also bump TTL on reads
// If an expert is being checked frequently, they should stay alive.
if env.storage().persistent().has(&key) {
env.storage().persistent().extend_ttl(
&key,
LEDGERS_THRESHOLD,
LEDGERS_EXTEND_TO
);
env.storage()
.persistent()
.extend_ttl(&key, LEDGERS_THRESHOLD, LEDGERS_EXTEND_TO);
}

env.storage()
Expand All @@ -80,6 +81,7 @@ pub fn get_expert_record(env: &Env, expert: &Address) -> ExpertRecord {
})
}

/// Get the expert status
pub fn get_expert_status(env: &Env, expert: &Address) -> ExpertStatus {
get_expert_record(env, expert).status
}
}
Loading