From fb6aaedce496de2d72d5a2f762cc4babbc5bdd8e Mon Sep 17 00:00:00 2001 From: Reinis Martinsons <77973553+Reinis-FRP@users.noreply.github.com> Date: Fri, 1 Sep 2023 19:20:50 +0300 Subject: [PATCH] fix (#6) * fix Signed-off-by: Reinis Martinsons * test: frequent source updates Signed-off-by: Reinis Martinsons * fix Signed-off-by: Reinis Martinsons * fix Signed-off-by: Reinis Martinsons * wip Signed-off-by: chrismaree * update tests Signed-off-by: Reinis Martinsons * wip Signed-off-by: chrismaree * wip Signed-off-by: chrismaree --------- Signed-off-by: Reinis Martinsons Signed-off-by: chrismaree Co-authored-by: chrismaree --- .../ChainlinkDestinationOracleAdapter.sol | 2 +- src/controllers/BaseController.sol | 16 +++--- .../OevOracle.BaseController.UpdateAnswer.sol | 50 ++++++++++++++++--- 3 files changed, 53 insertions(+), 15 deletions(-) diff --git a/src/adapters/chainlink/ChainlinkDestinationOracleAdapter.sol b/src/adapters/chainlink/ChainlinkDestinationOracleAdapter.sol index 62cf031..0a38392 100644 --- 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 index e5e3049..f0a0d09 100644 --- 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 index 2a93c72..dc0611c 100644 --- 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(); + } }