fix: [hexen-r9] add overflow check for bin liquidity (#211)

* fix: [hexen-r9] add overflow check for bin liquidity * fix: forge coverage stack too deep * optimization: minor adjust per comment * test: added test cases for swap and mint
pancakeswap · Nov 13, 2024 · c1e4f95 · c1e4f95
1 parent 15eb7d1
commit c1e4f95
Show file tree

Hide file tree

Showing 27 changed files with 181 additions and 25 deletions.
diff --git a/.forge-snapshots/BinCustomCurveHookTest#test_Swap_CustomCurve.snap b/.forge-snapshots/BinCustomCurveHookTest#test_Swap_CustomCurve.snap
@@ -1 +1 @@
-142478
+142475
diff --git a/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testDonateSucceedsWithHook.snap
@@ -1 +1 @@
-188410
+188265
diff --git a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap
@@ -1 +1 @@
-311254
+311495
diff --git a/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap
@@ -1 +1 @@
-189451
+190107
diff --git a/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap b/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap
@@ -1 +1 @@
-410336
+410577
diff --git a/.forge-snapshots/BinPoolManagerBytecodeSize.snap b/.forge-snapshots/BinPoolManagerBytecodeSize.snap
@@ -1 +1 @@
-23287
+23558
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap b/.forge-snapshots/BinPoolManagerTest#testGasDonate.snap
@@ -1 +1 @@
-118691
+118546
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap
@@ -1 +1 @@
-968475
+970284
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap
@@ -1 +1 @@
-327787
+329605
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap
@@ -1 +1 @@
-337511
+337752
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap
@@ -1 +1 @@
-140062
+140304
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap
@@ -1 +1 @@
-173098
+178450
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap
@@ -1 +1 @@
-179126
+184732
diff --git a/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap
@@ -1 +1 @@
-133129
+133785
diff --git a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap
@@ -1 +1 @@
-304550
+304791
diff --git a/.forge-snapshots/BinPoolSwapTest#testGas_exactOutputMultipleBin_WithEmptyBins.snap b/.forge-snapshots/BinPoolSwapTest#testGas_exactOutputMultipleBin_WithEmptyBins.snap
@@ -1 +1 @@
-109014
+115965
diff --git a/.forge-snapshots/BinPoolSwapTest#test_exactInputMultipleBin.snap b/.forge-snapshots/BinPoolSwapTest#test_exactInputMultipleBin.snap
@@ -1 +1 @@
-156553
+171392
diff --git a/.forge-snapshots/BinPoolSwapTest#test_exactInputSingleBin_SwapForX.snap b/.forge-snapshots/BinPoolSwapTest#test_exactInputSingleBin_SwapForX.snap
@@ -1 +1 @@
-62453
+63109
diff --git a/.forge-snapshots/BinPoolSwapTest#test_exactInputSingleBin_SwapForY.snap b/.forge-snapshots/BinPoolSwapTest#test_exactInputSingleBin_SwapForY.snap
@@ -1 +1 @@
-62422
+63078
diff --git a/.forge-snapshots/BinPoolSwapTest#test_exactOutputMultipleBin.snap b/.forge-snapshots/BinPoolSwapTest#test_exactOutputMultipleBin.snap
@@ -1 +1 @@
-151090
+165863
diff --git a/.forge-snapshots/BinPoolSwapTest#test_exactOutputSingleBin_SwapForX.snap b/.forge-snapshots/BinPoolSwapTest#test_exactOutputSingleBin_SwapForX.snap
@@ -1 +1 @@
-56810
+57403
diff --git a/.forge-snapshots/BinPoolSwapTest#test_exactOutputSingleBin_SwapForY.snap b/.forge-snapshots/BinPoolSwapTest#test_exactOutputSingleBin_SwapForY.snap
@@ -1 +1 @@
-56825
+57442
diff --git a/src/pool-bin/libraries/BinPool.sol b/src/pool-bin/libraries/BinPool.sol
@@ -48,6 +48,7 @@ library BinPool {
     error BinPool__NoLiquidityToReceiveFees();
     /// @dev if swap exactIn, x for y, unspecifiedToken = token y. if swap x for exact out y, unspecified token is x
     error BinPool__InsufficientAmountUnSpecified();
+    error BinPool__MaxLiquidityPerBinExceeded();
 
     /// @dev The state of a pool
     struct State {
@@ -169,6 +170,14 @@ library BinPool {
                     }
 
                     self.reserveOfBin[swapState.activeId] = binReserves.add(amountsInWithFees).sub(amountsOutOfBin);
+
+                    if (
+                        self.reserveOfBin[swapState.activeId].getLiquidity(
+                            swapState.activeId.getPriceFromId(params.binStep)
+                        ) > Constants.MAX_LIQUIDITY_PER_BIN
+                    ) {
+                        revert BinPool__MaxLiquidityPerBinExceeded();
+                    }
                 }
             }
 
@@ -347,9 +356,12 @@ library BinPool {
 
         /// @dev overflow check on total reserves and the resulting liquidity
         uint256 price = activeId.getPriceFromId(binStep);
-        binReserves.add(amountIn).getLiquidity(price);
+        bytes32 newReserves = binReserves.add(amountIn);
+        if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) {
+            revert BinPool__MaxLiquidityPerBinExceeded();
+        }
 
-        self.reserveOfBin[activeId] = binReserves.add(amountIn);
+        self.reserveOfBin[activeId] = newReserves;
         result = toBalanceDelta(-(amount0.safeInt128()), -(amount1.safeInt128()));
     }
 
