[Part 1 of 2] feat: minor tweaks (#30)

* chore: minor gas opt safeCast * test: update test * gas: updated forge gas
pancakeswap · May 8, 2024 · a9301b8 · a9301b8
1 parent ee06ebc
commit a9301b8
Show file tree

Hide file tree

Showing 15 changed files with 105 additions and 28 deletions.
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap
@@ -1 +1 @@
-401866
+401422
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap
@@ -1 +1 @@
-182854
+182410
diff --git a/.forge-snapshots/CLPoolManagerTest#addLiquidity_nativeToken.snap b/.forge-snapshots/CLPoolManagerTest#addLiquidity_nativeToken.snap
@@ -1 +1 @@
-247993
+247549
diff --git a/.forge-snapshots/CLPoolManagerTest#removeLiquidity_toNonEmpty.snap b/.forge-snapshots/CLPoolManagerTest#removeLiquidity_toNonEmpty.snap
@@ -1 +1 @@
-129601
+128995
diff --git a/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap b/.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap
@@ -1 +1 @@
-24940941
+24940739
diff --git a/.forge-snapshots/CLPositionTest#Position_update_remove.snap b/.forge-snapshots/CLPositionTest#Position_update_remove.snap
@@ -1 +1 @@
-1899
+1697
diff --git a/.forge-snapshots/LiquidityMathTest#addDeltaNegtive.snap b/.forge-snapshots/LiquidityMathTest#addDeltaNegtive.snap
@@ -1 +1 @@
-306
+116
diff --git a/.forge-snapshots/LiquidityMathTest#addDeltaPositive.snap b/.forge-snapshots/LiquidityMathTest#addDeltaPositive.snap
@@ -1 +1 @@
-212
+113
diff --git a/.forge-snapshots/TickTest#update.snap b/.forge-snapshots/TickTest#update.snap
@@ -1 +1 @@
-134926
+134819
diff --git a/src/pool-cl/libraries/LiquidityMath.sol b/src/pool-cl/libraries/LiquidityMath.sol
@@ -9,11 +9,14 @@ library LiquidityMath {
     /// @param y The delta by which liquidity should be changed
     /// @return z The liquidity delta
     function addDelta(uint128 x, int128 y) internal pure returns (uint128 z) {
-        // after 0.8.0 overflow is checked by default
-        if (y < 0) {
-            z = x - uint128(-y);
-        } else {
-            z = x + uint128(y);
+        assembly {
+            z := add(x, y)
+
+            if shr(128, z) {
+                // store 0x93dafdf1, error SafeCastOverflow at memory 0 address and revert from pointer 28, to byte 32
+                mstore(0x0, 0x93dafdf1)
+                revert(0x1c, 0x04)
+            }
         }
     }
 }
diff --git a/test/libraries/BalanceDelta.t.sol b/test/libraries/BalanceDelta.t.sol
@@ -5,7 +5,7 @@ import {Test} from "forge-std/Test.sol";
 import {BalanceDelta, toBalanceDelta} from "../../src/types/BalanceDelta.sol";
 
 contract TestBalanceDelta is Test {
-    function testToBalanceDelta() public {
+    function test_toBalanceDelta() public {
         BalanceDelta balanceDelta = toBalanceDelta(0, 0);
         assertEq(balanceDelta.amount0(), 0);
         assertEq(balanceDelta.amount1(), 0);
@@ -27,17 +27,52 @@ contract TestBalanceDelta is Test {
         assertEq(balanceDelta.amount1(), type(int128).min);
     }
 
-    function testToBalanceDelta(int128 x, int128 y) public {
+    function test_fuzz_toBalanceDelta(int128 x, int128 y) public {
+        BalanceDelta balanceDelta = toBalanceDelta(x, y);
+        int256 expectedBD = int256(uint256(bytes32(abi.encodePacked(x, y))));
+        assertEq(BalanceDelta.unwrap(balanceDelta), expectedBD);
+    }
+
+    function test_fuzz_amount0_amount1(int128 x, int128 y) public {
         BalanceDelta balanceDelta = toBalanceDelta(x, y);
         assertEq(balanceDelta.amount0(), x);
         assertEq(balanceDelta.amount1(), y);
     }
 
-    function testAdd(int128 a, int128 b, int128 c, int128 d) public {
+    function test_add() public {
+        BalanceDelta balanceDelta = toBalanceDelta(0, 0) + toBalanceDelta(0, 0);
+        assertEq(balanceDelta.amount0(), 0);
+        assertEq(balanceDelta.amount1(), 0);
+
+        balanceDelta = toBalanceDelta(-1000, 1000) + toBalanceDelta(1000, -1000);
+        assertEq(balanceDelta.amount0(), 0);
+        assertEq(balanceDelta.amount1(), 0);
+
+        balanceDelta =
+            toBalanceDelta(type(int128).min, type(int128).max) + toBalanceDelta(type(int128).max, type(int128).min);
+        assertEq(balanceDelta.amount0(), -1);
+        assertEq(balanceDelta.amount1(), -1);
+
+        balanceDelta = toBalanceDelta(type(int128).max / 2 + 1, type(int128).max / 2 + 1)
+            + toBalanceDelta(type(int128).max / 2, type(int128).max / 2);
+        assertEq(balanceDelta.amount0(), type(int128).max);
+        assertEq(balanceDelta.amount1(), type(int128).max);
+    }
+
+    function test_add_revertsOnOverflow() public {
+        // should revert because type(int128).max + 1 is not possible
+        vm.expectRevert();
+        toBalanceDelta(type(int128).max, 0) + toBalanceDelta(1, 0);
+
+        vm.expectRevert();
+        toBalanceDelta(0, type(int128).max) + toBalanceDelta(0, 1);
+    }
+
+    function test_fuzz_add(int128 a, int128 b, int128 c, int128 d) public {
         int256 ac = int256(a) + c;
         int256 bd = int256(b) + d;
 
-        // make sure the addition doesn't overflow
+        // if the addition overflows it should revert
         if (ac != int128(ac) || bd != int128(bd)) {
             vm.expectRevert();
         }
@@ -47,16 +82,52 @@ contract TestBalanceDelta is Test {
         assertEq(balanceDelta.amount1(), bd);
     }
 
-    function testSub(int128 a, int128 b, int128 c, int128 d) public {
+    function test_sub() public {
+        BalanceDelta balanceDelta = toBalanceDelta(0, 0) - toBalanceDelta(0, 0);
+        assertEq(balanceDelta.amount0(), 0);
+        assertEq(balanceDelta.amount1(), 0);
+
+        balanceDelta = toBalanceDelta(-1000, 1000) - toBalanceDelta(1000, -1000);
+        assertEq(balanceDelta.amount0(), -2000);
+        assertEq(balanceDelta.amount1(), 2000);
+
+        balanceDelta =
+            toBalanceDelta(-1000, -1000) - toBalanceDelta(-(type(int128).min + 1000), -(type(int128).min + 1000));
+        assertEq(balanceDelta.amount0(), type(int128).min);
+        assertEq(balanceDelta.amount1(), type(int128).min);
+
+        balanceDelta = toBalanceDelta(type(int128).min / 2, type(int128).min / 2)
+            - toBalanceDelta(-(type(int128).min / 2), -(type(int128).min / 2));
+        assertEq(balanceDelta.amount0(), type(int128).min);
+        assertEq(balanceDelta.amount1(), type(int128).min);
+    }
+
+    function test_sub_revertsOnUnderflow() public {
+        // should revert because type(int128).min - 1 is not possible
+        vm.expectRevert();
+        toBalanceDelta(type(int128).min, 0) - toBalanceDelta(1, 0);
+
+        vm.expectRevert();
+        toBalanceDelta(0, type(int128).min) - toBalanceDelta(0, 1);
+    }
+
+    function test_fuzz_sub(int128 a, int128 b, int128 c, int128 d) public {
         int256 ac = int256(a) - c;
         int256 bd = int256(b) - d;
 
-        // make sure the subtraction doesn't underflow
-        vm.assume(ac == int128(ac));
-        vm.assume(bd == int128(bd));
+        // if the subtraction underflows it should revert
+        if (ac != int128(ac) || bd != int128(bd)) {
+            vm.expectRevert();
+        }
 
         BalanceDelta balanceDelta = toBalanceDelta(a, b) - toBalanceDelta(c, d);
         assertEq(balanceDelta.amount0(), ac);
         assertEq(balanceDelta.amount1(), bd);
     }
+
+    function test_fuzz_eq(int128 a, int128 b, int128 c, int128 d) public {
+        bool isEqual = (toBalanceDelta(a, b) == toBalanceDelta(c, d));
+        if (a == c && b == d) assertTrue(isEqual);
+        else assertFalse(isEqual);
+    }
 }
diff --git a/test/pool-cl/CLPoolManager.t.sol b/test/pool-cl/CLPoolManager.t.sol
@@ -34,6 +34,7 @@ import {ProtocolFeeControllerTest} from "./helpers/ProtocolFeeControllerTest.sol
 import {IProtocolFeeController} from "../../src/interfaces/IProtocolFeeController.sol";
 import {CLFeeManagerHook} from "./helpers/CLFeeManagerHook.sol";
 import {CLNoOpTestHook} from "./helpers/CLNoOpTestHook.sol";
+import {SafeCast} from "../../src/libraries/SafeCast.sol";
 
 contract CLPoolManagerTest is Test, Deployers, TokenFixture, GasSnapshot {
     using PoolIdLibrary for PoolKey;
@@ -984,7 +985,7 @@ contract CLPoolManagerTest is Test, Deployers, TokenFixture, GasSnapshot {
         IERC20(Currency.unwrap(currency0)).approve(address(router), 1e30 ether);
         IERC20(Currency.unwrap(currency1)).approve(address(router), 1e30 ether);
 
-        vm.expectRevert(stdError.arithmeticError);
+        vm.expectRevert(SafeCast.SafeCastOverflow.selector);
         router.modifyPosition(
             key, ICLPoolManager.ModifyLiquidityParams({tickLower: 46000, tickUpper: 46050, liquidityDelta: -1}), ""
         );

diff --git a/test/pool-cl/libraries/CLPool.t.sol b/test/pool-cl/libraries/CLPool.t.sol
@@ -55,7 +55,7 @@ contract PoolTest is Test {
         } else if (params.tickUpper > TickMath.MAX_TICK) {
             vm.expectRevert(abi.encodeWithSelector(Tick.TickUpperOutOfBounds.selector, params.tickUpper));
         } else if (params.liquidityDelta < 0) {
-            vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x11));
+            vm.expectRevert(SafeCast.SafeCastOverflow.selector);
         } else if (params.liquidityDelta == 0) {
             vm.expectRevert(CLPosition.CannotUpdateEmptyPosition.selector);
         } else if (params.liquidityDelta > int128(Tick.tickSpacingToMaxLiquidityPerTick(params.tickSpacing))) {

diff --git a/test/pool-cl/libraries/CLPosition.t.sol b/test/pool-cl/libraries/CLPosition.t.sol
@@ -7,6 +7,7 @@ import {Test} from "forge-std/Test.sol";
 import {CLPosition} from "../../../src/pool-cl/libraries/CLPosition.sol";
 import {CLPool} from "../../../src/pool-cl/libraries/CLPool.sol";
 import {FixedPoint128} from "../../../src/pool-cl/libraries/FixedPoint128.sol";
+import {SafeCast} from "../../../src/libraries/SafeCast.sol";
 
 contract CLPositionTest is Test, GasSnapshot {
     using CLPosition for mapping(bytes32 => CLPosition.Info);
@@ -31,7 +32,7 @@ contract CLPositionTest is Test, GasSnapshot {
         if (liquidityDelta == 0) {
             vm.expectRevert(CLPosition.CannotUpdateEmptyPosition.selector);
         } else if (liquidityDelta < 0) {
-            vm.expectRevert(stdError.arithmeticError);
+            vm.expectRevert(SafeCast.SafeCastOverflow.selector);
         }
         (uint256 feesOwed0, uint256 feesOwed1) = info.update(liquidityDelta, feeGrowthInside0X128, feeGrowthInside1X128);
 

diff --git a/test/pool-cl/libraries/LiquidityMath.t.sol b/test/pool-cl/libraries/LiquidityMath.t.sol
@@ -4,6 +4,7 @@ pragma solidity ^0.8.24;
 import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol";
 import {stdError} from "forge-std/StdError.sol";
 import {Test} from "forge-std/Test.sol";
+import {SafeCast} from "../../../src/libraries/SafeCast.sol";
 import {LiquidityMath} from "../../../src/pool-cl/libraries/LiquidityMath.sol";
 
 contract LiquidityMathTest is Test, GasSnapshot {
@@ -22,16 +23,16 @@ contract LiquidityMathTest is Test, GasSnapshot {
     }
 
     function testAddDeltaOverflow() public {
-        vm.expectRevert(stdError.arithmeticError);
+        vm.expectRevert(SafeCast.SafeCastOverflow.selector);
         LiquidityMath.addDelta(2 ** 128 - 15, 15);
     }
 
     function testAddDeltaUnderflow() public {
         // underflow
-        vm.expectRevert(stdError.arithmeticError);
+        vm.expectRevert(SafeCast.SafeCastOverflow.selector);
         LiquidityMath.addDelta(0, -1);
 
-        vm.expectRevert(stdError.arithmeticError);
+        vm.expectRevert(SafeCast.SafeCastOverflow.selector);
         LiquidityMath.addDelta(3, -4);
     }
 }