Add locked unit reserve for payment hub withdrawals#3791
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a locked unit reserve mechanism for PaymentHub withdrawals, allowing users to withdraw funds even with active channels as long as they maintain sufficient locked reserves.
Key Changes
- Added
PaymentHubConfigglobal configuration to store per-coin locked unit requirements - Modified
withdraw_from_hubto check unlocked balance instead of blocking all withdrawals when channels are active - Added view helpers (
get_unlocked_balance_in_hub,get_required_locked_for_owner,get_locked_unit) and admin functionset_locked_unitto manage locked reserves - Updated tests to verify unlocked balance behavior and removal of the old "no withdrawals with active channels" restriction
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| frameworks/rooch-framework/sources/payment_channel.move | Implements PaymentHubConfig struct, unlocked balance calculation logic, admin API for setting locked units, and updates withdrawal validation to check unlocked balance instead of blocking all withdrawals with active channels |
| frameworks/rooch-framework/sources/tests/payment_channel_test.move | Adds tests for unlocked balance behavior with active channels and updates existing tests to reflect new withdrawal logic that allows partial withdrawals when locked reserves are maintained |
Comments suppressed due to low confidence (1)
frameworks/rooch-framework/sources/payment_channel.move:1525
- The documentation comment states "does not check for active channels", but this is misleading. The function bypasses the unlocked balance check entirely, not just the active channels check. This could allow the
transaction_gasmodule (the only friend) to withdraw locked reserves, potentially leaving channels underfunded.
Consider updating the comment to be more explicit: "does not check unlocked balance requirements or locked reserves" and ensure the transaction_gas module has proper safeguards to avoid withdrawing funds that should remain locked for active channels.
/// Internal function to withdraw specific coin type from payment hub
/// (no signer required and does not check for active channels)
/// Used by system contracts like transaction_gas module
|
Implemented requested changes:\n- Added LockedUnitConfigUpdatedEvent and emit in set_locked_unit\n- Enforced locked_unit monotonic non-decreasing updates to avoid sudden unlocks\n- Removed redundant copies/wrapper in locked unit helpers\n\nRe-ran: target/debug/rooch move test -p frameworks/rooch-framework payment_channel (pass). |
|
Addressed review feedback:\n- Added LockedUnitConfigUpdatedEvent and emit in set_locked_unit\n- set_locked_unit now enforces non-decreasing updates (avoids sudden unlock when channels active)\n- Removed redundant copies/wrapper and simplified helper logic\n\nTests rerun: target/debug/rooch move test -p frameworks/rooch-framework payment_channel (pass). |
|
@copilot addressed: locked unit event + monotonic update + copy cleanup already pushed; overflow concern noted but left as-is per discussion (Move aborts on overflow, governance expected to set sane values). |
Summary
Testing