Skip to content

Conversation

@KevinMB0220
Copy link
Contributor

@KevinMB0220 KevinMB0220 commented Jan 23, 2026

🧠 SkillSphere Pull Request 🌐

Mark with an x all the checkboxes that apply (like [x])

  • Closes Implement Batch Banning #5
  • Added tests (if necessary)
  • Ran cargo test (All tests passed)
  • Evidence attached (Screenshots, Logs, or Transaction Hashes)
  • Commented the code

📌 Type of Change

  • 📚 Documentation (updates to README, docs, or comments)
  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ Enhancement (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🏗️ Refactor (code improvement/cleanup without logical changes)

📝 Changes Description

Implemented batch banning functionality for the Identity Registry contract, allowing admins to ban multiple experts in a single transaction.

🔑 Changes Made

1️⃣ Batch Ban Logic (src/contract.rs)

  • Added batch_ban_experts function (lines 39–58)
  • Accepts a vector of expert addresses to ban
  • Validates a maximum batch size of 20 (gas safety)
  • Enforces admin-only authorization via admin.require_auth()
  • Iterates through each expert:
    • Ensures the expert is not already banned
    • Updates status to ExpertStatus::Banned
    • Emits an ExpertStatusChanged event per expert

2️⃣ Public API Exposure (src/lib.rs)

  • Exposed batch_ban_experts as part of the contract’s public interface
  • Follows the same pattern as the existing batch_add_experts function

⚙️ Technical Details

  • Gas Optimization
    • Single admin authentication for the entire batch
  • Safety Checks
    • Fails if batch size exceeds 20 (RegistryError::ExpertVecMax)
    • Fails atomically if any expert is already banned (RegistryError::AlreadyBanned)
  • Event Emission
    • Emits ExpertStatusChanged for each expert with:
      • Expert address
      • Previous status
      • New status (Banned)
      • Admin address

📊 Impact

  • Storage: No new storage keys introduced (reuses existing expert records)
  • Gas: More efficient than repeated ban_expert calls
  • Frontend: Events enable real-time UI updates for each banned expert

📸 Evidence

✅ Build Output

  • Build: Successful
  • Target: wasm32-unknown-unknown
  • Wasm Hash:
    10c7ceb88c70aad633f693e6fa18dfa9b015d8ba6d509220e90bbdcdefe59363

📦 Exported Functions

  • add_expert
  • ban_expert
  • batch_add_experts
  • batch_ban_expertsNEW
  • get_status
  • init
  • is_verified

🧪 Function Implementation

Location: contracts/identity-registry-contract/src/contract.rs:39–58

/// Batch ban experts by setting their status to Banned (Admin only)
pub fn batch_ban_experts(env: Env, experts: Vec<Address>) -> Result<(), RegistryError> {
    if experts.len() > 20 {
        return Err(RegistryError::ExpertVecMax);
    }

    let admin = storage::get_admin(&env).ok_or(RegistryError::NotInitialized)?;
    admin.require_auth();

    for expert in experts {
        let status = storage::get_expert_status(&env, &expert);
        if status == ExpertStatus::Banned {
            return Err(RegistryError::AlreadyBanned);
        }

        storage::set_expert_record(&env, &expert, ExpertStatus::Banned);
        events::emit_status_change(
            &env,
            expert,
            status,
            ExpertStatus::Banned,
            admin.clone(),
        );
    }

    Ok(())
}

🌌 Comments

✅ Acceptance Criteria (Issue #5)

  • Admin can ban 10+ experts in a single transaction (up to 20)
  • All targeted experts immediately return ExpertStatus::Banned
  • Events are emitted for every banned expert (frontend sync)

📝 Implementation Notes

  • Mirrors the batch_add_experts pattern for consistency
  • The 20-expert limit aligns with Soroban gas constraints
  • Transaction is fully atomic: if one expert fails, no state changes are applied

🔍 Suggested Tests (Future Work)

  • Batch banning exactly 20 experts
  • Failure when batch size exceeds 20
  • Failure when any expert is already banned
  • Event emission validation
  • Unauthorized access attempt

👀 Reviewer Checklist

  • Validate batch size enforcement
  • Confirm atomic failure behavior
  • Verify admin-only access control
  • Check event emission per expert
  • Review gas usage for large batches

Thank you for contributing to SkillSphere! 🌍
Together, we’re building a trustless, peer-to-peer consulting economy on Stellar. 🚀

Summary by CodeRabbit

  • New Features

    • Batch expert banning capability with a maximum of 20 experts per operation, enabling efficient bulk administrative management.
    • Validation to prevent duplicate bans and maintain registry data consistency and integrity.
  • Improvements

    • Mandatory admin authentication required for all expert status modification operations.
    • Enhanced event tracking system with historical status references for complete audit trails.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

📝 Walkthrough

Walkthrough

Added a new batch_ban_experts function to the identity registry contract that enables bulk expert status updates to Banned with admin-only access, enforcing a 20-expert maximum per transaction and emitting status-change events for each ban operation.

Changes

Cohort / File(s) Summary
Core Implementation
contracts/identity-registry-contract/src/contract.rs
Added batch_ban_experts function with admin authentication, 20-expert vector size validation, and status update logic that emits status-change events for each banned expert. Also updated verify_expert to include admin authentication and enhanced event emission with previous status tracking.
Public Interface
contracts/identity-registry-contract/src/lib.rs
Added public batch_ban_experts method to IdentityRegistryContract impl that delegates to the contract implementation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • Bosun-Josh121

Poem

🐰 A ban in bulk, so swift and neat,
Twenty experts, one heartbeat!
Events announce each status change,
Admin power—organized, not strange. 🚫✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(identity-registry): update contract logic and public API' accurately captures the main change: adding batch banning functionality to the Identity Registry contract.
Description check ✅ Passed The PR description follows the template with all required sections completed: linked issue, type of change (Enhancement), detailed changes, technical details, evidence of successful build, and test recommendations.
Linked Issues check ✅ Passed The implementation fully satisfies issue #5 requirements: batch_ban_experts function accepts a vector of addresses, enforces 20-expert limit, requires admin authentication, checks for already-banned experts atomically, updates status to Banned, and emits ExpertStatusChanged events for each expert.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing batch banning: the new batch_ban_experts function in contract.rs and its public API exposure in lib.rs. The verify_expert flow update mentioned in the summary aligns with maintaining event emission consistency across the codebase.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Bosun-Josh121 Bosun-Josh121 merged commit 83079f3 into LightForgeHub:main Jan 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Batch Banning

2 participants