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.

Sign up for GitHub

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

Jump to bottom

add swap to price tests #904

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

add swap to price tests #904

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
20d5855
add swap to price tests
snreynolds Oct 15, 2024
35e2746
gas
snreynolds Oct 15, 2024
7d19a7e
fuzz test
snreynolds Oct 17, 2024
64f235a
use price, rename
snreynolds Oct 17, 2024
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
41 changes: 41 additions & 0 deletions test/PoolManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,47 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
assertEq(manager.protocolFeesAccrued(currency1), expectedProtocolFee);
}

function test_swap_toLiquidity_fromMinPrice() public {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe worth naming this one withLiquidity and the other one withoutLiquidity?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(can still include withoutLiquidity in the other test name)

PoolKey memory _key = PoolKey(currency0, currency1, 500, 10, IHooks(address(0)));
manager.initialize(_key, TickMath.MIN_SQRT_PRICE);

IPoolManager.ModifyLiquidityParams memory params =
IPoolManager.ModifyLiquidityParams({tickLower: -10, tickUpper: 10, liquidityDelta: 100e18, salt: 0});
modifyLiquidityRouter.modifyLiquidity(_key, params, ZERO_BYTES);

// zeroForOne=false to swap higher
IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams(false, 1, TickMath.MAX_SQRT_PRICE - 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you instead used -1 for amountSpecified, I think this test could be more precise about the final price:

assertEq(sqrtPriceX96, TickMath.getSqrtPriceAtTick(-10));

PoolSwapTest.TestSettings memory testSettings =
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});

(uint160 sqrtPriceX96,,,) = manager.getSlot0(_key.toId());

swapRouter.swap(_key, swapParams, testSettings, ZERO_BYTES);

(sqrtPriceX96,,,) = manager.getSlot0(_key.toId());

// The swap pushes the price to the start of liquidity.
assertEq(TickMath.getTickAtSqrtPrice(sqrtPriceX96), -10);
}

function test_swap_toPrice_fromMaxPrice() public {
PoolKey memory _key = PoolKey(currency0, currency1, 500, 10, IHooks(address(0)));
manager.initialize(_key, TickMath.MAX_SQRT_PRICE - 1);

// zeroForOne=true to swap lower
IPoolManager.SwapParams memory swapParams = IPoolManager.SwapParams(true, 1, SQRT_PRICE_1_4);
PoolSwapTest.TestSettings memory testSettings =
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});

(uint160 sqrtPriceX96,,,) = manager.getSlot0(_key.toId());
swapRouter.swap(_key, swapParams, testSettings, ZERO_BYTES);

(sqrtPriceX96,,,) = manager.getSlot0(_key.toId());

// The swap pushes the price to the sqrtPriceLimit.
assertEq(TickMath.getTickAtSqrtPrice(SQRT_PRICE_1_4), TickMath.getTickAtSqrtPrice(sqrtPriceX96));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

}

function test_donate_failsIfNotInitialized() public {
vm.expectRevert(Pool.PoolNotInitialized.selector);
donateRouter.donate(uninitializedKey, 100, 100, ZERO_BYTES);
Expand Down
Loading