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.

Sign up for GitHub

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 of option 1 #213

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

<poc> Example of option 1 #213

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a761527
feat: example of option 1
ChefMist Nov 11, 2024
8f2cdda
feat: add comment
ChefMist Nov 11, 2024
aa9ae77
feat: add guard in _addShare (to discuss)
ChefMist Nov 12, 2024
87906ea
feat: optimise add_share
ChefMist Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testBurnSucceedsWithHook.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
178130
178365
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
311254
311512
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMintBurnFeeHookTest#test_Burn.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170145
170380
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMintBurnFeeHookTest#test_Mint.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
410336
410594
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerBytecodeSize.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23287
23406
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testBurnNativeCurrency.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133892
134080
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasBurnHalfBin.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142717
142975
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasBurnNineBins.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
289683
291375
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
127065
127253
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
968475
970797
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
327787
330109
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
337511
337769
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
140062
140320
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
304550
304808
18 changes: 18 additions & 0 deletions src/pool-bin/libraries/BinPool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__BelowMinimumShare(uint256 balanceShare);

/// @dev The state of a pool
struct State {
Expand All @@ -66,6 +67,9 @@ library BinPool {
mapping(bytes32 => bytes32) level2;
}

/// @dev when liquidity is removed, ensure there is either greater than min_share or 0 liquidity left
uint256 constant MINIMUM_SHARE = 1e9;

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 @@ -464,12 +468,26 @@ library BinPool {
function _subShare(State storage self, address owner, uint24 binId, bytes32 salt, uint256 shares) internal {
self.positions.get(owner, binId, salt).subShare(shares);
self.shareOfBin[binId] -= shares;

/// @dev Ensure bin total share is either 0 or greater than minimum share
uint256 balanceShare = self.shareOfBin[binId];
if (balanceShare > 0 && balanceShare < MINIMUM_SHARE) {
revert BinPool__BelowMinimumShare(balanceShare);
}
Copy link
Collaborator

@ChefSnoopy ChefSnoopy Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

only concern is that user maybe will always get revert when try to withdraw all , if left shares of bin is below MINIMUM_SHARE.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice, this would be a con of this approach

So example would be:

  1. alice added min_share + 1 liquidity = 1e9 + 1 (pool has 1e9 + 1 share)
  2. bob added 10 liquidity (pool has 1e9 + 1 + 10 share)
  3. alice remove 11 liquidity (pool left with 1e9 share)
  4. bob is stuck and cannot remove his share

Copy link
Collaborator

Choose a reason for hiding this comment

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

per discussion with Snoopy, might worth to consider to limit from the perspective of user position instead of the whole bin.

}

/// @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);
self.shareOfBin[binId] += shares;

/// @dev Ensure bin total share is either 0 or greater than minimum share
/// <WIP> to discuss if we want to enforce this -- otherwise user can potentially add below 1e9 liquidity
/// but can't withdraw if there's no other lp to help push the min share up
uint256 balanceShare = self.shareOfBin[binId];
if (balanceShare > 0 && balanceShare < MINIMUM_SHARE) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

[wip] if this option is taken, to discuss if we want this check.

revert BinPool__BelowMinimumShare(balanceShare);
}
}

/// @notice Enable bin id for a pool
Expand Down
5 changes: 3 additions & 2 deletions test/pool-bin/libraries/BinPoolDonate.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ contract BinPoolDonateTest is BinTestHelper {
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);
/// @dev Remove all share leaving between 1e9 and minShareForDinate
/// if its less than 1e9, it will throw BinPool__BelowMinimumShareInBurn error
remainingShare = bound(remainingShare, 1e9, poolManager.minBinShareForDonate() - 1);
uint256 aliceShare = poolManager.getPosition(poolId, alice, activeId, 0).share;
removeLiquidityFromBin(key, poolManager, alice, activeId, aliceShare - remainingShare, "");

Expand Down
Loading