Priority queue Audit fixes#362
Priority queue Audit fixes#362pankajjagtapp wants to merge 12 commits intopankaj/feat/priority-withdrawal-queuefrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84ce16f43d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| _dequeueWithdrawRequest(request); | ||
|
|
||
| if (wasFinalized) { | ||
| uint256 amountForShares = liquidityPool.amountForShare(request.shareOfEEth); |
There was a problem hiding this comment.
Cancel returns wrong shares via amount transfer
Medium Severity
_cancelWithdrawRequest now returns funds using safeTransfer(request.user, liquidityPool.amountForShare(request.shareOfEEth)), but EETH.transfer converts the amount back into shares using liquidityPool.sharesForAmount (floor). This double-rounding can transfer fewer than request.shareOfEEth shares, leaving dust shares in the queue and short-changing the requester.


Note
Medium Risk
Changes affect withdrawal/locking paths and fund accounting across rebases, so regressions could impact user withdrawals or protocol liquidity availability despite added test coverage.
Overview
Tightens priority-withdrawal and LP liquidity accounting to be rebase-safe.
PriorityWithdrawalQueuenow (a) locks/unlocks based on the raw requested amount, (b) returns eETH on cancel equal to the current value of the originally queued shares, (c) makespermitfailures non-fatal when allowance is already sufficient (front-run tolerant), (d) makesinvalidateRequestscancel both pending and finalized requests, and (e) rounds treasury splits up when handling remainder.Hardens LP withdrawals against locked-liquidity leakage.
LiquidityPool.withdrawaddsInsufficientUnlockedLiquidityfor non-NFT callers when NFT-queue liquidity is locked.Deployment/ops scripts are adjusted to be idempotent for Create2 deployments and to include RedemptionManager upgrades in the timelock transaction plan; tests are expanded to cover the new edge cases (rebases, single-item batch claim, permit front-run, and locked-liquidity).
Written by Cursor Bugbot for commit 28a16fe. This will update automatically on new commits. Configure here.