refactor: simplify swap fee update workflow (#15)

* refactor: simplify swap fee update workflow

* chore: prettier

.forge-snapshots/BinHookTest#testInitializeSucceedsWithHook.snap

@@ -1 +1 @@
-108918
+108917

.forge-snapshots/BinPoolManagerTest#testFuzzUpdateDynamicSwapFee.snap

@@ -1 +1 @@
-5977
+5118

.forge-snapshots/BinPoolManagerTest#testNoOpGas_Initialize.snap

@@ -1 +1 @@
-37118
+37117

.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap

@@ -1 +1 @@
-348803
+348804

.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap

@@ -1 +1 @@
-59351
+59352

.forge-snapshots/CLPoolManagerTest#addLiquidity_nativeToken.snap

@@ -1 +1 @@
-242396
+242397

.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap

@@ -1 +1 @@
-82513
+82470

.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap

@@ -1 +1 @@
-52475
+52432

.forge-snapshots/CLPoolManagerTest#removeLiquidity_toNonEmpty.snap

@@ -1 +1 @@
-42329
+42330

.forge-snapshots/CLPoolManagerTest#setLmPool.snap

@@ -1 +1 @@
-26295
+26296

.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap

@@ -1 +1 @@
-54821
+54822

.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap

@@ -1 +1 @@
-102919
+102920

.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap

@@ -1 +1 @@
-25042645
+25042646

.forge-snapshots/CLPoolManagerTest#swap_simple.snap

@@ -1 +1 @@
-36021
+36022

.forge-snapshots/CLPoolManagerTest#swap_useSurplusTokenAsInput.snap

@@ -1 +1 @@
-101418
+101419

.forge-snapshots/CLPoolManagerTest#swap_withHooks.snap

@@ -1 +1 @@
-41671
+41672

.forge-snapshots/CLPoolManagerTest#swap_withNative.snap

@@ -1 +1 @@
-36024
+36025

.forge-snapshots/CLPoolManagerTest#testFuzzUpdateDynamicSwapFee.snap

@@ -1 +1 @@
-5499
+4847

.forge-snapshots/CLPoolManagerTest#testNoOp_gas_Donate.snap

@@ -1 +1 @@
-19217
+19174

.forge-snapshots/CLPoolManagerTest#testNoOp_gas_ModifyPosition.snap

@@ -1 +1 @@
-29360
+29361

.forge-snapshots/CLPoolManagerTest#testNoOp_gas_Swap.snap

@@ -1 +1 @@
-21666
+21667

src/interfaces/IFees.sol

@@ -7,8 +7,6 @@ import {IProtocolFeeController} from "./IProtocolFeeController.sol";
 interface IFees {
     /// @notice Thrown when the protocol fee denominator is less than 4. Also thrown when the static or dynamic fee on a pool exceeds the upper limit.
     error FeeTooLarge();
-    /// @notice Thrown when an attempt to update pool swap fee but the pool does not have dynamic fee.
-    error FeeNotDynamic();
     /// @notice Thrown when not enough gas is provided to look up the protocol fee
     error ProtocolFeeCannotBeFetched();
     /// @notice Thrown when user not authorized to collect protocol fee

src/interfaces/IPoolManager.sol

@@ -11,6 +11,10 @@ interface IPoolManager {
     /// @notice PoolKey must have currencies where address(currency0) < address(currency1)
     error CurrenciesInitializedOutOfOrder();
 
+    /// @notice Thrown when a call to updateDynamicSwapFee is made by an address that is not the hook,
+    /// or on a pool that does not have a dynamic swap fee.
+    error UnauthorizedDynamicSwapFeeUpdate();
+
     /// @notice Emitted when protocol fee is updated
     /// @dev The event is emitted even if the updated protocolFee is the same as previous protocolFee
     event ProtocolFeeUpdated(PoolId indexed id, uint16 protocolFee);
@@ -28,5 +32,5 @@ interface IPoolManager {
     ///   1) when hook#beforeSwap() is called and hook call this function to update the swap fee
     ///   2) For BinPool only, when hook#beforeMint() is called and hook call this function to update the swap fee
     ///   3) other use case where the hook might want to on an ad-hoc basis increase/reduce swap fee
-    function updateDynamicSwapFee(PoolKey memory key) external;
+    function updateDynamicSwapFee(PoolKey memory key, uint24 newDynamicSwapFee) external;
 }

src/libraries/Hooks.sol

@@ -5,13 +5,13 @@ pragma solidity ^0.8.24;
 import {IHooks} from "../interfaces/IHooks.sol";
 import {PoolKey} from "../types/PoolKey.sol";
 import {Encoded} from "./math/Encoded.sol";
-import {FeeLibrary} from "./FeeLibrary.sol";
+import {SwapFeeLibrary} from "./SwapFeeLibrary.sol";
 import {ParametersHelper} from "./math/ParametersHelper.sol";
 
 library Hooks {
     using Encoded for bytes32;
     using ParametersHelper for bytes32;
-    using FeeLibrary for uint24;
+    using SwapFeeLibrary for uint24;
 
     bytes4 constant NO_OP_SELECTOR = bytes4(keccak256(abi.encodePacked("NoOp")));
 
@@ -34,7 +34,7 @@ library Hooks {
         if (address(poolKey.hooks) == address(0)) {
             /// @notice If the hooks address is 0, then the bitmap must be 0,
             /// in the same time, the dynamic fee should be disabled as well
-            if (bitmapInParameters == 0 && !poolKey.fee.isDynamicFee()) {
+            if (bitmapInParameters == 0 && !poolKey.fee.isDynamicSwapFee()) {
                 return;
             }
             revert HookConfigValidationError();

src/libraries/FeeLibrary.sol → src/libraries/SwapFeeLibrary.sol

@@ -5,11 +5,13 @@ pragma solidity ^0.8.24;
 /// @dev Library for parsing swap fee info from PoolKey.fee:
 /// 24 bits (upper 4 bits are used to store flag, if swap fee is static, parse lower 20 bits to get swap fee)
 /// 1. flag to indicate the activation of dynamic swap fee, otherwise static swap fee is used
-///     - if dynamic swap fee is activated, then the swap fee is controlled by IDynamicFeeManager(hook).getFee()
+///     - if dynamic swap fee is activated, then the swap fee can be updated by hook
 ///     - if dynamic swap fee is not activated, then the swap fee is controlled by PoolKey.fee itself
 /// 2. protocol fee is controlled by protocolFeeController, not PoolKey.fee
 ///     - protocol fee is controlled by IProtocolFeeController(hook).protocolFeeForPool()
-library FeeLibrary {
+library SwapFeeLibrary {
+    using SwapFeeLibrary for uint24;
+
     /// @dev swap fee is stored in PoolKey as uint24
     uint24 public constant STATIC_FEE_MASK = 0x0FFFFF;
     uint24 public constant DYNAMIC_FEE_FLAG = 0x800000; // 1000
@@ -19,15 +21,17 @@ library FeeLibrary {
     uint24 public constant TEN_PERCENT_FEE = 100_000;
 
     // swap fee for LP
-    function isDynamicFee(uint24 self) internal pure returns (bool) {
+    function isDynamicSwapFee(uint24 self) internal pure returns (bool) {
         return self & DYNAMIC_FEE_FLAG != 0;
     }
 
-    function isStaticFeeTooLarge(uint24 self, uint24 maxFee) internal pure returns (bool) {
-        return self & STATIC_FEE_MASK > maxFee;
+    function isSwapFeeTooLarge(uint24 self, uint24 maxFee) internal pure returns (bool) {
+        return self > maxFee;
     }
 
-    function getStaticFee(uint24 self) internal pure returns (uint24) {
-        return self & STATIC_FEE_MASK;
+    function getSwapFee(uint24 self) internal pure returns (uint24 swapFee) {
+        // the initial fee for a dynamic fee pool is 0
+        if (self.isDynamicSwapFee()) return 0;
+        swapFee = self & STATIC_FEE_MASK;
     }
 }

src/pool-bin/BinPoolManager.sol

@@ -15,7 +15,7 @@ import {PoolKey} from "../types/PoolKey.sol";
 import {BalanceDelta, BalanceDeltaLibrary} from "../types/BalanceDelta.sol";
 import {IVault} from "../interfaces/IVault.sol";
 import {BinPosition} from "./libraries/BinPosition.sol";
-import {FeeLibrary} from "../libraries/FeeLibrary.sol";
+import {SwapFeeLibrary} from "../libraries/SwapFeeLibrary.sol";
 import {PackedUint128Math} from "./libraries/math/PackedUint128Math.sol";
 import {Extsload} from "../Extsload.sol";
 import "./interfaces/IBinHooks.sol";
@@ -26,7 +26,7 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
     using BinPool for *;
     using BinPosition for mapping(bytes32 => BinPosition.Info);
     using BinPoolParametersHelper for bytes32;
-    using FeeLibrary for uint24;
+    using SwapFeeLibrary for uint24;
     using PackedUint128Math for bytes32;
     using Hooks for bytes32;
 
@@ -101,9 +101,6 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         override
         poolManagerMatch(address(key.poolManager))
     {
-        /// @dev Accept up to FeeLibrary.TEN_PERCENT_FEE for fee
-        if (key.fee.isStaticFeeTooLarge(FeeLibrary.TEN_PERCENT_FEE)) revert FeeTooLarge();
-
         uint16 binStep = key.parameters.getBinStep();
         if (binStep < MIN_BIN_STEP) revert BinStepTooSmall();
         if (binStep > MAX_BIN_STEP) revert BinStepTooLarge();
@@ -113,6 +110,10 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         Hooks.validateHookConfig(key);
         _validateHookNoOp(key);
 
+        /// @notice init value for dynamic swap fee is 0, but hook can still set it in afterInitialize
+        uint24 swapFee = key.fee.getSwapFee();
+        if (swapFee.isSwapFeeTooLarge(SwapFeeLibrary.TEN_PERCENT_FEE)) revert FeeTooLarge();
+
         if (key.parameters.shouldCall(HOOKS_BEFORE_INITIALIZE_OFFSET)) {
             if (hooks.beforeInitialize(msg.sender, key, activeId, hookData) != IBinHooks.beforeInitialize.selector) {
                 revert Hooks.InvalidHookResponse();
@@ -122,7 +123,6 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         PoolId id = key.toId();
 
         (, uint16 protocolFee) = _fetchProtocolFee(key);
-        uint24 swapFee = key.fee.isDynamicFee() ? _fetchDynamicSwapFee(key) : key.fee.getStaticFee();
         pools[id].initialize(activeId, protocolFee, swapFee);
 
         /// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one
@@ -196,14 +196,14 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         _checkPoolInitialized(id);
 
         uint24 totalSwapFee;
-        if (key.fee.isDynamicFee()) {
+        if (key.fee.isDynamicSwapFee()) {
             totalSwapFee = IBinDynamicFeeManager(address(key.hooks)).getFeeForSwapInSwapOut(
                 msg.sender, key, swapForY, 0, amountOut
             );
-            if (totalSwapFee > FeeLibrary.TEN_PERCENT_FEE) revert FeeTooLarge();
+            if (totalSwapFee > SwapFeeLibrary.TEN_PERCENT_FEE) revert FeeTooLarge();
         } else {
             // clear the top 4 bits since they may be flagged
-            totalSwapFee = key.fee.getStaticFee();
+            totalSwapFee = key.fee.getSwapFee();
         }
 
         (amountIn, amountOutLeft, fee) = pools[id].getSwapIn(
@@ -223,12 +223,12 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
         _checkPoolInitialized(id);
 
         uint24 totalSwapFee;
-        if (key.fee.isDynamicFee()) {
+        if (key.fee.isDynamicSwapFee()) {
             totalSwapFee =
                 IBinDynamicFeeManager(address(key.hooks)).getFeeForSwapInSwapOut(msg.sender, key, swapForY, amountIn, 0);
-            if (totalSwapFee > FeeLibrary.TEN_PERCENT_FEE) revert FeeTooLarge();
+            if (totalSwapFee > SwapFeeLibrary.TEN_PERCENT_FEE) revert FeeTooLarge();
         } else {
-            totalSwapFee = key.fee.getStaticFee();
+            totalSwapFee = key.fee.getSwapFee();
         }
 
         (amountInLeft, amountOut, fee) = pools[id].getSwapOut(
@@ -393,20 +393,13 @@ contract BinPoolManager is IBinPoolManager, Fees, Extsload {
     }
 
     /// @inheritdoc IPoolManager
-    function updateDynamicSwapFee(PoolKey memory key) external override {
-        if (key.fee.isDynamicFee()) {
-            uint24 newDynamicSwapFee = _fetchDynamicSwapFee(key);
-            PoolId id = key.toId();
-            pools[id].setSwapFee(newDynamicSwapFee);
-            emit DynamicSwapFeeUpdated(id, newDynamicSwapFee);
-        } else {
-            revert FeeNotDynamic();
-        }
-    }
+    function updateDynamicSwapFee(PoolKey memory key, uint24 newDynamicSwapFee) external override {
+        if (!key.fee.isDynamicSwapFee() || msg.sender != address(key.hooks)) revert UnauthorizedDynamicSwapFeeUpdate();
+        if (newDynamicSwapFee.isSwapFeeTooLarge(SwapFeeLibrary.TEN_PERCENT_FEE)) revert FeeTooLarge();
 
-    function _fetchDynamicSwapFee(PoolKey memory key) internal view returns (uint24 dynamicSwapFee) {
-        dynamicSwapFee = IBinDynamicFeeManager(address(key.hooks)).getFee(msg.sender, key);
-        if (dynamicSwapFee > FeeLibrary.TEN_PERCENT_FEE) revert FeeTooLarge();
+        PoolId id = key.toId();
+        pools[id].setSwapFee(newDynamicSwapFee);
+        emit DynamicSwapFeeUpdated(id, newDynamicSwapFee);
     }
 
     function _checkPoolInitialized(PoolId id) internal view {

src/pool-cl/CLPoolManager.sol

@@ -11,11 +11,10 @@ import {CLPool} from "./libraries/CLPool.sol";
 import {CLPosition} from "./libraries/CLPosition.sol";
 import {PoolKey} from "../types/PoolKey.sol";
 import {IPoolManager} from "../interfaces/IPoolManager.sol";
-import {ICLDynamicFeeManager} from "./interfaces/ICLDynamicFeeManager.sol";
 import {Hooks} from "../libraries/Hooks.sol";
 import {Tick} from "./libraries/Tick.sol";
 import {CLPoolParametersHelper} from "./libraries/CLPoolParametersHelper.sol";
-import {FeeLibrary} from "../libraries/FeeLibrary.sol";
+import {SwapFeeLibrary} from "../libraries/SwapFeeLibrary.sol";
 import {PoolId, PoolIdLibrary} from "../types/PoolId.sol";
 import {BalanceDelta, BalanceDeltaLibrary} from "../types/BalanceDelta.sol";
 import {Extsload} from "../Extsload.sol";
@@ -25,7 +24,7 @@ contract CLPoolManager is ICLPoolManager, Fees, Extsload {
     using SafeCast for int256;
     using PoolIdLibrary for PoolKey;
     using Hooks for bytes32;
-    using FeeLibrary for uint24;
+    using SwapFeeLibrary for uint24;
     using CLPoolParametersHelper for bytes32;
     using CLPool for *;
     using CLPosition for mapping(bytes32 => CLPosition.Info);
@@ -97,8 +96,6 @@ contract CLPoolManager is ICLPoolManager, Fees, Extsload {
         poolManagerMatch(address(key.poolManager))
         returns (int24 tick)
     {
-        if (key.fee.isStaticFeeTooLarge(FeeLibrary.ONE_HUNDRED_PERCENT_FEE)) revert FeeTooLarge();
-
         int24 tickSpacing = key.parameters.getTickSpacing();
         if (tickSpacing > MAX_TICK_SPACING) revert TickSpacingTooLarge();
         if (tickSpacing < MIN_TICK_SPACING) revert TickSpacingTooSmall();
@@ -108,6 +105,10 @@ contract CLPoolManager is ICLPoolManager, Fees, Extsload {
         Hooks.validateHookConfig(key);
         _validateHookNoOp(key);
 
+        /// @notice init value for dynamic swap fee is 0, but hook can still set it in afterInitialize
+        uint24 swapFee = key.fee.getSwapFee();
+        if (swapFee.isSwapFeeTooLarge(SwapFeeLibrary.ONE_HUNDRED_PERCENT_FEE)) revert FeeTooLarge();
+
         if (key.parameters.shouldCall(HOOKS_BEFORE_INITIALIZE_OFFSET)) {
             if (hooks.beforeInitialize(msg.sender, key, sqrtPriceX96, hookData) != ICLHooks.beforeInitialize.selector) {
                 revert Hooks.InvalidHookResponse();
@@ -116,7 +117,6 @@ contract CLPoolManager is ICLPoolManager, Fees, Extsload {
 
         PoolId id = key.toId();
         (, uint16 protocolFee) = _fetchProtocolFee(key);
-        uint24 swapFee = key.fee.isDynamicFee() ? _fetchDynamicSwapFee(key) : key.fee.getStaticFee();
         tick = pools[id].initialize(sqrtPriceX96, protocolFee, swapFee);
 
         /// @notice Make sure the first event is noted, so that later events from afterHook won't get mixed up with this one
@@ -327,20 +327,13 @@ contract CLPoolManager is ICLPoolManager, Fees, Extsload {
     }
 
     /// @inheritdoc IPoolManager
-    function updateDynamicSwapFee(PoolKey memory key) external override {
-        if (key.fee.isDynamicFee()) {
-            uint24 newDynamicSwapFee = _fetchDynamicSwapFee(key);
-            PoolId id = key.toId();
-            pools[id].setSwapFee(newDynamicSwapFee);
-            emit DynamicSwapFeeUpdated(id, newDynamicSwapFee);
-        } else {
-            revert FeeNotDynamic();
-        }
-    }
+    function updateDynamicSwapFee(PoolKey memory key, uint24 newDynamicSwapFee) external override {
+        if (!key.fee.isDynamicSwapFee() || msg.sender != address(key.hooks)) revert UnauthorizedDynamicSwapFeeUpdate();
+        if (newDynamicSwapFee.isSwapFeeTooLarge(SwapFeeLibrary.ONE_HUNDRED_PERCENT_FEE)) revert FeeTooLarge();
 
-    function _fetchDynamicSwapFee(PoolKey memory key) internal view returns (uint24 dynamicSwapFee) {
-        dynamicSwapFee = ICLDynamicFeeManager(address(key.hooks)).getFee(msg.sender, key);
-        if (dynamicSwapFee > FeeLibrary.ONE_HUNDRED_PERCENT_FEE) revert FeeTooLarge();
+        PoolId id = key.toId();
+        pools[id].setSwapFee(newDynamicSwapFee);
+        emit DynamicSwapFeeUpdated(id, newDynamicSwapFee);
     }
 
     function _checkPoolInitialized(PoolId id) internal view {