Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[POC]: example if we remove share to 0 if only lockup shares are left #217

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
178130
233208
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
311254
311574
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170145
221462
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
410336
410526
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerBytecodeSize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23287
23645
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133892
183557
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142717
143406
Original file line number Diff line number Diff line change
@@ -1 +1 @@
289683
330965
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
127065
175023
Original file line number Diff line number Diff line change
@@ -1 +1 @@
968475
971323
Original file line number Diff line number Diff line change
@@ -1 +1 @@
327787
330068
Original file line number Diff line number Diff line change
@@ -1 +1 @@
337511
337831
Original file line number Diff line number Diff line change
@@ -1 +1 @@
140062
140319
Original file line number Diff line number Diff line change
@@ -1 +1 @@
304550
304870
10 changes: 9 additions & 1 deletion src/pool-bin/BinPoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);

Expand Down
2 changes: 2 additions & 0 deletions src/pool-bin/interfaces/IBinPoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
68 changes: 52 additions & 16 deletions src/pool-bin/libraries/BinPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Copy link
Collaborator

@ChefMist ChefMist Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// 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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Copy link
Collaborator

@ChefMist ChefMist Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need comment on this -- so other devs can understand the logic below easily

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eg.

    /// @notice Subtract share from user's position and update total share supply of bin
    /// @param shares - amount of share to deduct 
    /// @return amountsToBurn amount of share burned, if bin is left with MINIMUM_SHARE, amountsToBurn will be share + MINIMUM_SHARE

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;
}

Expand Down
10 changes: 7 additions & 3 deletions test/pool-bin/BinHookReturnsDelta.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
22 changes: 13 additions & 9 deletions test/pool-bin/BinMintBurnFeeHook.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {}
Expand Down
39 changes: 27 additions & 12 deletions test/pool-bin/BinPoolManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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, "");
Expand Down Expand Up @@ -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, "");

Expand Down Expand Up @@ -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, "");

Expand All @@ -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);
}
}

Expand All @@ -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 =
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
Loading
Loading