From 7a1394425bb2aa4822dbeed83ceb72d07ed9ad17 Mon Sep 17 00:00:00 2001
From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Date: Sat, 6 Apr 2024 02:27:58 -0400
Subject: [PATCH 1/5] Optimize liquidity flipping calculation in `updateTick`

Updated the way liquidity flipping is calculated in 'Pool.sol' to use bitwise XOR operations for efficiency. This modification improved overall bytecode size in multiple snapshots which demonstrates an optimization of gas usage in our smart contracts.
---
 ...add liquidity to already existing position with salt.snap | 2 +-
 .forge-snapshots/addLiquidity CA fee.snap                    | 2 +-
 .forge-snapshots/addLiquidity with empty hook.snap           | 2 +-
 .forge-snapshots/addLiquidity with native token.snap         | 2 +-
 .forge-snapshots/addLiquidity.snap                           | 1 +
 .../create new liquidity to a position with salt.snap        | 2 +-
 .forge-snapshots/poolManager bytecode size.snap              | 2 +-
 .forge-snapshots/removeLiquidity CA fee.snap                 | 2 +-
 .forge-snapshots/removeLiquidity with empty hook.snap        | 2 +-
 .forge-snapshots/removeLiquidity with native token.snap      | 2 +-
 .../simple addLiquidity second addition same range.snap      | 2 +-
 .forge-snapshots/simple addLiquidity.snap                    | 2 +-
 .../simple removeLiquidity some liquidity remains.snap       | 2 +-
 .forge-snapshots/simple removeLiquidity.snap                 | 2 +-
 src/libraries/Pool.sol                                       | 5 ++++-
 15 files changed, 18 insertions(+), 14 deletions(-)
 create mode 100644 .forge-snapshots/addLiquidity.snap

diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap
index ff76e7f86..551175e9d 100644
--- a/.forge-snapshots/add liquidity to already existing position with salt.snap	
+++ b/.forge-snapshots/add liquidity to already existing position with salt.snap	
@@ -1 +1 @@
-153643
\ No newline at end of file
+153595
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap
index b4678d19b..861c9b1ca 100644
--- a/.forge-snapshots/addLiquidity CA fee.snap	
+++ b/.forge-snapshots/addLiquidity CA fee.snap	
@@ -1 +1 @@
-331309
\ No newline at end of file
+331261
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
index c59df0298..f3a9c00be 100644
--- a/.forge-snapshots/addLiquidity with empty hook.snap	
+++ b/.forge-snapshots/addLiquidity with empty hook.snap	
@@ -1 +1 @@
-286302
\ No newline at end of file
+286254
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
index c5bed833e..4062e0e2a 100644
--- a/.forge-snapshots/addLiquidity with native token.snap	
+++ b/.forge-snapshots/addLiquidity with native token.snap	
@@ -1 +1 @@
-143853
\ No newline at end of file
+143805
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity.snap b/.forge-snapshots/addLiquidity.snap
new file mode 100644
index 000000000..74bcc8545
--- /dev/null
+++ b/.forge-snapshots/addLiquidity.snap
@@ -0,0 +1 @@
+139363
\ No newline at end of file
diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap
index cbb3de708..aee7f3547 100644
--- a/.forge-snapshots/create new liquidity to a position with salt.snap	
+++ b/.forge-snapshots/create new liquidity to a position with salt.snap	
@@ -1 +1 @@
-301821
\ No newline at end of file
+301773
\ No newline at end of file
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
index 81ca44cf1..68abdf813 100644
--- a/.forge-snapshots/poolManager bytecode size.snap	
+++ b/.forge-snapshots/poolManager bytecode size.snap	
@@ -1 +1 @@
-21709
\ No newline at end of file
+21701
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap
index 082d59d10..8208b8fdd 100644
--- a/.forge-snapshots/removeLiquidity CA fee.snap	
+++ b/.forge-snapshots/removeLiquidity CA fee.snap	
@@ -1 +1 @@
-186775
\ No newline at end of file
+186727
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap
index e7fddfb22..6294149cb 100644
--- a/.forge-snapshots/removeLiquidity with empty hook.snap	
+++ b/.forge-snapshots/removeLiquidity with empty hook.snap	
@@ -1 +1 @@
-123143
\ No newline at end of file
+123095
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap
index 8b983838d..7cf734669 100644
--- a/.forge-snapshots/removeLiquidity with native token.snap	
+++ b/.forge-snapshots/removeLiquidity with native token.snap	
@@ -1 +1 @@
-119930
\ No newline at end of file
+119882
\ No newline at end of file
diff --git a/.forge-snapshots/simple addLiquidity second addition same range.snap b/.forge-snapshots/simple addLiquidity second addition same range.snap
index ccf9f3450..53081c566 100644
--- a/.forge-snapshots/simple addLiquidity second addition same range.snap	
+++ b/.forge-snapshots/simple addLiquidity second addition same range.snap	
@@ -1 +1 @@
-104932
\ No newline at end of file
+104884
\ No newline at end of file
diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap
index 8f3accdaa..764ed329d 100644
--- a/.forge-snapshots/simple addLiquidity.snap	
+++ b/.forge-snapshots/simple addLiquidity.snap	
@@ -1 +1 @@
-167424
\ No newline at end of file
+167376
\ No newline at end of file
diff --git a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap
index fc72404ca..7c7293630 100644
--- a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap	
+++ b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap	
@@ -1 +1 @@
-98529
\ No newline at end of file
+98481
\ No newline at end of file
diff --git a/.forge-snapshots/simple removeLiquidity.snap b/.forge-snapshots/simple removeLiquidity.snap
index 0eb1ac8de..869d22042 100644
--- a/.forge-snapshots/simple removeLiquidity.snap	
+++ b/.forge-snapshots/simple removeLiquidity.snap	
@@ -1 +1 @@
-90569
\ No newline at end of file
+90521
\ No newline at end of file
diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol
index 0b5e3d944..47e3a989a 100644
--- a/src/libraries/Pool.sol
+++ b/src/libraries/Pool.sol
@@ -532,7 +532,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

