fix: add extra check for key.parameters unused bits

pancakeswap · May 30, 2024 · 90136dc · 90136dc
1 parent a334dfa
commit 90136dc
Show file tree

Hide file tree

Showing 11 changed files with 117 additions and 4 deletions.
diff --git a/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap
@@ -1 +1 @@
-137462
+137614
diff --git a/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap b/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap
@@ -1 +1 @@
-32557
+29721
diff --git a/.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap b/.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap
@@ -1 +1 @@
-59894
+60038
diff --git a/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap b/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap
@@ -1 +1 @@
-32316
+32328
diff --git a/src/libraries/math/ParametersHelper.sol b/src/libraries/math/ParametersHelper.sol
@@ -7,6 +7,8 @@ import {Encoded} from "./Encoded.sol";
 library ParametersHelper {
     using Encoded for bytes32;
 
+    error UnusedBitsNonZero();
+
     uint256 internal constant OFFSET_HOOK = 0;
 
     /**
@@ -19,4 +21,10 @@ library ParametersHelper {
     function getHooksRegistrationBitmap(bytes32 params) internal pure returns (uint16 bitmap) {
         bitmap = params.decodeUint16(OFFSET_HOOK);
     }
+
+    function checkUnusedBitsAllZero(bytes32 params, uint256 mostSignificantUsedBitOffset) internal pure {
+        if ((uint256(params) >> (mostSignificantUsedBitOffset + 1)) != 0) {
+            revert UnusedBitsNonZero();
+        }
+    }
 }
diff --git a/src/pool-bin/BinPoolManager.sol b/src/pool-bin/BinPoolManager.sol
@@ -6,6 +6,7 @@ import {ProtocolFees} from "../ProtocolFees.sol";
 import {Hooks} from "../libraries/Hooks.sol";
 import {BinPool} from "./libraries/BinPool.sol";
 import {BinPoolParametersHelper} from "./libraries/BinPoolParametersHelper.sol";
+import {ParametersHelper} from "../libraries/math/ParametersHelper.sol";
 import {Currency, CurrencyLibrary} from "../types/Currency.sol";
 import {IPoolManager} from "../interfaces/IPoolManager.sol";
 import {IBinPoolManager} from "./interfaces/IBinPoolManager.sol";
@@ -100,6 +101,9 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
         if (binStep > MAX_BIN_STEP) revert BinStepTooLarge();
         if (key.currency0 >= key.currency1) revert CurrenciesInitializedOutOfOrder();
 
+        ParametersHelper.checkUnusedBitsAllZero(
+            key.parameters, BinPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS
+        );
         Hooks.validateHookConfig(key);
         BinHooks.validatePermissionsConflict(key);
 

diff --git a/src/pool-bin/libraries/BinPoolParametersHelper.sol b/src/pool-bin/libraries/BinPoolParametersHelper.sol
@@ -13,6 +13,7 @@ library BinPoolParametersHelper {
     using Encoded for bytes32;
 
     uint256 internal constant OFFSET_BIN_STEP = 16;
+    uint256 internal constant OFFSET_MOST_SIGNIFICANT_USED_BITS = 31;
 
     /// @dev Get binstep from the encoded pair parameters
     /// @param params The encoded pair parameters, as follows:

diff --git a/src/pool-cl/CLPoolManager.sol b/src/pool-cl/CLPoolManager.sol
@@ -14,6 +14,7 @@ import {IPoolManager} from "../interfaces/IPoolManager.sol";
 import {Hooks} from "../libraries/Hooks.sol";
 import {Tick} from "./libraries/Tick.sol";
 import {CLPoolParametersHelper} from "./libraries/CLPoolParametersHelper.sol";
+import {ParametersHelper} from "../libraries/math/ParametersHelper.sol";
 import {LPFeeLibrary} from "../libraries/LPFeeLibrary.sol";
 import {PoolId, PoolIdLibrary} from "../types/PoolId.sol";
 import {BalanceDelta, BalanceDeltaLibrary} from "../types/BalanceDelta.sol";
@@ -97,6 +98,9 @@ contract CLPoolManager is ICLPoolManager, ProtocolFees, Extsload {
         if (tickSpacing < MIN_TICK_SPACING) revert TickSpacingTooSmall();
         if (key.currency0 >= key.currency1) revert CurrenciesInitializedOutOfOrder();
 
+        ParametersHelper.checkUnusedBitsAllZero(
+            key.parameters, CLPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS
+        );
         Hooks.validateHookConfig(key);
         CLHooks.validatePermissionsConflict(key);
 

diff --git a/src/pool-cl/libraries/CLPoolParametersHelper.sol b/src/pool-cl/libraries/CLPoolParametersHelper.sol
@@ -16,6 +16,7 @@ library CLPoolParametersHelper {
     using Encoded for bytes32;
 
     uint256 internal constant OFFSET_TICK_SPACING = 16;
+    uint256 internal constant OFFSET_MOST_SIGNIFICANT_USED_BITS = 39;
 
     /**
      * @dev Get tickSpacing from the encoded pair parameters

diff --git a/test/pool-bin/BinPoolManager.t.sol b/test/pool-bin/BinPoolManager.t.sol
@@ -34,6 +34,7 @@ import {BinLiquidityHelper} from "./helpers/BinLiquidityHelper.sol";
 import {BinDonateHelper} from "./helpers/BinDonateHelper.sol";
 import {BinTestHelper} from "./helpers/BinTestHelper.sol";
 import {Hooks} from "../../src/libraries/Hooks.sol";
+import {ParametersHelper} from "../../src/libraries/math/ParametersHelper.sol";
 import {BinPosition} from "../../src/pool-bin/libraries/BinPosition.sol";
 
 contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper {
@@ -156,6 +157,30 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper {
         poolManager.initialize(key, activeId, new bytes(0));
     }
 
+    function test_FuzzInitializePoolUnusedBits(uint256 randomOneBitOffset) external {
+        randomOneBitOffset = bound(randomOneBitOffset, BinPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS, 255);
+
+        uint16 bitMap = 0x0008; // after mint call
+        MockBinHooks mockHooks = new MockBinHooks();
+        mockHooks.setHooksRegistrationBitmap(bitMap);
+
+        key = PoolKey({
+            currency0: currency0,
+            currency1: currency1,
+            // hooks: hook,
+            hooks: IHooks(address(mockHooks)),
+            poolManager: IPoolManager(address(poolManager)),
+            fee: uint24(3000), // 3000 = 0.3%
+            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));
+        }
+        poolManager.initialize(key, activeId, new bytes(0));
+    }
+
     function testInitializeHookValidation() public {
         uint16 bitMap = 0x0008; // after mint call
         MockBinHooks mockHooks = new MockBinHooks();

diff --git a/test/pool-cl/CLPoolManager.t.sol b/test/pool-cl/CLPoolManager.t.sol
@@ -246,6 +246,68 @@ contract CLPoolManagerTest is Test, NoIsolate, Deployers, TokenFixture, GasSnaps
         }
     }
 
+    function testInitialize_unusedBits() external {
+        // 1 at 39 bit
+        {
+            PoolKey memory key = PoolKey({
+                currency0: Currency.wrap(makeAddr("token0")),
+                currency1: Currency.wrap(makeAddr("token1")),
+                hooks: IHooks(address(0)),
+                poolManager: poolManager,
+                fee: uint24(3000),
+                parameters: bytes32(uint256(0x18000010000))
+            });
+
+            vm.expectRevert(abi.encodeWithSelector(ICLPoolManager.TickSpacingTooSmall.selector));
+            poolManager.initialize(key, TickMath.MIN_SQRT_RATIO, new bytes(0));
+        }
+
+        // 1 at 40 bit
+        {
+            PoolKey memory key = PoolKey({
+                currency0: Currency.wrap(makeAddr("token0")),
+                currency1: Currency.wrap(makeAddr("token1")),
+                hooks: IHooks(address(0)),
+                poolManager: poolManager,
+                fee: uint24(3000),
+                parameters: bytes32(uint256(0x10000010000))
+            });
+
+            vm.expectRevert(abi.encodeWithSelector(ParametersHelper.UnusedBitsNonZero.selector));
+            poolManager.initialize(key, TickMath.MIN_SQRT_RATIO, new bytes(0));
+        }
+
+        // 1 at 41 bit
+        {
+            PoolKey memory key = PoolKey({
+                currency0: Currency.wrap(makeAddr("token0")),
+                currency1: Currency.wrap(makeAddr("token1")),
+                hooks: IHooks(address(0)),
+                poolManager: poolManager,
+                fee: uint24(3000),
+                parameters: bytes32(uint256(0x20000010000))
+            });
+
+            vm.expectRevert(abi.encodeWithSelector(ParametersHelper.UnusedBitsNonZero.selector));
+            poolManager.initialize(key, TickMath.MIN_SQRT_RATIO, new bytes(0));
+        }
+
+        // 1 at 42 bit
+        {
+            PoolKey memory key = PoolKey({
+                currency0: Currency.wrap(makeAddr("token0")),
+                currency1: Currency.wrap(makeAddr("token1")),
+                hooks: IHooks(address(0)),
+                poolManager: poolManager,
+                fee: uint24(3000),
+                parameters: bytes32(uint256(0x40000010000))
+            });
+
+            vm.expectRevert(abi.encodeWithSelector(ParametersHelper.UnusedBitsNonZero.selector));
+            poolManager.initialize(key, TickMath.MIN_SQRT_RATIO, new bytes(0));
+        }
+    }
+
     function testInitialize_HookValidation() external {
         MockHooks hookAddr = new MockHooks();
 
@@ -352,6 +414,9 @@ contract CLPoolManagerTest is Test, NoIsolate, Deployers, TokenFixture, GasSnaps
         } else if (key.currency0 > key.currency1 || key.currency0 == key.currency1) {
             vm.expectRevert(abi.encodeWithSelector(IPoolManager.CurrenciesInitializedOutOfOrder.selector));
             poolManager.initialize(key, sqrtPriceX96, ZERO_BYTES);
+        } else if (!checkUnusedBitsAllZero(key.parameters)) {
+            vm.expectRevert(abi.encodeWithSelector(ParametersHelper.UnusedBitsNonZero.selector));
+            poolManager.initialize(key, sqrtPriceX96, ZERO_BYTES);
         } else if (!_validateHookConfig(key)) {
             vm.expectRevert(abi.encodeWithSelector(Hooks.HookConfigValidationError.selector));
             poolManager.initialize(key, sqrtPriceX96, ZERO_BYTES);
@@ -2901,6 +2966,11 @@ contract CLPoolManagerTest is Test, NoIsolate, Deployers, TokenFixture, GasSnaps
         router.donate(key, 100, 200, ZERO_BYTES);
     }
 
+    function checkUnusedBitsAllZero(bytes32 params) internal pure returns (bool) {
+        return
+            (uint256(params) & (type(uint256).max << CLPoolParametersHelper.OFFSET_MOST_SIGNIFICANT_USED_BITS + 1)) == 0;
+    }
+
     function _validateHookConfig(PoolKey memory poolKey) internal view returns (bool) {
         uint16 bitmapInParameters = poolKey.parameters.getHooksRegistrationBitmap();
         if (address(poolKey.hooks) == address(0)) {