accumulate seconds per liquidity instead of liquidity (Uniswap#467)

* first remove SecondsOutside.sol

* first pass of accumulating seconds per liquidity instead of liquidity * seconds

* fix the unit tests that were incorrect before

* no mulDiv necessary

* fix echidna tests

* fix compiling

* compute and set the seconds cumulative on ticks in swap

* assert initialized

* assert back to require

* revert the change to the echidna test

* shift left before dividing instead of shifting right

* fix the parentheses

* change the computation of reciprocal to use type(uint160).max

* avoid fresh sstores in the swap that crosses ticks

* store and update seconds outside

* update comments

* add the seconds per liquidity cumulative logic in modify position

* rename the filler bits

* unit tests

* another unit test for cross

* fix the test values

* few more unit tests and some documentation nits

* snapshots

* debug run slither

* debug run slither correct command

* put the number in a constant

* get the echidna tests to work

* fix the types

* address the nits

* view function for snapshots (Uniswap#471)

* add a view function for getting the current seconds per liquidity inside snapshot and seconds inside snapshot

* add some unit tests

* more tests

* remove only

* add a few more tests

* add the cumulative tick to the tick (Uniswap#481)

* put the cumulative tick outside in the tick

* first pass ofmerging the snapshot function

* bool is cheaper, reorder to match observation with tick cumulative first

* add comment

* noDelegateCall and some gas tests

* some comment polish

* more comment polish

* unskip tests, more comments

* fix skipped full oracle test failures

* interpolation calculation for max liquidity

* few more tests

contracts/UniswapV3Pool.sol

@@ -9,7 +9,6 @@ import './libraries/LowGasSafeMath.sol';
 import './libraries/SafeCast.sol';
 import './libraries/Tick.sol';
 import './libraries/TickBitmap.sol';
-import './libraries/SecondsOutside.sol';
 import './libraries/Position.sol';
 import './libraries/Oracle.sol';
 
@@ -35,7 +34,6 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
     using SafeCast for int256;
     using Tick for mapping(int24 => Tick.Info);
     using TickBitmap for mapping(int16 => uint256);
-    using SecondsOutside for mapping(int24 => uint256);
     using Position for mapping(bytes32 => Position.Info);
     using Position for Position.Info;
     using Oracle for Oracle.Observation[65535];
@@ -96,8 +94,6 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
     /// @inheritdoc IUniswapV3PoolState
     mapping(int16 => uint256) public override tickBitmap;
     /// @inheritdoc IUniswapV3PoolState
-    mapping(int24 => uint256) public override secondsOutside;
-    /// @inheritdoc IUniswapV3PoolState
     mapping(bytes32 => Position.Info) public override positions;
     /// @inheritdoc IUniswapV3PoolState
     Oracle.Observation[65535] public override observations;
@@ -159,10 +155,81 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
     }
 
     /// @inheritdoc IUniswapV3PoolDerivedState
-    function secondsInside(int24 tickLower, int24 tickUpper) external view override noDelegateCall returns (uint32) {
+    function snapshotCumulativesInside(int24 tickLower, int24 tickUpper)
+        external
+        view
+        override
+        noDelegateCall
+        returns (
+            int56 tickCumulativeInside,
+            uint160 secondsPerLiquidityInsideX128,
+            uint32 secondsInside
+        )
+    {
         checkTicks(tickLower, tickUpper);
-        require(ticks[tickLower].liquidityGross > 0 && ticks[tickUpper].liquidityGross > 0, 'X');
-        return secondsOutside.secondsInside(tickLower, tickUpper, slot0.tick, tickSpacing, _blockTimestamp());
+
+        int56 tickCumulativeLower;
+        int56 tickCumulativeUpper;
+        uint160 secondsPerLiquidityOutsideLowerX128;
+        uint160 secondsPerLiquidityOutsideUpperX128;
+        uint32 secondsOutsideLower;
+        uint32 secondsOutsideUpper;
+
+        {
+            Tick.Info storage lower = ticks[tickLower];
+            Tick.Info storage upper = ticks[tickUpper];
+            bool initializedLower;
+            (tickCumulativeLower, secondsPerLiquidityOutsideLowerX128, secondsOutsideLower, initializedLower) = (
+                lower.tickCumulativeOutside,
+                lower.secondsPerLiquidityOutsideX128,
+                lower.secondsOutside,
+                lower.initialized
+            );
+            require(initializedLower);
+
+            bool initializedUpper;
+            (tickCumulativeUpper, secondsPerLiquidityOutsideUpperX128, secondsOutsideUpper, initializedUpper) = (
+                upper.tickCumulativeOutside,
+                upper.secondsPerLiquidityOutsideX128,
+                upper.secondsOutside,
+                upper.initialized
+            );
+            require(initializedUpper);
+        }
+
+        Slot0 memory _slot0 = slot0;
+
+        if (_slot0.tick < tickLower) {
+            return (
+                tickCumulativeLower - tickCumulativeUpper,
+                secondsPerLiquidityOutsideLowerX128 - secondsPerLiquidityOutsideUpperX128,
+                secondsOutsideLower - secondsOutsideUpper
+            );
+        } else if (_slot0.tick < tickUpper) {
+            uint32 time = _blockTimestamp();
+            (int56 tickCumulative, uint160 secondsPerLiquidityCumulativeX128) =
+                observations.observeSingle(
+                    time,
+                    0,
+                    _slot0.tick,
+                    _slot0.observationIndex,
+                    liquidity,
+                    _slot0.observationCardinality
+                );
+            return (
+                tickCumulative - tickCumulativeLower - tickCumulativeUpper,
+                secondsPerLiquidityCumulativeX128 -
+                    secondsPerLiquidityOutsideLowerX128 -
+                    secondsPerLiquidityOutsideUpperX128,
+                time - secondsOutsideLower - secondsOutsideUpper
+            );
+        } else {
+            return (
+                tickCumulativeUpper - tickCumulativeLower,
+                secondsPerLiquidityOutsideUpperX128 - secondsPerLiquidityOutsideLowerX128,
+                secondsOutsideUpper - secondsOutsideLower
+            );
+        }
     }
 
     /// @inheritdoc IUniswapV3PoolDerivedState
@@ -171,7 +238,7 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
         view
         override
         noDelegateCall
-        returns (int56[] memory tickCumulatives, uint160[] memory liquidityCumulatives)
+        returns (int56[] memory tickCumulatives, uint160[] memory secondsPerLiquidityCumulativeX128s)
     {
         return
             observations.observe(
@@ -325,14 +392,26 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
         bool flippedLower;
         bool flippedUpper;
         if (liquidityDelta != 0) {
-            uint32 blockTimestamp = _blockTimestamp();
+            uint32 time = _blockTimestamp();
+            (int56 tickCumulative, uint160 secondsPerLiquidityCumulativeX128) =
+                observations.observeSingle(
+                    time,
+                    0,
+                    slot0.tick,
+                    slot0.observationIndex,
+                    liquidity,
+                    slot0.observationCardinality
+                );
 
             flippedLower = ticks.update(
                 tickLower,
                 tick,
                 liquidityDelta,
                 _feeGrowthGlobal0X128,
                 _feeGrowthGlobal1X128,
+                secondsPerLiquidityCumulativeX128,
+                tickCumulative,
+                time,
                 false,
                 maxLiquidityPerTick
             );
@@ -342,17 +421,18 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
                 liquidityDelta,
                 _feeGrowthGlobal0X128,
                 _feeGrowthGlobal1X128,
+                secondsPerLiquidityCumulativeX128,
+                tickCumulative,
+                time,
                 true,
                 maxLiquidityPerTick
             );
 
             if (flippedLower) {
                 tickBitmap.flipTick(tickLower, tickSpacing);
-                secondsOutside.initialize(tickLower, tick, tickSpacing, blockTimestamp);
             }
             if (flippedUpper) {
                 tickBitmap.flipTick(tickUpper, tickSpacing);
-                secondsOutside.initialize(tickUpper, tick, tickSpacing, blockTimestamp);
             }
         }
 
@@ -365,11 +445,9 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
         if (liquidityDelta < 0) {
             if (flippedLower) {
                 ticks.clear(tickLower);
-                secondsOutside.clear(tickLower, tickSpacing);
             }
             if (flippedUpper) {
                 ticks.clear(tickUpper);
-                secondsOutside.clear(tickUpper, tickSpacing);
             }
         }
     }
@@ -471,6 +549,12 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
         uint128 liquidityStart;
         // the timestamp of the current block
         uint32 blockTimestamp;
+        // the current value of the tick accumulator, computed only if we cross an initialized tick
+        int56 tickCumulative;
+        // the current value of seconds per liquidity accumulator, computed only if we cross an initialized tick
+        uint160 secondsPerLiquidityCumulativeX128;
+        // whether we've computed and cached the above two accumulators
+        bool computedLatestObservation;
     }
 
     // the top level state of the swap, the results of which are recorded in storage at the end
@@ -534,7 +618,10 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
             SwapCache({
                 liquidityStart: liquidity,
                 blockTimestamp: _blockTimestamp(),
-                feeProtocol: zeroForOne ? (slot0Start.feeProtocol % 16) : (slot0Start.feeProtocol >> 4)
+                feeProtocol: zeroForOne ? (slot0Start.feeProtocol % 16) : (slot0Start.feeProtocol >> 4),
+                secondsPerLiquidityCumulativeX128: 0,
+                tickCumulative: 0,
+                computedLatestObservation: false
             });
 
         bool exactInput = amountSpecified > 0;
@@ -606,18 +693,32 @@ contract UniswapV3Pool is IUniswapV3Pool, NoDelegateCall {
             if (state.sqrtPriceX96 == step.sqrtPriceNextX96) {
                 // if the tick is initialized, run the tick transition
                 if (step.initialized) {
+                    // check for the placeholder value, which we replace with the actual value the first time the swap
+                    // crosses an initialized tick
+                    if (!cache.computedLatestObservation) {
+                        (cache.tickCumulative, cache.secondsPerLiquidityCumulativeX128) = observations.observeSingle(
+                            cache.blockTimestamp,
+                            0,
+                            slot0Start.tick,
+                            slot0Start.observationIndex,
+                            cache.liquidityStart,
+                            slot0Start.observationCardinality
+                        );
+                        cache.computedLatestObservation = true;
+                    }
                     int128 liquidityNet =
                         ticks.cross(
                             step.tickNext,
                             (zeroForOne ? state.feeGrowthGlobalX128 : feeGrowthGlobal0X128),
-                            (zeroForOne ? feeGrowthGlobal1X128 : state.feeGrowthGlobalX128)
+                            (zeroForOne ? feeGrowthGlobal1X128 : state.feeGrowthGlobalX128),
+                            cache.secondsPerLiquidityCumulativeX128,
+                            cache.tickCumulative,
+                            cache.blockTimestamp
                         );
                     // if we're moving leftward, we interpret liquidityNet as the opposite sign
                     // safe because liquidityNet cannot be type(int128).min
                     if (zeroForOne) liquidityNet = -liquidityNet;
 
-                    secondsOutside.cross(step.tickNext, tickSpacing, cache.blockTimestamp);
-
                     state.liquidity = LiquidityMath.addDelta(state.liquidity, liquidityNet);
                 }
 

contracts/interfaces/pool/IUniswapV3PoolDerivedState.sol

@@ -5,16 +5,6 @@ pragma solidity >=0.5.0;
 /// @notice Contains view functions to provide information about the pool that is computed rather than stored on the
 /// blockchain. The functions here may have variable gas costs.
 interface IUniswapV3PoolDerivedState {
-    /// @notice Returns a relative timestamp value representing how long, in seconds, the pool has spent between
-    /// tickLower and tickUpper
-    /// @dev This timestamp is strictly relative. To get a useful elapsed time (i.e., duration) value, the value returned
-    /// by this method should be checkpointed externally after a position is minted, and again before a position is
-    /// burned. Thus the external contract must control the lifecycle of the position.
-    /// @param tickLower The lower tick of the range for which to get the seconds inside
-    /// @param tickUpper The upper tick of the range for which to get the seconds inside
-    /// @return A relative timestamp for how long the pool spent in the tick range
-    function secondsInside(int24 tickLower, int24 tickUpper) external view returns (uint32);
-
     /// @notice Returns the cumulative tick and liquidity as of each timestamp `secondsAgo` from the current block timestamp
     /// @dev To get a time weighted average tick or liquidity-in-range, you must call this with two values, one representing
     /// the beginning of the period and another for the end of the period. E.g., to get the last hour time-weighted average tick,
@@ -23,10 +13,28 @@ interface IUniswapV3PoolDerivedState {
     /// log base sqrt(1.0001) of token1 / token0. The TickMath library can be used to go from a tick value to a ratio.
     /// @param secondsAgos From how long ago each cumulative tick and liquidity value should be returned
     /// @return tickCumulatives Cumulative tick values as of each `secondsAgos` from the current block timestamp
-    /// @return liquidityCumulatives Cumulative liquidity-in-range value as of each `secondsAgos` from the current block
+    /// @return secondsPerLiquidityCumulativeX128s Cumulative seconds per liquidity-in-range value as of each `secondsAgos` from the current block
     /// timestamp
     function observe(uint32[] calldata secondsAgos)
         external
         view
-        returns (int56[] memory tickCumulatives, uint160[] memory liquidityCumulatives);
+        returns (int56[] memory tickCumulatives, uint160[] memory secondsPerLiquidityCumulativeX128s);
+
+    /// @notice Returns a snapshot of the tick cumulative, seconds per liquidity and seconds inside a tick range
+    /// @dev Snapshots must only be compared to other snapshots, taken over a period for which a position existed.
+    /// I.e., snapshots cannot be compared if a position is not held for the entire period between when the first
+    /// snapshot is taken and the second snapshot is taken.
+    /// @param tickLower The lower tick of the range
+    /// @param tickUpper The upper tick of the range
+    /// @return tickCumulativeInside The snapshot of the tick accumulator for the range
+    /// @return secondsPerLiquidityInsideX128 The snapshot of seconds per liquidity for the range
+    /// @return secondsInside The snapshot of seconds per liquidity for the range
+    function snapshotCumulativesInside(int24 tickLower, int24 tickUpper)
+        external
+        view
+        returns (
+            int56 tickCumulativeInside,
+            uint160 secondsPerLiquidityInsideX128,
+            uint32 secondsInside
+        );
 }

contracts/interfaces/pool/IUniswapV3PoolState.sol

@@ -54,25 +54,30 @@ interface IUniswapV3PoolState {
     /// liquidityNet how much liquidity changes when the pool price crosses the tick,
     /// feeGrowthOutside0X128 the fee growth on the other side of the tick from the current tick in token0,
     /// feeGrowthOutside1X128 the fee growth on the other side of the tick from the current tick in token1,
-    /// feeGrowthOutsideX128 values can only be used if the tick is initialized,
-    /// i.e. if liquidityGross is greater than 0. In addition, these values are only relative and are used to
-    /// compute snapshots.
+    /// tickCumulativeOutside the cumulative tick value on the other side of the tick from the current tick
+    /// secondsPerLiquidityOutsideX128 the seconds spent per liquidity on the other side of the tick from the current tick,
+    /// secondsOutside the seconds spent on the other side of the tick from the current tick,
+    /// initialized Set to true if the tick is initialized, i.e. liquidityGross is greater than 0, otherwise equal to false.
+    /// Outside values can only be used if the tick is initialized, i.e. if liquidityGross is greater than 0.
+    /// In addition, these values are only relative and must be used only in comparison to previous snapshots for
+    /// a specific position.
     function ticks(int24 tick)
         external
         view
         returns (
             uint128 liquidityGross,
             int128 liquidityNet,
             uint256 feeGrowthOutside0X128,
-            uint256 feeGrowthOutside1X128
+            uint256 feeGrowthOutside1X128,
+            int56 tickCumulativeOutside,
+            uint160 secondsPerLiquidityOutsideX128,
+            uint32 secondsOutside,
+            bool initialized
         );
 
     /// @notice Returns 256 packed tick initialized boolean values. See TickBitmap for more information
     function tickBitmap(int16 wordPosition) external view returns (uint256);
 
-    /// @notice Returns 8 packed tick seconds outside values. See SecondsOutside for more information
-    function secondsOutside(int24 wordPosition) external view returns (uint256);
-
     /// @notice Returns the information about a position by the position's key
     /// @param key The position's key is a hash of a preimage composed by the owner, tickLower and tickUpper
     /// @return _liquidity The amount of liquidity in the position,
@@ -96,18 +101,16 @@ interface IUniswapV3PoolState {
     /// @dev You most likely want to use #observe() instead of this method to get an observation as of some amount of time
     /// ago, rather than at a specific index in the array.
     /// @return blockTimestamp The timestamp of the observation,
-    /// Returns tickCumulative the current tick multiplied by seconds elapsed for the life of the pool as of the
-    /// observation,
-    /// Returns liquidityCumulative the current liquidity multiplied by seconds elapsed for the life of the pool as of
-    /// the observation,
+    /// Returns tickCumulative the tick multiplied by seconds elapsed for the life of the pool as of the observation timestamp,
+    /// Returns secondsPerLiquidityCumulativeX128 the seconds per in range liquidity for the life of the pool as of the observation timestamp,
     /// Returns initialized whether the observation has been initialized and the values are safe to use
     function observations(uint256 index)
         external
         view
         returns (
             uint32 blockTimestamp,
             int56 tickCumulative,
-            uint160 liquidityCumulative,
+            uint160 secondsPerLiquidityCumulativeX128,
             bool initialized
         );
 }