Revert SqrtPriceMath errors in assembly (#694)

* Optimize condition check and revert `PriceOverflow` error in assembly * Revert `SqrtPriceMath` errors in assembly * Revert `InvalidPrice` with inline assembly in `getAmount0Delta` * Add comments about Solidity equivalence
Uniswap · May 23, 2024 · 03636de · 03636de
1 parent 3351c80
commit 03636de
Show file tree

Hide file tree

Showing 39 changed files with 79 additions and 44 deletions.
diff --git a/.forge-snapshots/SwapMath_oneForZero_exactInCapped.snap b/.forge-snapshots/SwapMath_oneForZero_exactInCapped.snap
@@ -1 +1 @@
-1961
+1947
diff --git a/.forge-snapshots/SwapMath_oneForZero_exactInPartial.snap b/.forge-snapshots/SwapMath_oneForZero_exactInPartial.snap
@@ -1 +1 @@
-2468
+2390
diff --git a/.forge-snapshots/SwapMath_oneForZero_exactOutCapped.snap b/.forge-snapshots/SwapMath_oneForZero_exactOutCapped.snap
@@ -1 +1 @@
-1700
+1686
diff --git a/.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap b/.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap
@@ -1 +1 @@
-2956
+2831
diff --git a/.forge-snapshots/SwapMath_zeroForOne_exactInCapped.snap b/.forge-snapshots/SwapMath_zeroForOne_exactInCapped.snap
@@ -1 +1 @@
-2073
+2059
diff --git a/.forge-snapshots/SwapMath_zeroForOne_exactInPartial.snap b/.forge-snapshots/SwapMath_zeroForOne_exactInPartial.snap
@@ -1 +1 @@
-3021
+2941
diff --git a/.forge-snapshots/SwapMath_zeroForOne_exactOutCapped.snap b/.forge-snapshots/SwapMath_zeroForOne_exactOutCapped.snap
@@ -1 +1 @@
-1812
+1798
diff --git a/.forge-snapshots/SwapMath_zeroForOne_exactOutPartial.snap b/.forge-snapshots/SwapMath_zeroForOne_exactOutPartial.snap
@@ -1 +1 @@
-2554
+2458
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 @@
-150602
+150588
diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap
@@ -1 +1 @@
-327348
+327334
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
@@ -1 +1 @@
-281462
+281448
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
@@ -1 +1 @@
-140808
+140794
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 @@
-298780
+298766
diff --git a/.forge-snapshots/getAmount0Delta_gasCostForAmount0WhereRoundUpIsFalse.snap b/.forge-snapshots/getAmount0Delta_gasCostForAmount0WhereRoundUpIsFalse.snap
@@ -1 +1 @@
-516
+502
diff --git a/.forge-snapshots/getAmount0Delta_gasCostForAmount0WhereRoundUpIsTrue.snap b/.forge-snapshots/getAmount0Delta_gasCostForAmount0WhereRoundUpIsTrue.snap
@@ -1 +1 @@
-652
+638
diff --git a/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsFalseGas.snap b/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsFalseGas.snap
@@ -1 +1 @@
-572
+520
diff --git a/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsTrueGas.snap b/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsTrueGas.snap
@@ -1 +1 @@
-763
+711
diff --git a/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap b/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap
@@ -1 +1 @@
-843
+746
diff --git a/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsTrueGas.snap b/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsTrueGas.snap
@@ -1 +1 @@
-468
+398
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
@@ -1 +1 @@
-20725
+20580
diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap
@@ -1 +1 @@
-182941
+182927
diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap
@@ -1 +1 @@
-137230
+137216
diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap
@@ -1 +1 @@
-117016
+117002
diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap
@@ -1 +1 @@
-115044
+114950
diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap
@@ -1 +1 @@
-130209
+130115
diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap
@@ -1 +1 @@
-179850
+179756
diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap
@@ -1 +1 @@
-110546
+110466
diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap
@@ -1 +1 @@
-121893
+121813
diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap
@@ -1 +1 @@
-133767
+133642
diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap
@@ -1 +1 @@
-123014
+122930
diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap
@@ -1 +1 @@
-144784
+144718
diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap
@@ -1 +1 @@
-161442
+161348
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 @@
-218072
+217898
diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap
@@ -1 +1 @@
-145643
+145549
diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap
@@ -1 +1 @@
-138950
+138870
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 @@
-176884
+176786
diff --git a/.forge-snapshots/swap with return dynamic fee.snap b/.forge-snapshots/swap with return dynamic fee.snap
@@ -1 +1 @@
-152694
+152600
diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap
@@ -1 +1 @@
-155296
+155202
diff --git a/src/libraries/SqrtPriceMath.sol b/src/libraries/SqrtPriceMath.sol
@@ -55,7 +55,14 @@ library SqrtPriceMath {
                 uint256 product = amount * sqrtPX96;
                 // if the product overflows, we know the denominator underflows
                 // in addition, we must check that the denominator does not underflow
-                if (product / amount != sqrtPX96 || numerator1 <= product) revert PriceOverflow();
+                // equivalent: if (product / amount != sqrtPX96 || numerator1 <= product) revert PriceOverflow();
+                /// @solidity memory-safe-assembly
+                assembly {
+                    if iszero(and(eq(div(product, amount), sqrtPX96), gt(numerator1, product))) {
+                        mstore(0, 0xf5c787f1) // selector for PriceOverflow()
+                        revert(0x1c, 0x04)
+                    }
+                }
                 uint256 denominator = numerator1 - product;
                 return FullMath.mulDivRoundingUp(numerator1, sqrtPX96, denominator).toUint160();
             }
@@ -94,7 +101,14 @@ library SqrtPriceMath {
                     : FullMath.mulDivRoundingUp(amount, FixedPoint96.Q96, liquidity)
             );
 
-            if (sqrtPX96 <= quotient) revert NotEnoughLiquidity();
+            // equivalent: if (sqrtPX96 <= quotient) revert NotEnoughLiquidity();
+            /// @solidity memory-safe-assembly
+            assembly {
+                if iszero(gt(sqrtPX96, quotient)) {
+                    mstore(0, 0x4323a555) // selector for NotEnoughLiquidity()
+                    revert(0x1c, 0x04)
+                }
+            }
             // always fits 160 bits
             unchecked {
                 return uint160(sqrtPX96 - quotient);
@@ -114,7 +128,14 @@ library SqrtPriceMath {
         pure
         returns (uint160 sqrtQX96)
     {
-        if (sqrtPX96 == 0 || liquidity == 0) revert InvalidPriceOrLiquidity();
+        // equivalent: if (sqrtPX96 == 0 || liquidity == 0) revert InvalidPriceOrLiquidity();
+        /// @solidity memory-safe-assembly
+        assembly {
+            if or(iszero(sqrtPX96), iszero(liquidity)) {
+                mstore(0, 0x4f2461b8) // selector for InvalidPriceOrLiquidity()
+                revert(0x1c, 0x04)
+            }
+        }
 
         // round to make sure that we don't pass the target price
         return zeroForOne
@@ -134,7 +155,14 @@ library SqrtPriceMath {
         pure
         returns (uint160 sqrtQX96)
     {
-        if (sqrtPX96 == 0 || liquidity == 0) revert InvalidPriceOrLiquidity();
+        // equivalent: if (sqrtPX96 == 0 || liquidity == 0) revert InvalidPriceOrLiquidity();
+        /// @solidity memory-safe-assembly
+        assembly {
+            if or(iszero(sqrtPX96), iszero(liquidity)) {
+                mstore(0, 0x4f2461b8) // selector for InvalidPriceOrLiquidity()
+                revert(0x1c, 0x04)
+            }
+        }
 
         // round to make sure that we pass the target price
         return zeroForOne
@@ -158,11 +186,18 @@ library SqrtPriceMath {
         unchecked {
             if (sqrtPriceAX96 > sqrtPriceBX96) (sqrtPriceAX96, sqrtPriceBX96) = (sqrtPriceBX96, sqrtPriceAX96);
 
+            // equivalent: if (sqrtPriceAX96 == 0) revert InvalidPrice();
+            /// @solidity memory-safe-assembly
+            assembly {
+                if iszero(sqrtPriceAX96) {
+                    mstore(0, 0x00bfc921) // selector for InvalidPrice()
+                    revert(0x1c, 0x04)
+                }
+            }
+
             uint256 numerator1 = uint256(liquidity) << FixedPoint96.RESOLUTION;
             uint256 numerator2 = sqrtPriceBX96 - sqrtPriceAX96;
 
-            if (sqrtPriceAX96 == 0) revert InvalidPrice();
-
             return roundUp
                 ? UnsafeMath.divRoundingUp(FullMath.mulDivRoundingUp(numerator1, numerator2, sqrtPriceBX96), sqrtPriceAX96)
                 : FullMath.mulDiv(numerator1, numerator2, sqrtPriceBX96) / sqrtPriceAX96;