Remove hook fees and protocol fee on withdrawal (#432)

* Remove hook fees and protocol fee on withdrawal

* hook fee tests

* Update AccessLockHook.sol

* assume -> bound

* more comments on fee grnaularity

.forge-snapshots/donate gas with 1 token.snap

@@ -1 +1 @@
-139060
+139280

.forge-snapshots/donate gas with 2 tokens.snap

@@ -1 +1 @@
-186482
+186702

.forge-snapshots/gas overhead of no-op lock.snap

@@ -1 +1 @@
-15224
+15246

.forge-snapshots/initialize.snap

@@ -1 +1 @@
-78563
+75775

.forge-snapshots/mint with empty hook.snap

@@ -1 +1 @@
-319063
+318670

.forge-snapshots/mint with native token.snap

@@ -1 +1 @@
-201844
+201474

.forge-snapshots/mint.snap

@@ -1 +1 @@
-201764
+201394

.forge-snapshots/simple swap.snap

@@ -1 +1 @@
-205899
+205733

.forge-snapshots/swap against liquidity with native token.snap

@@ -1 +1 @@
-127924
+127793

.forge-snapshots/swap against liquidity.snap

@@ -1 +1 @@
-115415
+115284

.forge-snapshots/swap with dynamic fee.snap

@@ -1 +1 @@
-192325
+192160

.forge-snapshots/swap with hooks.snap

@@ -1 +1 @@
-115393
+115262

src/Fees.sol

@@ -3,7 +3,6 @@ pragma solidity ^0.8.19;
 
 import {Currency, CurrencyLibrary} from "./types/Currency.sol";
 import {IProtocolFeeController} from "./interfaces/IProtocolFeeController.sol";
-import {IHookFeeManager} from "./interfaces/IHookFeeManager.sol";
 import {IFees} from "./interfaces/IFees.sol";
 import {FeeLibrary} from "./libraries/FeeLibrary.sol";
 import {Pool} from "./libraries/Pool.sol";
@@ -22,8 +21,6 @@ abstract contract Fees is IFees, Owned {
 
     mapping(Currency currency => uint256) public protocolFeesAccrued;
 
-    mapping(address hookAddress => mapping(Currency currency => uint256)) public hookFeesAccrued;
-
     IProtocolFeeController public protocolFeeController;
 
     uint256 private immutable controllerGasLimit;
@@ -35,15 +32,16 @@ abstract contract Fees is IFees, Owned {
     /// @notice Fetch the protocol fees for a given pool, returning false if the call fails or the returned fees are invalid.
     /// @dev to prevent an invalid protocol fee controller from blocking pools from being initialized
     ///      the success of this function is NOT checked on initialize and if the call fails, the protocol fees are set to 0.
-    /// @dev the success of this function must be checked when called in setProtocolFees
-    function _fetchProtocolFees(PoolKey memory key) internal returns (bool success, uint24 protocolFees) {
+    /// @dev the success of this function must be checked when called in setProtocolFee
+    function _fetchProtocolFee(PoolKey memory key) internal returns (bool success, uint16 protocolFees) {
         if (address(protocolFeeController) != address(0)) {
             // note that EIP-150 mandates that calls requesting more than 63/64ths of remaining gas
             // will be allotted no more than this amount, so controllerGasLimit must be set with this
             // in mind.
             if (gasleft() < controllerGasLimit) revert ProtocolFeeCannotBeFetched();
+
             (bool _success, bytes memory _data) = address(protocolFeeController).call{gas: controllerGasLimit}(
-                abi.encodeWithSelector(IProtocolFeeController.protocolFeesForPool.selector, key)
+                abi.encodeWithSelector(IProtocolFeeController.protocolFeeForPool.selector, key)
             );
             // Ensure that the return data fits within a word
             if (!_success || _data.length > 32) return (false, 0);
@@ -52,44 +50,22 @@ abstract contract Fees is IFees, Owned {
             assembly {
                 returnData := mload(add(_data, 0x20))
             }
-            // Ensure return data does not overflow a uint24 and that the underlying fees are within bounds.
-            (success, protocolFees) = returnData == uint24(returnData) && _isValidProtocolFees(uint24(returnData))
-                ? (true, uint24(returnData))
+            // Ensure return data does not overflow a uint16 and that the underlying fees are within bounds.
+            (success, protocolFees) = returnData == uint16(returnData) && _isFeeWithinBounds(uint16(returnData))
+                ? (true, uint16(returnData))
                 : (false, 0);
         }
     }
 
-    /// @notice There is no cap on the hook fee, but it is specified as a percentage taken on the amount after the protocol fee is applied, if there is a protocol fee.
-    function _fetchHookFees(PoolKey memory key) internal view returns (uint24 hookFees) {
-        if (address(key.hooks) != address(0)) {
-            try IHookFeeManager(address(key.hooks)).getHookFees(key) returns (uint24 hookFeesRaw) {
-                uint24 swapFeeMask = key.fee.hasHookSwapFee() ? 0xFFF000 : 0;
-                uint24 withdrawFeeMask = key.fee.hasHookWithdrawFee() ? 0xFFF : 0;
-                uint24 fullFeeMask = swapFeeMask | withdrawFeeMask;
-                hookFees = hookFeesRaw & fullFeeMask;
-            } catch {}
-        }
-    }
-
     function _fetchDynamicSwapFee(PoolKey memory key) internal view returns (uint24 dynamicSwapFee) {
         dynamicSwapFee = IDynamicFeeManager(address(key.hooks)).getFee(msg.sender, key);
         if (dynamicSwapFee >= MAX_SWAP_FEE) revert FeeTooLarge();
     }
 
-    function _isValidProtocolFees(uint24 protocolFees) internal pure returns (bool) {
-        if (protocolFees != 0) {
-            uint16 protocolSwapFee = uint16(protocolFees >> 12);
-            uint16 protocolWithdrawFee = uint16(protocolFees & 0xFFF);
-            return _isFeeWithinBounds(protocolSwapFee) && _isFeeWithinBounds(protocolWithdrawFee);
-        }
-        return true;
-    }
-
-    /// @dev Only the lower 12 bits are used here to encode the fee denominator.
     function _isFeeWithinBounds(uint16 fee) internal pure returns (bool) {
         if (fee != 0) {
-            uint16 fee0 = fee % 64;
-            uint16 fee1 = fee >> 6;
+            uint16 fee0 = fee % 256;
+            uint16 fee1 = fee >> 8;
             // The fee is specified as a denominator so it cannot be LESS than the MIN_PROTOCOL_FEE_DENOMINATOR (unless it is 0).
             if (
                 (fee0 != 0 && fee0 < MIN_PROTOCOL_FEE_DENOMINATOR) || (fee1 != 0 && fee1 < MIN_PROTOCOL_FEE_DENOMINATOR)
@@ -115,17 +91,4 @@ abstract contract Fees is IFees, Owned {
         protocolFeesAccrued[currency] -= amountCollected;
         currency.transfer(recipient, amountCollected);
     }
-
-    function collectHookFees(address recipient, Currency currency, uint256 amount)
-        external
-        returns (uint256 amountCollected)
-    {
-        address hookAddress = msg.sender;
-
-        amountCollected = (amount == 0) ? hookFeesAccrued[hookAddress][currency] : amount;
-        recipient = (recipient == address(0)) ? hookAddress : recipient;
-
-        hookFeesAccrued[hookAddress][currency] -= amountCollected;
-        currency.transfer(recipient, amountCollected);
-    }
 }

src/PoolManager.sol

@@ -13,7 +13,6 @@ import {NoDelegateCall} from "./NoDelegateCall.sol";
 import {Owned} from "./Owned.sol";
 import {IHooks} from "./interfaces/IHooks.sol";
 import {IDynamicFeeManager} from "./interfaces/IDynamicFeeManager.sol";
-import {IHookFeeManager} from "./interfaces/IHookFeeManager.sol";
 import {IPoolManager} from "./interfaces/IPoolManager.sol";
 import {ILockCallback} from "./interfaces/callback/ILockCallback.sol";
 import {Fees} from "./Fees.sol";
@@ -57,11 +56,11 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims {
         external
         view
         override
-        returns (uint160 sqrtPriceX96, int24 tick, uint24 protocolFees, uint24 hookFees)
+        returns (uint160 sqrtPriceX96, int24 tick, uint16 protocolFee)
     {
         Pool.Slot0 memory slot0 = pools[id].slot0;
 
-        return (slot0.sqrtPriceX96, slot0.tick, slot0.protocolFees, slot0.hookFees);
+        return (slot0.sqrtPriceX96, slot0.tick, slot0.protocolFee);
     }
 
     /// @inheritdoc IPoolManager
@@ -130,10 +129,10 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims {
         }
 
         PoolId id = key.toId();
-        (, uint24 protocolFees) = _fetchProtocolFees(key);
+        (, uint16 protocolFee) = _fetchProtocolFee(key);
         uint24 swapFee = key.fee.isDynamicFee() ? _fetchDynamicSwapFee(key) : key.fee.getStaticFee();
 
-        tick = pools[id].initialize(sqrtPriceX96, protocolFees, _fetchHookFees(key), swapFee);
+        tick = pools[id].initialize(sqrtPriceX96, protocolFee, swapFee);
 
         if (key.hooks.shouldCallAfterInitialize()) {
             if (
@@ -217,8 +216,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims {
             }
         }
 
-        Pool.FeeAmounts memory feeAmounts;
-        (delta, feeAmounts) = pools[id].modifyPosition(
+        delta = pools[id].modifyPosition(
             Pool.ModifyPositionParams({
                 owner: msg.sender,
                 tickLower: params.tickLower,
@@ -230,21 +228,6 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims {
 
         _accountPoolBalanceDelta(key, delta);
 
-        unchecked {
-            if (feeAmounts.feeForProtocol0 > 0) {
-                protocolFeesAccrued[key.currency0] += feeAmounts.feeForProtocol0;
-            }
-            if (feeAmounts.feeForProtocol1 > 0) {
-                protocolFeesAccrued[key.currency1] += feeAmounts.feeForProtocol1;
-            }
-            if (feeAmounts.feeForHook0 > 0) {
-                hookFeesAccrued[address(key.hooks)][key.currency0] += feeAmounts.feeForHook0;
-            }
-            if (feeAmounts.feeForHook1 > 0) {
-                hookFeesAccrued[address(key.hooks)][key.currency1] += feeAmounts.feeForHook1;
-            }
-        }
-
         if (key.hooks.shouldCallAfterModifyPosition()) {
             if (
                 key.hooks.afterModifyPosition(msg.sender, key, params, delta, hookData)
@@ -286,10 +269,9 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims {
         }
 
         uint256 feeForProtocol;
-        uint256 feeForHook;
         uint24 swapFee;
         Pool.SwapState memory state;
-        (delta, feeForProtocol, feeForHook, swapFee, state) = pools[id].swap(
+        (delta, feeForProtocol, swapFee, state) = pools[id].swap(
             Pool.SwapParams({
                 tickSpacing: key.tickSpacing,
                 zeroForOne: params.zeroForOne,
@@ -305,9 +287,6 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims {
             if (feeForProtocol > 0) {
                 protocolFeesAccrued[params.zeroForOne ? key.currency0 : key.currency1] += feeForProtocol;
             }
-            if (feeForHook > 0) {
-                hookFeesAccrued[address(key.hooks)][params.zeroForOne ? key.currency0 : key.currency1] += feeForHook;
-            }
         }
 
         if (key.hooks.shouldCallAfterSwap()) {
@@ -391,19 +370,12 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims {
         _burnFrom(from, currency.toId(), amount);
     }
 
-    function setProtocolFees(PoolKey memory key) external {
-        (bool success, uint24 newProtocolFees) = _fetchProtocolFees(key);
+    function setProtocolFee(PoolKey memory key) external {
+        (bool success, uint16 newProtocolFee) = _fetchProtocolFee(key);
         if (!success) revert ProtocolFeeControllerCallFailedOrInvalidResult();
         PoolId id = key.toId();
-        pools[id].setProtocolFees(newProtocolFees);
-        emit ProtocolFeeUpdated(id, newProtocolFees);
-    }
-
-    function setHookFees(PoolKey memory key) external {
-        uint24 newHookFees = _fetchHookFees(key);
-        PoolId id = key.toId();
-        pools[id].setHookFees(newHookFees);
-        emit HookFeeUpdated(id, newHookFees);
+        pools[id].setProtocolFee(newProtocolFee);
+        emit ProtocolFeeUpdated(id, newProtocolFee);
     }
 
     function updateDynamicSwapFee(PoolKey memory key) external {

src/interfaces/IFees.sol

@@ -20,7 +20,4 @@ interface IFees {
 
     /// @notice Given a currency address, returns the protocol fees accrued in that currency
     function protocolFeesAccrued(Currency) external view returns (uint256);
-
-    /// @notice Given a hook and a currency address, returns the fees accrued
-    function hookFeesAccrued(address, Currency) external view returns (uint256);
 }

src/interfaces/IPoolManager.sol

@@ -81,9 +81,7 @@ interface IPoolManager is IFees {
         uint24 fee
     );
 
-    event ProtocolFeeUpdated(PoolId indexed id, uint24 protocolFees);
-
-    event HookFeeUpdated(PoolId indexed id, uint24 hookFees);
+    event ProtocolFeeUpdated(PoolId indexed id, uint16 protocolFee);
 
     event DynamicSwapFeeUpdated(PoolId indexed id, uint24 dynamicSwapFee);
 
@@ -94,10 +92,7 @@ interface IPoolManager is IFees {
     function MIN_TICK_SPACING() external view returns (int24);
 
     /// @notice Get the current value in slot0 of the given pool
-    function getSlot0(PoolId id)
-        external
-        view
-        returns (uint160 sqrtPriceX96, int24 tick, uint24 protocolFees, uint24 hookFees);
+    function getSlot0(PoolId id) external view returns (uint160 sqrtPriceX96, int24 tick, uint16 protocolFee);
 
     /// @notice Get the current value of liquidity of the given pool
     function getLiquidity(PoolId id) external view returns (uint128 liquidity);
@@ -187,12 +182,9 @@ interface IPoolManager is IFees {
     /// @notice Called by the user to pay what is owed
     function settle(Currency token) external payable returns (uint256 paid);
 
-    /// @notice Sets the protocol's swap and withdrawal fees for the given pool
-    /// Protocol fees are always a portion of a fee that is owed. If that underlying fee is 0, no protocol fees will accrue even if it is set to > 0.
-    function setProtocolFees(PoolKey memory key) external;
-
-    /// @notice Sets the hook's swap and withdrawal fees for the given pool
-    function setHookFees(PoolKey memory key) external;
+    /// @notice Sets the protocol's swap fee for the given pool
+    /// Protocol fees are always a portion of the LP swap fee that is owed. If that fee is 0, no protocol fees will accrue even if it is set to > 0.
+    function setProtocolFee(PoolKey memory key) external;
 
     /// @notice Updates the pools swap fees for the a pool that has enabled dynamic swap fees.
     function updateDynamicSwapFee(PoolKey memory key) external;

src/interfaces/IProtocolFeeController.sol

@@ -7,5 +7,5 @@ interface IProtocolFeeController {
     /// @notice Returns the protocol fees for a pool given the conditions of this contract
     /// @param poolKey The pool key to identify the pool. The controller may want to use attributes on the pool
     ///   to determine the protocol fee, hence the entire key is needed.
-    function protocolFeesForPool(PoolKey memory poolKey) external view returns (uint24);
+    function protocolFeeForPool(PoolKey memory poolKey) external view returns (uint16);
 }

src/libraries/FeeLibrary.sol

@@ -4,21 +4,11 @@ pragma solidity ^0.8.20;
 library FeeLibrary {
     uint24 public constant STATIC_FEE_MASK = 0x0FFFFF;
     uint24 public constant DYNAMIC_FEE_FLAG = 0x800000; // 1000
-    uint24 public constant HOOK_SWAP_FEE_FLAG = 0x400000; // 0100
-    uint24 public constant HOOK_WITHDRAW_FEE_FLAG = 0x200000; // 0010
 
     function isDynamicFee(uint24 self) internal pure returns (bool) {
         return self & DYNAMIC_FEE_FLAG != 0;
     }
 
-    function hasHookSwapFee(uint24 self) internal pure returns (bool) {
-        return self & HOOK_SWAP_FEE_FLAG != 0;
-    }
-
-    function hasHookWithdrawFee(uint24 self) internal pure returns (bool) {
-        return self & HOOK_WITHDRAW_FEE_FLAG != 0;
-    }
-
     function isStaticFeeTooLarge(uint24 self) internal pure returns (bool) {
         return self & STATIC_FEE_MASK >= 1000000;
     }

src/libraries/Hooks.sol

@@ -74,13 +74,11 @@ library Hooks {
         ) {
             return false;
         }
-        // If there is no hook contract set, then fee cannot be dynamic and there cannot be a hook fee on swap or withdrawal.
+        // If there is no hook contract set, then fee cannot be dynamic
+        // If a hook contract is set, it must have at least 1 flag set, or have a dynamic fee
         return address(hook) == address(0)
-            ? !fee.isDynamicFee() && !fee.hasHookSwapFee() && !fee.hasHookWithdrawFee()
-            : (
-                uint160(address(hook)) >= ACCESS_LOCK_FLAG || fee.isDynamicFee() || fee.hasHookSwapFee()
-                    || fee.hasHookWithdrawFee()
-            );
+            ? !fee.isDynamicFee()
+            : (uint160(address(hook)) >= ACCESS_LOCK_FLAG || fee.isDynamicFee());
     }
 
     function shouldCallBeforeInitialize(IHooks self) internal pure returns (bool) {