fix: use snapshotting library in bounded union source adapter

src/adapters/source-adapters/SnapshotSource.sol → src/adapters/lib/SnapshotSourceLib.sol

@@ -1,37 +1,36 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity 0.8.17;
 
-import {DiamondRootOval} from "../../DiamondRootOval.sol";
-
 /**
- * @title SnapshotSource contract to be used in conjunction with a source adapter that needs to snapshot historic data.
+ * @title SnapshotSourceLib library to be used by a source adapter that needs to snapshot historic data.
  */
-abstract contract SnapshotSource is DiamondRootOval {
+library SnapshotSourceLib {
     // Snapshot records the historical answer at a specific timestamp.
     struct Snapshot {
         int256 answer;
         uint256 timestamp;
     }
 
-    Snapshot[] public snapshots; // Historical answer and timestamp snapshots.
-
     event SnapshotTaken(uint256 snapshotIndex, uint256 indexed timestamp, int256 indexed answer);
 
     /**
      * @notice Returns the latest snapshot data.
+     * @param snapshots Pointer to source adapter's snapshots array.
      * @return Snapshot The latest snapshot data.
      */
-    function latestSnapshotData() public view returns (Snapshot memory) {
+    function latestSnapshotData(Snapshot[] storage snapshots) internal view returns (Snapshot memory) {
         if (snapshots.length > 0) return snapshots[snapshots.length - 1];
         return Snapshot(0, 0);
     }
 
     /**
      * @notice Snapshot the current source data.
+     * @param snapshots Pointer to source adapter's snapshots array.
+     * @param latestAnswer The latest answer from the source.
+     * @param latestTimestamp The timestamp of the latest answer from the source.
      */
-    function snapshotData() public virtual override {
-        (int256 answer, uint256 timestamp) = getLatestSourceData();
-        Snapshot memory snapshot = Snapshot(answer, timestamp);
+    function snapshotData(Snapshot[] storage snapshots, int256 latestAnswer, uint256 latestTimestamp) internal {
+        Snapshot memory snapshot = Snapshot(latestAnswer, latestTimestamp);
         if (snapshot.timestamp == 0) return; // Should not store invalid data.
 
         // We expect source timestamps to be increasing over time, but there is little we can do to recover if source
@@ -45,24 +44,34 @@ abstract contract SnapshotSource is DiamondRootOval {
         emit SnapshotTaken(snapshotIndex, snapshot.timestamp, snapshot.answer);
     }
 
-    function _tryLatestDataAt(uint256 timestamp, uint256 maxTraversal) internal view returns (Snapshot memory) {
-        (int256 answer, uint256 _timestamp) = getLatestSourceData();
-        Snapshot memory latestData = Snapshot(answer, _timestamp);
+    function _tryLatestDataAt(
+        Snapshot[] storage snapshots,
+        int256 latestAnswer,
+        uint256 latestTimestamp,
+        uint256 timestamp,
+        uint256 maxTraversal,
+        uint256 maxAge
+    ) internal view returns (Snapshot memory) {
+        Snapshot memory latestData = Snapshot(latestAnswer, latestTimestamp);
         // In the happy path there have been no source updates since requested time, so we can return the latest data.
         // We can use timestamp property as it matches the block timestamp of the latest source update.
         if (latestData.timestamp <= timestamp) return latestData;
 
         // Attempt traversing historical snapshot data. This might still be newer or uninitialized.
-        Snapshot memory historicalData = _searchSnapshotAt(timestamp, maxTraversal);
+        Snapshot memory historicalData = _searchSnapshotAt(snapshots, timestamp, maxTraversal);
 
         // Validate returned data. If it is uninitialized or too old we fallback to returning the current latest round data.
-        if (historicalData.timestamp >= block.timestamp - maxAge()) return historicalData;
+        if (historicalData.timestamp >= block.timestamp - maxAge) return historicalData;
         return latestData;
     }
 
     // Tries finding latest snapshotted data not newer than requested timestamp. Might still return newer data than
     // requested if exceeded traversal or hold uninitialized data that should be handled by the caller.
-    function _searchSnapshotAt(uint256 timestamp, uint256 maxTraversal) internal view returns (Snapshot memory) {
+    function _searchSnapshotAt(Snapshot[] storage snapshots, uint256 timestamp, uint256 maxTraversal)
+        internal
+        view
+        returns (Snapshot memory)
+    {
         Snapshot memory snapshot;
         uint256 traversedSnapshots = 0;
         uint256 snapshotId = snapshots.length; // Will decrement when entering loop.

src/adapters/source-adapters/BoundedUnionSourceAdapter.sol

@@ -6,10 +6,10 @@ import {SignedMath} from "openzeppelin-contracts/contracts/utils/math/SignedMath
 import {IAggregatorV3Source} from "../../interfaces/chainlink/IAggregatorV3Source.sol";
 import {IMedian} from "../../interfaces/chronicle/IMedian.sol";
 import {IPyth} from "../../interfaces/pyth/IPyth.sol";
+import {SnapshotSourceLib} from "../lib/SnapshotSourceLib.sol";
 import {ChainlinkSourceAdapter} from "./ChainlinkSourceAdapter.sol";
 import {ChronicleMedianSourceAdapter} from "./ChronicleMedianSourceAdapter.sol";
 import {PythSourceAdapter} from "./PythSourceAdapter.sol";
-import {SnapshotSource} from "./SnapshotSource.sol";
 
 /**
  * @title BoundedUnionSourceAdapter contract to read data from multiple sources and return the newest, contingent on it
@@ -25,8 +25,20 @@ abstract contract BoundedUnionSourceAdapter is
     ChronicleMedianSourceAdapter,
     PythSourceAdapter
 {
+    // Pack all source data into a struct to avoid stack too deep errors.
+    struct AllSourceData {
+        int256 clAnswer;
+        uint256 clTimestamp;
+        int256 crAnswer;
+        uint256 crTimestamp;
+        int256 pyAnswer;
+        uint256 pyTimestamp;
+    }
+
     uint256 public immutable BOUNDING_TOLERANCE;
 
+    SnapshotSourceLib.Snapshot[] public boundedUnionSnapshots; // Historical answer and timestamp snapshots.
+
     constructor(
         IAggregatorV3Source chainlink,
         IMedian chronicle,
@@ -48,11 +60,8 @@ abstract contract BoundedUnionSourceAdapter is
         override(ChainlinkSourceAdapter, ChronicleMedianSourceAdapter, PythSourceAdapter)
         returns (int256 answer, uint256 timestamp)
     {
-        (int256 clAnswer, uint256 clTimestamp) = ChainlinkSourceAdapter.getLatestSourceData();
-        (int256 crAnswer, uint256 crTimestamp) = ChronicleMedianSourceAdapter.getLatestSourceData();
-        (int256 pyAnswer, uint256 pyTimestamp) = PythSourceAdapter.getLatestSourceData();
-
-        return _selectBoundedPrice(clAnswer, clTimestamp, crAnswer, crTimestamp, pyAnswer, pyTimestamp);
+        AllSourceData memory data = _getAllLatestSourceData();
+        return _selectBoundedPrice(data);
     }
 
     /**
@@ -72,13 +81,15 @@ abstract contract BoundedUnionSourceAdapter is
     }
 
     /**
-     * @notice Snapshots is a no-op for this adapter as its never used.
+     * @notice Snapshot the current bounded union source data.
      */
-    function snapshotData() public override(ChainlinkSourceAdapter, SnapshotSource) {}
+    function snapshotData() public override(ChainlinkSourceAdapter, ChronicleMedianSourceAdapter, PythSourceAdapter) {
+        (int256 latestAnswer, uint256 latestTimestamp) = BoundedUnionSourceAdapter.getLatestSourceData();
+        SnapshotSourceLib.snapshotData(boundedUnionSnapshots, latestAnswer, latestTimestamp);
+    }
 
     /**
-     * @notice Tries getting latest data as of requested timestamp. Note that for all historic lookups we simply return
-     * the Chainlink data as this is the only supported source that has historical data.
+     * @notice Tries getting latest data as of requested timestamp.
      * @param timestamp The timestamp to try getting latest data at.
      * @param maxTraversal The maximum number of rounds to traverse when looking for historical data.
      * @return answer The answer as of requested timestamp, or earliest available data if not available, in 18 decimals.
@@ -91,32 +102,53 @@ abstract contract BoundedUnionSourceAdapter is
         override(ChainlinkSourceAdapter, ChronicleMedianSourceAdapter, PythSourceAdapter)
         returns (int256, uint256, uint256)
     {
-        // Chainlink has price history, so use tryLatestDataAt to pull the most recent price that satisfies the timestamp constraint.
-        (int256 clAnswer, uint256 clTimestamp,) = ChainlinkSourceAdapter.tryLatestDataAt(timestamp, maxTraversal);
+        // In the happy path there have been no source updates since requested time, so we can return the latest data.
+        AllSourceData memory data = _getAllLatestSourceData();
+        (int256 boundedAnswer, uint256 boundedTimestamp) = _selectBoundedPrice(data);
+        if (boundedTimestamp <= timestamp) return (boundedAnswer, boundedTimestamp, 1);
 
-        // For Chronicle and Pyth, just pull the most recent prices and drop them if they don't satisfy the constraint.
-        (int256 crAnswer, uint256 crTimestamp) = ChronicleMedianSourceAdapter.getLatestSourceData();
-        (int256 pyAnswer, uint256 pyTimestamp) = PythSourceAdapter.getLatestSourceData();
+        // Chainlink has price history, so use tryLatestDataAt to pull the most recent price that satisfies the timestamp constraint.
+        (data.clAnswer, data.clTimestamp,) = ChainlinkSourceAdapter.tryLatestDataAt(timestamp, maxTraversal);
 
-        // To "drop" Chronicle and Pyth, we set their timestamps to 0 (as old as possible) if they are too recent.
+        // "Drop" Chronicle and/or Pyth by setting their timestamps to 0 (as old as possible) if they are too recent.
         // This means that they will never be used if either or both are 0.
-        if (crTimestamp > timestamp) crTimestamp = 0;
-        if (pyTimestamp > timestamp) pyTimestamp = 0;
+        if (data.crTimestamp > timestamp) data.crTimestamp = 0;
+        if (data.pyTimestamp > timestamp) data.pyTimestamp = 0;
+
+        // Bounded union prices could have been captured at snapshot that satisfies time constraint.
+        SnapshotSourceLib.Snapshot memory snapshot = SnapshotSourceLib._tryLatestDataAt(
+            boundedUnionSnapshots, boundedAnswer, boundedTimestamp, timestamp, maxTraversal, maxAge()
+        );
+
+        // Update bounded data with constrained source data.
+        (boundedAnswer, boundedTimestamp) = _selectBoundedPrice(data);
+
+        // Return bounded data unless there is a newer snapshotted data that still satisfies time constraint.
+        if (boundedTimestamp > snapshot.timestamp || snapshot.timestamp > timestamp) {
+            return (boundedAnswer, boundedTimestamp, 1);
+        }
+        return (snapshot.answer, snapshot.timestamp, 1);
+    }
+
+    // Internal helper to get the latest data from all sources.
+    function _getAllLatestSourceData() internal view returns (AllSourceData memory) {
+        AllSourceData memory data;
+        (data.clAnswer, data.clTimestamp) = ChainlinkSourceAdapter.getLatestSourceData();
+        (data.crAnswer, data.crTimestamp) = ChronicleMedianSourceAdapter.getLatestSourceData();
+        (data.pyAnswer, data.pyTimestamp) = PythSourceAdapter.getLatestSourceData();
 
-        (int256 boundedAnswer, uint256 boundedTimestamp) =
-            _selectBoundedPrice(clAnswer, clTimestamp, crAnswer, crTimestamp, pyAnswer, pyTimestamp);
-        return (boundedAnswer, boundedTimestamp, 1);
+        return data;
     }
 
     // Selects the appropriate price from the three sources based on the bounding tolerance and logic.
-    function _selectBoundedPrice(int256 cl, uint256 clT, int256 cr, uint256 crT, int256 py, uint256 pyT)
-        internal
-        view
-        returns (int256, uint256)
-    {
+    function _selectBoundedPrice(AllSourceData memory data) internal view returns (int256, uint256) {
         int256 newestVal = 0;
         uint256 newestT = 0;
 
+        // Unpack the data to short named variables for better code readability below.
+        (int256 cl, uint256 clT, int256 cr, uint256 crT, int256 py, uint256 pyT) =
+            (data.clAnswer, data.clTimestamp, data.crAnswer, data.crTimestamp, data.pyAnswer, data.pyTimestamp);
+
         // For each price, check if it is within tolerance of the other two. If so, check if it is the newest.
         if (pyT > newestT && (_withinTolerance(py, cr) || _withinTolerance(py, cl))) (newestVal, newestT) = (py, pyT);
         if (crT > newestT && (_withinTolerance(cr, py) || _withinTolerance(cr, cl))) (newestVal, newestT) = (cr, crT);

src/adapters/source-adapters/ChronicleMedianSourceAdapter.sol

@@ -1,16 +1,19 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity 0.8.17;
 
-import {SnapshotSource} from "./SnapshotSource.sol";
+import {DiamondRootOval} from "../../DiamondRootOval.sol";
+import {SnapshotSourceLib} from "../lib/SnapshotSourceLib.sol";
 import {IMedian} from "../../interfaces/chronicle/IMedian.sol";
 import {SafeCast} from "openzeppelin-contracts/contracts/utils/math/SafeCast.sol";
 
 /**
  * @title ChronicleMedianSourceAdapter contract to read data from Chronicle and standardize it for Oval.
  */
-abstract contract ChronicleMedianSourceAdapter is SnapshotSource {
+abstract contract ChronicleMedianSourceAdapter is DiamondRootOval {
     IMedian public immutable CHRONICLE_SOURCE;
 
+    SnapshotSourceLib.Snapshot[] public chronicleMedianSnapshots; // Historical answer and timestamp snapshots.
+
     event SourceSet(address indexed sourceOracle);
 
     constructor(IMedian _chronicleSource) {
@@ -19,6 +22,14 @@ abstract contract ChronicleMedianSourceAdapter is SnapshotSource {
         emit SourceSet(address(_chronicleSource));
     }
 
+    /**
+     * @notice Snapshot the current source data.
+     */
+    function snapshotData() public virtual override {
+        (int256 latestAnswer, uint256 latestTimestamp) = ChronicleMedianSourceAdapter.getLatestSourceData();
+        SnapshotSourceLib.snapshotData(chronicleMedianSnapshots, latestAnswer, latestTimestamp);
+    }
+
     /**
      * @notice Returns the latest data from the source.
      * @dev The standard chronicle implementation will revert if the latest answer is not valid when calling the read
@@ -43,7 +54,7 @@ abstract contract ChronicleMedianSourceAdapter is SnapshotSource {
     /**
      * @notice Tries getting latest data as of requested timestamp. If this is not possible, returns the earliest data
      * available past the requested timestamp within provided traversal limitations.
-     * @dev Chronicle does not support historical lookups so this uses SnapshotSource to get historic data.
+     * @dev Chronicle does not support historical lookups so this uses SnapshotSourceLib to get historic data.
      * @param timestamp The timestamp to try getting latest data at.
      * @param maxTraversal The maximum number of rounds to traverse when looking for historical data.
      * @return answer The answer as of requested timestamp, or earliest available data if not available, in 18 decimals.
@@ -57,7 +68,10 @@ abstract contract ChronicleMedianSourceAdapter is SnapshotSource {
         override
         returns (int256, uint256, uint256)
     {
-        Snapshot memory snapshot = _tryLatestDataAt(timestamp, maxTraversal);
+        (int256 latestAnswer, uint256 latestTimestamp) = ChronicleMedianSourceAdapter.getLatestSourceData();
+        SnapshotSourceLib.Snapshot memory snapshot = SnapshotSourceLib._tryLatestDataAt(
+            chronicleMedianSnapshots, latestAnswer, latestTimestamp, timestamp, maxTraversal, maxAge()
+        );
         return (snapshot.answer, snapshot.timestamp, 1);
     }
 }

src/adapters/source-adapters/OSMSourceAdapter.sol

@@ -1,19 +1,22 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity 0.8.17;
 
-import {SnapshotSource} from "./SnapshotSource.sol";
+import {DiamondRootOval} from "../../DiamondRootOval.sol";
+import {SnapshotSourceLib} from "../lib/SnapshotSourceLib.sol";
 import {IOSM} from "../../interfaces/makerdao/IOSM.sol";
 
 /**
  * @title OSMSourceAdapter contract to read data from MakerDAO OSM and standardize it for Oval.
  */
-abstract contract OSMSourceAdapter is SnapshotSource {
+abstract contract OSMSourceAdapter is DiamondRootOval {
     IOSM public immutable osmSource;
 
     // MakerDAO performs decimal conversion in collateral adapter contracts, so all oracle prices are expected to have
     // 18 decimals and we can skip decimal conversion.
     uint8 public constant decimals = 18;
 
+    SnapshotSourceLib.Snapshot[] public osmSnapshots; // Historical answer and timestamp snapshots.
+
     event SourceSet(address indexed sourceOracle);
 
     constructor(IOSM source) {
@@ -22,6 +25,14 @@ abstract contract OSMSourceAdapter is SnapshotSource {
         emit SourceSet(address(source));
     }
 
+    /**
+     * @notice Snapshot the current source data.
+     */
+    function snapshotData() public virtual override {
+        (int256 latestAnswer, uint256 latestTimestamp) = OSMSourceAdapter.getLatestSourceData();
+        SnapshotSourceLib.snapshotData(osmSnapshots, latestAnswer, latestTimestamp);
+    }
+
     /**
      * @notice Returns the latest data from the source.
      * @return answer The latest answer in 18 decimals.
@@ -44,7 +55,7 @@ abstract contract OSMSourceAdapter is SnapshotSource {
     /**
      * @notice Tries getting latest data as of requested timestamp. If this is not possible, returns the earliest data
      * available past the requested timestamp within provided traversal limitations.
-     * @dev OSM does not support historical lookups so this uses SnapshotSource to get historic data.
+     * @dev OSM does not support historical lookups so this uses SnapshotSourceLib to get historic data.
      * @param timestamp The timestamp to try getting latest data at.
      * @param maxTraversal The maximum number of rounds to traverse when looking for historical data.
      * @return answer The answer as of requested timestamp, or earliest available data if not available, in 18 decimals.
@@ -57,7 +68,10 @@ abstract contract OSMSourceAdapter is SnapshotSource {
         override
         returns (int256, uint256, uint256)
     {
-        Snapshot memory snapshot = _tryLatestDataAt(timestamp, maxTraversal);
+        (int256 latestAnswer, uint256 latestTimestamp) = OSMSourceAdapter.getLatestSourceData();
+        SnapshotSourceLib.Snapshot memory snapshot = SnapshotSourceLib._tryLatestDataAt(
+            osmSnapshots, latestAnswer, latestTimestamp, timestamp, maxTraversal, maxAge()
+        );
         return (snapshot.answer, snapshot.timestamp, 1);
     }
 }