From a4e4c8134f4b72991f830249598c3372ec80cac8 Mon Sep 17 00:00:00 2001 From: gretzke Date: Tue, 4 Jun 2024 01:18:17 +0200 Subject: [PATCH 1/3] measure global fee growth and fees collected after fuzzing and make sure it's equal between v3 and v4 --- src/test/Fuzzers.sol | 4 +-- src/test/PoolModifyLiquidityTest.sol | 14 ++++---- test/PoolManager.swap.t.sol | 52 +++++++++++++++++++++++----- test/utils/V3Helper.sol | 9 +++++ 4 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/test/Fuzzers.sol b/src/test/Fuzzers.sol index eccf55b57..dffa62938 100644 --- a/src/test/Fuzzers.sol +++ b/src/test/Fuzzers.sol @@ -165,7 +165,7 @@ contract Fuzzers is StdUtils { bytes memory hookData ) internal returns (IPoolManager.ModifyLiquidityParams memory result, BalanceDelta delta) { result = createFuzzyLiquidityParams(key, params, sqrtPriceX96); - delta = modifyLiquidityRouter.modifyLiquidity(key, result, hookData); + (delta,) = modifyLiquidityRouter.modifyLiquidity(key, result, hookData); } // There exists possible positions in the pool, so we tighten the boundaries of liquidity. @@ -178,6 +178,6 @@ contract Fuzzers is StdUtils { uint256 maxPositions ) internal returns (IPoolManager.ModifyLiquidityParams memory result, BalanceDelta delta) { result = createFuzzyLiquidityParamsWithTightBound(key, params, sqrtPriceX96, maxPositions); - delta = modifyLiquidityRouter.modifyLiquidity(key, result, hookData); + (delta,) = modifyLiquidityRouter.modifyLiquidity(key, result, hookData); } } diff --git a/src/test/PoolModifyLiquidityTest.sol b/src/test/PoolModifyLiquidityTest.sol index 2349d3e3d..2dad19dac 100644 --- a/src/test/PoolModifyLiquidityTest.sol +++ b/src/test/PoolModifyLiquidityTest.sol @@ -35,8 +35,8 @@ contract PoolModifyLiquidityTest is PoolTestBase { PoolKey memory key, IPoolManager.ModifyLiquidityParams memory params, bytes memory hookData - ) external payable returns (BalanceDelta delta) { - delta = modifyLiquidity(key, params, hookData, false, false); + ) external payable returns (BalanceDelta delta, BalanceDelta feesAccrued) { + (delta, feesAccrued) = modifyLiquidity(key, params, hookData, false, false); } function modifyLiquidity( @@ -45,10 +45,10 @@ contract PoolModifyLiquidityTest is PoolTestBase { bytes memory hookData, bool settleUsingBurn, bool takeClaims - ) public payable returns (BalanceDelta delta) { - delta = abi.decode( + ) public payable returns (BalanceDelta delta, BalanceDelta feesAccrued) { + (delta, feesAccrued) = abi.decode( manager.unlock(abi.encode(CallbackData(msg.sender, key, params, hookData, settleUsingBurn, takeClaims))), - (BalanceDelta) + (BalanceDelta, BalanceDelta) ); uint256 ethBalance = address(this).balance; @@ -66,7 +66,7 @@ contract PoolModifyLiquidityTest is PoolTestBase { data.key.toId(), address(this), data.params.tickLower, data.params.tickUpper, data.params.salt ).liquidity; - (BalanceDelta delta,) = manager.modifyLiquidity(data.key, data.params, data.hookData); + (BalanceDelta delta, BalanceDelta feesAccrued) = manager.modifyLiquidity(data.key, data.params, data.hookData); uint128 liquidityAfter = manager.getPosition( data.key.toId(), address(this), data.params.tickLower, data.params.tickUpper, data.params.salt @@ -92,6 +92,6 @@ contract PoolModifyLiquidityTest is PoolTestBase { if (delta0 > 0) data.key.currency0.take(manager, data.sender, uint256(delta0), data.takeClaims); if (delta1 > 0) data.key.currency1.take(manager, data.sender, uint256(delta1), data.takeClaims); - return abi.encode(delta); + return abi.encode(delta, feesAccrued); } } diff --git a/test/PoolManager.swap.t.sol b/test/PoolManager.swap.t.sol index 5062bf993..ddc21d13c 100644 --- a/test/PoolManager.swap.t.sol +++ b/test/PoolManager.swap.t.sol @@ -15,9 +15,15 @@ import {SqrtPriceMath} from "../src/libraries/SqrtPriceMath.sol"; import {TickMath} from "../src/libraries/TickMath.sol"; import {SafeCast} from "../src/libraries/SafeCast.sol"; import {LiquidityAmounts} from "./utils/LiquidityAmounts.sol"; +import {StateLibrary} from "../src/libraries/StateLibrary.sol"; +import {PoolIdLibrary} from "../src/types/PoolId.sol"; abstract contract V3Fuzzer is V3Helper, Deployers, Fuzzers, IUniswapV3MintCallback, IUniswapV3SwapCallback { using CurrencyLibrary for Currency; + using StateLibrary for IPoolManager; + using PoolIdLibrary for PoolKey; + + IPoolManager.ModifyLiquidityParams[] liquidityParams; function setUp() public virtual override { super.setUp(); @@ -75,11 +81,12 @@ abstract contract V3Fuzzer is V3Helper, Deployers, Fuzzers, IUniswapV3MintCallba ); modifyLiquidityRouter.modifyLiquidity(key_, v4LiquidityParams, ""); + liquidityParams.push(v4LiquidityParams); } function swap(IUniswapV3Pool pool, PoolKey memory key_, bool zeroForOne, int128 amountSpecified) internal - returns (int256 amount0Diff, int256 amount1Diff) + returns (int256 amount0Diff, int256 amount1Diff, bool overflows) { if (amountSpecified == 0) amountSpecified = 1; if (amountSpecified == type(int128).min) amountSpecified = type(int128).min + 1; @@ -93,7 +100,6 @@ abstract contract V3Fuzzer is V3Helper, Deployers, Fuzzers, IUniswapV3MintCallba "" ); // v3 can handle bigger numbers than v4, so if we exceed int128, check that the next call reverts - bool overflows = false; if ( amount0Delta > type(int128).max || amount1Delta > type(int128).max || amount0Delta < type(int128).min || amount1Delta < type(int128).min @@ -134,6 +140,30 @@ abstract contract V3Fuzzer is V3Helper, Deployers, Fuzzers, IUniswapV3MintCallba if (amount0Delta > 0) currency0.transfer(msg.sender, uint256(amount0Delta)); if (amount1Delta > 0) currency1.transfer(msg.sender, uint256(amount1Delta)); } + + function verifyFees(IUniswapV3Pool pool, PoolKey memory key_) internal { + (uint256 v4FeeGrowth0, uint256 v4FeeGrowth1) = manager.getFeeGrowthGlobals(key_.toId()); + uint256 v3FeeGrowth0 = pool.feeGrowthGlobal0X128(); + uint256 v3FeeGrowth1 = pool.feeGrowthGlobal1X128(); + assertEq(v4FeeGrowth0, v3FeeGrowth0); + assertEq(v4FeeGrowth1, v3FeeGrowth1); + + BalanceDelta v3FeesTotal = toBalanceDelta(0, 0); + BalanceDelta v4FeesTotal = toBalanceDelta(0, 0); + uint256 len = liquidityParams.length; + for (uint256 i = 0; i < len; ++i) { + IPoolManager.ModifyLiquidityParams memory params = liquidityParams[i]; + pool.burn(params.tickLower, params.tickUpper, 0); + (uint128 v3Amount0, uint128 v3Amount1) = + pool.collect(address(this), params.tickLower, params.tickUpper, type(uint128).max, type(uint128).max); + v3FeesTotal = v3FeesTotal + toBalanceDelta(int128(v3Amount0), int128(v3Amount1)); + params.liquidityDelta = 0; + (, BalanceDelta feesAccrued) = modifyLiquidityRouter.modifyLiquidity(key_, params, ""); + + v4FeesTotal = v4FeesTotal + feesAccrued; + } + assertTrue(v3FeesTotal == v4FeesTotal); + } } contract V3SwapTests is V3Fuzzer { @@ -150,9 +180,11 @@ contract V3SwapTests is V3Fuzzer { (IUniswapV3Pool pool, PoolKey memory key_, uint160 sqrtPriceX96) = initPools(feeSeed, tickSpacingSeed, sqrtPriceX96seed); addLiquidity(pool, key_, sqrtPriceX96, lowerTickUnsanitized, upperTickUnsanitized, liquidityDeltaUnbound, false); - (int256 amount0Diff, int256 amount1Diff) = swap(pool, key_, zeroForOne, swapAmount); + (int256 amount0Diff, int256 amount1Diff, bool overflows) = swap(pool, key_, zeroForOne, swapAmount); + if (overflows) return; assertEq(amount0Diff, 0); assertEq(amount1Diff, 0); + verifyFees(pool, key_); } struct TightLiquidityParams { @@ -164,28 +196,30 @@ contract V3SwapTests is V3Fuzzer { function test_shouldSwapEqualMultipleLP( uint24 feeSeed, int24 tickSpacingSeed, - TightLiquidityParams[] memory liquidityParams, + TightLiquidityParams[] memory liquidityParams_, int256 sqrtPriceX96seed, int128 swapAmount, bool zeroForOne ) public { (IUniswapV3Pool pool, PoolKey memory key_, uint160 sqrtPriceX96) = initPools(feeSeed, tickSpacingSeed, sqrtPriceX96seed); - for (uint256 i = 0; i < liquidityParams.length; ++i) { + for (uint256 i = 0; i < liquidityParams_.length; ++i) { if (i == 20) break; addLiquidity( pool, key_, sqrtPriceX96, - liquidityParams[i].lowerTickUnsanitized, - liquidityParams[i].upperTickUnsanitized, - liquidityParams[i].liquidityDeltaUnbound, + liquidityParams_[i].lowerTickUnsanitized, + liquidityParams_[i].upperTickUnsanitized, + liquidityParams_[i].liquidityDeltaUnbound, true ); } - (int256 amount0Diff, int256 amount1Diff) = swap(pool, key_, zeroForOne, swapAmount); + (int256 amount0Diff, int256 amount1Diff, bool overflows) = swap(pool, key_, zeroForOne, swapAmount); + if (overflows) return; assertEq(amount0Diff, 0); assertEq(amount1Diff, 0); + verifyFees(pool, key_); } } diff --git a/test/utils/V3Helper.sol b/test/utils/V3Helper.sol index fed050567..5970c7d8d 100644 --- a/test/utils/V3Helper.sol +++ b/test/utils/V3Helper.sol @@ -23,6 +23,15 @@ interface IUniswapV3Pool { uint160 sqrtPriceLimitX96, bytes calldata data ) external returns (int256 amount0, int256 amount1); + function collect( + address recipient, + int24 tickLower, + int24 tickUpper, + uint128 amount0Requested, + uint128 amount1Requested + ) external returns (uint128 amount0, uint128 amount1); + function feeGrowthGlobal0X128() external view returns (uint256); + function feeGrowthGlobal1X128() external view returns (uint256); } interface IUniswapV3MintCallback { From 0fac8103d046eeebb08bc7fd3d4eafb5a38ca4e7 Mon Sep 17 00:00:00 2001 From: gretzke Date: Tue, 4 Jun 2024 02:21:51 +0200 Subject: [PATCH 2/3] fix compilation --- test/ModifyLiquidity.t.sol | 6 +++--- test/PoolManager.t.sol | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/ModifyLiquidity.t.sol b/test/ModifyLiquidity.t.sol index 5cf4687b5..08329fc3a 100644 --- a/test/ModifyLiquidity.t.sol +++ b/test/ModifyLiquidity.t.sol @@ -60,7 +60,7 @@ contract ModifyLiquidityTest is Test, Logger, Deployers, JavascriptFfi, Fuzzers, logParams(params); - (BalanceDelta delta) = modifyLiquidityRouter.modifyLiquidity(simpleKey, params, ZERO_BYTES); + (BalanceDelta delta,) = modifyLiquidityRouter.modifyLiquidity(simpleKey, params, ZERO_BYTES); (int128 jsDelta0, int128 jsDelta1) = _modifyLiquidityJS(simplePoolId, params); @@ -85,7 +85,7 @@ contract ModifyLiquidityTest is Test, Logger, Deployers, JavascriptFfi, Fuzzers, salt: 0 }); - (BalanceDelta delta) = modifyLiquidityRouter.modifyLiquidity(wp0, params, ZERO_BYTES); + (BalanceDelta delta,) = modifyLiquidityRouter.modifyLiquidity(wp0, params, ZERO_BYTES); (int128 jsDelta0, int128 jsDelta1) = _modifyLiquidityJS(wpId0, params); @@ -112,7 +112,7 @@ contract ModifyLiquidityTest is Test, Logger, Deployers, JavascriptFfi, Fuzzers, params.tickLower = 10; - (BalanceDelta delta) = modifyLiquidityRouter.modifyLiquidity(wp0, params, ZERO_BYTES); + (BalanceDelta delta,) = modifyLiquidityRouter.modifyLiquidity(wp0, params, ZERO_BYTES); (int128 jsDelta0, int128 jsDelta1) = _modifyLiquidityJS(wpId0, params); diff --git a/test/PoolManager.t.sol b/test/PoolManager.t.sol index 23deb8d08..efe4ab4c9 100644 --- a/test/PoolManager.t.sol +++ b/test/PoolManager.t.sol @@ -171,7 +171,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { (key,) = initPool(currency0, currency1, IHooks(mockAddr), 3000, sqrtPriceX96, ZERO_BYTES); - BalanceDelta balanceDelta = modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); + (BalanceDelta balanceDelta,) = modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); bytes32 beforeSelector = MockHooks.beforeAddLiquidity.selector; bytes memory beforeParams = abi.encode(address(modifyLiquidityRouter), key, LIQUIDITY_PARAMS, ZERO_BYTES); @@ -200,7 +200,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { (key,) = initPool(currency0, currency1, IHooks(mockAddr), 3000, sqrtPriceX96, ZERO_BYTES); modifyLiquidityRouter.modifyLiquidity(key, LIQUIDITY_PARAMS, ZERO_BYTES); - BalanceDelta balanceDelta = modifyLiquidityRouter.modifyLiquidity(key, REMOVE_LIQUIDITY_PARAMS, ZERO_BYTES); + (BalanceDelta balanceDelta,) = modifyLiquidityRouter.modifyLiquidity(key, REMOVE_LIQUIDITY_PARAMS, ZERO_BYTES); bytes32 beforeSelector = MockHooks.beforeRemoveLiquidity.selector; bytes memory beforeParams = abi.encode(address(modifyLiquidityRouter), key, REMOVE_LIQUIDITY_PARAMS, ZERO_BYTES); From 0e29da81256fada0b8e89ff05de1bd82b57faca9 Mon Sep 17 00:00:00 2001 From: gretzke Date: Tue, 4 Jun 2024 17:21:40 +0200 Subject: [PATCH 3/3] regenerate gas snapshots --- .../add liquidity to already existing position with salt.snap | 2 +- .forge-snapshots/addLiquidity CA fee.snap | 2 +- .forge-snapshots/addLiquidity with empty hook.snap | 2 +- .forge-snapshots/addLiquidity with native token.snap | 2 +- .../create new liquidity to a position with salt.snap | 2 +- .forge-snapshots/removeLiquidity CA fee.snap | 2 +- .forge-snapshots/removeLiquidity with empty hook.snap | 2 +- .forge-snapshots/removeLiquidity with native token.snap | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) 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 a9eed9add..7c448f8c3 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 @@ -146557 \ No newline at end of file +146650 \ No newline at end of file diff --git a/.forge-snapshots/addLiquidity CA fee.snap b/.forge-snapshots/addLiquidity CA fee.snap index 9cf5c729f..e93293efa 100644 --- a/.forge-snapshots/addLiquidity CA fee.snap +++ b/.forge-snapshots/addLiquidity CA fee.snap @@ -1 +1 @@ -172605 \ No newline at end of file +172698 \ 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 3679da0b5..d2f2eb9e7 100644 --- a/.forge-snapshots/addLiquidity with empty hook.snap +++ b/.forge-snapshots/addLiquidity with empty hook.snap @@ -1 +1 @@ -275714 \ No newline at end of file +275807 \ 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 2ef5dba23..25e06cb1c 100644 --- a/.forge-snapshots/addLiquidity with native token.snap +++ b/.forge-snapshots/addLiquidity with native token.snap @@ -1 +1 @@ -136837 \ No newline at end of file +136931 \ 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 21edfca7b..33cfad22c 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 @@ -294762 \ No newline at end of file +294855 \ No newline at end of file diff --git a/.forge-snapshots/removeLiquidity CA fee.snap b/.forge-snapshots/removeLiquidity CA fee.snap index 0ac66b60f..872c87199 100644 --- a/.forge-snapshots/removeLiquidity CA fee.snap +++ b/.forge-snapshots/removeLiquidity CA fee.snap @@ -1 +1 @@ -142125 \ No newline at end of file +142218 \ 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 d01ca1855..48ab4692e 100644 --- a/.forge-snapshots/removeLiquidity with empty hook.snap +++ b/.forge-snapshots/removeLiquidity with empty hook.snap @@ -1 +1 @@ -131597 \ No newline at end of file +131690 \ 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 5c90b03af..a30dd3ef0 100644 --- a/.forge-snapshots/removeLiquidity with native token.snap +++ b/.forge-snapshots/removeLiquidity with native token.snap @@ -1 +1 @@ -113398 \ No newline at end of file +113473 \ No newline at end of file