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

Optimize liquidity flipping calculations in Pool #602

Closed
wants to merge 9 commits into from
Original file line number Diff line number Diff line change
@@ -1 +1 @@
149323
149188
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
325681
325546
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 @@
279677
279542
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 @@
139505
139370
1 change: 1 addition & 0 deletions .forge-snapshots/addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
139302
Original file line number Diff line number Diff line change
@@ -1 +1 @@
297501
297366
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
19769
19688
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
181200
181065
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 @@
135371
135236
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 @@
115664
115529
Original file line number Diff line number Diff line change
@@ -1 +1 @@
102765
102630
2 changes: 1 addition & 1 deletion .forge-snapshots/simple addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
165257
165122
Original file line number Diff line number Diff line change
@@ -1 +1 @@
96455
96320
2 changes: 1 addition & 1 deletion .forge-snapshots/simple removeLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
88495
88360
19 changes: 18 additions & 1 deletion src/libraries/LiquidityMath.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ pragma solidity ^0.8.20;
/// @title Math library for liquidity
library LiquidityMath {
/// @notice Add a signed liquidity delta to liquidity and revert if it overflows or underflows
/// @dev Equivalent to `z = y < 0 ? x - uint128(-y) : x + uint128(y);`
/// @param x The liquidity before change
/// @param y The delta by which liquidity should be changed
/// @return z The liquidity delta
/// @return z The liquidity after
function addDelta(uint128 x, int128 y) internal pure returns (uint128 z) {
/// @solidity memory-safe-assembly
assembly {
z := add(x, y)
if shr(128, z) {
Expand All @@ -17,4 +19,19 @@ library LiquidityMath {
}
}
}

/// @notice Flips the sign of a liquidity delta if a condition is true
/// @dev Equivalent to `res = flip ? -liquidityDelta : liquidityDelta;`
/// @dev If flipping `type(int128).min`, `1 << 127` is returned which is a valid `int256`
/// @param liquidityDelta The liquidity delta to potentially flip
/// @param flip Whether to flip the sign of the liquidity delta
/// @return res The potentially flipped liquidity delta
function flipLiquidityDelta(int128 liquidityDelta, bool flip) internal pure returns (int256 res) {
assembly {
shuhuiluo marked this conversation as resolved.
Show resolved Hide resolved
// if flip = true, res = -liquidityDelta = ~liquidityDelta + 1 = (-1) ^ liquidityDelta + 1
// if flip = false, res = liquidityDelta = 0 ^ liquidityDelta + 0
// therefore, res = (-flip) ^ liquidityDelta + flip
res := add(xor(sub(0, flip), liquidityDelta), flip)
}
}
}
27 changes: 20 additions & 7 deletions src/libraries/Pool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -404,11 +404,11 @@ library Pool {
: (self.feeGrowthGlobal0X128, state.feeGrowthGlobalX128);
int128 liquidityNet =
Pool.crossTick(self, step.tickNext, feeGrowthGlobal0X128, feeGrowthGlobal1X128);
// if we're moving leftward, we interpret liquidityNet as the opposite sign
// safe because liquidityNet cannot be type(int128).min
unchecked {
if (zeroForOne) liquidityNet = -liquidityNet;
}
// if we're moving leftward, we interpret `liquidityNet` as the opposite sign
// `flipLiquidityDelta` can handle `type(int128).min` and return `1 << 127` as a valid `int256`
// the soft wrap to `int128` is safe because `liquidityNet` is immediately consumed by `addDelta`
// written in inline assembly
liquidityNet = int128(LiquidityMath.flipLiquidityDelta(liquidityNet, zeroForOne));

state.liquidity = LiquidityMath.addDelta(state.liquidity, liquidityNet);
}
Expand Down Expand Up @@ -529,7 +529,10 @@ library Pool {

liquidityGrossAfter = LiquidityMath.addDelta(liquidityGrossBefore, liquidityDelta);

flipped = (liquidityGrossAfter == 0) != (liquidityGrossBefore == 0);
// Equivalent to `flipped = (liquidityGrossAfter == 0) != (liquidityGrossBefore == 0);`
assembly {
flipped := xor(iszero(liquidityGrossAfter), iszero(liquidityGrossBefore))
}

if (liquidityGrossBefore == 0) {
// by convention, we assume that all growth before a tick was initialized happened _below_ the tick
Expand All @@ -540,7 +543,17 @@ library Pool {
}

// 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;
// Equivalent to `liquidityNet = upper ? liquidityNetBefore - liquidityDelta : liquidityNetBefore + liquidityDelta;`
// `int128 liquidityDelta` is passed from `modifyLiquidity` and is sanitized in `PoolManager`
// `flipLiquidityDelta` can handle `type(int128).min` and return `1 << 127` as a valid `int256`
int256 _liquidityDelta = LiquidityMath.flipLiquidityDelta(liquidityDelta, upper);
// declare an int256 to prevent implicit conversion when calling toInt128
int256 liquidityNet;
assembly {
liquidityNet := add(liquidityNetBefore, _liquidityDelta)
}
// ensure the sum is a valid `int128`
liquidityNet.toInt128();
assembly {
// 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
Expand Down
64 changes: 53 additions & 11 deletions test/libraries/LiquidityMath.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
pragma solidity ^0.8.0;

import {Test} from "forge-std/Test.sol";
import {SafeCast} from "src/libraries/SafeCast.sol";
import {LiquidityMathTest as LiquidityMath} from "src/test/LiquidityMathTest.sol";
import {SafeCast} from "../../src/libraries/SafeCast.sol";
import {LiquidityMath} from "../../src/libraries/LiquidityMath.sol";
import {LiquidityMathTest as LiquidityMathMock} from "../../src/test/LiquidityMathTest.sol";

contract LiquidityMathRef {
function addDelta(uint128 x, int128 y) external pure returns (uint128) {
Expand All @@ -12,12 +13,15 @@ contract LiquidityMathRef {
}

contract LiquidityMathTest is Test {
LiquidityMath internal liquidityMath;
uint128 internal constant NEG_INT128_MIN = uint128(type(int128).min);

LiquidityMathMock internal liquidityMath;
LiquidityMathRef internal liquidityMathRef;

function setUp() public {
liquidityMath = new LiquidityMath();
liquidityMath = new LiquidityMathMock();
liquidityMathRef = new LiquidityMathRef();
assertEq(NEG_INT128_MIN, 1 << 127);
}

/// @notice Test the revert reason for underflow
Expand All @@ -34,19 +38,25 @@ contract LiquidityMathTest is Test {
liquidityMath.addDelta(type(uint128).max, 1);
}

/// @notice Test the ternary expression reverts when subtracting `type(int128).min`
function test_addDelta_sub_int128min_throwsForReferenceOnly() public {
assertEq(liquidityMath.addDelta(uint128(type(int128).min), type(int128).min), 0);
assertEq(liquidityMath.addDelta(NEG_INT128_MIN, type(int128).min), 0);
vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x11));
liquidityMathRef.addDelta(uint128(type(int128).min), type(int128).min);
liquidityMathRef.addDelta(NEG_INT128_MIN, type(int128).min);
}

function test_addDelta_sub_int128min_fuzz(uint128 x) public view {
x = uint128(bound(x, uint128(type(int128).min), type(uint128).max));
assertEq(liquidityMath.addDelta(x, type(int128).min), x - uint128(type(int128).min));
/// @notice Test the assembly implementation of `addDelta` with `type(int128).min`
function test_fuzz_addDelta_sub_int128min(uint128 x) public {
if (x < NEG_INT128_MIN) {
vm.expectRevert(SafeCast.SafeCastOverflow.selector);
liquidityMath.addDelta(x, type(int128).min);
} else {
assertEq(liquidityMath.addDelta(x, type(int128).min), x - NEG_INT128_MIN);
}
}

/// @notice Test the equivalence of the new `addDelta` and the reference implementation
function test_addDelta_fuzz(uint128 x, int128 y) public {
/// @notice Test the equivalence of `addDelta` and the reference implementation
function test_fuzz_addDelta(uint128 x, int128 y) public {
vm.assume(y != type(int128).min);
try liquidityMath.addDelta(x, y) returns (uint128 z) {
assertEq(z, liquidityMathRef.addDelta(x, y));
Expand All @@ -56,4 +66,36 @@ contract LiquidityMathTest is Test {
liquidityMathRef.addDelta(x, y);
}
}

/// @notice Test `flipLiquidityDelta` against the ternary expression
function test_fuzz_flipLiquidityDelta(int128 liquidityDelta, bool flip) public pure {
assertEq(
LiquidityMath.flipLiquidityDelta(liquidityDelta, flip),
flip ? -int256(liquidityDelta) : int256(liquidityDelta)
);
}

/// @notice Test the usage of `flipLiquidityDelta` on `type(int128).min` followed by `addDelta`
function test_fuzz_flipLiquidityDelta_int128min(uint128 liquidity) public {
int256 liquidityNet = LiquidityMath.flipLiquidityDelta(type(int128).min, true);
assertEq(liquidityNet, 1 << 127);
// soft wrap to int128
int128 liquidityNet128 = int128(liquidityNet);
// implicit upcast to int256 involves sign extension
assertEq(liquidityNet128, type(int128).min);
int256 _liquidityNet;
assembly {
// direct stack assignment
_liquidityNet := liquidityNet128
}
// verify the content remains the same
assertEq(_liquidityNet, 1 << 127);
if (liquidity < 1 << 127) {
// verify the soft wrap to int128 is passed truthfully without sign extension
assertEq(LiquidityMath.addDelta(liquidity, int128(liquidityNet)), liquidity + (1 << 127));
} else {
vm.expectRevert(SafeCast.SafeCastOverflow.selector);
LiquidityMath.addDelta(liquidity, int128(liquidityNet));
}
}
}
Loading