Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 1 addition & 1 deletion contracts/identity-registry-contract/src/events.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::types::ExpertStatus;
use soroban_sdk::{contracttype, Address, Env, Symbol, String};
use soroban_sdk::{contracttype, Address, Env, String, Symbol};

// The Event Data Structure
#[contracttype]
Expand Down
63 changes: 42 additions & 21 deletions contracts/payment-vault-contract/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use soroban_sdk::{Address, Env, token};
use crate::storage;
use crate::types::{BookingRecord, BookingStatus};
use crate::error::VaultError;
use crate::events;
use crate::storage;
use crate::types::{BookingRecord, BookingStatus};
use soroban_sdk::{token, Address, Env};

pub fn initialize_vault(
env: &Env,
admin: &Address,
token: &Address,
oracle: &Address
oracle: &Address,
) -> Result<(), VaultError> {
// 1. Check if already initialized
if storage::has_admin(env) {
Expand All @@ -23,13 +23,33 @@ pub fn initialize_vault(
Ok(())
}

pub fn pause(env: &Env) -> Result<(), VaultError> {
let admin = storage::get_admin(env).ok_or(VaultError::NotInitialized)?;
admin.require_auth();
storage::set_paused(env, true);
events::contract_paused(env, true);
Ok(())
}

pub fn unpause(env: &Env) -> Result<(), VaultError> {
let admin = storage::get_admin(env).ok_or(VaultError::NotInitialized)?;
admin.require_auth();
storage::set_paused(env, false);
events::contract_paused(env, false);
Ok(())
}
Comment on lines +26 to +40
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

pause/unpause should guard against redundant state transitions.

Calling pause() when the contract is already paused silently succeeds and emits a spurious paused=true event; unpause() when already unpaused does the same. This creates misleading audit trails for indexers and monitoring tools and gives callers no signal that the call was a no-op.

Consider returning an error (or at minimum skipping the event) when the requested state matches the current state:

🛡️ Proposed fix for idempotency guards
 pub fn pause(env: &Env) -> Result<(), VaultError> {
     let admin = storage::get_admin(env).ok_or(VaultError::NotInitialized)?;
     admin.require_auth();
+    if storage::is_paused(env) {
+        return Err(VaultError::ContractPaused); // already paused
+    }
     storage::set_paused(env, true);
     events::contract_paused(env, true);
     Ok(())
 }

 pub fn unpause(env: &Env) -> Result<(), VaultError> {
     let admin = storage::get_admin(env).ok_or(VaultError::NotInitialized)?;
     admin.require_auth();
+    if !storage::is_paused(env) {
+        return Err(VaultError::NotInitialized); // reuse or add a dedicated NotPaused variant
+    }
     storage::set_paused(env, false);
     events::contract_paused(env, false);
     Ok(())
 }

Alternatively, add a dedicated VaultError::NotPaused = 9 variant for clarity instead of reusing NotInitialized.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contracts/payment-vault-contract/src/contract.rs` around lines 26 - 40, The
pause/unpause functions should first read the current paused state (e.g. via
storage::is_paused(env) or storage::get_paused(env)) and if the requested state
equals the current state return a clear error (add VaultError::AlreadyPaused and
VaultError::AlreadyUnpaused or similar) instead of proceeding; update
contract::pause and contract::unpause to require auth, check the current state,
return the new error when redundant, and only call storage::set_paused(...) and
events::contract_paused(...) when an actual state change occurs.


pub fn book_session(
env: &Env,
user: &Address,
expert: &Address,
rate_per_second: i128,
max_duration: u64,
) -> Result<u64, VaultError> {
if storage::is_paused(env) {
return Err(VaultError::ContractPaused);
}

// Require authorization from the user creating the booking
user.require_auth();

Expand Down Expand Up @@ -84,13 +104,16 @@ pub fn finalize_session(
booking_id: u64,
actual_duration: u64,
) -> Result<(), VaultError> {
if storage::is_paused(env) {
return Err(VaultError::ContractPaused);
}

// 1. Require Oracle authorization
let oracle = storage::get_oracle(env);
oracle.require_auth();

// 2. Get booking and verify it exists
let booking = storage::get_booking(env, booking_id)
.ok_or(VaultError::BookingNotFound)?;
let booking = storage::get_booking(env, booking_id).ok_or(VaultError::BookingNotFound)?;

// 3. Verify booking is in Pending status
if booking.status != BookingStatus::Pending {
Expand Down Expand Up @@ -134,17 +157,16 @@ pub fn finalize_session(
/// 24 hours in seconds
const RECLAIM_TIMEOUT: u64 = 86400;

pub fn reclaim_stale_session(
env: &Env,
user: &Address,
booking_id: u64,
) -> Result<(), VaultError> {
pub fn reclaim_stale_session(env: &Env, user: &Address, booking_id: u64) -> Result<(), VaultError> {
if storage::is_paused(env) {
return Err(VaultError::ContractPaused);
}

// 1. Require user authorization
user.require_auth();

// 2. Get booking and verify it exists
let booking = storage::get_booking(env, booking_id)
.ok_or(VaultError::BookingNotFound)?;
let booking = storage::get_booking(env, booking_id).ok_or(VaultError::BookingNotFound)?;

// 3. Verify the caller is the booking owner
if booking.user != *user {
Expand Down Expand Up @@ -177,17 +199,16 @@ pub fn reclaim_stale_session(
Ok(())
}

pub fn reject_session(
env: &Env,
expert: &Address,
booking_id: u64,
) -> Result<(), VaultError> {
pub fn reject_session(env: &Env, expert: &Address, booking_id: u64) -> Result<(), VaultError> {
if storage::is_paused(env) {
return Err(VaultError::ContractPaused);
}

// 1. Require expert authorization
expert.require_auth();

// 2. Get booking and verify it exists
let booking = storage::get_booking(env, booking_id)
.ok_or(VaultError::BookingNotFound)?;
let booking = storage::get_booking(env, booking_id).ok_or(VaultError::BookingNotFound)?;

// 3. Verify the caller is the expert in the booking
if booking.expert != *expert {
Expand All @@ -212,4 +233,4 @@ pub fn reject_session(
events::session_rejected(env, booking_id, "Expert declined session");

Ok(())
}
}
3 changes: 2 additions & 1 deletion contracts/payment-vault-contract/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ pub enum VaultError {
BookingNotPending = 5,
InvalidAmount = 6,
ReclaimTooEarly = 7,
}
ContractPaused = 8,
}
19 changes: 16 additions & 3 deletions contracts/payment-vault-contract/src/events.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
use soroban_sdk::{Address, Env, symbol_short};
use soroban_sdk::{symbol_short, Address, Env};

/// Emitted when a new booking is created
pub fn booking_created(env: &Env, booking_id: u64, user: &Address, expert: &Address, deposit: i128) {
pub fn booking_created(
env: &Env,
booking_id: u64,
user: &Address,
expert: &Address,
deposit: i128,
) {
let topics = (symbol_short!("booked"), booking_id);
env.events().publish(topics, (user.clone(), expert.clone(), deposit));
env.events()
.publish(topics, (user.clone(), expert.clone(), deposit));
}

/// Emitted when a session is finalized
Expand All @@ -17,6 +24,12 @@ pub fn session_reclaimed(env: &Env, booking_id: u64, amount: i128) {
env.events().publish(topics, amount);
}

/// Emitted when the contract is paused or unpaused
pub fn contract_paused(env: &Env, paused: bool) {
let topics = (symbol_short!("paused"),);
env.events().publish(topics, paused);
}

/// Emitted when an expert rejects a pending session
pub fn session_rejected(env: &Env, booking_id: u64, reason: &str) {
let topics = (symbol_short!("reject"), booking_id);
Expand Down
24 changes: 16 additions & 8 deletions contracts/payment-vault-contract/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +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::VaultError;
use crate::types::BookingRecord;
use soroban_sdk::{contract, contractimpl, Address, Env, Vec};

#[contract]
pub struct PaymentVaultContract;
Expand All @@ -22,11 +22,23 @@ impl PaymentVaultContract {
env: Env,
admin: Address,
token: Address,
oracle: Address
oracle: Address,
) -> Result<(), VaultError> {
contract::initialize_vault(&env, &admin, &token, &oracle)
}

/// Pause the contract (Admin-only)
/// Halts all state-changing operations in an emergency
pub fn pause(env: Env) -> Result<(), VaultError> {
contract::pause(&env)
}

/// Unpause the contract (Admin-only)
/// Resumes normal contract operations
pub fn unpause(env: Env) -> Result<(), VaultError> {
contract::unpause(&env)
}

/// Book a session with an expert
/// User deposits tokens upfront based on rate_per_second * max_duration
pub fn book_session(
Expand Down Expand Up @@ -61,11 +73,7 @@ impl PaymentVaultContract {

/// Reject a pending session (Expert-only)
/// Experts can reject a pending booking, instantly refunding the user
pub fn reject_session(
env: Env,
expert: Address,
booking_id: u64,
) -> Result<(), VaultError> {
pub fn reject_session(env: Env, expert: Address, booking_id: u64) -> Result<(), VaultError> {
contract::reject_session(&env, &expert, booking_id)
}

Expand Down
25 changes: 20 additions & 5 deletions contracts/payment-vault-contract/src/storage.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
use soroban_sdk::{contracttype, Address, Env};
use crate::types::{BookingRecord, BookingStatus};
use soroban_sdk::{contracttype, Address, Env};

#[contracttype]
#[derive(Clone)]
pub enum DataKey {
Admin,
Token,
Oracle,
Booking(u64), // Booking ID -> BookingRecord
BookingCounter, // Counter for generating unique booking IDs
UserBookings(Address), // User Address -> Vec<u64> of booking IDs
Booking(u64), // Booking ID -> BookingRecord
BookingCounter, // Counter for generating unique booking IDs
UserBookings(Address), // User Address -> Vec<u64> of booking IDs
ExpertBookings(Address), // Expert Address -> Vec<u64> of booking IDs
IsPaused, // Circuit breaker flag
}

// --- Admin ---
Expand Down Expand Up @@ -45,6 +46,18 @@ pub fn get_oracle(env: &Env) -> Address {
env.storage().instance().get(&DataKey::Oracle).unwrap()
}

// --- Pause (Circuit Breaker) ---
pub fn set_paused(env: &Env, paused: bool) {
env.storage().instance().set(&DataKey::IsPaused, &paused);
}

pub fn is_paused(env: &Env) -> bool {
env.storage()
.instance()
.get(&DataKey::IsPaused)
.unwrap_or(false)
}

// --- Booking Counter ---
pub fn get_next_booking_id(env: &Env) -> u64 {
let current: u64 = env
Expand All @@ -53,7 +66,9 @@ pub fn get_next_booking_id(env: &Env) -> u64 {
.get(&DataKey::BookingCounter)
.unwrap_or(0);
let next = current + 1;
env.storage().instance().set(&DataKey::BookingCounter, &next);
env.storage()
.instance()
.set(&DataKey::BookingCounter, &next);
next
}

Expand Down
Loading