nit: move the max liquidity invariant to the pool since it's a pool p… (

#65)

* move the oracle into a hook, remove time based stuff from pool manager

fixes #53

* partially complete implementation of the v3 oracle hook

* partially complete implementation of the v3 oracle hook

* rename, plus some documentation, plus the idea about forcing the geomean oracle to be a single pool with locked liquidity

* working on unit tests

* finish merge

* get the test to pass!

* add a mock time geomean oracle

* some #afterInitialize tests

* add an #afterInitialize tests, limit max tick spacing

* more beforeSwap tests

* use the MAX_TICK_SPACING on the pool manager, add to interface

* inheritdoc

* afterModifyPosition tests

* remove unused variables

* nit: move the max liquidity invariant to the pool since it's a pool property and not a tick property

* fix redundant sloads

* can skip tick liquidity checks on burns

* force max range liquidity in the oracle

* add gas tests

* use a cheaper formula to compute the max liquidity per tick since we call it more often

* add tests for the invariant that the new formula for
tickSpacingToMaxLiquidityPerTick relies on

* add a comment

contracts/PoolManager.sol

@@ -194,7 +194,6 @@ contract PoolManager is IPoolManager, NoDelegateCall, ERC1155, IERC1155Receiver
                 tickLower: params.tickLower,
                 tickUpper: params.tickUpper,
                 liquidityDelta: params.liquidityDelta.toInt128(),
-                maxLiquidityPerTick: Tick.tickSpacingToMaxLiquidityPerTick(key.tickSpacing),
                 tickSpacing: key.tickSpacing
             })
         );

contracts/libraries/Pool.sol

