Merged
Conversation
Add helper function to check if an expert's status is Verified. Returns true only for Verified status, false for Unverified or Banned. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Expose is_verified as a public contract function that can be called without authentication, allowing external contracts and frontend to check expert verification status. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Test is_verified and get_status functions: - Verify is_verified returns false for random/unverified addresses - Verify is_verified returns true for verified experts - Verify is_verified returns false for banned experts Co-Authored-By: Claude Opus 4.5 <[email protected]>
📝 WalkthroughWalkthroughThis PR adds a read-only public getter Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@contracts/identity-registry-contract/src/test.rs`:
- Around line 334-359: The test currently calls env.mock_all_auths() up front
which hides whether getters require auth; update test_getters to perform the
unauthenticated checks first (call client.is_verified and client.get_status for
random_address and for expert before any mocking), then call
env.mock_all_auths() only when performing admin actions (wrap/mock only around
client.add_expert(&expert) and client.ban_expert(&expert) or call it immediately
before those calls), keeping admin initialization (client.init(&admin)) as
needed; ensure references to IdentityRegistryContractClient methods
client.is_verified, client.get_status, client.add_expert, and client.ban_expert
are used in that order so the test asserts no signature is required for getters.
Comment on lines
+334
to
+359
| #[test] | ||
| fn test_getters() { | ||
| 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); | ||
|
|
||
| // Test 1: Check is_verified on a random address (should be false) | ||
| let random_address = Address::generate(&env); | ||
| assert_eq!(client.is_verified(&random_address), false); | ||
| assert_eq!(client.get_status(&random_address), ExpertStatus::Unverified); | ||
|
|
||
| // Test 2: Verify an expert and check is_verified (should be true) | ||
| let expert = Address::generate(&env); | ||
| client.add_expert(&expert); | ||
| assert_eq!(client.is_verified(&expert), true); | ||
| assert_eq!(client.get_status(&expert), ExpertStatus::Verified); | ||
|
|
||
| // Test 3: Ban the expert and check is_verified (should be false) | ||
| client.ban_expert(&expert); | ||
| assert_eq!(client.is_verified(&expert), false); | ||
| assert_eq!(client.get_status(&expert), ExpertStatus::Banned); |
There was a problem hiding this comment.
Verify unauthenticated access explicitly in test_getters.
env.mock_all_auths() at the top can mask accidental auth requirements. To assert the “no signature required” behavior, run the getter checks before mocking auth, then mock only for admin calls.
🔧 Suggested test tweak
fn test_getters() {
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);
// Test 1: Check is_verified on a random address (should be false)
let random_address = Address::generate(&env);
assert_eq!(client.is_verified(&random_address), false);
assert_eq!(client.get_status(&random_address), ExpertStatus::Unverified);
// Test 2: Verify an expert and check is_verified (should be true)
let expert = Address::generate(&env);
+ env.mock_all_auths();
client.add_expert(&expert);
assert_eq!(client.is_verified(&expert), true);
assert_eq!(client.get_status(&expert), ExpertStatus::Verified);
// Test 3: Ban the expert and check is_verified (should be false)
client.ban_expert(&expert);
assert_eq!(client.is_verified(&expert), false);
assert_eq!(client.get_status(&expert), ExpertStatus::Banned);
}📝 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
| #[test] | |
| fn test_getters() { | |
| 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); | |
| // Test 1: Check is_verified on a random address (should be false) | |
| let random_address = Address::generate(&env); | |
| assert_eq!(client.is_verified(&random_address), false); | |
| assert_eq!(client.get_status(&random_address), ExpertStatus::Unverified); | |
| // Test 2: Verify an expert and check is_verified (should be true) | |
| let expert = Address::generate(&env); | |
| client.add_expert(&expert); | |
| assert_eq!(client.is_verified(&expert), true); | |
| assert_eq!(client.get_status(&expert), ExpertStatus::Verified); | |
| // Test 3: Ban the expert and check is_verified (should be false) | |
| client.ban_expert(&expert); | |
| assert_eq!(client.is_verified(&expert), false); | |
| assert_eq!(client.get_status(&expert), ExpertStatus::Banned); | |
| #[test] | |
| fn test_getters() { | |
| let env = Env::default(); | |
| let contract_id = env.register(IdentityRegistryContract, ()); | |
| let client = IdentityRegistryContractClient::new(&env, &contract_id); | |
| let admin = Address::generate(&env); | |
| env.mock_all_auths(); | |
| client.init(&admin); | |
| env.unmock_all_auths(); | |
| // Test 1: Check is_verified on a random address (should be false) | |
| let random_address = Address::generate(&env); | |
| assert_eq!(client.is_verified(&random_address), false); | |
| assert_eq!(client.get_status(&random_address), ExpertStatus::Unverified); | |
| // Test 2: Verify an expert and check is_verified (should be true) | |
| let expert = Address::generate(&env); | |
| env.mock_all_auths(); | |
| client.add_expert(&expert); | |
| env.unmock_all_auths(); | |
| assert_eq!(client.is_verified(&expert), true); | |
| assert_eq!(client.get_status(&expert), ExpertStatus::Verified); | |
| // Test 3: Ban the expert and check is_verified (should be false) | |
| env.mock_all_auths(); | |
| client.ban_expert(&expert); | |
| env.unmock_all_auths(); | |
| assert_eq!(client.is_verified(&expert), false); | |
| assert_eq!(client.get_status(&expert), ExpertStatus::Banned); | |
| } |
🤖 Prompt for AI Agents
In `@contracts/identity-registry-contract/src/test.rs` around lines 334 - 359, The
test currently calls env.mock_all_auths() up front which hides whether getters
require auth; update test_getters to perform the unauthenticated checks first
(call client.is_verified and client.get_status for random_address and for expert
before any mocking), then call env.mock_all_auths() only when performing admin
actions (wrap/mock only around client.add_expert(&expert) and
client.ban_expert(&expert) or call it immediately before those calls), keeping
admin initialization (client.init(&admin)) as needed; ensure references to
IdentityRegistryContractClient methods client.is_verified, client.get_status,
client.add_expert, and client.ban_expert are used in that order so the test
asserts no signature is required for getters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🧠 SkillSphere Pull Request 🌐
Mark with an
xall the checkboxes that apply (like[x])cargo test(All tests passed)📌 Type of Change
📝 Changes Description
Add read-only public getter functions to allow external contracts (like Payment Vault) and Frontend to check expert verification status without Admin permissions:
is_verified(env, expert) -> boolhelper functionis_verifiedas public function (no authentication required)test_getterscovering unverified, verified, and banned states📸 Evidence
🌌 Comments
is_verifiedreturnstrueonly forExpertStatus::Verified,falseforUnverifiedorBannedget_statusandis_verifiedcan be called without providing a signatureThank you for contributing to SkillSphere! 🌍
We are glad you have chosen to help us democratize access to knowledge on the Stellar network. Your contribution brings us one step closer to a trustless, peer-to-peer consulting economy. Let's build the future together! 🚀
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.