Implement Stale Booking Reclaim (User Safety)

[Josue19-08]

🧠 SkillSphere Pull Request 🌐

Mark with an x all the checkboxes that apply (like [x])

• Closes Implement "Stale Booking Reclaim" (User Safety) #17
• Added tests (if necessary)
• Ran cargo test (All tests passed)
• Evidence attached (Screenshots, Logs, or Transaction Hashes)
• Commented the code

---

📌 Type of Change

• 📚 Documentation (updates to README, docs, or comments)
• 🐛 Bug fix (non-breaking change which fixes an issue)
• ✨ Enhancement (non-breaking change which adds functionality)
• 💥 Breaking change (fix or feature that would cause existing functionality to change)
• 🏗️ Refactor (code improvement/cleanup without logical changes)

---

📝 Changes Description

Implement time-based mechanism for users to reclaim funds if booking stays in Pending status for more than 24 hours (protects users when Oracle/backend crashes):

• types.rs: Add Reclaimed status to BookingStatus enum and created_at: u64 field to BookingRecord
• error.rs: Add ReclaimTooEarly error for premature reclaim attempts
• events.rs: Add session_reclaimed event emission
• contract.rs: Populate created_at in book_session and implement reclaim_stale_session with user auth, ownership check, and 24h timeout
• lib.rs: Expose reclaim_stale_session as public contract function
• test.rs: Add 4 comprehensive tests for the reclaim functionality

---

📸 Evidence

running 13 tests
test test::test_booking_not_found ... ok
test test::test_initialization ... ok
test test::test_reclaim_already_finalized ... ok
test test::test_full_duration_no_refund ... ok
test test::test_reclaim_stale_session_too_early ... ok
test test::test_double_finalization_protection ... ok
test test::test_partial_duration_scenario ... ok
test test::test_reclaim_stale_session_success ... ok
test test::test_reclaim_stale_session_wrong_user ... ok
test test::test_oracle_authorization_enforcement ... ok
test test::test_get_user_and_expert_bookings ... ok
test test::test_book_session_balance_transfer ... ok
test test::test_zero_duration_finalization ... ok

test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Summary by CodeRabbit

• New Features

  • Users can reclaim deposits for pending bookings after a 24-hour timeout.
  • Booking creation timestamps are now recorded.
  • Bookings can transition to a reclaimed status and emit a reclaim event.

• Bug Fixes

  • Reclaims attempted before the timeout are rejected.

• Tests

  • Added comprehensive tests covering reclaim success, too-early attempts, wrong-user, and finalized-session cases.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a 24-hour reclaim flow letting booking owners recover deposits from long-pending bookings: records booking creation time, new reclaim API and contract logic, new event and error, booking status variant for reclaimed, and tests covering reclaim scenarios.

Changes

Cohort / File(s)	Summary
Core Reclaim Logic contracts/payment-vault-contract/src/contract.rs, contracts/payment-vault-contract/src/lib.rs	Added reclaim_stale_session(env, user, booking_id) with user auth, ownership/status checks, 24-hour timeout (RECLAIM_TIMEOUT), transfer of total_deposit back to user, booking status update to Reclaimed, and emission of session_reclaimed. finalize_session now returns Ok(()) explicitly.
Data Model contracts/payment-vault-contract/src/types.rs	Added pub created_at: u64 to BookingRecord and new BookingStatus::Reclaimed = 2.
Events contracts/payment-vault-contract/src/events.rs	Added session_reclaimed(env, booking_id, amount) to emit reclaim events with topic (symbol_short!("reclaim"), booking_id) and payload amount.
Errors contracts/payment-vault-contract/src/error.rs	Added ReclaimTooEarly = 7 variant to VaultError.
Storage / Misc contracts/payment-vault-contract/src/storage.rs	Minor annotation change: added #[allow(dead_code)] to get_admin (no functional change).
Tests contracts/payment-vault-contract/src/test.rs	Added ledger-based tests for reclaim behavior: too-early rejection, successful reclaim after advancing ledger time, wrong-user rejection, and reclaim-after-finalize rejection; imports updated to manipulate ledger timestamp.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Contract
    participant Storage
    participant Ledger
    participant Events

    User->>Contract: reclaim_stale_session(booking_id)
    Contract->>User: require_auth()
    User-->>Contract: authorized
    Contract->>Storage: get booking by id
    Storage-->>Contract: booking { owner, status=Pending, created_at, total_deposit }
    Contract->>Ledger: current_ts = ledger.timestamp()
    Ledger-->>Contract: current_ts
    Contract->>Contract: check current_ts > created_at + RECLAIM_TIMEOUT
    alt timeout elapsed
        Contract->>Contract: transfer total_deposit -> user
        Contract->>Storage: update booking.status = Reclaimed
        Storage-->>Contract: updated
        Contract->>Events: emit session_reclaimed(booking_id, amount)
        Events-->>Contract: published
        Contract->>User: Ok(())
    else too early
        Contract->>User: Err(ReclaimTooEarly)
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Feat/payment vault events #11: Adds booking lifecycle and related booking creation/finalization logic that this reclaim flow extends (same booking structures and events).
• feat(vault): Implemented book_session and deposit locking logic #14: Introduced initial booking data model and storage logic that were further extended here (created_at and reclaim handling).

Poem

🐰 A hop, a tick, a ledger's chime,
If sessions wait past counted time,
I nibble locks and send it back—
Safe funds returned along the track.
Reclaimed with a rabbit's rhyme.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.47% 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 clearly and specifically describes the main feature being implemented: a stale booking reclaim mechanism for user safety.
Description check	✅ Passed	The PR description follows the template with all required sections completed: issue reference, test confirmation, evidence with test results, and detailed change description across all modified files.
Linked Issues check	✅ Passed	All coding requirements from issue #17 are met: created_at field added, reclaim function implemented with auth/ownership/status/timeout checks, session_reclaimed event added, and four tests covering edge cases.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing the stale booking reclaim feature; no unrelated modifications or scope creep detected.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

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

contracts/payment-vault-contract/src/storage.rs (1)

15-32: Storage schema upgrade may break existing bookings if migrating from an earlier contract version.
Adding the created_at field to the Booking struct (a #[contracttype] named-field struct) will break deserialization of pre-existing Booking records stored on-chain. The derived decoder expects all struct fields; old records missing created_at will fail to decode. If this contract is being upgraded from a version without created_at, add a migration path (e.g., versioned storage keys, an upgrade function that rebuilds records, or custom deserialization with defaults). Note: Adding BookingStatus::Reclaimed is safe—new enum variants are backward-compatible.

[coderabbitai]

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

contracts/payment-vault-contract/src/types.rs (1)

7-25: Storage compatibility breaks deserialization when adding non-optional created_at field.

Adding a non-optional field to an already-deployed #[contracttype] struct will fail deserialization on existing stored BookingRecord entries. Soroban's #[contracttype] macro encodes structs as maps keyed by field name and errors if any field key is missing during decoding—Rust's Default is not applied.

If this contract has live deployments with stored bookings:

• Wrap created_at in Option<u64> to handle missing keys gracefully, or
• Implement a versioned storage strategy (e.g., BookingRecordV2 + migration code) or
• Add a state migration function to backfill the field before contract upgrade

Choose one of these patterns before deploying an update.