feat: gas optimization and fix dynamic fee fuzz (#60)

pancakeswap · May 31, 2024 · 79e15e2 · 79e15e2
1 parent 90136dc
commit 79e15e2
Show file tree

Hide file tree

Showing 11 changed files with 33 additions and 24 deletions.
diff --git a/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap
@@ -1 +1 @@
-137614
+137531
diff --git a/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap b/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap
@@ -1 +1 @@
-29721
+32155
diff --git a/.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap b/.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap
@@ -1 +1 @@
-60038
+59963
diff --git a/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap b/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap
@@ -1 +1 @@
-32328
+31820
diff --git a/src/libraries/math/ParametersHelper.sol b/src/libraries/math/ParametersHelper.sol
@@ -22,8 +22,8 @@ library ParametersHelper {
         bitmap = params.decodeUint16(OFFSET_HOOK);
     }
 
-    function checkUnusedBitsAllZero(bytes32 params, uint256 mostSignificantUsedBitOffset) internal pure {
-        if ((uint256(params) >> (mostSignificantUsedBitOffset + 1)) != 0) {
+    function checkUnusedBitsAllZero(bytes32 params, uint256 mostSignificantUnUsedBitOffset) internal pure {
+        if ((uint256(params) >> (mostSignificantUnUsedBitOffset)) != 0) {
             revert UnusedBitsNonZero();
         }
     }

diff --git a/src/pool-bin/BinPoolManager.sol b/src/pool-bin/BinPoolManager.sol
@@ -102,7 +102,7 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         if (key.currency0 >= key.currency1) revert CurrenciesInitializedOutOfOrder();
 
         ParametersHelper.checkUnusedBitsAllZero(
-            key.parameters, BinPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS
+            key.parameters, BinPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_UNUSED_BITS
         );
         Hooks.validateHookConfig(key);
         BinHooks.validatePermissionsConflict(key);

diff --git a/src/pool-bin/libraries/BinPoolParametersHelper.sol b/src/pool-bin/libraries/BinPoolParametersHelper.sol
@@ -9,17 +9,18 @@ import {Encoded} from "../../libraries/math/Encoded.sol";
 /// The parameters are stored in a single bytes32 variable in the following format:
 /// [0 - 16[: reserve for hooks
 /// [16 - 31[: binStep (16 bits)
+/// [32 - 256[: unused
 library BinPoolParametersHelper {
     using Encoded for bytes32;
 
     uint256 internal constant OFFSET_BIN_STEP = 16;
-    uint256 internal constant OFFSET_MOST_SIGNIFICANT_USED_BITS = 31;
+    uint256 internal constant OFFSET_MOST_SIGNIFICANT_UNUSED_BITS = 32;
 
     /// @dev Get binstep from the encoded pair parameters
     /// @param params The encoded pair parameters, as follows:
     /// [0 - 15[: bitmap for hooks registration
     /// [16 - 31[: binSteps (16 bits)
-    /// [32 - 256[: other parameters
+    /// [32 - 256[: unused
     /// @return binStep The binStep
     function getBinStep(bytes32 params) internal pure returns (uint16 binStep) {
         binStep = params.decodeUint16(OFFSET_BIN_STEP);

diff --git a/src/pool-cl/CLPoolManager.sol b/src/pool-cl/CLPoolManager.sol
@@ -99,7 +99,7 @@ contract CLPoolManager is ICLPoolManager, ProtocolFees, Extsload {
         if (key.currency0 >= key.currency1) revert CurrenciesInitializedOutOfOrder();
 
         ParametersHelper.checkUnusedBitsAllZero(
-            key.parameters, CLPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS
+            key.parameters, CLPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_UNUSED_BITS
         );
         Hooks.validateHookConfig(key);
         CLHooks.validatePermissionsConflict(key);

diff --git a/src/pool-cl/libraries/CLPoolParametersHelper.sol b/src/pool-cl/libraries/CLPoolParametersHelper.sol
@@ -11,19 +11,20 @@ import {Encoded} from "../../libraries/math/Encoded.sol";
  *
  * [0 - 15[: reserve for hooks
  * [16 - 39[: tickSpacing (24 bits)
+ * [40 - 256[: unused
  */
 library CLPoolParametersHelper {
     using Encoded for bytes32;
 
     uint256 internal constant OFFSET_TICK_SPACING = 16;
-    uint256 internal constant OFFSET_MOST_SIGNIFICANT_USED_BITS = 39;
+    uint256 internal constant OFFSET_MOST_SIGNIFICANT_UNUSED_BITS = 40;
 
     /**
      * @dev Get tickSpacing from the encoded pair parameters
      * @param params The encoded pair parameters, as follows:
      * [0 - 16[: hooks registration bitmaps
      * [16 - 39[: tickSpacing (24 bits)
-     * [40 - 256[: other parameters
+     * [40 - 256[: unused
      * @return tickSpacing The tickSpacing
      */
     function getTickSpacing(bytes32 params) internal pure returns (int24 tickSpacing) {

diff --git a/test/pool-bin/BinPoolManager.t.sol b/test/pool-bin/BinPoolManager.t.sol
@@ -158,7 +158,7 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper {
     }
 
     function test_FuzzInitializePoolUnusedBits(uint256 randomOneBitOffset) external {
-        randomOneBitOffset = bound(randomOneBitOffset, BinPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS, 255);
+        randomOneBitOffset = bound(randomOneBitOffset, BinPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_UNUSED_BITS, 255);
 
         uint16 bitMap = 0x0008; // after mint call
         MockBinHooks mockHooks = new MockBinHooks();
@@ -174,10 +174,7 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper {
             parameters: bytes32(uint256(bitMap) | (1 << randomOneBitOffset)).setBinStep(1)
         });
 
-        // just a bigger binStep if equals to BinPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS
-        if (randomOneBitOffset != BinPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS) {
-            vm.expectRevert(abi.encodeWithSelector(ParametersHelper.UnusedBitsNonZero.selector));
-        }
+        vm.expectRevert(abi.encodeWithSelector(ParametersHelper.UnusedBitsNonZero.selector));
         poolManager.initialize(key, activeId, new bytes(0));
     }
 
@@ -978,10 +975,15 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper {
         vm.expectEmit();
         emit DynamicLPFeeUpdated(key.toId(), _lpFee);
 
-        snapStart("BinPoolManagerTest#testFuzzUpdateDynamicLPFee");
         vm.prank(address(binFeeManagerHook));
-        poolManager.updateDynamicLPFee(key, _lpFee);
-        snapEnd();
+        if (_lpFee != 0) {
+            // temp fix to only record gas if _lpFee !=0. todo use snapLastCall to make this part of code easier to read
+            snapStart("BinPoolManagerTest#testFuzzUpdateDynamicLPFee");
+            poolManager.updateDynamicLPFee(key, _lpFee);
+            snapEnd();
+        } else {
+            poolManager.updateDynamicLPFee(key, _lpFee);
+        }
 
         (,, uint24 swapFee) = poolManager.getSlot0(key.toId());
         assertEq(swapFee, _lpFee);

diff --git a/test/pool-cl/CLPoolManager.t.sol b/test/pool-cl/CLPoolManager.t.sol
@@ -2829,10 +2829,15 @@ contract CLPoolManagerTest is Test, NoIsolate, Deployers, TokenFixture, GasSnaps
         vm.expectEmit();
         emit DynamicLPFeeUpdated(key.toId(), _swapFee);
 
-        snapStart("CLPoolManagerTest#testFuzzUpdateDynamicLPFee");
         vm.prank(address(clFeeManagerHook));
-        poolManager.updateDynamicLPFee(key, _swapFee);
-        snapEnd();
+        if (_swapFee != 0) {
+            // temp fix to only record gas if _swapFee !=0. todo use snapLastCall to make this part of code easier to read
+            snapStart("CLPoolManagerTest#testFuzzUpdateDynamicLPFee");
+            poolManager.updateDynamicLPFee(key, _swapFee);
+            snapEnd();
+        } else {
+            poolManager.updateDynamicLPFee(key, _swapFee);
+        }
 
         (,,, uint24 swapFee) = poolManager.getSlot0(key.toId());
         assertEq(swapFee, _swapFee);
@@ -2968,7 +2973,7 @@ contract CLPoolManagerTest is Test, NoIsolate, Deployers, TokenFixture, GasSnaps
 
     function checkUnusedBitsAllZero(bytes32 params) internal pure returns (bool) {
         return
-            (uint256(params) & (type(uint256).max << CLPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS + 1)) == 0;
+            (uint256(params) & (type(uint256).max << CLPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_UNUSED_BITS)) == 0;
     }
 
     function _validateHookConfig(PoolKey memory poolKey) internal view returns (bool) {