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/ hexen r7 option 1 optimization #214

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

POC/ hexen r7 option 1 optimization #214

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 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
3d0d39a
feat: Check user position share compared with min
ChefSnoopy Nov 12, 2024
a03e4f9
feat: Remove all shares if the remaining shares fall below the minimu…
ChefSnoopy Nov 12, 2024
24212b4
fix: Update amountToBurn logic
ChefSnoopy 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
178922
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
311556
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
170937
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
410638
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
23612
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
134526
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
143512
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
295386
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
127698
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
971193
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
330505
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
337813
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
140364
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
304852
22 changes: 22 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__UserPositionBelowMinimumShare(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 @@ -313,6 +317,12 @@ library BinPool {
bytes32 binReserves = self.reserveOfBin[id];
uint256 supply = self.shareOfBin[id];

uint256 userTotalShare = self.positions.get(params.from, id, params.salt).share;
/// @dev Remove all shares if the remaining shares fall below the minimum threshold
if (amountToBurn > userTotalShare - MINIMUM_SHARE) {
amountToBurn = userTotalShare;
}

_subShare(self, params.from, id, params.salt, amountToBurn);

bytes32 amountsOutFromBin = binReserves.getAmountOutOfBin(amountToBurn, supply);
Expand Down Expand Up @@ -464,12 +474,24 @@ 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 user's position share is either 0 or greater than minimum share
uint256 userPositionShare = self.positions.get(owner, binId, salt).share;
if (userPositionShare > 0 && userPositionShare < MINIMUM_SHARE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (userPositionShare > 0 && userPositionShare < MINIMUM_SHARE) {
if (userPositionShare < MINIMUM_SHARE) {

I think this will be sufficient

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no , if removed all , userPositionShare will be 0 now , will get revert error

revert BinPool__UserPositionBelowMinimumShare(userPositionShare);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be better to add this part in BinPosition::addShare, BinPosition::subShare:

  1. Conceptually, the check belongs to user position
  2. save some gas since it avoids call position.get twice

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok , this is just a POC. will refactor logic if decide to use this.

}

/// @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 user's position share is either 0 or greater than minimum share
uint256 userPositionShare = self.positions.get(owner, binId, salt).share;
if (userPositionShare < MINIMUM_SHARE) {
revert BinPool__UserPositionBelowMinimumShare(userPositionShare);
}
}

/// @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
2 changes: 1 addition & 1 deletion test/pool-bin/libraries/BinPoolLiquidity.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ contract BinPoolLiquidityTest is BinTestHelper {
ids[0] = activeId;
balances[0] = poolManager.getPosition(poolId, bob, activeId, 0).share + 1;

vm.expectRevert(stdError.arithmeticError);
// vm.expectRevert(stdError.arithmeticError);
removeLiquidity(key, poolManager, bob, ids, balances);
}

Expand Down
Loading