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

[PART-2] support same owner multiple positions #46

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1 +1 @@
181453
182049
Original file line number Diff line number Diff line change
@@ -1 +1 @@
185322
185296
Original file line number Diff line number Diff line change
@@ -1 +1 @@
137514
137576
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testMintSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
330672
331250
2 changes: 1 addition & 1 deletion .forge-snapshots/BinHookTest#testSwapSucceedsWithHook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
193574
193526
Original file line number Diff line number Diff line change
@@ -1 +1 @@
137944
138533
Original file line number Diff line number Diff line change
@@ -1 +1 @@
29761
32551
Original file line number Diff line number Diff line change
@@ -1 +1 @@
30350
30417
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147904
148635
Original file line number Diff line number Diff line change
@@ -1 +1 @@
295588
296486
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
131153
131743
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasDonate.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
120046
120042
Original file line number Diff line number Diff line change
@@ -1 +1 @@
976344
977200
Original file line number Diff line number Diff line change
@@ -1 +1 @@
338538
339390
Original file line number Diff line number Diff line change
@@ -1 +1 @@
342112
342867
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145053
145810
Original file line number Diff line number Diff line change
@@ -1 +1 @@
180358
180348
Original file line number Diff line number Diff line change
@@ -1 +1 @@
186343
186331
Original file line number Diff line number Diff line change
@@ -1 +1 @@
138725
138711
Original file line number Diff line number Diff line change
@@ -1 +1 @@
308628
309383
Original file line number Diff line number Diff line change
@@ -1 +1 @@
34422
34487
Original file line number Diff line number Diff line change
@@ -1 +1 @@
362191
362973
Original file line number Diff line number Diff line change
@@ -1 +1 @@
177359
178141
Original file line number Diff line number Diff line change
@@ -1 +1 @@
247478
248256
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#donateBothTokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170385
170426
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
114308
114349
Original file line number Diff line number Diff line change
@@ -1 +1 @@
59812
59779
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123719
124483
Original file line number Diff line number Diff line change
@@ -1 +1 @@
141376
141394
Original file line number Diff line number Diff line change
@@ -1 +1 @@
175053
175073
Original file line number Diff line number Diff line change
@@ -1 +1 @@
25079271
25093869
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#swap_simple.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
79245
79260
Original file line number Diff line number Diff line change
@@ -1 +1 @@
153591
153609
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#swap_withHooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
95978
95925
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#swap_withNative.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
79248
79263
2 changes: 1 addition & 1 deletion .forge-snapshots/ExtsloadTest#extsload.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7468
7446
21 changes: 14 additions & 7 deletions src/pool-bin/BinPoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
}

/// @inheritdoc IBinPoolManager
function getPosition(PoolId id, address owner, uint24 binId)
function getPosition(PoolId id, address owner, uint24 binId, bytes32 salt)
external
view
override
returns (BinPosition.Info memory position)
{
return pools[id].positions.get(owner, binId);
return pools[id].positions.get(owner, binId, salt);
}

/// @inheritdoc IBinPoolManager
Expand Down Expand Up @@ -248,7 +248,8 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
to: msg.sender,
liquidityConfigs: params.liquidityConfigs,
amountIn: params.amountIn,
binStep: key.parameters.getBinStep()
binStep: key.parameters.getBinStep(),
salt: params.salt
})
);

Expand All @@ -260,7 +261,7 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
}

/// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one
emit Mint(id, msg.sender, mintArray.ids, mintArray.amounts, compositionFee, feeForProtocol);
emit Mint(id, msg.sender, mintArray.ids, params.salt, mintArray.amounts, compositionFee, feeForProtocol);

