Skip to content

feat: submit review logic #61

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
Luluameh merged 1 commit into from
Aug 20, 2026
Merged

feat: submit review logic #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 75 additions & 1 deletion contracts/reputation-scoring-contract/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::error::ReputationError;
use crate::events;
use crate::storage;
use soroban_sdk::{Address, BytesN, Env};
use crate::types::{BookingRecord, BookingStatus, ExpertStats, ReviewRecord};
use soroban_sdk::{Address, BytesN, Env, IntoVal, Symbol};

pub fn initialize(
env: &Env,
Expand Down Expand Up @@ -79,3 +80,76 @@ pub fn penalize_expert(

Ok(())
}

pub fn submit_review(
env: &Env,
reviewer: &Address,
booking_id: u64,
score: u32,
) -> Result<(), ReputationError> {
storage::get_admin(env).ok_or(ReputationError::NotInitialized)?;

if storage::is_paused(env) {
return Err(ReputationError::ContractPaused);
}

reviewer.require_auth();

// Validate score 1–5
if score < 1 || score > 5 {
return Err(ReputationError::InvalidScore);
}

// Prevent duplicate reviews
if storage::has_review(env, booking_id) {
return Err(ReputationError::AlreadyReviewed);
}

// Cross-contract call to vault: get_booking
let vault_address = storage::get_vault_address(env)
.ok_or(ReputationError::NotInitialized)?;

let booking: BookingRecord = env.invoke_contract(
&vault_address,
&Symbol::new(env, "get_booking"),
soroban_sdk::vec![env, booking_id.into_val(env)],
);
Comment thread
arandomogg marked this conversation as resolved.

// Check booking is Complete
if booking.status != BookingStatus::Complete {
return Err(ReputationError::BookingNotComplete);
}

// Verify reviewer is the booking user
if *reviewer != booking.user {
return Err(ReputationError::NotBookingUser);
}

// Store review
let review = ReviewRecord {
booking_id,
reviewer: reviewer.clone(),
expert: booking.expert.clone(),
score,
timestamp: env.ledger().timestamp(),
};
storage::set_review(env, booking_id, &review);

// Update expert stats
let mut stats = storage::get_expert_stats(env, &booking.expert);
stats.total_score += score as u64;
stats.review_count += 1;
storage::set_expert_stats(env, &booking.expert, &stats);

events::review_submitted(env, booking_id, reviewer, &booking.expert, score);

Ok(())
}

pub fn get_review(env: &Env, booking_id: u64) -> Option<ReviewRecord> {
storage::get_review(env, booking_id)
}

pub fn get_expert_stats(env: &Env, expert: &Address) -> ExpertStats {
storage::get_expert_stats(env, expert)
}
4 changes: 4 additions & 0 deletions contracts/reputation-scoring-contract/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,9 @@ pub enum ReputationError {
NotInitialized = 1,
AlreadyInitialized = 2,
ContractPaused = 3,
InvalidScore = 4,
BookingNotComplete = 5,
AlreadyReviewed = 6,
NotBookingUser = 7,
Comment thread
arandomogg marked this conversation as resolved.
NotAuthorized = 8,
}
21 changes: 17 additions & 4 deletions contracts/reputation-scoring-contract/src/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,23 @@ pub fn admin_transferred(env: &Env, old_admin: &Address, new_admin: &Address) {
env.events()
.publish(topics, (old_admin.clone(), new_admin.clone()));
}

/// Emitted when an expert is penalized following a dispute resolution.
pub fn expert_penalized(env: &Env, expert: &Address, penalty_points: u64, new_score: u64) {
let topics = (symbol_short!("penalized"),);
env.events().publish(
topics,
(expert.clone(), penalty_points, new_score),
);
env.events()
.publish(topics, (expert.clone(), penalty_points, new_score));
}

/// Emitted when a review is submitted.
pub fn review_submitted(
env: &Env,
booking_id: u64,
reviewer: &Address,
expert: &Address,
score: u32,
) {
let topics = (symbol_short!("review"),);
env.events()
.publish(topics, (booking_id, reviewer.clone(), expert.clone(), score));
}
18 changes: 18 additions & 0 deletions contracts/reputation-scoring-contract/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mod test;
mod types;

use crate::error::ReputationError;
use crate::types::{ExpertStats, ReviewRecord};
use soroban_sdk::{contract, contractimpl, Address, BytesN, Env};

#[contract]
Expand Down Expand Up @@ -43,4 +44,21 @@ impl ReputationScoringContract {
) -> Result<(), ReputationError> {
contract::penalize_expert(&env, &expert, penalty_points)
}

pub fn submit_review(
env: Env,
reviewer: Address,
booking_id: u64,
score: u32,
) -> Result<(), ReputationError> {
contract::submit_review(&env, &reviewer, booking_id, score)
}
Comment thread
arandomogg marked this conversation as resolved.
Comment on lines +48 to +55

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

submit_review still doesn't accept expert/review_uri; previously-agreed fix wasn't applied.

Issue #54 requires validating that the "supplied user and expert match the booking" and submitting a review URI, but this entrypoint only accepts reviewer, booking_id, score. The author previously confirmed this would be updated across lib.rs, contract.rs, types.rs (ReviewRecord.review_uri), and events.rs (review_submitted payload), but none of those changes are present in the current diff.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/reputation-scoring-contract/src/lib.rs` around lines 48 - 55,
Update the submit_review entrypoint and its contract::submit_review flow to
accept both the supplied user/reviewer and expert plus review_uri, then validate
that the supplied parties match the booking before recording the review. Extend
types.rs ReviewRecord with review_uri and update events.rs review_submitted to
include the URI, propagating the new data through storage and event emission
consistently.


pub fn get_review(env: Env, booking_id: u64) -> Option<ReviewRecord> {
contract::get_review(&env, booking_id)
}

pub fn get_expert_stats(env: Env, expert: Address) -> ExpertStats {
contract::get_expert_stats(&env, &expert)
}
}
53 changes: 51 additions & 2 deletions contracts/reputation-scoring-contract/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::types::{ExpertStats, ReviewRecord};
use soroban_sdk::{contracttype, Address, Env};

