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

Disallow owner from claiming protocol fees #474

Merged
merged 7 commits into from
Feb 12, 2024
Merged

Disallow owner from claiming protocol fees #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a74cd8b
don't allow owner to claim fees, update tests
zhongeric Feb 7, 2024
9b4df97
Add revert test for invalid caller
zhongeric Feb 7, 2024
04478e7
fix: fix directory paths in README (#472)
partylikeits1983 Feb 12, 2024
eff1639
Remove custom solc in favor of the official 0.8.24 (#473)
saucepoint Feb 12, 2024
7fe7dd9
udpate gas again
zhongeric Feb 12, 2024
340a9ed
Merge branch 'main' into remove-owner-pfc
zhongeric Feb 12, 2024
f8822d3
gas snaps lol
zhongeric Feb 12, 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/erc20 collect protocol fees.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
26983
24960
2 changes: 1 addition & 1 deletion .forge-snapshots/native collect protocol fees.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
38656
36633
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 @@
24630
24607
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 @@
173778
173820
2 changes: 1 addition & 1 deletion src/Fees.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ abstract contract Fees is IFees, Owned {
external
returns (uint256 amountCollected)
{
if (msg.sender != owner && msg.sender != address(protocolFeeController)) revert InvalidCaller();
if (msg.sender != address(protocolFeeController)) revert InvalidCaller();

amountCollected = (amount == 0) ? protocolFeesAccrued[currency] : amount;
protocolFeesAccrued[currency] -= amountCollected;
Expand Down
14 changes: 12 additions & 2 deletions test/PoolManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {IPoolManager} from "../src/interfaces/IPoolManager.sol";
import {IFees} from "../src/interfaces/IFees.sol";
import {IProtocolFeeController} from "../src/interfaces/IProtocolFeeController.sol";
import {PoolManager} from "../src/PoolManager.sol";
import {Owned} from "../src/Owned.sol";
import {TickMath} from "../src/libraries/TickMath.sol";
import {Pool} from "../src/libraries/Pool.sol";
import {Deployers} from "./utils/Deployers.sol";
Expand Down Expand Up @@ -1051,7 +1052,12 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
assertEq(slot0.protocolFee, protocolFee);
}

function test_collectProtocolFees_ERC20_allowsOwnerToAccumulateFees_gas() public {
function test_collectProtocolFees_revertsIfCallerIsNotController() public {
vm.expectRevert(Owned.InvalidCaller.selector);
manager.collectProtocolFees(address(1), currency0, 0);
}

function test_collectProtocolFees_ERC20_accumulateFees_gas() public {
uint16 protocolFee = 1028; // 00000100 00000100
uint256 expectedFees = 7;

Expand All @@ -1071,6 +1077,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));
snapStart("erc20 collect protocol fees");
manager.collectProtocolFees(address(1), currency0, expectedFees);
snapEnd();
Expand Down Expand Up @@ -1316,12 +1323,13 @@ 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));
manager.collectProtocolFees(address(1), currency0, 0);
assertEq(currency0.balanceOf(address(1)), expectedFees);
assertEq(manager.protocolFeesAccrued(currency0), 0);
}

function test_collectProtocolFees_nativeToken_allowsOwnerToAccumulateFees_gas() public {
function test_collectProtocolFees_nativeToken_accumulateFees_gas() public {
uint16 protocolFee = 1028; // 00000100 00000100
uint256 expectedFees = 7;
Currency nativeCurrency = CurrencyLibrary.NATIVE;
Expand All @@ -1343,6 +1351,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));
snapStart("native collect protocol fees");
manager.collectProtocolFees(address(1), nativeCurrency, expectedFees);
snapEnd();
Expand Down Expand Up @@ -1371,6 +1380,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));
manager.collectProtocolFees(address(1), nativeCurrency, 0);
assertEq(nativeCurrency.balanceOf(address(1)), expectedFees);
assertEq(manager.protocolFeesAccrued(nativeCurrency), 0);
Expand Down
Loading