Optimize liquidity flipping calculations in Pool
#602
Conversation
|
Replaces #569 |
Updated the way liquidity flipping is calculated in 'Pool.sol' to use bitwise XOR operations for efficiency. This modification improved overall bytecode size in multiple snapshots which demonstrates an optimization of gas usage in our smart contracts.
src/libraries/Pool.sol
Outdated
| int128 liquidityNet = upper ? liquidityNetBefore - liquidityDelta : liquidityNetBefore + liquidityDelta; | ||
| // Equivalent to `liquidityNet = upper ? liquidityNetBefore - liquidityDelta : liquidityNetBefore + liquidityDelta;` | ||
| // `int128 liquidityDelta` is passed from `modifyLiquidity` and should be sanitized in `PoolManager` | ||
| liquidityDelta = LiquidityMath.flipLiquidityDelta(liquidityDelta, upper); |
There was a problem hiding this comment.
would love a comment here about why liquidityDelta cannot be min int128 to show the use of the new function is safe - thats already commented on the other use
There was a problem hiding this comment.
oh ignore me thats what youre saying about the sanitization in PoolManager haha
There was a problem hiding this comment.
It's a little different. So liquidityDelta in Pool.updateTick is passed from Pool.modifyLiquidity and PoolManager.modifyLiquidity. And PoolManager.modifyLiquidity calls toInt128() on params.liquidityDelta. But after toInt128() a int128 can still be type(int128).min.
There was a problem hiding this comment.
First of all, the original ternary expressions revert on the negation of type(int128).min.
I am not sure where the comment "safe because liquidityNet cannot be type(int128).min" came from inside Pool.swap. I guess the max liquidityNet is limited by tickSpacingToMaxLiquidityPerTick.
If a type(int128).min is flipped in flipLiquidityDelta, instead of reverting, 1 << 127 will be returned which is a valid int256 but not a valid int128.
There are two usages of flipLiquidityDelta.
- When
flipLiquidityDeltais used onliquidityNetinPool.swap,liquidityNetis immediately consumed byaddDeltawhich treats it as anint256and only reverts if the resulting liquidity overflowsuint128. - When
flipLiquidityDeltais used onliquidityDeltainPool.updateTick, it is then used byliquidityNet := add(liquidityNetBefore, liquidityDelta)in the assembly block which also treats it as anint256. The validity of the sum is checked bytoInt128().
So even though the natspec of flipLiquidityDelta says "liquidityDelta should never be type(int128).min", its current usage is able to handle type(int128).min.
…eration A new function `flipLiquidityDelta` is added to optimize the process of flipping the sign of a liquidity delta if certain conditions are met. Code complexity has been reduced by avoiding the need to use unchecked blocks and applied assembly for efficiency. The refactoring also resulted in changes in the snapshot files.
…(int128).min` This commit refactors the `flipLiquidityDelta` function in `LiquidityMath` library that can now handle `type(int128).min` and returns a valid `int256`. It also modifies the use of `flipLiquidityDelta` in `Pool` library along with improved explanations with inline comments. Furthermore, it updates the tests in `LiquidityMath.t.sol` by refining the fuzzing, modifying import statements and introducing a new test case for `flipLiquidityDelta` handling `type(int128).min`.
Related Issue
Which issue does this pull request resolve?
Description of changes
A new function
LiquidityMath.flipLiquidityDeltais added to optimize the process of flipping the sign of a liquidity delta if certain conditions are met. Code complexity has been reduced by avoiding the need to use unchecked blocks and applied assembly for efficiency. The refactoring also resulted in changes in the snapshot files.