Skip to content

LiquidityPenaltyHook L-01: Skip fee aggregation on an expired removal - #189

Open
luiz-lvj wants to merge 1 commit into
masterfrom
fix/liquidity-penalty-expired-removal
Open

luiz-lvj wants to merge 1 commit into
masterfrom
fix/liquidity-penalty-expired-removal

Conversation

@luiz-lvj

Copy link
Copy Markdown
Collaborator

On removal, _afterRemoveLiquidity settled the position's withheld fees and aggregated them with the fresh fees (feeDelta + withheldFees) before checking whether the penalty had expired. Each BalanceDelta component is an int128, so a component sum exceeding int128.max reverts, and because the aggregation ran unconditionally it reverted even for a position past its offset that only wanted its fees back; on rollback the withheld-fee storage was restored, so every later removal re-hit the overflow. Combined with both tick boundaries being saturated — which blocks the add path that would otherwise checkpoint the fresh fees — this could leave a position's principal and fees inaccessible.

The aggregation now runs only inside the in-window branch, where it is actually used. An expired position returns its withheld fees without aggregating, so it can always be removed once the offset has passed. The reachable behavior is unchanged, since the aggregated value was already unused on the expired path.

@luiz-lvj
luiz-lvj requested a review from gonzaotc September 10, 2026 19:36
@netlify

netlify Bot commented Sep 10, 2026

Copy link
Copy Markdown

Deploy Preview for uniswap-hooks failed. Why did it fail? →

Name Link
🔨 Latest commit 438329a
🔍 Latest deploy log https://app.netlify.com/projects/uniswap-hooks/deploys/6aa306ddc03cb70008cac8b8

@netlify

netlify Bot commented Sep 10, 2026

Copy link
Copy Markdown

Deploy Preview for uniswap-hooks-docs failed. Why did it fail? →

Name Link
🔨 Latest commit 438329a
🔍 Latest deploy log https://app.netlify.com/projects/uniswap-hooks-docs/deploys/6aa306dd5f89a8000858b64a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant