Skip to content

Conversation

@Majormaxx
Copy link
Contributor

@Majormaxx Majormaxx commented Jan 23, 2026

Summary

This PR implements the "Book Session" logic for the SkillSphere Payment Vault contract as specified in Issue #6. It refactors the existing booking functionality to align with the project's decentralized identity/payment architecture, introducing a structured BookingRecord and a "Pay per Second" deposit locking mechanism.

Related Issue

Closes #6

Changes Made

  • Types & Data Structures: Created src/types.rs containing BookingRecord and BookingStatus to centralize contract data models.
  • Storage Refactoring: Updated storage.rs to persist BookingRecord and aligned storage keys.
  • Contract Logic:
    • Implemented book_session function with require_auth verification.
    • Added upfront deposit calculation (rate_per_second * max_duration).
    • Integrated token transfer logic from user to contract vault.
    • Added BookingCreated event emission for indexer visibility.
  • Interface Alignment: Exposed book_session in the public contract implementation and updated cross-module imports.
  • Test Suite: Updated existing tests to reflect naming changes and added comprehensive balance-verification tests.

Testing Done

  • All 8 tests passing locally (cargo test)
  • Verified balance decrease for users upon booking.
  • Verified unique booking ID generation and contract deposit locking.
  • Build check successful (cargo build --release).

Quality Checklist

  • Code follows project naming conventions (camelCase for variables, Past tense active voice for commits).
  • Self-reviewed the changes for storage efficiency and security.
  • Documentation updated via JSDoc-style comments (Rust doc-comments).
  • No breaking changes for core vault initialization logic.
  • Atomic commits with conventional format.

Summary by CodeRabbit

  • New Features

    • Booking sessions now automatically transfer deposits from user to contract upon creation.
    • Booking events are now emitted for tracking and monitoring session creation.
  • Refactor

    • Session booking API updated: function renamed and parameters restructured for clearer rate-per-second and maximum duration specifications.

✏️ Tip: You can customize this high-level summary in your review settings.

- Created types.rs to centralize data structures
- Defined BookingRecord struct and BookingStatus enum
- Aligned field names with issue specification (rate_per_second, max_duration)

Refs: LightForgeHub#6
- Migrated storage logic to use refined types from types module
- Updated DataKey and persistence methods
- Enhanced storage organization for better maintainability

Refs: LightForgeHub#6
- Renamed create_booking to book_session for consistency
- Implemented deposit locking logic with specifying rate and duration
- Added booking_created event for frontend tracking

Refs: LightForgeHub#6
- Exposed book_session instead of create_booking
- Registered types module as a public module
- Updated interface to reflect refined parameter naming

Refs: LightForgeHub#6
- Re-aligned existing tests with new naming and structure
- Added test_book_session_balance_transfer to verify contract locking logic
- Ensured 100% pass rate for current and new tests

Refs: LightForgeHub#6
@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

📝 Walkthrough

Walkthrough

This PR renames create_booking to book_session and refactors the booking system to use rate-per-second and max-duration parameters. A new types.rs module introduces BookingRecord and BookingStatus types. The contract now transfers deposits from users and emits booking creation events.

Changes

Cohort / File(s) Summary
Type Definitions
contracts/payment-vault-contract/src/types.rs
New module introducing BookingStatus enum (Pending/Complete) and BookingRecord struct with fields: id, user, expert, rate_per_second, max_duration, total_deposit, status
Storage Layer
contracts/payment-vault-contract/src/storage.rs
Updated to use BookingRecord instead of local Booking struct; imports types from crate::types; adjusted function signatures for save/get operations
Contract Logic
contracts/payment-vault-contract/src/contract.rs
Function renamed: create_bookingbook_session; parameters changed: rate, booked_durationrate_per_second, max_duration; added token transfer logic (deposit calculation and transfer); uses new BookingRecord type; updated finalization logic to calculate expert pay using rate_per_second
Events
contracts/payment-vault-contract/src/events.rs
New function booking_created() added to emit "booked" events with booking_id, user, expert, and deposit
Public Interface
contracts/payment-vault-contract/src/lib.rs
Added mod types declaration; renamed public function create_bookingbook_session; updated parameter names and documentation; updated internal call to new function name
Tests
contracts/payment-vault-contract/src/test.rs
Updated existing tests to use book_session with new parameter names; added new test test_book_session_balance_transfer verifying user balance decreases, contract balance increases, and booking IDs are sequential

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Contract as Payment Vault
    participant Token as Token Contract
    
    User->>Contract: book_session(user, expert, rate_per_second, max_duration)
    Note over Contract: Calculate deposit = rate_per_second × max_duration
    Contract->>Token: transfer(user, contract, deposit)
    Token->>Token: Update balances
    Token-->>Contract: Transfer success
    Contract->>Contract: Create BookingRecord and store
    Contract->>Contract: Emit booking_created event
    Contract-->>User: Return booking_id
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Feat/payment vault events #11: Implements the complementary booking lifecycle including session finalization with token payouts and modifies the same booking-related functions, storage, and event patterns.

Poem

🐰 A booking blooms with care so keen,
Rate-per-second now keeps the scene,
Deposits locked, transfers flow true,
BookingRecords bridge me and you! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: implementing book_session function with deposit locking logic for the payment vault contract.
Description check ✅ Passed The PR description is comprehensive, includes related issue reference (#6), detailed change summary, testing evidence, and quality checklist following the template structure.
Linked Issues check ✅ Passed All objectives from Issue #6 are met: BookingRecord struct created with required fields, book_session implemented with auth/deposit calculation/token transfer/storage/return, exposed in lib.rs, and tests verify balance transfers and booking ID uniqueness.
Out of Scope Changes check ✅ Passed All changes are directly related to Issue #6 requirements: types definition, storage refactoring, contract logic implementation, interface exposure, and test updates for book_session functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
contracts/payment-vault-contract/src/contract.rs (2)

95-102: Add duration bounds validation and use checked multiplication for payout calculations.

The actual_duration parameter is accepted from an untrusted oracle caller without validating against booking.max_duration, allowing over-duration inputs to proceed. Additionally, the multiplication booking.rate_per_second * (actual_duration as i128) can silently overflow. While the subsequent checks catch negative refunds, this leaves room for calculation errors and inconsistent state. Use checked_mul() and validate actual_duration <= booking.max_duration upfront.

Suggested fix
-    let expert_pay = booking.rate_per_second * (actual_duration as i128);
+    if actual_duration > booking.max_duration {
+        return Err(VaultError::InvalidAmount);
+    }
+
+    let expert_pay = booking.rate_per_second
+        .checked_mul(actual_duration as i128)
+        .ok_or(VaultError::InvalidAmount)?;

26-46: Guard against i128 overflow in deposit calculations.

The unchecked multiplications at line 42 (book_session) and line 96 (finalize_session) can overflow without being caught. The current check if total_deposit <= 0 only detects negative wraps, missing positive overflows where the result wraps to a small positive value. This would cause incorrect token transfers.

Use checked_mul to safely calculate:

  • rate_per_second * (max_duration as i128) at line 42
  • booking.rate_per_second * (actual_duration as i128) at line 96

Return VaultError::InvalidAmount on overflow.

Suggested fix for line 42
-    let total_deposit = rate_per_second * (max_duration as i128);
+    let total_deposit = rate_per_second
+        .checked_mul(max_duration as i128)
+        .ok_or(VaultError::InvalidAmount)?;

@Bosun-Josh121 Bosun-Josh121 merged commit 7a047c2 into LightForgeHub:main Jan 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement "Book Session" (Deposit)

2 participants