Admin dispute resolution for Pending sessions (#32)#59
Merged
Bosun-Josh121 merged 2 commits intoLightForgeHub:mainfrom Mar 29, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds admin-only dispute resolution and remainder recovery flows to the payment vault: admins can split a booking's escrow between user and expert, mark the booking Changes
Sequence DiagramsequenceDiagram
actor Admin
participant Contract
participant Storage
participant Token as TokenTransfer
participant Events
Admin->>Contract: resolve_dispute(booking_id, user_refund, expert_pay)
rect rgba(100, 150, 200, 0.5)
Note over Contract: Validation Phase
Contract->>Contract: check paused & require_admin_auth
Contract->>Storage: load_booking(booking_id)
Contract->>Contract: assert booking exists && status == Pending
Contract->>Contract: validate amounts (>=0, no overflow, ≤ total_deposit)
end
rect rgba(150, 200, 150, 0.5)
Note over Contract: Fund Distribution Phase
alt user_refund > 0
Contract->>Token: transfer(user_refund) from contract to booking.user
end
alt expert_pay > 0
Contract->>Token: transfer(expert_pay) from contract to booking.expert
end
end
rect rgba(200, 150, 150, 0.5)
Note over Contract: State Update & Emission
Contract->>Storage: update_booking(status=DisputedAndResolved, dispute_* fields)
Contract->>Events: dispute_resolved(booking_id, user_refund, expert_pay)
Events->>Events: publish topic/payload
end
Contract-->>Admin: Ok(())
sequenceDiagram
actor Admin
participant Contract
participant Storage
participant Token as TokenTransfer
participant Events
Admin->>Contract: recover_disputed_remainder(booking_id)
rect rgba(100, 150, 200, 0.5)
Note over Contract: Validation Phase
Contract->>Contract: check paused & require_admin_auth
Contract->>Storage: load_booking(booking_id)
Contract->>Contract: assert status == DisputedAndResolved
Contract->>Contract: assert !dispute_remainder_recovered
end
rect rgba(150, 200, 150, 0.5)
Note over Contract: Remainder Calculation & Transfer
Contract->>Contract: remainder = total_deposit - dispute_user_refund - dispute_expert_pay
alt remainder > 0
Contract->>Token: transfer(remainder) to admin
end
end
rect rgba(200, 150, 150, 0.5)
Note over Contract: Finalize
Contract->>Storage: set dispute_remainder_recovered = true and persist
Contract->>Events: disputed_remainder_recovered(booking_id, remainder)
end
Contract-->>Admin: Ok(remainder)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
contracts/payment-vault-contract/src/test.rs (2)
1485-1486: Consider adding a test for negative split amounts.The test suite thoroughly covers the main scenarios. Consider adding a test that passes negative values for
user_refundorexpert_payto verify theInvalidAmounterror is returned, ensuring the validation at contract.rs lines 440-442 is exercised.🤖 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 1485 - 1486, Add a unit test in contracts/payment-vault-contract/src/test.rs (e.g., test_negative_split_amounts) that calls the dispute resolution entry point (the resolve_dispute or equivalent function in contract.rs) with negative values for user_refund and/or expert_pay and assert it returns the InvalidAmount error; ensure the test exercises the validation branch around the InvalidAmount condition referenced in contract.rs (lines ~440-442) and verifies both single-negative and mixed-negative parameter cases to cover validation paths.
2-2: Minor: Duplicate import exists intest_expert_rejects_pending_session.The
BookingStatusimport is now at module level (line 2), but there's a duplicateuse crate::types::BookingStatus;insidetest_expert_rejects_pending_sessionat line 540 andtest_user_cancels_before_session_starts_successat line 1290. Consider removing those inner imports for consistency.🤖 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` at line 2, Remove the redundant inner imports of BookingStatus inside the test functions; since BookingStatus is already imported at module scope (use crate::types::BookingStatus;), delete the duplicate use crate::types::BookingStatus; lines found inside the functions test_expert_rejects_pending_session and test_user_cancels_before_session_starts_success so the tests rely on the module-level import only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/payment-vault-contract/src/test.rs`:
- Around line 1642-1671: The test shows resolve_dispute leaves a remainder
locked in the contract; fix by adding an admin recovery path or documenting the
behavior. Either implement an admin-only method (e.g.,
admin_recover_disputed_remainder or recover_orphaned_funds) that (1) requires
caller == admin, (2) checks the booking is in DisputedAndResolved (or another
terminal status) and not already recovered, (3) computes remainder =
booking.deposit - booking.user_refund - booking.expert_pay, (4) transfers
remainder to admin (or to a configurable recipient) and marks the booking as
recovered to prevent double-withdrawal; update resolve_dispute/Booking struct to
record user_refund/expert_pay/status if needed. Or alternatively, update
contract docs and tests to explicitly state that resolve_dispute can leave
remainders locked and add guidance for dispute amounts—change tests referencing
"Funds remain locked in contract" accordingly.
---
Nitpick comments:
In `@contracts/payment-vault-contract/src/test.rs`:
- Around line 1485-1486: Add a unit test in
contracts/payment-vault-contract/src/test.rs (e.g., test_negative_split_amounts)
that calls the dispute resolution entry point (the resolve_dispute or equivalent
function in contract.rs) with negative values for user_refund and/or expert_pay
and assert it returns the InvalidAmount error; ensure the test exercises the
validation branch around the InvalidAmount condition referenced in contract.rs
(lines ~440-442) and verifies both single-negative and mixed-negative parameter
cases to cover validation paths.
- Line 2: Remove the redundant inner imports of BookingStatus inside the test
functions; since BookingStatus is already imported at module scope (use
crate::types::BookingStatus;), delete the duplicate use
crate::types::BookingStatus; lines found inside the functions
test_expert_rejects_pending_session and
test_user_cancels_before_session_starts_success so the tests rely on the
module-level import only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 257a2f2c-ba24-44b7-950a-94f9cc949b91
📒 Files selected for processing (5)
contracts/payment-vault-contract/src/contract.rscontracts/payment-vault-contract/src/events.rscontracts/payment-vault-contract/src/lib.rscontracts/payment-vault-contract/src/test.rscontracts/payment-vault-contract/src/types.rs
Implement admin fund recovery mechanism or document remainder handling in dispute resolution.
Collaborator
|
Nice work @EtimbukUdofia |
Contributor
Author
|
@Bosun-Josh121 Thank you. Still open for further collaboration. |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Admin Dispute Resolution
Closes #32
Summary
Adds an admin-only resolve_dispute endpoint to the payment-vault-contract that allows forcefully splitting escrowed funds for a Pending booking. This is the fallback mechanism for when the Oracle crashes or an unresolvable dispute occurs between user and expert.
Summary by CodeRabbit
New Features
Events
Errors
Tests