From 1b4b44e6eb0a7a58c0430e6626c098128f9c7962 Mon Sep 17 00:00:00 2001 From: ChefMist <133624774+ChefMist@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:32:46 +0800 Subject: [PATCH] feat: burn 1e3 shares for first mint into BinPool (#212) * feat: example of option 2 * feat: fix all existing test cases with min share burned * test: fix remaining tests * feat: clean up more test * test: format test code * feat: include updated gas cost * feat: update IBinPoolManager doc per PR * feat: burn 1e3 shares for first mint into BinPool - part 2 (#215) * feat: example if we tweak add/remove bin to tree * feat: updated gas cost * feat: add test around getNextNonEmptyBin * test: updated forge gas snapshot --- .../BinHookTest#testBurnSucceedsWithHook.snap | 2 +- .../BinHookTest#testMintSucceedsWithHook.snap | 2 +- .../BinMintBurnFeeHookTest#test_Burn.snap | 2 +- .../BinMintBurnFeeHookTest#test_Mint.snap | 2 +- .../BinPoolManagerBytecodeSize.snap | 2 +- ...oolManagerTest#testBurnNativeCurrency.snap | 2 +- ...BinPoolManagerTest#testGasBurnHalfBin.snap | 2 +- ...inPoolManagerTest#testGasBurnNineBins.snap | 2 +- .../BinPoolManagerTest#testGasBurnOneBin.snap | 2 +- ...nPoolManagerTest#testGasMintNneBins-1.snap | 2 +- ...nPoolManagerTest#testGasMintNneBins-2.snap | 2 +- ...inPoolManagerTest#testGasMintOneBin-1.snap | 2 +- ...inPoolManagerTest#testGasMintOneBin-2.snap | 2 +- ...oolManagerTest#testMintNativeCurrency.snap | 2 +- src/pool-bin/interfaces/IBinPoolManager.sol | 2 + src/pool-bin/libraries/BinPool.sol | 30 ++++-- test/pool-bin/BinHookReturnsDelta.t.sol | 10 +- test/pool-bin/BinMintBurnFeeHook.t.sol | 20 ++-- test/pool-bin/BinPoolManager.t.sol | 100 +++++++++++++++--- .../BinPoolManagerBehaviorComparison.t.sol | 18 +++- test/pool-bin/helpers/BinMintBurnFeeHook.sol | 4 - test/pool-bin/libraries/BinPoolDonate.t.sol | 23 ++-- .../pool-bin/libraries/BinPoolLiquidity.t.sol | 39 ++++--- 23 files changed, 199 insertions(+), 75 deletions(-) diff --git a/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap index 436357a4..5aa39133 100644 --- a/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap @@ -1 +1 @@ -178130 \ No newline at end of file +197607 \ No newline at end of file diff --git a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap index 07ee066b..6e6fe373 100644 --- a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap @@ -1 +1 @@ -311495 \ No newline at end of file +311821 \ No newline at end of file diff --git a/.forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap b/.forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap index d6fd559c..2c98c3ad 100644 --- a/.forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap +++ b/.forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap @@ -1 +1 @@ -170145 \ No newline at end of file +185889 \ No newline at end of file diff --git a/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap b/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap index 0e89e7af..32b1b9d3 100644 --- a/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap +++ b/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap @@ -1 +1 @@ -410577 \ No newline at end of file +410773 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerBytecodeSize.snap b/.forge-snapshots/BinPoolManagerBytecodeSize.snap index e8a0dfb4..cfe77212 100644 --- a/.forge-snapshots/BinPoolManagerBytecodeSize.snap +++ b/.forge-snapshots/BinPoolManagerBytecodeSize.snap @@ -1 +1 @@ -23558 \ No newline at end of file +23675 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap index 02574599..fec8e379 100644 --- a/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap +++ b/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap @@ -1 +1 @@ -133892 \ No newline at end of file +147984 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap index f53d2c9a..69ae72a0 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap @@ -1 +1 @@ -142717 \ No newline at end of file +142994 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap index 994141b9..57e79e25 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap @@ -1 +1 @@ -289683 \ No newline at end of file +295835 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap index ba16a41b..0371d19b 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap @@ -1 +1 @@ -127065 \ No newline at end of file +139450 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap index 2aa17409..0cf34439 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap @@ -1 +1 @@ -970284 \ No newline at end of file +973186 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap index eeef0a41..d20ddee8 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap @@ -1 +1 @@ -329605 \ No newline at end of file +331940 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap index d25d3e11..194dc826 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap @@ -1 +1 @@ -337752 \ No newline at end of file +338078 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap index ddde8d98..b9a1803f 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap @@ -1 +1 @@ -140304 \ No newline at end of file +140567 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap index f7fe2469..68ac59b8 100644 --- a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap +++ b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap @@ -1 +1 @@ -304791 \ No newline at end of file +305117 \ No newline at end of file diff --git a/src/pool-bin/interfaces/IBinPoolManager.sol b/src/pool-bin/interfaces/IBinPoolManager.sol index e5737176..d2280340 100644 --- a/src/pool-bin/interfaces/IBinPoolManager.sol +++ b/src/pool-bin/interfaces/IBinPoolManager.sol @@ -170,6 +170,8 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload { function initialize(PoolKey memory key, uint24 activeId) external; /// @notice Add liquidity to a pool + /// @dev For the first liquidity added to a bin, the share minted would be slightly lessser (1e3 lesser) to prevent + /// share inflation attack. /// @return delta BalanceDelta, will be negative indicating how much total amt0 and amt1 liquidity added /// @return mintArray Liquidity added in which ids, how much amt0, amt1 and how much liquidity added function mint(PoolKey memory key, IBinPoolManager.MintParams calldata params, bytes calldata hookData) diff --git a/src/pool-bin/libraries/BinPool.sol b/src/pool-bin/libraries/BinPool.sol index 4cdc6455..0c2c92ba 100644 --- a/src/pool-bin/libraries/BinPool.sol +++ b/src/pool-bin/libraries/BinPool.sol @@ -67,6 +67,10 @@ library BinPool { mapping(bytes32 => bytes32) level2; } + /// @dev when a bin has supply for the first time, 1e3 share will be locked up + /// this is to prevent share inflation attack on BinPool type + uint256 constant MINIMUM_SHARE = 1e3; + function initialize(State storage self, uint24 activeId, uint24 protocolFee, uint24 lpFee) internal { /// An initialized pool will not have activeId: 0 if (self.slot0.activeId() != 0) revert PoolAlreadyInitialized(); @@ -330,7 +334,8 @@ library BinPool { binReserves = binReserves.sub(amountsOutFromBin); - if (supply == amountToBurn) _removeBinIdToTree(self, id); + /// @dev _removeBinIdToTree if supply is MINIMUM_SHARE after burning as min share is too low liquidity for trade anyway + if (supply - amountToBurn == MINIMUM_SHARE) _removeBinIdToTree(self, id); self.reserveOfBin[id] = binReserves; amounts[i] = amountsOutFromBin; @@ -397,12 +402,12 @@ library BinPool { amountsLeft = amountsLeft.sub(amountsIn); feeAmountToProtocol = feeAmountToProtocol.add(binFeeAmt); + shares = _addShare(self, params.to, id, params.salt, shares); + arrays.ids[i] = id; arrays.amounts[i] = amountsInToBin; arrays.liquidityMinted[i] = shares; - _addShare(self, params.to, id, params.salt, shares); - compositionFeeAmount = compositionFeeAmount.add(binCompositionFee); unchecked { @@ -467,7 +472,8 @@ library BinPool { } if (shares == 0 || amountsInToBin == 0) revert BinPool__ZeroShares(id); - if (supply == 0) _addBinIdToTree(self, id); + /// @dev if supply was originally MINIMUM_SHARE (people added and remove liquidity before) or 0 (new bin), add binId to tree + if (supply <= MINIMUM_SHARE) _addBinIdToTree(self, id); bytes32 newReserves = binReserves.add(amountsInToBin); if (newReserves.getLiquidity(price) > Constants.MAX_LIQUIDITY_PER_BIN) { @@ -484,8 +490,20 @@ library BinPool { } /// @notice Add share to user's position and update total share supply of bin - function _addShare(State storage self, address owner, uint24 binId, bytes32 salt, uint256 shares) internal { - self.positions.get(owner, binId, salt).addShare(shares); + /// @dev if bin is empty, deduct MINIMUM_SHARE from shares + /// @return userShareAdded The amount of share added to user's position + function _addShare(State storage self, address owner, uint24 binId, bytes32 salt, uint256 shares) + internal + returns (uint256 userShareAdded) + { + userShareAdded = shares; + if (self.shareOfBin[binId] == 0) { + /// @dev Only for first liquidity provider for the bin, deduct MINIMUM_SHARE, expected to underflow + /// if shares < MINIMUM_SHARE for first mint + userShareAdded = shares - MINIMUM_SHARE; + } + + self.positions.get(owner, binId, salt).addShare(userShareAdded); self.shareOfBin[binId] += shares; } diff --git a/test/pool-bin/BinHookReturnsDelta.t.sol b/test/pool-bin/BinHookReturnsDelta.t.sol index 930a5e63..c0fdec95 100644 --- a/test/pool-bin/BinHookReturnsDelta.t.sol +++ b/test/pool-bin/BinHookReturnsDelta.t.sol @@ -127,11 +127,11 @@ contract BinHookReturnsDelta is Test, GasSnapshot, BinTestHelper { binLiquidityHelper.burn(key, burnParams, ""); (uint128 reserveXAfter, uint128 reserveYAfter,,) = poolManager.getBin(key.toId(), activeId); - - assertEq(reserveXAfter, 0); - assertEq(reserveYAfter, 0); - assertEq(token0.balanceOf(address(binReturnsDeltaHook)), 0.1 ether); - assertEq(token1.balanceOf(address(binReturnsDeltaHook)), 0.1 ether); + // reserve non zero due to min liquidity (1e3) locked up in the bin + assertEq(reserveXAfter, 1); + assertEq(reserveYAfter, 1); + assertEq(token0.balanceOf(address(binReturnsDeltaHook)), 0.1 ether - 1); + assertEq(token1.balanceOf(address(binReturnsDeltaHook)), 0.1 ether - 1); } function testSwap_noSwap_specifyInput() external { diff --git a/test/pool-bin/BinMintBurnFeeHook.t.sol b/test/pool-bin/BinMintBurnFeeHook.t.sol index b85548cb..bd7f530c 100644 --- a/test/pool-bin/BinMintBurnFeeHook.t.sol +++ b/test/pool-bin/BinMintBurnFeeHook.t.sol @@ -113,21 +113,25 @@ contract BinMintBurnFeeHookTest is Test, GasSnapshot, BinTestHelper { assertEq(vault.balanceOf(address(binMintBurnFeeHook), key.currency0), 2 ether); assertEq(vault.balanceOf(address(binMintBurnFeeHook), key.currency1), 2 ether); + // take 4x the burn amount as fee IBinPoolManager.BurnParams memory burnParams = _getSingleBinBurnLiquidityParams(key, poolManager, activeId, address(binLiquidityHelper), 100); snapStart("BinMintBurnFeeHookTest#test_Burn"); binLiquidityHelper.burn(key, burnParams, ""); snapEnd(); - // +1 from remove liqudiity, -4 from hook fee - assertEq(token0.balanceOf(address(this)), 7 ether + 1 ether - 4 ether); - assertEq(token1.balanceOf(address(this)), 7 ether + 1 ether - 4 ether); + // +1 eth from remove liqudiity, -4 eth from hook fee + // +3 from min_liquidity amount as -1 (min_liquidity) + 1 * 4 (fee) + assertEq(token0.balanceOf(address(this)), 7 ether + 1 ether - 4 ether + 3); + assertEq(token1.balanceOf(address(this)), 7 ether + 1 ether - 4 ether + 3); - // -1 from remove liquidity, +4 from hook calling vault.mint - assertEq(token0.balanceOf(address(vault)), 3 ether - 1 ether + 4 ether); - assertEq(token1.balanceOf(address(vault)), 3 ether - 1 ether + 4 ether); - assertEq(vault.balanceOf(address(binMintBurnFeeHook), key.currency0), 2 ether + 4 ether); - assertEq(vault.balanceOf(address(binMintBurnFeeHook), key.currency1), 2 ether + 4 ether); + // -1 eth from remove liquidity, +4 eth from hook calling vault.mint + assertEq(token0.balanceOf(address(vault)), 3 ether - 1 ether + 4 ether - 3); + assertEq(token1.balanceOf(address(vault)), 3 ether - 1 ether + 4 ether - 3); + + // -4 as due to min_liquidity = 1, hook took 4 token less fee + assertEq(vault.balanceOf(address(binMintBurnFeeHook), key.currency0), 2 ether + 4 ether - 4); + assertEq(vault.balanceOf(address(binMintBurnFeeHook), key.currency1), 2 ether + 4 ether - 4); } receive() external payable {} diff --git a/test/pool-bin/BinPoolManager.t.sol b/test/pool-bin/BinPoolManager.t.sol index fc7a11a3..48897b46 100644 --- a/test/pool-bin/BinPoolManager.t.sol +++ b/test/pool-bin/BinPoolManager.t.sol @@ -458,9 +458,17 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { for (uint256 i = 0; i < binIds.length; i++) { (uint128 binReserveX, uint128 binReserveY,,) = poolManager.getBin(key.toId(), binIds[i]); - // make sure the liquidity is added to the correct bin - assertEq(binReserveX, 0 ether); - assertEq(binReserveY, 0 ether); + // should have 1 token left due to min liquidity + if (binIds[i] < activeId) { + assertEq(binReserveX, 0); + assertEq(binReserveY, 1); + } else if (binIds[i] > activeId) { + assertEq(binReserveX, 1); + assertEq(binReserveY, 0); + } else { + assertEq(binReserveX, 1); + assertEq(binReserveY, 1); + } BinPosition.Info memory position = poolManager.getPosition(key.toId(), address(binLiquidityHelper), binIds[i], salt); @@ -480,6 +488,7 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { token0.mint(address(this), 30 ether); token1.mint(address(this), 30 ether); + // mint for salt1 (IBinPoolManager.MintParams memory mintParams, uint24[] memory binIds) = _getMultipleBinMintParams(activeId, 2 ether, 2 ether, 5, 5, salt1); binLiquidityHelper.mint(key, mintParams, ""); @@ -514,6 +523,7 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { } { + // now mint for salt2 (mintParams, binIds) = _getMultipleBinMintParams(activeId, 2 ether, 2 ether, 5, 5, salt2); binLiquidityHelper.mint(key, mintParams, ""); @@ -542,11 +552,13 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { // only position with salt 0 should be empty assertTrue(position0.share == 0); assertTrue(position1.share != 0); - assertTrue(position1.share == position2.share); + // // 1e3 is MINIMUM_SHARE locked when added liquidity first + assertTrue(position1.share + 1e3 == position2.share); } } { + // now mint for salt0 (mintParams, binIds) = _getMultipleBinMintParams(activeId, 2 ether, 2 ether, 5, 5, salt0); binLiquidityHelper.mint(key, mintParams, ""); @@ -572,9 +584,10 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { BinPosition.Info memory position2 = poolManager.getPosition(key.toId(), address(binLiquidityHelper), binIds[i], salt2); + // 1e3 is MINIMUM_SHARE locked when added liquidity first assertTrue(position0.share != 0); - assertTrue(position1.share == position0.share); - assertTrue(position1.share == position2.share); + assertTrue(position1.share + 1e3 == position0.share); + assertTrue(position1.share + 1e3 == position2.share); } } @@ -589,13 +602,13 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { // make sure the liquidity is added to the correct bin if (binIds[i] < activeId) { assertEq(binReserveX, 0 ether); - assertEq(binReserveY, 0.4 ether * 2); + assertEq(binReserveY, 0.4 ether * 2 + 1); } else if (binIds[i] > activeId) { - assertEq(binReserveX, 0.4 ether * 2); + assertEq(binReserveX, 0.4 ether * 2 + 1); assertEq(binReserveY, 0 ether); } else { - assertEq(binReserveX, 0.4 ether * 2); - assertEq(binReserveY, 0.4 ether * 2); + assertEq(binReserveX, 0.4 ether * 2 + 1); + assertEq(binReserveY, 0.4 ether * 2 + 1); } BinPosition.Info memory position0 = @@ -628,7 +641,7 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { uint256[] memory ids = new uint256[](1); bytes32[] memory amounts = new bytes32[](1); ids[0] = activeId; - amounts[0] = uint128(1e18).encode(uint128(1e18)); + amounts[0] = uint128(1e18 - 1).encode(uint128(1e18 - 1)); // -1 due to minshare locked up vm.expectEmit(); emit IBinPoolManager.Burn(key.toId(), address(binLiquidityHelper), ids, 0, amounts); @@ -697,7 +710,7 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { uint256[] memory ids = new uint256[](1); bytes32[] memory amounts = new bytes32[](1); ids[0] = activeId; - amounts[0] = uint128(1e18).encode(uint128(1e18)); + amounts[0] = uint128(1e18 - 1).encode(uint128(1e18 - 1)); // -1 due to minshare locked up vm.expectEmit(); emit IBinPoolManager.Burn(key.toId(), address(binLiquidityHelper), ids, 0, amounts); @@ -1181,6 +1194,69 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper { assertEq(shares, liquidity); } + function test_getNextNonEmptyBin() public { + poolManager.initialize(key, activeId); + + // add 1 eth of tokenX and 1 eth to activeId - 2 to active + 2 bins + token0.mint(address(this), 10 ether); + token1.mint(address(this), 10 ether); + (IBinPoolManager.MintParams memory mintParams,) = _getMultipleBinMintParams(activeId, 2 ether, 2 ether, 3, 3); + binLiquidityHelper.mint(key, mintParams, ""); + + // swapForY is true, means search for bin to the left as tokenY reside on the left side of the bin + bool swapForY = true; + for (uint24 i = 0; i < 5; i++) { + // [-2, -1, activeId, 1, 2] are bins initialized due to liqudiity adding above + uint24 nextEmptyBin = poolManager.getNextNonEmptyBin(key.toId(), swapForY, activeId + 3 - i); + assertEq(nextEmptyBin, activeId + 2 - i); + } + + IBinPoolManager.BurnParams memory burnParams; + + // burn activeId-1 + burnParams = _getSingleBinBurnLiquidityParams(key, poolManager, activeId - 1, address(binLiquidityHelper), 100); + binLiquidityHelper.burn(key, burnParams, ""); + + // as activeId-1 bin is empty now, verify the next non empty bin to the left of activeId is activeId-2 + assertEq(poolManager.getNextNonEmptyBin(key.toId(), true, activeId), activeId - 2); + + // burn activeId+1 + burnParams = _getSingleBinBurnLiquidityParams(key, poolManager, activeId + 1, address(binLiquidityHelper), 100); + binLiquidityHelper.burn(key, burnParams, ""); + + // as activeId+1 bin is empty now, verify the next non empty bin to the left of activeId+2 is activeId + assertEq(poolManager.getNextNonEmptyBin(key.toId(), true, activeId + 2), activeId); + } + + function test_getNextNonEmptyBin_AddRemoveAddLiquidity() public { + // initialize + poolManager.initialize(key, activeId); + + // mint, verify activeId is in treeMath + token0.mint(address(this), 2 ether); + token1.mint(address(this), 2 ether); + IBinPoolManager.MintParams memory mintParams = _getSingleBinMintParams(activeId, 1 ether, 1 ether); + binLiquidityHelper.mint(key, mintParams, ""); + assertEq(poolManager.getNextNonEmptyBin(key.toId(), true, activeId + 1), activeId); + + // remove, verify activeId not in treeMath + IBinPoolManager.BurnParams memory burnParams; + burnParams = _getSingleBinBurnLiquidityParams(key, poolManager, activeId, address(binLiquidityHelper), 100); + binLiquidityHelper.burn(key, burnParams, ""); + assertEq(poolManager.getNextNonEmptyBin(key.toId(), true, activeId + 1), type(uint24).max); + + // mint, verify activeId in treeMath again + binLiquidityHelper.mint(key, mintParams, ""); + assertEq(poolManager.getNextNonEmptyBin(key.toId(), true, activeId + 1), activeId); + } + + function test_getNextNonEmptyBin_NoBinWithLiqudiity() public { + poolManager.initialize(key, activeId); + + assertEq(poolManager.getNextNonEmptyBin(key.toId(), true, activeId), type(uint24).max); + assertEq(poolManager.getNextNonEmptyBin(key.toId(), false, activeId), 0); + } + receive() external payable {} function supportsInterface(bytes4) external pure returns (bool) { diff --git a/test/pool-bin/BinPoolManagerBehaviorComparison.t.sol b/test/pool-bin/BinPoolManagerBehaviorComparison.t.sol index a484676d..89cbfad4 100644 --- a/test/pool-bin/BinPoolManagerBehaviorComparison.t.sol +++ b/test/pool-bin/BinPoolManagerBehaviorComparison.t.sol @@ -36,6 +36,9 @@ abstract contract LBFuzzer is LBHelper, BinTestHelper { Currency currency0; Currency currency1; + /// @dev when a bin has supply for the first time, 1e3 share will be locked up + uint256 constant MINIMUM_SHARE = 1e3; + function setUp() public virtual override { super.setUp(); @@ -135,9 +138,18 @@ abstract contract LBFuzzer is LBHelper, BinTestHelper { for (uint256 i = 0; i < liquidityMinted.length; i++) { uint24 id = uint24(uint256(mintParams.liquidityConfigs[i])); BinPosition.Info memory positionInfoAfter = manager.getPosition(key.toId(), address(liquidityHelper), id, 0); - assertEq( - liquidityMinted[i], positionInfoAfter.share - sharesBefore[i], "Expecting to mint same liquidity !" - ); + if (sharesBefore[i] == 0) { + // first mint, so positionInfoAfter.share has MINIMUM_SHARE lesser + assertEq( + liquidityMinted[i], + positionInfoAfter.share - sharesBefore[i] + MINIMUM_SHARE, + "Expecting to mint same liquidity !" + ); + } else { + assertEq( + liquidityMinted[i], positionInfoAfter.share - sharesBefore[i], "Expecting to mint same liquidity !" + ); + } } } diff --git a/test/pool-bin/helpers/BinMintBurnFeeHook.sol b/test/pool-bin/helpers/BinMintBurnFeeHook.sol index 373a86e9..a051bf8f 100644 --- a/test/pool-bin/helpers/BinMintBurnFeeHook.sol +++ b/test/pool-bin/helpers/BinMintBurnFeeHook.sol @@ -84,10 +84,6 @@ contract BinMintBurnFeeHook is BaseBinTestHook { BalanceDelta delta, bytes calldata ) external override returns (bytes4, BalanceDelta) { - console2.log("afterBurn delta"); - console2.logInt(delta.amount0()); - console2.logInt(delta.amount1()); - int128 amt0Fee; if (delta.amount0() > 0) { amt0Fee = (delta.amount0()) * 4; diff --git a/test/pool-bin/libraries/BinPoolDonate.t.sol b/test/pool-bin/libraries/BinPoolDonate.t.sol index 5c8ebca8..24470644 100644 --- a/test/pool-bin/libraries/BinPoolDonate.t.sol +++ b/test/pool-bin/libraries/BinPoolDonate.t.sol @@ -60,16 +60,22 @@ contract BinPoolDonateTest is BinTestHelper { } function testDonate_InsufficientBinShareForDonate(uint256 remainingShare) public { + uint256 MINIMUM_SHARE = 1e3; + // Initialize pool and add liqudiity poolManager.initialize(key, activeId); addLiquidityToBin(key, poolManager, alice, activeId, 1e18, 1e18, 1e18, 1e18, ""); // Remove all share leaving less than MIN_LIQUIDITY_BEFORE_DONATE shares - remainingShare = bound(remainingShare, 1, poolManager.minBinShareForDonate() - 1); + remainingShare = bound(remainingShare, 1, poolManager.minBinShareForDonate() - 1 - MINIMUM_SHARE); uint256 aliceShare = poolManager.getPosition(poolId, alice, activeId, 0).share; removeLiquidityFromBin(key, poolManager, alice, activeId, aliceShare - remainingShare, ""); - vm.expectRevert(abi.encodeWithSelector(IBinPoolManager.InsufficientBinShareForDonate.selector, remainingShare)); + vm.expectRevert( + abi.encodeWithSelector( + IBinPoolManager.InsufficientBinShareForDonate.selector, remainingShare + MINIMUM_SHARE + ) + ); poolManager.donate(key, 1e18, 1e18, ""); } @@ -101,16 +107,17 @@ contract BinPoolDonateTest is BinTestHelper { assertEq(removeDelta1.amount0(), 2e18); assertEq(removeDelta1.amount1(), 2e18); + // lesser than 2e18 as alice is the first lp provider and liquidity locked up BalanceDelta removeDelta2 = removeLiquidityFromBin(key, poolManager, alice, activeId, aliceShare, ""); - assertEq(removeDelta2.amount0(), 2e18); - assertEq(removeDelta2.amount1(), 2e18); + assertEq(removeDelta2.amount0(), 2e18 - 1); + assertEq(removeDelta2.amount1(), 2e18 - 1); - // Verify no reserve remaining + // Verify only min_liquidity worth of token locked up (reserveX, reserveY,,) = poolManager.getBin(poolId, activeId); - assertEq(reserveX, 0); - assertEq(reserveY, 0); + assertEq(reserveX, 1); + assertEq(reserveY, 1); - vm.expectRevert(abi.encodeWithSelector(IBinPoolManager.InsufficientBinShareForDonate.selector, 0)); + vm.expectRevert(abi.encodeWithSelector(IBinPoolManager.InsufficientBinShareForDonate.selector, 1e3)); poolManager.donate(key, 1e18, 1e18, ""); } diff --git a/test/pool-bin/libraries/BinPoolLiquidity.t.sol b/test/pool-bin/libraries/BinPoolLiquidity.t.sol index 72f7d003..cf20c658 100644 --- a/test/pool-bin/libraries/BinPoolLiquidity.t.sol +++ b/test/pool-bin/libraries/BinPoolLiquidity.t.sol @@ -268,10 +268,11 @@ contract BinPoolLiquidityTest is BinTestHelper { assertEq(binReserveY, 0, "test_SimpleMint::6"); } - assertEq(poolManager.getPosition(poolId, bob, id, 0).share, 2 * balances[i], "test_DoubleMint:7"); + // balances[i] was from first mint, so add 1e3 (min share) as second mint does not burn min share + assertEq(poolManager.getPosition(poolId, bob, id, 0).share, 2 * balances[i] + 1e3, "test_DoubleMint:7"); // Only bob minted, all the shares in the bin should be bob's - assertEq(poolManager.getPosition(poolId, bob, id, 0).share, totalShares); + assertEq(poolManager.getPosition(poolId, bob, id, 0).share, totalShares - 1e3, "test_DoubleMint:8"); } } @@ -291,8 +292,9 @@ contract BinPoolLiquidityTest is BinTestHelper { BalanceDelta bobDelta = removeLiquidityFromBin(key, poolManager, bob, activeId, bobBal, ""); uint256 aliceBal = poolManager.getPosition(key.toId(), alice, activeId, 0).share; BalanceDelta aliceDelta = removeLiquidityFromBin(key, poolManager, alice, activeId, aliceBal, ""); - assertEq(aliceDelta.amount0() + bobDelta.amount0(), 200 ether); - assertEq(aliceDelta.amount1() + bobDelta.amount1(), 1 ether); + // -1 tokenX and -1 tokenY as they are locked as min liquidity + assertEq(aliceDelta.amount0() + bobDelta.amount0(), 200 ether - 1); + assertEq(aliceDelta.amount1() + bobDelta.amount1(), 1 ether - 1); } function test_MintWithDifferentBins() external { @@ -327,8 +329,11 @@ contract BinPoolLiquidityTest is BinTestHelper { "test_MintWithDifferentBins::1" ); // composition fee, so share will be lesser than 2 * balances[i] } else { + // 1e3 as balance[i] was from first mint, so add 1e3 (min share) as second mint does not burn min share assertEq( - poolManager.getPosition(poolId, bob, id, 0).share, 2 * balances[i], "test_MintWithDifferentBins::2" + poolManager.getPosition(poolId, bob, id, 0).share, + 2 * balances[i] + 1e3, + "test_MintWithDifferentBins::2" ); } } @@ -389,8 +394,8 @@ contract BinPoolLiquidityTest is BinTestHelper { function test_SimpleBurn() external { poolManager.initialize(key, activeId); - uint256 amountX = 100 * 10 ** 18; - uint256 amountY = 100 * 10 ** 18; + uint256 amountX = 100 ether; + uint256 amountY = 100 ether; uint8 nbBinX = 6; uint8 nbBinY = 6; vault.updateCurrentPoolKey(key); @@ -412,25 +417,27 @@ contract BinPoolLiquidityTest is BinTestHelper { vault.updateCurrentPoolKey(key); removeLiquidity(key, poolManager, bob, ids, balances); + // 6 as liquidity was added to 6 bins, each bin lock 1 token due to min share + uint256 lockedTokenDueToMinShare = 6; { // balanceDelta positive (so user need to call take/mint) uint256 balanceDelta0 = uint128(vault.balanceDeltaOfPool(poolId).amount0()); - assertEq(uint256(balanceDelta0), reserveX, "test_SimpleBurn::1"); + assertEq(uint256(balanceDelta0), reserveX - lockedTokenDueToMinShare, "test_SimpleBurn::1"); uint256 balanceDelta1 = uint128(vault.balanceDeltaOfPool(poolId).amount1()); - assertEq(uint256(balanceDelta1), reserveY, "test_SimpleBurn::1"); + assertEq(uint256(balanceDelta1), reserveY - lockedTokenDueToMinShare, "test_SimpleBurn::2"); } reserveX = vault.reservesOfApp(address(key.poolManager), key.currency0); reserveY = vault.reservesOfApp(address(key.poolManager), key.currency1); - assertEq(reserveX, 0, "test_BurnPartial::3"); - assertEq(reserveY, 0, "test_BurnPartial::4"); + assertEq(reserveX, lockedTokenDueToMinShare, "test_BurnPartial::3"); + assertEq(reserveY, lockedTokenDueToMinShare, "test_BurnPartial::4"); } function test_BurnHalfTwice() external { poolManager.initialize(key, activeId); - uint256 amountX = 100 * 10 ** 18; - uint256 amountY = 100 * 10 ** 18; + uint256 amountX = 100 ether; + uint256 amountY = 100 ether; uint8 nbBinX = 6; uint8 nbBinY = 6; @@ -469,10 +476,12 @@ contract BinPoolLiquidityTest is BinTestHelper { removeLiquidity(key, poolManager, bob, ids, halfbalances); + // 6 as liquidity was added to 6 bins, each bin lock 1 token due to min share + uint256 lockedTokenDueToMinShare = 6; reserveX = vault.reservesOfApp(address(key.poolManager), key.currency0); // vault.reservesOfPool(poolId, 0); reserveY = vault.reservesOfApp(address(key.poolManager), key.currency1); // vault.reservesOfPool(poolId, 1); - assertEq(reserveX, 0, "test_BurnPartial::5"); - assertEq(reserveY, 0, "test_BurnPartial::6"); + assertEq(reserveX, lockedTokenDueToMinShare, "test_BurnPartial::5"); + assertEq(reserveY, lockedTokenDueToMinShare, "test_BurnPartial::6"); } function test_revert_BurnEmptyArraysOrDifferent() external {