@@ -457,7 +469,12 @@ library BinPool {
         if (shares == 0 || amountsInToBin == 0) revert BinPool__ZeroShares(id);
         if (supply == 0) _addBinIdToTree(self, id);
 
-        self.reserveOfBin[id] = binReserves.add(amountsInToBin);
+        bytes32 newReserves = binReserves.add(amountsInToBin);
+        if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) {
+            revert BinPool__MaxLiquidityPerBinExceeded();
+        }
+
+        self.reserveOfBin[id] = newReserves;
     }
 
     /// @notice Subtract share from user's position and update total share supply of bin

diff --git a/src/pool-bin/libraries/Constants.sol b/src/pool-bin/libraries/Constants.sol
@@ -11,4 +11,8 @@ library Constants {
     uint256 internal constant SQUARED_PRECISION = PRECISION * PRECISION;
 
     uint256 internal constant BASIS_POINT_MAX = 10_000;
+
+    // (2^256 - 1) / (2 * log(2**128) / log(1.0001))
+    uint256 internal constant MAX_LIQUIDITY_PER_BIN =
+        65251743116719673010965625540244653191619923014385985379600384103134737;
 }
diff --git a/test/pool-bin/libraries/BinPoolDonate.t.sol b/test/pool-bin/libraries/BinPoolDonate.t.sol
@@ -17,11 +17,14 @@ import {PackedUint128Math} from "../../../src/pool-bin/libraries/math/PackedUint
 import {SafeCast} from "../../../src/pool-bin/libraries/math/SafeCast.sol";
 import {BinPoolParametersHelper} from "../../../src/pool-bin/libraries/BinPoolParametersHelper.sol";
 import {BinTestHelper} from "../helpers/BinTestHelper.sol";
