fix (#6)

* fix Signed-off-by: Reinis Martinsons <reinis@umaproject.org> * test: frequent source updates Signed-off-by: Reinis Martinsons <reinis@umaproject.org> * fix Signed-off-by: Reinis Martinsons <reinis@umaproject.org> * fix Signed-off-by: Reinis Martinsons <reinis@umaproject.org> * wip Signed-off-by: chrismaree <christopher.maree@gmail.com> * update tests Signed-off-by: Reinis Martinsons <reinis@umaproject.org> * wip Signed-off-by: chrismaree <christopher.maree@gmail.com> * wip Signed-off-by: chrismaree <christopher.maree@gmail.com> --------- Signed-off-by: Reinis Martinsons <reinis@umaproject.org> Signed-off-by: chrismaree <christopher.maree@gmail.com> Co-authored-by: chrismaree <christopher.maree@gmail.com>
UMAprotocol · Sep 1, 2023 · fb6aaed · fb6aaed
1 parent 9845262
commit fb6aaed
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 15 deletions.
diff --git a/src/adapters/chainlink/ChainlinkDestinationOracleAdapter.sol b/src/adapters/chainlink/ChainlinkDestinationOracleAdapter.sol
@@ -28,6 +28,6 @@ contract ChainlinkDestinationOracleAdapter is IAggregatorV3, BaseController {
         view
         returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)
     {
-        return (0, latestAnswer(), 0, 0, 0);
+        return (0, latestAnswer(), 0, 0, 0); //TODO: this should have a timestamp in it
     }
 }
diff --git a/src/controllers/BaseController.sol b/src/controllers/BaseController.sol
@@ -29,16 +29,16 @@ contract BaseController is IBaseController, OevOracle, Ownable {
         return updaters[caller];
     }
 
-    function canReturnCache(
-        uint256 cachedLatestTimestamp,
-        uint256 updateTimestamp
-    ) public view override(IBaseController, OevOracle) returns (bool) {
-        uint256 currentTimestamp = block.timestamp;
-
+    function canReturnCache(uint256 cachedLatestTimestamp, uint256 updateTimestamp)
+        public
+        view
+        override(IBaseController, OevOracle)
+        returns (bool)
+    {
         // If cache was updated within the permission window, return the cached value.
-        if (currentTimestamp - updateTimestamp < permissionWindow) return true;
+        if (block.timestamp - updateTimestamp < permissionWindow) return true;
 
         // Otherwise check against last update time on source adapter.
-        return (currentTimestamp - sourceAdapter.latestTimestamp() < permissionWindow);
+        return (sourceAdapter.latestTimestamp() - updateTimestamp < permissionWindow);
     }
 }
diff --git a/test/unit/OevOracle.BaseController.UpdateAnswer.sol b/test/unit/OevOracle.BaseController.UpdateAnswer.sol
@@ -91,14 +91,21 @@ contract OevOracleUpdateAnswer is Common {
         oevOracle.updateAnswer();
         assertTrue(oevOracle.canReturnCache(oevOracle.cachedLatestTimestamp(), oevOracle.lastUpdateTimestamp()));
 
-        // Advance past the end of the permission window. Now, when reading values from the OevOracle, the source
-        // adapter should be read directly, bypassing the cache.
-        vm.warp(block.timestamp + 2 minutes);
+        // Advancing time past the permission window should not yet pass through source values before source update.
+        uint256 pastPermissionWindow = block.timestamp + 2 minutes;
+        vm.warp(pastPermissionWindow);
+        assertTrue(oevOracle.canReturnCache(oevOracle.cachedLatestTimestamp(), oevOracle.lastUpdateTimestamp()));
+
+        // When source updates past the end of the permission window, reading values from the OevOracle should pass
+        // through values from the source adapter, bypassing the cache.
+        int256 nextNewAnswer = newAnswer + 1e18;
+        sourceAdapter.setLatestAnswer(nextNewAnswer);
+        sourceAdapter.setLatestTimestamp(pastPermissionWindow);
         assertFalse(oevOracle.canReturnCache(oevOracle.cachedLatestTimestamp(), oevOracle.lastUpdateTimestamp()));
         verifyOevOracleMatchesSourceAdapter();
         // Despite this, the cache should be out of date.
-        assertTrue(oevOracle.cachedLatestAnswer() != newAnswer);
-        assertTrue(oevOracle.cachedLatestTimestamp() != newTimestamp);
+        assertTrue(oevOracle.cachedLatestAnswer() == initialPrice);
+        assertTrue(oevOracle.cachedLatestTimestamp() == initialTimestamp);
     }
 
     function testUpdatesWithinPermissionWindow() public {
@@ -132,9 +139,16 @@ contract OevOracleUpdateAnswer is Common {
         assertTrue(oevOracle.internalLatestAnswer() == initialPrice);
         assertTrue(oevOracle.internalLatestTimestamp() == initialTimestamp);
 
-        // Advancing time past the permission window should pass through source values without update.
+        // Advancing time past the permission window should not yet pass through source values before source update.
         uint256 pastPermissionWindow = newTimestamp + oevOracle.permissionWindow() + 1;
         vm.warp(pastPermissionWindow);
+        assertTrue(oevOracle.internalLatestAnswer() == initialPrice);
+        assertTrue(oevOracle.internalLatestTimestamp() == initialTimestamp);
+
+        // When source updates, OEVOracle should pass through values from the source adapter, bypassing the cache.
+        int256 nextNewAnswer = newAnswer + 1e18;
+        sourceAdapter.setLatestAnswer(nextNewAnswer);
+        sourceAdapter.setLatestTimestamp(pastPermissionWindow);
         verifyOevOracleMatchesSourceAdapter();
     }
 
@@ -165,4 +179,28 @@ contract OevOracleUpdateAnswer is Common {
         assertTrue(oevOracle.internalLatestAnswer() == newAnswer);
         assertTrue(oevOracle.internalLatestTimestamp() == beforePermissionWindow);
     }
+
+    function testFrequentSourceUpdatesNoOevUpdates() public {
+        syncOevOracleWithSourceAdapter();
+
+        // Advance time to within the permission window and update the source.
+        uint256 beforePermissionWindow = block.timestamp + oevOracle.permissionWindow() - 1;
+        vm.warp(beforePermissionWindow);
+        sourceAdapter.setLatestAnswer(newAnswer);
+        sourceAdapter.setLatestTimestamp(beforePermissionWindow);
+
+        // Within permission window, initial values from cache would be returned.
+        assertTrue(oevOracle.internalLatestAnswer() == initialPrice);
+        assertTrue(oevOracle.internalLatestTimestamp() == initialTimestamp);
+
+        // Advance time to within the next permission window and update the source without updating the OevOracle.
+        uint256 nextBeforePermissionWindow = block.timestamp + oevOracle.permissionWindow() - 1;
+        vm.warp(nextBeforePermissionWindow);
+        int256 nextNewAnswer = newAnswer + 1e18;
+        sourceAdapter.setLatestAnswer(nextNewAnswer);
+        sourceAdapter.setLatestTimestamp(nextBeforePermissionWindow);
+
+        // Now that OevOracle has not been updated for more than the permission window, the source values should be passed through.
+        verifyOevOracleMatchesSourceAdapter();
+    }
 }