Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
32 changes: 32 additions & 0 deletions contracts/payment-vault-contract/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,22 @@ 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

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 set_my_rate(env: &Env, expert: &Address, rate_per_second: i128) -> Result<(), VaultError> {
expert.require_auth();

Expand All @@ -42,6 +58,10 @@ pub fn book_session(
expert: &Address,
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 @@ -100,6 +120,10 @@ 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();
Expand Down Expand Up @@ -150,6 +174,10 @@ pub fn finalize_session(
const RECLAIM_TIMEOUT: u64 = 86400;

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();

Expand Down Expand Up @@ -188,6 +216,10 @@ pub fn reclaim_stale_session(env: &Env, user: &Address, booking_id: u64) -> Resu
}

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();

Expand Down
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,5 +11,6 @@ pub enum VaultError {
BookingNotPending = 5,
InvalidAmount = 6,
ReclaimTooEarly = 7,
ExpertRateNotSet = 8,
ContractPaused = 8,
ExpertRateNotSet = 9,
}
6 changes: 6 additions & 0 deletions contracts/payment-vault-contract/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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
12 changes: 12 additions & 0 deletions contracts/payment-vault-contract/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ impl PaymentVaultContract {
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)
}

/// Set an expert's own rate per second
pub fn set_my_rate(env: Env, expert: Address, rate_per_second: i128) -> Result<(), VaultError> {
contract::set_my_rate(&env, &expert, rate_per_second)
Expand Down
13 changes: 13 additions & 0 deletions contracts/payment-vault-contract/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub enum DataKey {
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
ExpertRate(Address), // Expert Address -> rate per second (i128)
}

Expand Down Expand Up @@ -46,6 +47,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 Down
211 changes: 210 additions & 1 deletion contracts/payment-vault-contract/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,8 @@ fn test_reject_nonexistent_booking() {
assert!(result.is_err());
}

// ==================== Expert Rate Tests ====================

#[test]
fn test_expert_can_set_and_update_rate() {
let env = Env::default();
Expand Down Expand Up @@ -749,7 +751,7 @@ fn test_book_session_calculates_correct_deposit() {
let max_duration = 100_u64;
let expected_deposit = stored_rate * (max_duration as i128); // 1500 tokens

let booking_id = client.book_session(&user, &expert, &max_duration);
let _booking_id = client.book_session(&user, &expert, &max_duration);

// Verify correct deposit was extracted
assert_eq!(token.balance(&user), initial_balance - expected_deposit);
Expand Down Expand Up @@ -781,3 +783,210 @@ fn test_book_session_fails_if_expert_rate_not_set() {

assert!(res.is_err());
}

// ==================== Pausability (Circuit Breaker) Tests ====================

#[test]
fn test_pause_blocks_book_session() {
let env = Env::default();
env.mock_all_auths();

let admin = Address::generate(&env);
let user = Address::generate(&env);
let expert = Address::generate(&env);
let oracle = Address::generate(&env);

let token_admin = Address::generate(&env);
let token = create_token_contract(&env, &token_admin);
token.mint(&user, &10_000);

let client = create_client(&env);
client.init(&admin, &token.address, &oracle);

// Set expert rate before pausing
client.set_my_rate(&expert, &10_i128);

// Admin pauses the contract
let result = client.try_pause();
assert!(result.is_ok());

// User tries to book a session while paused (should fail)
let result = client.try_book_session(&user, &expert, &100);
assert!(result.is_err());

// Verify user's balance is unchanged
assert_eq!(token.balance(&user), 10_000);
}

#[test]
fn test_pause_blocks_finalize_session() {
let env = Env::default();
env.mock_all_auths();

let admin = Address::generate(&env);
let user = Address::generate(&env);
let expert = Address::generate(&env);
let oracle = Address::generate(&env);

let token_admin = Address::generate(&env);
let token = create_token_contract(&env, &token_admin);
token.mint(&user, &10_000);

let client = create_client(&env);
client.init(&admin, &token.address, &oracle);

// Book session while unpaused
let booking_id = {
client.set_my_rate(&expert, &10_i128);
client.book_session(&user, &expert, &100)
};

// Admin pauses the contract
client.pause();

// Oracle tries to finalize while paused (should fail)
let result = client.try_finalize_session(&booking_id, &50);
assert!(result.is_err());
}

#[test]
fn test_pause_blocks_reclaim_stale_session() {
let env = Env::default();
env.mock_all_auths();

let admin = Address::generate(&env);
let user = Address::generate(&env);
let expert = Address::generate(&env);
let oracle = Address::generate(&env);

let token_admin = Address::generate(&env);
let token = create_token_contract(&env, &token_admin);
token.mint(&user, &10_000);

let client = create_client(&env);
client.init(&admin, &token.address, &oracle);

// Book session while unpaused
let booking_id = {
client.set_my_rate(&expert, &10_i128);
client.book_session(&user, &expert, &100)
};

// Advance time past reclaim timeout
env.ledger()
.set_timestamp(env.ledger().timestamp() + 90_000);

// Admin pauses the contract
client.pause();

// User tries to reclaim while paused (should fail)
let result = client.try_reclaim_stale_session(&user, &booking_id);
assert!(result.is_err());
}

#[test]
fn test_pause_blocks_reject_session() {
let env = Env::default();
env.mock_all_auths();

let admin = Address::generate(&env);
let user = Address::generate(&env);
let expert = Address::generate(&env);
let oracle = Address::generate(&env);

let token_admin = Address::generate(&env);
let token = create_token_contract(&env, &token_admin);
token.mint(&user, &10_000);

let client = create_client(&env);
client.init(&admin, &token.address, &oracle);

// Book session while unpaused
let booking_id = {
client.set_my_rate(&expert, &10_i128);
client.book_session(&user, &expert, &100)
};

// Admin pauses the contract
client.pause();

// Expert tries to reject while paused (should fail)
let result = client.try_reject_session(&expert, &booking_id);
assert!(result.is_err());
}

#[test]
fn test_unpause_resumes_operations() {
let env = Env::default();
env.mock_all_auths();

let admin = Address::generate(&env);
let user = Address::generate(&env);
let expert = Address::generate(&env);
let oracle = Address::generate(&env);

let token_admin = Address::generate(&env);
let token = create_token_contract(&env, &token_admin);
token.mint(&user, &10_000);

let client = create_client(&env);
client.init(&admin, &token.address, &oracle);

// Set expert rate
client.set_my_rate(&expert, &10_i128);

// Admin pauses the contract
client.pause();

// Booking should fail while paused
let result = client.try_book_session(&user, &expert, &100);
assert!(result.is_err());

// Admin unpauses the contract
let result = client.try_unpause();
assert!(result.is_ok());

// Booking should succeed after unpause
let booking_id = client.book_session(&user, &expert, &100);
assert_eq!(booking_id, 1);
assert_eq!(token.balance(&user), 9_000);
assert_eq!(token.balance(&client.address), 1_000);
}

#[test]
fn test_read_only_functions_work_while_paused() {
let env = Env::default();
env.mock_all_auths();

let admin = Address::generate(&env);
let user = Address::generate(&env);
let expert = Address::generate(&env);
let oracle = Address::generate(&env);

let token_admin = Address::generate(&env);
let token = create_token_contract(&env, &token_admin);
token.mint(&user, &10_000);

let client = create_client(&env);
client.init(&admin, &token.address, &oracle);

// Book a session before pausing
let booking_id = {
client.set_my_rate(&expert, &10_i128);
client.book_session(&user, &expert, &100)
};

// Admin pauses the contract
client.pause();

// Read-only functions should still work
let booking = client.get_booking(&booking_id);
assert!(booking.is_some());
assert_eq!(booking.unwrap().id, booking_id);

let user_bookings = client.get_user_bookings(&user);
assert_eq!(user_bookings.len(), 1);

let expert_bookings = client.get_expert_bookings(&expert);
assert_eq!(expert_bookings.len(), 1);
}
Comment on lines +789 to +992
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a test verifying that unauthorized callers cannot pause/unpause.

All pausability tests use mock_all_auths(), which bypasses Soroban's authorization checks entirely. There is no test verifying that a non-admin (e.g., user or oracle) calling pause() or unpause() is rejected. For a security-critical circuit breaker this is an important coverage gap.

💡 Suggested test skeleton
#[test]
fn test_non_admin_cannot_pause() {
    let env = Env::default();
    env.mock_all_auths(); // set up contract

    let admin = Address::generate(&env);
    let non_admin = Address::generate(&env);
    let token = Address::generate(&env);
    let oracle = Address::generate(&env);

    let client = create_client(&env);
    client.init(&admin, &token, &oracle);

    // Clear mocked auths; supply only non_admin's auth
    env.set_auths(&[/* non_admin auth for pause */]);
    let result = client.try_pause();
    assert!(result.is_err());
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contracts/payment-vault-contract/src/test.rs` around lines 648 - 833, The
tests currently call env.mock_all_auths() which disables Soroban auth checks, so
add new tests (e.g., test_non_admin_cannot_pause and
test_non_admin_cannot_unpause) that initialize via create_client and client.init
but then clear or replace mocked auths (use env.set_auths or equivalent) to
supply only a non-admin signer before calling client.try_pause() and
client.try_unpause(), and assert those calls return Err; reference try_pause,
try_unpause, create_client, client.init, env.mock_all_auths, and env.set_auths
to locate where to change the auth setup and assertions.

16 changes: 8 additions & 8 deletions contracts/payment-vault-contract/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ pub enum BookingStatus {
#[contracttype]
#[derive(Clone, Debug)]
pub struct BookingRecord {
pub id: u64, // Storage key identifier
pub user: Address, // User who created the booking
pub expert: Address, // Expert providing consultation
pub rate_per_second: i128, // Payment rate per second
pub max_duration: u64, // Maximum booked duration in seconds
pub total_deposit: i128, // Total deposit (rate_per_second * max_duration)
pub status: BookingStatus, // Current booking status
pub created_at: u64, // Ledger timestamp when booking was created
pub id: u64, // Storage key identifier
pub user: Address, // User who created the booking
pub expert: Address, // Expert providing consultation
pub rate_per_second: i128, // Payment rate per second
pub max_duration: u64, // Maximum booked duration in seconds
pub total_deposit: i128, // Total deposit (rate_per_second * max_duration)
pub status: BookingStatus, // Current booking status
pub created_at: u64, // Ledger timestamp when booking was created
}