pull in changes and refactor hook calls

Uniswap · Dec 12, 2023 · 6c497bc · 6c497bc
2 parents 399f250 + cda1f48
commit 6c497bc
Show file tree

Hide file tree

Showing 57 changed files with 972 additions and 1,660 deletions.
diff --git a/.forge-snapshots/HooksShouldCallBeforeSwap.snap b/.forge-snapshots/HooksShouldCallBeforeSwap.snap
@@ -1 +1 @@
-34
+116
diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap
@@ -1 +1 @@
-320193
+323392
diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap
@@ -1 +1 @@
-202979
+200192
diff --git a/.forge-snapshots/addLiquidity.snap b/.forge-snapshots/addLiquidity.snap
@@ -1 +1 @@
-202925
+200160
diff --git a/.forge-snapshots/before swap hook, already cached dynamic fee.snap b/.forge-snapshots/before swap hook, already cached dynamic fee.snap
@@ -1 +1 @@
-193128
+194497
diff --git a/.forge-snapshots/cached dynamic fee, no hooks.snap b/.forge-snapshots/cached dynamic fee, no hooks.snap
@@ -1 +1 @@
-148469
+146864
diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap
@@ -1 +1 @@
-139105
+137968
diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap
@@ -1 +1 @@
-186572
+185390
diff --git a/.forge-snapshots/erc20 collect protocol fees.snap b/.forge-snapshots/erc20 collect protocol fees.snap
@@ -1 +1 @@
-26991
+26968
diff --git a/.forge-snapshots/gas overhead of no-op lock.snap b/.forge-snapshots/gas overhead of no-op lock.snap
@@ -1 +1 @@
-15224
+15246
diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap
@@ -1 +1 @@
-78605
+74288
diff --git a/.forge-snapshots/mint with empty hook.snap b/.forge-snapshots/mint with empty hook.snap
@@ -1 +1 @@
-319063
+323262
diff --git a/.forge-snapshots/mint with native token.snap b/.forge-snapshots/mint with native token.snap
@@ -1 +1 @@
-201844
+200100
diff --git a/.forge-snapshots/mint.snap b/.forge-snapshots/mint.snap
@@ -1 +1 @@
-201764
+200042
diff --git a/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap b/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap
@@ -1 +1 @@
-251938
+255250
diff --git a/.forge-snapshots/modify position with noop.snap b/.forge-snapshots/modify position with noop.snap
@@ -1 +1 @@
-56865
+58217
diff --git a/.forge-snapshots/native collect protocol fees.snap b/.forge-snapshots/native collect protocol fees.snap
@@ -1 +1 @@
-38657
+38634
diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap
@@ -1 +1 @@
-26933
+23773
diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap
@@ -1 +1 @@
-105900
+107037
diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap
@@ -1 +1 @@
-211283
+208397
diff --git a/.forge-snapshots/removeLiquidity.snap b/.forge-snapshots/removeLiquidity.snap
@@ -1 +1 @@
-207550
+204687
diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap
@@ -1 +1 @@
-197354
+195835
diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap
@@ -1 +1 @@
-205922
+204403
diff --git a/.forge-snapshots/simpleSwapEOAInitiated.snap b/.forge-snapshots/simpleSwapEOAInitiated.snap
@@ -1 +1 @@
-177210
+175672
diff --git a/.forge-snapshots/simpleSwapNativeEOAInitiated.snap b/.forge-snapshots/simpleSwapNativeEOAInitiated.snap
@@ -1 +1 @@
-173872
+172328
diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap
@@ -1 +1 @@
-127947
+126463
diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap
@@ -1 +1 @@
-115438
+113954
diff --git a/.forge-snapshots/swap burn claim for input.snap b/.forge-snapshots/swap burn claim for input.snap
@@ -1 +1 @@
-134487
+133127
diff --git a/.forge-snapshots/swap mint output as claim.snap b/.forge-snapshots/swap mint output as claim.snap
@@ -1 +1 @@
-218137
+216596
diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap
@@ -1 +1 @@
-192370
+193598
diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap
@@ -1 +1 @@
-115416
+113932
diff --git a/.forge-snapshots/swap with noop.snap b/.forge-snapshots/swap with noop.snap
@@ -1 +1 @@
-49690
+51524
diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap
@@ -1 +1 @@
-198941
+200341
diff --git a/src/Fees.sol b/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);
-    }
 }