Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add extra check for key.parameters unused bits #56

Merged
merged 3 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1 +1 @@
137462
137531
Original file line number Diff line number Diff line change
@@ -1 +1 @@
32533
32155
Original file line number Diff line number Diff line change
@@ -1 +1 @@
59894
59963
Original file line number Diff line number Diff line change
@@ -1 +1 @@
32328
31820
8 changes: 8 additions & 0 deletions src/libraries/math/ParametersHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {Encoded} from "./Encoded.sol";
library ParametersHelper {
using Encoded for bytes32;

error UnusedBitsNonZero();

uint256 internal constant OFFSET_HOOK = 0;

/**
Expand All @@ -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 mostSignificantUnUsedBitOffset) internal pure {
if ((uint256(params) >> (mostSignificantUnUsedBitOffset)) != 0) {
revert UnusedBitsNonZero();
}
}
}
4 changes: 4 additions & 0 deletions src/pool-bin/BinPoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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_UNUSED_BITS
);
Hooks.validateHookConfig(key);
BinHooks.validatePermissionsConflict(key);

Expand Down
4 changes: 3 additions & 1 deletion src/pool-bin/libraries/BinPoolParametersHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +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_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);
Expand Down
4 changes: 4 additions & 0 deletions src/pool-cl/CLPoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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_UNUSED_BITS
);
Hooks.validateHookConfig(key);
CLHooks.validatePermissionsConflict(key);

Expand Down
4 changes: 3 additions & 1 deletion src/pool-cl/libraries/CLPoolParametersHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +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_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) {
Expand Down
33 changes: 30 additions & 3 deletions test/pool-bin/BinPoolManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -156,6 +157,27 @@ 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_UNUSED_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)
});

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();
Expand Down Expand Up @@ -953,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);
Expand Down
81 changes: 78 additions & 3 deletions test/pool-cl/CLPoolManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -2764,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);
Expand Down Expand Up @@ -2901,6 +2971,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_UNUSED_BITS)) == 0;
}

function _validateHookConfig(PoolKey memory poolKey) internal view returns (bool) {
uint16 bitmapInParameters = poolKey.parameters.getHooksRegistrationBitmap();
if (address(poolKey.hooks) == address(0)) {
Expand Down
Loading