#[contracttype]
Expand All @@ -8,8 +9,12 @@ pub enum DataKey {
IsPaused,
ExpertScore(Address),
ExpertReviews(Address),
Review(u64), // booking_id → ReviewRecord
ExpertStats(Address), // expert → ExpertStats
}

// --- Admin ---

pub fn has_admin(env: &Env) -> bool {
env.storage().instance().has(&DataKey::Admin)
}
Expand All @@ -22,10 +27,18 @@ pub fn get_admin(env: &Env) -> Option<Address> {
env.storage().instance().get(&DataKey::Admin)
}

// --- Vault ---

pub fn set_vault_address(env: &Env, vault: &Address) {
env.storage().instance().set(&DataKey::VaultAddress, vault);
}

pub fn get_vault_address(env: &Env) -> Option<Address> {
env.storage().instance().get(&DataKey::VaultAddress)
}

// --- Pause ---

pub fn is_paused(env: &Env) -> bool {
env.storage()
.instance()
Expand All @@ -37,6 +50,8 @@ pub fn set_paused(env: &Env, paused: bool) {
env.storage().instance().set(&DataKey::IsPaused, &paused);
}

// --- Expert Score (dispute penalties) ---

pub fn get_expert_score(env: &Env, expert: &Address) -> u64 {
env.storage()
.instance()
Expand All @@ -63,6 +78,40 @@ pub fn set_expert_reviews(env: &Env, expert: &Address, count: u64) {
.set(&DataKey::ExpertReviews(expert.clone()), &count);
}
Comment on lines 53 to 79

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## Locate storage.rs"
fd -a 'storage\.rs$' . | sed 's#^\./##'

echo
echo "## Outline storage.rs"
ast-grep outline contracts/reputation-scoring-contract/src/storage.rs --view compact || true

echo
echo "## Relevant section"
sed -n '1,130p' contracts/reputation-scoring-contract/src/storage.rs

echo
echo "## DataKey definitions"
rg -n "enum DataKey|ExpertScore|ExpertReviews|Review|ExpertStats|instance|persistent" contracts/reputation-scoring-contract/src/storage.rs

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 5296

🌐 Web query:

Soroban instance storage single ledger entry per contract persistent storage unbounded user balances docs

💡 Result:

In Soroban, Instance storage is physically stored within a single ledger entry associated with the contract instance [1][2]. Because it resides in this shared entry, it is subject to a hard size limit defined by the network's maximum ledger entry size (typically in the order of 100 KB) [1][3][4]. Due to this structural limitation, Instance storage is not suitable for data that can grow in an unbounded fashion, such as user balances or large lists of records [3][4][5]. Attempting to store unbounded data in Instance storage can lead to Denial of Service (DoS) risks where the contract becomes unable to function once the ledger entry reaches its capacity [5]. For unbounded data like user balances, Persistent storage should be used instead [6][5]. Unlike Instance storage, Persistent storage allows each key-value pair to reside in its own separate ledger entry [6][7]. This design provides effectively unbounded aggregate capacity because each entry has its own independent lifetime (TTL) and capacity limit [6][7]. Using separate entries also avoids the performance overhead of loading the entire Instance storage map on every contract invocation, as the host only loads the specific entry required for the transaction [3][4][7].

Citations:

Move per-expert scoring/counters to persistent storage.

ExpertScore(Address) and ExpertReviews(Address) can grow with the number of experts, so storing them in instance storage loads a shared 100 KB ledger entry on every call and can hit instance-storage size limits. Review and ExpertStats already use persistent storage here; use the same approach for these per-expert values.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/reputation-scoring-contract/src/storage.rs` around lines 53 - 79,
Update get_expert_score, set_expert_score, get_expert_reviews, and
set_expert_reviews to use env.storage().persistent() instead of instance() for
the per-expert DataKey::ExpertScore and DataKey::ExpertReviews entries, matching
the storage approach used by Review and ExpertStats.


pub fn get_vault_address(env: &Env) -> Option<Address> {
env.storage().instance().get(&DataKey::VaultAddress)
// --- Reviews ---

pub fn has_review(env: &Env, booking_id: u64) -> bool {
env.storage()
.persistent()
.has(&DataKey::Review(booking_id))
}

pub fn set_review(env: &Env, booking_id: u64, review: &ReviewRecord) {
env.storage()
.persistent()
.set(&DataKey::Review(booking_id), review);
}

pub fn get_review(env: &Env, booking_id: u64) -> Option<ReviewRecord> {
env.storage()
.persistent()
.get(&DataKey::Review(booking_id))
}

// --- Expert Stats ---

pub fn get_expert_stats(env: &Env, expert: &Address) -> ExpertStats {
env.storage()
.persistent()
.get(&DataKey::ExpertStats(expert.clone()))
.unwrap_or(ExpertStats {
total_score: 0,
review_count: 0,
})
}

pub fn set_expert_stats(env: &Env, expert: &Address, stats: &ExpertStats) {
env.storage()
.persistent()
.set(&DataKey::ExpertStats(expert.clone()), stats);
}
Loading
Loading