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

* Remove hook fees and protocol fee on withdrawal

* hook fee tests

* Update AccessLockHook.sol

* assume -> bound

* more comments on fee grnaularity

.forge-snapshots/before swap hook, already cached dynamic fee.snap

@@ -1 +1 @@
-193083
+192918

.forge-snapshots/cached dynamic fee, no hooks.snap

@@ -1 +1 @@
-148446
+148280

.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/erc20 collect protocol fees.snap

@@ -1 +1 @@
-26991
+26968

.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/mintWithEmptyHookEOAInitiated.snap

@@ -1 +1 @@
-250959
+250566

.forge-snapshots/modify position with noop.snap

@@ -1 +1 @@
-56530
+56582

.forge-snapshots/native collect protocol fees.snap

@@ -1 +1 @@
-38657
+38634

.forge-snapshots/poolManager bytecode size.snap

@@ -1 +1 @@
-26329
+24030

.forge-snapshots/simple swap with native.snap

@@ -1 +1 @@
-197331
+197165

.forge-snapshots/simple swap.snap

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

.forge-snapshots/simpleSwapEOAInitiated.snap

@@ -1 +1 @@
-177187
+176999

.forge-snapshots/simpleSwapNativeEOAInitiated.snap

@@ -1 +1 @@
-173849
+173661

.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 burn claim for input.snap

@@ -1 +1 @@
-134509
+134457

.forge-snapshots/swap mint output as claim.snap

@@ -1 +1 @@
-218114
+217926

.forge-snapshots/swap with dynamic fee.snap

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

.forge-snapshots/swap with hooks.snap

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

.forge-snapshots/swap with noop.snap

@@ -1 +1 @@
-49690
+49888

.forge-snapshots/update dynamic fee in before swap.snap

@@ -1 +1 @@
-198919
+198762

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, Claims {
         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, Claims {
         }
 
         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, Claims {
             }
         }
 
-        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, Claims {
 
         _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, Claims {
         }
 
         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, Claims {
             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, Claims {
         _burn(currency, 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);
 }