From 79e15e2a18d74a61c323ee3e08b59274c61907a8 Mon Sep 17 00:00:00 2001 From: ChefMist <133624774+ChefMist@users.noreply.github.com> Date: Fri, 31 May 2024 10:47:35 +0800 Subject: [PATCH] feat: gas optimization and fix dynamic fee fuzz (#60) --- ...ookTest#testInitializeSucceedsWithHook.snap | 2 +- ...ManagerTest#testFuzzUpdateDynamicLPFee.snap | 2 +- ...PoolManagerTest#initializeWithoutHooks.snap | 2 +- ...ManagerTest#testFuzzUpdateDynamicLPFee.snap | 2 +- src/libraries/math/ParametersHelper.sol | 4 ++-- src/pool-bin/BinPoolManager.sol | 2 +- .../libraries/BinPoolParametersHelper.sol | 5 +++-- src/pool-cl/CLPoolManager.sol | 2 +- .../libraries/CLPoolParametersHelper.sol | 5 +++-- test/pool-bin/BinPoolManager.t.sol | 18 ++++++++++-------- test/pool-cl/CLPoolManager.t.sol | 13 +++++++++---- 11 files changed, 33 insertions(+), 24 deletions(-) diff --git a/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap b/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap index cc0faa74..bcf21868 100644 --- a/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap +++ b/.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap @@ -1 +1 @@ -137614 \ No newline at end of file +137531 \ No newline at end of file diff --git a/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap b/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap index e5584fb8..ae3136b0 100644 --- a/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap +++ b/.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicLPFee.snap @@ -1 +1 @@ -29721 \ No newline at end of file +32155 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap b/.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap index ef0d3876..e5314134 100644 --- a/.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap +++ b/.forge-snapshots/CLPoolManagerTest#initializeWithoutHooks.snap @@ -1 +1 @@ -60038 \ No newline at end of file +59963 \ No newline at end of file diff --git a/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap b/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap index df3f7901..87265ba2 100644 --- a/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap +++ b/.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicLPFee.snap @@ -1 +1 @@ -32328 \ No newline at end of file +31820 \ No newline at end of file diff --git a/src/libraries/math/ParametersHelper.sol b/src/libraries/math/ParametersHelper.sol index a8f2fe56..011408e5 100644 --- 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 index 16fd83d5..b43ec75b 100644 --- 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 index ed9e80a3..7af453cb 100644 --- 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 index a5b06c7a..a0f67d4b 100644 --- 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 index fb7fa45a..cb35bfc8 100644 --- 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 index e8ae4246..3c5ad900 100644 --- 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 index 92ed4119..e4c17f76 100644 --- 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) {