From 4fcacd529328242393c618dde3b7bbe60aac8da3 Mon Sep 17 00:00:00 2001 From: ChefMist <133624774+ChefMist@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:20:02 +0800 Subject: [PATCH 1/4] feat: increase max protocol fee --- src/libraries/ProtocolFeeLibrary.sol | 8 +++---- test/ProtocolFees.t.sol | 28 ++++++++++++------------- test/libraries/ProtocolFeeLibrary.t.sol | 4 +++- test/pool-cl/CLPoolManager.t.sol | 4 +--- test/pool-cl/CLProtocolFees.t.sol | 6 +++--- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/libraries/ProtocolFeeLibrary.sol b/src/libraries/ProtocolFeeLibrary.sol index 7dfedc7d..ac4a70e9 100644 --- a/src/libraries/ProtocolFeeLibrary.sol +++ b/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; diff --git a/test/ProtocolFees.t.sol b/test/ProtocolFees.t.sol index d871f2a2..a72ed801 100644 --- a/test/ProtocolFees.t.sol +++ b/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); } diff --git a/test/libraries/ProtocolFeeLibrary.t.sol b/test/libraries/ProtocolFeeLibrary.t.sol index 87a8168c..e0305467 100644 --- a/test/libraries/ProtocolFeeLibrary.t.sol +++ b/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 diff --git a/test/pool-cl/CLPoolManager.t.sol b/test/pool-cl/CLPoolManager.t.sol index 380939c5..28843454 100644 --- a/test/pool-cl/CLPoolManager.t.sol +++ b/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}); diff --git a/test/pool-cl/CLProtocolFees.t.sol b/test/pool-cl/CLProtocolFees.t.sol index 4157727b..029becc0 100644 --- a/test/pool-cl/CLProtocolFees.t.sol +++ b/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); From 2697472269bdff675dcb5aad8261059ce06f1671 Mon Sep 17 00:00:00 2001 From: ChefMist <133624774+ChefMist@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:41:58 +0800 Subject: [PATCH 2/4] feat: update fuzzing test to cover protocolFee --- test/pool-cl/libraries/CLPool.t.sol | 44 +++++++++++++++++++---------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/test/pool-cl/libraries/CLPool.t.sol b/test/pool-cl/libraries/CLPool.t.sol index 58866677..0ad1765b 100644 --- a/test/pool-cl/libraries/CLPool.t.sol +++ b/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); From 7d1ccfac2e081ffb928b75efce5da00ce3d30810 Mon Sep 17 00:00:00 2001 From: ChefMist <133624774+ChefMist@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:13:48 +0800 Subject: [PATCH 3/4] test: update bin pool test around protoocl fee --- .../libraries/math/PackedUint128Math.t.sol | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/test/pool-bin/libraries/math/PackedUint128Math.t.sol b/test/pool-bin/libraries/math/PackedUint128Math.t.sol index 1d711a27..5966b9ae 100644 --- a/test/pool-bin/libraries/math/PackedUint128Math.t.sol +++ b/test/pool-bin/libraries/math/PackedUint128Math.t.sol @@ -179,31 +179,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))); } { - // 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 = (1000 << 12) + 1000; // 0.1% 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(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))); + } + + { + bytes32 totalFee = uint128(10_000).encode(uint128(10_000)); + uint24 protocolFee = (4000 << 12) + 4000; // 0.4% fee + + 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))); } } } From 8791a18e843009c3974b30494ebf9bb0c5e55e2b Mon Sep 17 00:00:00 2001 From: ChefMist <133624774+ChefMist@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:17:45 +0800 Subject: [PATCH 4/4] lint fix --- .../libraries/math/PackedUint128Math.t.sol | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/test/pool-bin/libraries/math/PackedUint128Math.t.sol b/test/pool-bin/libraries/math/PackedUint128Math.t.sol index 5966b9ae..cf000a44 100644 --- a/test/pool-bin/libraries/math/PackedUint128Math.t.sol +++ b/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(); @@ -188,7 +194,7 @@ contract PackedUint128MathTest is Test { { bytes32 totalFee = uint128(10_000).encode(uint128(10_000)); - uint24 protocolFee = (100 << 12) + 100; // 0.01% fee + uint24 protocolFee = (100 << 12) + 100; // 0.01% fee // lpFee 0% assertEq(totalFee.getProtocolFeeAmt(protocolFee, 100), uint128(10_000).encode(uint128(10_000))); @@ -205,9 +211,9 @@ contract PackedUint128MathTest is Test { { bytes32 totalFee = uint128(10_000).encode(uint128(10_000)); - uint24 protocolFee = (4000 << 12) + 4000; // 0.4% fee + uint24 protocolFee = (4000 << 12) + 4000; // 0.4% fee - uint24 swapFee = ProtocolFeeLibrary.calculateSwapFee(4000, 3000); + 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))); }