Skip to content

Test/mint slippage revert rollback #522

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

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

Test/mint slippage revert rollback #522

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e134447
fix(test): correct alice balance1 token in mint transfer burn test
anxbt Apr 6, 2026
f7aa1a7
test: strengthen mint slippage revert rollback assertions
anxbt Apr 8, 2026
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
43 changes: 40 additions & 3 deletions test/position-managers/PositionManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers {
uint256 tokenId = lpm.nextTokenId();
mint(config, 1e18, ActionConstants.MSG_SENDER, ZERO_BYTES);
BalanceDelta delta = getLastDelta();
uint128 amount1Delta = uint128(-delta.amount0());
uint128 amount1Delta = uint128(-delta.amount1());

bytes memory calls =
getDecreaseEncoded(tokenId, config, 1e18, MIN_SLIPPAGE_DECREASE, amount1Delta + 1 wei, ZERO_BYTES);
Expand Down Expand Up @@ -745,7 +745,7 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers {
bytes memory calls = getBurnEncoded(tokenId, config, ZERO_BYTES);

uint256 balance0BeforeAlice = currency0.balanceOf(alice);
uint256 balance1BeforeAlice = currency0.balanceOf(alice);
uint256 balance1BeforeAlice = currency1.balanceOf(alice);

vm.prank(alice);
lpm.modifyLiquidities(calls, _deadline);
Expand Down Expand Up @@ -974,5 +974,42 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers {
assertLt(uint256(int256(deltaDecrease.amount1())), uint256(int256(-deltaMint.amount1()))); // amount1 in the second position was greater than amount1 in the first position
}

function test_mint_slippageRevert() public {}
function test_mint_slippageRevert() public {

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

New test name test_mint_slippageRevert breaks the underscore naming pattern used throughout this file (e.g., test_mint_slippage_revertAmount0, test_decreaseLiquidity_slippage_revertAmount1). Renaming to match the existing test_<action>_<scenario> convention will make the test suite easier to scan and search.

Suggested change
function test_mint_slippageRevert() public {
function test_mint_slippage_revert() public {

Copilot uses AI. Check for mistakes.
PositionConfig memory config = PositionConfig({poolKey: key, tickLower: -120, tickUpper: 120});
uint256 liquidity = 100e18;

(uint256 amount0, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity(
SQRT_PRICE_1_1,
TickMath.getSqrtPriceAtTick(config.tickLower),
TickMath.getSqrtPriceAtTick(config.tickUpper),
uint128(liquidity)
);

// Snapshot state to prove full rollback on revert.
uint256 nextTokenIdBefore = lpm.nextTokenId();
uint256 balance0Before = currency0.balanceOfSelf();
uint256 balance1Before = currency1.balanceOfSelf();

// Make both slippage limits intentionally too strict by 1 wei.
bytes memory calls = getMintEncoded(
config,
liquidity,
uint128(amount0 - 1 wei),
uint128(amount1 - 1 wei),
ActionConstants.MSG_SENDER,
ZERO_BYTES
);

// Position manager checks amount0 first for this path.
vm.expectRevert(
abi.encodeWithSelector(
SlippageCheck.MaximumAmountExceeded.selector, uint128(amount0 - 1 wei), uint128(amount0 + 1 wei)
)
Comment on lines +993 to +1007

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

The comment says the slippage limits are "too strict by 1 wei", but the code sets amount0Max/amount1Max to amount0 - 1 wei / amount1 - 1 wei while the revert expectation indicates the requested amount is amount0 + 1 wei. If the intent is “1 wei below required” (per PR description and the earlier test_mint_slippage_exactDoesNotRevert behavior), consider using amount0/amount1 as the max values (or update the comment to reflect the actual gap).

Copilot uses AI. Check for mistakes.
);
lpm.modifyLiquidities(calls, _deadline);

assertEq(lpm.nextTokenId(), nextTokenIdBefore);
assertEq(currency0.balanceOfSelf(), balance0Before);
assertEq(currency1.balanceOfSelf(), balance1Before);
}
}
Loading