@@ -32,6 +32,9 @@ library Pool {
     /// @param tickUpper The invalid tickUpper
     error TickUpperOutOfBounds(int24 tickUpper);
 
+    /// @notice For the tick spacing, the tick has too much liquidity
+    error TickLiquidityOverflow(int24 tick);
+
     /// @notice Thrown when interacting with an uninitialized tick that must be initialized
     /// @param tick The uninitialized tick
     error TickNotInitialized(int24 tick);
@@ -99,15 +102,15 @@ library Pool {
         int24 tickUpper;
         // any change in liquidity
         int128 liquidityDelta;
-        // the max liquidity per tick
-        uint128 maxLiquidityPerTick;
         // the spacing between ticks
         int24 tickSpacing;
     }
 
     struct ModifyPositionState {
         bool flippedLower;
+        uint128 liquidityGrossAfterLower;
         bool flippedUpper;
+        uint128 liquidityGrossAfterUpper;
         uint256 feeGrowthInside0X128;
         uint256 feeGrowthInside1X128;
     }
@@ -127,25 +130,31 @@ library Pool {
             ModifyPositionState memory state;
             // if we need to update the ticks, do it
             if (params.liquidityDelta != 0) {
-                state.flippedLower = self.ticks.update(
+                (state.flippedLower, state.liquidityGrossAfterLower) = self.ticks.update(
                     params.tickLower,
                     self.slot0.tick,
                     params.liquidityDelta,
                     self.feeGrowthGlobal0X128,
                     self.feeGrowthGlobal1X128,
-                    false,
-                    params.maxLiquidityPerTick
+                    false
                 );
-                state.flippedUpper = self.ticks.update(
+                (state.flippedUpper, state.liquidityGrossAfterUpper) = self.ticks.update(
                     params.tickUpper,
                     self.slot0.tick,
                     params.liquidityDelta,
                     self.feeGrowthGlobal0X128,
                     self.feeGrowthGlobal1X128,
-                    true,
-                    params.maxLiquidityPerTick
+                    true
                 );
 
+                if (params.liquidityDelta > 0) {
+                    uint128 maxLiquidityPerTick = Tick.tickSpacingToMaxLiquidityPerTick(params.tickSpacing);
+                    if (state.liquidityGrossAfterLower > maxLiquidityPerTick)
+                        revert TickLiquidityOverflow(params.tickLower);
+                    if (state.liquidityGrossAfterUpper > maxLiquidityPerTick)
+                        revert TickLiquidityOverflow(params.tickUpper);
+                }
+
                 if (state.flippedLower) {
                     self.tickBitmap.flipTick(params.tickLower, params.tickSpacing);
                 }

contracts/libraries/Tick.sol

@@ -31,10 +31,11 @@ library Tick {
     /// @return The max liquidity per tick
     function tickSpacingToMaxLiquidityPerTick(int24 tickSpacing) internal pure returns (uint128) {
         unchecked {
-            uint24 numTicks = uint24(
-                (TickMath.maxUsableTick(tickSpacing) - TickMath.minUsableTick(tickSpacing)) / tickSpacing
-            ) + 1; // 0 tick is not counted by this
-            return type(uint128).max / numTicks;
+            return
+                uint128(
+                    (type(uint128).max * uint256(int256(tickSpacing))) /
+                        uint256(int256(TickMath.MAX_TICK * 2 + tickSpacing))
+                );
         }
     }
 
@@ -94,27 +95,24 @@ library Tick {
     /// @param feeGrowthGlobal0X128 The all-time global fee growth, per unit of liquidity, in token0
     /// @param feeGrowthGlobal1X128 The all-time global fee growth, per unit of liquidity, in token1
     /// @param upper true for updating a position's upper tick, or false for updating a position's lower tick
-    /// @param maxLiquidity The maximum liquidity allocation for a single tick
     /// @return flipped Whether the tick was flipped from initialized to uninitialized, or vice versa
+    /// @return liquidityGrossAfter The total amount of  liquidity for all positions that references the tick after the update
     function update(
         mapping(int24 => Tick.Info) storage self,
         int24 tick,
         int24 tickCurrent,
         int128 liquidityDelta,
         uint256 feeGrowthGlobal0X128,
         uint256 feeGrowthGlobal1X128,
-        bool upper,
-        uint128 maxLiquidity
-    ) internal returns (bool flipped) {
+        bool upper
+    ) internal returns (bool flipped, uint128 liquidityGrossAfter) {
         Tick.Info storage info = self[tick];
 
         uint128 liquidityGrossBefore = info.liquidityGross;
-        uint128 liquidityGrossAfter = liquidityDelta < 0
+        liquidityGrossAfter = liquidityDelta < 0
             ? liquidityGrossBefore - uint128(-liquidityDelta)
             : liquidityGrossBefore + uint128(liquidityDelta);
 
-        if (liquidityGrossAfter > maxLiquidity) revert TickLiquidityOverflow(tick);
-
         flipped = (liquidityGrossAfter == 0) != (liquidityGrossBefore == 0);
 
         if (liquidityGrossBefore == 0) {

contracts/test/TickMathTest.sol

@@ -31,4 +31,12 @@ contract TickMathTest {
     function MAX_SQRT_RATIO() external pure returns (uint160) {
         return TickMath.MAX_SQRT_RATIO;
     }
+
+    function MIN_TICK() external pure returns (int24) {
+        return TickMath.MIN_TICK;
+    }
+
+    function MAX_TICK() external pure returns (int24) {
+        return TickMath.MAX_TICK;
+    }
 }

contracts/test/TickOverflowSafetyEchidnaTest.sol

@@ -8,7 +8,6 @@ contract TickOverflowSafetyEchidnaTest {
 
     int24 private constant MIN_TICK = -16;
     int24 private constant MAX_TICK = 16;
-    uint128 private constant MAX_LIQUIDITY = type(uint128).max / 32;
 
     mapping(int24 => Tick.Info) private ticks;
     int24 private tick = 0;
@@ -42,23 +41,21 @@ contract TickOverflowSafetyEchidnaTest {
         require(tickLower > MIN_TICK);
         require(tickUpper < MAX_TICK);
         require(tickLower < tickUpper);
-        bool flippedLower = ticks.update(
+        (bool flippedLower, ) = ticks.update(
             tickLower,
             tick,
             liquidityDelta,
             feeGrowthGlobal0X128,
             feeGrowthGlobal1X128,
-            false,
-            MAX_LIQUIDITY
+            false
         );
-        bool flippedUpper = ticks.update(
+        (bool flippedUpper, ) = ticks.update(
             tickUpper,
             tick,
             liquidityDelta,
             feeGrowthGlobal0X128,
             feeGrowthGlobal1X128,
-            true,
-            MAX_LIQUIDITY
+            true
         );
 
         if (flippedLower) {

contracts/test/TickTest.sol

@@ -13,6 +13,12 @@ contract TickTest {
         return Tick.tickSpacingToMaxLiquidityPerTick(tickSpacing);
     }
 
+    function getGasCostOfTickSpacingToMaxLiquidityPerTick(int24 tickSpacing) external view returns (uint256) {
+        uint256 gasBefore = gasleft();
+        uint128 maxLiquidity = Tick.tickSpacingToMaxLiquidityPerTick(tickSpacing);
+        return gasBefore - gasleft();
+    }
+
     function setTick(int24 tick, Tick.Info memory info) external {
         ticks[tick] = info;
     }
@@ -33,19 +39,9 @@ contract TickTest {
         int128 liquidityDelta,
         uint256 feeGrowthGlobal0X128,
         uint256 feeGrowthGlobal1X128,
-        bool upper,
-        uint128 maxLiquidity
-    ) external returns (bool flipped) {
-        return
-            ticks.update(
-                tick,
-                tickCurrent,
-                liquidityDelta,
-                feeGrowthGlobal0X128,
-                feeGrowthGlobal1X128,
-                upper,
-                maxLiquidity
-            );
+        bool upper
+    ) external returns (bool flipped, uint128 liquidityGrossAfter) {
+        return ticks.update(tick, tickCurrent, liquidityDelta, feeGrowthGlobal0X128, feeGrowthGlobal1X128, upper);
     }
 
     function clear(int24 tick) external {

test/Tick.spec.ts

@@ -1,8 +1,10 @@
+import snapshotGasCost from '@uniswap/snapshot-gas-cost'
 import { ethers } from 'hardhat'
 import { BigNumber } from 'ethers'
 import { TickTest } from '../typechain/TickTest'
+import { MAX_TICK, MAX_TICK_SPACING, MIN_TICK } from './shared/constants'
 import { expect } from './shared/expect'
-import { FeeAmount, getMaxLiquidityPerTick, TICK_SPACINGS } from './shared/utilities'
+import { FeeAmount, getMaxTick, getMinTick, TICK_SPACINGS } from './shared/utilities'
 
 const MaxUint128 = BigNumber.from(2).pow(128).sub(1)
 
@@ -17,30 +19,56 @@ describe('Tick', () => {
   })
 
   describe('#tickSpacingToMaxLiquidityPerTick', () => {
-    it('returns the correct value for low fee', async () => {
+    function checkCantOverflow(tickSpacing: number, maxLiquidityPerTick: BigNumber) {
+      expect(
+        maxLiquidityPerTick.mul((getMaxTick(tickSpacing) - getMinTick(tickSpacing)) / tickSpacing + 1),
+        'max liquidity if all ticks are full'
+      ).to.be.lte(MaxUint128)
+    }
+
+    it('returns the correct value for low fee tick spacing', async () => {
       const maxLiquidityPerTick = await tickTest.tickSpacingToMaxLiquidityPerTick(TICK_SPACINGS[FeeAmount.LOW])
-      expect(maxLiquidityPerTick).to.eq('1917569901783203986719870431555990') // 110.8 bits
-      expect(maxLiquidityPerTick).to.eq(getMaxLiquidityPerTick(TICK_SPACINGS[FeeAmount.LOW]))
+      expect(maxLiquidityPerTick).to.eq('1917565579412846627735051215301243')
+      checkCantOverflow(TICK_SPACINGS[FeeAmount.LOW], maxLiquidityPerTick)
     })
-    it('returns the correct value for medium fee', async () => {
+    it('returns the correct value for medium fee tick spacing', async () => {
       const maxLiquidityPerTick = await tickTest.tickSpacingToMaxLiquidityPerTick(TICK_SPACINGS[FeeAmount.MEDIUM])
-      expect(maxLiquidityPerTick).to.eq('11505743598341114571880798222544994') // 113.1 bits
-      expect(maxLiquidityPerTick).to.eq(getMaxLiquidityPerTick(TICK_SPACINGS[FeeAmount.MEDIUM]))
+      expect(maxLiquidityPerTick).to.eq('11505069308564788430434325881101413') // 113.1 bits
+      checkCantOverflow(TICK_SPACINGS[FeeAmount.MEDIUM], maxLiquidityPerTick)
     })
-    it('returns the correct value for high fee', async () => {
+    it('returns the correct value for high fee tick spacing', async () => {
       const maxLiquidityPerTick = await tickTest.tickSpacingToMaxLiquidityPerTick(TICK_SPACINGS[FeeAmount.HIGH])
-      expect(maxLiquidityPerTick).to.eq('38350317471085141830651933667504588') // 114.7 bits
-      expect(maxLiquidityPerTick).to.eq(getMaxLiquidityPerTick(TICK_SPACINGS[FeeAmount.HIGH]))
+      expect(maxLiquidityPerTick).to.eq('38347205785278154309959589375342946') // 114.7 bits
+      checkCantOverflow(TICK_SPACINGS[FeeAmount.HIGH], maxLiquidityPerTick)
+    })
+
+    it('returns the correct value for 1', async () => {
+      const maxLiquidityPerTick = await tickTest.tickSpacingToMaxLiquidityPerTick(1)
+      expect(maxLiquidityPerTick).to.eq('191757530477355301479181766273477') // 126 bits
+      checkCantOverflow(1, maxLiquidityPerTick)
     })
     it('returns the correct value for entire range', async () => {
       const maxLiquidityPerTick = await tickTest.tickSpacingToMaxLiquidityPerTick(887272)
       expect(maxLiquidityPerTick).to.eq(MaxUint128.div(3)) // 126 bits
-      expect(maxLiquidityPerTick).to.eq(getMaxLiquidityPerTick(887272))
+      checkCantOverflow(887272, maxLiquidityPerTick)
     })
+
     it('returns the correct value for 2302', async () => {
       const maxLiquidityPerTick = await tickTest.tickSpacingToMaxLiquidityPerTick(2302)
-      expect(maxLiquidityPerTick).to.eq('441351967472034323558203122479595605') // 118 bits
-      expect(maxLiquidityPerTick).to.eq(getMaxLiquidityPerTick(2302))
+      expect(maxLiquidityPerTick).to.eq('440854192570431170114173285871668350') // 118 bits
+      checkCantOverflow(2302, maxLiquidityPerTick)
+    })
+
+    it('gas cost min tick spacing', async () => {
+      await snapshotGasCost(tickTest.getGasCostOfTickSpacingToMaxLiquidityPerTick(1))
+    })
+
+    it('gas cost 60 tick spacing', async () => {
+      await snapshotGasCost(tickTest.getGasCostOfTickSpacingToMaxLiquidityPerTick(60))
+    })
+
+    it('gas cost max tick spacing', async () => {
+      await snapshotGasCost(tickTest.getGasCostOfTickSpacingToMaxLiquidityPerTick(MAX_TICK_SPACING))
     })
   })
 
@@ -124,57 +152,56 @@ describe('Tick', () => {
 
   describe('#update', async () => {
     it('flips from zero to nonzero', async () => {
-      expect(await tickTest.callStatic.update(0, 0, 1, 0, 0, false, 3)).to.eq(true)
+      const { flipped, liquidityGrossAfter } = await tickTest.callStatic.update(0, 0, 1, 0, 0, false)
+      expect(flipped).to.eq(true)
+      expect(liquidityGrossAfter).to.eq(1)
     })
     it('does not flip from nonzero to greater nonzero', async () => {
-      await tickTest.update(0, 0, 1, 0, 0, false, 3)
-      expect(await tickTest.callStatic.update(0, 0, 1, 0, 0, false, 3)).to.eq(false)
+      await tickTest.update(0, 0, 1, 0, 0, false)
+      const { flipped, liquidityGrossAfter } = await tickTest.callStatic.update(0, 0, 1, 0, 0, false)
+      expect(flipped).to.eq(false)
+      expect(liquidityGrossAfter).to.eq(2)
     })
     it('flips from nonzero to zero', async () => {
-      await tickTest.update(0, 0, 1, 0, 0, false, 3)
-      expect(await tickTest.callStatic.update(0, 0, -1, 0, 0, false, 3)).to.eq(true)
+      await tickTest.update(0, 0, 1, 0, 0, false)
+      const { flipped, liquidityGrossAfter } = await tickTest.callStatic.update(0, 0, -1, 0, 0, false)
+      expect(flipped).to.eq(true)
+      expect(liquidityGrossAfter).to.eq(0)
     })
     it('does not flip from nonzero to lesser nonzero', async () => {
-      await tickTest.update(0, 0, 2, 0, 0, false, 3)
-      expect(await tickTest.callStatic.update(0, 0, -1, 0, 0, false, 3)).to.eq(false)
-    })
-    it('does not flip from nonzero to lesser nonzero', async () => {
-      await tickTest.update(0, 0, 2, 0, 0, false, 3)
-      expect(await tickTest.callStatic.update(0, 0, -1, 0, 0, false, 3)).to.eq(false)
-    })
-    it('reverts if total liquidity gross is greater than max', async () => {
-      await tickTest.update(0, 0, 2, 0, 0, false, 3)
-      await tickTest.update(0, 0, 1, 0, 0, true, 3)
-      await expect(tickTest.update(0, 0, 1, 0, 0, false, 3)).to.be.revertedWith('TickLiquidityOverflow(0)')
+      await tickTest.update(0, 0, 2, 0, 0, false)
+      const { flipped, liquidityGrossAfter } = await tickTest.callStatic.update(0, 0, -1, 0, 0, false)
+      expect(flipped).to.eq(false)
+      expect(liquidityGrossAfter).to.eq(1)
     })
     it('nets the liquidity based on upper flag', async () => {
-      await tickTest.update(0, 0, 2, 0, 0, false, 10)
-      await tickTest.update(0, 0, 1, 0, 0, true, 10)
-      await tickTest.update(0, 0, 3, 0, 0, true, 10)
-      await tickTest.update(0, 0, 1, 0, 0, false, 10)
+      await tickTest.update(0, 0, 2, 0, 0, false)
+      await tickTest.update(0, 0, 1, 0, 0, true)
+      await tickTest.update(0, 0, 3, 0, 0, true)
+      await tickTest.update(0, 0, 1, 0, 0, false)
       const { liquidityGross, liquidityNet } = await tickTest.ticks(0)
       expect(liquidityGross).to.eq(2 + 1 + 3 + 1)
       expect(liquidityNet).to.eq(2 - 1 - 3 + 1)
     })
     it('reverts on overflow liquidity gross', async () => {
-      await tickTest.update(0, 0, MaxUint128.div(2).sub(1), 0, 0, false, MaxUint128)
-      await expect(tickTest.update(0, 0, MaxUint128.div(2).sub(1), 0, 0, false, MaxUint128)).to.be.reverted
+      await tickTest.update(0, 0, MaxUint128.div(2).sub(1), 0, 0, false)
+      await expect(tickTest.update(0, 0, MaxUint128.div(2).sub(1), 0, 0, false)).to.be.reverted
     })
     it('assumes all growth happens below ticks lte current tick', async () => {
-      await tickTest.update(1, 1, 1, 1, 2, false, MaxUint128)
+      await tickTest.update(1, 1, 1, 1, 2, false)
       const { feeGrowthOutside0X128, feeGrowthOutside1X128 } = await tickTest.ticks(1)
       expect(feeGrowthOutside0X128).to.eq(1)
       expect(feeGrowthOutside1X128).to.eq(2)
     })
     it('does not set any growth fields if tick is already initialized', async () => {
-      await tickTest.update(1, 1, 1, 1, 2, false, MaxUint128)
-      await tickTest.update(1, 1, 1, 6, 7, false, MaxUint128)
+      await tickTest.update(1, 1, 1, 1, 2, false)
+      await tickTest.update(1, 1, 1, 6, 7, false)
       const { feeGrowthOutside0X128, feeGrowthOutside1X128 } = await tickTest.ticks(1)
       expect(feeGrowthOutside0X128).to.eq(1)
       expect(feeGrowthOutside1X128).to.eq(2)
     })
     it('does not set any growth fields for ticks gt current tick', async () => {
-      await tickTest.update(2, 1, 1, 1, 2, false, MaxUint128)
+      await tickTest.update(2, 1, 1, 1, 2, false)
       const { feeGrowthOutside0X128, feeGrowthOutside1X128 } = await tickTest.ticks(2)
       expect(feeGrowthOutside0X128).to.eq(0)
       expect(feeGrowthOutside1X128).to.eq(0)