[Internal S-45] feat: increase max protocol fee (#188)

* feat: increase max protocol fee

* feat: update fuzzing test to cover protocolFee

* test: update bin pool test around protoocl fee

* lint fix

src/libraries/ProtocolFeeLibrary.sol

@@ -6,12 +6,12 @@ import "./math/UnsafeMath.sol";
 
 library ProtocolFeeLibrary {
     /// @dev Increasing these values could lead to overflow in Pool.swap
-    /// @notice Max protocol fee is 0.1% (1000 pips)
-    uint16 public constant MAX_PROTOCOL_FEE = 1000;
+    /// @notice Max protocol fee is 0.4% (4000 pips)
+    uint16 public constant MAX_PROTOCOL_FEE = 4000;
 
     /// @notice Thresholds used for optimized bounds checks on protocol fees
-    uint24 internal constant FEE_0_THRESHOLD = 1001;
-    uint24 internal constant FEE_1_THRESHOLD = 1001 << 12;
+    uint24 internal constant FEE_0_THRESHOLD = 4001;
+    uint24 internal constant FEE_1_THRESHOLD = 4001 << 12;
 
     /// @notice the protocol fee is represented in hundredths of a bip
     uint256 internal constant PIPS_DENOMINATOR = 1_000_000;

test/ProtocolFees.t.sol

@@ -173,15 +173,15 @@ contract ProtocolFeesTest is Test {
     }
 
     function testSwap_OnlyProtocolFee() public {
-        // set protocolFee as 0.1% of fee
+        // set protocolFee as 0.4% of fee
         uint24 protocolFee = _buildProtocolFee(ProtocolFeeLibrary.MAX_PROTOCOL_FEE, ProtocolFeeLibrary.MAX_PROTOCOL_FEE);
         feeController.setProtocolFeeForPool(key, protocolFee);
         poolManager.setProtocolFeeController(IProtocolFeeController(address(feeController)));
 
         poolManager.initialize(key);
         (uint256 protocolFee0, uint256 protocolFee1) = poolManager.swap(key, 1e18, 1e18);
-        assertEq(protocolFee0, 1e15);
-        assertEq(protocolFee1, 1e15);
+        assertEq(protocolFee0, 4e15);
+        assertEq(protocolFee1, 4e15);
     }
 
     function test_CollectProtocolFee_OnlyFeeController() public {
@@ -198,33 +198,33 @@ contract ProtocolFeesTest is Test {
     }
 
     function test_CollectProtocolFee() public {
-        // set protocolFee as 0.1% of fee
+        // set protocolFee as 0.4% of fee
         uint24 protocolFee = _buildProtocolFee(ProtocolFeeLibrary.MAX_PROTOCOL_FEE, ProtocolFeeLibrary.MAX_PROTOCOL_FEE);
         feeController.setProtocolFeeForPool(key, protocolFee);
         poolManager.setProtocolFeeController(IProtocolFeeController(address(feeController)));
         poolManager.initialize(key);
         (uint256 protocolFee0, uint256 protocolFee1) = poolManager.swap(key, 1e18, 1e18);
-        assertEq(protocolFee0, 1e15);
-        assertEq(protocolFee1, 1e15);
+        assertEq(protocolFee0, 4e15);
+        assertEq(protocolFee1, 4e15);
 
         // send some token to vault as poolManager.swap doesn't have tokens
-        token0.mint(address(vault), 1e15);
-        token1.mint(address(vault), 1e15);
+        token0.mint(address(vault), 4e15);
+        token1.mint(address(vault), 4e15);
 
         // before collect
         assertEq(token0.balanceOf(alice), 0);
         assertEq(token1.balanceOf(alice), 0);
-        assertEq(token0.balanceOf(address(vault)), 1e15);
-        assertEq(token1.balanceOf(address(vault)), 1e15);
+        assertEq(token0.balanceOf(address(vault)), 4e15);
+        assertEq(token1.balanceOf(address(vault)), 4e15);
 
         // collect
         vm.startPrank(address(feeController));
-        poolManager.collectProtocolFees(alice, Currency.wrap(address(token0)), 1e15);
-        poolManager.collectProtocolFees(alice, Currency.wrap(address(token1)), 1e15);
+        poolManager.collectProtocolFees(alice, Currency.wrap(address(token0)), 4e15);
+        poolManager.collectProtocolFees(alice, Currency.wrap(address(token1)), 4e15);
 
         // after collect
-        assertEq(token0.balanceOf(alice), 1e15);
-        assertEq(token1.balanceOf(alice), 1e15);
+        assertEq(token0.balanceOf(alice), 4e15);
+        assertEq(token1.balanceOf(alice), 4e15);
         assertEq(token0.balanceOf(address(vault)), 0);
         assertEq(token1.balanceOf(address(vault)), 0);
     }

test/libraries/ProtocolFeeLibrary.t.sol

@@ -60,7 +60,9 @@ contract ProtocolFeeLibraryTest is Test, GasSnapshot {
             ),
             LPFeeLibrary.ONE_HUNDRED_PERCENT_FEE
         );
-        assertEq(ProtocolFeeLibrary.calculateSwapFee(ProtocolFeeLibrary.MAX_PROTOCOL_FEE, 3000), 3997);
+        assertEq(ProtocolFeeLibrary.calculateSwapFee(1000, 3000), 3997);
+        // 0.4% + 0.3% * 0 + 3000 * (1000000 - 4000) / 1000000
+        assertEq(ProtocolFeeLibrary.calculateSwapFee(ProtocolFeeLibrary.MAX_PROTOCOL_FEE, 3000), 6988);
         assertEq(
             ProtocolFeeLibrary.calculateSwapFee(ProtocolFeeLibrary.MAX_PROTOCOL_FEE, 0),
             ProtocolFeeLibrary.MAX_PROTOCOL_FEE

test/pool-bin/libraries/math/PackedUint128Math.t.sol

@@ -149,8 +149,14 @@ contract PackedUint128MathTest is Test {
         assertEq(x.gt(y), x1 > y1 || x2 > y2, "testFuzz_GreaterThan::1");
     }
 
-    function testFuzz_getProtocolFeeAmt(bytes32 x, uint24 protocolFee, uint24 swapFee) external pure {
-        protocolFee = uint24(bound(protocolFee, 0, 1_000_000));
+    function testFuzz_getProtocolFeeAmt(bytes32 x, uint16 protocolFee0, uint16 protocolFee1, uint24 swapFee)
+        external
+        pure
+    {
+        protocolFee0 = uint16(bound(protocolFee0, 0, ProtocolFeeLibrary.MAX_PROTOCOL_FEE));
+        protocolFee1 = uint16(bound(protocolFee1, 0, ProtocolFeeLibrary.MAX_PROTOCOL_FEE));
+        uint24 protocolFee = protocolFee1 << 12 | protocolFee0;
+
         swapFee = uint24(bound(swapFee, protocolFee, 1_000_000));
 
         (uint128 x1, uint128 x2) = x.decode();
@@ -179,31 +185,37 @@ contract PackedUint128MathTest is Test {
         amounts.getProtocolFeeAmt(protocolFee, swapFee);
     }
 
-    function test_getProtocolFeeAmt() external pure {
+    function test_getProtocolFeeAmtX() external pure {
         {
-            // 0% fee
-            bytes32 x = uint128(100).encode(uint128(100));
-            uint24 fee = (0 << 12) + 0; // amt / 0 = 0%
-            assertEq(x.getProtocolFeeAmt(fee, 0), 0);
+            bytes32 totalFee = uint128(100).encode(uint128(100));
+            uint24 protocolFee = (0 << 12) + 0; // 0% fee
+            assertEq(totalFee.getProtocolFeeAmt(protocolFee, 0), 0);
         }
 
         {
-            // 0.01% fee
-            bytes32 x = uint128(10000).encode(uint128(10000));
-            uint24 fee = (100 << 12) + 100;
+            bytes32 totalFee = uint128(10_000).encode(uint128(10_000));
+            uint24 protocolFee = (100 << 12) + 100; // 0.01% fee
 
             // lpFee 0%
-            assertEq(x.getProtocolFeeAmt(fee, 100), uint128(10000).encode(uint128(10000)));
+            assertEq(totalFee.getProtocolFeeAmt(protocolFee, 100), uint128(10_000).encode(uint128(10_000)));
+        }
+
+        {
+            bytes32 totalFee = uint128(10_000).encode(uint128(10_000));
+            uint24 protocolFee = (1000 << 12) + 1000; // 0.1% fee
+
+            uint24 swapFee = ProtocolFeeLibrary.calculateSwapFee(1000, 3000); // 0.1% protocolFee, 0.3% lpFee
+            // protocolFee is roughly more than 1/4 as protocolFee is taken out first
+            assertEq(totalFee.getProtocolFeeAmt(protocolFee, swapFee), uint128(2501).encode(uint128(2501)));
         }
 
         {
-            // 0.1% fee
-            bytes32 x = uint128(10000).encode(uint128(10000));
-            uint24 fee = (1000 << 12) + 1000;
+            bytes32 totalFee = uint128(10_000).encode(uint128(10_000));
+            uint24 protocolFee = (4000 << 12) + 4000; // 0.4% fee
 
-            // 0.3% lp fee => swap fee 0.3997%
-            uint24 swapFee = ProtocolFeeLibrary.calculateSwapFee(1000, 3000);
-            assertEq(x.getProtocolFeeAmt(fee, swapFee), uint128(2501).encode(uint128(2501)));
+            uint24 swapFee = ProtocolFeeLibrary.calculateSwapFee(4000, 3000);
+            // protocolFee is roughly more than 4/7 as protocolFee is taken out first
+            assertEq(totalFee.getProtocolFeeAmt(protocolFee, swapFee), uint128(5724).encode(uint128(5724)));
         }
     }
 }

test/pool-cl/CLPoolManager.t.sol

@@ -1918,9 +1918,7 @@ contract CLPoolManagerTest is Test, NoIsolate, Deployers, TokenFixture, GasSnaps
 
         // 0.1%
         vm.prank(address(feeController));
-        poolManager.setProtocolFee(
-            key, ProtocolFeeLibrary.MAX_PROTOCOL_FEE | (uint24(ProtocolFeeLibrary.MAX_PROTOCOL_FEE) << 12)
-        );
+        poolManager.setProtocolFee(key, 1000 | (uint24(1000) << 12));
 
         ICLPoolManager.ModifyLiquidityParams memory modifyPositionParams =
             ICLPoolManager.ModifyLiquidityParams({tickLower: -60, tickUpper: 60, liquidityDelta: 1 ether, salt: 0});

test/pool-cl/CLProtocolFees.t.sol

@@ -133,13 +133,13 @@ contract CLProtocolFeesTest is Test, Deployers, TokenFixture, GasSnapshot {
             ZERO_BYTES
         );
 
-        uint256 expectedProtocolAmount1 = 10000 * (protocolFee >> 12) / ProtocolFeeLibrary.PIPS_DENOMINATOR;
+        uint256 expectedProtocolAmount1 = 10000 * (uint256(protocolFee >> 12)) / ProtocolFeeLibrary.PIPS_DENOMINATOR;
         assertEq(manager.protocolFeesAccrued(currency0), 0);
         assertEq(manager.protocolFeesAccrued(currency1), expectedProtocolAmount1);
     }
 
     function testCollectFees() public {
-        uint24 protocolFee = ProtocolFeeLibrary.MAX_PROTOCOL_FEE | (uint24(ProtocolFeeLibrary.MAX_PROTOCOL_FEE) << 12); // 0.1% protocol fee
+        uint24 protocolFee = ProtocolFeeLibrary.MAX_PROTOCOL_FEE | (uint24(ProtocolFeeLibrary.MAX_PROTOCOL_FEE) << 12); // 0.4% protocol fee
         manager.setProtocolFeeController(IProtocolFeeController(protocolFeeController));
         vm.prank(address(protocolFeeController));
         manager.setProtocolFee(key, protocolFee);
@@ -158,7 +158,7 @@ contract CLProtocolFeesTest is Test, Deployers, TokenFixture, GasSnapshot {
             ZERO_BYTES
         );
 
-        uint256 expectedProtocolFees = 1000000 * 0.001;
+        uint256 expectedProtocolFees = 1000000 * 0.004;
         vm.prank(address(protocolFeeController));
         manager.collectProtocolFees(address(protocolFeeController), currency1, 0);
         assertEq(currency1.balanceOf(address(protocolFeeController)), expectedProtocolFees);

test/pool-cl/libraries/CLPool.t.sol

@@ -28,10 +28,7 @@ contract PoolTest is Test {
 
     CLPool.State state;
 
-    function testPoolInitialize(uint160 sqrtPriceX96, uint16 protocolFee, uint24 lpFee) public {
-        protocolFee = uint16(bound(protocolFee, 0, 2 ** 16 - 1));
-        lpFee = uint24(bound(lpFee, 0, 999999));
-
+    function testPoolInitialize(uint160 sqrtPriceX96, uint24 protocolFee, uint24 lpFee) public {
         if (sqrtPriceX96 < TickMath.MIN_SQRT_RATIO || sqrtPriceX96 >= TickMath.MAX_SQRT_RATIO) {
             vm.expectRevert(abi.encodeWithSelector(TickMath.InvalidSqrtRatio.selector, sqrtPriceX96));
             state.initialize(sqrtPriceX96, protocolFee, lpFee);
@@ -46,14 +43,16 @@ contract PoolTest is Test {
         }
     }
 
-    function testModifyPosition(uint160 sqrtPriceX96, CLPool.ModifyLiquidityParams memory params, uint24 lpFee)
-        public
-    {
+    function testModifyPosition(
+        uint160 sqrtPriceX96,
+        CLPool.ModifyLiquidityParams memory params,
+        uint24 lpFee,
+        uint24 protocolFee
+    ) public {
         // Assumptions tested in PoolManager.t.sol
-        params.tickSpacing = int24(bound(params.tickSpacing, 1, 32767));
-        lpFee = uint24(bound(lpFee, 0, LPFeeLibrary.ONE_HUNDRED_PERCENT_FEE - 1));
+        params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING));
 
-        testPoolInitialize(sqrtPriceX96, 0, lpFee);
+        testPoolInitialize(sqrtPriceX96, protocolFee, lpFee);
 
         if (params.tickLower >= params.tickUpper) {
             vm.expectRevert(abi.encodeWithSelector(Tick.TicksMisordered.selector, params.tickLower, params.tickUpper));
@@ -98,7 +97,9 @@ contract PoolTest is Test {
         uint160 sqrtPriceX96,
         CLPool.ModifyLiquidityParams memory modifyLiquidityParams,
         CLPool.SwapParams memory swapParams,
-        uint24 lpFee
+        uint24 lpFee,
+        uint16 protocolFee0,
+        uint16 protocolFee1
     ) public {
         // modifyLiquidityParams = CLPool.ModifyLiquidityParams({
         //     owner: 0x250Eb93F2C350590E52cdb977b8BcF502a1Db7e7,
@@ -120,11 +121,17 @@ contract PoolTest is Test {
         // 2. and the effect price is either too large or too small (due to larger price slippage or inproper liquidity range)
         // It will cause the amountUnspecified to be out of int128 range hence the tx reverts with SafeCastOverflow
         // try to comment following three limitations and uncomment above case and rerun the test to verify
+
+        lpFee = uint24(bound(lpFee, 0, LPFeeLibrary.ONE_HUNDRED_PERCENT_FEE));
+        protocolFee0 = uint16(bound(protocolFee0, 0, ProtocolFeeLibrary.MAX_PROTOCOL_FEE));
+        protocolFee1 = uint16(bound(protocolFee1, 0, ProtocolFeeLibrary.MAX_PROTOCOL_FEE));
+        uint24 protocolFee = protocolFee1 << 12 | protocolFee0;
+
         modifyLiquidityParams.tickLower = -100;
         modifyLiquidityParams.tickUpper = 100;
         swapParams.amountSpecified = int256(bound(swapParams.amountSpecified, 0, type(int128).max));
 
-        testModifyPosition(sqrtPriceX96, modifyLiquidityParams, lpFee);
+        testModifyPosition(sqrtPriceX96, modifyLiquidityParams, lpFee, protocolFee);
 
         swapParams.tickSpacing = modifyLiquidityParams.tickSpacing;
         CLSlot0 slot0 = state.slot0;
@@ -204,11 +211,18 @@ contract PoolTest is Test {
     function testDonate(
         uint160 sqrtPriceX96,
         CLPool.ModifyLiquidityParams memory params,
-        uint24 swapFee,
         uint256 amount0,
-        uint256 amount1
+        uint256 amount1,
+        uint24 lpFee,
+        uint16 protocolFee0,
+        uint16 protocolFee1
     ) public {
-        testModifyPosition(sqrtPriceX96, params, swapFee);
+        lpFee = uint24(bound(lpFee, 0, LPFeeLibrary.ONE_HUNDRED_PERCENT_FEE));
+        protocolFee0 = uint16(bound(protocolFee0, 0, ProtocolFeeLibrary.MAX_PROTOCOL_FEE));
+        protocolFee1 = uint16(bound(protocolFee1, 0, ProtocolFeeLibrary.MAX_PROTOCOL_FEE));
+        uint24 protocolFee = protocolFee1 << 12 | protocolFee0;
+
+        testModifyPosition(sqrtPriceX96, params, lpFee, protocolFee);
 
         int24 tick = TickMath.getTickAtSqrtRatio(sqrtPriceX96);