Implement "Expert Metadata" (Profile Data) #21
Conversation
📝 WalkthroughWalkthroughThis PR implements expert profile metadata storage by adding a Changes
Sequence DiagramssequenceDiagram
participant Expert as Expert (Address)
participant Contract as Contract
participant Storage as Storage
participant Events as Events
Expert->>Contract: update_profile(new_uri)
Contract->>Contract: expert.require_auth()
Contract->>Storage: get_expert_record(expert)
Storage-->>Contract: ExpertRecord {status, data_uri, ...}
alt status != Verified
Contract-->>Expert: Error: NotVerified
else length > limit
Contract-->>Expert: Error: UriTooLong
else Valid
Contract->>Storage: set_expert_record(expert, Verified, new_uri)
Storage->>Storage: persist updated record
Contract->>Events: emit_profile_updated(expert, new_uri)
Events->>Events: publish event
Contract-->>Expert: Ok()
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
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/events.rs`:
- Line 15: Remove the #[allow(deprecated)] suppression and refactor the events
to use the new #[contractevent] macro: annotate ExpertStatusChangedEvent with
#[contractevent(topics = ["status_change"])] and ProfileUpdatedEvent with
#[contractevent(topics = ["profile_updated"])], keep the same fields (Address,
ExpertStatus, etc.), then replace calls that used env.events().publish(...) with
the new pattern by constructing the event struct and calling .publish(env)
inside the existing emit functions (e.g., emit_status_change and
emit_profile_updated) so they accept &Env and call ExpertStatusChangedEvent {
... }.publish(env) and ProfileUpdatedEvent { ... }.publish(env) respectively.
🧹 Nitpick comments (3)
contracts/identity-registry-contract/src/test.rs (1)
51-77: Consider adding event verification.The test correctly verifies that the URI is updated in storage. However, the comment on line 76 states that event assertion is skipped to avoid flakiness. If the event emission is an important part of the contract behavior (which it appears to be based on
emit_profile_updatedbeing called inupdate_profile), consider adding event verification similar totest_expert_status_changed_event.💡 Optional: Add event verification
// Assert record updated env.as_contract(&contract_id, || { let rec = storage::get_expert_record(&env, &expert); assert_eq!(rec.data_uri, uri2); }); - // Event assertion skipped to avoid flakiness in event buffers + // Verify profile_updated event was emitted + let events = env.events().all(); + let event = events.last().unwrap(); + assert_eq!(event.0, contract_id); + let topic: Symbol = event.1.get(0).unwrap().try_into_val(&env).unwrap(); + assert_eq!(topic, Symbol::new(&env, "profile_updated")); }contracts/identity-registry-contract/src/contract.rs (2)
32-34: Hoist empty URI creation outside the loop.
Minor gas/alloc savings by creating once and cloning per iteration.♻️ Suggested tweak
- for expert in experts { + let empty_uri = String::from_str(&env, ""); + for expert in experts { let status = storage::get_expert_status(&env, &expert); if status == ExpertStatus::Verified { return Err(RegistryError::AlreadyVerified); } - // Default empty URI for batch adds - let empty_uri = String::from_str(&env, ""); - storage::set_expert_record(&env, &expert, ExpertStatus::Verified, empty_uri); + // Default empty URI for batch adds + storage::set_expert_record(&env, &expert, ExpertStatus::Verified, empty_uri.clone()); events::emit_status_change(&env, expert, status, ExpertStatus::Verified, admin.clone()); }
63-79: Centralize URI length limit and clarify thatString::len()returns byte count, not character count.Both
verify_expertandupdate_profileduplicate the 64-byte check. Define a constant at the module level and use it in both functions to keep them in sync. Note: the current comment says "~64 chars" butsoroban_sdk::String::len()returns the byte length of the underlyingu8array, so URIs with multi-byte UTF-8 characters may appear shorter in character count than the 64-byte limit.♻️ Suggested refactor
+const MAX_URI_LEN: u32 = 64; + pub fn verify_expert(env: &Env, expert: &Address, data_uri: String) -> Result<(), RegistryError> { @@ - if data_uri.len() > 64 { + if data_uri.len() > MAX_URI_LEN { return Err(RegistryError::UriTooLong); } @@ pub fn update_profile(env: &Env, expert: &Address, new_uri: String) -> Result<(), RegistryError> { @@ - if new_uri.len() > 64 { + if new_uri.len() > MAX_URI_LEN { return Err(RegistryError::UriTooLong); }
| } | ||
|
|
||
| // Helper function to emit the status change event | ||
| #[allow(deprecated)] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
soroban-sdk events publish deprecated 2025
💡 Result:
In Aug 2025, the Rust Soroban SDK (rs-soroban-sdk) deprecated the old Events::publish(...) API (i.e., env.events().publish(...)). The deprecation landed in PR #1524 (merged Aug 6, 2025) and is called out in the v23.0.0-rc.3 release notes (published Aug 28, 2025) as “Deprecate old events publish method”. [1][2]
Replacement (Protocol 23 / “Whisk” pattern): define events with the #[contractevent] macro and publish by calling publish(&env) on the event value/struct (so the event is included in the contract spec). [1][3]
Example from Stellar docs:
#[contractevent(topics = ["COUNTER", "increment"], data_format = "single-value")]
struct IncrementEvent { count: u32 }
IncrementEvent { count }.publish(&env);[3]
Why: the #[contractevent] approach is now preferred because it embeds event definitions into the contract spec, enabling downstream tooling/bindings for events. [1]
Sources:
[1] stellar/rs-soroban-sdk#1524
[2] https://github.com/stellar/rs-soroban-sdk/releases/tag/v23.0.0-rc.3
[3] https://developers.stellar.org/docs/build/smart-contracts/example-contracts/events
🏁 Script executed:
fd -e toml -type f | head -20Repository: LightForgeHub/SkillSphere-Contracts
Length of output: 249
🏁 Script executed:
find . -name "Cargo.toml" -type fRepository: LightForgeHub/SkillSphere-Contracts
Length of output: 187
🏁 Script executed:
cat -n Cargo.toml 2>/dev/null || cat -n contracts/Cargo.toml 2>/dev/null || find . -name "Cargo.toml" -exec cat -n {} \;Repository: LightForgeHub/SkillSphere-Contracts
Length of output: 743
🏁 Script executed:
cat -n contracts/identity-registry-contract/src/events.rsRepository: LightForgeHub/SkillSphere-Contracts
Length of output: 1665
Refactor event emission to use the modern #[contractevent] macro pattern.
The env.events().publish() API was deprecated in soroban-sdk v23.0.0-rc.3 and is replaced by the #[contractevent] macro pattern. Your project uses soroban-sdk 23.1.1, where this deprecation is active. Remove the #[allow(deprecated)] suppression and refactor both ExpertStatusChangedEvent and ProfileUpdatedEvent to use #[contractevent]:
#[contractevent(topics = ["status_change"])]
pub struct ExpertStatusChangedEvent {
pub expert: Address,
pub old_status: ExpertStatus,
pub new_status: ExpertStatus,
pub admin: Address,
}
pub fn emit_status_change(env: &Env, expert: Address, old_status: ExpertStatus, new_status: ExpertStatus, admin: Address) {
ExpertStatusChangedEvent { expert, old_status, new_status, admin }.publish(env);
}This approach embeds event definitions in the contract spec and enables downstream tooling. Apply the same pattern to ProfileUpdatedEvent with topics = ["profile_updated"].
🤖 Prompt for AI Agents
In `@contracts/identity-registry-contract/src/events.rs` at line 15, Remove the
#[allow(deprecated)] suppression and refactor the events to use the new
#[contractevent] macro: annotate ExpertStatusChangedEvent with
#[contractevent(topics = ["status_change"])] and ProfileUpdatedEvent with
#[contractevent(topics = ["profile_updated"])], keep the same fields (Address,
ExpertStatus, etc.), then replace calls that used env.events().publish(...) with
the new pattern by constructing the event struct and calling .publish(env)
inside the existing emit functions (e.g., emit_status_change and
emit_profile_updated) so they accept &Env and call ExpertStatusChangedEvent {
... }.publish(env) and ProfileUpdatedEvent { ... }.publish(env) respectively.
2 participants
🧠 SkillSphere Pull Request 🌐
Mark with an
xall the checkboxes that apply (like[x])cargo test(All tests passed)📌 Type of Change
📝 Changes Description
Added a gas-efficient way to associate off-chain profile metadata with experts by storing only a short data_uri (capped at 64 chars) during verification and allowing verified experts to update it with auth. This keeps on-chain data minimal (one small string per expert), defaults to an empty string in batch adds, and preserves existing URIs on bans.
Impact on storage/gas: minimal increase due to a short string field in ExpertRecord and occasional small event on profile updates; overall design avoids storing large text on-chain to remain cost effective.
📸 Evidence
(A photo, log output, or Soroban CLI output is required as evidence. If this is a smart contract change, please include the test results or invocation logs.)
🌌 Comments
(Any extra notes for the reviewer? Are there any specific edge cases they should test?)
Thank 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
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.