Skip to content

Commit

Permalink
remove IProtocolFeeController (#907)
Browse files Browse the repository at this point in the history
* remove IProtocolFeeController

* typo

* remove cast

* remove more casts

---------

Co-authored-by: Alice <[email protected]>
  • Loading branch information
snreynolds and hensha256 authored Oct 21, 2024
1 parent f25e9fe commit d9403bd
Show file tree
Hide file tree
Showing 16 changed files with 47 additions and 87 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23682
23659
Original file line number Diff line number Diff line change
@@ -1 +1 @@
206251
206263
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139284
139272
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with return dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145577
145589
2 changes: 1 addition & 1 deletion .forge-snapshots/update dynamic fee in before swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147869
147857
11 changes: 5 additions & 6 deletions src/ProtocolFees.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity ^0.8.0;

import {Currency} from "./types/Currency.sol";
import {IProtocolFeeController} from "./interfaces/IProtocolFeeController.sol";
import {IProtocolFees} from "./interfaces/IProtocolFees.sol";
import {PoolKey} from "./types/PoolKey.sol";
import {ProtocolFeeLibrary} from "./libraries/ProtocolFeeLibrary.sol";
Expand All @@ -21,19 +20,19 @@ abstract contract ProtocolFees is IProtocolFees, Owned {
mapping(Currency currency => uint256 amount) public protocolFeesAccrued;

/// @inheritdoc IProtocolFees
IProtocolFeeController public protocolFeeController;
address public protocolFeeController;

constructor() Owned(msg.sender) {}

/// @inheritdoc IProtocolFees
function setProtocolFeeController(IProtocolFeeController controller) external onlyOwner {
function setProtocolFeeController(address controller) external onlyOwner {
protocolFeeController = controller;
emit ProtocolFeeControllerUpdated(address(controller));
emit ProtocolFeeControllerUpdated(controller);
}

/// @inheritdoc IProtocolFees
function setProtocolFee(PoolKey memory key, uint24 newProtocolFee) external {
if (msg.sender != address(protocolFeeController)) InvalidCaller.selector.revertWith();
if (msg.sender != protocolFeeController) InvalidCaller.selector.revertWith();
if (!newProtocolFee.isValidProtocolFee()) ProtocolFeeTooLarge.selector.revertWith(newProtocolFee);
PoolId id = key.toId();
_getPool(id).setProtocolFee(newProtocolFee);
Expand All @@ -45,7 +44,7 @@ abstract contract ProtocolFees is IProtocolFees, Owned {
external
returns (uint256 amountCollected)
{
if (msg.sender != address(protocolFeeController)) InvalidCaller.selector.revertWith();
if (msg.sender != protocolFeeController) InvalidCaller.selector.revertWith();
if (_isUnlocked()) ContractUnlocked.selector.revertWith();

amountCollected = (amount == 0) ? protocolFeesAccrued[currency] : amount;
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IPoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ interface IPoolManager is IProtocolFees, IERC6909Claims, IExtsload, IExttload {
/// @param tickSpacing The minimum number of ticks between initialized ticks
/// @param hooks The hooks contract address for the pool, or address(0) if none
/// @param sqrtPriceX96 The price of the pool on initialization
/// @param tick The initial tick of the pool corresponding to the intialized price
/// @param tick The initial tick of the pool corresponding to the initialized price
event Initialize(
PoolId indexed id,
Currency indexed currency0,
Expand Down
15 changes: 0 additions & 15 deletions src/interfaces/IProtocolFeeController.sol

This file was deleted.

7 changes: 3 additions & 4 deletions src/interfaces/IProtocolFees.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pragma solidity ^0.8.0;

import {Currency} from "../types/Currency.sol";
import {IProtocolFeeController} from "../interfaces/IProtocolFeeController.sol";
import {PoolId} from "../types/PoolId.sol";
import {PoolKey} from "../types/PoolKey.sol";

Expand Down Expand Up @@ -35,7 +34,7 @@ interface IProtocolFees {

/// @notice Sets the protocol fee controller
/// @param controller The new protocol fee controller
function setProtocolFeeController(IProtocolFeeController controller) external;
function setProtocolFeeController(address controller) external;

/// @notice Collects the protocol fees for a given recipient and currency, returning the amount collected
/// @dev This will revert if the contract is unlocked
Expand All @@ -48,6 +47,6 @@ interface IProtocolFees {
returns (uint256 amountCollected);

/// @notice Returns the current protocol fee controller address
/// @return IProtocolFeeController The currency protocol fee controller
function protocolFeeController() external view returns (IProtocolFeeController);
/// @return address The current protocol fee controller address
function protocolFeeController() external view returns (address);
}
2 changes: 1 addition & 1 deletion src/test/Fuzzers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ contract Fuzzers is StdUtils {
result.liquidityDelta = boundLiquidityDelta(key, params.liquidityDelta, liquidityDeltaFromAmounts);
}

// Creates liquidity parameters with a stricter bound. Should be used if multiple positions being intitialized on the pool, with potential for tick overlap.
// Creates liquidity parameters with a stricter bound. Should be used if multiple positions being initialized on the pool, with potential for tick overlap.
function createFuzzyLiquidityParamsWithTightBound(
PoolKey memory key,
IPoolManager.ModifyLiquidityParams memory params,
Expand Down
19 changes: 0 additions & 19 deletions src/test/ProtocolFeeControllerTest.sol

This file was deleted.

8 changes: 4 additions & 4 deletions test/DynamicFees.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {

dynamicFeesHooks.setFee(999999);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(key, 1000);

IPoolManager.SwapParams memory params =
Expand All @@ -239,7 +239,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {

dynamicFeesHooks.setFee(1000000);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(key, 1000);

IPoolManager.SwapParams memory params =
Expand All @@ -261,7 +261,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {

dynamicFeesHooks.setFee(123);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(key, 1000);

PoolSwapTest.TestSettings memory testSettings =
Expand All @@ -288,7 +288,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
uint24 protocolFee = (uint24(protocolFee1) << 12) | uint24(protocolFee0);
dynamicFeesHooks.setFee(lpFee);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(key, protocolFee);

IPoolManager.SwapParams memory params = IPoolManager.SwapParams({
Expand Down
23 changes: 11 additions & 12 deletions test/PoolManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {IHooks} from "../src/interfaces/IHooks.sol";
import {Hooks} from "../src/libraries/Hooks.sol";
import {IPoolManager} from "../src/interfaces/IPoolManager.sol";
import {IProtocolFees} from "../src/interfaces/IProtocolFees.sol";
import {IProtocolFeeController} from "../src/interfaces/IProtocolFeeController.sol";
import {PoolManager} from "../src/PoolManager.sol";
import {TickMath} from "../src/libraries/TickMath.sol";
import {Pool} from "../src/libraries/Pool.sol";
Expand Down Expand Up @@ -775,7 +774,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
(,, uint24 slot0ProtocolFee,) = manager.getSlot0(key.toId());
assertEq(slot0ProtocolFee, 0);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(key, protocolFee);

(,, slot0ProtocolFee,) = manager.getSlot0(key.toId());
Expand Down Expand Up @@ -990,7 +989,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
(,, uint24 slot0ProtocolFee,) = manager.getSlot0(key.toId());
assertEq(slot0ProtocolFee, 0);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(key, MAX_PROTOCOL_FEE_BOTH_TOKENS);

(,, slot0ProtocolFee,) = manager.getSlot0(key.toId());
Expand All @@ -1006,7 +1005,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
assertEq(manager.protocolFeesAccrued(currency0), expectedFees);
assertEq(manager.protocolFeesAccrued(currency1), 0);
assertEq(currency0.balanceOf(address(1)), 0);
vm.prank(address(feeController));
vm.prank(feeController);
manager.collectProtocolFees(address(1), currency0, expectedFees);
snapLastCall("erc20 collect protocol fees");
assertEq(currency0.balanceOf(address(1)), expectedFees);
Expand All @@ -1019,7 +1018,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
(,, uint24 slot0ProtocolFee,) = manager.getSlot0(key.toId());
assertEq(slot0ProtocolFee, 0);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(key, MAX_PROTOCOL_FEE_BOTH_TOKENS);

(,, slot0ProtocolFee,) = manager.getSlot0(key.toId());
Expand All @@ -1035,7 +1034,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
assertEq(manager.protocolFeesAccrued(currency0), expectedFees);
assertEq(manager.protocolFeesAccrued(currency1), 0);
assertEq(currency0.balanceOf(address(1)), 0);
vm.prank(address(feeController));
vm.prank(feeController);
manager.collectProtocolFees(address(1), currency0, expectedFees);
assertEq(currency0.balanceOf(address(1)), expectedFees);
assertEq(manager.protocolFeesAccrued(currency0), 0);
Expand All @@ -1047,7 +1046,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
(,, uint24 slot0ProtocolFee,) = manager.getSlot0(key.toId());
assertEq(slot0ProtocolFee, 0);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(key, MAX_PROTOCOL_FEE_BOTH_TOKENS);

(,, slot0ProtocolFee,) = manager.getSlot0(key.toId());
Expand All @@ -1063,7 +1062,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
assertEq(manager.protocolFeesAccrued(currency0), 0);
assertEq(manager.protocolFeesAccrued(currency1), expectedFees);
assertEq(currency1.balanceOf(address(1)), 0);
vm.prank(address(feeController));
vm.prank(feeController);
manager.collectProtocolFees(address(1), currency1, 0);
assertEq(currency1.balanceOf(address(1)), expectedFees);
assertEq(manager.protocolFeesAccrued(currency1), 0);
Expand All @@ -1076,7 +1075,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
(,, uint24 slot0ProtocolFee,) = manager.getSlot0(nativeKey.toId());
assertEq(slot0ProtocolFee, 0);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(nativeKey, MAX_PROTOCOL_FEE_BOTH_TOKENS);

(,, slot0ProtocolFee,) = manager.getSlot0(nativeKey.toId());
Expand All @@ -1092,7 +1091,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
assertEq(manager.protocolFeesAccrued(nativeCurrency), expectedFees);
assertEq(manager.protocolFeesAccrued(currency1), 0);
assertEq(nativeCurrency.balanceOf(address(1)), 0);
vm.prank(address(feeController));
vm.prank(feeController);
manager.collectProtocolFees(address(1), nativeCurrency, expectedFees);
snapLastCall("native collect protocol fees");
assertEq(nativeCurrency.balanceOf(address(1)), expectedFees);
Expand All @@ -1106,7 +1105,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
(,, uint24 slot0ProtocolFee,) = manager.getSlot0(nativeKey.toId());
assertEq(slot0ProtocolFee, 0);

vm.prank(address(feeController));
vm.prank(feeController);
manager.setProtocolFee(nativeKey, MAX_PROTOCOL_FEE_BOTH_TOKENS);

(,, slot0ProtocolFee,) = manager.getSlot0(nativeKey.toId());
Expand All @@ -1122,7 +1121,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
assertEq(manager.protocolFeesAccrued(nativeCurrency), expectedFees);
assertEq(manager.protocolFeesAccrued(currency1), 0);
assertEq(nativeCurrency.balanceOf(address(1)), 0);
vm.prank(address(feeController));
vm.prank(feeController);
manager.collectProtocolFees(address(1), nativeCurrency, 0);
assertEq(nativeCurrency.balanceOf(address(1)), expectedFees);
assertEq(manager.protocolFeesAccrued(nativeCurrency), 0);
Expand Down
2 changes: 0 additions & 2 deletions test/PoolManagerInitialize.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import {PoolKey} from "../src/types/PoolKey.sol";
import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol";
import {PoolId} from "../src/types/PoolId.sol";
import {LPFeeLibrary} from "../src/libraries/LPFeeLibrary.sol";
import {ProtocolFeeControllerTest} from "../src/test/ProtocolFeeControllerTest.sol";
import {IProtocolFeeController} from "../src/interfaces/IProtocolFeeController.sol";
import {ProtocolFeeLibrary} from "../src/libraries/ProtocolFeeLibrary.sol";
import {StateLibrary} from "../src/libraries/StateLibrary.sol";

Expand Down
27 changes: 13 additions & 14 deletions test/ProtocolFeesImplementation.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {Deployers} from "../test/utils/Deployers.sol";
import {PoolId} from "../src/types/PoolId.sol";
import {IHooks} from "../src/interfaces/IHooks.sol";
import {Constants} from "../test/utils/Constants.sol";
import {ProtocolFeeControllerTest} from "../src/test/ProtocolFeeControllerTest.sol";

contract ProtocolFeesTest is Test, GasSnapshot, Deployers {
using ProtocolFeeLibrary for uint24;
Expand All @@ -28,34 +27,34 @@ contract ProtocolFeesTest is Test, GasSnapshot, Deployers {

function setUp() public {
protocolFees = new ProtocolFeesImplementation();
feeController = new ProtocolFeeControllerTest();
feeController = makeAddr("feeController");
(currency0, currency1) = deployAndMint2Currencies();
MockERC20(Currency.unwrap(currency0)).transfer(address(protocolFees), 2 ** 255);
}

function test_setProtocolFeeController_succeedsNoRevert() public {
assertEq(address(protocolFees.protocolFeeController()), address(0));
assertEq(protocolFees.protocolFeeController(), address(0));
vm.expectEmit(true, false, false, false, address(protocolFees));
emit ProtocolFeeControllerUpdated(address(feeController));
emit ProtocolFeeControllerUpdated(feeController);
protocolFees.setProtocolFeeController(feeController);
assertEq(address(protocolFees.protocolFeeController()), address(feeController));
assertEq(protocolFees.protocolFeeController(), feeController);
}

function test_setProtocolFeeController_revertsWithNotAuthorized() public {
assertEq(address(protocolFees.protocolFeeController()), address(0));
assertEq(protocolFees.protocolFeeController(), address(0));

vm.prank(address(1)); // not the owner address
vm.expectRevert("UNAUTHORIZED");
protocolFees.setProtocolFeeController(feeController);
assertEq(address(protocolFees.protocolFeeController()), address(0));
assertEq(protocolFees.protocolFeeController(), address(0));
}

function test_setProtocolFee_succeeds_gas() public {
PoolKey memory key = PoolKey(currency0, currency1, 3000, 60, IHooks(address(0)));
protocolFees.setProtocolFeeController(feeController);
// Set price to pretend that the pool is initialized
protocolFees.setPrice(key, Constants.SQRT_PRICE_1_1);
vm.prank(address(feeController));
vm.prank(feeController);
vm.expectEmit(true, false, false, true, address(protocolFees));
emit ProtocolFeeUpdated(key.toId(), MAX_PROTOCOL_FEE_BOTH_TOKENS);
protocolFees.setProtocolFee(key, MAX_PROTOCOL_FEE_BOTH_TOKENS);
Expand All @@ -72,12 +71,12 @@ contract ProtocolFeesTest is Test, GasSnapshot, Deployers {
uint24 protocolFee = MAX_PROTOCOL_FEE_BOTH_TOKENS + 1;

protocolFees.setProtocolFeeController(feeController);
vm.prank(address(feeController));
vm.prank(feeController);
vm.expectRevert(abi.encodeWithSelector(IProtocolFees.ProtocolFeeTooLarge.selector, protocolFee));
protocolFees.setProtocolFee(key, protocolFee);

protocolFee = MAX_PROTOCOL_FEE_BOTH_TOKENS + (1 << 12);
vm.prank(address(feeController));
vm.prank(feeController);
vm.expectRevert(abi.encodeWithSelector(IProtocolFees.ProtocolFeeTooLarge.selector, protocolFee));
protocolFees.setProtocolFee(key, protocolFee);
}
Expand All @@ -88,7 +87,7 @@ contract ProtocolFeesTest is Test, GasSnapshot, Deployers {
protocolFees.setPrice(key, Constants.SQRT_PRICE_1_1);
uint16 fee0 = protocolFee.getZeroForOneFee();
uint16 fee1 = protocolFee.getOneForZeroFee();
vm.prank(address(feeController));
vm.prank(feeController);
if ((fee0 > 1000) || (fee1 > 1000)) {
vm.expectRevert(abi.encodeWithSelector(IProtocolFees.ProtocolFeeTooLarge.selector, protocolFee));
protocolFees.setProtocolFee(key, protocolFee);
Expand All @@ -108,7 +107,7 @@ contract ProtocolFeesTest is Test, GasSnapshot, Deployers {
protocolFees.setIsUnlocked(true);

protocolFees.setProtocolFeeController(feeController);
vm.prank(address(feeController));
vm.prank(feeController);

vm.expectRevert(IProtocolFees.ContractUnlocked.selector);
protocolFees.collectProtocolFees(address(1), currency0, 0);
Expand All @@ -120,7 +119,7 @@ contract ProtocolFeesTest is Test, GasSnapshot, Deployers {
assertEq(protocolFees.protocolFeesAccrued(currency0), 100);

protocolFees.setProtocolFeeController(feeController);
vm.prank(address(feeController));
vm.prank(feeController);
protocolFees.collectProtocolFees(address(this), currency0, 100);
assertEq(protocolFees.protocolFeesAccrued(currency0), 0);
assertEq(currency0.balanceOf(address(this)), 100);
Expand All @@ -140,7 +139,7 @@ contract ProtocolFeesTest is Test, GasSnapshot, Deployers {
}

protocolFees.setProtocolFeeController(feeController);
vm.prank(address(feeController));
vm.prank(feeController);
if (amount > feesAccrued) {
vm.expectRevert();
}
Expand Down
Loading

0 comments on commit d9403bd

Please sign in to comment.