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

round swap fee up and check if protocol fee = swap fee #905

Merged
merged 7 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
Original file line number Diff line number Diff line change
@@ -1 +1 @@
144663
144639
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170959
170928
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
274264
274240
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
135141
135120
Original file line number Diff line number Diff line change
@@ -1 +1 @@
292855
292831
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 1 token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
106345
106333
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 2 tokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145766
145748
2 changes: 1 addition & 1 deletion .forge-snapshots/erc20 collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
57454
57451
2 changes: 1 addition & 1 deletion .forge-snapshots/extsload getFeeGrowthGlobals.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
777
774
2 changes: 1 addition & 1 deletion .forge-snapshots/extsload getPositionInfo.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
952
949
2 changes: 1 addition & 1 deletion .forge-snapshots/extsload getTickFeeGrowthOutside.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
777
774
2 changes: 1 addition & 1 deletion .forge-snapshots/extsload getTickInfo.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
952
949
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
51536
51533
2 changes: 1 addition & 1 deletion .forge-snapshots/native collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
59726
59723
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23621
23682
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
141219
141194
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
130621
130603
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
112535
112523
Original file line number Diff line number Diff line change
@@ -1 +1 @@
98862
98850
2 changes: 1 addition & 1 deletion .forge-snapshots/simple addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
161407
161395
Original file line number Diff line number Diff line change
@@ -1 +1 @@
92995
92986
2 changes: 1 addition & 1 deletion .forge-snapshots/simple removeLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
85108
85099
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap with native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
108515
108434
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123347
123263
2 changes: 1 addition & 1 deletion .forge-snapshots/swap CA custom curve + swap noop.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124653
124635
2 changes: 1 addition & 1 deletion .forge-snapshots/swap CA fee on unspecified.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
154773
154686
Original file line number Diff line number Diff line change
@@ -1 +1 @@
105637
105569
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
116717
116646
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129281
129285
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn native 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
118725
118672
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint native output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139747
139739
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
155188
155104
Original file line number Diff line number Diff line change
@@ -1 +1 @@
206403
206251
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139368
139284
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
132345
132274
1 change: 0 additions & 1 deletion .forge-snapshots/swap with lp fee and protocol fee.snap

This file was deleted.

