Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove hook fees and protocol fee on withdrawal #432

Merged
merged 7 commits into from
Dec 8, 2023
Merged

Remove hook fees and protocol fee on withdrawal #432

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8f1353c
Remove hook fees and protocol fee on withdrawal
hensha256 Dec 5, 2023
a87d3a0
hook fee tests
hensha256 Dec 6, 2023
f76c69d
Update AccessLockHook.sol
hensha256 Dec 7, 2023
07a172c
Merge branch 'main' into remove-hook-and-withdrawal-fee
hensha256 Dec 7, 2023
6093b60
Merge branch 'main' into remove-hook-and-withdrawal-fee
hensha256 Dec 8, 2023
703136d
assume -> bound
hensha256 Dec 8, 2023
adc68d3
more comments on fee grnaularity
hensha256 Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .forge-snapshots/before swap hook, already cached dynamic fee.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
193153
192720
2 changes: 1 addition & 1 deletion .forge-snapshots/cached dynamic fee, no hooks.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
148516
148082
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 1 token.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139130
139082
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 2 tokens.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
186552
186504
2 changes: 1 addition & 1 deletion .forge-snapshots/gas overhead of no-op lock.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
15268
15224
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
78018
74906
2 changes: 1 addition & 1 deletion .forge-snapshots/mint with empty hook.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
319051
318560
2 changes: 1 addition & 1 deletion .forge-snapshots/mint with native token.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
201832
201364
2 changes: 1 addition & 1 deletion .forge-snapshots/mint.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
201752
201284
2 changes: 1 addition & 1 deletion .forge-snapshots/mintWithEmptyHookEOAInitiated.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
250947
250456
2 changes: 1 addition & 1 deletion .forge-snapshots/modify position with noop.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
56515
56469
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
25378
23124
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap with native.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
197401
196967
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
205969
205535
2 changes: 1 addition & 1 deletion .forge-snapshots/simpleSwapEOAInitiated.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
177213
176801
2 changes: 1 addition & 1 deletion .forge-snapshots/simpleSwapNativeEOAInitiated.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
173875
173463
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity with native token.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
127994
127595
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115485
115086
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn claim for input.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
134557
134282
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as claim.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
218140
217728
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with dynamic fee.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
192395
191962
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115463
115064
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with noop.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
49734
49686
2 changes: 1 addition & 1 deletion .forge-snapshots/update dynamic fee in before swap.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
198977
198542
44 changes: 5 additions & 39 deletions src/Fees.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand All @@ -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);
}
}

Expand All @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this? If we want to increase protocol fee granularity I think thats a separate discussion. I think this means the fee can be a lot smaller and I thought we intentionally had it bound before (both from a min and a max).

In any case it's worth a comment on what the min fee value is now that there are 8 bits that can be used instead of 6.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont personally remember us giving it a lower bound for any reason other than running out of space in slot0 - but happy to change it back!

My reasoning was just that increasing bits only really increases flexibility of what fee you want to set - and our bound on >=4 will still hold.

4 bits gives a granularity of 6.67%.
6 bits gives a granularity of 1.58%
8 bits gives a granularity of 0.38%

So with 6 bits its impossible to set a fee close to 1% or close to 2% - it has to be 0%, 1.58%, or 3.16%. 8 bits gives more flexibility for that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that's fine, can you add a comment? I just wasn't expecting that to change, not that its a bad change but just deserves some reasoning :)

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)
Expand All @@ -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);
}
}
44 changes: 8 additions & 36 deletions src/PoolManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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()) {
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions src/interfaces/IFees.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
13 changes: 0 additions & 13 deletions src/interfaces/IHookFeeManager.sol
View file
Open in desktop

This file was deleted.

18 changes: 5 additions & 13 deletions src/interfaces/IPoolManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IProtocolFeeController.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading