From ff968085f579be5015038d68ccd5873853f4a9d4 Mon Sep 17 00:00:00 2001 From: Reinis Martinsons <77973553+Reinis-FRP@users.noreply.github.com> Date: Mon, 11 Sep 2023 17:16:44 +0300 Subject: [PATCH] fix: source round (#7) * fix: source round detection Signed-off-by: Reinis Martinsons * feat: implement round updates in mean adapter Signed-off-by: Reinis Martinsons * feat: add delayed source data Signed-off-by: Reinis Martinsons * feat: add todos Signed-off-by: Reinis Martinsons * feat: chainlink round traversal Signed-off-by: Reinis Martinsons * fix: remove latest round Signed-off-by: Reinis Martinsons * fix: remove latest round from mock Signed-off-by: Reinis Martinsons * fix: internal function naming Signed-off-by: Reinis Martinsons * fix: return round data Signed-off-by: Reinis Martinsons * fix: comments Signed-off-by: Reinis Martinsons * fix: return latest in source if no historical data Signed-off-by: Reinis Martinsons * fix: add comment on updated at Signed-off-by: Reinis Martinsons * feat: move all permission window logic to oev Signed-off-by: Reinis Martinsons * feat: pass max traversal from oev Signed-off-by: Reinis Martinsons --------- Signed-off-by: Reinis Martinsons --- src/OevOracle.sol | 36 ++++-- src/adapters/BaseDestinationOracleAdapter.sol | 2 +- src/adapters/MeanSourceOracleAdapter.sol | 12 ++ .../ChainlinkSourceOracleAdapter.sol | 116 +++++++++++++++++- ...UniswapAnchoredViewSourceOracleAdapter.sol | 16 ++- .../makerdao/ChronicleSourceOracleAdapter.sol | 12 ++ src/controllers/BaseController.sol | 22 +--- src/interfaces/IBaseController.sol | 2 - src/interfaces/IBaseOracleAdapter.sol | 4 + src/interfaces/IOevOracle.sol | 2 + .../chainlink/IAggregatorV3Source.sol | 17 +++ test/mocks/MockSourceOracleAdapter.sol | 18 +++ .../OevOracle.BaseController.UpdateAnswer.sol | 6 +- 13 files changed, 224 insertions(+), 41 deletions(-) create mode 100644 src/interfaces/chainlink/IAggregatorV3Source.sol diff --git a/src/OevOracle.sol b/src/OevOracle.sol index 27937e1..cdcb492 100644 --- a/src/OevOracle.sol +++ b/src/OevOracle.sol @@ -1,12 +1,18 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.17; +import "openzeppelin-contracts/contracts/access/Ownable.sol"; import "./interfaces/IBaseOracleAdapter.sol"; import "./interfaces/IOevOracle.sol"; -abstract contract OevOracle is IOevOracle { +abstract contract OevOracle is IOevOracle, Ownable { IBaseOracleAdapter public sourceAdapter; + uint256 public permissionWindow = 10 minutes; + + // TODO: Test gas costs of traversing history to decide on reasonable value. + uint256 public maxTraversal = 10; // Max number of historical source updates to traverse. + int256 public cachedLatestAnswer; // Always 18 decimals. uint256 public cachedLatestTimestamp; uint256 public lastUpdateTimestamp; @@ -28,20 +34,34 @@ abstract contract OevOracle is IOevOracle { // TODO: Decide how to handle negative values. Since not all source oracles support negative values we may need to // limit the support only to positive values. function internalLatestAnswer() public view override returns (int256) { - if (canReturnCache(cachedLatestTimestamp, lastUpdateTimestamp)) return cachedLatestAnswer; - return sourceAdapter.latestAnswer(); + if (canReturnCache(lastUpdateTimestamp)) return cachedLatestAnswer; + // Attempt to return latest answer as of before permission window. If this is not available return latest answer. + return sourceAdapter.tryLatestAnswerAt(block.timestamp - permissionWindow, maxTraversal); } function internalLatestTimestamp() public view override returns (uint256) { - if (canReturnCache(cachedLatestTimestamp, lastUpdateTimestamp)) return cachedLatestTimestamp; - return sourceAdapter.latestTimestamp(); + if (canReturnCache(lastUpdateTimestamp)) return cachedLatestTimestamp; + // Attempt to return latest timestamp as of before permission window. If this is not available return latest timestamp. + return sourceAdapter.tryLatestTimestampAt(block.timestamp - permissionWindow, maxTraversal); } function canUpdate(address caller, uint256 cachedLatestTimestamp) public view virtual returns (bool); - function canReturnCache(uint256 _cachedLatestTimestamp, uint256 _updateTimestamp) + function setPermissionWindow(uint256 _permissionWindow) public onlyOwner { + permissionWindow = _permissionWindow; + } + + function setMaxTraversal(uint256 _maxTraversal) public onlyOwner { + maxTraversal = _maxTraversal; + } + + function canReturnCache(uint256 updateTimestamp) public view - virtual - returns (bool); + override + returns (bool) + { + // If cache was updated within the permission window, return the cached value. + return (block.timestamp - updateTimestamp < permissionWindow); + } } diff --git a/src/adapters/BaseDestinationOracleAdapter.sol b/src/adapters/BaseDestinationOracleAdapter.sol index 0867a4c..b3baf6b 100644 --- a/src/adapters/BaseDestinationOracleAdapter.sol +++ b/src/adapters/BaseDestinationOracleAdapter.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.17; import "../controllers/BaseController.sol"; import "../interfaces/IBaseOracleAdapter.sol"; -contract BaseDestinationOracleAdapter is BaseController, IBaseOracleAdapter { +abstract contract BaseDestinationOracleAdapter is BaseController, IBaseOracleAdapter { constructor(address _sourceAdapter) BaseController(_sourceAdapter) {} function decimals() public view override returns (uint8) { diff --git a/src/adapters/MeanSourceOracleAdapter.sol b/src/adapters/MeanSourceOracleAdapter.sol index 5a8dfb3..c8a7b12 100644 --- a/src/adapters/MeanSourceOracleAdapter.sol +++ b/src/adapters/MeanSourceOracleAdapter.sol @@ -33,4 +33,16 @@ contract MeanSourceOracleAdapter is IBaseOracleAdapter { function decimals() public view override returns (uint8) { return 18; } + + function tryLatestAnswerAt(uint256 timestamp, uint256 maxTraversal) public view override returns (int256) { + // TODO: implement traversing source rounds to get latest data as of requested timestamp. + // TODO: this also requires adding update method to store historical data on source adapter. + return latestAnswer(); + } + + function tryLatestTimestampAt(uint256 timestamp, uint256 maxTraversal) public view override returns (uint256) { + // TODO: implement traversing source rounds to get latest data as of requested timestamp. + // TODO: this also requires adding update method to store historical data on source adapter. + return latestTimestamp(); + } } diff --git a/src/adapters/chainlink/ChainlinkSourceOracleAdapter.sol b/src/adapters/chainlink/ChainlinkSourceOracleAdapter.sol index 47c1245..fb30915 100644 --- a/src/adapters/chainlink/ChainlinkSourceOracleAdapter.sol +++ b/src/adapters/chainlink/ChainlinkSourceOracleAdapter.sol @@ -2,13 +2,21 @@ pragma solidity 0.8.17; import "../../interfaces/IBaseOracleAdapter.sol"; -import "../../interfaces/chainlink/IAggregatorV3.sol"; +import "../../interfaces/chainlink/IAggregatorV3Source.sol"; contract ChainlinkSourceOracleAdapter is IBaseOracleAdapter { - IAggregatorV3 public source; + struct RoundData { + uint80 roundId; + int256 answer; + uint256 startedAt; + uint256 updatedAt; + uint80 answeredInRound; + } + + IAggregatorV3Source public source; constructor(address _source) { - source = IAggregatorV3(_source); + source = IAggregatorV3Source(_source); } function latestAnswer() public view override returns (int256) { @@ -22,4 +30,106 @@ contract ChainlinkSourceOracleAdapter is IBaseOracleAdapter { function decimals() public view override returns (uint8) { return source.decimals(); } + + // Tries getting latest answer as of requested timestamp. If this is not available returns the current latest answer. + function tryLatestAnswerAt( + uint256 timestamp, + uint256 maxTraversal + ) public view override returns (int256) { + return _tryLatestRoundDataAt(timestamp, maxTraversal).answer; + } + + // Tries getting latest timestamp as of requested timestamp. If this is not available returns the current latest timestamp. + function tryLatestTimestampAt( + uint256 timestamp, + uint256 maxTraversal + ) public view override returns (uint256) { + return _tryLatestRoundDataAt(timestamp, maxTraversal).updatedAt; + } + + // Tries getting latest round data as of requested timestamp. If this is not available returns the current latest round data. + function _tryLatestRoundDataAt( + uint256 timestamp, + uint256 maxTraversal + ) + internal + view + returns (RoundData memory) + { + ( + uint80 roundId, + int256 answer, + uint256 startedAt, + uint256 updatedAt, + uint80 answeredInRound + ) = source.latestRoundData(); + RoundData memory latestRoundData = RoundData({ + roundId: roundId, + answer: answer, + startedAt: startedAt, + updatedAt: updatedAt, + answeredInRound: answeredInRound + }); + // In the happy path there have been no source updates since requested time, so we can return the latest data. + // We can use updatedAt property as it matches the block timestamp of the latest source transmission. + if (latestRoundData.updatedAt <= timestamp) return latestRoundData; + + // Attempt traversing historical round data. + RoundData memory historicalRoundData = _searchRoundDataAt(timestamp, latestRoundData.roundId - 1, maxTraversal); + + // Validate returned data. If it is not valid this means we failed to find data at requested timestamp so we + // fallback to returning the current latest round data. + if (historicalRoundData.updatedAt > 0) return historicalRoundData; + return latestRoundData; + } + + // This returns uninitialized data if traversed all rounds of the current phase aggregator or exceeded maximum + // allowed round traversal. The caller should check if updatedAt is 0 to determine if the data is valid. + function _searchRoundDataAt( + uint256 timestamp, + uint80 roundId, + uint256 maxTraversal + ) + internal + view + returns (RoundData memory) + { + RoundData memory roundData; + uint80 traversedRounds = 0; + + while (traversedRounds < uint80(maxTraversal)) { + ( + uint80 _roundId, + int256 answer, + uint256 startedAt, + uint256 updatedAt, + uint80 answeredInRound + ) = source.getRoundData(roundId); + roundData = RoundData({ + roundId: _roundId, + answer: answer, + startedAt: startedAt, + updatedAt: updatedAt, + answeredInRound: answeredInRound + }); + // As per Chainlink documentation https://docs.chain.link/data-feeds/historical-data#roundid-in-proxy + // roundId on the aggregator proxy is composed of phaseId (higher 16 bits) and roundId from phase aggregator + // (lower 64 bits). The aggregator proxy does not keep track when its phase aggregators got switched. This + // means that we can only traverse rounds of the current phase aggregator. When phase aggregators are + // switched there is normally an overlap period when both new and old phase aggregators receive updates. + // Without knowing exact time when the aggregator proxy switched them we might end up returning historical + // data from the new phase aggregator that was not yet available on the aggregator proxy at the requested + // timestamp. + // Since phase aggregators are starting at round 1 we can detect unitialized round 0 (as in phase + // aggregator) in updatedAt and return unitialized round data that should be handled by the caller. + if (roundData.updatedAt <= timestamp || roundData.updatedAt == 0) return roundData; + roundId = roundData.roundId - 1; + traversedRounds++; + } + + // Since we traversed past maximum allowed round traversal without finding latest data at requested timestamp, + // we must invalidate it so that the caller can fallback to the current latest data. + roundData.updatedAt = 0; + return roundData; + } } diff --git a/src/adapters/compound/UniswapAnchoredViewSourceOracleAdapter.sol b/src/adapters/compound/UniswapAnchoredViewSourceOracleAdapter.sol index f87ff0f..677d856 100644 --- a/src/adapters/compound/UniswapAnchoredViewSourceOracleAdapter.sol +++ b/src/adapters/compound/UniswapAnchoredViewSourceOracleAdapter.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.17; import "../../interfaces/IBaseOracleAdapter.sol"; -import "../../interfaces/chainlink/IAggregatorV3.sol"; +import "../../interfaces/chainlink/IAggregatorV3Source.sol"; import "../../interfaces/compound/IUniswapAnchoredView.sol"; import "../../interfaces/compound/ICToken.sol"; import "../../interfaces/compound/IValidatorProxy.sol"; @@ -14,7 +14,7 @@ contract UniswapAnchoredViewSourceOracleAdapter is IBaseOracleAdapter { uint8 public decimals; - IAggregatorV3 public aggregator; + IAggregatorV3Source public aggregator; constructor(IUniswapAnchoredView _source, address _cToken) { source = _source; @@ -24,7 +24,7 @@ contract UniswapAnchoredViewSourceOracleAdapter is IBaseOracleAdapter { IUniswapAnchoredView.TokenConfig memory tokenConfig = source.getTokenConfigByCToken(address(cToken)); // TODO: make sure this does not change over time (address current,,) = IValidatorProxy(tokenConfig.reporter).getAggregators(); - aggregator = IAggregatorV3(current); + aggregator = IAggregatorV3Source(current); } function latestAnswer() public view override returns (int256) { @@ -36,4 +36,14 @@ contract UniswapAnchoredViewSourceOracleAdapter is IBaseOracleAdapter { function latestTimestamp() public view override returns (uint256) { return aggregator.latestTimestamp(); } + + function tryLatestAnswerAt(uint256 timestamp, uint256 maxTraversal) public view override returns (int256) { + // TODO: implement traversing source rounds to get latest data as of requested timestamp. + return latestAnswer(); + } + + function tryLatestTimestampAt(uint256 timestamp, uint256 maxTraversal) public view override returns (uint256) { + // TODO: implement traversing source rounds to get latest data as of requested timestamp. + return latestTimestamp(); + } } diff --git a/src/adapters/makerdao/ChronicleSourceOracleAdapter.sol b/src/adapters/makerdao/ChronicleSourceOracleAdapter.sol index 1ed5a88..2dcf848 100644 --- a/src/adapters/makerdao/ChronicleSourceOracleAdapter.sol +++ b/src/adapters/makerdao/ChronicleSourceOracleAdapter.sol @@ -23,4 +23,16 @@ contract ChronicleSourceOracleAdapter is IBaseOracleAdapter { function latestTimestamp() public view override returns (uint256) { return source.zzz(); } + + function tryLatestAnswerAt(uint256 timestamp, uint256 maxTraversal) public view override returns (int256) { + // TODO: implement traversing source rounds to get latest data as of requested timestamp. + // TODO: this also requires adding update method to store historical data on source adapter. + return latestAnswer(); + } + + function tryLatestTimestampAt(uint256 timestamp, uint256 maxTraversal) public view override returns (uint256) { + // TODO: implement traversing source rounds to get latest data as of requested timestamp. + // TODO: this also requires adding update method to store historical data on source adapter. + return latestTimestamp(); + } } diff --git a/src/controllers/BaseController.sol b/src/controllers/BaseController.sol index f0a0d09..69bfcd8 100644 --- a/src/controllers/BaseController.sol +++ b/src/controllers/BaseController.sol @@ -1,25 +1,18 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.17; -import "openzeppelin-contracts/contracts/access/Ownable.sol"; import "../OevOracle.sol"; import "../interfaces/IBaseController.sol"; -contract BaseController is IBaseController, OevOracle, Ownable { +contract BaseController is IBaseController, OevOracle { constructor(address _sourceAdapter) OevOracle(_sourceAdapter) {} mapping(address => bool) public updaters; - uint256 public permissionWindow = 10 minutes; - function setUpdater(address _updater, bool allowed) public onlyOwner { updaters[_updater] = allowed; } - function setPermissionWindow(uint256 _permissionWindow) public onlyOwner { - permissionWindow = _permissionWindow; - } - function canUpdate(address caller, uint256 cachedLatestTimestamp) public view @@ -28,17 +21,4 @@ contract BaseController is IBaseController, OevOracle, Ownable { { return updaters[caller]; } - - 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 (block.timestamp - updateTimestamp < permissionWindow) return true; - - // Otherwise check against last update time on source adapter. - return (sourceAdapter.latestTimestamp() - updateTimestamp < permissionWindow); - } } diff --git a/src/interfaces/IBaseController.sol b/src/interfaces/IBaseController.sol index 207ec72..0ed4837 100644 --- a/src/interfaces/IBaseController.sol +++ b/src/interfaces/IBaseController.sol @@ -3,6 +3,4 @@ pragma solidity 0.8.17; interface IBaseController { function canUpdate(address caller, uint256 cachedLatestTimestamp) external view returns (bool); - - function canReturnCache(uint256 cachedLatestTimestamp, uint256 updateTimestamp) external view returns (bool); } diff --git a/src/interfaces/IBaseOracleAdapter.sol b/src/interfaces/IBaseOracleAdapter.sol index a327196..86a3ec4 100644 --- a/src/interfaces/IBaseOracleAdapter.sol +++ b/src/interfaces/IBaseOracleAdapter.sol @@ -8,4 +8,8 @@ interface IBaseOracleAdapter { function latestTimestamp() external view returns (uint256); function decimals() external view returns (uint8); + + function tryLatestAnswerAt(uint256 timestamp, uint256 maxTraversal) external view returns (int256); + + function tryLatestTimestampAt(uint256 timestamp, uint256 maxTraversal) external view returns (uint256); } diff --git a/src/interfaces/IOevOracle.sol b/src/interfaces/IOevOracle.sol index e86f211..ced8b76 100644 --- a/src/interfaces/IOevOracle.sol +++ b/src/interfaces/IOevOracle.sol @@ -5,4 +5,6 @@ interface IOevOracle { function internalLatestAnswer() external view returns (int256); function internalLatestTimestamp() external view returns (uint256); + + function canReturnCache(uint256 updateTimestamp) external view returns (bool); } diff --git a/src/interfaces/chainlink/IAggregatorV3Source.sol b/src/interfaces/chainlink/IAggregatorV3Source.sol new file mode 100644 index 0000000..ff4ca44 --- /dev/null +++ b/src/interfaces/chainlink/IAggregatorV3Source.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.17; + +import "./IAggregatorV3.sol"; + +interface IAggregatorV3Source is IAggregatorV3 { + + function latestRoundData() + external + view + returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); + + function getRoundData(uint80 _roundId) + external + view + returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); +} diff --git a/test/mocks/MockSourceOracleAdapter.sol b/test/mocks/MockSourceOracleAdapter.sol index 047ea02..f762fa8 100644 --- a/test/mocks/MockSourceOracleAdapter.sol +++ b/test/mocks/MockSourceOracleAdapter.sol @@ -7,6 +7,8 @@ contract MockSourceOracleAdapter is IBaseOracleAdapter { int256 public latestAnswer; uint256 public latestTimestamp; uint8 public decimals; + mapping(uint256 => int256) public _latestAnswerAt; + mapping(uint256 => uint256) public _latestTimestampAt; constructor(uint8 _decimals) { decimals = _decimals; @@ -19,4 +21,20 @@ contract MockSourceOracleAdapter is IBaseOracleAdapter { function setLatestTimestamp(uint256 _latestTimestamp) public { latestTimestamp = _latestTimestamp; } + + function tryLatestAnswerAt(uint256 timestamp, uint256 maxTraversal) public view override returns (int256) { + return _latestAnswerAt[timestamp]; + } + + function tryLatestTimestampAt(uint256 timestamp, uint256 maxTraversal) public view override returns (uint256) { + return _latestTimestampAt[timestamp]; + } + + function setLatestAnswerAt(uint256 timestamp, int256 _latestAnswer) public { + _latestAnswerAt[timestamp] = _latestAnswer; + } + + function setLatestTimestampAt(uint256 timestamp, uint256 _latestTimestamp) public { + _latestTimestampAt[timestamp] = _latestTimestamp; + } } diff --git a/test/unit/OevOracle.BaseController.UpdateAnswer.sol b/test/unit/OevOracle.BaseController.UpdateAnswer.sol index dc0611c..5c2105f 100644 --- a/test/unit/OevOracle.BaseController.UpdateAnswer.sol +++ b/test/unit/OevOracle.BaseController.UpdateAnswer.sol @@ -89,19 +89,19 @@ contract OevOracleUpdateAnswer is Common { vm.expectRevert("Controller blocked canUpdate"); vm.prank(random); oevOracle.updateAnswer(); - assertTrue(oevOracle.canReturnCache(oevOracle.cachedLatestTimestamp(), oevOracle.lastUpdateTimestamp())); + assertTrue(oevOracle.canReturnCache(oevOracle.lastUpdateTimestamp())); // 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())); + assertTrue(oevOracle.canReturnCache(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())); + assertFalse(oevOracle.canReturnCache(oevOracle.lastUpdateTimestamp())); verifyOevOracleMatchesSourceAdapter(); // Despite this, the cache should be out of date. assertTrue(oevOracle.cachedLatestAnswer() == initialPrice);