2 changes: 1 addition & 1 deletion .forge-snapshots/swap with return dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145661
145577
2 changes: 1 addition & 1 deletion .forge-snapshots/update dynamic fee in before swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147956
147869
7 changes: 5 additions & 2 deletions src/libraries/Pool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,11 @@ library Pool {
unchecked {
// step.amountIn does not include the swap fee, as it's already been taken from it,
// so add it back to get the total amountIn and use that to calculate the amount of fees owed to the protocol
// this line cannot overflow due to limits on the size of protocolFee and params.amountSpecified
uint256 delta = (step.amountIn + step.feeAmount) * protocolFee / ProtocolFeeLibrary.PIPS_DENOMINATOR;
// cannot overflow due to limits on the size of protocolFee and params.amountSpecified
// this rounds down to favor LPs over the protocol
uint256 delta = (swapFee == protocolFee)
? step.feeAmount // lp fee is 0, so the entire fee is owed to the protocol instead
: (step.amountIn + step.feeAmount) * protocolFee / ProtocolFeeLibrary.PIPS_DENOMINATOR;
// subtract it from the total fee and add it to the protocol fee
step.feeAmount -= delta;
amountToProtocol += delta;
Expand Down
7 changes: 3 additions & 4 deletions src/libraries/ProtocolFeeLibrary.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,15 @@ library ProtocolFeeLibrary {

// The protocol fee is taken from the input amount first and then the LP fee is taken from the remaining
// The swap fee is capped at 100%
// Equivalent to protocolFee + lpFee(1_000_000 - protocolFee) / 1_000_000
// Equivalent to protocolFee + lpFee(1_000_000 - protocolFee) / 1_000_000 (rounded up)
/// @dev here `self` is just a single direction's protocol fee, not a packed type of 2 protocol fees
function calculateSwapFee(uint16 self, uint24 lpFee) internal pure returns (uint24 swapFee) {
// protocolFee + lpFee - (protocolFee * lpFee / 1_000_000). Div rounds up to favor LPs over the protocol.
// protocolFee + lpFee - (protocolFee * lpFee / 1_000_000)
assembly ("memory-safe") {
self := and(self, 0xfff)
lpFee := and(lpFee, 0xffffff)
let numerator := mul(self, lpFee)
let divRoundingUp := add(div(numerator, PIPS_DENOMINATOR), gt(mod(numerator, PIPS_DENOMINATOR), 0))
swapFee := sub(add(self, lpFee), divRoundingUp)
swapFee := sub(add(self, lpFee), div(numerator, PIPS_DENOMINATOR))
}
}
}
23 changes: 12 additions & 11 deletions test/DynamicFees.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ import {MockERC20} from "solmate/src/test/utils/mocks/MockERC20.sol";
import {Pool} from "../src/libraries/Pool.sol";
import {BalanceDelta, BalanceDeltaLibrary} from "../src/types/BalanceDelta.sol";
import {StateLibrary} from "../src/libraries/StateLibrary.sol";
import {ProtocolFeeLibrary} from "../src/libraries/ProtocolFeeLibrary.sol";

contract TestDynamicFees is Test, Deployers, GasSnapshot {
using StateLibrary for IPoolManager;
using ProtocolFeeLibrary for uint16;

DynamicFeesTestHook dynamicFeesHooks = DynamicFeesTestHook(
address(
Expand Down Expand Up @@ -228,16 +230,8 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
PoolSwapTest.TestSettings memory testSettings =
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});

vm.expectEmit(true, true, true, true, address(manager));
emit Swap(key.toId(), address(swapRouter), -101000000, 100, 79228162514264329670727698909, 1e18, -1, 999999);

BalanceDelta delta = swapRouter.swap(key, params, testSettings, ZERO_BYTES);
snapLastCall("swap with lp fee and protocol fee");

uint256 expectedProtocolFee = uint256(uint128(-delta.amount0())) * 1000 / 1e6;
assertEq(manager.protocolFeesAccrued(currency0), expectedProtocolFee);

assertEq(_fetchPoolLPFee(key), 999999);
vm.expectRevert(Pool.InvalidFeeForExactOut.selector);
swapRouter.swap(key, params, testSettings, ZERO_BYTES);
}

function test_swap_100PercentFee_AmountIn_WithProtocol() public {
Expand Down Expand Up @@ -274,7 +268,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});

vm.expectEmit(true, true, true, true, address(manager));
emit Swap(key.toId(), address(swapRouter), -100, 98, 79228162514264329749955861424, 1e18, -1, 1122);
emit Swap(key.toId(), address(swapRouter), -100, 98, 79228162514264329749955861424, 1e18, -1, 1123);

swapRouter.swap(key, SWAP_PARAMS, testSettings, ZERO_BYTES);

Expand Down Expand Up @@ -307,7 +301,14 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {

BalanceDelta delta = swapRouter.swap(key, params, testSettings, ZERO_BYTES);

uint24 swapFee = uint16(protocolFee).calculateSwapFee(lpFee);

uint256 expectedProtocolFee = uint256(uint128(-delta.amount0())) * protocolFee0 / 1e6;
if (lpFee == 0) {
assertEq(protocolFee0, swapFee);
if (((uint256(uint128(-delta.amount0())) * protocolFee0) % 1e6) != 0) expectedProtocolFee++;
}

assertEq(manager.protocolFeesAccrued(currency0), expectedProtocolFee);
}

Expand Down
12 changes: 6 additions & 6 deletions test/libraries/ProtocolFeeLibrary.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,18 @@ contract ProtocolFeeLibraryTest is Test, GasSnapshot {
protocolFee = uint16(bound(protocolFee, 0, ProtocolFeeLibrary.MAX_PROTOCOL_FEE));
lpFee = uint24(bound(lpFee, 0, LPFeeLibrary.MAX_LP_FEE));
uint24 swapFee = ProtocolFeeLibrary.calculateSwapFee(protocolFee, lpFee);
// if lp fee is not the max, the swap fee should never be the max since the protocol fee is taken off first and then the lp fee is taken from the remaining amount
if (lpFee < LPFeeLibrary.MAX_LP_FEE) {
assertLt(swapFee, LPFeeLibrary.MAX_LP_FEE);
assertLe(swapFee, LPFeeLibrary.MAX_LP_FEE);
} else {
// otherwise it is equal to max, and can therefore never be larger
// if lp fee is equal to max, swap fee can never be larger
assertEq(swapFee, LPFeeLibrary.MAX_LP_FEE);
}

assertGe(swapFee, lpFee);
// protocolFee + lpFee(1_000_000 - protocolFee) / 1_000_000 (rounded up)
uint256 expectedSwapFee = protocolFee + (1e6 - protocolFee) * uint256(lpFee) / 1e6;
if (((1e6 - protocolFee) * uint256(lpFee)) % 1e6 != 0) expectedSwapFee++;

uint256 expectedSwapFee =
protocolFee + lpFee * uint256(LPFeeLibrary.MAX_LP_FEE - protocolFee) / LPFeeLibrary.MAX_LP_FEE;
assertGe(swapFee, lpFee);
assertEq(swapFee, uint24(expectedSwapFee));
}
}
Loading