+import {Constants} from "../../../src/pool-bin/libraries/Constants.sol";
+import {PriceHelper} from "../../../src/pool-bin/libraries/PriceHelper.sol";
 
 contract BinPoolDonateTest is BinTestHelper {
     using PackedUint128Math for bytes32;
     using BinPoolParametersHelper for bytes32;
     using SafeCast for uint256;
+    using BinHelper for bytes32;
 
     MockVault public vault;
     BinPoolManager public poolManager;
@@ -119,6 +122,14 @@ contract BinPoolDonateTest is BinTestHelper {
         addLiquidityToBin(key, poolManager, bob, activeId, 1e18, 1e18, 1e18, 1e18, "");
         poolManager.getPosition(poolId, bob, activeId, 0).share;
 
+        bytes32 newReserves = PackedUint128Math.encode(1e18 + amt0, 1e18 + amt1);
+        uint256 price = PriceHelper.getPriceFromId(activeId, poolParam.getBinStep());
+        if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) {
+            vm.expectRevert(BinPool.BinPool__MaxLiquidityPerBinExceeded.selector);
+            poolManager.donate(key, amt0, amt1, "");
+            return;
+        }
+
         poolManager.donate(key, amt0, amt1, "");
 
         // Verify reserve after donate

diff --git a/test/pool-bin/libraries/BinPoolLiquidity.t.sol b/test/pool-bin/libraries/BinPoolLiquidity.t.sol
@@ -19,11 +19,14 @@ import {LiquidityConfigurations} from "../../../src/pool-bin/libraries/math/Liqu
 import {IBinPoolManager} from "../../../src/pool-bin/interfaces/IBinPoolManager.sol";
 import {BinPoolParametersHelper} from "../../../src/pool-bin/libraries/BinPoolParametersHelper.sol";
 import {BinTestHelper} from "../helpers/BinTestHelper.sol";
+import {PriceHelper} from "../../../src/pool-bin/libraries/PriceHelper.sol";
+import {BinHelper} from "../../../src/pool-bin/libraries/BinHelper.sol";
 
 contract BinPoolLiquidityTest is BinTestHelper {
     using PackedUint128Math for bytes32;
     using BinPoolParametersHelper for bytes32;
     using SafeCast for uint256;
+    using BinHelper for bytes32;
 
     MockVault public vault;
     BinPoolManager public poolManager;
@@ -53,6 +56,104 @@ contract BinPoolLiquidityTest is BinTestHelper {
         poolId = key.toId();
     }
 
+    function test_MintFuzz(uint128 amountX, uint128 amountY) external {
+        amountX = uint128(bound(amountX, 1 ether, uint128(type(int128).max)));
+        amountY = uint128(bound(amountY, 1 ether, uint128(type(int128).max)));
+
+        uint8 nbBinX = 6;
+        uint8 nbBinY = 6;
+
+        poolManager.initialize(key, activeId);
+
+        BinPool.MintArrays memory array;
+        vault.updateCurrentPoolKey(key);
+
+        // check if the new liquidity will exceed the max liquidity per bin
+        bool shouldRevert = false;
+        bytes32 newReserves = PackedUint128Math.encode(amountX / nbBinX, amountY / nbBinY);
+        {
+            uint256 total = getTotalBins(nbBinX, nbBinY);
+            for (uint256 i; i < total; ++i) {
+                uint24 id = getId(activeId, i, nbBinY);
+                uint256 price = PriceHelper.getPriceFromId(id, poolParam.getBinStep());
+                if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) {
+                    shouldRevert = true;
+                    break;
+                }
+            }
+        }
+
+        if (shouldRevert) {
+            vm.expectRevert(BinPool.BinPool__MaxLiquidityPerBinExceeded.selector);
+            (, array) = addLiquidity(key, poolManager, bob, activeId, amountX, amountY, nbBinX, nbBinY);
+            return;
+        }
+
+        (, array) = addLiquidity(key, poolManager, bob, activeId, amountX, amountY, nbBinX, nbBinY);
+
+        {
+            // verify X and Y amount
+            uint256 amtXBalanceDelta = uint256(-int256(vault.balanceDeltaOfPool(poolId).amount0()));
+            uint256 amountXLeft = amountX - ((amountX * (Constants.PRECISION / nbBinX)) / 1e18) * nbBinX;
+            assertEq(amountX, amtXBalanceDelta + amountXLeft, "test_MintFuzz::1");
+
+            uint256 amtYBalanceDelta = uint256(-int256(vault.balanceDeltaOfPool(poolId).amount1()));
+            uint256 amountYLeft = amountY - ((amountY * (Constants.PRECISION / nbBinY)) / 1e18) * nbBinY;
+            assertEq(amountY, amtYBalanceDelta + amountYLeft, "test_MintFUzz::2");
+        }
+        {
+            // verify each binId has the right reserve
+            uint256 total = getTotalBins(nbBinX, nbBinY);
+            for (uint256 i; i < total; ++i) {
+                uint24 id = getId(activeId, i, nbBinY);
+
+                (uint128 binReserveX, uint128 binReserveY,,) = poolManager.getBin(poolId, id);
+
+                if (id < activeId) {
+                    assertEq(binReserveX, 0, "test_MintFuzz::3");
+                    assertEq(binReserveY, (amountY * (Constants.PRECISION / nbBinY)) / 1e18, "test_MintFuzz::4");
+                } else if (id == activeId) {
+                    assertApproxEqRel(
+                        binReserveX, (amountX * (Constants.PRECISION / nbBinX)) / 1e18, 1e15, "test_MintFuzz::5"
+                    );
+                    assertApproxEqRel(
+                        binReserveY, (amountY * (Constants.PRECISION / nbBinY)) / 1e18, 1e15, "test_MintFuzz::6"
+                    );
+                } else {
+                    assertEq(binReserveX, (amountX * (Constants.PRECISION / nbBinX)) / 1e18, "test_MintFuzz::7");
+                    assertEq(binReserveY, 0, "test_MintFuzz::8");
+                }
+
+                assertGt(poolManager.getPosition(poolId, bob, id, 0).share, 0, "test_MintFuzz::9");
+            }
+        }
+        {
+            uint256 total = getTotalBins(nbBinX, nbBinY);
+            for (uint256 i; i < total; ++i) {
+                uint24 id = getId(activeId, i, nbBinY);
+
+                // verify id
+                assertEq(id, array.ids[i]);
+
+                // verify amount
+                (uint128 x, uint128 y) = array.amounts[i].decode();
+                if (id < activeId) {
+                    assertEq(x, 0);
+                    assertApproxEqRel(y, amountY / 6, 1e15); // approx amount within 0.1%,
+                } else if (id == activeId) {
+                    assertApproxEqRel(y, amountY / 6, 1e15); // approx amount within 0.1%
+                    assertApproxEqRel(x, amountX / 6, 1e15); // approx amount within 0.1%
+                } else {
+                    assertApproxEqRel(x, amountX / 6, 1e15); // approx amount within 0.1%
+                    assertEq(y, 0);
+                }
+
+                // verify liquidity minted
+                assertEq(poolManager.getPosition(poolId, bob, id, 0).share, array.liquidityMinted[i]);
+            }
+        }
+    }
+
     function test_SimpleMintX() external {
         poolManager.initialize(key, activeId);
 

diff --git a/test/pool-bin/libraries/BinPoolSwap.t.sol b/test/pool-bin/libraries/BinPoolSwap.t.sol
@@ -18,6 +18,7 @@ import {BinPoolParametersHelper} from "../../../src/pool-bin/libraries/BinPoolPa
 import {BinTestHelper} from "../helpers/BinTestHelper.sol";
 import {IProtocolFeeController} from "../../../src/interfaces/IProtocolFeeController.sol";
 import {MockProtocolFeeController} from "../../../src/test/fee/MockProtocolFeeController.sol";
+import {Constants} from "../../../src/pool-bin/libraries/Constants.sol";
 import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol";
 
 contract BinPoolSwapTest is BinTestHelper, GasSnapshot {
@@ -269,6 +270,28 @@ contract BinPoolSwapTest is BinTestHelper, GasSnapshot {
         poolManager.swap(key, false, -int128(amountIn), "0x");
     }
 
+    function test_revert_swapMaxLiquidityPerBinfuzz(int128 amountSpecified) external {
+        vm.assume(amountSpecified != 0);
+
+        // Add liquidity to the point where it is close to the max liquidity per bin
+        poolManager.initialize(key, activeId);
+        addLiquidity(
+            key,
+            poolManager,
+            bob,
+            activeId,
+            // when price is 1:1, then Constants.MAX_LIQUIDITY_PER_BIN >> 128 / 2 is the threshold
+            (Constants.MAX_LIQUIDITY_PER_BIN >> 128) / 2,
+            (Constants.MAX_LIQUIDITY_PER_BIN >> 128) / 2,
+            1,
+            1
+        );
+
+        // arbitrary amount of token will trigger the revert
+        vm.expectRevert(BinPool.BinPool__MaxLiquidityPerBinExceeded.selector);
+        poolManager.swap(key, false, amountSpecified, "0x");
+    }
+
     function test_revert_SwapOutOfLiquidity() external {
         // Add liquidity of 1e18 on each side
         poolManager.initialize(key, activeId);