From 3f0e841b128b7b978e15d09147196dad796cb394 Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Wed, 15 May 2024 16:55:39 -0400 Subject: [PATCH 01/10] state to self --- src/libraries/Pool.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index d3f55d49f..e6b0d8049 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -470,16 +470,16 @@ library Pool { } /// @notice Donates the given amount of currency0 and currency1 to the pool - function donate(State storage state, uint256 amount0, uint256 amount1) internal returns (BalanceDelta delta) { - uint128 liquidity = state.liquidity; + function donate(State storage self, uint256 amount0, uint256 amount1) internal returns (BalanceDelta delta) { + uint128 liquidity = self.liquidity; if (liquidity == 0) revert NoLiquidityToReceiveFees(); delta = toBalanceDelta(-(amount0.toInt128()), -(amount1.toInt128())); unchecked { if (amount0 > 0) { - state.feeGrowthGlobal0X128 += FullMath.mulDiv(amount0, FixedPoint128.Q128, liquidity); + self.feeGrowthGlobal0X128 += FullMath.mulDiv(amount0, FixedPoint128.Q128, liquidity); } if (amount1 > 0) { - state.feeGrowthGlobal1X128 += FullMath.mulDiv(amount1, FixedPoint128.Q128, liquidity); + self.feeGrowthGlobal1X128 += FullMath.mulDiv(amount1, FixedPoint128.Q128, liquidity); } } } From 8e4f6cba63499160258e5caeb17b76a784e59043 Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Wed, 15 May 2024 16:56:41 -0400 Subject: [PATCH 02/10] fuzz and unit tests --- test/libraries/Pool.t.sol | 246 +++++++++++++++++++++++++++++++++----- 1 file changed, 214 insertions(+), 32 deletions(-) diff --git a/test/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index 6ce4e4ed9..e93a596a5 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -3,17 +3,17 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; import {Vm} from "forge-std/Vm.sol"; -import {Pool} from "src/libraries/Pool.sol"; -import {PoolManager} from "src/PoolManager.sol"; -import {Position} from "src/libraries/Position.sol"; -import {TickMath} from "src/libraries/TickMath.sol"; -import {TickBitmap} from "src/libraries/TickBitmap.sol"; -import {LiquidityAmounts} from "test/utils/LiquidityAmounts.sol"; -import {Constants} from "test/utils/Constants.sol"; -import {BalanceDelta} from "src/types/BalanceDelta.sol"; -import {SafeCast} from "src/libraries/SafeCast.sol"; -import {ProtocolFeeLibrary} from "src/libraries/ProtocolFeeLibrary.sol"; -import {LPFeeLibrary} from "src/libraries/LPFeeLibrary.sol"; +import {Pool} from "../../src/libraries/Pool.sol"; +import {PoolManager} from "../../src/PoolManager.sol"; +import {Position} from "../../src/libraries/Position.sol"; +import {TickMath} from "../../src/libraries/TickMath.sol"; +import {TickBitmap} from "../../src/libraries/TickBitmap.sol"; +import {LiquidityAmounts} from "../utils/LiquidityAmounts.sol"; +import {Constants} from "../utils/Constants.sol"; +import {BalanceDelta} from "../../src/types/BalanceDelta.sol"; +import {SafeCast} from "../../src/libraries/SafeCast.sol"; +import {ProtocolFeeLibrary} from "../../src/libraries/ProtocolFeeLibrary.sol"; +import {LPFeeLibrary} from "../../src/libraries/LPFeeLibrary.sol"; contract PoolTest is Test { using Pool for Pool.State; @@ -23,7 +23,7 @@ contract PoolTest is Test { uint24 constant MAX_PROTOCOL_FEE = ProtocolFeeLibrary.MAX_PROTOCOL_FEE; // 0.1% uint24 constant MAX_LP_FEE = LPFeeLibrary.MAX_LP_FEE; // 100% - function testPoolInitialize(uint160 sqrtPriceX96, uint24 protocolFee, uint24 swapFee) public { + function test_fuzz_initialize(uint160 sqrtPriceX96, uint24 protocolFee, uint24 swapFee) public { if (sqrtPriceX96 < TickMath.MIN_SQRT_PRICE || sqrtPriceX96 >= TickMath.MAX_SQRT_PRICE) { vm.expectRevert(TickMath.InvalidSqrtPrice.selector); state.initialize(sqrtPriceX96, protocolFee, swapFee); @@ -37,7 +37,104 @@ contract PoolTest is Test { } } - function testModifyLiquidity( + function test_initialize_updatesState() public { + uint160 sqrtPriceX96 = Constants.SQRT_PRICE_1_1; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + state.initialize(sqrtPriceX96, protocolFee, lpFee); + assertEq(state.slot0.sqrtPriceX96, sqrtPriceX96); + assertEq(state.slot0.protocolFee, protocolFee); + assertEq(state.slot0.tick, TickMath.getTickAtSqrtPrice(sqrtPriceX96)); + assertLt(state.slot0.tick, TickMath.MAX_TICK); + assertGt(state.slot0.tick, TickMath.MIN_TICK - 1); + } + + function test_initialize_revertsWithInvalidSqrtPrice_tooLow() public { + uint160 sqrtPriceX96 = TickMath.MIN_SQRT_PRICE - 1; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + vm.expectRevert(TickMath.InvalidSqrtPrice.selector); + state.initialize(sqrtPriceX96, protocolFee, lpFee); + } + + function test_initialize_revertsWithInvalidSqrtPrice_tooHigh() public { + uint160 sqrtPriceX96 = TickMath.MAX_SQRT_PRICE; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + vm.expectRevert(TickMath.InvalidSqrtPrice.selector); + state.initialize(sqrtPriceX96, protocolFee, lpFee); + } + + function test_initialize_revertsWithPoolAlreadyInitialized() public { + uint160 sqrtPriceX96 = TickMath.MIN_SQRT_PRICE; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + state.initialize(sqrtPriceX96, protocolFee, lpFee); + vm.expectRevert(Pool.PoolAlreadyInitialized.selector); + state.initialize(sqrtPriceX96, protocolFee, lpFee); + } + + function test_fuzz_setProtocolFee(uint160 sqrtPriceX96, uint24 protocolFee, uint24 lpFee, uint24 newProtocolFee, bool initialized) public { + if (initialized) { + test_fuzz_initialize(sqrtPriceX96, protocolFee, lpFee); + state.setProtocolFee(newProtocolFee); + assertEq(state.slot0.protocolFee, newProtocolFee); + } else { + vm.expectRevert(Pool.PoolNotInitialized.selector); + state.setProtocolFee(newProtocolFee); + } + } + + function test_setProtocolFee_setsCorrectFee() public { + uint160 sqrtPriceX96 = TickMath.MIN_SQRT_PRICE; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + uint24 newProtocolFee = 5000; + test_fuzz_initialize(sqrtPriceX96, protocolFee, lpFee); + state.setProtocolFee(newProtocolFee); + assertEq(state.slot0.protocolFee, newProtocolFee); + } + + function test_setProtocolFee_revertsWithPoolNotInitialized() public { + uint160 sqrtPriceX96 = TickMath.MIN_SQRT_PRICE; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + uint24 newProtocolFee = 5000; + vm.expectRevert(Pool.PoolNotInitialized.selector); + state.setProtocolFee(newProtocolFee); + } + + function test_fuzz_setLpFee(uint160 sqrtPriceX96, uint24 protocolFee, uint24 lpFee, uint24 newLpFee, bool initialized) public { + if (initialized) { + test_fuzz_initialize(sqrtPriceX96, protocolFee, lpFee); + state.setLPFee(newLpFee); + assertEq(state.slot0.lpFee, newLpFee); + } else { + vm.expectRevert(Pool.PoolNotInitialized.selector); + state.setLPFee(newLpFee); + } + } + + function test_setLPFee_succeedsWithNewFee() public { + uint160 sqrtPriceX96 = TickMath.MIN_SQRT_PRICE; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + uint24 newLpFee = 5000; + test_fuzz_initialize(sqrtPriceX96, protocolFee, lpFee); + state.setLPFee(newLpFee); + assertEq(state.slot0.lpFee, newLpFee); + } + + function test_setLPFee_revertsWithPoolNotInitialized() public { + uint160 sqrtPriceX96 = TickMath.MIN_SQRT_PRICE; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + uint24 newLpFee = 5000; + vm.expectRevert(Pool.PoolNotInitialized.selector); + state.setLPFee(newLpFee); + } + + function test_fuzz_modifyLiquidity( uint160 sqrtPriceX96, uint24 protocolFee, uint24 lpFee, @@ -46,8 +143,9 @@ contract PoolTest is Test { // Assumptions tested in PoolManager.t.sol params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); - testPoolInitialize(sqrtPriceX96, protocolFee, lpFee); + test_fuzz_initialize(sqrtPriceX96, protocolFee, lpFee); + // currently an empty position if (params.tickLower >= params.tickUpper) { vm.expectRevert(abi.encodeWithSelector(Pool.TicksMisordered.selector, params.tickLower, params.tickUpper)); } else if (params.tickLower < TickMath.MIN_TICK) { @@ -70,7 +168,7 @@ contract PoolTest is Test { ); } else { // We need the assumptions above to calculate this - uint256 maxInt128InTypeU256 = uint256(uint128(Constants.MAX_UINT128)); + uint256 maxInt128InTypeUint256 = uint256(uint128(type(int128).max)); (uint256 amount0, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity( sqrtPriceX96, TickMath.getSqrtPriceAtTick(params.tickLower), @@ -78,7 +176,7 @@ contract PoolTest is Test { uint128(params.liquidityDelta) ); - if ((amount0 > maxInt128InTypeU256) || (amount1 > maxInt128InTypeU256)) { + if ((amount0 > maxInt128InTypeUint256) || (amount1 > maxInt128InTypeUint256)) { vm.expectRevert(abi.encodeWithSelector(SafeCast.SafeCastOverflow.selector)); } } @@ -87,12 +185,14 @@ contract PoolTest is Test { state.modifyLiquidity(params); } - function testSwap( + function test_fuzz_swap( uint160 sqrtPriceX96, uint24 lpFee, uint16 protocolFee0, uint16 protocolFee1, - Pool.SwapParams memory params + Pool.SwapParams memory params, + int24 tickLower, + int24 tickUpper ) public { // Assumptions tested in PoolManager.t.sol params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); @@ -101,15 +201,17 @@ contract PoolTest is Test { protocolFee1 = uint16(bound(protocolFee1, 0, MAX_PROTOCOL_FEE)); uint24 protocolFee = protocolFee1 << 12 | protocolFee0; + vm.assume(tickLower < tickUpper); + // initialize and add liquidity - testModifyLiquidity( + test_fuzz_modifyLiquidity( sqrtPriceX96, protocolFee, lpFee, Pool.ModifyLiquidityParams({ owner: address(this), - tickLower: -120, - tickUpper: 120, + tickLower: tickLower, + tickUpper: tickUpper, liquidityDelta: 1e18, tickSpacing: 60, salt: 0 @@ -117,7 +219,7 @@ contract PoolTest is Test { ); Pool.Slot0 memory slot0 = state.slot0; - if (params.amountSpecified > 0 && lpFee == MAX_LP_FEE) { + if (params.amountSpecified >= 0 && lpFee == MAX_LP_FEE) { vm.expectRevert(Pool.InvalidFeeForExactOut.selector); state.swap(params); } else if (params.zeroForOne && params.amountSpecified != 0) { @@ -144,17 +246,97 @@ contract PoolTest is Test { vm.expectRevert(abi.encodeWithSelector(Pool.PriceLimitOutOfBounds.selector, params.sqrtPriceLimitX96)); state.swap(params); } - } else { - uint160 sqrtPriceBefore = state.slot0.sqrtPriceX96; - state.swap(params); + } - if (params.amountSpecified == 0) { - assertEq(sqrtPriceBefore, state.slot0.sqrtPriceX96, "amountSpecified == 0"); - } else if (params.zeroForOne) { - assertGe(state.slot0.sqrtPriceX96, params.sqrtPriceLimitX96, "zeroForOne"); - } else { - assertLe(state.slot0.sqrtPriceX96, params.sqrtPriceLimitX96, "oneForZero"); - } + uint160 sqrtPriceBefore = state.slot0.sqrtPriceX96; + state.swap(params); + + if (params.amountSpecified == 0) { + assertEq(sqrtPriceBefore, state.slot0.sqrtPriceX96, "amountSpecified == 0"); + } else if (params.zeroForOne) { + assertGe(state.slot0.sqrtPriceX96, params.sqrtPriceLimitX96, "zeroForOne"); + } else { + assertLe(state.slot0.sqrtPriceX96, params.sqrtPriceLimitX96, "oneForZero"); } + + } + + function test_swap_oneForZero_priceLessThanOrEqaulToLimit() public { + // Assumptions tested in PoolManager.t.sol + uint160 sqrtPriceX96 = Constants.SQRT_PRICE_1_1; + uint24 protocolFee = 0; + uint24 lpFee = 0; + Pool.SwapParams memory params = Pool.SwapParams({tickSpacing: -2448282, zeroForOne: false, amountSpecified: 2459, sqrtPriceLimitX96: Constants.SQRT_PRICE_2_1 }); + params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); + + // initialize and add liquidity + test_fuzz_modifyLiquidity( + sqrtPriceX96, + protocolFee, + lpFee, + Pool.ModifyLiquidityParams({ + owner: address(this), + tickLower: -120, + tickUpper: 120, + liquidityDelta: 1e18, + tickSpacing: 60, + salt: 0 + }) + ); + + state.swap(params); + + assertLe(state.slot0.sqrtPriceX96, params.sqrtPriceLimitX96, "oneForZero"); + } + + function test_swap_zeroForOne_priceGreaterThanOrEqualToLimit() public { + // Assumptions tested in PoolManager.t.sol + uint160 sqrtPriceX96 = Constants.SQRT_PRICE_1_1; + uint24 protocolFee = 0; + uint24 lpFee = 0; + Pool.SwapParams memory params = Pool.SwapParams({tickSpacing: -2448282, zeroForOne: true, amountSpecified: 2459, sqrtPriceLimitX96: Constants.SQRT_PRICE_1_2 }); + params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); + + // initialize and add liquidity + test_fuzz_modifyLiquidity( + sqrtPriceX96, + protocolFee, + lpFee, + Pool.ModifyLiquidityParams({ + owner: address(this), + tickLower: -120, + tickUpper: 120, + liquidityDelta: 1e18, + tickSpacing: 60, + salt: 0 + }) + ); + + state.swap(params); + + assertGe(state.slot0.sqrtPriceX96, params.sqrtPriceLimitX96, "zeroForOne"); + } + + function test_fuzz_donate(uint160 sqrtPriceX96, int24 tickLower, int24 tickUpper, Pool.ModifyLiquidityParams memory params, uint24 lpFee, uint24 protocolFee, uint256 amount0, uint256 amount1 + ) public { + sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE-1)); + params.tickUpper = int24(bound(params.tickUpper, TickMath.getTickAtSqrtPrice(sqrtPriceX96), TickMath.MAX_TICK)); + params.tickLower = int24(bound(params.tickLower, TickMath.MIN_TICK, TickMath.getTickAtSqrtPrice(sqrtPriceX96))); + amount0 = bound(amount0, 0, uint256(int256(type(int128).max))); + amount1 = bound(amount1, 0, uint256(int256(type(int128).max))); + + vm.expectRevert(Pool.NoLiquidityToReceiveFees.selector); + state.donate(amount0, amount1); + + test_fuzz_modifyLiquidity(sqrtPriceX96, protocolFee, lpFee, params); + state.donate(amount0, amount1); + } + + function test_fuzz_tickSpacingToMaxLiquidityPerTick(int24 tickSpacing) public { + vm.assume(tickSpacing > 0); + int24 minTick = (TickMath.MIN_TICK / tickSpacing) * tickSpacing; + int24 maxTick = (TickMath.MAX_TICK / tickSpacing) * tickSpacing; + uint24 numTicks = uint24((maxTick - minTick) / tickSpacing) + 1; + assertGe(type(uint128).max / numTicks, Pool.tickSpacingToMaxLiquidityPerTick(tickSpacing)); } } From 39cd7acc5e509d3f59bd831c5b075358c72aa0cb Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Thu, 23 May 2024 16:09:36 -0400 Subject: [PATCH 03/10] format --- test/libraries/Pool.t.sol | 49 ++++++++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/test/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index 1320ea716..c71d094c6 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -77,7 +77,13 @@ contract PoolTest is Test { state.initialize(sqrtPriceX96, protocolFee, lpFee); } - function test_fuzz_setProtocolFee(uint160 sqrtPriceX96, uint24 protocolFee, uint24 lpFee, uint24 newProtocolFee, bool initialized) public { + function test_fuzz_setProtocolFee( + uint160 sqrtPriceX96, + uint24 protocolFee, + uint24 lpFee, + uint24 newProtocolFee, + bool initialized + ) public { if (initialized) { test_fuzz_initialize(sqrtPriceX96, protocolFee, lpFee); state.setProtocolFee(newProtocolFee); @@ -107,7 +113,13 @@ contract PoolTest is Test { state.setProtocolFee(newProtocolFee); } - function test_fuzz_setLpFee(uint160 sqrtPriceX96, uint24 protocolFee, uint24 lpFee, uint24 newLpFee, bool initialized) public { + function test_fuzz_setLpFee( + uint160 sqrtPriceX96, + uint24 protocolFee, + uint24 lpFee, + uint24 newLpFee, + bool initialized + ) public { if (initialized) { test_fuzz_initialize(sqrtPriceX96, protocolFee, lpFee); state.setLPFee(newLpFee); @@ -255,7 +267,7 @@ contract PoolTest is Test { vm.expectRevert(abi.encodeWithSelector(Pool.PriceLimitOutOfBounds.selector, params.sqrtPriceLimitX96)); state.swap(params); } - } + } uint160 sqrtPriceBefore = state.slot0.sqrtPriceX96(); state.swap(params); @@ -267,7 +279,6 @@ contract PoolTest is Test { } else { assertLe(state.slot0.sqrtPriceX96(), params.sqrtPriceLimitX96, "oneForZero"); } - } function test_swap_oneForZero_priceLessThanOrEqaulToLimit() public { @@ -275,7 +286,13 @@ contract PoolTest is Test { uint160 sqrtPriceX96 = Constants.SQRT_PRICE_1_1; uint24 protocolFee = 0; uint24 lpFee = 0; - Pool.SwapParams memory params = Pool.SwapParams({tickSpacing: -2448282, zeroForOne: false, amountSpecified: 2459, sqrtPriceLimitX96: Constants.SQRT_PRICE_2_1 }); + Pool.SwapParams memory params = Pool.SwapParams({ + tickSpacing: -2448282, + zeroForOne: false, + amountSpecified: 2459, + sqrtPriceLimitX96: Constants.SQRT_PRICE_2_1, + lpFeeOverride: 0 + }); params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); // initialize and add liquidity @@ -295,7 +312,7 @@ contract PoolTest is Test { state.swap(params); - assertLe(state.slot0.sqrtPriceX96(), params.sqrtPriceLimitX96, "oneForZero"); + assertLe(state.slot0.sqrtPriceX96(), params.sqrtPriceLimitX96, "oneForZero"); } function test_swap_zeroForOne_priceGreaterThanOrEqualToLimit() public { @@ -303,7 +320,13 @@ contract PoolTest is Test { uint160 sqrtPriceX96 = Constants.SQRT_PRICE_1_1; uint24 protocolFee = 0; uint24 lpFee = 0; - Pool.SwapParams memory params = Pool.SwapParams({tickSpacing: -2448282, zeroForOne: true, amountSpecified: 2459, sqrtPriceLimitX96: Constants.SQRT_PRICE_1_2 }); + Pool.SwapParams memory params = Pool.SwapParams({ + tickSpacing: -2448282, + zeroForOne: true, + amountSpecified: 2459, + sqrtPriceLimitX96: Constants.SQRT_PRICE_1_2, + lpFeeOverride: 0 + }); params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); // initialize and add liquidity @@ -326,9 +349,17 @@ contract PoolTest is Test { assertGe(state.slot0.sqrtPriceX96(), params.sqrtPriceLimitX96, "zeroForOne"); } - function test_fuzz_donate(uint160 sqrtPriceX96, int24 tickLower, int24 tickUpper, Pool.ModifyLiquidityParams memory params, uint24 lpFee, uint24 protocolFee, uint256 amount0, uint256 amount1 + function test_fuzz_donate( + uint160 sqrtPriceX96, + int24 tickLower, + int24 tickUpper, + Pool.ModifyLiquidityParams memory params, + uint24 lpFee, + uint24 protocolFee, + uint256 amount0, + uint256 amount1 ) public { - sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE-1)); + sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); params.tickUpper = int24(bound(params.tickUpper, TickMath.getTickAtSqrtPrice(sqrtPriceX96), TickMath.MAX_TICK)); params.tickLower = int24(bound(params.tickLower, TickMath.MIN_TICK, TickMath.getTickAtSqrtPrice(sqrtPriceX96))); amount0 = bound(amount0, 0, uint256(int256(type(int128).max))); From 2163d83478b8870965789b2b4bb9ef7fa8b11807 Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Thu, 23 May 2024 19:17:20 -0400 Subject: [PATCH 04/10] coverage --- test/libraries/Pool.t.sol | 61 +++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/test/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index c71d094c6..d9d4ceaec 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -105,9 +105,6 @@ contract PoolTest is Test { } function test_setProtocolFee_revertsWithPoolNotInitialized() public { - uint160 sqrtPriceX96 = TickMath.MIN_SQRT_PRICE; - uint24 protocolFee = MAX_PROTOCOL_FEE; - uint24 lpFee = 3000; uint24 newProtocolFee = 5000; vm.expectRevert(Pool.PoolNotInitialized.selector); state.setProtocolFee(newProtocolFee); @@ -141,9 +138,6 @@ contract PoolTest is Test { } function test_setLPFee_revertsWithPoolNotInitialized() public { - uint160 sqrtPriceX96 = TickMath.MIN_SQRT_PRICE; - uint24 protocolFee = MAX_PROTOCOL_FEE; - uint24 lpFee = 3000; uint24 newLpFee = 5000; vm.expectRevert(Pool.PoolNotInitialized.selector); state.setLPFee(newLpFee); @@ -171,8 +165,11 @@ contract PoolTest is Test { vm.expectRevert(SafeCast.SafeCastOverflow.selector); } else if (params.liquidityDelta == 0) { vm.expectRevert(Position.CannotUpdateEmptyPosition.selector); + //should be old + new if greater than max + // need for both lower and upper } else if (params.liquidityDelta > int128(Pool.tickSpacingToMaxLiquidityPerTick(params.tickSpacing))) { vm.expectRevert(abi.encodeWithSelector(Pool.TickLiquidityOverflow.selector, params.tickLower)); + // only if the lower/higher got flipped } else if (params.tickLower % params.tickSpacing != 0) { vm.expectRevert( abi.encodeWithSelector(TickBitmap.TickMisaligned.selector, params.tickLower, params.tickSpacing) @@ -200,6 +197,52 @@ contract PoolTest is Test { state.modifyLiquidity(params); } + function test_modifyLiquidity_revertsWithTickLiquidityOverflow_lower() public { + uint160 sqrtPriceX96 = Constants.SQRT_PRICE_1_1; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + state.initialize(sqrtPriceX96, protocolFee, lpFee); + Pool.ModifyLiquidityParams memory params = Pool.ModifyLiquidityParams({ + owner: address(this), + tickLower: -120, + tickUpper: 120, + liquidityDelta: 1e18, + tickSpacing: 60, + salt: 0 + }); + state.ticks[-120] = Pool.TickInfo({ + liquidityGross: type(uint128).max - uint128(params.liquidityDelta), + liquidityNet: 0, + feeGrowthOutside0X128: 0, + feeGrowthOutside1X128: 0 + }); + vm.expectRevert(abi.encodeWithSelector(Pool.TickLiquidityOverflow.selector, -120)); + state.modifyLiquidity(params); + } + + function test_modifyLiquidity_revertsWithTickLiquidityOverflow_upper() public { + uint160 sqrtPriceX96 = Constants.SQRT_PRICE_1_1; + uint24 protocolFee = MAX_PROTOCOL_FEE; + uint24 lpFee = 3000; + state.initialize(sqrtPriceX96, protocolFee, lpFee); + Pool.ModifyLiquidityParams memory params = Pool.ModifyLiquidityParams({ + owner: address(this), + tickLower: -120, + tickUpper: 120, + liquidityDelta: 1e18, + tickSpacing: 60, + salt: 0 + }); + state.ticks[120] = Pool.TickInfo({ + liquidityGross: type(uint128).max - uint128(params.liquidityDelta), + liquidityNet: 0, + feeGrowthOutside0X128: 0, + feeGrowthOutside1X128: 0 + }); + vm.expectRevert(abi.encodeWithSelector(Pool.TickLiquidityOverflow.selector, 120)); + state.modifyLiquidity(params); + } + function test_fuzz_swap( uint160 sqrtPriceX96, uint24 lpFee, @@ -239,10 +282,8 @@ contract PoolTest is Test { if (params.amountSpecified >= 0 && swapFee == MAX_LP_FEE) { vm.expectRevert(Pool.InvalidFeeForExactOut.selector); - state.swap(params); } else if (!swapFee.isValid()) { vm.expectRevert(LPFeeLibrary.FeeTooLarge.selector); - state.swap(params); } else if (params.zeroForOne && params.amountSpecified != 0) { if (params.sqrtPriceLimitX96 >= slot0.sqrtPriceX96()) { vm.expectRevert( @@ -250,10 +291,8 @@ contract PoolTest is Test { Pool.PriceLimitAlreadyExceeded.selector, slot0.sqrtPriceX96(), params.sqrtPriceLimitX96 ) ); - state.swap(params); } else if (params.sqrtPriceLimitX96 <= TickMath.MIN_SQRT_PRICE) { vm.expectRevert(abi.encodeWithSelector(Pool.PriceLimitOutOfBounds.selector, params.sqrtPriceLimitX96)); - state.swap(params); } } else if (!params.zeroForOne && params.amountSpecified != 0) { if (params.sqrtPriceLimitX96 <= slot0.sqrtPriceX96()) { @@ -262,10 +301,8 @@ contract PoolTest is Test { Pool.PriceLimitAlreadyExceeded.selector, slot0.sqrtPriceX96(), params.sqrtPriceLimitX96 ) ); - state.swap(params); } else if (params.sqrtPriceLimitX96 >= TickMath.MAX_SQRT_PRICE) { vm.expectRevert(abi.encodeWithSelector(Pool.PriceLimitOutOfBounds.selector, params.sqrtPriceLimitX96)); - state.swap(params); } } From f27858627c02913cf9a5afea022324f63c30a95b Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Thu, 23 May 2024 19:18:02 -0400 Subject: [PATCH 05/10] equivalent to v3 code commented --- src/libraries/Pool.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index dd1aa6610..94536aaf8 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -568,6 +568,13 @@ library Pool { / uint256(int256(TickMath.MAX_TICK * 2 + tickSpacing)) ); } + + // unchecked { + // uint24 numTicks = uint24( + // (TickMath.maxUsableTick(tickSpacing) - TickMath.minUsableTick(tickSpacing)) / tickSpacing + // ) + 1; // 0 tick is not counted by this + // return type(uint128).max / numTicks; + // } } function isNotInitialized(State storage self) internal view returns (bool) { From 4a85dbc497e512ac61d34322cd52da1e642ea1e3 Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Fri, 24 May 2024 11:31:06 -0400 Subject: [PATCH 06/10] bound sqrt price --- test/libraries/Pool.t.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/test/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index d9d4ceaec..786a70700 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -151,6 +151,7 @@ contract PoolTest is Test { ) public { // Assumptions tested in PoolManager.t.sol params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); + sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); test_fuzz_initialize(sqrtPriceX96, protocolFee, lpFee); From 4017e1debb6750c238623dc772b104ba00f72cec Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Tue, 28 May 2024 14:51:07 -0400 Subject: [PATCH 07/10] linters --- src/libraries/Pool.sol | 2 +- test/libraries/Pool.t.sol | 45 +++++++++++++++++---------------------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/libraries/Pool.sol b/src/libraries/Pool.sol index ad872b095..b993713d2 100644 --- a/src/libraries/Pool.sol +++ b/src/libraries/Pool.sol @@ -458,7 +458,7 @@ library Pool { /// @notice Donates the given amount of currency0 and currency1 to the pool function donate(State storage self, uint256 amount0, uint256 amount1) internal returns (BalanceDelta delta) { uint128 liquidity = self.liquidity; - if (liquidity == 0) revert NoLiquidityToReceiveFees(); + if (liquidity == 0) NoLiquidityToReceiveFees.selector.revertWith(); unchecked { // negation safe as amount0 and amount1 are always positive delta = toBalanceDelta(-(amount0.toInt128()), -(amount1.toInt128())); diff --git a/test/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index 2b44d515b..3f8a57c38 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -163,17 +163,22 @@ contract PoolTest is Test, GasSnapshot { vm.expectRevert(abi.encodeWithSelector(Pool.TickLowerOutOfBounds.selector, params.tickLower)); } else if (params.tickUpper > TickMath.MAX_TICK) { vm.expectRevert(abi.encodeWithSelector(Pool.TickUpperOutOfBounds.selector, params.tickUpper)); - } else if (params.liquidityDelta < 0) { // can't remove liquidity before adding it + } else if (params.liquidityDelta < 0) { + // can't remove liquidity before adding it vm.expectRevert(SafeCast.SafeCastOverflow.selector); - } else if (params.liquidityDelta == 0) { // b/c position is empty + } else if (params.liquidityDelta == 0) { + // b/c position is empty vm.expectRevert(Position.CannotUpdateEmptyPosition.selector); - } else if (params.liquidityDelta > int128(Pool.tickSpacingToMaxLiquidityPerTick(params.tickSpacing))) { // b/c liquidity before starts at 0 + } else if (params.liquidityDelta > int128(Pool.tickSpacingToMaxLiquidityPerTick(params.tickSpacing))) { + // b/c liquidity before starts at 0 vm.expectRevert(abi.encodeWithSelector(Pool.TickLiquidityOverflow.selector, params.tickLower)); - } else if (params.tickLower % params.tickSpacing != 0) { // since tick will always be flipped first time + } else if (params.tickLower % params.tickSpacing != 0) { + // since tick will always be flipped first time vm.expectRevert( abi.encodeWithSelector(TickBitmap.TickMisaligned.selector, params.tickLower, params.tickSpacing) ); - } else if (params.tickUpper % params.tickSpacing != 0) { // since tick will always be flipped first time + } else if (params.tickUpper % params.tickSpacing != 0) { + // since tick will always be flipped first time vm.expectRevert( abi.encodeWithSelector(TickBitmap.TickMisaligned.selector, params.tickUpper, params.tickSpacing) ); @@ -311,9 +316,11 @@ contract PoolTest is Test, GasSnapshot { if (params.amountSpecified == 0) { assertEq(sqrtPriceBefore, state.slot0.sqrtPriceX96(), "amountSpecified == 0"); - } else if (params.zeroForOne) { // fuzz test not checking this, checking in unit test: test_swap_zeroForOne_priceGreaterThanOrEqualToLimit + } else if (params.zeroForOne) { + // fuzz test not checking this, checking in unit test: test_swap_zeroForOne_priceGreaterThanOrEqualToLimit assertGe(state.slot0.sqrtPriceX96(), params.sqrtPriceLimitX96, "zeroForOne"); - } else { // fuzz test not checking this, checking in unit test: test_swap_oneForZero_priceLessThanOrEqualToLimit + } else { + // fuzz test not checking this, checking in unit test: test_swap_oneForZero_priceLessThanOrEqualToLimit assertLe(state.slot0.sqrtPriceX96(), params.sqrtPriceLimitX96, "oneForZero"); } } @@ -379,9 +386,7 @@ contract PoolTest is Test, GasSnapshot { vm.expectRevert( abi.encodeWithSelector( - Pool.PriceLimitAlreadyExceeded.selector, - state.slot0.sqrtPriceX96(), - params.sqrtPriceLimitX96 + Pool.PriceLimitAlreadyExceeded.selector, state.slot0.sqrtPriceX96(), params.sqrtPriceLimitX96 ) ); state.swap(params); @@ -395,7 +400,7 @@ contract PoolTest is Test, GasSnapshot { tickSpacing: TickMath.MIN_TICK_SPACING, zeroForOne: true, amountSpecified: 2459, - sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE , + sqrtPriceLimitX96: TickMath.MIN_SQRT_PRICE, lpFeeOverride: 0 }); params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); @@ -415,12 +420,7 @@ contract PoolTest is Test, GasSnapshot { }) ); - vm.expectRevert( - abi.encodeWithSelector( - Pool.PriceLimitOutOfBounds.selector, - params.sqrtPriceLimitX96 - ) - ); + vm.expectRevert(abi.encodeWithSelector(Pool.PriceLimitOutOfBounds.selector, params.sqrtPriceLimitX96)); state.swap(params); } @@ -454,9 +454,7 @@ contract PoolTest is Test, GasSnapshot { vm.expectRevert( abi.encodeWithSelector( - Pool.PriceLimitAlreadyExceeded.selector, - state.slot0.sqrtPriceX96(), - params.sqrtPriceLimitX96 + Pool.PriceLimitAlreadyExceeded.selector, state.slot0.sqrtPriceX96(), params.sqrtPriceLimitX96 ) ); state.swap(params); @@ -490,12 +488,7 @@ contract PoolTest is Test, GasSnapshot { }) ); - vm.expectRevert( - abi.encodeWithSelector( - Pool.PriceLimitOutOfBounds.selector, - params.sqrtPriceLimitX96 - ) - ); + vm.expectRevert(abi.encodeWithSelector(Pool.PriceLimitOutOfBounds.selector, params.sqrtPriceLimitX96)); state.swap(params); } From 715b67641788ca51ca2c1b8f7374b40098087bda Mon Sep 17 00:00:00 2001 From: Alice Henshaw Date: Wed, 29 May 2024 13:03:07 -0300 Subject: [PATCH 08/10] extra verbosity in CI --- .github/workflows/tests-pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml index 9399261d3..98aab439e 100644 --- a/.github/workflows/tests-pr.yml +++ b/.github/workflows/tests-pr.yml @@ -27,7 +27,7 @@ jobs: FOUNDRY_PROFILE: pr - name: Run tests - run: forge test --isolate -vvv + run: forge test --isolate -vvvvv env: FOUNDRY_PROFILE: pr FORGE_SNAPSHOT_CHECK: true From 15cefc804c049d4b10a3047a78fa649e8fde7a37 Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Wed, 29 May 2024 12:55:06 -0400 Subject: [PATCH 09/10] fix test --- test/libraries/Pool.t.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index 3f8a57c38..24e1e18d2 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -17,6 +17,7 @@ import {ProtocolFeeLibrary} from "../../src/libraries/ProtocolFeeLibrary.sol"; import {LPFeeLibrary} from "../../src/libraries/LPFeeLibrary.sol"; import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; + contract PoolTest is Test, GasSnapshot { using Pool for Pool.State; using LPFeeLibrary for uint24; @@ -259,6 +260,7 @@ contract PoolTest is Test, GasSnapshot { ) public { // Assumptions tested in PoolManager.t.sol params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); + sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_PRICE, TickMath.MAX_SQRT_PRICE - 1)); lpFee = uint24(bound(lpFee, 0, MAX_LP_FEE)); protocolFee0 = uint16(bound(protocolFee0, 0, MAX_PROTOCOL_FEE)); protocolFee1 = uint16(bound(protocolFee1, 0, MAX_PROTOCOL_FEE)); @@ -276,7 +278,7 @@ contract PoolTest is Test, GasSnapshot { tickLower: tickLower, tickUpper: tickUpper, liquidityDelta: 1e18, - tickSpacing: 60, + tickSpacing: params.tickSpacing, salt: 0 }) ); From 2bce1e9819f9259e6429a1c2a262a2446e84b9c6 Mon Sep 17 00:00:00 2001 From: Diana Kocsis Date: Wed, 29 May 2024 13:07:05 -0400 Subject: [PATCH 10/10] extra fuzz, switch back verbosity, fix linters --- .github/workflows/tests-pr.yml | 2 +- test/libraries/Pool.t.sol | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml index 98aab439e..9399261d3 100644 --- a/.github/workflows/tests-pr.yml +++ b/.github/workflows/tests-pr.yml @@ -27,7 +27,7 @@ jobs: FOUNDRY_PROFILE: pr - name: Run tests - run: forge test --isolate -vvvvv + run: forge test --isolate -vvv env: FOUNDRY_PROFILE: pr FORGE_SNAPSHOT_CHECK: true diff --git a/test/libraries/Pool.t.sol b/test/libraries/Pool.t.sol index 24e1e18d2..0e281b91f 100644 --- a/test/libraries/Pool.t.sol +++ b/test/libraries/Pool.t.sol @@ -17,7 +17,6 @@ import {ProtocolFeeLibrary} from "../../src/libraries/ProtocolFeeLibrary.sol"; import {LPFeeLibrary} from "../../src/libraries/LPFeeLibrary.sol"; import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol"; - contract PoolTest is Test, GasSnapshot { using Pool for Pool.State; using LPFeeLibrary for uint24; @@ -256,7 +255,8 @@ contract PoolTest is Test, GasSnapshot { uint16 protocolFee1, Pool.SwapParams memory params, int24 tickLower, - int24 tickUpper + int24 tickUpper, + int128 liquidityDelta ) public { // Assumptions tested in PoolManager.t.sol params.tickSpacing = int24(bound(params.tickSpacing, TickMath.MIN_TICK_SPACING, TickMath.MAX_TICK_SPACING)); @@ -265,6 +265,7 @@ contract PoolTest is Test, GasSnapshot { protocolFee0 = uint16(bound(protocolFee0, 0, MAX_PROTOCOL_FEE)); protocolFee1 = uint16(bound(protocolFee1, 0, MAX_PROTOCOL_FEE)); uint24 protocolFee = protocolFee1 << 12 | protocolFee0; + liquidityDelta = int128(bound(liquidityDelta, 1, type(int128).max)); vm.assume(tickLower < tickUpper); @@ -277,7 +278,7 @@ contract PoolTest is Test, GasSnapshot { owner: address(this), tickLower: tickLower, tickUpper: tickUpper, - liquidityDelta: 1e18, + liquidityDelta: liquidityDelta, tickSpacing: params.tickSpacing, salt: 0 })