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

Jump to bottom

remove IProtocolFeeController #907

Merged
merged 6 commits into from
Oct 21, 2024
Merged

remove IProtocolFeeController #907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
27a8b76
remove IProtocolFeeController
snreynolds Oct 18, 2024
32430ad
typo
snreynolds Oct 18, 2024
e82c4ec
remove cast
snreynolds Oct 18, 2024
f9ab940
remove more casts
snreynolds Oct 18, 2024
3e01545
merge main
snreynolds Oct 21, 2024
80d9100
Merge branch 'main' into remove-i-protocol-fee-controller
hensha256 Oct 21, 2024
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/poolManager bytecode size.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23682
23659
2 changes: 1 addition & 1 deletion .forge-snapshots/swap skips hook call if hook is caller.snap
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147869
147857
11 changes: 5 additions & 6 deletions src/ProtocolFees.sol
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

7 changes: 3 additions & 4 deletions src/interfaces/IProtocolFees.sol
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

8 changes: 4 additions & 4 deletions test/DynamicFees.t.sol
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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");
snreynolds marked this conversation as resolved.
Show resolved Hide resolved
(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
Loading