Skip to content

refactor(storage): fix unbounded vector growth with O(1) indexed storage (#24) #46

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
Bosun-Josh121 merged 1 commit into from
Mar 26, 2026
Merged

refactor(storage): fix unbounded vector growth with O(1) indexed storage (#24) #46

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
48 changes: 32 additions & 16 deletions contracts/payment-vault-contract/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#![no_std]

mod contract;
mod error;
mod events;
Expand Down Expand Up @@ -44,8 +43,8 @@ impl PaymentVaultContract {
contract::set_my_rate(&env, &expert, rate_per_second)
}

/// Book a session with an expert
/// User deposits tokens upfront based on rate_per_second * max_duration
/// Book a session with an expert.
/// User deposits tokens upfront based on rate_per_second * max_duration.
pub fn book_session(
env: Env,
user: Address,
Expand All @@ -55,8 +54,8 @@ impl PaymentVaultContract {
contract::book_session(&env, &user, &expert, max_duration)
}

/// Finalize a session (Oracle-only)
/// Calculates payments based on actual duration and processes refunds
/// Finalize a session (Oracle-only).
/// Calculates payments based on actual duration and processes refunds.
pub fn finalize_session(
env: Env,
booking_id: u64,
Expand All @@ -65,8 +64,8 @@ impl PaymentVaultContract {
contract::finalize_session(&env, booking_id, actual_duration)
}

/// Reclaim funds from a stale booking (User-only)
/// Users can reclaim their deposit if the booking has been pending for more than 24 hours
/// Reclaim funds from a stale booking (User-only).
/// Users can reclaim their deposit if the booking has been pending for more than 24 hours.
pub fn reclaim_stale_session(
env: Env,
user: Address,
Expand All @@ -75,23 +74,40 @@ impl PaymentVaultContract {
contract::reclaim_stale_session(&env, &user, booking_id)
}

/// Reject a pending session (Expert-only)
/// Experts can reject a pending booking, instantly refunding the user
/// 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> {
contract::reject_session(&env, &expert, booking_id)
}

/// Get all booking IDs for a specific user
pub fn get_user_bookings(env: Env, user: Address) -> Vec<u64> {
storage::get_user_bookings(&env, &user)
/// Get a paginated list of booking IDs for a specific user.
/// `start_index` is 0-based. Returns at most `limit` booking IDs.
pub fn get_user_bookings(env: Env, user: Address, start_index: u32, limit: u32) -> Vec<u64> {
storage::get_user_bookings_paginated(&env, &user, start_index, limit)
}

/// Get the total number of bookings a user has ever made.
pub fn get_user_booking_count(env: Env, user: Address) -> u32 {
storage::get_user_booking_count(&env, &user)
}

/// Get a paginated list of booking IDs for a specific expert.
/// `start_index` is 0-based. Returns at most `limit` booking IDs.
pub fn get_expert_bookings(
env: Env,
expert: Address,
start_index: u32,
limit: u32,
) -> Vec<u64> {
storage::get_expert_bookings_paginated(&env, &expert, start_index, limit)
}

/// Get all booking IDs for a specific expert
pub fn get_expert_bookings(env: Env, expert: Address) -> Vec<u64> {
storage::get_expert_bookings(&env, &expert)
/// Get the total number of bookings an expert has ever received.
pub fn get_expert_booking_count(env: Env, expert: Address) -> u32 {
storage::get_expert_booking_count(&env, &expert)
}

/// Get booking details by booking ID (read-only)
/// Get booking details by booking ID (read-only).
pub fn get_booking(env: Env, booking_id: u64) -> Option<BookingRecord> {
storage::get_booking(&env, booking_id)
}
Expand Down
122 changes: 91 additions & 31 deletions contracts/payment-vault-contract/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,17 @@ 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
ExpertBookings(Address), // Expert Address -> Vec<u64> of booking IDs
IsPaused, // Circuit breaker flag
ExpertRate(Address), // Expert Address -> rate per second (i128)
Booking(u64), // Booking ID -> BookingRecord
BookingCounter, // Counter for generating unique booking IDs
// ── Indexed User Booking List ──────────────────────────────────────────
// Replaces the old Vec<u64> approach with O(1) per-write composite keys.
UserBooking(Address, u32), // (user, index) -> booking_id
UserBookingCount(Address), // user -> total count (u32)
// ── Indexed Expert Booking List ────────────────────────────────────────
ExpertBooking(Address, u32), // (expert, index) -> booking_id
ExpertBookingCount(Address), // expert -> total count (u32)
IsPaused, // Circuit breaker flag
ExpertRate(Address), // Expert Address -> rate per second (i128)
}

// --- Admin ---
Expand Down Expand Up @@ -93,47 +98,102 @@ pub fn update_booking_status(env: &Env, booking_id: u64, status: BookingStatus)
}
}

// --- User and Expert Booking Lists ---
pub fn add_booking_to_user_list(env: &Env, user: &Address, booking_id: u64) {
let mut user_bookings: soroban_sdk::Vec<u64> = env
.storage()
.persistent()
.get(&DataKey::UserBookings(user.clone()))
.unwrap_or(soroban_sdk::Vec::new(env));

user_bookings.push_back(booking_id);
// --- User Booking List (O(1) indexed storage) ---

/// Returns how many bookings a user has booked in total.
pub fn get_user_booking_count(env: &Env, user: &Address) -> u32 {
env.storage()
.persistent()
.set(&DataKey::UserBookings(user.clone()), &user_bookings);
.get(&DataKey::UserBookingCount(user.clone()))
.unwrap_or(0u32)
}

pub fn add_booking_to_expert_list(env: &Env, expert: &Address, booking_id: u64) {
let mut expert_bookings: soroban_sdk::Vec<u64> = env
.storage()
/// Appends a booking_id to the user's list in O(1) — no Vec load/save.
pub fn add_booking_to_user_list(env: &Env, user: &Address, booking_id: u64) {
let count = get_user_booking_count(env, user);
// Store the new booking_id at slot `count` (0-indexed)
env.storage()
.persistent()
.get(&DataKey::ExpertBookings(expert.clone()))
.unwrap_or(soroban_sdk::Vec::new(env));

expert_bookings.push_back(booking_id);

.set(&DataKey::UserBooking(user.clone(), count), &booking_id);
// Increment the counter
env.storage()
.persistent()
.set(&DataKey::ExpertBookings(expert.clone()), &expert_bookings);
.set(&DataKey::UserBookingCount(user.clone()), &(count + 1));
}

/// Returns a paginated slice of booking IDs for a user.
/// `start_index` is 0-based; returns at most `limit` items.
pub fn get_user_bookings_paginated(
env: &Env,
user: &Address,
start_index: u32,
limit: u32,
) -> soroban_sdk::Vec<u64> {
let count = get_user_booking_count(env, user);
let mut result = soroban_sdk::Vec::new(env);

let end = (start_index + limit).min(count);
let mut i = start_index;
while i < end {
if let Some(booking_id) = env
.storage()
.persistent()
.get::<DataKey, u64>(&DataKey::UserBooking(user.clone(), i))
{
result.push_back(booking_id);
}
i += 1;
}

result
}
Comment on lines +126 to 149
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the current pagination math and whether release overflow checks are configured.
rg -n 'get_(user|expert)_bookings_paginated|start_index\s*\+\s*limit|saturating_add|checked_add|MAX_BOOKINGS_PAGE_SIZE' contracts/payment-vault-contract/src

fd '^Cargo.toml$' | while read -r file; do
  echo "--- $file"
  sed -n '/\[profile.release\]/,/^\[/p' "$file"
done

Repository: LightForgeHub/SkillSphere-Contracts

Length of output: 1503

Implement explicit pagination size bounds.

The limit parameter is fully caller-controlled, allowing requests for arbitrarily large pages. While start_index + limit arithmetic will panic on overflow in release builds (overflow-checks enabled in Cargo.toml), relying on panics is not idiomatic defense—explicit bounds should be enforced instead. Clamp limit to a contract-defined maximum to prevent DoS and excessive resource consumption.

This applies to both get_user_bookings_paginated (line 126–149) and get_expert_bookings_paginated (line 174–197).

🛠️ Proposed fix 
+const MAX_BOOKINGS_PAGE_SIZE: u32 = 100;
+
 pub fn get_user_bookings_paginated(
     env: &Env,
     user: &Address,
     start_index: u32,
     limit: u32,
 ) -> soroban_sdk::Vec<u64> {
     let count = get_user_booking_count(env, user);
     let mut result = soroban_sdk::Vec::new(env);
 
-    let end = (start_index + limit).min(count);
+    let end = start_index
+        .saturating_add(limit.min(MAX_BOOKINGS_PAGE_SIZE))
+        .min(count);
     let mut i = start_index;
     while i < end {
         if let Some(booking_id) = env
             .storage()
             .persistent()
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/payment-vault-contract/src/storage.rs` around lines 126 - 149, The
functions get_user_bookings_paginated and get_expert_bookings_paginated allow an
unbounded caller-controlled limit and can overflow when computing start_index +
limit; define a contract-level constant (e.g. MAX_PAGE_SIZE) and clamp the
incoming limit to that (limit = limit.min(MAX_PAGE_SIZE)), then compute the end
using a safe add (e.g. start_index.saturating_add(limit) or
start_index.checked_add(limit).unwrap_or_max()) before taking min(count) to
avoid overflow; apply these changes to the functions named
get_user_bookings_paginated and get_expert_bookings_paginated and ensure the
clamped limit is used when iterating and pushing booking ids.


pub fn get_user_bookings(env: &Env, user: &Address) -> soroban_sdk::Vec<u64> {
// --- Expert Booking List (O(1) indexed storage) ---

/// Returns how many bookings an expert has in total.
pub fn get_expert_booking_count(env: &Env, expert: &Address) -> u32 {
env.storage()
.persistent()
.get(&DataKey::UserBookings(user.clone()))
.unwrap_or(soroban_sdk::Vec::new(env))
.get(&DataKey::ExpertBookingCount(expert.clone()))
.unwrap_or(0u32)
}

pub fn get_expert_bookings(env: &Env, expert: &Address) -> soroban_sdk::Vec<u64> {
/// Appends a booking_id to the expert's list in O(1) — no Vec load/save.
pub fn add_booking_to_expert_list(env: &Env, expert: &Address, booking_id: u64) {
let count = get_expert_booking_count(env, expert);
env.storage()
.persistent()
.get(&DataKey::ExpertBookings(expert.clone()))
.unwrap_or(soroban_sdk::Vec::new(env))
.set(&DataKey::ExpertBooking(expert.clone(), count), &booking_id);
env.storage()
.persistent()
.set(&DataKey::ExpertBookingCount(expert.clone()), &(count + 1));
}

/// Returns a paginated slice of booking IDs for an expert.
/// `start_index` is 0-based; returns at most `limit` items.
pub fn get_expert_bookings_paginated(
env: &Env,
expert: &Address,
start_index: u32,
limit: u32,
) -> soroban_sdk::Vec<u64> {
let count = get_expert_booking_count(env, expert);
let mut result = soroban_sdk::Vec::new(env);

let end = (start_index + limit).min(count);
let mut i = start_index;
while i < end {
if let Some(booking_id) = env
.storage()
.persistent()
.get::<DataKey, u64>(&DataKey::ExpertBooking(expert.clone(), i))
{
result.push_back(booking_id);
}
i += 1;
}

result
}

// --- Expert Rates ---
Expand Down
Loading
Loading