diff --git a/.forge-snapshots/add liquidity to already existing position with salt.snap b/.forge-snapshots/add liquidity to already existing position with salt.snap index 342163227..567cfb72b 100644 --- a/.forge-snapshots/add liquidity to already existing position with salt.snap +++ b/.forge-snapshots/add liquidity to already existing position with salt.snap @@ -1 +1 @@ -144663 \ No newline at end of file +144639 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap index d4dff6e54..8342ac755 100644 --- a/.forge-snapshots/addLiquidity CA fee.snap +++ b/.forge-snapshots/addLiquidity CA fee.snap @@ -1 +1 @@ -170959 \ No newline at end of file +170928 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with empty hook.snap b/.forge-snapshots/addLiquidity with empty hook.snap index 9d9c26d43..b04e884a5 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -274264 \ No newline at end of file +274240 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity with native token.snap b/.forge-snapshots/addLiquidity with native token.snap index a7684fdf5..cd7fd0616 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -135141 \ No newline at end of file +135120 \ No newline at end of file diff --git a/.forge-snapshots/create new liquidity to a position with salt.snap b/.forge-snapshots/create new liquidity to a position with salt.snap index 3c4d3e51e..521237ab9 100644 --- a/.forge-snapshots/create new liquidity to a position with salt.snap +++ b/.forge-snapshots/create new liquidity to a position with salt.snap @@ -1 +1 @@ -292855 \ No newline at end of file +292831 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 1 token.snap b/.forge-snapshots/donate gas with 1 token.snap index cb54169dc..f2a99d3bb 100644 --- a/.forge-snapshots/donate gas with 1 token.snap +++ b/.forge-snapshots/donate gas with 1 token.snap @@ -1 +1 @@ -106345 \ No newline at end of file +106333 \ No newline at end of file diff --git a/.forge-snapshots/donate gas with 2 tokens.snap b/.forge-snapshots/donate gas with 2 tokens.snap index 0f05365bd..a491bae33 100644 --- a/.forge-snapshots/donate gas with 2 tokens.snap +++ b/.forge-snapshots/donate gas with 2 tokens.snap @@ -1 +1 @@ -145766 \ No newline at end of file +145748 \ No newline at end of file diff --git a/.forge-snapshots/erc20 collect protocol fees.snap b/.forge-snapshots/erc20 collect protocol fees.snap index 7606bacbf..91a7b5c6f 100644 --- a/.forge-snapshots/erc20 collect protocol fees.snap +++ b/.forge-snapshots/erc20 collect protocol fees.snap @@ -1 +1 @@ -57454 \ No newline at end of file +57451 \ No newline at end of file diff --git a/.forge-snapshots/extsload getFeeGrowthGlobals.snap b/.forge-snapshots/extsload getFeeGrowthGlobals.snap index 5c1d4541a..fc0b67d07 100644 --- a/.forge-snapshots/extsload getFeeGrowthGlobals.snap +++ b/.forge-snapshots/extsload getFeeGrowthGlobals.snap @@ -1 +1 @@ -777 \ No newline at end of file +774 \ No newline at end of file diff --git a/.forge-snapshots/extsload getPositionInfo.snap b/.forge-snapshots/extsload getPositionInfo.snap index 9f02e1c9e..397787a80 100644 --- a/.forge-snapshots/extsload getPositionInfo.snap +++ b/.forge-snapshots/extsload getPositionInfo.snap @@ -1 +1 @@ -952 \ No newline at end of file +949 \ No newline at end of file diff --git a/.forge-snapshots/extsload getTickFeeGrowthOutside.snap b/.forge-snapshots/extsload getTickFeeGrowthOutside.snap index 5c1d4541a..fc0b67d07 100644 --- a/.forge-snapshots/extsload getTickFeeGrowthOutside.snap +++ b/.forge-snapshots/extsload getTickFeeGrowthOutside.snap @@ -1 +1 @@ -777 \ No newline at end of file +774 \ No newline at end of file diff --git a/.forge-snapshots/extsload getTickInfo.snap b/.forge-snapshots/extsload getTickInfo.snap index 9f02e1c9e..397787a80 100644 --- a/.forge-snapshots/extsload getTickInfo.snap +++ b/.forge-snapshots/extsload getTickInfo.snap @@ -1 +1 @@ -952 \ No newline at end of file +949 \ No newline at end of file diff --git a/.forge-snapshots/initialize.snap b/.forge-snapshots/initialize.snap index 450c7974e..bf8722821 100644 --- a/.forge-snapshots/initialize.snap +++ b/.forge-snapshots/initialize.snap @@ -1 +1 @@ -51536 \ No newline at end of file +51533 \ No newline at end of file diff --git a/.forge-snapshots/native collect protocol fees.snap b/.forge-snapshots/native collect protocol fees.snap index 1c6c78447..a9aafc5e5 100644 --- a/.forge-snapshots/native collect protocol fees.snap +++ b/.forge-snapshots/native collect protocol fees.snap @@ -1 +1 @@ -59726 \ No newline at end of file +59723 \ No newline at end of file diff --git a/.forge-snapshots/poolManager bytecode size.snap b/.forge-snapshots/poolManager bytecode size.snap index 37be6af7a..f06e8f4ab 100644 --- a/.forge-snapshots/poolManager bytecode size.snap +++ b/.forge-snapshots/poolManager bytecode size.snap @@ -1 +1 @@ -23621 \ No newline at end of file +23682 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap index 3ea79996d..f58102b65 100644 --- a/.forge-snapshots/removeLiquidity CA fee.snap +++ b/.forge-snapshots/removeLiquidity CA fee.snap @@ -1 +1 @@ -141219 \ No newline at end of file +141194 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with empty hook.snap b/.forge-snapshots/removeLiquidity with empty hook.snap index 07ce00e06..bac1084ca 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -130621 \ No newline at end of file +130603 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity with native token.snap b/.forge-snapshots/removeLiquidity with native token.snap index 35f7a9f26..19f882d70 100644 --- a/.forge-snapshots/removeLiquidity with native token.snap +++ b/.forge-snapshots/removeLiquidity with native token.snap @@ -1 +1 @@ -112535 \ No newline at end of file +112523 \ No newline at end of file diff --git a/.forge-snapshots/simple addLiquidity second addition same range.snap b/.forge-snapshots/simple addLiquidity second addition same range.snap index 85250c467..d3a590e01 100644 --- a/.forge-snapshots/simple addLiquidity second addition same range.snap +++ b/.forge-snapshots/simple addLiquidity second addition same range.snap @@ -1 +1 @@ -98862 \ No newline at end of file +98850 \ No newline at end of file diff --git a/.forge-snapshots/simple addLiquidity.snap b/.forge-snapshots/simple addLiquidity.snap index 16a8472c7..216625993 100644 --- a/.forge-snapshots/simple addLiquidity.snap +++ b/.forge-snapshots/simple addLiquidity.snap @@ -1 +1 @@ -161407 \ No newline at end of file +161395 \ No newline at end of file diff --git a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap index 94c9272f7..c8020f5a6 100644 --- a/.forge-snapshots/simple removeLiquidity some liquidity remains.snap +++ b/.forge-snapshots/simple removeLiquidity some liquidity remains.snap @@ -1 +1 @@ -92995 \ No newline at end of file +92986 \ No newline at end of file diff --git a/.forge-snapshots/simple removeLiquidity.snap b/.forge-snapshots/simple removeLiquidity.snap index f12c87689..85b1f5818 100644 --- a/.forge-snapshots/simple removeLiquidity.snap +++ b/.forge-snapshots/simple removeLiquidity.snap @@ -1 +1 @@ -85108 \ No newline at end of file +85099 \ No newline at end of file diff --git a/.forge-snapshots/simple swap with native.snap b/.forge-snapshots/simple swap with native.snap index 4bf8e4447..3a563cd16 100644 --- a/.forge-snapshots/simple swap with native.snap +++ b/.forge-snapshots/simple swap with native.snap @@ -1 +1 @@ -108515 \ No newline at end of file +108434 \ No newline at end of file diff --git a/.forge-snapshots/simple swap.snap b/.forge-snapshots/simple swap.snap index 141489c72..fcab2fe6a 100644 --- a/.forge-snapshots/simple swap.snap +++ b/.forge-snapshots/simple swap.snap @@ -1 +1 @@ -123347 \ No newline at end of file +123263 \ No newline at end of file diff --git a/.forge-snapshots/swap CA custom curve + swap noop.snap b/.forge-snapshots/swap CA custom curve + swap noop.snap index 8ad9f5434..401a560a7 100644 --- a/.forge-snapshots/swap CA custom curve + swap noop.snap +++ b/.forge-snapshots/swap CA custom curve + swap noop.snap @@ -1 +1 @@ -124653 \ No newline at end of file +124635 \ No newline at end of file diff --git a/.forge-snapshots/swap CA fee on unspecified.snap b/.forge-snapshots/swap CA fee on unspecified.snap index 6360928ab..13dcef08d 100644 --- a/.forge-snapshots/swap CA fee on unspecified.snap +++ b/.forge-snapshots/swap CA fee on unspecified.snap @@ -1 +1 @@ -154773 \ No newline at end of file +154686 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity with native token.snap b/.forge-snapshots/swap against liquidity with native token.snap index 24cd0f5c3..bb452f916 100644 --- a/.forge-snapshots/swap against liquidity with native token.snap +++ b/.forge-snapshots/swap against liquidity with native token.snap @@ -1 +1 @@ -105637 \ No newline at end of file +105569 \ No newline at end of file diff --git a/.forge-snapshots/swap against liquidity.snap b/.forge-snapshots/swap against liquidity.snap index 0452eb72f..70783815d 100644 --- a/.forge-snapshots/swap against liquidity.snap +++ b/.forge-snapshots/swap against liquidity.snap @@ -1 +1 @@ -116717 \ No newline at end of file +116646 \ No newline at end of file diff --git a/.forge-snapshots/swap burn 6909 for input.snap b/.forge-snapshots/swap burn 6909 for input.snap index 8cfb479ef..0be5b3104 100644 --- a/.forge-snapshots/swap burn 6909 for input.snap +++ b/.forge-snapshots/swap burn 6909 for input.snap @@ -1 +1 @@ -129281 \ No newline at end of file +129285 \ No newline at end of file diff --git a/.forge-snapshots/swap burn native 6909 for input.snap b/.forge-snapshots/swap burn native 6909 for input.snap index 4b0be0884..6b81ca8c9 100644 --- a/.forge-snapshots/swap burn native 6909 for input.snap +++ b/.forge-snapshots/swap burn native 6909 for input.snap @@ -1 +1 @@ -118725 \ No newline at end of file +118672 \ No newline at end of file diff --git a/.forge-snapshots/swap mint native output as 6909.snap b/.forge-snapshots/swap mint native output as 6909.snap index 63a0a75cf..d1c41e163 100644 --- a/.forge-snapshots/swap mint native output as 6909.snap +++ b/.forge-snapshots/swap mint native output as 6909.snap @@ -1 +1 @@ -139747 \ No newline at end of file +139739 \ No newline at end of file diff --git a/.forge-snapshots/swap mint output as 6909.snap b/.forge-snapshots/swap mint output as 6909.snap index 72228b87c..b5ec0883d 100644 --- a/.forge-snapshots/swap mint output as 6909.snap +++ b/.forge-snapshots/swap mint output as 6909.snap @@ -1 +1 @@ -155188 \ No newline at end of file +155104 \ No newline at end of file diff --git a/.forge-snapshots/swap skips hook call if hook is caller.snap b/.forge-snapshots/swap skips hook call if hook is caller.snap index f1e126934..403b7eb63 100644 --- a/.forge-snapshots/swap skips hook call if hook is caller.snap +++ b/.forge-snapshots/swap skips hook call if hook is caller.snap @@ -1 +1 @@ -206403 \ No newline at end of file +206251 \ No newline at end of file diff --git a/.forge-snapshots/swap with dynamic fee.snap b/.forge-snapshots/swap with dynamic fee.snap index feaacc0ab..2f7c85061 100644 --- a/.forge-snapshots/swap with dynamic fee.snap +++ b/.forge-snapshots/swap with dynamic fee.snap @@ -1 +1 @@ -139368 \ No newline at end of file +139284 \ No newline at end of file diff --git a/.forge-snapshots/swap with hooks.snap b/.forge-snapshots/swap with hooks.snap index f36933737..e215016a8 100644 --- a/.forge-snapshots/swap with hooks.snap +++ b/.forge-snapshots/swap with hooks.snap @@ -1 +1 @@ -132345 \ No newline at end of file +132274 \ No newline at end of file diff --git a/.forge-snapshots/swap with lp fee and protocol fee.snap b/.forge-snapshots/swap with lp fee and protocol fee.snap deleted file mode 100644 index 8c298d1fe..000000000 --- a/.forge-snapshots/swap with lp fee and protocol fee.snap +++ /dev/null @@ -1 +0,0 @@ -169593 \ No newline at end of file diff --git a/.forge-snapshots/swap with return dynamic fee.snap b/.forge-snapshots/swap with return dynamic fee.snap index bcbae20f7..fd902a4a0 100644 --- a/.forge-snapshots/swap with return dynamic fee.snap +++ b/.forge-snapshots/swap with return dynamic fee.snap @@ -1 +1 @@ -145661 \ No newline at end of file +145577 \ No newline at end of file diff --git a/.forge-snapshots/update dynamic fee in before swap.snap b/.forge-snapshots/update dynamic fee in before swap.snap index 8a6f38517..2bf5b6d0e 100644 --- a/.forge-snapshots/update dynamic fee in before swap.snap +++ b/.forge-snapshots/update dynamic fee in before swap.snap @@ -1 +1 @@ -147956 \ No newline at end of file +147869 \ No newline at end of file diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index bdd9b3994..fc714f6d2 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -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; diff --git a/src/libraries/ProtocolFeeLibrary.sol b/src/libraries/ProtocolFeeLibrary.sol index 48adf0fd3..512a49928 100644 --- a/src/libraries/ProtocolFeeLibrary.sol +++ b/src/libraries/ProtocolFeeLibrary.sol @@ -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)) } } } diff --git a/test/DynamicFees.t.sol b/test/DynamicFees.t.sol index cd8a3b332..5cab241b3 100644 --- a/test/DynamicFees.t.sol +++ b/test/DynamicFees.t.sol @@ -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( @@ -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 { @@ -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); @@ -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); } diff --git a/test/libraries/ProtocolFeeLibrary.t.sol b/test/libraries/ProtocolFeeLibrary.t.sol index ba341a4d6..043ffd214 100644 --- a/test/libraries/ProtocolFeeLibrary.t.sol +++ b/test/libraries/ProtocolFeeLibrary.t.sol @@ -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)); } }