Skip to content

Conversation

@supercontracts
Copy link
Collaborator

@supercontracts supercontracts commented Dec 18, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Removed enforcement of requested minimum token amounts when removing liquidity; withdrawals no longer fail based on those minimums and instead use balance-based rate calculations.
    • Decrease liquidity flow now omits the user-specified minimums from the withdrawal call path, which may change failure behavior on low-slippage or partial-fill scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@octane-security-app
Copy link

Summary by Octane

New Contracts

No new contracts were added.

Updated Contracts

  • UniswapV4Lib.sol: Added minimum amount constraints for token balances to ensure sufficient liquidity in swaps.

🔗 Commit Hash: 394a4a8

@coderabbitai
Copy link

coderabbitai bot commented Dec 18, 2025

Walkthrough

Removed amount0Min and amount1Min from the decrease-liquidity call path in UniswapV4Lib.sol: the min constraints are no longer passed into _decreaseLiquidity or included in the decrease calldata; internal balance-based rate-limit calculations remain.

Changes

Cohort / File(s) Summary
Decrease flow signature change
src/libraries/UniswapV4Lib.sol
Removed amount0Min and amount1Min from the decrease-liquidity flow: they are no longer accepted by _getDecreaseLiquidityCallData or propagated in the decrease calldata. Internal balance computations (e.g., rateLimitDecrease from endingBalance - startingBalance) remain but no longer use the removed min parameters.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify callers no longer supply or expect amount0Min/amount1Min in calldata/signatures.
  • Ensure downstream callers or interfaces (external contracts, tests) were updated to match removed parameters to avoid mismatches.
  • Confirm that removing explicit min checks doesn't introduce unintended slippage exposure or violate intended invariants.

Suggested labels

Priority: Medium

Suggested reviewers

  • lucas-manuel

Poem

🐰 I hopped through code with whiskers keen,
Removed two mins from the liquidity scene.
Simpler calls now, but guard your stash,
I bounced away quick — a tiny, tidy dash. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title claims to enforce amount0Min/amount1Min constraints, but the changes actually remove these parameters from the decrease liquidity flow, which contradicts the stated objective. Revise the title to accurately reflect the change: remove or reword to indicate that amount0Min/amount1Min are being removed from the decrease liquidity path, or clarify the new enforcement mechanism if that is the intent.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/cantina-12-enfore-amount

📜 Recent review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a388a41 and 39aba0a.

📒 Files selected for processing (1)
  • src/libraries/UniswapV4Lib.sol (0 hunks)
💤 Files with no reviewable changes (1)
  • src/libraries/UniswapV4Lib.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: test
  • GitHub Check: coverage

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 18, 2025
@octane-security-app
Copy link

Overview

Vulnerabilities found: 7                                                                                
Warnings found: 6                                                                                

🔗 Commit Hash: 394a4a8
🛡️ Octane Dashboard: All vulnerabilities

lucas-manuel
lucas-manuel previously approved these changes Dec 18, 2025
@deluca-mike deluca-mike force-pushed the fix/cantina-12-enfore-amount branch from a388a41 to 39aba0a Compare December 19, 2025 16:24
@github-actions
Copy link

Coverage after merging fix/cantina-12-enfore-amount into dev will be

99.36%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
deploy
   ControllerDeploy.sol100%100%100%100%
   ForeignControllerInit.sol100%100%100%100%
   MainnetControllerInit.sol97.37%93.33%100%100%152, 90
src
   ALMProxy.sol100%100%100%100%
   ALMProxyFreezable.sol100%100%100%100%
   ForeignController.sol94.90%84.62%95.65%97.22%128–129, 129, 129, 316–317, 573
   MainnetController.sol99.18%100%98.28%99.24%538–539
   OTCBuffer.sol100%100%100%100%
   RateLimitHelpers.sol100%100%100%100%
   RateLimits.sol100%100%100%100%
src/libraries
   AaveLib.sol100%100%100%100%
   ApproveLib.sol100%100%100%100%
   CCTPLib.sol100%100%100%100%
   CurveLib.sol100%100%100%100%
   ERC4626Lib.sol96%75%100%100%108
   PSMLib.sol100%100%100%100%
   UniswapV4Lib.sol99.31%95.65%100%100%277

@lucas-manuel lucas-manuel merged commit 7b59cf5 into dev Dec 19, 2025
4 checks passed
@lucas-manuel lucas-manuel deleted the fix/cantina-12-enfore-amount branch December 19, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants