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

Simplify modifyLiquidity #879

Closed
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -1 +1 @@
144651
144730
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170947
171019
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
274218
273346
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
135141
135223
Original file line number Diff line number Diff line change
@@ -1 +1 @@
292843
291971
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 1 token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
106333
106321
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 2 tokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145754
145736
2 changes: 1 addition & 1 deletion .forge-snapshots/erc20 collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
57442
57439
2 changes: 1 addition & 1 deletion .forge-snapshots/extsload getFeeGrowthGlobals.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
777
774
2 changes: 1 addition & 1 deletion .forge-snapshots/extsload getPositionInfo.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
952
949
2 changes: 1 addition & 1 deletion .forge-snapshots/extsload getTickFeeGrowthOutside.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
777
774
2 changes: 1 addition & 1 deletion .forge-snapshots/extsload getTickInfo.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
952
949
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
60025
60021
2 changes: 1 addition & 1 deletion .forge-snapshots/native collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
59726
59723
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24073
23931
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
141229
140746
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
130597
130121
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
112535
112156
Original file line number Diff line number Diff line change
@@ -1 +1 @@
98850
98952
2 changes: 1 addition & 1 deletion .forge-snapshots/simple addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
161395
155893
Original file line number Diff line number Diff line change
@@ -1 +1 @@
92983
93028
2 changes: 1 addition & 1 deletion .forge-snapshots/simple removeLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
85096
84640
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap with native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
108515
108503
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123335
123320
2 changes: 1 addition & 1 deletion .forge-snapshots/swap CA custom curve + swap noop.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124663
124639
2 changes: 1 addition & 1 deletion .forge-snapshots/swap CA fee on unspecified.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
154783
154765
Original file line number Diff line number Diff line change
@@ -1 +1 @@
105637
105625
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
116705
116690
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129269
129257
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn native 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
118725
118713
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint native output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139747
139732
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
155176
155161
Original file line number Diff line number Diff line change
@@ -1 +1 @@
206425
206398
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139356
139341
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
132343
132328
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with lp fee and protocol fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
169603
169585
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with return dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145683
145668
2 changes: 1 addition & 1 deletion .forge-snapshots/update dynamic fee in before swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147966
147948
75 changes: 31 additions & 44 deletions src/libraries/Pool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,19 @@ library Pool {
int24 tickUpper = params.tickUpper;
checkTicks(tickLower, tickUpper);

// accrue fee growth and bring position data up to date
{
(uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) =
getFeeGrowthInside(self, tickLower, tickUpper);

Position.State storage position = self.positions.get(params.owner, tickLower, tickUpper, params.salt);
(uint256 feesOwed0, uint256 feesOwed1) =
position.update(liquidityDelta, feeGrowthInside0X128, feeGrowthInside1X128);

// Fees earned from LPing are calculated, and returned
feeDelta = toBalanceDelta(feesOwed0.toInt128(), feesOwed1.toInt128());
}

{
ModifyLiquidityState memory state;

Expand Down Expand Up @@ -179,28 +192,6 @@ library Pool {
self.tickBitmap.flipTick(tickUpper, params.tickSpacing);
}
}

{
(uint256 feeGrowthInside0X128, uint256 feeGrowthInside1X128) =
getFeeGrowthInside(self, tickLower, tickUpper);

Position.State storage position = self.positions.get(params.owner, tickLower, tickUpper, params.salt);
(uint256 feesOwed0, uint256 feesOwed1) =
position.update(liquidityDelta, feeGrowthInside0X128, feeGrowthInside1X128);

// Fees earned from LPing are calculated, and returned
feeDelta = toBalanceDelta(feesOwed0.toInt128(), feesOwed1.toInt128());
}

// clear any tick data that is no longer needed
if (liquidityDelta < 0) {
if (state.flippedLower) {
clearTick(self, tickLower);
}
if (state.flippedUpper) {
clearTick(self, tickUpper);
}
}
}

if (liquidityDelta != 0) {
Expand Down Expand Up @@ -527,29 +518,25 @@ library Pool {

flipped = (liquidityGrossAfter == 0) != (liquidityGrossBefore == 0);

if (liquidityGrossBefore == 0) {
// by convention, we assume that all growth before a tick was initialized happened _below_ the tick
if (tick <= self.slot0.tick()) {
info.feeGrowthOutside0X128 = self.feeGrowthGlobal0X128;
info.feeGrowthOutside1X128 = self.feeGrowthGlobal1X128;
}
}

// when the lower (upper) tick is crossed left to right (right to left), liquidity must be added (removed)
int128 liquidityNet = upper ? liquidityNetBefore - liquidityDelta : liquidityNetBefore + liquidityDelta;
assembly ("memory-safe") {
// liquidityGrossAfter and liquidityNet are packed in the first slot of `info`
// So we can store them with a single sstore by packing them ourselves first
sstore(
info.slot,
// bitwise OR to pack liquidityGrossAfter and liquidityNet
or(
// Put liquidityGrossAfter in the lower bits, clearing out the upper bits
and(liquidityGrossAfter, 0xffffffffffffffffffffffffffffffff),
// Shift liquidityNet to put it in the upper bits (no need for signextend since we're shifting left)
shl(128, liquidityNet)
if (liquidityGrossAfter == 0) {
clearTick(self, tick);
} else {
// when the lower (upper) tick is crossed left to right (right to left), liquidity must be added (removed)
int128 liquidityNet = upper ? liquidityNetBefore - liquidityDelta : liquidityNetBefore + liquidityDelta;
assembly ("memory-safe") {
// liquidityGrossAfter and liquidityNet are packed in the first slot of `info`
// So we can store them with a single sstore by packing them ourselves first
sstore(
info.slot,
// bitwise OR to pack liquidityGrossAfter and liquidityNet
or(
// Put liquidityGrossAfter in the lower bits, clearing out the upper bits
and(liquidityGrossAfter, 0xffffffffffffffffffffffffffffffff),
// Shift liquidityNet to put it in the upper bits (no need for signextend since we're shifting left)
shl(128, liquidityNet)
)
)
)
}
}
}

Expand Down
18 changes: 9 additions & 9 deletions test/Tick.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -330,14 +330,14 @@ contract TickTest is Test, GasSnapshot {
update(0, 0, int128(Constants.MAX_UINT128 / 2 - 1), 0, 0, false);
}

function testTick_update_assumesAllGrowthHappensBelowTicksLteCurrentTick() public {
function testTick_update_doesNotSetAnyGrowthFieldsIfTickIsNotInitialized() public {
Pool.TickInfo memory tickInfo;

update(1, 1, 1, 1, 2, false);
tickInfo = ticks(1);

assertEq(tickInfo.feeGrowthOutside0X128, 1);
assertEq(tickInfo.feeGrowthOutside1X128, 2);
assertEq(tickInfo.feeGrowthOutside0X128, 0);
assertEq(tickInfo.feeGrowthOutside1X128, 0);
}

function testTick_update_doesNotSetAnyGrowthFieldsIfTickIsAlreadyInitialized() public {
Expand All @@ -347,8 +347,8 @@ contract TickTest is Test, GasSnapshot {
update(1, 1, 1, 6, 7, false);
tickInfo = ticks(1);

assertEq(tickInfo.feeGrowthOutside0X128, 1);
assertEq(tickInfo.feeGrowthOutside1X128, 2);
assertEq(tickInfo.feeGrowthOutside0X128, 0);
assertEq(tickInfo.feeGrowthOutside1X128, 0);
}

function testTick_update_doesNotSetAnyGrowthFieldsForTicksGtCurrentTick() public {
Expand Down Expand Up @@ -401,15 +401,15 @@ contract TickTest is Test, GasSnapshot {

info.feeGrowthOutside0X128 = 0;
info.feeGrowthOutside1X128 = 0;
info.liquidityGross = 1;
info.liquidityGross = uint128(Constants.MAX_UINT128 / 2);
info.liquidityNet = int128(Constants.MAX_UINT128 / 2);

setTick(2, info);
update(2, 1, -1, 1, 2, false);

info = ticks(2);

assertEq(info.liquidityGross, 0);
assertEq(info.liquidityGross, uint128(Constants.MAX_UINT128 / 2 - 1));
assertEq(info.liquidityNet, int128(Constants.MAX_UINT128 / 2 - 1));
}

Expand All @@ -418,7 +418,7 @@ contract TickTest is Test, GasSnapshot {

info.feeGrowthOutside0X128 = 0;
info.feeGrowthOutside1X128 = 0;
info.liquidityGross = 0;
info.liquidityGross = uint128(Constants.MAX_UINT128 / 2 - 1);
info.liquidityNet = int128(Constants.MAX_UINT128 / 2 - 1);

setTick(2, info);
Expand All @@ -427,7 +427,7 @@ contract TickTest is Test, GasSnapshot {

info = ticks(2);

assertEq(info.liquidityGross, 1);
assertEq(info.liquidityGross, uint128(Constants.MAX_UINT128 / 2));
assertEq(info.liquidityNet, int128(Constants.MAX_UINT128 / 2));
}

Expand Down
Loading