Optimize overflow check in mulDivRoundingUp (#667)

* Optimize overflow check in `mulDivRoundingUp` This commit optimizes the overflow check within the `mulDivRoundingUp` function of FullMath library. This is achieved by combining two steps: the increment of the `result` and the overflow check, into a single step using the `++` operator. This simplifies the code, improves its readability and brings down the gas usage slightly, as reflected in updated snapshot files. * Update assertions in `test_fuzz_mulDivRoundingUp`
Uniswap · May 23, 2024 · 4ea21e0 · 4ea21e0
1 parent 5402ce1
commit 4ea21e0
Show file tree

Hide file tree

Showing 32 changed files with 39 additions and 36 deletions.
diff --git a/.forge-snapshots/SwapMath_oneForZero_exactInCapped.snap b/.forge-snapshots/SwapMath_oneForZero_exactInCapped.snap
@@ -1 +1 @@
-1974
+1961
diff --git a/.forge-snapshots/SwapMath_oneForZero_exactOutCapped.snap b/.forge-snapshots/SwapMath_oneForZero_exactOutCapped.snap
@@ -1 +1 @@
-1713
+1700
diff --git a/.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap b/.forge-snapshots/SwapMath_oneForZero_exactOutPartial.snap
@@ -1 +1 @@
-2982
+2956
diff --git a/.forge-snapshots/SwapMath_zeroForOne_exactInCapped.snap b/.forge-snapshots/SwapMath_zeroForOne_exactInCapped.snap
@@ -1 +1 @@
-2091
+2073
diff --git a/.forge-snapshots/SwapMath_zeroForOne_exactInPartial.snap b/.forge-snapshots/SwapMath_zeroForOne_exactInPartial.snap
@@ -1 +1 @@
-3044
+3021
diff --git a/.forge-snapshots/SwapMath_zeroForOne_exactOutCapped.snap b/.forge-snapshots/SwapMath_zeroForOne_exactOutCapped.snap
@@ -1 +1 @@
-1830
+1812
diff --git a/.forge-snapshots/SwapMath_zeroForOne_exactOutPartial.snap b/.forge-snapshots/SwapMath_zeroForOne_exactOutPartial.snap
@@ -1 +1 @@
-2572
+2554
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 @@
-153006
+152993
diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap
@@ -1 +1 @@
-330666
+330653
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
@@ -1 +1 @@
-285662
+285649
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
@@ -1 +1 @@
-143216
+143203
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 @@
-301184
+301171
diff --git a/.forge-snapshots/getAmount0Delta_gasCostForAmount0WhereRoundUpIsTrue.snap b/.forge-snapshots/getAmount0Delta_gasCostForAmount0WhereRoundUpIsTrue.snap
@@ -1 +1 @@
-665
+652
diff --git a/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsTrueGas.snap b/.forge-snapshots/getNextSqrtPriceFromInput_zeroForOneEqualsTrueGas.snap
@@ -1 +1 @@
-776
+763
diff --git a/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap b/.forge-snapshots/getNextSqrtPriceFromOutput_zeroForOneEqualsFalseGas.snap
@@ -1 +1 @@
-856
+843
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
@@ -1 +1 @@
-21173
+21161
diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap
@@ -1 +1 @@
-115813
+115780
diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap
@@ -1 +1 @@
-130974
+130941
diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap
@@ -1 +1 @@
-182094
+182061
diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap
@@ -1 +1 @@
-111942
+111903
diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap
@@ -1 +1 @@
-123285
+123246
diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap
@@ -1 +1 @@
-135150
+135124
diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap
@@ -1 +1 @@
-124397
+124371
diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap
@@ -1 +1 @@
-162828
+162795
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 @@
-220585
+220513
diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap
@@ -1 +1 @@
-147029
+146996
diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap
@@ -1 +1 @@
-142010
+141971
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 @@
-179060
+179040
diff --git a/.forge-snapshots/swap with return dynamic fee.snap b/.forge-snapshots/swap with return dynamic fee.snap
@@ -1 +1 @@
-154882
+154849
diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap
@@ -1 +1 @@
-157485
+157452
diff --git a/src/libraries/FullMath.sol b/src/libraries/FullMath.sol
@@ -109,9 +109,8 @@ library FullMath {
     function mulDivRoundingUp(uint256 a, uint256 b, uint256 denominator) internal pure returns (uint256 result) {
         unchecked {
             result = mulDiv(a, b, denominator);
-            if (mulmod(a, b, denominator) > 0) {
-                require(result < type(uint256).max);
-                result++;
+            if (mulmod(a, b, denominator) != 0) {
+                require(++result > 0);
             }
         }
     }

diff --git a/test/libraries/FullMath.t.sol b/test/libraries/FullMath.t.sol
@@ -52,7 +52,7 @@ contract FullMathTest is Test {
     function test_fuzz_mulDiv(uint256 x, uint256 y, uint256 d) public pure {
         vm.assume(d != 0);
         vm.assume(y != 0);
-        vm.assume(x <= type(uint256).max / y);
+        x = bound(x, 0, type(uint256).max / y);
         assertEq(FullMath.mulDiv(x, y, d), x * y / d);
     }
 
@@ -97,10 +97,14 @@ contract FullMathTest is Test {
     function test_fuzz_mulDivRoundingUp(uint256 x, uint256 y, uint256 d) public pure {
         vm.assume(d != 0);
         vm.assume(y != 0);
-        vm.assume(x <= type(uint256).max / y);
+        x = bound(x, 0, type(uint256).max / y);
         uint256 numerator = x * y;
         uint256 result = FullMath.mulDivRoundingUp(x, y, d);
-        assertTrue(result == numerator / d || result == numerator / d + 1);
+        if (mulmod(x, y, d) > 0) {
+            assertEq(result, numerator / d + 1);
+        } else {
+            assertEq(result, numerator / d);
+        }
     }
 
     function test_invariant_mulDivRounding(uint256 x, uint256 y, uint256 d) public pure {