add swap to price tests #904
Conversation
| @@ -808,6 +808,47 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot { | |||
| assertEq(manager.protocolFeesAccrued(currency1), expectedProtocolFee); | |||
| } | |||
|
|
|||
| function test_swap_toLiquidity_fromMinPrice() public { | |||
There was a problem hiding this comment.
maybe worth naming this one withLiquidity and the other one withoutLiquidity?
There was a problem hiding this comment.
I think toLiquidity is accurate because this test traverses the swap until reaching nonzero liquidity. The relevant limit is the output amount, whereas in the other test the relevant limit is the price.
There was a problem hiding this comment.
(can still include withoutLiquidity in the other test name)
test/PoolManager.t.sol
Outdated
| modifyLiquidityRouter.modifyLiquidity(_key, params, ZERO_BYTES); | ||
|
|
||
| // zeroForOne=false to swap higher | ||
| IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams(false, 1, TickMath.MAX_SQRT_PRICE - 1); |
There was a problem hiding this comment.
If you instead used -1 for amountSpecified, I think this test could be more precise about the final price:
assertEq(sqrtPriceX96, TickMath.getSqrtPriceAtTick(-10));
test/PoolManager.t.sol
Outdated
| (sqrtPriceX96,,,) = manager.getSlot0(_key.toId()); | ||
|
|
||
| // The swap pushes the price to the sqrtPriceLimit. | ||
| assertEq(TickMath.getTickAtSqrtPrice(SQRT_PRICE_1_4), TickMath.getTickAtSqrtPrice(sqrtPriceX96)); |
There was a problem hiding this comment.
It's weird that this is testing by tick rather than by price. Shouldn't this test instead verify that the limit price is equal to the final price?
assertEq(SQRT_PRICE_1_4, sqrtPriceX96);
| } else { | ||
| // the price is between the position, so let's just swap down | ||
| zeroForOne = true; | ||
| sqrtPriceX96Limit = TickMath.MIN_SQRT_PRICE + 1; |
There was a problem hiding this comment.
I think this is the case that's failing the fuzz test
|
|
||
| modifyLiquidityRouter.modifyLiquidity(_key, params, ZERO_BYTES); | ||
|
|
||
| IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams(zeroForOne, 1, sqrtPriceX96Limit); |
There was a problem hiding this comment.
The failure happens because the price does move slightly. It's not ideal for the test to fail probabilistically. It fails because the getTickAtSqrtPrice flips from 0 to -1.
I think that case would pass if this amountSpecified was -1, because that amount would become fee and not move the price.
There was a problem hiding this comment.
Yeah I meant to change to -1 thanks!
Related Issue
Which issue does this pull request resolve?
Description of changes