From 8f1353cb7abff7e83900eb11769d326aae46e292 Mon Sep 17 00:00:00 2001 From: hensha256 <henshawalice@gmail.com> Date: Tue, 5 Dec 2023 16:02:16 -0500 Subject: [PATCH 1/5] Remove hook fees and protocol fee on withdrawal --- ...swap hook, already cached dynamic fee.snap | 2 +- .../cached dynamic fee, no hooks.snap | 2 +- .forge-snapshots/donate gas with 1 token.snap | 2 +- .../donate gas with 2 tokens.snap | 2 +- .../gas overhead of no-op lock.snap | 2 +- .forge-snapshots/initialize.snap | 2 +- .forge-snapshots/mint with empty hook.snap | 2 +- .forge-snapshots/mint with native token.snap | 2 +- .forge-snapshots/mint.snap | 2 +- .../mintWithEmptyHookEOAInitiated.snap | 2 +- .../modify position with noop.snap | 2 +- .../poolManager bytecode size.snap | 2 +- .forge-snapshots/simple swap with native.snap | 2 +- .forge-snapshots/simple swap.snap | 2 +- .forge-snapshots/simpleSwapEOAInitiated.snap | 2 +- .../simpleSwapNativeEOAInitiated.snap | 2 +- ...p against liquidity with native token.snap | 2 +- .forge-snapshots/swap against liquidity.snap | 2 +- .../swap burn claim for input.snap | 2 +- .../swap mint output as claim.snap | 2 +- .forge-snapshots/swap with dynamic fee.snap | 2 +- .forge-snapshots/swap with hooks.snap | 2 +- .forge-snapshots/swap with noop.snap | 2 +- .../update dynamic fee in before swap.snap | 2 +- src/Fees.sol | 44 +- src/PoolManager.sol | 44 +- src/interfaces/IFees.sol | 3 - src/interfaces/IHookFeeManager.sol | 13 - src/interfaces/IPoolManager.sol | 18 +- src/interfaces/IProtocolFeeController.sol | 2 +- src/libraries/FeeLibrary.sol | 10 - src/libraries/Hooks.sol | 10 +- src/libraries/Pool.sol | 130 +--- src/test/MockHooks.sol | 13 +- src/test/ProtocolFeeControllerTest.sol | 9 +- test/Fees.t.sol | 573 ------------------ test/Hooks.t.sol | 2 +- test/Pool.t.sol | 28 +- test/PoolManager.t.sol | 107 ++-- test/PoolManagerInitialize.t.sol | 29 +- 40 files changed, 156 insertions(+), 927 deletions(-) delete mode 100644 src/interfaces/IHookFeeManager.sol delete mode 100644 test/Fees.t.sol diff --git a/.forge-snapshots/before swap hook, already cached dynamic fee.snap b/.forge-snapshots/before swap hook, already cached dynamic fee.snap index f7c4a7117..b1a948e7c 100644 --- a/.forge-snapshots/before swap hook, already cached dynamic fee.snap +++ b/.forge-snapshots/before swap hook, already cached dynamic fee.snap @@ -1 +1 @@ -193153 \ No newline at end of file +192720 \ No newline at end of file diff --git a/.forge-snapshots/cached dynamic fee, no hooks.snap b/.forge-snapshots/cached dynamic fee, no hooks.snap index 9ea1db117..0c506469f 100644 --- a/.forge-snapshots/cached dynamic fee, no hooks.snap +++ b/.forge-snapshots/cached dynamic fee, no hooks.snap @@ -1 +1 @@ -148516 \ No newline at end of file +148082 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap index 1bc661eb3..9503a6cd0 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -139130 \ No newline at end of file +139082 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap index 36e65184f..84f92b0f9 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -186552 \ No newline at end of file +186504 \ No newline at end of file diff --git a/.forge-snapshots/gas overhead of no-op lock.snap b/.forge-snapshots/gas overhead of no-op lock.snap index d8e63744a..fd0c301b5 100644 --- a/.forge-snapshots/gas overhead of no-op lock.snap +++ b/.forge-snapshots/gas overhead of no-op lock.snap @@ -1 +1 @@ -15268 \ No newline at end of file +15224 \ No newline at end of file diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap index 5e24d8253..2d9f88574 100644 --- a/.forge-snapshots/initialize.snap +++ b/.forge-snapshots/initialize.snap @@ -1 +1 @@ -78018 \ No newline at end of file +74906 \ No newline at end of file diff --git a/.forge-snapshots/mint with empty hook.snap b/.forge-snapshots/mint with empty hook.snap index 8537190e0..95d44ac2c 100644 --- a/.forge-snapshots/mint with empty hook.snap +++ b/.forge-snapshots/mint with empty hook.snap @@ -1 +1 @@ -319051 \ No newline at end of file +318560 \ No newline at end of file diff --git a/.forge-snapshots/mint with native token.snap b/.forge-snapshots/mint with native token.snap index eff86ee1d..f8ee19892 100644 --- a/.forge-snapshots/mint with native token.snap +++ b/.forge-snapshots/mint with native token.snap @@ -1 +1 @@ -201832 \ No newline at end of file +201364 \ No newline at end of file diff --git a/.forge-snapshots/mint.snap b/.forge-snapshots/mint.snap index 78eb8f6bf..34abec0d1 100644 --- a/.forge-snapshots/mint.snap +++ b/.forge-snapshots/mint.snap @@ -1 +1 @@ -201752 \ No newline at end of file +201284 \ No newline at end of file diff --git a/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap b/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap index eda502201..69d3fc9ff 100644 --- a/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap +++ b/.forge-snapshots/mintWithEmptyHookEOAInitiated.snap @@ -1 +1 @@ -250947 \ No newline at end of file +250456 \ No newline at end of file diff --git a/.forge-snapshots/modify position with noop.snap b/.forge-snapshots/modify position with noop.snap index 8d9f09965..5662cfc29 100644 --- a/.forge-snapshots/modify position with noop.snap +++ b/.forge-snapshots/modify position with noop.snap @@ -1 +1 @@ -56515 \ No newline at end of file +56469 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index d4620039b..872c44da4 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -25378 \ No newline at end of file +23124 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index 0eb569864..c0e439a54 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -197401 \ No newline at end of file +196967 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 6b8f63604..2c047be3e 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -205969 \ No newline at end of file +205535 \ No newline at end of file diff --git a/.forge-snapshots/simpleSwapEOAInitiated.snap b/.forge-snapshots/simpleSwapEOAInitiated.snap index 33e2de06b..93a53cb1d 100644 --- a/.forge-snapshots/simpleSwapEOAInitiated.snap +++ b/.forge-snapshots/simpleSwapEOAInitiated.snap @@ -1 +1 @@ -177213 \ No newline at end of file +176801 \ No newline at end of file diff --git a/.forge-snapshots/simpleSwapNativeEOAInitiated.snap b/.forge-snapshots/simpleSwapNativeEOAInitiated.snap index 4766ba9f9..bc6e79938 100644 --- a/.forge-snapshots/simpleSwapNativeEOAInitiated.snap +++ b/.forge-snapshots/simpleSwapNativeEOAInitiated.snap @@ -1 +1 @@ -173875 \ No newline at end of file +173463 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index 8ddd40ca9..0e84741b6 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -127994 \ No newline at end of file +127595 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index 26359d0c4..3fa1f03ef 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -115485 \ No newline at end of file +115086 \ No newline at end of file diff --git a/.forge-snapshots/swap burn claim for input.snap b/.forge-snapshots/swap burn claim for input.snap index f6035b282..54e652c94 100644 --- a/.forge-snapshots/swap burn claim for input.snap +++ b/.forge-snapshots/swap burn claim for input.snap @@ -1 +1 @@ -134557 \ No newline at end of file +134282 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as claim.snap b/.forge-snapshots/swap mint output as claim.snap index 312b47616..e50e69518 100644 --- a/.forge-snapshots/swap mint output as claim.snap +++ b/.forge-snapshots/swap mint output as claim.snap @@ -1 +1 @@ -218140 \ No newline at end of file +217728 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index 599f4da06..b562855ce 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -192395 \ No newline at end of file +191962 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index 0e7a8ddb0..efdd79eb2 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -115463 \ No newline at end of file +115064 \ No newline at end of file diff --git a/.forge-snapshots/swap with noop.snap b/.forge-snapshots/swap with noop.snap index 1ffec7516..2e09d9ffc 100644 --- a/.forge-snapshots/swap with noop.snap +++ b/.forge-snapshots/swap with noop.snap @@ -1 +1 @@ -49734 \ No newline at end of file +49686 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index 2935aef3c..e23eb14f3 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -198977 \ No newline at end of file +198542 \ No newline at end of file diff --git a/src/Fees.sol b/src/Fees.sol index 72b79f298..4dc840645 100644 --- 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; @@ -32,36 +29,18 @@ abstract contract Fees is IFees, Owned { controllerGasLimit = _controllerGasLimit; } - function _fetchProtocolFees(PoolKey memory key) internal view returns (uint24 protocolFees) { - uint16 protocolSwapFee; - uint16 protocolWithdrawFee; + function _fetchProtocolFees(PoolKey memory key) internal view returns (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(); try protocolFeeController.protocolFeesForPool{gas: controllerGasLimit}(key) returns ( - uint24 updatedProtocolFees + uint16 updatedProtocolFees ) { - protocolSwapFee = uint16(updatedProtocolFees >> 12); - protocolWithdrawFee = uint16(updatedProtocolFees & 0xFFF); - protocolFees = updatedProtocolFees; } catch {} - _checkProtocolFee(protocolSwapFee); - _checkProtocolFee(protocolWithdrawFee); - } - } - - /// @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 {} + _checkProtocolFee(protocolFees); } } @@ -73,8 +52,8 @@ abstract contract Fees is IFees, Owned { /// @dev Only the lower 12 bits are used here to encode the fee denominator. function _checkProtocolFee(uint16 fee) internal pure { 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) @@ -99,17 +78,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); - } } diff --git a/src/PoolManager.sol b/src/PoolManager.sol index 2b8ffde26..d5a587721 100644 --- a/src/PoolManager.sol +++ b/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"; @@ -54,11 +53,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,7 +129,7 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims { uint24 swapFee = key.fee.isDynamicFee() ? _fetchDynamicSwapFee(key) : key.fee.getStaticFee(); - tick = pools[id].initialize(sqrtPriceX96, _fetchProtocolFees(key), _fetchHookFees(key), swapFee); + tick = pools[id].initialize(sqrtPriceX96, _fetchProtocolFees(key), swapFee); if (key.hooks.shouldCallAfterInitialize()) { if ( @@ -214,8 +213,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, @@ -227,21 +225,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) @@ -283,10 +266,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, @@ -302,9 +284,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()) { @@ -388,20 +367,13 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims { _burn(currency, amount); } - function setProtocolFees(PoolKey memory key) external { - uint24 newProtocolFees = _fetchProtocolFees(key); + function setProtocolFee(PoolKey memory key) external { + uint16 newProtocolFees = _fetchProtocolFees(key); PoolId id = key.toId(); - pools[id].setProtocolFees(newProtocolFees); + pools[id].setProtocolFee(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); - } - function updateDynamicSwapFee(PoolKey memory key) external { if (key.fee.isDynamicFee()) { uint24 newDynamicSwapFee = _fetchDynamicSwapFee(key); diff --git a/src/interfaces/IFees.sol b/src/interfaces/IFees.sol index 943f6d4cd..e77a65447 100644 --- a/src/interfaces/IFees.sol +++ b/src/interfaces/IFees.sol @@ -18,7 +18,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); } diff --git a/src/interfaces/IHookFeeManager.sol b/src/interfaces/IHookFeeManager.sol deleted file mode 100644 index 8d491a4f6..000000000 --- a/src/interfaces/IHookFeeManager.sol +++ /dev/null @@ -1,13 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; - -import {PoolKey} from "../types/PoolKey.sol"; - -/// @notice The interface for setting a fee on swap or fee on withdraw to the hook -/// @dev This callback is only made if the Fee.HOOK_SWAP_FEE_FLAG or Fee.HOOK_WITHDRAW_FEE_FLAG in set in the pool's key.fee. -interface IHookFeeManager { - /// @notice Gets the fee a hook can take at swap/withdraw. Upper bits used for swap and lower bits for withdraw. - /// @param key The pool key - /// @return The hook fees for swapping (upper bits set) and withdrawing (lower bits set). - function getHookFees(PoolKey calldata key) external view returns (uint24); -} diff --git a/src/interfaces/IPoolManager.sol b/src/interfaces/IPoolManager.sol index f66bb8eaa..c8d7998ec 100644 --- a/src/interfaces/IPoolManager.sol +++ b/src/interfaces/IPoolManager.sol @@ -82,9 +82,7 @@ interface IPoolManager is IFees, IClaims { 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); @@ -95,10 +93,7 @@ interface IPoolManager is IFees, IClaims { 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); @@ -188,12 +183,9 @@ interface IPoolManager is IFees, IClaims { /// @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; diff --git a/src/interfaces/IProtocolFeeController.sol b/src/interfaces/IProtocolFeeController.sol index 18612a37f..21b7f85ab 100644 --- a/src/interfaces/IProtocolFeeController.sol +++ b/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 protocolFeesForPool(PoolKey memory poolKey) external view returns (uint16); } diff --git a/src/libraries/FeeLibrary.sol b/src/libraries/FeeLibrary.sol index 3b5484dd2..98718cceb 100644 --- a/src/libraries/FeeLibrary.sol +++ b/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; } diff --git a/src/libraries/Hooks.sol b/src/libraries/Hooks.sol index 3577a4934..d2bdc8b3a 100644 --- a/src/libraries/Hooks.sol +++ b/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) { diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index b93265c62..5eedaf0d6 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -59,22 +59,14 @@ library Pool { /// @notice Thrown by donate if there is currently 0 liquidity, since the fees will not go to any liquidity providers error NoLiquidityToReceiveFees(); - /// Each uint24 variable packs both the swap fees and the withdraw fees represented as integer denominators (1/x). The upper 12 bits are the swap fees, and the lower 12 bits - /// are the withdraw fees. For swap fees, the upper 6 bits are the fee for trading 1 for 0, and the lower 6 are for 0 for 1 and are taken as a percentage of the lp swap fee. - /// For withdraw fees the upper 6 bits are the fee on amount1, and the lower 6 are for amount0 and are taken as a percentage of the principle amount of the underlying position. - /// bits 24 22 20 18 16 14 12 10 8 6 4 2 0 - /// | swapFees | withdrawFees | - /// ┌────────┬────────┬────────┬────────┐ - /// protocolFees: | 1->0 | 0->1 | fee1 | fee0 | - /// hookFees: | 1->0 | 0->1 | fee1 | fee0 | - /// └────────┴────────┴────────┴────────┘ struct Slot0 { // the current price uint160 sqrtPriceX96; // the current tick int24 tick; - uint24 protocolFees; - uint24 hookFees; + // protocol swap fee represented as integer denominator (1/x), taken as a % of the LP swap fee + // upper 8 bits are for 1->0, and the lower 8 are for 0->1 + uint16 protocolFee; // used for the swap fee, either static at initialize or dynamic via hook uint24 swapFee; } @@ -109,7 +101,7 @@ library Pool { if (tickUpper > TickMath.MAX_TICK) revert TickUpperOutOfBounds(tickUpper); } - function initialize(State storage self, uint160 sqrtPriceX96, uint24 protocolFees, uint24 hookFees, uint24 swapFee) + function initialize(State storage self, uint160 sqrtPriceX96, uint16 protocolFee, uint24 swapFee) internal returns (int24 tick) { @@ -117,33 +109,13 @@ library Pool { tick = TickMath.getTickAtSqrtRatio(sqrtPriceX96); - self.slot0 = Slot0({ - sqrtPriceX96: sqrtPriceX96, - tick: tick, - protocolFees: protocolFees, - hookFees: hookFees, - swapFee: swapFee - }); - } - - function getSwapFee(uint24 feesStorage) internal pure returns (uint16) { - return uint16(feesStorage >> 12); - } - - function getWithdrawFee(uint24 feesStorage) internal pure returns (uint16) { - return uint16(feesStorage & 0xFFF); - } - - function setProtocolFees(State storage self, uint24 protocolFees) internal { - if (self.isNotInitialized()) revert PoolNotInitialized(); - - self.slot0.protocolFees = protocolFees; + self.slot0 = Slot0({sqrtPriceX96: sqrtPriceX96, tick: tick, protocolFee: protocolFee, swapFee: swapFee}); } - function setHookFees(State storage self, uint24 hookFees) internal { + function setProtocolFee(State storage self, uint16 protocolFee) internal { if (self.isNotInitialized()) revert PoolNotInitialized(); - self.slot0.hookFees = hookFees; + self.slot0.protocolFee = protocolFee; } /// @notice Only dynamic fee pools may update the swap fee. @@ -173,20 +145,13 @@ library Pool { uint256 feeGrowthInside1X128; } - struct FeeAmounts { - uint256 feeForProtocol0; - uint256 feeForProtocol1; - uint256 feeForHook0; - uint256 feeForHook1; - } - /// @notice Effect changes to a position in a pool /// @dev PoolManager checks that the pool is initialized before calling /// @param params the position details and the change to the position's liquidity to effect /// @return result the deltas of the token balances of the pool function modifyPosition(State storage self, ModifyPositionParams memory params) internal - returns (BalanceDelta result, FeeAmounts memory fees) + returns (BalanceDelta result) { checkTicks(params.tickLower, params.tickUpper); @@ -280,70 +245,15 @@ library Pool { } } - if (params.liquidityDelta < 0 && getWithdrawFee(self.slot0.hookFees) > 0) { - // Only take fees if the hook withdraw fee is set and the liquidity is being removed. - fees = _calculateExternalFees(self, result); - - // Amounts are balances owed to the pool. When negative, they represent the balance a user can take. - // Since protocol and hook fees are extracted on the balance a user can take - // they are owed (added) back to the pool where they are kept to be collected by the fee recipients. - result = result - + toBalanceDelta( - fees.feeForHook0.toInt128() + fees.feeForProtocol0.toInt128(), - fees.feeForHook1.toInt128() + fees.feeForProtocol1.toInt128() - ); - } - // Fees earned from LPing are removed from the pool balance. result = result - toBalanceDelta(feesOwed0.toInt128(), feesOwed1.toInt128()); } - function _calculateExternalFees(State storage self, BalanceDelta result) - internal - view - returns (FeeAmounts memory fees) - { - int128 amount0 = result.amount0(); - int128 amount1 = result.amount1(); - - Slot0 memory slot0Cache = self.slot0; - uint24 hookFees = slot0Cache.hookFees; - uint24 protocolFees = slot0Cache.protocolFees; - - uint16 hookFee0 = getWithdrawFee(hookFees) % 64; - uint16 hookFee1 = getWithdrawFee(hookFees) >> 6; - - uint16 protocolFee0 = getWithdrawFee(protocolFees) % 64; - uint16 protocolFee1 = getWithdrawFee(protocolFees) >> 6; - - if (amount0 < 0 && hookFee0 > 0) { - fees.feeForHook0 = uint128(-amount0) / hookFee0; - } - if (amount1 < 0 && hookFee1 > 0) { - fees.feeForHook1 = uint128(-amount1) / hookFee1; - } - - // A protocol fee is only applied if the hook fee is applied. - if (protocolFee0 > 0 && fees.feeForHook0 > 0) { - fees.feeForProtocol0 = fees.feeForHook0 / protocolFee0; - fees.feeForHook0 -= fees.feeForProtocol0; - } - - if (protocolFee1 > 0 && fees.feeForHook1 > 0) { - fees.feeForProtocol1 = fees.feeForHook1 / protocolFee1; - fees.feeForHook1 -= fees.feeForProtocol1; - } - - return fees; - } - struct SwapCache { // liquidity at the beginning of the swap uint128 liquidityStart; // the protocol fee for the input token - uint16 protocolFee; - // the hook fee for the input token - uint16 hookFee; + uint8 protocolFee; } // the top level state of the swap, the results of which are recorded in storage at the end @@ -390,13 +300,7 @@ library Pool { /// @dev PoolManager checks that the pool is initialized before calling function swap(State storage self, SwapParams memory params) internal - returns ( - BalanceDelta result, - uint256 feeForProtocol, - uint256 feeForHook, - uint24 swapFee, - SwapState memory state - ) + returns (BalanceDelta result, uint256 feeForProtocol, uint24 swapFee, SwapState memory state) { if (params.amountSpecified == 0) revert SwapAmountCannotBeZero(); @@ -420,10 +324,7 @@ library Pool { SwapCache memory cache = SwapCache({ liquidityStart: self.liquidity, - protocolFee: params.zeroForOne - ? (getSwapFee(slot0Start.protocolFees) % 64) - : (getSwapFee(slot0Start.protocolFees) >> 6), - hookFee: params.zeroForOne ? (getSwapFee(slot0Start.hookFees) % 64) : (getSwapFee(slot0Start.hookFees) >> 6) + protocolFee: params.zeroForOne ? uint8(slot0Start.protocolFee % 256) : uint8(slot0Start.protocolFee >> 8) }); bool exactInput = params.amountSpecified > 0; @@ -492,15 +393,6 @@ library Pool { } } - if (cache.hookFee > 0) { - // step.feeAmount has already been updated to account for the protocol fee - uint256 delta = step.feeAmount / cache.hookFee; - unchecked { - step.feeAmount -= delta; - feeForHook += delta; - } - } - // update global fee tracker if (state.liquidity > 0) { unchecked { diff --git a/src/test/MockHooks.sol b/src/test/MockHooks.sol index e6a9ffa23..f1865e7a1 100644 --- a/src/test/MockHooks.sol +++ b/src/test/MockHooks.sol @@ -6,10 +6,9 @@ import {IHooks} from "../interfaces/IHooks.sol"; import {IPoolManager} from "../interfaces/IPoolManager.sol"; import {PoolKey} from "../types/PoolKey.sol"; import {BalanceDelta} from "../types/BalanceDelta.sol"; -import {IHookFeeManager} from "../interfaces/IHookFeeManager.sol"; import {PoolId, PoolIdLibrary} from "../types/PoolId.sol"; -contract MockHooks is IHooks, IHookFeeManager { +contract MockHooks is IHooks { using PoolIdLibrary for PoolKey; using Hooks for IHooks; @@ -26,8 +25,6 @@ contract MockHooks is IHooks, IHookFeeManager { mapping(PoolId => uint16) public swapFees; - mapping(PoolId => uint16) public withdrawFees; - function beforeInitialize(address, PoolKey calldata, uint160, bytes calldata hookData) external override @@ -113,10 +110,6 @@ contract MockHooks is IHooks, IHookFeeManager { return returnValues[selector] == bytes4(0) ? selector : returnValues[selector]; } - function getHookFees(PoolKey calldata key) external view override returns (uint24) { - return (uint24(swapFees[key.toId()]) << 12 | withdrawFees[key.toId()]); - } - function setReturnValue(bytes4 key, bytes4 value) external { returnValues[key] = value; } @@ -124,8 +117,4 @@ contract MockHooks is IHooks, IHookFeeManager { function setSwapFee(PoolKey calldata key, uint16 value) external { swapFees[key.toId()] = value; } - - function setWithdrawFee(PoolKey calldata key, uint16 value) external { - withdrawFees[key.toId()] = value; - } } diff --git a/src/test/ProtocolFeeControllerTest.sol b/src/test/ProtocolFeeControllerTest.sol index 369ba42bd..74bd16a3e 100644 --- a/src/test/ProtocolFeeControllerTest.sol +++ b/src/test/ProtocolFeeControllerTest.sol @@ -10,18 +10,13 @@ contract ProtocolFeeControllerTest is IProtocolFeeController { using PoolIdLibrary for PoolKey; mapping(PoolId => uint16) public swapFeeForPool; - mapping(PoolId => uint16) public withdrawFeeForPool; - function protocolFeesForPool(PoolKey memory key) external view returns (uint24) { - return (uint24(swapFeeForPool[key.toId()]) << 12 | withdrawFeeForPool[key.toId()]); + function protocolFeesForPool(PoolKey memory key) external view returns (uint16) { + return swapFeeForPool[key.toId()]; } // for tests to set pool protocol fees function setSwapFeeForPool(PoolId id, uint16 fee) external { swapFeeForPool[id] = fee; } - - function setWithdrawFeeForPool(PoolId id, uint16 fee) external { - withdrawFeeForPool[id] = fee; - } } diff --git a/test/Fees.t.sol b/test/Fees.t.sol deleted file mode 100644 index a79457f9e..000000000 --- a/test/Fees.t.sol +++ /dev/null @@ -1,573 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.20; - -import {Test} from "forge-std/Test.sol"; -import {Vm} from "forge-std/Vm.sol"; -import {IHooks} from "../src/interfaces/IHooks.sol"; -import {Hooks} from "../src/libraries/Hooks.sol"; -import {FeeLibrary} from "../src/libraries/FeeLibrary.sol"; -import {IPoolManager} from "../src/interfaces/IPoolManager.sol"; -import {IFees} from "../src/interfaces/IFees.sol"; -import {PoolManager} from "../src/PoolManager.sol"; -import {TickMath} from "../src/libraries/TickMath.sol"; -import {Pool} from "../src/libraries/Pool.sol"; -import {PoolIdLibrary} from "../src/types/PoolId.sol"; -import {Deployers} from "./utils/Deployers.sol"; -import {PoolModifyPositionTest} from "../src/test/PoolModifyPositionTest.sol"; -import {Currency, CurrencyLibrary} from "../src/types/Currency.sol"; -import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; -import {MockHooks} from "../src/test/MockHooks.sol"; -import {PoolSwapTest} from "../src/test/PoolSwapTest.sol"; -import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; -import {ProtocolFeeControllerTest} from "../src/test/ProtocolFeeControllerTest.sol"; -import {IProtocolFeeController} from "../src/interfaces/IProtocolFeeController.sol"; -import {Fees} from "../src/Fees.sol"; -import {BalanceDelta} from "../src/types/BalanceDelta.sol"; -import {PoolKey} from "../src/types/PoolKey.sol"; - -contract FeesTest is Test, Deployers, GasSnapshot { - using Hooks for IHooks; - using Pool for Pool.State; - using PoolIdLibrary for PoolKey; - using CurrencyLibrary for Currency; - - MockHooks hook; - - // key0 hook enabled fee on swap - PoolKey key0; - // key1 hook enabled fee on withdraw - PoolKey key1; - // key2 hook enabled fee on swap and withdraw - PoolKey key2; - // key3 no hook - PoolKey key3; - - bool _zeroForOne = true; - bool _oneForZero = false; - - function setUp() public { - deployFreshManagerAndRouters(); - (currency0, currency1) = deployMintAndApprove2Currencies(); - - address hookAddr = address(99); // can't be a zero address, but does not have to have any other hook flags specified - MockHooks impl = new MockHooks(); - vm.etch(hookAddr, address(impl).code); - hook = MockHooks(hookAddr); - - key0 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: FeeLibrary.HOOK_SWAP_FEE_FLAG | uint24(3000), - hooks: hook, - tickSpacing: 60 - }); - - key1 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: FeeLibrary.HOOK_WITHDRAW_FEE_FLAG | uint24(3000), - hooks: hook, - tickSpacing: 60 - }); - - key2 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: FeeLibrary.HOOK_WITHDRAW_FEE_FLAG | FeeLibrary.HOOK_SWAP_FEE_FLAG | uint24(3000), - hooks: hook, - tickSpacing: 60 - }); - - key3 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: uint24(3000), - hooks: IHooks(address(0)), - tickSpacing: 60 - }); - - initializeRouter.initialize(key0, SQRT_RATIO_1_1, ZERO_BYTES); - initializeRouter.initialize(key1, SQRT_RATIO_1_1, ZERO_BYTES); - initializeRouter.initialize(key2, SQRT_RATIO_1_1, ZERO_BYTES); - initializeRouter.initialize(key3, SQRT_RATIO_1_1, ZERO_BYTES); - } - - function testInitializeFailsNoHook() public { - PoolKey memory key4 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: FeeLibrary.HOOK_WITHDRAW_FEE_FLAG | FeeLibrary.HOOK_SWAP_FEE_FLAG | uint24(3000), - hooks: IHooks(address(0)), - tickSpacing: 60 - }); - - vm.expectRevert(abi.encodeWithSelector(Hooks.HookAddressNotValid.selector, address(0))); - initializeRouter.initialize(key4, SQRT_RATIO_1_1, ZERO_BYTES); - - key4 = PoolKey({ - currency0: currency0, - currency1: currency1, - fee: FeeLibrary.DYNAMIC_FEE_FLAG, - hooks: IHooks(address(0)), - tickSpacing: 60 - }); - - vm.expectRevert(abi.encodeWithSelector(Hooks.HookAddressNotValid.selector, address(0))); - initializeRouter.initialize(key4, SQRT_RATIO_1_1, ZERO_BYTES); - } - - function testInitializeHookSwapFee(uint16 fee) public { - vm.assume(fee < 2 ** 12); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.hookFees), 0); - - hook.setSwapFee(key0, fee); - manager.setHookFees(key0); - - (slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.hookFees), fee); - assertEq(getWithdrawFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.protocolFees), 0); - assertEq(getWithdrawFee(slot0.protocolFees), 0); - } - - function testInitializeHookWithdrawFee(uint16 fee) public { - vm.assume(fee < 2 ** 12); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key1.toId()); - assertEq(getWithdrawFee(slot0.hookFees), 0); - - hook.setWithdrawFee(key1, fee); - manager.setHookFees(key1); - - (slot0,,,) = manager.pools(key1.toId()); - assertEq(getWithdrawFee(slot0.hookFees), fee); - assertEq(getSwapFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.protocolFees), 0); - assertEq(getWithdrawFee(slot0.protocolFees), 0); - } - - function testInitializeBothHookFee(uint16 swapFee, uint16 withdrawFee) public { - vm.assume(swapFee < 2 ** 12 && withdrawFee < 2 ** 12); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key2.toId()); - assertEq(getSwapFee(slot0.hookFees), 0); - assertEq(getWithdrawFee(slot0.hookFees), 0); - - hook.setSwapFee(key2, swapFee); - hook.setWithdrawFee(key2, withdrawFee); - manager.setHookFees(key2); - - (slot0,,,) = manager.pools(key2.toId()); - assertEq(getSwapFee(slot0.hookFees), swapFee); - assertEq(getWithdrawFee(slot0.hookFees), withdrawFee); - } - - function testInitializeHookProtocolSwapFee(uint16 hookSwapFee, uint16 protocolSwapFee) public { - vm.assume(hookSwapFee < 2 ** 12 && protocolSwapFee < 2 ** 12); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.protocolFees), 0); - - feeController.setSwapFeeForPool(key0.toId(), protocolSwapFee); - - uint16 protocolSwapFee1 = protocolSwapFee >> 6; - uint16 protocolSwapFee0 = protocolSwapFee % 64; - - if ((protocolSwapFee0 != 0 && protocolSwapFee0 < 4) || (protocolSwapFee1 != 0 && protocolSwapFee1 < 4)) { - protocolSwapFee = 0; - vm.expectRevert(IFees.FeeTooLarge.selector); - } - manager.setProtocolFees(key0); - - hook.setSwapFee(key0, hookSwapFee); - manager.setHookFees(key0); - - (slot0,,,) = manager.pools(key0.toId()); - - assertEq(getWithdrawFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.hookFees), hookSwapFee); - assertEq(getSwapFee(slot0.protocolFees), protocolSwapFee); - assertEq(getWithdrawFee(slot0.protocolFees), 0); - } - - function testInitializeAllFees( - uint16 hookSwapFee, - uint16 hookWithdrawFee, - uint16 protocolSwapFee, - uint16 protocolWithdrawFee - ) public { - vm.assume( - hookSwapFee < 2 ** 12 && hookWithdrawFee < 2 ** 12 && protocolSwapFee < 2 ** 12 - && protocolWithdrawFee < 2 ** 12 - ); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key2.toId()); - assertEq(getSwapFee(slot0.hookFees), 0); - assertEq(getWithdrawFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.protocolFees), 0); - assertEq(getWithdrawFee(slot0.protocolFees), 0); - - feeController.setSwapFeeForPool(key2.toId(), protocolSwapFee); - feeController.setWithdrawFeeForPool(key2.toId(), protocolWithdrawFee); - - uint16 protocolSwapFee1 = protocolSwapFee >> 6; - uint16 protocolSwapFee0 = protocolSwapFee % 64; - uint16 protocolWithdrawFee1 = protocolWithdrawFee >> 6; - uint16 protocolWithdrawFee0 = protocolWithdrawFee % 64; - - if ( - (protocolSwapFee1 != 0 && protocolSwapFee1 < 4) || (protocolSwapFee0 != 0 && protocolSwapFee0 < 4) - || (protocolWithdrawFee1 != 0 && protocolWithdrawFee1 < 4) - || (protocolWithdrawFee0 != 0 && protocolWithdrawFee0 < 4) - ) { - protocolSwapFee = 0; - protocolWithdrawFee = 0; - vm.expectRevert(IFees.FeeTooLarge.selector); - } - manager.setProtocolFees(key2); - - hook.setSwapFee(key2, hookSwapFee); - hook.setWithdrawFee(key2, hookWithdrawFee); - manager.setHookFees(key2); - - (slot0,,,) = manager.pools(key2.toId()); - - assertEq(getWithdrawFee(slot0.hookFees), hookWithdrawFee); - assertEq(getSwapFee(slot0.hookFees), hookSwapFee); - assertEq(getSwapFee(slot0.protocolFees), protocolSwapFee); - assertEq(getWithdrawFee(slot0.protocolFees), protocolWithdrawFee); - } - - function testProtocolFeeOnWithdrawalRemainsZeroIfNoHookWithdrawalFeeSet( - uint16 hookSwapFee, - uint16 protocolWithdrawFee - ) public { - vm.assume(hookSwapFee < 2 ** 12 && protocolWithdrawFee < 2 ** 12); - vm.assume(protocolWithdrawFee >> 6 >= 4); - vm.assume(protocolWithdrawFee % 64 >= 4); - - // On a pool whose hook has not set a withdraw fee, the protocol should not accrue any value even if it has set a withdraw fee. - hook.setSwapFee(key0, hookSwapFee); - manager.setHookFees(key0); - - // set fee on the fee controller - feeController.setWithdrawFeeForPool(key0.toId(), protocolWithdrawFee); - manager.setProtocolFees(key0); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId()); - assertEq(getWithdrawFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.hookFees), hookSwapFee); - assertEq(getSwapFee(slot0.protocolFees), 0); - assertEq(getWithdrawFee(slot0.protocolFees), protocolWithdrawFee); - - IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-60, 60, 10e18); - modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES); - - IPoolManager.ModifyPositionParams memory params2 = IPoolManager.ModifyPositionParams(-60, 60, -10e18); - modifyPositionRouter.modifyPosition(key0, params2, ZERO_BYTES); - - // Fees dont accrue when key.fee does not specify a withdrawal param even if the protocol fee is set. - assertEq(manager.protocolFeesAccrued(currency0), 0); - assertEq(manager.protocolFeesAccrued(currency1), 0); - assertEq(manager.hookFeesAccrued(address(key0.hooks), currency0), 0); - assertEq(manager.hookFeesAccrued(address(key0.hooks), currency1), 0); - } - - // function testFeeOutOfBoundsReverts(uint16 newFee) external { - // newFee = uint16(bound(newFee, 2 ** 12, type(uint16).max)); - - // hook.setSwapFee(key0, newFee); - // vm.expectRevert(abi.encodeWithSelector(IFees.FeeDenominatorOutOfBounds.selector, newFee)); - // manager.setHookFees(key0); - - // hook.setWithdrawFee(key0, newFee); - // vm.expectRevert(abi.encodeWithSelector(IFees.FeeDenominatorOutOfBounds.selector, newFee)); - // manager.setHookFees(key0); - - // manager.setProtocolFeeController(IProtocolFeeController(feeController)); - - // feeController.setSwapFeeForPool(key0.toId(), newFee); - // vm.expectRevert(abi.encodeWithSelector(IFees.FeeDenominatorOutOfBounds.selector, newFee)); - // manager.setProtocolFees(key0); - - // feeController.setWithdrawFeeForPool(key0.toId(), newFee); - // vm.expectRevert(abi.encodeWithSelector(IFees.FeeDenominatorOutOfBounds.selector, newFee)); - // manager.setProtocolFees(key0); - // } - - function testHookWithdrawFeeProtocolWithdrawFee(uint16 hookWithdrawFee, uint16 protocolWithdrawFee) public { - vm.assume(protocolWithdrawFee < 2 ** 12); - vm.assume(hookWithdrawFee < 2 ** 12); - vm.assume(protocolWithdrawFee >> 6 >= 4); - vm.assume(protocolWithdrawFee % 64 >= 4); - - hook.setWithdrawFee(key1, hookWithdrawFee); - manager.setHookFees(key1); - - feeController.setWithdrawFeeForPool(key1.toId(), protocolWithdrawFee); - manager.setProtocolFees(key1); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key1.toId()); - - assertEq(getWithdrawFee(slot0.hookFees), hookWithdrawFee); - assertEq(getSwapFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.protocolFees), 0); - assertEq(getWithdrawFee(slot0.protocolFees), protocolWithdrawFee); - - int256 liquidityDelta = 10000; - // The underlying amount for a liquidity delta of 10000 is 29. - uint256 underlyingAmount0 = 29; - uint256 underlyingAmount1 = 29; - - IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-60, 60, liquidityDelta); - BalanceDelta delta = modifyPositionRouter.modifyPosition(key1, params, ZERO_BYTES); - - // Fees dont accrue for positive liquidity delta. - assertEq(manager.protocolFeesAccrued(currency0), 0); - assertEq(manager.protocolFeesAccrued(currency1), 0); - assertEq(manager.hookFeesAccrued(address(key1.hooks), currency0), 0); - assertEq(manager.hookFeesAccrued(address(key1.hooks), currency1), 0); - - IPoolManager.ModifyPositionParams memory params2 = IPoolManager.ModifyPositionParams(-60, 60, -liquidityDelta); - delta = modifyPositionRouter.modifyPosition(key1, params2, ZERO_BYTES); - - uint16 hookFee0 = (hookWithdrawFee % 64); - uint16 hookFee1 = (hookWithdrawFee >> 6); - uint16 protocolFee0 = (protocolWithdrawFee % 64); - uint16 protocolFee1 = (protocolWithdrawFee >> 6); - - // Fees should accrue to both the protocol and hook. - uint256 initialHookAmount0 = hookFee0 == 0 ? 0 : underlyingAmount0 / hookFee0; - uint256 initialHookAmount1 = hookFee1 == 0 ? 0 : underlyingAmount1 / hookFee1; - - uint256 expectedProtocolAmount0 = protocolFee0 == 0 ? 0 : initialHookAmount0 / protocolFee0; - uint256 expectedProtocolAmount1 = protocolFee1 == 0 ? 0 : initialHookAmount1 / protocolFee1; - // Adjust the hook fee amounts after the protocol fee is taken. - uint256 expectedHookFee0 = initialHookAmount0 - expectedProtocolAmount0; - uint256 expectedHookFee1 = initialHookAmount1 - expectedProtocolAmount1; - - assertEq(manager.protocolFeesAccrued(currency0), expectedProtocolAmount0); - assertEq(manager.protocolFeesAccrued(currency1), expectedProtocolAmount1); - assertEq(manager.hookFeesAccrued(address(key1.hooks), currency0), expectedHookFee0); - assertEq(manager.hookFeesAccrued(address(key1.hooks), currency1), expectedHookFee1); - } - - function testNoHookProtocolFee(uint16 protocolSwapFee, uint16 protocolWithdrawFee) public { - vm.assume(protocolSwapFee < 2 ** 12 && protocolWithdrawFee < 2 ** 12); - - vm.assume(protocolSwapFee >> 6 >= 4); - vm.assume(protocolSwapFee % 64 >= 4); - vm.assume(protocolWithdrawFee >> 6 >= 4); - vm.assume(protocolWithdrawFee % 64 >= 4); - - feeController.setSwapFeeForPool(key3.toId(), protocolSwapFee); - feeController.setWithdrawFeeForPool(key3.toId(), protocolWithdrawFee); - manager.setProtocolFees(key3); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key3.toId()); - assertEq(getWithdrawFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.hookFees), 0); - assertEq(getSwapFee(slot0.protocolFees), protocolSwapFee); - assertEq(getWithdrawFee(slot0.protocolFees), protocolWithdrawFee); - - int256 liquidityDelta = 10000; - IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-60, 60, liquidityDelta); - modifyPositionRouter.modifyPosition(key3, params, ZERO_BYTES); - - // Fees dont accrue for positive liquidity delta. - assertEq(manager.protocolFeesAccrued(currency0), 0); - assertEq(manager.protocolFeesAccrued(currency1), 0); - assertEq(manager.hookFeesAccrued(address(key3.hooks), currency0), 0); - assertEq(manager.hookFeesAccrued(address(key3.hooks), currency1), 0); - - IPoolManager.ModifyPositionParams memory params2 = IPoolManager.ModifyPositionParams(-60, 60, -liquidityDelta); - modifyPositionRouter.modifyPosition(key3, params2, ZERO_BYTES); - - uint16 protocolSwapFee1 = (protocolSwapFee >> 6); - - // No fees should accrue bc there is no hook so the protocol cant take withdraw fees. - assertEq(manager.protocolFeesAccrued(currency0), 0); - assertEq(manager.protocolFeesAccrued(currency1), 0); - - // add larger liquidity - params = IPoolManager.ModifyPositionParams(-60, 60, 10e18); - modifyPositionRouter.modifyPosition(key3, params, ZERO_BYTES); - - MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max); - swapRouter.swap( - key3, - IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1), - PoolSwapTest.TestSettings(true, true, false), - ZERO_BYTES - ); - // key3 pool is 30 bps => 10000 * 0.003 (.3%) = 30 - uint256 expectedSwapFeeAccrued = 30; - - uint256 expectedProtocolAmount1 = protocolSwapFee1 == 0 ? 0 : expectedSwapFeeAccrued / protocolSwapFee1; - assertEq(manager.protocolFeesAccrued(currency0), 0); - assertEq(manager.protocolFeesAccrued(currency1), expectedProtocolAmount1); - } - - function testProtocolSwapFeeAndHookSwapFeeSameDirection() public { - uint16 protocolFee = _computeFee(_oneForZero, 10); // 10% on 1 to 0 swaps - feeController.setSwapFeeForPool(key0.toId(), protocolFee); - manager.setProtocolFees(key0); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.protocolFees), protocolFee); - assertEq(getWithdrawFee(slot0.protocolFees), 0); - - uint16 hookFee = _computeFee(_oneForZero, 5); // 20% on 1 to 0 swaps - hook.setSwapFee(key0, hookFee); - manager.setHookFees(key0); - (slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.hookFees), hookFee); - assertEq(getWithdrawFee(slot0.hookFees), 0); - - IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-120, 120, 10e18); - modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES); - // 1 for 0 swap - MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max); - swapRouter.swap( - key0, - IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1), - PoolSwapTest.TestSettings(true, true, false), - ZERO_BYTES - ); - - assertEq(manager.protocolFeesAccrued(currency1), 3); // 10% of 30 is 3 - assertEq(manager.hookFeesAccrued(address(key0.hooks), currency1), 5); // 27 * .2 is 5.4 so 5 rounding down - } - - function testInitializeWithSwapProtocolFeeAndHookFeeDifferentDirections() public { - uint16 protocolFee = _computeFee(_oneForZero, 10); // 10% fee on 1 to 0 swaps - feeController.setSwapFeeForPool(key0.toId(), protocolFee); - manager.setProtocolFees(key0); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.protocolFees), protocolFee); - assertEq(getWithdrawFee(slot0.protocolFees), 0); - - uint16 hookFee = _computeFee(_zeroForOne, 5); // 20% on 0 to 1 swaps - - hook.setSwapFee(key0, hookFee); - manager.setHookFees(key0); - (slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.hookFees), hookFee); - assertEq(getWithdrawFee(slot0.hookFees), 0); - - IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-120, 120, 10e18); - modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES); - // 1 for 0 swap - MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max); - swapRouter.swap( - key0, - IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1), - PoolSwapTest.TestSettings(true, true, false), - ZERO_BYTES - ); - - assertEq(manager.protocolFeesAccrued(currency1), 3); // 10% of 30 is 3 - assertEq(manager.hookFeesAccrued(address(key0.hooks), currency1), 0); // hook fee only taken on 0 to 1 swaps - } - - function testSwapWithProtocolFeeAllAndHookFeeAllButOnlySwapFlag() public { - // Protocol should not be able to withdraw since the hook withdraw fee is not set - uint16 protocolFee = _computeFee(_oneForZero, 4) | _computeFee(_zeroForOne, 4); // max fees on both amounts - feeController.setWithdrawFeeForPool(key0.toId(), protocolFee); // - manager.setProtocolFees(key0); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.protocolFees), 0); - assertEq(getWithdrawFee(slot0.protocolFees), protocolFee); // successfully sets the fee, but is never applied - - uint16 hookSwapFee = _computeFee(_oneForZero, 4); // 25% on 1 to 0 swaps - uint16 hookWithdrawFee = _computeFee(_oneForZero, 4) | _computeFee(_zeroForOne, 4); // max fees on both amounts - hook.setSwapFee(key0, hookSwapFee); - hook.setWithdrawFee(key0, hookWithdrawFee); - manager.setHookFees(key0); - (slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.hookFees), hookSwapFee); - assertEq(getWithdrawFee(slot0.hookFees), 0); // Even though the contract sets a withdraw fee it will not be applied bc the pool key.fee did not assert a withdraw flag. - - IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-120, 120, 10e18); - modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES); - // 1 for 0 swap - MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max); - swapRouter.swap( - key0, - IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1), - PoolSwapTest.TestSettings(true, true, false), - ZERO_BYTES - ); - - assertEq(manager.protocolFeesAccrued(currency1), 0); // No protocol fee was accrued on swap - assertEq(manager.protocolFeesAccrued(currency0), 0); // No protocol fee was accrued on swap - assertEq(manager.hookFeesAccrued(address(key0.hooks), currency1), 7); // 25% on 1 to 0, 25% of 30 is 7.5 so 7 - - modifyPositionRouter.modifyPosition(key0, IPoolManager.ModifyPositionParams(-120, 120, -10e18), ZERO_BYTES); - - assertEq(manager.protocolFeesAccrued(currency1), 0); // No protocol fee was accrued on withdraw - assertEq(manager.protocolFeesAccrued(currency0), 0); // No protocol fee was accrued on withdraw - assertEq(manager.hookFeesAccrued(address(key0.hooks), currency1), 7); // Same amount of fees for hook. - assertEq(manager.hookFeesAccrued(address(key0.hooks), currency0), 0); // Same amount of fees for hook. - } - - function testCollectFees() public { - uint16 protocolFee = _computeFee(_oneForZero, 10); // 10% on 1 to 0 swaps - feeController.setSwapFeeForPool(key0.toId(), protocolFee); - manager.setProtocolFees(key0); - - (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.protocolFees), protocolFee); - - uint16 hookFee = _computeFee(_oneForZero, 5); // 20% on 1 to 0 swaps - hook.setSwapFee(key0, hookFee); - manager.setHookFees(key0); - - (slot0,,,) = manager.pools(key0.toId()); - assertEq(getSwapFee(slot0.hookFees), hookFee); - - IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-120, 120, 10e18); - modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES); - // 1 for 0 swap - MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max); - swapRouter.swap( - key0, - IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1), - PoolSwapTest.TestSettings(true, true, false), - ZERO_BYTES - ); - - uint256 expectedProtocolFees = 3; // 10% of 30 is 3 - vm.prank(address(feeController)); - manager.collectProtocolFees(address(feeController), currency1, 0); - assertEq(currency1.balanceOf(address(feeController)), expectedProtocolFees); - - uint256 expectedHookFees = 5; // 20% of 27 (30-3) is 5.4, round down is 5 - vm.prank(address(hook)); - // Addr(0) recipient will be the hook. - manager.collectHookFees(address(hook), currency1, 0); - assertEq(currency1.balanceOf(address(hook)), expectedHookFees); - } - - // If zeroForOne is true, then value is set on the lower bits. If zeroForOne is false, then value is set on the higher bits. - function _computeFee(bool zeroForOne, uint16 value) internal pure returns (uint16 fee) { - if (zeroForOne) { - fee = value % 64; - } else { - fee = value << 6; - } - } - - function getSwapFee(uint24 feesStorage) internal pure returns (uint16) { - return uint16(feesStorage >> 12); - } - - function getWithdrawFee(uint24 feesStorage) internal pure returns (uint16) { - return uint16(feesStorage & 0xFFF); - } -} diff --git a/test/Hooks.t.sol b/test/Hooks.t.sol index db0707640..c768c5187 100644 --- a/test/Hooks.t.sol +++ b/test/Hooks.t.sol @@ -46,7 +46,7 @@ contract HooksTest is Test, Deployers, GasSnapshot { function test_initialize_succeedsWithHook() public { initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, new bytes(123)); - (uint160 sqrtPriceX96,,,) = manager.getSlot0(uninitializedKey.toId()); + (uint160 sqrtPriceX96,,) = manager.getSlot0(uninitializedKey.toId()); assertEq(sqrtPriceX96, SQRT_RATIO_1_1); assertEq(mockHooks.beforeInitializeData(), new bytes(123)); assertEq(mockHooks.afterInitializeData(), new bytes(123)); diff --git a/test/Pool.t.sol b/test/Pool.t.sol index 76179b089..b900eedb3 100644 --- a/test/Pool.t.sol +++ b/test/Pool.t.sol @@ -14,26 +14,14 @@ contract PoolTest is Test { Pool.State state; - function testPoolInitialize(uint160 sqrtPriceX96, uint16 protocolFee, uint16 hookFee, uint24 dynamicFee) public { - vm.assume(protocolFee < 2 ** 12 && hookFee < 2 ** 12); - + function testPoolInitialize(uint160 sqrtPriceX96, uint16 protocolFee, uint24 dynamicFee) public { if (sqrtPriceX96 < TickMath.MIN_SQRT_RATIO || sqrtPriceX96 >= TickMath.MAX_SQRT_RATIO) { vm.expectRevert(TickMath.InvalidSqrtRatio.selector); - state.initialize( - sqrtPriceX96, - _formatSwapAndWithdrawFee(protocolFee, protocolFee), - _formatSwapAndWithdrawFee(hookFee, hookFee), - dynamicFee - ); + state.initialize(sqrtPriceX96, protocolFee, dynamicFee); } else { - state.initialize( - sqrtPriceX96, - _formatSwapAndWithdrawFee(protocolFee, protocolFee), - _formatSwapAndWithdrawFee(hookFee, hookFee), - dynamicFee - ); + state.initialize(sqrtPriceX96, protocolFee, dynamicFee); assertEq(state.slot0.sqrtPriceX96, sqrtPriceX96); - assertEq(state.slot0.protocolFees >> 12, protocolFee); + assertEq(state.slot0.protocolFee, protocolFee); assertEq(state.slot0.tick, TickMath.getTickAtSqrtRatio(sqrtPriceX96)); assertLt(state.slot0.tick, TickMath.MAX_TICK); assertGt(state.slot0.tick, TickMath.MIN_TICK - 1); @@ -45,7 +33,7 @@ contract PoolTest is Test { vm.assume(params.tickSpacing >= TickMath.MIN_TICK_SPACING); vm.assume(params.tickSpacing <= TickMath.MAX_TICK_SPACING); - testPoolInitialize(sqrtPriceX96, 0, 0, 0); + testPoolInitialize(sqrtPriceX96, 0, 0); if (params.tickLower >= params.tickUpper) { vm.expectRevert(abi.encodeWithSelector(Pool.TicksMisordered.selector, params.tickLower, params.tickUpper)); @@ -79,7 +67,7 @@ contract PoolTest is Test { vm.assume(params.tickSpacing <= TickMath.MAX_TICK_SPACING); vm.assume(swapFee < 1000000); - testPoolInitialize(sqrtPriceX96, 0, 0, 0); + testPoolInitialize(sqrtPriceX96, 0, 0); Pool.Slot0 memory slot0 = state.slot0; if (params.amountSpecified == 0) { @@ -114,8 +102,4 @@ contract PoolTest is Test { assertGe(state.slot0.sqrtPriceX96, params.sqrtPriceLimitX96); } } - - function _formatSwapAndWithdrawFee(uint16 swapFee, uint16 withdrawFee) internal pure returns (uint24) { - return (uint24(swapFee) << 12) | withdrawFee; - } } diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol index 17ee83518..1c7f1cccc 100644 --- a/test/PoolManager.t.sol +++ b/test/PoolManager.t.sol @@ -50,7 +50,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { ); event Mint(address indexed to, Currency indexed currency, uint256 amount); event Burn(address indexed from, Currency indexed currency, uint256 amount); - event ProtocolFeeUpdated(PoolId indexed id, uint24 protocolFees); + event ProtocolFeeUpdated(PoolId indexed id, uint16 protocolFee); PoolLockTest lockTest; @@ -500,7 +500,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { snapEnd(); } - function test_swap_GasMintClaimIfOutputNotTaken() public { + function test_swap_mintClaimIfOutputNotTaken_gas() public { IPoolManager.SwapParams memory params = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_RATIO_1_2}); @@ -517,7 +517,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { assertEq(claimsBalance, 98); } - function test_swap_GasUseClaimAsInput() public { + function test_swap_useClaimAsInput_gas() public { IPoolManager.SwapParams memory params = IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100, sqrtPriceLimitX96: SQRT_RATIO_1_2}); @@ -580,6 +580,45 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { snapEnd(); } + function test_swap_accruesProtocolFees(uint8 protocolFee1, uint8 protocolFee0) public { + vm.assume(protocolFee1 >= 4); + vm.assume(protocolFee0 >= 4); + + uint16 protocolFee = (uint16(protocolFee1) << 8) | (uint16(protocolFee0) & uint16(0xFF)); + + feeController.setSwapFeeForPool(key.toId(), protocolFee); + manager.setProtocolFee(key); + + (Pool.Slot0 memory slot0,,,) = manager.pools(key.toId()); + assertEq(slot0.protocolFee, protocolFee); + + // Add liquidity - Fees dont accrue for positive liquidity delta. + IPoolManager.ModifyPositionParams memory params = LIQ_PARAMS; + modifyPositionRouter.modifyPosition(key, params, ZERO_BYTES); + + assertEq(manager.protocolFeesAccrued(currency0), 0); + assertEq(manager.protocolFeesAccrued(currency1), 0); + + // Remove liquidity - Fees dont accrue for negative liquidity delta. + params.liquidityDelta = -LIQ_PARAMS.liquidityDelta; + modifyPositionRouter.modifyPosition(key, params, ZERO_BYTES); + + assertEq(manager.protocolFeesAccrued(currency0), 0); + assertEq(manager.protocolFeesAccrued(currency1), 0); + + // Now re-add the liquidity to test swap + params.liquidityDelta = LIQ_PARAMS.liquidityDelta; + modifyPositionRouter.modifyPosition(key, params, ZERO_BYTES); + + IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1); + swapRouter.swap(key, swapParams, PoolSwapTest.TestSettings(true, true, false), ZERO_BYTES); + + uint256 expectedTotalSwapFee = uint256(swapParams.amountSpecified) * key.fee / 1e6; + uint256 expectedProtocolFee = expectedTotalSwapFee / protocolFee1; + assertEq(manager.protocolFeesAccrued(currency0), 0); + assertEq(manager.protocolFeesAccrued(currency1), expectedProtocolFee); + } + function test_donate_failsIfNotInitialized() public { vm.expectRevert(abi.encodeWithSelector(Pool.PoolNotInitialized.selector)); donateRouter.donate(uninitializedKey, 100, 100, ZERO_BYTES); @@ -707,38 +746,36 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { takeRouter.take{value: 1}(nativeKey, 1, 1); // assertions inside takeRouter because it takes then settles } - function test_setProtocolFee_updatesProtocolFeeForInitializedPool() public { - uint24 protocolFee = 4; - + function test_setProtocolFee_updatesProtocolFeeForInitializedPool(uint16 protocolFee) public { (Pool.Slot0 memory slot0,,,) = manager.pools(key.toId()); - assertEq(slot0.protocolFees, 0); - feeController.setSwapFeeForPool(key.toId(), uint16(protocolFee)); - - vm.expectEmit(false, false, false, true); - emit ProtocolFeeUpdated(key.toId(), protocolFee << 12); - manager.setProtocolFees(key); - } - - function test_collectProtocolFees_initializesWithProtocolFeeIfCalled() public { - uint24 protocolFee = 260; // 0001 00 00 0100 - - // sets the upper 12 bits - feeController.setSwapFeeForPool(uninitializedKey.toId(), uint16(protocolFee)); - - initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES); - (Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId()); - assertEq(slot0.protocolFees, protocolFee << 12); + assertEq(slot0.protocolFee, 0); + feeController.setSwapFeeForPool(key.toId(), protocolFee); + + uint8 fee0 = uint8(protocolFee >> 8); + uint8 fee1 = uint8(protocolFee % 256); + + if ((0 < fee0 && fee0 < 4) || (0 < fee1 && fee1 < 4)) { + vm.expectRevert(IFees.FeeTooLarge.selector); + manager.setProtocolFee(key); + } else { + vm.expectEmit(false, false, false, true); + emit ProtocolFeeUpdated(key.toId(), protocolFee); + manager.setProtocolFee(key); + + (slot0,,,) = manager.pools(key.toId()); + assertEq(slot0.protocolFee, protocolFee); + } } function test_collectProtocolFees_ERC20_allowsOwnerToAccumulateFees_gas() public { - uint24 protocolFee = 260; // 0001 00 00 0100 + uint16 protocolFee = 1028; // 00000100 00000100 uint256 expectedFees = 7; feeController.setSwapFeeForPool(key.toId(), uint16(protocolFee)); - manager.setProtocolFees(key); + manager.setProtocolFee(key); (Pool.Slot0 memory slot0,,,) = manager.pools(key.toId()); - assertEq(slot0.protocolFees, protocolFee << 12); + assertEq(slot0.protocolFee, protocolFee); swapRouter.swap( key, @@ -978,14 +1015,14 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_collectProtocolFees_ERC20_returnsAllFeesIf0IsProvidedAsParameter() public { - uint24 protocolFee = 260; // 0001 00 00 0100 + uint16 protocolFee = 1028; // 00000100 00000100 uint256 expectedFees = 7; feeController.setSwapFeeForPool(key.toId(), uint16(protocolFee)); - manager.setProtocolFees(key); + manager.setProtocolFee(key); (Pool.Slot0 memory slot0,,,) = manager.pools(key.toId()); - assertEq(slot0.protocolFees, protocolFee << 12); + assertEq(slot0.protocolFee, protocolFee); swapRouter.swap( key, @@ -1003,16 +1040,16 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_collectProtocolFees_nativeToken_allowsOwnerToAccumulateFees_gas() public { - uint24 protocolFee = 260; // 0001 00 00 0100 + uint16 protocolFee = 1028; // 00000100 00000100 uint256 expectedFees = 7; Currency nativeCurrency = CurrencyLibrary.NATIVE; // set protocol fee before initializing the pool as it is fetched on initialization feeController.setSwapFeeForPool(nativeKey.toId(), uint16(protocolFee)); - manager.setProtocolFees(nativeKey); + manager.setProtocolFee(nativeKey); (Pool.Slot0 memory slot0,,,) = manager.pools(nativeKey.toId()); - assertEq(slot0.protocolFees, protocolFee << 12); + assertEq(slot0.protocolFee, protocolFee); swapRouter.swap{value: 10000}( nativeKey, @@ -1032,15 +1069,15 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_collectProtocolFees_nativeToken_returnsAllFeesIf0IsProvidedAsParameter() public { - uint24 protocolFee = 260; // 0001 00 00 0100 + uint16 protocolFee = 1028; // 00000100 00000100 uint256 expectedFees = 7; Currency nativeCurrency = CurrencyLibrary.NATIVE; feeController.setSwapFeeForPool(nativeKey.toId(), uint16(protocolFee)); - manager.setProtocolFees(nativeKey); + manager.setProtocolFee(nativeKey); (Pool.Slot0 memory slot0,,,) = manager.pools(nativeKey.toId()); - assertEq(slot0.protocolFees, protocolFee << 12); + assertEq(slot0.protocolFee, protocolFee); swapRouter.swap{value: 10000}( nativeKey, diff --git a/test/PoolManagerInitialize.t.sol b/test/PoolManagerInitialize.t.sol index 2f0006b41..a61b17f2a 100644 --- a/test/PoolManagerInitialize.t.sol +++ b/test/PoolManagerInitialize.t.sol @@ -81,7 +81,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { (Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId()); assertEq(slot0.sqrtPriceX96, sqrtPriceX96); - assertEq(slot0.protocolFees, 0); + assertEq(slot0.protocolFee, 0); } } @@ -104,7 +104,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { (Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId()); assertEq(slot0.sqrtPriceX96, sqrtPriceX96); - assertEq(slot0.protocolFees >> 12, 0); + assertEq(slot0.protocolFee, 0); assertEq(slot0.tick, TickMath.getTickAtSqrtRatio(sqrtPriceX96)); } @@ -203,20 +203,23 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot { initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); } - function test_initialize_fetchFeeWhenController(uint160 sqrtPriceX96) public { - // Assumptions tested in Pool.t.sol - vm.assume(sqrtPriceX96 >= TickMath.MIN_SQRT_RATIO); - vm.assume(sqrtPriceX96 < TickMath.MAX_SQRT_RATIO); - + function test_initialize_fetchFeeWhenController(uint16 protocolFee) public { manager.setProtocolFeeController(feeController); - uint16 poolProtocolFee = 4; - feeController.setSwapFeeForPool(uninitializedKey.toId(), poolProtocolFee); + feeController.setSwapFeeForPool(uninitializedKey.toId(), protocolFee); - initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES); + uint8 fee0 = uint8(protocolFee >> 8); + uint8 fee1 = uint8(protocolFee % 256); - (Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId()); - assertEq(slot0.sqrtPriceX96, sqrtPriceX96); - assertEq(slot0.protocolFees >> 12, poolProtocolFee); + if ((0 < fee0 && fee0 < 4) || (0 < fee1 && fee1 < 4)) { + vm.expectRevert(IFees.FeeTooLarge.selector); + initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES); + } else { + initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES); + + (Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId()); + assertEq(slot0.sqrtPriceX96, SQRT_RATIO_1_1); + assertEq(slot0.protocolFee, protocolFee); + } } function test_initialize_revertsWhenPoolAlreadyInitialized(uint160 sqrtPriceX96) public { From a87d3a068dc3adac882ac249b651bcd10e2ede5a Mon Sep 17 00:00:00 2001 From: hensha256 <henshawalice@gmail.com> Date: Wed, 6 Dec 2023 14:18:53 -0500 Subject: [PATCH 2/5] hook fee tests --- src/test/AccessLockHook.sol | 58 +++++++++++ test/AccessLock.t.sol | 201 ++++++++++++++++++++++++++---------- 2 files changed, 207 insertions(+), 52 deletions(-) diff --git a/src/test/AccessLockHook.sol b/src/test/AccessLockHook.sol index 33a6e4758..ca78eda76 100644 --- a/src/test/AccessLockHook.sol +++ b/src/test/AccessLockHook.sol @@ -13,6 +13,8 @@ import {ILockCallback} from "../interfaces/callback/ILockCallback.sol"; import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; import {Constants} from "../../test/utils/Constants.sol"; import {PoolIdLibrary} from "../types/PoolId.sol"; +import {BalanceDelta} from "../types/BalanceDelta.sol"; +import {console2} from "lib/forge-std/src/console2.sol"; contract AccessLockHook is Test, BaseTestHooks { using PoolIdLibrary for PoolKey; @@ -221,3 +223,59 @@ contract AccessLockHook3 is Test, ILockCallback, BaseTestHooks { return data; } } + +contract AccessLockFeeHook is Test, BaseTestHooks { + IPoolManager manager; + + uint256 constant WITHDRAWAL_FEE_BIPS = 40; // 40/10000 = 0.4% + uint256 constant SWAP_FEE_BIPS = 55; // 55/10000 = 0.55% + uint256 constant TOTAL_BIPS = 10000; + + constructor(IPoolManager _manager) { + manager = _manager; + } + + function afterModifyPosition( + address, /* sender **/ + PoolKey calldata key, + IPoolManager.ModifyPositionParams calldata, /* params **/ + BalanceDelta delta, + bytes calldata /* hookData **/ + ) external override returns (bytes4) { + int128 amount0 = delta.amount0(); + int128 amount1 = delta.amount1(); + + // positive delta => user owes money => liquidity addition + if (amount0 >= 0 && amount1 >= 0) return IHooks.afterModifyPosition.selector; + + // negative delta => user is owed money => liquidity withdrawal + uint256 amount0Fee = uint128(-amount0) * WITHDRAWAL_FEE_BIPS / TOTAL_BIPS; + uint256 amount1Fee = uint128(-amount1) * WITHDRAWAL_FEE_BIPS / TOTAL_BIPS; + + manager.take(key.currency0, address(this), amount0Fee); + manager.take(key.currency1, address(this), amount1Fee); + + return IHooks.afterModifyPosition.selector; + } + + function afterSwap( + address, /* sender **/ + PoolKey calldata key, + IPoolManager.SwapParams calldata params, + BalanceDelta delta, + bytes calldata /* hookData **/ + ) external override returns (bytes4) { + int128 amount0 = delta.amount0(); + int128 amount1 = delta.amount1(); + + // fee on output token - output delta will be negative + (Currency feeCurrency, uint256 outputAmount) = + (params.zeroForOne) ? (key.currency1, uint128(-amount1)) : (key.currency0, uint128(-amount0)); + + uint256 feeAmount = outputAmount * SWAP_FEE_BIPS / TOTAL_BIPS; + + manager.take(feeCurrency, address(this), feeAmount); + + return IHooks.afterSwap.selector; + } +} diff --git a/test/AccessLock.t.sol b/test/AccessLock.t.sol index 3373ed9fb..082860391 100644 --- a/test/AccessLock.t.sol +++ b/test/AccessLock.t.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; -import {AccessLockHook, AccessLockHook2, AccessLockHook3} from "../src/test/AccessLockHook.sol"; +import {AccessLockHook, AccessLockHook2, AccessLockHook3, AccessLockFeeHook} from "../src/test/AccessLockHook.sol"; import {IPoolManager} from "../src/interfaces/IPoolManager.sol"; import {PoolModifyPositionTest} from "../src/test/PoolModifyPositionTest.sol"; import {PoolSwapTest} from "../src/test/PoolSwapTest.sol"; @@ -14,21 +14,27 @@ import {Currency, CurrencyLibrary} from "../src/types/Currency.sol"; import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; import {Hooks} from "../src/libraries/Hooks.sol"; import {IHooks} from "../src/interfaces/IHooks.sol"; -import {BalanceDelta} from "../src/types/BalanceDelta.sol"; +import {BalanceDelta, BalanceDeltaLibrary} from "../src/types/BalanceDelta.sol"; import {Pool} from "../src/libraries/Pool.sol"; import {TickMath} from "../src/libraries/TickMath.sol"; import {PoolIdLibrary} from "../src/types/PoolId.sol"; +import {console2} from "lib/forge-std/src/console2.sol"; contract AccessLockTest is Test, Deployers { using Pool for Pool.State; using CurrencyLibrary for Currency; using PoolIdLibrary for PoolKey; + using BalanceDeltaLibrary for BalanceDelta; AccessLockHook accessLockHook; AccessLockHook noAccessLockHook; AccessLockHook2 accessLockHook2; AccessLockHook3 accessLockHook3; - AccessLockHook accessLockHook4; + AccessLockHook accessLockNoOpHook; + AccessLockFeeHook accessLockFeeHook; + + // global for stack too deep errors + BalanceDelta delta; function setUp() public { // Initialize managers and routers. @@ -45,9 +51,8 @@ contract AccessLockTest is Test, Deployers { deployCodeTo("AccessLockHook.sol:AccessLockHook", abi.encode(manager), accessLockAddress); accessLockHook = AccessLockHook(accessLockAddress); - (key,) = initPool( - currency0, currency1, IHooks(address(accessLockHook)), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES - ); + (key,) = + initPool(currency0, currency1, IHooks(accessLockAddress), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); // Create AccessLockHook2. address accessLockAddress2 = address(uint160(Hooks.ACCESS_LOCK_FLAG | Hooks.BEFORE_MODIFY_POSITION_FLAG)); @@ -68,14 +73,20 @@ contract AccessLockTest is Test, Deployers { noAccessLockHook = AccessLockHook(noAccessLockHookAddress); // Create AccessLockHook with NoOp. - address accessLockHook4Address = address( + address accessLockNoOpHookAddress = address( uint160( Hooks.NO_OP_FLAG | Hooks.ACCESS_LOCK_FLAG | Hooks.BEFORE_INITIALIZE_FLAG | Hooks.BEFORE_SWAP_FLAG | Hooks.BEFORE_MODIFY_POSITION_FLAG | Hooks.BEFORE_DONATE_FLAG ) ); - deployCodeTo("AccessLockHook.sol:AccessLockHook", abi.encode(manager), accessLockHook4Address); - accessLockHook4 = AccessLockHook(accessLockHook4Address); + deployCodeTo("AccessLockHook.sol:AccessLockHook", abi.encode(manager), accessLockNoOpHookAddress); + accessLockNoOpHook = AccessLockHook(accessLockNoOpHookAddress); + + // Create AccessLockFeeHook + address accessLockFeeHookAddress = + address(uint160(Hooks.ACCESS_LOCK_FLAG | Hooks.AFTER_SWAP_FLAG | Hooks.AFTER_MODIFY_POSITION_FLAG)); + deployCodeTo("AccessLockHook.sol:AccessLockFeeHook", abi.encode(manager), accessLockFeeHookAddress); + accessLockFeeHook = AccessLockFeeHook(accessLockFeeHookAddress); } function test_onlyByLocker_revertsForNoAccessLockPool() public { @@ -123,10 +134,8 @@ contract AccessLockTest is Test, Deployers { uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - BalanceDelta delta = modifyPositionRouter.modifyPosition( - key, - IPoolManager.ModifyPositionParams(0, 60, 1 * 10 ** 18), - abi.encode(amount, AccessLockHook.LockAction.Mint) + delta = modifyPositionRouter.modifyPosition( + key, IPoolManager.ModifyPositionParams(0, 60, 1e18), abi.encode(amount, AccessLockHook.LockAction.Mint) ); uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); @@ -153,10 +162,8 @@ contract AccessLockTest is Test, Deployers { uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); // Hook only takes currency 1 rn. - BalanceDelta delta = modifyPositionRouter.modifyPosition( - key, - IPoolManager.ModifyPositionParams(-60, 60, 1 * 10 ** 18), - abi.encode(amount, AccessLockHook.LockAction.Take) + delta = modifyPositionRouter.modifyPosition( + key, IPoolManager.ModifyPositionParams(-60, 60, 1e18), abi.encode(amount, AccessLockHook.LockAction.Take) ); uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); @@ -201,7 +208,7 @@ contract AccessLockTest is Test, Deployers { modifyPositionRouter.modifyPosition( key, - IPoolManager.ModifyPositionParams(-120, 120, 1 * 10 ** 18), + IPoolManager.ModifyPositionParams(-120, 120, 1e18), abi.encode(amount, AccessLockHook.LockAction.ModifyPosition) ); uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); @@ -226,7 +233,7 @@ contract AccessLockTest is Test, Deployers { modifyPositionRouter.modifyPosition( key, - IPoolManager.ModifyPositionParams(-120, 120, 1 * 10 ** 18), + IPoolManager.ModifyPositionParams(-120, 120, 1e18), abi.encode(amount, AccessLockHook.LockAction.Donate) ); uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); @@ -246,7 +253,7 @@ contract AccessLockTest is Test, Deployers { ZERO_BYTES ); - BalanceDelta delta = swapRouter.swap( + delta = swapRouter.swap( key, IPoolManager.SwapParams(true, 10000, TickMath.MIN_SQRT_RATIO + 1), PoolSwapTest.TestSettings({withdrawTokens: false, settleUsingTransfer: true, currencyAlreadySent: false}), @@ -285,7 +292,7 @@ contract AccessLockTest is Test, Deployers { // Assertions in the hook. Takes and then settles within the hook. modifyPositionRouter.modifyPosition( key, - IPoolManager.ModifyPositionParams(-120, 120, 1 * 10 ** 18), + IPoolManager.ModifyPositionParams(-120, 120, 1e18), abi.encode(amount, AccessLockHook.LockAction.Settle) ); } @@ -293,9 +300,7 @@ contract AccessLockTest is Test, Deployers { function test_beforeModifyPosition_initialize_succeedsWithAccessLock() public { // The hook intitializes a new pool with the new key at Constants.SQRT_RATIO_1_2; modifyPositionRouter.modifyPosition( - key, - IPoolManager.ModifyPositionParams(-120, 120, 1 * 10 ** 18), - abi.encode(0, AccessLockHook.LockAction.Initialize) + key, IPoolManager.ModifyPositionParams(-120, 120, 1e18), abi.encode(0, AccessLockHook.LockAction.Initialize) ); PoolKey memory newKey = PoolKey({ @@ -330,7 +335,7 @@ contract AccessLockTest is Test, Deployers { uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); // Small amount to swap (like NoOp). This way we can expect balances to just be from the hook applied delta. - BalanceDelta delta = swapRouter.swap( + delta = swapRouter.swap( key, IPoolManager.SwapParams(true, 1, TickMath.MIN_SQRT_RATIO + 1), PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), @@ -363,7 +368,7 @@ contract AccessLockTest is Test, Deployers { // Hook only takes currency 1 rn. // Use small amount to NoOp. - BalanceDelta delta = swapRouter.swap( + delta = swapRouter.swap( key, IPoolManager.SwapParams(true, 1, TickMath.MIN_SQRT_RATIO + 1), PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), @@ -483,8 +488,7 @@ contract AccessLockTest is Test, Deployers { uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - BalanceDelta delta = - donateRouter.donate(key, 1 * 10 ** 18, 1 * 10 ** 18, abi.encode(amount, AccessLockHook.LockAction.Mint)); + delta = donateRouter.donate(key, 1e18, 1e18, abi.encode(amount, AccessLockHook.LockAction.Mint)); uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); @@ -511,8 +515,7 @@ contract AccessLockTest is Test, Deployers { uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); // Hook only takes currency 1 rn. - BalanceDelta delta = - donateRouter.donate(key, 1 * 10 ** 18, 1 * 10 ** 18, abi.encode(amount, AccessLockHook.LockAction.Take)); + delta = donateRouter.donate(key, 1e18, 1e18, abi.encode(amount, AccessLockHook.LockAction.Take)); // Take applies a positive delta in currency1. // Donate applies a positive delta in currency0 and currency1. uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); @@ -563,9 +566,7 @@ contract AccessLockTest is Test, Deployers { uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - donateRouter.donate( - key, 1 * 10 ** 18, 1 * 10 ** 18, abi.encode(amount, AccessLockHook.LockAction.ModifyPosition) - ); + donateRouter.donate(key, 1e18, 1e18, abi.encode(amount, AccessLockHook.LockAction.ModifyPosition)); uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); @@ -589,7 +590,7 @@ contract AccessLockTest is Test, Deployers { uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); // Make the swap amount small (like a NoOp). - donateRouter.donate(key, 1 * 10 ** 18, 1 * 10 ** 18, abi.encode(amount, AccessLockHook.LockAction.Donate)); + donateRouter.donate(key, 1e18, 1e18, abi.encode(amount, AccessLockHook.LockAction.Donate)); uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); uint256 balanceOfAfter1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); @@ -612,12 +613,12 @@ contract AccessLockTest is Test, Deployers { currency1: currency1, fee: Constants.FEE_MEDIUM, tickSpacing: 60, - hooks: IHooks(address(accessLockHook4)) + hooks: IHooks(address(accessLockNoOpHook)) }); initializeRouter.initialize(key1, SQRT_RATIO_1_1, abi.encode(amount, AccessLockHook.LockAction.Mint)); - assertEq(manager.balanceOf(address(accessLockHook4), currency1), amount); + assertEq(manager.balanceOf(address(accessLockNoOpHook), currency1), amount); } function test_beforeInitialize_take_succeedsWithAccessLock(uint128 amount) public { @@ -626,7 +627,7 @@ contract AccessLockTest is Test, Deployers { currency1: currency1, fee: Constants.FEE_MEDIUM, tickSpacing: 60, - hooks: IHooks(address(accessLockHook4)) + hooks: IHooks(address(accessLockNoOpHook)) }); // Add liquidity to a different pool there is something to take. @@ -641,7 +642,7 @@ contract AccessLockTest is Test, Deployers { initializeRouter.initialize(key1, SQRT_RATIO_1_1, abi.encode(amount, AccessLockHook.LockAction.Take)); - assertEq(MockERC20(Currency.unwrap(currency1)).balanceOf(address(accessLockHook4)), amount); + assertEq(MockERC20(Currency.unwrap(currency1)).balanceOf(address(accessLockNoOpHook)), amount); } function test_beforeInitialize_swap_revertsOnPoolNotInitialized(uint128 amount) public { @@ -652,7 +653,7 @@ contract AccessLockTest is Test, Deployers { currency1: currency1, fee: Constants.FEE_MEDIUM, tickSpacing: 60, - hooks: IHooks(address(accessLockHook4)) + hooks: IHooks(address(accessLockNoOpHook)) }); vm.expectRevert(IPoolManager.PoolNotInitialized.selector); @@ -667,7 +668,7 @@ contract AccessLockTest is Test, Deployers { currency1: currency1, fee: Constants.FEE_MEDIUM, tickSpacing: 60, - hooks: IHooks(address(accessLockHook4)) + hooks: IHooks(address(accessLockNoOpHook)) }); vm.expectRevert(IPoolManager.PoolNotInitialized.selector); @@ -682,13 +683,101 @@ contract AccessLockTest is Test, Deployers { currency1: currency1, fee: Constants.FEE_MEDIUM, tickSpacing: 60, - hooks: IHooks(address(accessLockHook4)) + hooks: IHooks(address(accessLockNoOpHook)) }); vm.expectRevert(IPoolManager.PoolNotInitialized.selector); initializeRouter.initialize(key1, SQRT_RATIO_1_1, abi.encode(amount, AccessLockHook.LockAction.Donate)); } + /** + * + * HOOK FEE TESTS + * + */ + + function test_hookFees_takesFeeOnWithdrawal() public { + (key,) = initPool( + currency0, currency1, IHooks(address(accessLockFeeHook)), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES + ); + + (uint256 userBalanceBefore0, uint256 poolBalanceBefore0, uint256 reservesBefore0) = _fetchBalances(currency0); + (uint256 userBalanceBefore1, uint256 poolBalanceBefore1, uint256 reservesBefore1) = _fetchBalances(currency1); + + // add liquidity + delta = modifyPositionRouter.modifyPosition(key, LIQ_PARAMS, ZERO_BYTES); + + (uint256 userBalanceAfter0, uint256 poolBalanceAfter0, uint256 reservesAfter0) = _fetchBalances(currency0); + (uint256 userBalanceAfter1, uint256 poolBalanceAfter1, uint256 reservesAfter1) = _fetchBalances(currency1); + + assert(delta.amount0() > 0 && delta.amount1() > 0); + assertEq(userBalanceBefore0 - uint128(delta.amount0()), userBalanceAfter0, "addLiq user balance currency0"); + assertEq(userBalanceBefore1 - uint128(delta.amount1()), userBalanceAfter1, "addLiq user balance currency1"); + assertEq(poolBalanceBefore0 + uint128(delta.amount0()), poolBalanceAfter0, "addLiq pool balance currency0"); + assertEq(poolBalanceBefore1 + uint128(delta.amount1()), poolBalanceAfter1, "addLiq pool balance currency1"); + assertEq(reservesBefore0 + uint128(delta.amount0()), reservesAfter0, "addLiq reserves currency0"); + assertEq(reservesBefore1 + uint128(delta.amount1()), reservesAfter1, "addLiq reserves currency1"); + + (userBalanceBefore0, poolBalanceBefore0, reservesBefore0) = + (userBalanceAfter0, poolBalanceAfter0, reservesAfter0); + (userBalanceBefore1, poolBalanceBefore1, reservesBefore1) = + (userBalanceAfter1, poolBalanceAfter1, reservesAfter1); + + // remove liquidity, a 40 bip fee should be taken + LIQ_PARAMS.liquidityDelta *= -1; + delta = modifyPositionRouter.modifyPosition(key, LIQ_PARAMS, ZERO_BYTES); + + (userBalanceAfter0, poolBalanceAfter0, reservesAfter0) = _fetchBalances(currency0); + (userBalanceAfter1, poolBalanceAfter1, reservesAfter1) = _fetchBalances(currency1); + + assert(delta.amount0() < 0 && delta.amount1() < 0); + + uint256 totalWithdraw0 = uint128(-delta.amount0()) - (uint128(-delta.amount0()) * 40 / 10000); + uint256 totalWithdraw1 = uint128(-delta.amount1()) - (uint128(-delta.amount1()) * 40 / 10000); + + assertEq(userBalanceBefore0 + totalWithdraw0, userBalanceAfter0, "removeLiq user balance currency0"); + assertEq(userBalanceBefore1 + totalWithdraw1, userBalanceAfter1, "removeLiq user balance currency1"); + assertEq(poolBalanceBefore0 - uint128(-delta.amount0()), poolBalanceAfter0, "removeLiq pool balance currency0"); + assertEq(poolBalanceBefore1 - uint128(-delta.amount1()), poolBalanceAfter1, "removeLiq pool balance currency1"); + assertEq(reservesBefore0 - uint128(-delta.amount0()), reservesAfter0, "removeLiq reserves currency0"); + assertEq(reservesBefore1 - uint128(-delta.amount1()), reservesAfter1, "removeLiq reserves currency1"); + } + + function test_hookFees_takesFeeOnInputOfSwap() public { + (key,) = initPool( + currency0, currency1, IHooks(address(accessLockFeeHook)), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES + ); + + // add liquidity + delta = modifyPositionRouter.modifyPosition(key, LIQ_PARAMS, ZERO_BYTES); + + // now swap, with a hook fee of 55 bips + (uint256 userBalanceBefore0, uint256 poolBalanceBefore0, uint256 reservesBefore0) = _fetchBalances(currency0); + (uint256 userBalanceBefore1, uint256 poolBalanceBefore1, uint256 reservesBefore1) = _fetchBalances(currency1); + + delta = swapRouter.swap( + key, + IPoolManager.SwapParams({zeroForOne: true, amountSpecified: 100000, sqrtPriceLimitX96: SQRT_RATIO_1_2}), + PoolSwapTest.TestSettings({withdrawTokens: true, settleUsingTransfer: true, currencyAlreadySent: false}), + ZERO_BYTES + ); + + assert(delta.amount0() > 0 && delta.amount1() < 0); + + uint256 amountIn0 = uint128(delta.amount0()); + uint256 userAmountOut1 = uint128(-delta.amount1()) - (uint128(-delta.amount1()) * 55 / 10000); + + (uint256 userBalanceAfter0, uint256 poolBalanceAfter0, uint256 reservesAfter0) = _fetchBalances(currency0); + (uint256 userBalanceAfter1, uint256 poolBalanceAfter1, uint256 reservesAfter1) = _fetchBalances(currency1); + + assertEq(userBalanceBefore0 - amountIn0, userBalanceAfter0, "swap user balance currency0"); + assertEq(userBalanceBefore1 + userAmountOut1, userBalanceAfter1, "swap user balance currency1"); + assertEq(poolBalanceBefore0 + amountIn0, poolBalanceAfter0, "swap pool balance currency0"); + assertEq(poolBalanceBefore1 - uint128(-delta.amount1()), poolBalanceAfter1, "swap pool balance currency1"); + assertEq(reservesBefore0 + amountIn0, reservesAfter0, "swap reserves currency0"); + assertEq(reservesBefore1 - uint128(-delta.amount1()), reservesAfter1, "swap reserves currency1"); + } + /** * * EDGE CASE TESTS @@ -701,10 +790,8 @@ contract AccessLockTest is Test, Deployers { uint256 balanceOfBefore1 = MockERC20(Currency.unwrap(currency1)).balanceOf(address(this)); uint256 balanceOfBefore0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); - BalanceDelta delta = modifyPositionRouter.modifyPosition( - key, - IPoolManager.ModifyPositionParams(0, 60, 1 * 10 ** 18), - abi.encode(amount, AccessLockHook.LockAction.Mint) + delta = modifyPositionRouter.modifyPosition( + key, IPoolManager.ModifyPositionParams(0, 60, 1e18), abi.encode(amount, AccessLockHook.LockAction.Mint) ); uint256 balanceOfAfter0 = MockERC20(Currency.unwrap(currency0)).balanceOf(address(this)); @@ -729,7 +816,7 @@ contract AccessLockTest is Test, Deployers { ) ); delta = modifyPositionRouter.modifyPosition( - keyAccessLockHook2, IPoolManager.ModifyPositionParams(0, 60, 1 * 10 ** 18), abi.encode(true, key) + keyAccessLockHook2, IPoolManager.ModifyPositionParams(0, 60, 1e18), abi.encode(true, key) ); } @@ -741,18 +828,18 @@ contract AccessLockTest is Test, Deployers { initPool(currency0, currency1, IHooks(accessLockHook2), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); modifyPositionRouter.modifyPosition( - keyAccessLockHook2, IPoolManager.ModifyPositionParams(0, 60, 1 * 10 ** 18), abi.encode(false, keyWithNoHook) + keyAccessLockHook2, IPoolManager.ModifyPositionParams(0, 60, 1e18), abi.encode(false, keyWithNoHook) ); assertEq(manager.balanceOf(address(accessLockHook2), currency1), 10); } function test_onlyByLocker_revertsWhenThereIsNoOutsideLock() public { - modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 1 * 10 ** 18), ZERO_BYTES); + modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 1e18), ZERO_BYTES); assertEq(address(manager.getCurrentHook()), address(0)); vm.expectRevert(abi.encodeWithSelector(IPoolManager.LockedBy.selector, address(0), address(0))); vm.prank(address(key.hooks)); - manager.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 1 * 10 ** 18), ZERO_BYTES); + manager.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 1e18), ZERO_BYTES); } function test_getCurrentHook_isClearedAfterNestedLock() public { @@ -773,13 +860,13 @@ contract AccessLockTest is Test, Deployers { // Asserts are in the AccessLockHook3. modifyPositionRouter.modifyPosition( - keyAccessLockHook3, IPoolManager.ModifyPositionParams(0, 60, 1 * 10 ** 18), ZERO_BYTES + keyAccessLockHook3, IPoolManager.ModifyPositionParams(0, 60, 1e18), ZERO_BYTES ); } function test_getCurrentHook_isClearedAfterNoOpOnAllHooks() public { (PoolKey memory noOpKey,) = - initPool(currency0, currency1, IHooks(accessLockHook4), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); + initPool(currency0, currency1, IHooks(accessLockNoOpHook), Constants.FEE_MEDIUM, SQRT_RATIO_1_1, ZERO_BYTES); // Assertions for current hook address in AccessLockHook and respective routers. // beforeModifyPosition noOp @@ -790,7 +877,7 @@ contract AccessLockTest is Test, Deployers { ); // beforeDonate noOp - donateRouter.donate(noOpKey, 1 * 10 ** 18, 1 * 10 ** 18, abi.encode(0, AccessLockHook.LockAction.NoOp)); + donateRouter.donate(noOpKey, 1e18, 1e18, abi.encode(0, AccessLockHook.LockAction.NoOp)); // beforeSwap noOp swapRouter.swap( @@ -800,4 +887,14 @@ contract AccessLockTest is Test, Deployers { abi.encode(0, AccessLockHook.LockAction.NoOp) ); } + + function _fetchBalances(Currency currency) + internal + view + returns (uint256 userBalance, uint256 poolBalance, uint256 reserves) + { + userBalance = currency.balanceOf(address(this)); + poolBalance = currency.balanceOf(address(manager)); + reserves = manager.reservesOf(currency); + } } From f76c69d68ae0fa4554bb02b94f1138c521510f8d Mon Sep 17 00:00:00 2001 From: Alice <34962750+hensha256@users.noreply.github.com> Date: Thu, 7 Dec 2023 14:16:31 -0500 Subject: [PATCH 3/5] Update AccessLockHook.sol --- src/test/AccessLockHook.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/AccessLockHook.sol b/src/test/AccessLockHook.sol index ca78eda76..8293af2fb 100644 --- a/src/test/AccessLockHook.sol +++ b/src/test/AccessLockHook.sol @@ -14,7 +14,6 @@ import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; import {Constants} from "../../test/utils/Constants.sol"; import {PoolIdLibrary} from "../types/PoolId.sol"; import {BalanceDelta} from "../types/BalanceDelta.sol"; -import {console2} from "lib/forge-std/src/console2.sol"; contract AccessLockHook is Test, BaseTestHooks { using PoolIdLibrary for PoolKey; From 703136d06744d2e8365b6949cfcdfb61412dc648 Mon Sep 17 00:00:00 2001 From: hensha256 <henshawalice@gmail.com> Date: Fri, 8 Dec 2023 15:35:41 -0300 Subject: [PATCH 4/5] assume -> bound --- test/PoolManager.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol index 43cee7e65..ba24f930a 100644 --- a/test/PoolManager.t.sol +++ b/test/PoolManager.t.sol @@ -577,8 +577,8 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { } function test_swap_accruesProtocolFees(uint8 protocolFee1, uint8 protocolFee0) public { - vm.assume(protocolFee1 >= 4); - vm.assume(protocolFee0 >= 4); + protocolFee0 = uint8(bound(protocolFee0, 4, type(uint8).max)); + protocolFee1 = uint8(bound(protocolFee1, 4, type(uint8).max)); uint16 protocolFee = (uint16(protocolFee1) << 8) | (uint16(protocolFee0) & uint16(0xFF)); From adc68d35438bdfba2e65bd9bdd175b6187ea5dbb Mon Sep 17 00:00:00 2001 From: hensha256 <henshawalice@gmail.com> Date: Fri, 8 Dec 2023 16:15:31 -0300 Subject: [PATCH 5/5] more comments on fee grnaularity --- src/libraries/Pool.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index 5eedaf0d6..89b437373 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -66,6 +66,8 @@ library Pool { int24 tick; // protocol swap fee represented as integer denominator (1/x), taken as a % of the LP swap fee // upper 8 bits are for 1->0, and the lower 8 are for 0->1 + // the minimum permitted denominator is 4 - meaning the maximum protocol fee is 25% + // granularity is increments of 0.38% (100/type(uint8).max) uint16 protocolFee; // used for the swap fee, either static at initialize or dynamic via hook uint24 swapFee;