diff --git a/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap index 436357a..84d8794 100644 --- a/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap @@ -1 +1 @@ -178130 \ No newline at end of file +233208 \ No newline at end of file diff --git a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap index 607b6b5..74fcdec 100644 --- a/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap @@ -1 +1 @@ -311254 \ No newline at end of file +311574 \ No newline at end of file diff --git a/.forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap b/.forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap index d6fd559..393a1b9 100644 --- a/.forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap +++ b/.forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap @@ -1 +1 @@ -170145 \ No newline at end of file +221462 \ No newline at end of file diff --git a/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap b/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap index 4128ba0..d4c2ddf 100644 --- a/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap +++ b/.forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap @@ -1 +1 @@ -410336 \ No newline at end of file +410526 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerBytecodeSize.snap b/.forge-snapshots/BinPoolManagerBytecodeSize.snap index c050174..09c5ab4 100644 --- a/.forge-snapshots/BinPoolManagerBytecodeSize.snap +++ b/.forge-snapshots/BinPoolManagerBytecodeSize.snap @@ -1 +1 @@ -23287 \ No newline at end of file +23645 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap index 0257459..0269b4b 100644 --- a/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap +++ b/.forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap @@ -1 +1 @@ -133892 \ No newline at end of file +183557 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap index f53d2c9..61fe680 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap @@ -1 +1 @@ -142717 \ No newline at end of file +143406 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap index 994141b..440460f 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap @@ -1 +1 @@ -289683 \ No newline at end of file +330965 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap b/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap index ba16a41..ec4240e 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap @@ -1 +1 @@ -127065 \ No newline at end of file +175023 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap index dc44a72..bafeff3 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap @@ -1 +1 @@ -968475 \ No newline at end of file +971323 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap index 4a863ea..494e0e1 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap @@ -1 +1 @@ -327787 \ No newline at end of file +330068 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap index f71e10b..48956a3 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap @@ -1 +1 @@ -337511 \ No newline at end of file +337831 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap index 140b1dd..98d849b 100644 --- a/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap +++ b/.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap @@ -1 +1 @@ -140062 \ No newline at end of file +140319 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap index f9388fc..488b3ff 100644 --- a/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap +++ b/.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap @@ -1 +1 @@ -304550 \ No newline at end of file +304870 \ No newline at end of file diff --git a/src/pool-bin/BinPoolManager.sol b/src/pool-bin/BinPoolManager.sol index 1f77e0f..2859094 100644 --- a/src/pool-bin/BinPoolManager.sol +++ b/src/pool-bin/BinPoolManager.sol @@ -239,7 +239,8 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload { uint256[] memory binIds; bytes32[] memory amountRemoved; - (delta, binIds, amountRemoved) = pool.burn( + bytes32 feeAmountToProtocol; + (delta, binIds, amountRemoved, feeAmountToProtocol) = pool.burn( BinPool.BurnParams({ from: msg.sender, ids: params.ids, @@ -248,6 +249,13 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload { }) ); + unchecked { + if (feeAmountToProtocol > 0) { + protocolFeesAccrued[key.currency0] += feeAmountToProtocol.decodeX(); + protocolFeesAccrued[key.currency1] += feeAmountToProtocol.decodeY(); + } + } + /// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one emit Burn(id, msg.sender, binIds, params.salt, amountRemoved); diff --git a/src/pool-bin/interfaces/IBinPoolManager.sol b/src/pool-bin/interfaces/IBinPoolManager.sol index e573717..d228034 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 8fb7708..d7047e7 100644 --- a/src/pool-bin/libraries/BinPool.sol +++ b/src/pool-bin/libraries/BinPool.sol @@ -66,6 +66,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(); @@ -294,7 +298,7 @@ library BinPool { /// @return result the delta of the token balance of the pool function burn(State storage self, BurnParams memory params) internal - returns (BalanceDelta result, uint256[] memory ids, bytes32[] memory amounts) + returns (BalanceDelta result, uint256[] memory ids, bytes32[] memory amounts, bytes32 feeAmountToProtocol) { ids = params.ids; uint256 idsLength = ids.length; @@ -313,15 +317,26 @@ library BinPool { bytes32 binReserves = self.reserveOfBin[id]; uint256 supply = self.shareOfBin[id]; - _subShare(self, params.from, id, params.salt, amountToBurn); - - bytes32 amountsOutFromBin = binReserves.getAmountOutOfBin(amountToBurn, supply); - - if (amountsOutFromBin == 0) revert BinPool__ZeroAmountsOut(id); - - binReserves = binReserves.sub(amountsOutFromBin); - - if (supply == amountToBurn) _removeBinIdToTree(self, id); + /// @notice if user is last lp for this bin, amountToBurn would be amountToBurn + MINIMUM_SHARE + amountToBurn = _subShare(self, params.from, id, params.salt, amountToBurn); + + bytes32 amountsOutFromBin; + /// @dev if user is last lp for this bin, supply == amountToBurn == amountsToBurn[i] + MINIMUM_SHARE + /// then we will remove all the liqudity from the bin where MINIMUM_SHARE will be withdrawn as protocol fee + /// to prevent MINIMUM_SHARE from being locked up in the bin which cause extra gas cost when swap + if (supply == amountToBurn) { + _removeBinIdToTree(self, id); + + /// @notice withdraw all the liquidity from the bin, the locked up share will be withdrawn as protocol fee + /// @dev we can't use "binReserves.getAmountOutOfBin(MINIMUM_SHARE, supply)" to calc the fee as it will round down to 0 + amountsOutFromBin = binReserves.getAmountOutOfBin(amountToBurn - MINIMUM_SHARE, supply); + feeAmountToProtocol = feeAmountToProtocol.add(binReserves.sub(amountsOutFromBin)); + binReserves = 0; + } else { + amountsOutFromBin = binReserves.getAmountOutOfBin(amountToBurn, supply); + binReserves = binReserves.sub(amountsOutFromBin); + } + if (amountsOutFromBin.add(feeAmountToProtocol) == 0) revert BinPool__ZeroAmountsOut(id); self.reserveOfBin[id] = binReserves; amounts[i] = amountsOutFromBin; @@ -385,12 +400,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 { @@ -461,14 +476,35 @@ library BinPool { } /// @notice Subtract share from user's position and update total share supply of bin - function _subShare(State storage self, address owner, uint24 binId, bytes32 salt, uint256 shares) internal { + /// @param shares - amount of share to deduct specified by user + /// @return amountsToBurn amount of share burned, nornally equals to shares except when bin is left with MINIMUM_SHARE, amountsToBurn will be share + MINIMUM_SHARE + function _subShare(State storage self, address owner, uint24 binId, bytes32 salt, uint256 shares) + internal + returns (uint256 amountsToBurn) + { self.positions.get(owner, binId, salt).subShare(shares); - self.shareOfBin[binId] -= shares; + amountsToBurn = shares; + if (self.shareOfBin[binId] - shares == MINIMUM_SHARE) { + amountsToBurn += MINIMUM_SHARE; + } + self.shareOfBin[binId] -= amountsToBurn; } /// @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 930a5e6..60f7029 100644 --- a/test/pool-bin/BinHookReturnsDelta.t.sol +++ b/test/pool-bin/BinHookReturnsDelta.t.sol @@ -127,11 +127,15 @@ contract BinHookReturnsDelta is Test, GasSnapshot, BinTestHelper { binLiquidityHelper.burn(key, burnParams, ""); (uint128 reserveXAfter, uint128 reserveYAfter,,) = poolManager.getBin(key.toId(), activeId); - + // reserve back to zero due to min liquidity locked up in the bin will be withdrawn as protocol fee assertEq(reserveXAfter, 0); assertEq(reserveYAfter, 0); - assertEq(token0.balanceOf(address(binReturnsDeltaHook)), 0.1 ether); - assertEq(token1.balanceOf(address(binReturnsDeltaHook)), 0.1 ether); + assertEq(token0.balanceOf(address(binReturnsDeltaHook)), 0.1 ether - 1); + assertEq(token1.balanceOf(address(binReturnsDeltaHook)), 0.1 ether - 1); + + // check protocol fee increase + assertEq(poolManager.protocolFeesAccrued(key.currency0), 1); + assertEq(poolManager.protocolFeesAccrued(key.currency1), 1); } function testSwap_noSwap_specifyInput() external { diff --git a/test/pool-bin/BinMintBurnFeeHook.t.sol b/test/pool-bin/BinMintBurnFeeHook.t.sol index b85548c..a4d4a5a 100644 --- a/test/pool-bin/BinMintBurnFeeHook.t.sol +++ b/test/pool-bin/BinMintBurnFeeHook.t.sol @@ -104,7 +104,7 @@ contract BinMintBurnFeeHookTest is Test, GasSnapshot, BinTestHelper { token1.mint(address(this), 10 ether); IBinPoolManager.MintParams memory mintParams = _getSingleBinMintParams(activeId, 1 ether, 1 ether); - BalanceDelta delta = binLiquidityHelper.mint(key, mintParams, abi.encode(0)); + binLiquidityHelper.mint(key, mintParams, abi.encode(0)); assertEq(token0.balanceOf(address(this)), 7 ether); assertEq(token1.balanceOf(address(this)), 7 ether); @@ -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 ether from remove liqudiity, -4 ether 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 ether from remove liquidity, +4 ether from hook calling vault.mint + // -3 as min_liquidity amount as 1 (min_liquidity) still in vault but hook's fee is less 1 * 4 (fee) + assertEq(token0.balanceOf(address(vault)), 3 ether - 1 ether + 4 ether - 3); + assertEq(token1.balanceOf(address(vault)), 3 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); + 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 fc7a11a..1e40b15 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, 0); + } else if (binIds[i] > activeId) { + assertEq(binReserveX, 0); + assertEq(binReserveY, 0); + } else { + assertEq(binReserveX, 0); + assertEq(binReserveY, 0); + } 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,8 @@ 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)); + // lock-up 1e3 share are not burned by user but instead withdrawn as protocol fee + amounts[0] = uint128(1e18 - 1).encode(uint128(1e18 - 1)); vm.expectEmit(); emit IBinPoolManager.Burn(key.toId(), address(binLiquidityHelper), ids, 0, amounts); @@ -697,7 +711,8 @@ 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)); + // lock-up 1e3 share are not burned by user but instead withdrawn as protocol fee + amounts[0] = uint128(1e18 - 1).encode(uint128(1e18 - 1)); vm.expectEmit(); emit IBinPoolManager.Burn(key.toId(), address(binLiquidityHelper), ids, 0, amounts); diff --git a/test/pool-bin/BinPoolManagerBehaviorComparison.t.sol b/test/pool-bin/BinPoolManagerBehaviorComparison.t.sol index a484676..89cbfad 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 373a86e..a051bf8 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 9efc1dd..f37aac3 100644 --- a/test/pool-bin/libraries/BinPoolDonate.t.sol +++ b/test/pool-bin/libraries/BinPoolDonate.t.sol @@ -57,16 +57,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, ""); } @@ -81,9 +87,11 @@ contract BinPoolDonateTest is BinTestHelper { // Verify reserve before donate uint128 reserveX; uint128 reserveY; - (reserveX, reserveY,,) = poolManager.getBin(poolId, activeId); + uint256 totalSupply; + (reserveX, reserveY,, totalSupply) = poolManager.getBin(poolId, activeId); assertEq(reserveX, 2e18); assertEq(reserveY, 2e18); + assertEq(totalSupply, aliceShare + bobShare + BinPool.MINIMUM_SHARE); // Donate poolManager.donate(key, 2e18, 2e18, ""); @@ -98,15 +106,20 @@ 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 - (reserveX, reserveY,,) = poolManager.getBin(poolId, activeId); + // can see the reserve is back to 0 after all liquidity has been removed + (reserveX, reserveY,, totalSupply) = poolManager.getBin(poolId, activeId); assertEq(reserveX, 0); assertEq(reserveY, 0); + assertEq(totalSupply, 0); + assertEq(poolManager.protocolFeesAccrued(key.currency0), 1); + assertEq(poolManager.protocolFeesAccrued(key.currency1), 1); + // Verify min_liquidity worth of token locked up is accumulated to protocol fee vm.expectRevert(abi.encodeWithSelector(IBinPoolManager.InsufficientBinShareForDonate.selector, 0)); poolManager.donate(key, 1e18, 1e18, ""); } diff --git a/test/pool-bin/libraries/BinPoolFee.t.sol b/test/pool-bin/libraries/BinPoolFee.t.sol index 4e75903..01ec5da 100644 --- a/test/pool-bin/libraries/BinPoolFee.t.sol +++ b/test/pool-bin/libraries/BinPoolFee.t.sol @@ -325,7 +325,7 @@ contract BinPoolFeeTest is BinTestHelper { addLiquidityToBin(_key, poolManager, bob, activeId, amountX, amountY, 1e18, 1e18, ""); } - function test_Burn_NoFee() external { + function test_Burn_MinimumShareAsFee() external { // add liqudiity uint24 activeId = ID_ONE; // where token price are the same _addLiquidityForBurnTest(activeId, key); @@ -337,9 +337,29 @@ contract BinPoolFeeTest is BinTestHelper { ids[0] = activeId; removeLiquidity(key, poolManager, bob, ids, balances); - // check fee. no hook for pool, so can skip check - assertEq(poolManager.protocolFeesAccrued(currency0), 0, "test_Burn_NoFee::1"); - assertEq(poolManager.protocolFeesAccrued(currency1), 0, "test_Burn_NoFee::1"); + // check fee as minimum share will be withdrawn as fee + // no hook for pool, so can skip check + assertEq(poolManager.protocolFeesAccrued(currency0), 1, "test_Burn_MinimumShareAsFee::1"); + assertEq(poolManager.protocolFeesAccrued(currency1), 1, "test_Burn_MinimumShareAsFee::1"); + } + + function test_Burn_NoMinimumShareAsFee() external { + // add liqudiity + uint24 activeId = ID_ONE; // where token price are the same + _addLiquidityForBurnTest(activeId, key); + + // then remove liquidity + uint256[] memory balances = new uint256[](1); + uint256[] memory ids = new uint256[](1); + // remove 1 share less than what was minted, so that MINIMUM_SHARE is not withdrawn as fee + balances[0] = poolManager.getPosition(poolId, bob, activeId, 0).share - 1; + ids[0] = activeId; + removeLiquidity(key, poolManager, bob, ids, balances); + + // check fee as minimum share will not be withdrawn as fee as there are still 1 user's share left + // no hook for pool, so can skip check + assertEq(poolManager.protocolFeesAccrued(currency0), 0, "test_Burn_NoMinimumShareAsFee::1"); + assertEq(poolManager.protocolFeesAccrued(currency1), 0, "test_Burn_NoMinimumShareAsFee::1"); } function test_Swap_NoFee() external { diff --git a/test/pool-bin/libraries/BinPoolLiquidity.t.sol b/test/pool-bin/libraries/BinPoolLiquidity.t.sol index b71537f..c4a3427 100644 --- a/test/pool-bin/libraries/BinPoolLiquidity.t.sol +++ b/test/pool-bin/libraries/BinPoolLiquidity.t.sol @@ -167,10 +167,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"); } } @@ -190,8 +191,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 { @@ -226,8 +228,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" ); } } @@ -288,8 +293,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); @@ -311,25 +316,31 @@ 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_SimpleBurn::3"); + assertEq(reserveY, lockedTokenDueToMinShare, "test_SimpleBurn::4"); + + // make sure all those locked up tokens are now protocol fee + assertEq(poolManager.protocolFeesAccrued(key.currency0), lockedTokenDueToMinShare, "test_SimpleBurn::5"); + assertEq(poolManager.protocolFeesAccrued(key.currency1), lockedTokenDueToMinShare, "test_SimpleBurn::6"); } 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; @@ -368,10 +379,16 @@ 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"); + + // make sure all those locked up tokens are now protocol fee + assertEq(poolManager.protocolFeesAccrued(key.currency0), lockedTokenDueToMinShare, "test_BurnPartial::7"); + assertEq(poolManager.protocolFeesAccrued(key.currency1), lockedTokenDueToMinShare, "test_BurnPartial::8"); } function test_revert_BurnEmptyArraysOrDifferent() external {