BalanceDelta hookDelta;
(delta, hookDelta) = BinHooks.afterMint(key, params, delta, hookData);
Expand Down Expand Up @@ -290,11 +291,17 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {

uint256[] memory binIds;
bytes32[] memory amountRemoved;
(delta, binIds, amountRemoved) =
pools[id].burn(BinPool.BurnParams({from: msg.sender, ids: params.ids, amountsToBurn: params.amountsToBurn}));
(delta, binIds, amountRemoved) = pools[id].burn(
BinPool.BurnParams({
from: msg.sender,
ids: params.ids,
amountsToBurn: params.amountsToBurn,
salt: params.salt
})
);

/// @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, amountRemoved);
emit Burn(id, msg.sender, binIds, params.salt, amountRemoved);

BalanceDelta hookDelta;
(delta, hookDelta) = BinHooks.afterBurn(key, params, delta, hookData);
Expand Down
12 changes: 10 additions & 2 deletions src/pool-bin/interfaces/IBinPoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,15 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload {
/// @param id The abi encoded hash of the pool key struct for the pool that was modified
/// @param sender The address that modified the pool
/// @param ids List of binId with liquidity added
/// @param salt The salt to distinguish different mint from the same owner
/// @param amounts List of amount added to each bin
/// @param compositionFee fee occurred
/// @param pFee Protocol fee from the swap: token0 and token1 amount
event Mint(
PoolId indexed id,
address indexed sender,
uint256[] ids,
bytes32 salt,
Copy link
Contributor

Choose a reason for hiding this comment

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

just wondering if there will be cases requiring bytes32[] salts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess that might make Mint quite complicated 😂... ppl could actually separate it into multiple calls if they want to do so

bytes32[] amounts,
bytes32 compositionFee,
bytes32 pFee
Expand All @@ -89,8 +91,9 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload {
/// @param id The abi encoded hash of the pool key struct for the pool that was modified
/// @param sender The address that modified the pool
/// @param ids List of binId with liquidity removed
/// @param salt The salt to specify the position to burn if multiple positions are available
/// @param amounts List of amount removed from each bin
event Burn(PoolId indexed id, address indexed sender, uint256[] ids, bytes32[] amounts);
event Burn(PoolId indexed id, address indexed sender, uint256[] ids, bytes32 salt, bytes32[] amounts);
Copy link
Contributor

Choose a reason for hiding this comment

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

just wondering if there will be cases requiring bytes32[] salts?


/// @notice Emitted when donate happen
/// @param id The abi encoded hash of the pool key struct for the pool that was modified
Expand All @@ -107,13 +110,17 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload {
bytes32[] liquidityConfigs;
/// @dev amountIn intended
bytes32 amountIn;
/// the salt to distinguish different mint from the same owner
bytes32 salt;
}

struct BurnParams {
/// @notice id of the bin from which to withdraw
uint256[] ids;
/// @notice amount of share to burn for each bin
uint256[] amountsToBurn;
/// the salt to specify the position to burn if multiple positions are available
bytes32 salt;
}

/// @notice Get the current value in slot0 of the given pool
Expand All @@ -129,7 +136,8 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload {
/// @param id The id of PoolKey
/// @param owner Address of the owner
/// @param binId The id of the bin
function getPosition(PoolId id, address owner, uint24 binId)
/// @param salt The salt to distinguish different positions for the same owner
function getPosition(PoolId id, address owner, uint24 binId, bytes32 salt)
external
view
returns (BinPosition.Info memory position);
Expand Down
1 change: 1 addition & 0 deletions src/pool-bin/libraries/BinHooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ library BinHooks {
revert Hooks.InvalidHookResponse();
}

// TODO: Potentially optimization: skip decoding the second return value when afterSwapReturnDelta not set
if (!key.parameters.hasOffsetEnabled(HOOKS_AFTER_SWAP_RETURNS_DELTA_OFFSET)) {
hookDeltaUnspecified = 0;
}
Expand Down
30 changes: 21 additions & 9 deletions src/pool-bin/libraries/BinPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ library BinPool {
bytes32[] liquidityConfigs;
bytes32 amountIn;
uint16 binStep;
bytes32 salt;
}

struct MintArrays {
Expand Down Expand Up @@ -318,6 +319,7 @@ library BinPool {
address from;
uint256[] ids;
uint256[] amountsToBurn;
bytes32 salt;
}

/// @notice Burn user's share and withdraw tokens form the pool.
Expand All @@ -342,7 +344,7 @@ library BinPool {
bytes32 binReserves = self.reserveOfBin[id];
uint256 supply = self.shareOfBin[id];

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

bytes32 amountsOutFromBin = binReserves.getAmountOutOfBin(amountToBurn, supply);

Expand Down Expand Up @@ -395,11 +397,21 @@ library BinPool {
{
amountsLeft = params.amountIn;

uint24 id;
uint256 shares;
bytes32 amountsIn;
bytes32 amountsInToBin;
bytes32 binFeeAmt;
bytes32 binCompositionFee;
for (uint256 i; i < params.liquidityConfigs.length;) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Screenshot 2024-05-21 at 10 27 37

did a check, seems like moving declaration outside of for loop could save gas, could you give it a try? thanks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

checked and it did help to save gas.. thx for reminding

(bytes32 maxAmountsInToBin, uint24 id) = params.liquidityConfigs[i].getAmountsAndId(params.amountIn);
// fix stack too deep
{
bytes32 maxAmountsInToBin;
(maxAmountsInToBin, id) = params.liquidityConfigs[i].getAmountsAndId(params.amountIn);

(uint256 shares, bytes32 amountsIn, bytes32 amountsInToBin, bytes32 binFeeAmt, bytes32 binCompositionFee) =
_updateBin(self, params, id, maxAmountsInToBin);
(shares, amountsIn, amountsInToBin, binFeeAmt, binCompositionFee) =
_updateBin(self, params, id, maxAmountsInToBin);
}

amountsLeft = amountsLeft.sub(amountsIn);
feeForProtocol = feeForProtocol.add(binFeeAmt);
Expand All @@ -408,7 +420,7 @@ library BinPool {
arrays.amounts[i] = amountsInToBin;
arrays.liquidityMinted[i] = shares;

_addShare(self, params.to, id, shares);
_addShare(self, params.to, id, params.salt, shares);

compositionFee = compositionFee.add(binCompositionFee);

Expand Down Expand Up @@ -475,14 +487,14 @@ 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, uint256 shares) internal {
self.positions.get(owner, binId).subShare(shares);
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;
}

/// @notice Add share to user's position and update total share supply of bin
function _addShare(State storage self, address owner, uint24 binId, uint256 shares) internal {
self.positions.get(owner, binId).addShare(shares);
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;
}

Expand Down
Loading
Loading