From 424d94ecf234fa4fac5adb459ee7c1f2ffbb2a93 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 20 May 2024 14:59:51 -0400 Subject: [PATCH 1/2] feat: add maxAge parameter to limit the staleness of a historical data point (#14) Signed-off-by: Matt Rice Signed-off-by: chrismaree Co-authored-by: chrismaree --- src/DiamondRootOval.sol | 7 +++- .../ChainlinkSourceAdapter.sol | 4 +- .../source-adapters/SnapshotSource.sol | 4 +- src/controllers/BaseController.sol | 18 +++++++++ src/controllers/ImmutableController.sol | 12 +++++- src/interfaces/IBaseController.sol | 1 + test/fork/aave/AaveV2.Liquidation.sol | 2 +- test/fork/aave/AaveV3.Liquidation.sol | 2 +- test/fork/adapters/ChainlinkSourceAdapter.sol | 39 +++++++++++++------ .../adapters/ChronicleMedianSourceAdapter.sol | 8 +--- test/fork/adapters/OSMSourceAdapter.sol | 8 +--- test/fork/adapters/PythSourceAdapter.sol | 13 +------ test/fork/adapters/UnionSourceAdapter.sol | 15 ++----- .../UniswapAnchoredViewSourceAdapter.sol | 8 +--- test/fork/compound/CompoundV2.Liquidation.sol | 2 +- test/unit/BaseController.sol | 13 +++++++ test/unit/ImmutableController.sol | 13 +++++-- test/unit/SnapshotSource.SnapshotData.sol | 38 ++++++++++++++++++ .../BoundedUnionSource.SelectBoundedPrice.sol | 1 + 19 files changed, 145 insertions(+), 63 deletions(-) diff --git a/src/DiamondRootOval.sol b/src/DiamondRootOval.sol index c8223eb..1d0f294 100644 --- a/src/DiamondRootOval.sol +++ b/src/DiamondRootOval.sol @@ -75,13 +75,16 @@ abstract contract DiamondRootOval is IBaseController, IOval, IBaseOracleAdapter * @notice Time window that bounds how long the permissioned actor has to call the unlockLatestValue function after * a new source update is posted. If the permissioned actor does not call unlockLatestValue within this window of a * new source price, the latest value will be made available to everyone without going through an MEV-Share auction. - * @return lockWindow time in seconds. */ function lockWindow() public view virtual returns (uint256); /** * @notice Max number of historical source updates to traverse when looking for a historic value in the past. - * @return maxTraversal max number of historical source updates to traverse. */ function maxTraversal() public view virtual returns (uint256); + + /** + * @notice Max age of a historical price that can be used instead of the current price. + */ + function maxAge() public view virtual returns (uint256); } diff --git a/src/adapters/source-adapters/ChainlinkSourceAdapter.sol b/src/adapters/source-adapters/ChainlinkSourceAdapter.sol index 7fb3fae..fe3dbf9 100644 --- a/src/adapters/source-adapters/ChainlinkSourceAdapter.sol +++ b/src/adapters/source-adapters/ChainlinkSourceAdapter.sol @@ -94,7 +94,9 @@ abstract contract ChainlinkSourceAdapter is DiamondRootOval { _searchRoundDataAt(timestamp, roundId, maxTraversal); // Validate returned data. If it is uninitialized we fallback to returning the current latest round data. - if (historicalUpdatedAt > 0) return (historicalAnswer, historicalUpdatedAt, historicalRoundId); + if (historicalUpdatedAt > block.timestamp - maxAge()) { + return (historicalAnswer, historicalUpdatedAt, historicalRoundId); + } return (answer, updatedAt, roundId); } diff --git a/src/adapters/source-adapters/SnapshotSource.sol b/src/adapters/source-adapters/SnapshotSource.sol index 52a00be..358743e 100644 --- a/src/adapters/source-adapters/SnapshotSource.sol +++ b/src/adapters/source-adapters/SnapshotSource.sol @@ -55,8 +55,8 @@ abstract contract SnapshotSource is DiamondRootOval { // Attempt traversing historical snapshot data. This might still be newer or uninitialized. Snapshot memory historicalData = _searchSnapshotAt(timestamp, maxTraversal); - // Validate returned data. If it is uninitialized we fallback to returning the current latest round data. - if (historicalData.timestamp > 0) return historicalData; + // 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; return latestData; } diff --git a/src/controllers/BaseController.sol b/src/controllers/BaseController.sol index b4b7504..36a30dd 100644 --- a/src/controllers/BaseController.sol +++ b/src/controllers/BaseController.sol @@ -12,6 +12,7 @@ abstract contract BaseController is Ownable, Oval { // these don't need to be public since they can be accessed via the accessor functions below. uint256 private lockWindow_ = 60; // The lockWindow in seconds. uint256 private maxTraversal_ = 10; // The maximum number of rounds to traverse when looking for historical data. + uint256 private maxAge_ = 1 days; // Default 1 day. mapping(address => bool) public unlockers; @@ -66,6 +67,16 @@ abstract contract BaseController is Ownable, Oval { emit MaxTraversalSet(newMaxTraversal); } + /** + * @notice Enables the owner to set the maxAge. + * @param newMaxAge The maxAge to set + */ + function setMaxAge(uint256 newMaxAge) public onlyOwner { + maxAge_ = newMaxAge; + + emit MaxAgeSet(newMaxAge); + } + /** * @notice Time window that bounds how long the permissioned actor has to call the unlockLatestValue function after * a new source update is posted. If the permissioned actor does not call unlockLatestValue within this window of a @@ -83,4 +94,11 @@ abstract contract BaseController is Ownable, Oval { function maxTraversal() public view override returns (uint256) { return maxTraversal_; } + + /** + * @notice Max age of a historical price that can be used instead of the current price. + */ + function maxAge() public view override returns (uint256) { + return maxAge_; + } } diff --git a/src/controllers/ImmutableController.sol b/src/controllers/ImmutableController.sol index cadbe06..0b48665 100644 --- a/src/controllers/ImmutableController.sol +++ b/src/controllers/ImmutableController.sol @@ -13,12 +13,14 @@ import {Oval} from "../Oval.sol"; abstract contract ImmutableController is Oval { uint256 private immutable LOCK_WINDOW; // The lockWindow in seconds. uint256 private immutable MAX_TRAVERSAL; // The maximum number of rounds to traverse when looking for historical data. + uint256 private immutable MAX_AGE; mapping(address => bool) public unlockers; - constructor(uint256 _lockWindow, uint256 _maxTraversal, address[] memory _unlockers) { + constructor(uint256 _lockWindow, uint256 _maxTraversal, address[] memory _unlockers, uint256 _maxAge) { LOCK_WINDOW = _lockWindow; MAX_TRAVERSAL = _maxTraversal; + MAX_AGE = _maxAge; for (uint256 i = 0; i < _unlockers.length; i++) { unlockers[_unlockers[i]] = true; @@ -27,6 +29,7 @@ abstract contract ImmutableController is Oval { emit LockWindowSet(_lockWindow); emit MaxTraversalSet(_maxTraversal); + emit MaxAgeSet(_maxAge); } /** @@ -57,4 +60,11 @@ abstract contract ImmutableController is Oval { function maxTraversal() public view override returns (uint256) { return MAX_TRAVERSAL; } + + /** + * @notice Max age of a historical price that can be used instead of the current price. + */ + function maxAge() public view override returns (uint256) { + return MAX_AGE; + } } diff --git a/src/interfaces/IBaseController.sol b/src/interfaces/IBaseController.sol index 8137608..57e6793 100644 --- a/src/interfaces/IBaseController.sol +++ b/src/interfaces/IBaseController.sol @@ -5,6 +5,7 @@ interface IBaseController { event LockWindowSet(uint256 indexed lockWindow); event MaxTraversalSet(uint256 indexed maxTraversal); event UnlockerSet(address indexed unlocker, bool indexed allowed); + event MaxAgeSet(uint256 indexed newMaxAge); function canUnlock(address caller, uint256 cachedLatestTimestamp) external view returns (bool); } diff --git a/test/fork/aave/AaveV2.Liquidation.sol b/test/fork/aave/AaveV2.Liquidation.sol index e1ed042..2101af0 100644 --- a/test/fork/aave/AaveV2.Liquidation.sol +++ b/test/fork/aave/AaveV2.Liquidation.sol @@ -20,7 +20,7 @@ interface Usdc is IERC20 { contract TestedOval is ImmutableController, ChainlinkSourceAdapter, ChainlinkDestinationAdapter { constructor(IAggregatorV3Source source, uint8 decimals, address[] memory unlockers) ChainlinkSourceAdapter(source) - ImmutableController(60, 10, unlockers) + ImmutableController(60, 10, unlockers, 86400) ChainlinkDestinationAdapter(decimals) {} } diff --git a/test/fork/aave/AaveV3.Liquidation.sol b/test/fork/aave/AaveV3.Liquidation.sol index 4788c47..2e2dcfd 100644 --- a/test/fork/aave/AaveV3.Liquidation.sol +++ b/test/fork/aave/AaveV3.Liquidation.sol @@ -20,7 +20,7 @@ interface Usdc is IERC20 { contract TestedOval is ImmutableController, ChainlinkSourceAdapter, ChainlinkDestinationAdapter { constructor(IAggregatorV3Source source, uint8 decimals, address[] memory unlockers) ChainlinkSourceAdapter(source) - ImmutableController(60, 10, unlockers) + ImmutableController(60, 10, unlockers, 86400) ChainlinkDestinationAdapter(decimals) {} } diff --git a/test/fork/adapters/ChainlinkSourceAdapter.sol b/test/fork/adapters/ChainlinkSourceAdapter.sol index 6ec2c96..d24e3f2 100644 --- a/test/fork/adapters/ChainlinkSourceAdapter.sol +++ b/test/fork/adapters/ChainlinkSourceAdapter.sol @@ -8,18 +8,8 @@ import {ChainlinkSourceAdapter} from "../../../src/adapters/source-adapters/Chai import {DecimalLib} from "../../../src/adapters/lib/DecimalLib.sol"; import {IAggregatorV3Source} from "../../../src/interfaces/chainlink/IAggregatorV3Source.sol"; -contract TestedSourceAdapter is ChainlinkSourceAdapter { +contract TestedSourceAdapter is ChainlinkSourceAdapter, BaseController { constructor(IAggregatorV3Source source) ChainlinkSourceAdapter(source) {} - - function internalLatestData() public view override returns (int256, uint256, uint256) {} - - function internalDataAtRound(uint256 roundId) public view override returns (int256, uint256) {} - - function canUnlock(address caller, uint256 cachedLatestTimestamp) public view virtual override returns (bool) {} - - function lockWindow() public view virtual override returns (uint256) {} - - function maxTraversal() public view virtual override returns (uint256) {} } contract ChainlinkSourceAdapterTest is CommonTest { @@ -107,6 +97,33 @@ contract ChainlinkSourceAdapterTest is CommonTest { assertTrue(uint256(roundId) == lookBackRoundId); } + function testCorrectlyBoundsMaxLooBackByMaxAge() public { + // Value returned at 2 days should be the same as the value returned at 1 day as the max age is 1 day. + assertTrue(sourceAdapter.maxAge() == 1 days); + (int256 lookBackPricePastWindow, uint256 lookBackTimestampPastWindow, uint256 lookBackRoundIdPastWindow) = + sourceAdapter.tryLatestDataAt(block.timestamp - 2 days, 50); + + (int256 lookBackPriceAtLimit, uint256 lookBackTimestampAtLimit, uint256 lookBackRoundIdAtLimit) = + sourceAdapter.tryLatestDataAt(block.timestamp - 1 days, 50); + + assertTrue(lookBackPricePastWindow == lookBackPriceAtLimit); + assertTrue(lookBackTimestampPastWindow == lookBackTimestampAtLimit); + assertTrue(lookBackRoundIdPastWindow == lookBackRoundIdAtLimit); + } + + function testExtendingMaxAgeCorrectlyExtendsWindowOfReturnedValue() public { + sourceAdapter.setMaxAge(2 days); + (int256 lookBackPricePastWindow, uint256 lookBackTimestampPastWindow, uint256 lookBackRoundIdPastWindow) = + sourceAdapter.tryLatestDataAt(block.timestamp - 3 days, 50); + + (int256 lookBackPriceAtLimit, uint256 lookBackTimestampAtLimit, uint256 lookBackRoundIdAtLimit) = + sourceAdapter.tryLatestDataAt(block.timestamp - 2 days, 50); + + assertTrue(lookBackPricePastWindow == lookBackPriceAtLimit); + assertTrue(lookBackTimestampPastWindow == lookBackTimestampAtLimit); + assertTrue(lookBackRoundIdPastWindow == lookBackRoundIdAtLimit); + } + function testNonHistoricalData() public { uint256 targetTime = block.timestamp - 1 hours; diff --git a/test/fork/adapters/ChronicleMedianSourceAdapter.sol b/test/fork/adapters/ChronicleMedianSourceAdapter.sol index 1c184f7..1420591 100644 --- a/test/fork/adapters/ChronicleMedianSourceAdapter.sol +++ b/test/fork/adapters/ChronicleMedianSourceAdapter.sol @@ -3,16 +3,12 @@ pragma solidity 0.8.17; import {CommonTest} from "../../Common.sol"; +import {BaseController} from "../../../src/controllers/BaseController.sol"; import {ChronicleMedianSourceAdapter} from "../../../src/adapters/source-adapters/ChronicleMedianSourceAdapter.sol"; import {IMedian} from "../../../src/interfaces/chronicle/IMedian.sol"; -contract TestedSourceAdapter is ChronicleMedianSourceAdapter { +contract TestedSourceAdapter is ChronicleMedianSourceAdapter, BaseController { constructor(IMedian source) ChronicleMedianSourceAdapter(source) {} - function internalLatestData() public view override returns (int256, uint256, uint256) {} - function internalDataAtRound(uint256 roundId) public view override returns (int256, uint256) {} - function canUnlock(address caller, uint256 cachedLatestTimestamp) public view virtual override returns (bool) {} - function lockWindow() public view virtual override returns (uint256) {} - function maxTraversal() public view virtual override returns (uint256) {} } contract ChronicleMedianSourceAdapterTest is CommonTest { diff --git a/test/fork/adapters/OSMSourceAdapter.sol b/test/fork/adapters/OSMSourceAdapter.sol index b9d34e6..bf818dd 100644 --- a/test/fork/adapters/OSMSourceAdapter.sol +++ b/test/fork/adapters/OSMSourceAdapter.sol @@ -3,17 +3,13 @@ pragma solidity 0.8.17; import {CommonTest} from "../../Common.sol"; +import {BaseController} from "../../../src/controllers/BaseController.sol"; import {OSMSourceAdapter} from "../../../src/adapters/source-adapters/OSMSourceAdapter.sol"; import {IOSM} from "../../../src/interfaces/makerdao/IOSM.sol"; import {IMedian} from "../../../src/interfaces/chronicle/IMedian.sol"; -contract TestedSourceAdapter is OSMSourceAdapter { +contract TestedSourceAdapter is OSMSourceAdapter, BaseController { constructor(IOSM source) OSMSourceAdapter(source) {} - function internalLatestData() public view override returns (int256, uint256, uint256) {} - function internalDataAtRound(uint256 roundId) public view override returns (int256, uint256) {} - function canUnlock(address caller, uint256 cachedLatestTimestamp) public view virtual override returns (bool) {} - function lockWindow() public view virtual override returns (uint256) {} - function maxTraversal() public view virtual override returns (uint256) {} } contract OSMSourceAdapterTest is CommonTest { diff --git a/test/fork/adapters/PythSourceAdapter.sol b/test/fork/adapters/PythSourceAdapter.sol index 55c4f0d..7268829 100644 --- a/test/fork/adapters/PythSourceAdapter.sol +++ b/test/fork/adapters/PythSourceAdapter.sol @@ -3,21 +3,12 @@ pragma solidity 0.8.17; import {CommonTest} from "../../Common.sol"; +import {BaseController} from "../../../src/controllers/BaseController.sol"; import {PythSourceAdapter} from "../../../src/adapters/source-adapters/PythSourceAdapter.sol"; import {IPyth} from "../../../src/interfaces/pyth/IPyth.sol"; -contract TestedSourceAdapter is PythSourceAdapter { +contract TestedSourceAdapter is PythSourceAdapter, BaseController { constructor(IPyth source, bytes32 priceId) PythSourceAdapter(source, priceId) {} - - function internalLatestData() public view override returns (int256, uint256, uint256) {} - - function internalDataAtRound(uint256 roundId) public view override returns (int256, uint256) {} - - function canUnlock(address caller, uint256 cachedLatestTimestamp) public view virtual override returns (bool) {} - - function lockWindow() public view virtual override returns (uint256) {} - - function maxTraversal() public view virtual override returns (uint256) {} } contract PythSourceAdapterTest is CommonTest { diff --git a/test/fork/adapters/UnionSourceAdapter.sol b/test/fork/adapters/UnionSourceAdapter.sol index 262e9c4..ca0e14b 100644 --- a/test/fork/adapters/UnionSourceAdapter.sol +++ b/test/fork/adapters/UnionSourceAdapter.sol @@ -2,26 +2,18 @@ pragma solidity 0.8.17; import {CommonTest} from "../../Common.sol"; + +import {BaseController} from "../../../src/controllers/BaseController.sol"; import {UnionSourceAdapter} from "../../../src/adapters/source-adapters/UnionSourceAdapter.sol"; import {IAggregatorV3Source} from "../../../src/interfaces/chainlink/IAggregatorV3Source.sol"; import {IMedian} from "../../../src/interfaces/chronicle/IMedian.sol"; import {IPyth} from "../../../src/interfaces/pyth/IPyth.sol"; import {DecimalLib} from "../../../src/adapters/lib/DecimalLib.sol"; -contract TestedSourceAdapter is UnionSourceAdapter { +contract TestedSourceAdapter is UnionSourceAdapter, BaseController { constructor(IAggregatorV3Source chainlink, IMedian chronicle, IPyth pyth, bytes32 pythPriceId) UnionSourceAdapter(chainlink, chronicle, pyth, pythPriceId) {} - - function internalLatestData() public view override returns (int256, uint256, uint256) {} - - function internalDataAtRound(uint256 roundId) public view override returns (int256, uint256) {} - - function canUnlock(address caller, uint256 cachedLatestTimestamp) public view virtual override returns (bool) {} - - function lockWindow() public view virtual override returns (uint256) {} - - function maxTraversal() public view virtual override returns (uint256) {} } contract UnionSourceAdapterTest is CommonTest { @@ -174,6 +166,7 @@ contract UnionSourceAdapterTest is CommonTest { // Fork to a block where chronicle was the newest. vm.createSelectFork("mainnet", targetChronicleBlock); uint256 targetTimestamp = block.timestamp; + sourceAdapter.setMaxAge(2 days); // Set max age to 2 days to disable this logic for the test. _whitelistOnChronicle(); // Snapshotting union adapter should not affect historical lookups, but we do it just to prove it does not interfere. diff --git a/test/fork/adapters/UniswapAnchoredViewSourceAdapter.sol b/test/fork/adapters/UniswapAnchoredViewSourceAdapter.sol index d04c3e7..0567ef2 100644 --- a/test/fork/adapters/UniswapAnchoredViewSourceAdapter.sol +++ b/test/fork/adapters/UniswapAnchoredViewSourceAdapter.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.17; import {CommonTest} from "../../Common.sol"; +import {BaseController} from "../../../src/controllers/BaseController.sol"; import {IValidatorProxyTest} from "../interfaces/compoundV2/IValidatorProxy.sol"; import {MockChainlinkV3Aggregator} from "../../mocks/MockChainlinkV3Aggregator.sol"; import {UniswapAnchoredViewSourceAdapter} from @@ -10,13 +11,8 @@ import {UniswapAnchoredViewSourceAdapter} from import {IAccessControlledAggregatorV3} from "../../../src/interfaces/chainlink/IAccessControlledAggregatorV3.sol"; import {IUniswapAnchoredView} from "../../../src/interfaces/compound/IUniswapAnchoredView.sol"; -contract TestedSourceAdapter is UniswapAnchoredViewSourceAdapter { +contract TestedSourceAdapter is UniswapAnchoredViewSourceAdapter, BaseController { constructor(IUniswapAnchoredView source, address cToken) UniswapAnchoredViewSourceAdapter(source, cToken) {} - function internalLatestData() public view override returns (int256, uint256, uint256) {} - function internalDataAtRound(uint256 roundId) public view override returns (int256, uint256) {} - function canUnlock(address caller, uint256 cachedLatestTimestamp) public view virtual override returns (bool) {} - function lockWindow() public view virtual override returns (uint256) {} - function maxTraversal() public view virtual override returns (uint256) {} } contract UniswapAnchoredViewSourceAdapterTest is CommonTest { diff --git a/test/fork/compound/CompoundV2.Liquidation.sol b/test/fork/compound/CompoundV2.Liquidation.sol index 5b617dd..d8d9df4 100644 --- a/test/fork/compound/CompoundV2.Liquidation.sol +++ b/test/fork/compound/CompoundV2.Liquidation.sol @@ -27,7 +27,7 @@ interface Usdc is IERC20 { contract TestedOval is ImmutableController, UniswapAnchoredViewSourceAdapter, BaseDestinationAdapter { constructor(IUniswapAnchoredView source, address cToken, address[] memory unlockers) UniswapAnchoredViewSourceAdapter(source, cToken) - ImmutableController(60, 10, unlockers) + ImmutableController(60, 10, unlockers, 86400) BaseDestinationAdapter() {} } diff --git a/test/unit/BaseController.sol b/test/unit/BaseController.sol index 8caffc0..7a2083c 100644 --- a/test/unit/BaseController.sol +++ b/test/unit/BaseController.sol @@ -65,4 +65,17 @@ contract BaseControllerTest is CommonTest { vm.expectRevert("Ownable: caller is not the owner"); baseController.setMaxTraversal(100); } + + function testOwnerCanSetMaxAge() public { + uint256 newMaxAge = 7200; // 2 hours in seconds, different from default 1 day + vm.prank(owner); + baseController.setMaxAge(newMaxAge); + assertTrue(baseController.maxAge() == newMaxAge); + } + + function testNonOwnerCannotSetMaxAge() public { + vm.prank(random); + vm.expectRevert("Ownable: caller is not the owner"); + baseController.setMaxAge(7200); + } } diff --git a/test/unit/ImmutableController.sol b/test/unit/ImmutableController.sol index 4bacf79..ee86a47 100644 --- a/test/unit/ImmutableController.sol +++ b/test/unit/ImmutableController.sol @@ -7,9 +7,15 @@ import {BaseDestinationAdapter} from "../../src/adapters/destination-adapters/Ba import {MockSourceAdapter} from "../mocks/MockSourceAdapter.sol"; contract TestImmutableController is ImmutableController, MockSourceAdapter, BaseDestinationAdapter { - constructor(uint8 decimals, uint256 _lockWindow, uint256 _maxTraversal, address[] memory _unlockers) + constructor( + uint8 decimals, + uint256 _lockWindow, + uint256 _maxTraversal, + address[] memory _unlockers, + uint256 _maxAge + ) MockSourceAdapter(decimals) - ImmutableController(_lockWindow, _maxTraversal, _unlockers) + ImmutableController(_lockWindow, _maxTraversal, _unlockers, _maxAge) BaseDestinationAdapter() {} } @@ -19,6 +25,7 @@ contract ImmutableControllerTest is CommonTest { uint256 lockWindow = 60; uint256 maxTraversal = 10; address[] unlockers; + uint256 maxAge = 86400; uint256 lastUnlockTime = 1690000000; @@ -28,7 +35,7 @@ contract ImmutableControllerTest is CommonTest { unlockers.push(permissionedUnlocker); vm.startPrank(owner); - immutableController = new TestImmutableController(decimals, lockWindow, maxTraversal, unlockers); + immutableController = new TestImmutableController(decimals, lockWindow, maxTraversal, unlockers, maxAge); vm.stopPrank(); } diff --git a/test/unit/SnapshotSource.SnapshotData.sol b/test/unit/SnapshotSource.SnapshotData.sol index 1e5cdbf..f5b501f 100644 --- a/test/unit/SnapshotSource.SnapshotData.sol +++ b/test/unit/SnapshotSource.SnapshotData.sol @@ -5,6 +5,7 @@ import {CommonTest} from "../Common.sol"; import {MockSnapshotSourceAdapter} from "../mocks/MockSnapshotSourceAdapter.sol"; import {Oval} from "../../src/Oval.sol"; import {BaseController} from "../../src/controllers/BaseController.sol"; +import "forge-std/console.sol"; contract TestSnapshotSource is MockSnapshotSourceAdapter, Oval, BaseController {} @@ -74,4 +75,41 @@ contract SnapshotSourceSnapshotDataTest is CommonTest { snapshot = snapshotSource.latestSnapshotData(); assertTrue(snapshot.answer == 100 && snapshot.timestamp == 1000); } + + function testMaxAgeIsRespected() public { + // Set maxAge to 2000 for testing + snapshotSource.setMaxAge(2000); + + // Publish data at different timestamps + vm.warp(1000); + snapshotSource.publishSourceData(100, 1000); + snapshotSource.snapshotData(); + + vm.warp(2000); + snapshotSource.publishSourceData(200, 2000); + snapshotSource.snapshotData(); + + vm.warp(3000); + snapshotSource.publishSourceData(300, 3000); + snapshotSource.snapshotData(); + + vm.warp(4000); + snapshotSource.publishSourceData(400, 4000); + snapshotSource.snapshotData(); + + // Verify behavior when requesting data within the maxAge limit + (int256 answerAt4000, uint256 timestampAt4000,) = snapshotSource.tryLatestDataAt(4000, 10); + assertTrue(answerAt4000 == 400 && timestampAt4000 == 4000); + + (int256 answerAt3000, uint256 timestampAt3000,) = snapshotSource.tryLatestDataAt(3000, 10); + assertTrue(answerAt3000 == 300 && timestampAt3000 == 3000); + + // Request data at the limit of maxAge should still work. + (int256 answerAt2000, uint256 timestampAt2000,) = snapshotSource.tryLatestDataAt(2000, 10); + assertTrue(answerAt2000 == 200 && timestampAt2000 == 2000); + + // Request data older than maxAge (1000), should get the latest available data at 4000. + (int256 answerAt1000, uint256 timestampAt1000,) = snapshotSource.tryLatestDataAt(1000, 10); + assertTrue(answerAt1000 == 400 && timestampAt1000 == 4000); + } } diff --git a/test/unit/adapters/BoundedUnionSource.SelectBoundedPrice.sol b/test/unit/adapters/BoundedUnionSource.SelectBoundedPrice.sol index 521c941..2b4c88d 100644 --- a/test/unit/adapters/BoundedUnionSource.SelectBoundedPrice.sol +++ b/test/unit/adapters/BoundedUnionSource.SelectBoundedPrice.sol @@ -39,6 +39,7 @@ contract TestBoundedUnionSource is BoundedUnionSourceAdapter { function lockWindow() public view virtual override returns (uint256) {} function maxTraversal() public view virtual override returns (uint256) {} + function maxAge() public view virtual override returns (uint256) {} } contract MinimalChainlinkAdapter { From 8af05b6c1282941a2db51db809b49debade922d2 Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 20 May 2024 17:44:17 -0400 Subject: [PATCH 2/2] fix build/tests Signed-off-by: Matt Rice --- src/controllers/MutableUnlockersController.sol | 13 ++++++++++++- src/factories/StandardChainlinkFactory.sol | 9 ++++++--- src/factories/StandardChronicleFactory.sol | 9 ++++++--- src/factories/StandardCoinbaseFactory.sol | 9 ++++++--- src/factories/StandardPythFactory.sol | 9 ++++++--- .../adapters/RedStoneAsChainlinkSourceAdapter.sol | 12 +----------- test/unit/CoinbaseSourceAdapter.sol | 12 +----------- test/unit/MutableUnlockersController.sol | 2 +- test/unit/RedStoneOracle.sol | 1 - test/unit/StandardChainlinkFactory.sol | 5 +++-- test/unit/StandardChronicleFactory.sol | 5 +++-- test/unit/StandardCoinbaseFactory.sol | 5 +++-- test/unit/StandardPythFactory.sol | 5 +++-- 13 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/controllers/MutableUnlockersController.sol b/src/controllers/MutableUnlockersController.sol index 1b6b89b..a7acf13 100644 --- a/src/controllers/MutableUnlockersController.sol +++ b/src/controllers/MutableUnlockersController.sol @@ -11,18 +11,22 @@ abstract contract MutableUnlockersController is Ownable, Oval { // these don't need to be public since they can be accessed via the accessor functions below. uint256 private immutable LOCK_WINDOW; // The lockWindow in seconds. uint256 private immutable MAX_TRAVERSAL; // The maximum number of rounds to traverse when looking for historical data. + uint256 private immutable MAX_AGE; // Max age for a historical price used by Oval instead of the current price. mapping(address => bool) public unlockers; - constructor(uint256 _lockWindow, uint256 _maxTraversal, address[] memory _unlockers) { + constructor(uint256 _lockWindow, uint256 _maxTraversal, address[] memory _unlockers, uint256 _maxAge) { LOCK_WINDOW = _lockWindow; MAX_TRAVERSAL = _maxTraversal; + MAX_AGE = _maxAge; + for (uint256 i = 0; i < _unlockers.length; i++) { setUnlocker(_unlockers[i], true); } emit LockWindowSet(_lockWindow); emit MaxTraversalSet(_maxTraversal); + emit MaxAgeSet(_maxAge); } /** @@ -65,4 +69,11 @@ abstract contract MutableUnlockersController is Ownable, Oval { function maxTraversal() public view override returns (uint256) { return MAX_TRAVERSAL; } + + /** + * @notice Max age of a historical price that can be used instead of the current price. + */ + function maxAge() public view override returns (uint256) { + return MAX_AGE; + } } diff --git a/src/factories/StandardChainlinkFactory.sol b/src/factories/StandardChainlinkFactory.sol index 62fd00e..0d16f42 100644 --- a/src/factories/StandardChainlinkFactory.sol +++ b/src/factories/StandardChainlinkFactory.sol @@ -18,10 +18,11 @@ contract OvalChainlink is MutableUnlockersController, ChainlinkSourceAdapter, Ch address[] memory unlockers, uint256 lockWindow, uint256 maxTraversal, + uint256 maxAge, address owner ) ChainlinkSourceAdapter(source) - MutableUnlockersController(lockWindow, maxTraversal, unlockers) + MutableUnlockersController(lockWindow, maxTraversal, unlockers, maxAge) ChainlinkDestinationAdapter(18) { _transferOwnership(owner); @@ -44,10 +45,12 @@ contract StandardChainlinkFactory is Ownable, BaseFactory { * @param source the Chainlink oracle source contract. * @param lockWindow the lockWindow used for this Oval instance. This is the length of the window * for the Oval auction to be run and, thus, the maximum time that prices will be delayed. + * @param maxAge max age of a price that is used in place of the current price. If the only available price is + * older than this, OEV is not captured and the current price is provided. * @return oval deployed oval address. */ - function create(IAggregatorV3Source source, uint256 lockWindow) external returns (address oval) { - oval = address(new OvalChainlink(source, defaultUnlockers, lockWindow, MAX_TRAVERSAL, owner())); + function create(IAggregatorV3Source source, uint256 lockWindow, uint256 maxAge) external returns (address oval) { + oval = address(new OvalChainlink(source, defaultUnlockers, lockWindow, MAX_TRAVERSAL, maxAge, owner())); emit OvalDeployed(msg.sender, oval, lockWindow, MAX_TRAVERSAL, owner(), defaultUnlockers); } } diff --git a/src/factories/StandardChronicleFactory.sol b/src/factories/StandardChronicleFactory.sol index 62633bc..b5025b7 100644 --- a/src/factories/StandardChronicleFactory.sol +++ b/src/factories/StandardChronicleFactory.sol @@ -19,10 +19,11 @@ contract OvalChronicle is MutableUnlockersController, ChronicleMedianSourceAdapt address[] memory _unlockers, uint256 _lockWindow, uint256 _maxTraversal, + uint256 _maxAge, address _owner ) ChronicleMedianSourceAdapter(_source) - MutableUnlockersController(_lockWindow, _maxTraversal, _unlockers) + MutableUnlockersController(_lockWindow, _maxTraversal, _unlockers, _maxAge) ChainlinkDestinationAdapter(18) { _transferOwnership(_owner); @@ -45,10 +46,12 @@ contract StandardChronicleFactory is Ownable, BaseFactory { * @param chronicle Chronicle source contract. * @param lockWindow the lockWindow used for this Oval instance. This is the length of the window * for the Oval auction to be run and, thus, the maximum time that prices will be delayed. + * @param maxAge max age of a price that is used in place of the current price. If the only available price is + * older than this, OEV is not captured and the current price is provided. * @return oval deployed oval address. */ - function create(IMedian chronicle, uint256 lockWindow) external returns (address oval) { - oval = address(new OvalChronicle(chronicle, defaultUnlockers, lockWindow, MAX_TRAVERSAL, owner())); + function create(IMedian chronicle, uint256 lockWindow, uint256 maxAge) external returns (address oval) { + oval = address(new OvalChronicle(chronicle, defaultUnlockers, lockWindow, MAX_TRAVERSAL, maxAge, owner())); emit OvalDeployed(msg.sender, oval, lockWindow, MAX_TRAVERSAL, owner(), defaultUnlockers); } } diff --git a/src/factories/StandardCoinbaseFactory.sol b/src/factories/StandardCoinbaseFactory.sol index 8d5740a..9aee47a 100644 --- a/src/factories/StandardCoinbaseFactory.sol +++ b/src/factories/StandardCoinbaseFactory.sol @@ -19,10 +19,11 @@ contract OvalCoinbase is MutableUnlockersController, CoinbaseSourceAdapter, Chai address[] memory _unlockers, uint256 _lockWindow, uint256 _maxTraversal, + uint256 _maxAge, address _owner ) CoinbaseSourceAdapter(_source, _ticker) - MutableUnlockersController(_lockWindow, _maxTraversal, _unlockers) + MutableUnlockersController(_lockWindow, _maxTraversal, _unlockers, _maxAge) ChainlinkDestinationAdapter(18) { _transferOwnership(_owner); @@ -49,10 +50,12 @@ contract StandardCoinbaseFactory is Ownable, BaseFactory { * @param ticker the Coinbase oracle's ticker. * @param lockWindow the lockWindow used for this Oval instance. This is the length of the window * for the Oval auction to be run and, thus, the maximum time that prices will be delayed. + * @param maxAge max age of a price that is used in place of the current price. If the only available price is + * older than this, OEV is not captured and the current price is provided. * @return oval deployed oval address. */ - function create(string memory ticker, uint256 lockWindow) external returns (address oval) { - oval = address(new OvalCoinbase(SOURCE, ticker, defaultUnlockers, lockWindow, MAX_TRAVERSAL, owner())); + function create(string memory ticker, uint256 lockWindow, uint256 maxAge) external returns (address oval) { + oval = address(new OvalCoinbase(SOURCE, ticker, defaultUnlockers, lockWindow, MAX_TRAVERSAL, maxAge, owner())); emit OvalDeployed(msg.sender, oval, lockWindow, MAX_TRAVERSAL, owner(), defaultUnlockers); } } diff --git a/src/factories/StandardPythFactory.sol b/src/factories/StandardPythFactory.sol index 9bcea08..1529e0a 100644 --- a/src/factories/StandardPythFactory.sol +++ b/src/factories/StandardPythFactory.sol @@ -20,10 +20,11 @@ contract OvalPyth is MutableUnlockersController, PythSourceAdapter, ChainlinkDes address[] memory unlockers, uint256 lockWindow, uint256 maxTraversal, + uint256 maxAge, address owner ) PythSourceAdapter(source, pythPriceId) - MutableUnlockersController(lockWindow, maxTraversal, unlockers) + MutableUnlockersController(lockWindow, maxTraversal, unlockers, maxAge) ChainlinkDestinationAdapter(18) { _transferOwnership(owner); @@ -50,10 +51,12 @@ contract StandardPythFactory is Ownable, BaseFactory { * @param pythPriceId the Pyth price id. * @param lockWindow the lockWindow used for this Oval instance. This is the length of the window * for the Oval auction to be run and, thus, the maximum time that prices will be delayed. + * @param maxAge max age of a price that is used in place of the current price. If the only available price is + * older than this, OEV is not captured and the current price is provided. * @return oval deployed oval address. */ - function create(bytes32 pythPriceId, uint256 lockWindow) external returns (address oval) { - oval = address(new OvalPyth(pyth, pythPriceId, defaultUnlockers, lockWindow, MAX_TRAVERSAL, owner())); + function create(bytes32 pythPriceId, uint256 lockWindow, uint256 maxAge) external returns (address oval) { + oval = address(new OvalPyth(pyth, pythPriceId, defaultUnlockers, lockWindow, MAX_TRAVERSAL, maxAge, owner())); emit OvalDeployed(msg.sender, oval, lockWindow, MAX_TRAVERSAL, owner(), defaultUnlockers); } } diff --git a/test/fork/adapters/RedStoneAsChainlinkSourceAdapter.sol b/test/fork/adapters/RedStoneAsChainlinkSourceAdapter.sol index d1430eb..816cd18 100644 --- a/test/fork/adapters/RedStoneAsChainlinkSourceAdapter.sol +++ b/test/fork/adapters/RedStoneAsChainlinkSourceAdapter.sol @@ -14,18 +14,8 @@ import {IAggregatorV3Source} from "../../../src/interfaces/chainlink/IAggregator import {MergedPriceFeedAdapterWithRounds} from "redstone-oracle/on-chain-relayer/contracts/price-feeds/with-rounds/MergedPriceFeedAdapterWithRounds.sol"; -contract TestedSourceAdapter is ChainlinkSourceAdapter { +contract TestedSourceAdapter is ChainlinkSourceAdapter, BaseController { constructor(IAggregatorV3Source source) ChainlinkSourceAdapter(source) {} - - function internalLatestData() public view override returns (int256, uint256, uint256) {} - - function internalDataAtRound(uint256 roundId) public view override returns (int256, uint256) {} - - function canUnlock(address caller, uint256 cachedLatestTimestamp) public view virtual override returns (bool) {} - - function lockWindow() public view virtual override returns (uint256) {} - - function maxTraversal() public view virtual override returns (uint256) {} } contract RedstoneAsChainlinkSourceAdapterTest is CommonTest { diff --git a/test/unit/CoinbaseSourceAdapter.sol b/test/unit/CoinbaseSourceAdapter.sol index 120570e..a6324cb 100644 --- a/test/unit/CoinbaseSourceAdapter.sol +++ b/test/unit/CoinbaseSourceAdapter.sol @@ -9,18 +9,8 @@ import {DecimalLib} from "../../src/adapters/lib/DecimalLib.sol"; import {IAggregatorV3SourceCoinbase} from "../../src/interfaces/coinbase/IAggregatorV3SourceCoinbase.sol"; import {CoinbaseOracle} from "../../src/oracles/CoinbaseOracle.sol"; -contract TestedSourceAdapter is CoinbaseSourceAdapter { +contract TestedSourceAdapter is CoinbaseSourceAdapter, BaseController { constructor(IAggregatorV3SourceCoinbase source, string memory ticker) CoinbaseSourceAdapter(source, ticker) {} - - function internalLatestData() public view override returns (int256, uint256, uint256) {} - - function canUnlock(address caller, uint256 cachedLatestTimestamp) public view virtual override returns (bool) {} - - function lockWindow() public view virtual override returns (uint256) {} - - function maxTraversal() public view virtual override returns (uint256) {} - - function internalDataAtRound(uint256 roundId) public view override returns (int256, uint256) {} } contract CoinbaseSourceAdapterTest is CommonTest { diff --git a/test/unit/MutableUnlockersController.sol b/test/unit/MutableUnlockersController.sol index 5abd105..45ed31d 100644 --- a/test/unit/MutableUnlockersController.sol +++ b/test/unit/MutableUnlockersController.sol @@ -7,7 +7,7 @@ import {BaseDestinationAdapter} from "../../src/adapters/destination-adapters/Ba contract TestMutableUnlockersController is MutableUnlockersController, MockSourceAdapter, BaseDestinationAdapter { constructor(address[] memory _unlockers) - MutableUnlockersController(300, 15, _unlockers) + MutableUnlockersController(300, 15, _unlockers, 86400) MockSourceAdapter(18) // Assuming 18 decimals for the mock source adapter BaseDestinationAdapter() {} diff --git a/test/unit/RedStoneOracle.sol b/test/unit/RedStoneOracle.sol index c3c4d15..432674e 100644 --- a/test/unit/RedStoneOracle.sol +++ b/test/unit/RedStoneOracle.sol @@ -16,7 +16,6 @@ import {IAggregatorV3Source} from "../../src/interfaces/chainlink/IAggregatorV3S import {TestedSourceAdapter} from "../fork/adapters/ChainlinkSourceAdapter.sol"; - contract MockRedstonePayload is CommonTest { function getRedstonePayload(string memory priceFeed) public returns (bytes memory) { string[] memory args = new string[](4); diff --git a/test/unit/StandardChainlinkFactory.sol b/test/unit/StandardChainlinkFactory.sol index c24cf72..eee0a8a 100644 --- a/test/unit/StandardChainlinkFactory.sol +++ b/test/unit/StandardChainlinkFactory.sol @@ -13,6 +13,7 @@ contract StandardChainlinkFactoryTest is CommonTest { address[] unlockers; uint256 lockWindow = 300; uint256 maxTraversal = 15; + uint256 maxAge = 86400; function setUp() public { mockSource = new MockChainlinkV3Aggregator(8, 420); @@ -21,7 +22,7 @@ contract StandardChainlinkFactoryTest is CommonTest { } function testCreateMutableUnlockerOvalChainlink() public { - address created = factory.create(IAggregatorV3Source(address(mockSource)), lockWindow); + address created = factory.create(IAggregatorV3Source(address(mockSource)), lockWindow, maxAge); assertTrue(created != address(0)); // Check if the address is set, non-zero. @@ -37,7 +38,7 @@ contract StandardChainlinkFactoryTest is CommonTest { } function testOwnerCanChangeUnlockers() public { - address created = factory.create(IAggregatorV3Source(address(mockSource)), lockWindow); + address created = factory.create(IAggregatorV3Source(address(mockSource)), lockWindow, maxAge); OvalChainlink instance = OvalChainlink(created); address newUnlocker = address(0x789); diff --git a/test/unit/StandardChronicleFactory.sol b/test/unit/StandardChronicleFactory.sol index 6c21a47..cd5694c 100644 --- a/test/unit/StandardChronicleFactory.sol +++ b/test/unit/StandardChronicleFactory.sol @@ -12,6 +12,7 @@ contract StandardChronicleFactoryTest is CommonTest { address[] unlockers; uint256 lockWindow = 300; uint256 maxTraversal = 15; + uint256 maxAge = 86400; function setUp() public { mockSource = IMedian(address(0x456)); @@ -20,7 +21,7 @@ contract StandardChronicleFactoryTest is CommonTest { } function testCreateMutableUnlockerOvalChronicle() public { - address created = factory.create(mockSource, lockWindow); + address created = factory.create(mockSource, lockWindow, maxAge); assertTrue(created != address(0)); // Check if the address is set, non-zero. @@ -36,7 +37,7 @@ contract StandardChronicleFactoryTest is CommonTest { } function testOwnerCanChangeUnlockers() public { - address created = factory.create(mockSource, lockWindow); + address created = factory.create(mockSource, lockWindow, maxAge); OvalChronicle instance = OvalChronicle(created); address newUnlocker = address(0x789); diff --git a/test/unit/StandardCoinbaseFactory.sol b/test/unit/StandardCoinbaseFactory.sol index 91cac2c..b7e201a 100644 --- a/test/unit/StandardCoinbaseFactory.sol +++ b/test/unit/StandardCoinbaseFactory.sol @@ -14,6 +14,7 @@ contract StandardCoinbaseFactoryTest is CommonTest { uint256 lockWindow = 300; uint256 maxTraversal = 15; string ticker = "test ticker"; + uint256 maxAge = 86400; function setUp() public { mockSource = IAggregatorV3SourceCoinbase(address(new MockChainlinkV3Aggregator(8, 420))); @@ -22,7 +23,7 @@ contract StandardCoinbaseFactoryTest is CommonTest { } function testCreateMutableUnlockerOvalCoinbase() public { - address created = factory.create(ticker, lockWindow); + address created = factory.create(ticker, lockWindow, maxAge); assertTrue(created != address(0)); // Check if the address is set, non-zero. @@ -38,7 +39,7 @@ contract StandardCoinbaseFactoryTest is CommonTest { } function testOwnerCanChangeUnlockers() public { - address created = factory.create(ticker, lockWindow); + address created = factory.create(ticker, lockWindow, maxAge); OvalCoinbase instance = OvalCoinbase(created); address newUnlocker = address(0x789); diff --git a/test/unit/StandardPythFactory.sol b/test/unit/StandardPythFactory.sol index de80a60..afa0977 100644 --- a/test/unit/StandardPythFactory.sol +++ b/test/unit/StandardPythFactory.sol @@ -12,6 +12,7 @@ contract StandardPythFactoryTest is CommonTest { address[] unlockers; uint256 lockWindow = 300; uint256 maxTraversal = 15; + uint256 maxAge = 86400; function setUp() public { mockSource = IPyth(address(0x456)); @@ -20,7 +21,7 @@ contract StandardPythFactoryTest is CommonTest { } function testCreateMutableUnlockerOvalPyth() public { - address created = factory.create(bytes32(uint256(0x789)), lockWindow); + address created = factory.create(bytes32(uint256(0x789)), lockWindow, maxAge); assertTrue(created != address(0)); // Check if the address is set, non-zero. @@ -36,7 +37,7 @@ contract StandardPythFactoryTest is CommonTest { } function testOwnerCanChangeUnlockers() public { - address created = factory.create(bytes32(uint256(0x789)), lockWindow); + address created = factory.create(bytes32(uint256(0x789)), lockWindow, maxAge); OvalPyth instance = OvalPyth(created); address newUnlocker = address(0x789);