From 38b39f63978ec92d420f4a04d4b9b6991111af1e Mon Sep 17 00:00:00 2001
From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Date: Sat, 6 Apr 2024 03:37:36 -0400
Subject: [PATCH 2/5] Add `LiquidityMath.flipLiquidityDelta` to optimize
 liquidity delta operation

A new function `flipLiquidityDelta` is added to optimize the process of flipping the sign of a liquidity delta if certain conditions are met. Code complexity has been reduced by avoiding the need to use unchecked blocks and applied assembly for efficiency. The refactoring also resulted in changes in the snapshot files.
---
 ...o already existing position with salt.snap |  2 +-
 .forge-snapshots/addLiquidity CA fee.snap     |  2 +-
 .../addLiquidity with empty hook.snap         |  2 +-
 .../addLiquidity with native token.snap       |  2 +-
 .forge-snapshots/addLiquidity.snap            |  2 +-
 ...new liquidity to a position with salt.snap |  2 +-
 .../poolManager bytecode size.snap            |  2 +-
 .forge-snapshots/removeLiquidity CA fee.snap  |  2 +-
 .../removeLiquidity with empty hook.snap      |  2 +-
 .../removeLiquidity with native token.snap    |  2 +-
 ...dLiquidity second addition same range.snap |  2 +-
 .forge-snapshots/simple addLiquidity.snap     |  2 +-
 ...emoveLiquidity some liquidity remains.snap |  2 +-
 .forge-snapshots/simple removeLiquidity.snap  |  2 +-
 src/libraries/LiquidityMath.sol               | 18 +++++++++-
 src/libraries/Pool.sol                        | 14 +++++---
 test/libraries/LiquidityMath.t.sol            | 35 ++++++++++++++-----
 17 files changed, 67 insertions(+), 28 deletions(-)

diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap
index 551175e9d..11f801fab 100644
--- a/.forge-snapshots/add liquidity to already existing position with salt.snap	
+++ b/.forge-snapshots/add liquidity to already existing position with salt.snap	
@@ -1 +1 @@
-153595
\ No newline at end of file
+153514
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap
index 861c9b1ca..ef6a9ec96 100644
--- a/.forge-snapshots/addLiquidity CA fee.snap	
+++ b/.forge-snapshots/addLiquidity CA fee.snap	
@@ -1 +1 @@
-331261
\ No newline at end of file
+331180
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
index f3a9c00be..ec80749a0 100644
--- a/.forge-snapshots/addLiquidity with empty hook.snap	
+++ b/.forge-snapshots/addLiquidity with empty hook.snap	
@@ -1 +1 @@
-286254
\ No newline at end of file
+286173
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
index 4062e0e2a..fdccefd75 100644
--- a/.forge-snapshots/addLiquidity with native token.snap	
+++ b/.forge-snapshots/addLiquidity with native token.snap	
@@ -1 +1 @@
-143805
\ No newline at end of file
+143724
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity.snap b/.forge-snapshots/addLiquidity.snap
index 74bcc8545..54638b698 100644
--- a/.forge-snapshots/addLiquidity.snap
+++ b/.forge-snapshots/addLiquidity.snap
@@ -1 +1 @@
-139363
\ No newline at end of file
+139302
\ No newline at end of file
diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap
index aee7f3547..5ce4c89d4 100644
--- a/.forge-snapshots/create new liquidity to a position with salt.snap	
+++ b/.forge-snapshots/create new liquidity to a position with salt.snap	
@@ -1 +1 @@
-301773
\ No newline at end of file
+301692
\ No newline at end of file
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
index 68abdf813..211d80156 100644
--- a/.forge-snapshots/poolManager bytecode size.snap	
+++ b/.forge-snapshots/poolManager bytecode size.snap	
@@ -1 +1 @@
-21701
\ No newline at end of file
+21629
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap
index 8208b8fdd..bd33171a9 100644
--- a/.forge-snapshots/removeLiquidity CA fee.snap	
+++ b/.forge-snapshots/removeLiquidity CA fee.snap	
@@ -1 +1 @@
-186727
\ No newline at end of file
+186646
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap
index 6294149cb..7328f4391 100644
--- a/.forge-snapshots/removeLiquidity with empty hook.snap	
+++ b/.forge-snapshots/removeLiquidity with empty hook.snap	
@@ -1 +1 @@
-123095
\ No newline at end of file
+123014
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap
index 7cf734669..e0f3fb4fc 100644
--- a/.forge-snapshots/removeLiquidity with native token.snap	
+++ b/.forge-snapshots/removeLiquidity with native token.snap	
@@ -1 +1 @@
-119882
\ No newline at end of file
+119801
\ No newline at end of file
diff --git a/.forge-snapshots/simple addLiquidity second addition same range.snap b/.forge-snapshots/simple addLiquidity second addition same range.snap
index 53081c566..7a49c9a59 100644
--- a/.forge-snapshots/simple addLiquidity second addition same range.snap	
+++ b/.forge-snapshots/simple addLiquidity second addition same range.snap	
@@ -1 +1 @@
-104884
\ No newline at end of file
+104803
\ No newline at end of file
diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap
index 764ed329d..dea472d02 100644
--- a/.forge-snapshots/simple addLiquidity.snap	
+++ b/.forge-snapshots/simple addLiquidity.snap	
@@ -1 +1 @@
-167376
\ No newline at end of file
+167295
\ No newline at end of file
diff --git a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap
index 7c7293630..e3d292f2d 100644
--- a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap	
+++ b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap	
@@ -1 +1 @@
-98481
\ No newline at end of file
+98400
\ No newline at end of file
diff --git a/.forge-snapshots/simple removeLiquidity.snap b/.forge-snapshots/simple removeLiquidity.snap
index 869d22042..781b911f0 100644
--- a/.forge-snapshots/simple removeLiquidity.snap	
+++ b/.forge-snapshots/simple removeLiquidity.snap	
@@ -1 +1 @@
-90521
\ No newline at end of file
+90440
\ No newline at end of file
diff --git a/src/libraries/LiquidityMath.sol b/src/libraries/LiquidityMath.sol
index 02d94b4dd..4fca72a2a 100644
--- a/src/libraries/LiquidityMath.sol
+++ b/src/libraries/LiquidityMath.sol
@@ -4,9 +4,10 @@ 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) {
         assembly {
             z := add(x, y)
@@ -17,4 +18,19 @@ library LiquidityMath {
             }
         }
     }
+
+    /// @notice Flips the sign of a liquidity delta if a condition is true
+    /// @dev Equivalent to `res = flip ? -liquidityDelta : liquidityDelta;`
+    /// @dev `liquidityDelta` should never be `type(int128).min`
+    /// @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 (int128 res) {
+        assembly {
+            // 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)
+        }
+    }
 }
diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol
index 47e3a989a..84f7dd76c 100644
--- a/src/libraries/Pool.sol
+++ b/src/libraries/Pool.sol
@@ -409,9 +409,7 @@ library Pool {
                         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;
-                    }
+                    liquidityNet = LiquidityMath.flipLiquidityDelta(liquidityNet, zeroForOne);
 
                     state.liquidity = LiquidityMath.addDelta(state.liquidity, liquidityNet);
                 }
