Optimize TickMath bounds check in assembly (#669)

* Optimize bounds check in `TickMath.getTickAtSqrtPrice` A new constant, MAX_SQRT_PRICE_MINUS_MIN_SQRT_PRICE_MINUS_ONE, is introduced for efficient bounds checking. The getTickAtSqrtPrice function is further optimized by using assembly-level operations for checking boundary conditions, which enhances the overall performance. It keeps the price within the specified bounds and throws an "InvalidSqrtPrice" error if the price is out of range. * Do `absTick` bounds check in assembly in `getSqrtPriceAtTick` Updated the MAX_TICK constant value from `-MIN_TICK` to `887272` in TickMath.sol. Enhanced safety check within `getSqrtPriceAtTick` function, replacing previous condition with an assembly block to add memory-safe verification. The new check reverts with `InvalidTick()` if the absolute value of `tick` exceeds `MAX_TICK`. * Add fuzz tests for boundary conditions in `TickMath` This commit adds fuzz tests in the `TickMath` test suite to verify that an exception is thrown when exceeding the allowed tick and square root price ranges. The test cases are designed to provide random values that exceed the range of values that the functions `getSqrtPriceAtTick` and `getTickAtSqrtPrice` can operate on without errors. * Update gas snapshots
Uniswap · May 21, 2024 · c32b28b · c32b28b
1 parent eb6a671
commit c32b28b
Show file tree

Hide file tree

Showing 35 changed files with 78 additions and 36 deletions.
diff --git a/.forge-snapshots/TickMathGetTickAtSqrtPrice.snap b/.forge-snapshots/TickMathGetTickAtSqrtPrice.snap
@@ -1 +1 @@
-224621
+218521
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
@@ -1 +1 @@
-153427
+153346
diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap
@@ -1 +1 @@
-331093
+331012
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
@@ -1 +1 @@
-286086
+286005
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
@@ -1 +1 @@
-143637
+143556
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
@@ -1 +1 @@
-301605
+301524
diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap
@@ -1 +1 @@
-61775
+61714
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
@@ -1 +1 @@
-21575
+21463
diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap
@@ -1 +1 @@
-186559
+186473
diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap
@@ -1 +1 @@
-122927
+122841
diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap
@@ -1 +1 @@
-119714
+119628
diff --git a/.forge-snapshots/simple addLiquidity second addition same range.snap b/.forge-snapshots/simple addLiquidity second addition same range.snap
@@ -1 +1 @@
-104746
+104665
diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap
@@ -1 +1 @@
-167238
+167157
diff --git a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap
@@ -1 +1 @@
-98343
+98257
diff --git a/.forge-snapshots/simple removeLiquidity.snap b/.forge-snapshots/simple removeLiquidity.snap
@@ -1 +1 @@
-90383
+90297
diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap
@@ -1 +1 @@
-116430
+116207
diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap
@@ -1 +1 @@
-131591
+131368
diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap
@@ -1 +1 @@
-182729
+182506
diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap
@@ -1 +1 @@
-112303
+112161
diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap
@@ -1 +1 @@
-123646
+123504
diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap
@@ -1 +1 @@
-135638
+135496
diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap
@@ -1 +1 @@
-124901
+124759
diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap
@@ -1 +1 @@
-146895
+146753
diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap
@@ -1 +1 @@
-163445
+163222
diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap
@@ -1 +1 @@
-221557
+221192
diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap
@@ -1 +1 @@
-147646
+147423
diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap
@@ -1 +1 @@
-123658
+123516
diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap
@@ -1 +1 @@
-179826
+179603
diff --git a/.forge-snapshots/swap with return dynamic fee.snap b/.forge-snapshots/swap with return dynamic fee.snap
@@ -1 +1 @@
-155505
+155282
diff --git a/.forge-snapshots/tickSpacingToMaxLiquidityPerTick_gasCost60TickSpacing.snap b/.forge-snapshots/tickSpacingToMaxLiquidityPerTick_gasCost60TickSpacing.snap
@@ -1 +1 @@
-192
+197
diff --git a/.forge-snapshots/tickSpacingToMaxLiquidityPerTick_gasCostMaxTickSpacing.snap b/.forge-snapshots/tickSpacingToMaxLiquidityPerTick_gasCostMaxTickSpacing.snap
@@ -1 +1 @@
-192
+197
diff --git a/.forge-snapshots/tickSpacingToMaxLiquidityPerTick_gasCostMinTickSpacing.snap b/.forge-snapshots/tickSpacingToMaxLiquidityPerTick_gasCostMinTickSpacing.snap
@@ -1 +1 @@
-192
+197
diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap
@@ -1 +1 @@
-158108
+157885
diff --git a/src/libraries/TickMath.sol b/src/libraries/TickMath.sol
@@ -13,7 +13,7 @@ library TickMath {
     /// @dev The minimum tick that may be passed to #getSqrtPriceAtTick computed from log base 1.0001 of 2**-128
     int24 internal constant MIN_TICK = -887272;
     /// @dev The maximum tick that may be passed to #getSqrtPriceAtTick computed from log base 1.0001 of 2**128
-    int24 internal constant MAX_TICK = -MIN_TICK;
+    int24 internal constant MAX_TICK = 887272;
 
     /// @dev The minimum tick spacing value drawn from the range of type int16 that is greater than 0, i.e. min from the range [1, 32767]
     int24 internal constant MIN_TICK_SPACING = 1;
@@ -24,6 +24,9 @@ library TickMath {
     uint160 internal constant MIN_SQRT_PRICE = 4295128739;
     /// @dev The maximum value that can be returned from #getSqrtPriceAtTick. Equivalent to getSqrtPriceAtTick(MAX_TICK)
     uint160 internal constant MAX_SQRT_PRICE = 1461446703485210103287273052203988822378723970342;
+    /// @dev A threshold used for optimized bounds check, equals `MAX_SQRT_PRICE - MIN_SQRT_PRICE - 1`
+    uint160 internal constant MAX_SQRT_PRICE_MINUS_MIN_SQRT_PRICE_MINUS_ONE =
+        1461446703485210103287273052203988822378723970342 - 4295128739 - 1;
 
     /// @notice Given a tickSpacing, compute the maximum usable tick
     function maxUsableTick(int24 tickSpacing) internal pure returns (int24) {
@@ -55,7 +58,15 @@ library TickMath {
                 // Either case, |tick| = mask ^ (tick + mask)
                 absTick := xor(mask, add(mask, tick))
             }
-            if (absTick > uint256(int256(MAX_TICK))) revert InvalidTick();
+            // Equivalent: if (absTick > MAX_TICK) revert InvalidTick();
+            /// @solidity memory-safe-assembly
+            assembly {
+                if gt(absTick, MAX_TICK) {
+                    // store 4-byte selector of "InvalidTick()" at memory [0x1c, 0x20)
+                    mstore(0, 0xce8ef7fc)
+                    revert(0x1c, 0x04)
+                }
+            }
 
             uint256 price =
                 absTick & 0x1 != 0 ? 0xfffcb933bd6fad37aa2d162d1a594001 : 0x100000000000000000000000000000000;
@@ -100,8 +111,19 @@ library TickMath {
     /// @return tick The greatest tick for which the price is less than or equal to the input price
     function getTickAtSqrtPrice(uint160 sqrtPriceX96) internal pure returns (int24 tick) {
         unchecked {
+            // Equivalent: if (sqrtPriceX96 < MIN_SQRT_PRICE || sqrtPriceX96 >= MAX_SQRT_PRICE) revert InvalidSqrtPrice();
             // second inequality must be < because the price can never reach the price at the max tick
-            if (sqrtPriceX96 < MIN_SQRT_PRICE || sqrtPriceX96 >= MAX_SQRT_PRICE) revert InvalidSqrtPrice();
+            /// @solidity memory-safe-assembly
+            assembly {
+                // if sqrtPriceX96 < MIN_SQRT_PRICE, the `sub` underflows and `gt` is true
+                // if sqrtPriceX96 >= MAX_SQRT_PRICE, sqrtPriceX96 - MIN_SQRT_PRICE > MAX_SQRT_PRICE - MIN_SQRT_PRICE - 1
+                if gt(sub(sqrtPriceX96, MIN_SQRT_PRICE), MAX_SQRT_PRICE_MINUS_MIN_SQRT_PRICE_MINUS_ONE) {
+                    // store 4-byte selector of "InvalidSqrtPrice()" at memory [0x1c, 0x20)
+                    mstore(0, 0x31efafe8)
+                    revert(0x1c, 0x04)
+                }
+            }
+
             uint256 price = uint256(sqrtPriceX96) << 32;
 
             uint256 r = price;

diff --git a/test/libraries/TickMath.t.sol b/test/libraries/TickMath.t.sol
@@ -60,6 +60,16 @@ contract TickMathTestTest is Test, JavascriptFfi, GasSnapshot {
         tickMath.getSqrtPriceAtTick(MAX_TICK + 1);
     }
 
+    function test_fuzz_getSqrtPriceAtTick_throwsForTooLarge(int24 tick) public {
+        if (tick > 0) {
+            tick = int24(bound(tick, MAX_TICK + 1, type(int24).max));
+        } else {
+            tick = int24(bound(tick, type(int24).min, MIN_TICK - 1));
+        }
+        vm.expectRevert(TickMath.InvalidTick.selector);
+        tickMath.getSqrtPriceAtTick(tick);
+    }
+
     function test_getSqrtPriceAtTick_isValidMinTick() public view {
         assertEq(tickMath.getSqrtPriceAtTick(MIN_TICK), tickMath.MIN_SQRT_PRICE());
         assertEq(tickMath.getSqrtPriceAtTick(MIN_TICK), 4295128739);
@@ -102,6 +112,16 @@ contract TickMathTestTest is Test, JavascriptFfi, GasSnapshot {
         tickMath.getTickAtSqrtPrice(MAX_SQRT_PRICE);
     }
 
+    function test_fuzz_getTickAtSqrtPrice_throwsForInvalid(uint160 sqrtPriceX96, bool gte) public {
+        if (gte) {
+            sqrtPriceX96 = uint160(bound(sqrtPriceX96, MAX_SQRT_PRICE, type(uint160).max));
+        } else {
+            sqrtPriceX96 = uint160(bound(sqrtPriceX96, 0, MIN_SQRT_PRICE - 1));
+        }
+        vm.expectRevert(TickMath.InvalidSqrtPrice.selector);
+        tickMath.getTickAtSqrtPrice(sqrtPriceX96);
+    }
+
     function test_getTickAtSqrtPrice_isValidMinSqrtPrice() public view {
         assertEq(tickMath.getTickAtSqrtPrice(MIN_SQRT_PRICE), MIN_TICK);
     }