posm slippage #225
posm slippage #225
Conversation
src/PositionManager.sol
Outdated
|
|
||
| if (positionConfigs[tokenId] != config.toId()) revert IncorrectPositionConfigForTokenId(tokenId); | ||
| // Note: The tokenId is used as the salt for this position, so every minted position has unique storage in the pool manager. | ||
| BalanceDelta liquidityDelta = _modifyLiquidity(config, liquidity.toInt256(), bytes32(tokenId), hookData); | ||
| if (liquidityDelta.amount0() < 0 && amount0Max < uint128(-liquidityDelta.amount0())) revert SlippageExceeded(); |
There was a problem hiding this comment.
helper func! same code is happening twice
_checkSlippage(amount, amountMax);
There was a problem hiding this comment.
is there a case where liquidityDelta < 0 ? I guess a weird hook?
There was a problem hiding this comment.
liqDelta < 0 -- majority of cases, users paying tokens to increase liquidity. where we want to ensure slippage
0 < liqDelta -- cases where fee revenue spillover needs to be collected
There was a problem hiding this comment.
right i forgot about spillover case
src/PositionManager.sol
Outdated
| if (amount0Max < uint128(-liquidityDelta.amount0())) revert SlippageExceeded(); | ||
| if (amount1Max < uint128(-liquidityDelta.amount1())) revert SlippageExceeded(); |
There was a problem hiding this comment.
why is this slippage check different from _increase? shouldn't mint and increase be the same?
There was a problem hiding this comment.
_mint is guaranteed to be negative delta so the conditional is cleaner
_increase can be positive (spill over fee revenue to be collected), where a slippage check is irrelevant
| @@ -114,7 +114,9 @@ contract IncreaseLiquidityTest is Test, PosmTestSetup, Fuzzers { | |||
|
|
|||
| // TODO: Can we make this easier to re-invest fees, so that you don't need to know the exact collect amount? | |||
| Plan memory planner = Planner.init(); | |||
| planner.add(Actions.INCREASE_LIQUIDITY, abi.encode(tokenIdAlice, config, liquidityDelta, ZERO_BYTES)); | |||
| planner.add( | |||
| Actions.INCREASE_LIQUIDITY, abi.encode(tokenIdAlice, config, liquidityDelta, 0 wei, 0 wei, ZERO_BYTES) | |||
There was a problem hiding this comment.
how come this is 0 and not MAX_SLIPPAGE_INCREASE?
There was a problem hiding this comment.
we're ~perfectly autocompounding fees in this test so a strict slippage (no tokens paid by the caller) is an additional assurance
Related Issue
Closes #177
Description of changes
Slippage parameters and checks on:
Note: observing off-by-one wei #192 for decrease and burn. i.e. minting and burning the position (without moving price) is returning original delta - 1 wei