@@ -546,7 +544,15 @@ 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 should be sanitized in `PoolManager`
+        liquidityDelta = LiquidityMath.flipLiquidityDelta(liquidityDelta, upper);
+        // declare an int256 to prevent implicit conversion when calling toInt128
+        int256 liquidityNet;
+        assembly {
+            liquidityNet := add(liquidityNetBefore, liquidityDelta)
+        }
+        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
diff --git a/test/libraries/LiquidityMath.t.sol b/test/libraries/LiquidityMath.t.sol
index 6466f2b8b..62b5ada0c 100644
--- a/test/libraries/LiquidityMath.t.sol
+++ b/test/libraries/LiquidityMath.t.sol
@@ -3,7 +3,8 @@ 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 {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) {
@@ -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
@@ -34,18 +38,24 @@ 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_addDelta_sub_int128min_fuzz(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
+    /// @notice Test the equivalence of `addDelta` and the reference implementation
     function test_addDelta_fuzz(uint128 x, int128 y) public {
         vm.assume(y != type(int128).min);
         try liquidityMath.addDelta(x, y) returns (uint128 z) {
@@ -56,4 +66,11 @@ contract LiquidityMathTest is Test {
             liquidityMathRef.addDelta(x, y);
         }
     }
+
+    /// @notice Test `flipLiquidityDelta` against the ternary expression
+    function test_flipLiquidityDelta_fuzz(int128 liquidityDelta, bool flip) public {
+        // `liquidityDelta` should never be `type(int128).min`
+        vm.assume(liquidityDelta != type(int128).min);
+        assertEq(LiquidityMath.flipLiquidityDelta(liquidityDelta, flip), flip ? -liquidityDelta : liquidityDelta);
+    }
 }

From aef94522493c6a261474c22285e0a5350b76e761 Mon Sep 17 00:00:00 2001
From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Date: Wed, 17 Apr 2024 00:55:12 -0400
Subject: [PATCH 3/5] Add `memory-safe-assembly` directive to
 `LiquidityMath.addDelta`

---
 src/libraries/LiquidityMath.sol | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libraries/LiquidityMath.sol b/src/libraries/LiquidityMath.sol
index 4fca72a2a..0774c84a3 100644
--- a/src/libraries/LiquidityMath.sol
+++ b/src/libraries/LiquidityMath.sol
@@ -9,6 +9,7 @@ library LiquidityMath {
     /// @param y The delta by which liquidity should be changed
     /// @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) {

From ec842a925f635f02143ff696e0efc43a46343032 Mon Sep 17 00:00:00 2001
From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Date: Tue, 14 May 2024 21:23:50 -0400
Subject: [PATCH 4/5] Change test function mutability

---
 test/libraries/LiquidityMath.t.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/libraries/LiquidityMath.t.sol b/test/libraries/LiquidityMath.t.sol
index 62b5ada0c..caed26ad3 100644
--- a/test/libraries/LiquidityMath.t.sol
+++ b/test/libraries/LiquidityMath.t.sol
@@ -68,7 +68,7 @@ contract LiquidityMathTest is Test {
     }
 
     /// @notice Test `flipLiquidityDelta` against the ternary expression
-    function test_flipLiquidityDelta_fuzz(int128 liquidityDelta, bool flip) public {
+    function test_flipLiquidityDelta_fuzz(int128 liquidityDelta, bool flip) public pure {
         // `liquidityDelta` should never be `type(int128).min`
         vm.assume(liquidityDelta != type(int128).min);
         assertEq(LiquidityMath.flipLiquidityDelta(liquidityDelta, flip), flip ? -liquidityDelta : liquidityDelta);

From 22ea97e7b2bfabff2db4d6b5c6fcd851f169df7e Mon Sep 17 00:00:00 2001
From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com>
Date: Sat, 18 May 2024 21:26:51 -0400
Subject: [PATCH 5/5] Add comments and a fuzz test for `flipLiquidityDelta`
 regarding `type(int128).min`

This commit refactors the `flipLiquidityDelta` function in `LiquidityMath` library that can now handle `type(int128).min` and returns a valid `int256`. It also modifies the use of `flipLiquidityDelta` in `Pool` library along with improved explanations with inline comments. Furthermore, it updates the tests in `LiquidityMath.t.sol` by refining the fuzzing, modifying import statements and introducing a new test case for `flipLiquidityDelta` handling `type(int128).min`.
---
 ...o already existing position with salt.snap |  2 +-
 .forge-snapshots/addLiquidity CA fee.snap     |  2 +-
 .../addLiquidity with empty hook.snap         |  2 +-
 .../addLiquidity with native token.snap       |  2 +-
 ...new liquidity to a position with salt.snap |  2 +-
 .../poolManager bytecode size.snap            |  2 +-
 .forge-snapshots/removeLiquidity CA fee.snap  |  2 +-
 .../removeLiquidity with empty hook.snap      |  2 +-
 .../removeLiquidity with native token.snap    |  2 +-
 ...dLiquidity second addition same range.snap |  2 +-
 .forge-snapshots/simple addLiquidity.snap     |  2 +-
 ...emoveLiquidity some liquidity remains.snap |  2 +-
 .forge-snapshots/simple removeLiquidity.snap  |  2 +-
 src/libraries/LiquidityMath.sol               |  4 +-
 src/libraries/Pool.sol                        | 16 ++++---
 test/libraries/LiquidityMath.t.sol            | 43 +++++++++++++++----
 16 files changed, 59 insertions(+), 30 deletions(-)

diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap
index 11f801fab..01fce2445 100644
--- a/.forge-snapshots/add liquidity to already existing position with salt.snap	
+++ b/.forge-snapshots/add liquidity to already existing position with salt.snap	
@@ -1 +1 @@
-153514
\ No newline at end of file
+153508
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap
index ef6a9ec96..1d0d1c656 100644
--- a/.forge-snapshots/addLiquidity CA fee.snap	
+++ b/.forge-snapshots/addLiquidity CA fee.snap	
@@ -1 +1 @@
-331180
\ No newline at end of file
+331174
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
index ec80749a0..396f316a7 100644
--- a/.forge-snapshots/addLiquidity with empty hook.snap	
+++ b/.forge-snapshots/addLiquidity with empty hook.snap	
@@ -1 +1 @@
-286173
\ No newline at end of file
+286167
\ No newline at end of file
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
index fdccefd75..f3e8a2c19 100644
--- a/.forge-snapshots/addLiquidity with native token.snap	
+++ b/.forge-snapshots/addLiquidity with native token.snap	
@@ -1 +1 @@
-143724
\ No newline at end of file
+143718
\ No newline at end of file
diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap
index 5ce4c89d4..f1610ec12 100644
--- a/.forge-snapshots/create new liquidity to a position with salt.snap	
+++ b/.forge-snapshots/create new liquidity to a position with salt.snap	
@@ -1 +1 @@
-301692
\ No newline at end of file
+301686
\ No newline at end of file
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
index 211d80156..564b84727 100644
--- a/.forge-snapshots/poolManager bytecode size.snap	
+++ b/.forge-snapshots/poolManager bytecode size.snap	
@@ -1 +1 @@
-21629
\ No newline at end of file
+21628
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap
index bd33171a9..a3dbe8de7 100644
--- a/.forge-snapshots/removeLiquidity CA fee.snap	
+++ b/.forge-snapshots/removeLiquidity CA fee.snap	
@@ -1 +1 @@
-186646
\ No newline at end of file
+186640
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap
index 7328f4391..4bc0883a7 100644
--- a/.forge-snapshots/removeLiquidity with empty hook.snap	
+++ b/.forge-snapshots/removeLiquidity with empty hook.snap	
@@ -1 +1 @@
-123014
\ No newline at end of file
+123008
\ No newline at end of file
diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap
index e0f3fb4fc..92f721838 100644
--- a/.forge-snapshots/removeLiquidity with native token.snap	
+++ b/.forge-snapshots/removeLiquidity with native token.snap	
@@ -1 +1 @@
-119801
\ No newline at end of file
+119795
\ No newline at end of file
diff --git a/.forge-snapshots/simple addLiquidity second addition same range.snap b/.forge-snapshots/simple addLiquidity second addition same range.snap
index 7a49c9a59..0bbbb3ea4 100644
--- a/.forge-snapshots/simple addLiquidity second addition same range.snap	
+++ b/.forge-snapshots/simple addLiquidity second addition same range.snap	
@@ -1 +1 @@
-104803
\ No newline at end of file
+104797
\ No newline at end of file
diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap
index dea472d02..a34eba201 100644
--- a/.forge-snapshots/simple addLiquidity.snap	
+++ b/.forge-snapshots/simple addLiquidity.snap	
@@ -1 +1 @@
-167295
\ No newline at end of file
+167289
\ No newline at end of file
diff --git a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap
index e3d292f2d..75e347498 100644
--- a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap	
+++ b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap	
@@ -1 +1 @@
-98400
\ No newline at end of file
+98394
\ No newline at end of file
diff --git a/.forge-snapshots/simple removeLiquidity.snap b/.forge-snapshots/simple removeLiquidity.snap
index 781b911f0..8c3ac0c18 100644
--- a/.forge-snapshots/simple removeLiquidity.snap	
+++ b/.forge-snapshots/simple removeLiquidity.snap	
@@ -1 +1 @@
-90440
\ No newline at end of file
+90434
\ No newline at end of file
diff --git a/src/libraries/LiquidityMath.sol b/src/libraries/LiquidityMath.sol
index 0774c84a3..70eb89f95 100644
--- a/src/libraries/LiquidityMath.sol
+++ b/src/libraries/LiquidityMath.sol
@@ -22,11 +22,11 @@ library LiquidityMath {
 
     /// @notice Flips the sign of a liquidity delta if a condition is true
     /// @dev Equivalent to `res = flip ? -liquidityDelta : liquidityDelta;`
-    /// @dev `liquidityDelta` should never be `type(int128).min`
+    /// @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 (int128 res) {
+    function flipLiquidityDelta(int128 liquidityDelta, bool flip) internal pure returns (int256 res) {
         assembly {
             // if flip = true, res = -liquidityDelta = ~liquidityDelta + 1 = (-1) ^ liquidityDelta + 1
             // if flip = false, res = liquidityDelta = 0 ^ liquidityDelta + 0
diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol
index 84f7dd76c..fb1fe5a5c 100644
--- a/src/libraries/Pool.sol
+++ b/src/libraries/Pool.sol
@@ -407,9 +407,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
-                    liquidityNet = LiquidityMath.flipLiquidityDelta(liquidityNet, zeroForOne);
+                    // 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);
                 }
@@ -545,13 +547,15 @@ library Pool {
 
         // when the lower (upper) tick is crossed left to right (right to left), liquidity must be added (removed)
         // Equivalent to `liquidityNet = upper ? liquidityNetBefore - liquidityDelta : liquidityNetBefore + liquidityDelta;`
-        // `int128 liquidityDelta` is passed from `modifyLiquidity` and should be sanitized in `PoolManager`
-        liquidityDelta = LiquidityMath.flipLiquidityDelta(liquidityDelta, upper);
+        // `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)
+            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`
diff --git a/test/libraries/LiquidityMath.t.sol b/test/libraries/LiquidityMath.t.sol
index caed26ad3..ada121bcd 100644
--- a/test/libraries/LiquidityMath.t.sol
+++ b/test/libraries/LiquidityMath.t.sol
@@ -2,9 +2,9 @@
 pragma solidity ^0.8.0;
 
 import {Test} from "forge-std/Test.sol";
-import {SafeCast} from "src/libraries/SafeCast.sol";
-import {LiquidityMath} from "src/libraries/LiquidityMath.sol";
-import {LiquidityMathTest as LiquidityMathMock} 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) {
@@ -46,7 +46,7 @@ contract LiquidityMathTest is Test {
     }
 
     /// @notice Test the assembly implementation of `addDelta` with `type(int128).min`
-    function test_addDelta_sub_int128min_fuzz(uint128 x) public {
+    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);
@@ -56,7 +56,7 @@ contract LiquidityMathTest is Test {
     }
 
     /// @notice Test the equivalence of `addDelta` and the reference implementation
-    function test_addDelta_fuzz(uint128 x, int128 y) public {
+    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));
@@ -68,9 +68,34 @@ contract LiquidityMathTest is Test {
     }
 
     /// @notice Test `flipLiquidityDelta` against the ternary expression
-    function test_flipLiquidityDelta_fuzz(int128 liquidityDelta, bool flip) public pure {
-        // `liquidityDelta` should never be `type(int128).min`
-        vm.assume(liquidityDelta != type(int128).min);
-        assertEq(LiquidityMath.flipLiquidityDelta(liquidityDelta, flip), flip ? -liquidityDelta : liquidityDelta);
+    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));
+        }
     }
 }