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.

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

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

Merged
merged 5 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions src/libraries/ProtocolFeeLibrary.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
28 changes: 14 additions & 14 deletions test/ProtocolFees.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
Expand Down
4 changes: 3 additions & 1 deletion test/libraries/ProtocolFeeLibrary.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 29 additions & 17 deletions test/pool-bin/libraries/math/PackedUint128Math.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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)));
}
}
}
4 changes: 1 addition & 3 deletions test/pool-cl/CLPoolManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand Down
6 changes: 3 additions & 3 deletions test/pool-cl/CLProtocolFees.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
44 changes: 29 additions & 15 deletions test/pool-cl/libraries/CLPool.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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));
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
Loading