Test/mint slippage revert rollback#522
Conversation
There was a problem hiding this comment.
Pull request overview
Updates PositionManager slippage-revert tests to correctly assert token deltas/balances and adds a mint slippage revert test that verifies full state rollback (no NFT minted, balances unchanged) after a revert.
Changes:
- Fix decrease-liquidity slippage revert test to use
delta.amount1()for amount1 slippage bounds. - Fix burn/transfer test to snapshot
currency1balance (was incorrectly readingcurrency0twice). - Implement
test_mint_slippageRevertwith revert expectation plus rollback assertions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| function test_mint_slippageRevert() public {} | ||
| function test_mint_slippageRevert() public { |
There was a problem hiding this comment.
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.
| function test_mint_slippageRevert() public { | |
| function test_mint_slippage_revert() public { |
| // 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) | ||
| ) |
There was a problem hiding this comment.
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).
Related Issue
Which issue does this pull request resolve?
Description of changes
line 655