Open
Conversation
bucket-bot
reviewed
Apr 5, 2026
Contributor
bucket-bot
left a comment
There was a problem hiding this comment.
Code review
Found 1 issue:
- Partial position decrease can hard-abort when accrued fees exceed remaining collateral, which blocks a risk-reducing action and creates inconsistent behavior vs full close. In full close you cap fee deduction with
min(collateral.value()), but in partial decrease you withdraw exacttotal_feeandfunding_feefrom position collateral. If collateral is already stressed (e.g. after realized loss),withdraw_collateralaborts and user cannot partially de-risk.
- Fee withdrawal path:
trading.move#L1245-L1267 - Full-close capped behavior reference:
trading.move#L1150-L1174
Suggested fix: cap partial-close fee deductions the same way as full close (or pre-check and route to liquidation explicitly), so partial reduce remains executable and predictable under low-collateral states.
waterx-sdk/contracts/waterx_perp/sources/trading.move
Lines 1245 to 1267 in b776f09
🤖 Generated with Rex (OpenClaw)
…rror Brings simulate error surfacing, Pyth gRPC/JSON-RPC fallbacks, and testnet OI assertions so CI matches PR #14; merges cleanly with fee/calc work. Made-with: Cursor
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
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.
This pull request introduces major improvements to the fee structure and risk management in the WaterX perpetual protocol. The main changes are the addition of a configurable impact fee system for trading, a rework of liquidation and insurance fee logic, and updates to how trading fees are calculated and validated across the codebase. It also includes a fix for a position registration issue when opening new positions, as well as a variable naming correction for consistency. Together, these changes improve fee flexibility and accuracy, better reflect market risk, strengthen protocol safety, and fix an important accounting edge case.
Fee Structure and Risk Management Enhancements:
MarketConfig, including parameters for maximum impact fee, LP exposure allocation, curvature, and scale, with default values and validation to ensure safe configuration.trading.moveto usecalculate_total_trading_fee_bps, ensuring all trading, closing, and liquidation fees reflect both base and impact fees.Liquidation and Insurance Fee Logic:
Collateral and Position Safety Checks:
Bug Fixes and Consistency Improvements:
account_registry, ensuring account state stays consistent and positions can be tracked correctly after creation.colleteraltocollateralfor better clarity and consistency across the codebase.Documentation Updates:
ARCHITECTURE.mdto reflect the new liquidation and insurance fee model, and clarified protocol fee and keeper fee handling.