Skip to content

Commit f0d5316

Browse files
bowdchapati23philbow61
authored
Revert "feat: remove maxTimestampSpread and only rely on expiry" (#504)
This reverts commit ac1c5406441c01119eaf9c0de54512f7cfe1ff19. ### Description Brings back the maxTimestampSpread setting. A visual explanation here: <img width="918" alt="image" src="https://github.com/user-attachments/assets/02dfaadb-f029-4f38-8bd5-965644b4a4b5"> We need it because CELO/USD has a heartbeat of 24hr and PHP/USD 5minutes, therefore we could be in a situation where the spread between reports is quite large, and if we rely only on max timestamp spread we could be reporting with stale data. The rule of thumb is: - Set `maxTimestampSpread` by considering the largest heartbeat frequency. - Set `tokenExpiry` in SortedOracles by considering the shortest heartbeat frequency. ### Other changes N/A ### Tested Yup ### Related issues N/A ### Backwards compatibility N/A ### Documentation N/A --------- Co-authored-by: chapati <[email protected]> Co-authored-by: philbow61 <[email protected]>
1 parent 6feadf3 commit f0d5316

File tree

7 files changed

+180
-70
lines changed

7 files changed

+180
-70
lines changed

contracts/interfaces/IChainlinkRelayer.sol

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,7 @@ interface IChainlinkRelayer {
2121

2222
function getAggregators() external returns (ChainlinkAggregator[] memory);
2323

24+
function maxTimestampSpread() external returns (uint256);
25+
2426
function relay() external;
2527
}

contracts/interfaces/IChainlinkRelayerFactory.sol

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ interface IChainlinkRelayerFactory {
4444
function deployRelayer(
4545
address rateFeedId,
4646
string calldata rateFeedDescription,
47+
uint256 maxTimestampSpread,
4748
IChainlinkRelayer.ChainlinkAggregator[] calldata aggregators
4849
) external returns (address);
4950

@@ -52,6 +53,7 @@ interface IChainlinkRelayerFactory {
5253
function redeployRelayer(
5354
address rateFeedId,
5455
string calldata rateFeedDescription,
56+
uint256 maxTimestampSpread,
5557
IChainlinkRelayer.ChainlinkAggregator[] calldata aggregators
5658
) external returns (address);
5759

@@ -62,6 +64,7 @@ interface IChainlinkRelayerFactory {
6264
function computedRelayerAddress(
6365
address rateFeedId,
6466
string calldata rateFeedDescription,
67+
uint256 maxTimestampSpread,
6568
IChainlinkRelayer.ChainlinkAggregator[] calldata aggregators
6669
) external returns (address);
6770
}

contracts/oracles/ChainlinkRelayerFactory.sol

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ contract ChainlinkRelayerFactory is IChainlinkRelayerFactory, OwnableUpgradeable
105105
* @notice Deploys a new relayer contract.
106106
* @param rateFeedId The rate feed ID for which the relayer will report.
107107
* @param rateFeedDescription Human-readable rate feed, which the relayer will report on, i.e. "CELO/USD".
108+
* @param maxTimestampSpread Max difference in milliseconds between the earliest and
109+
* latest timestamp of all aggregators in the price path.
108110
* @param aggregators Array of ChainlinkAggregator structs defining the price path.
109111
* See contract-level @dev comment in the ChainlinkRelayerV1 contract,
110112
* for an explanation on price paths.
@@ -113,13 +115,14 @@ contract ChainlinkRelayerFactory is IChainlinkRelayerFactory, OwnableUpgradeable
113115
function deployRelayer(
114116
address rateFeedId,
115117
string calldata rateFeedDescription,
118+
uint256 maxTimestampSpread,
116119
IChainlinkRelayer.ChainlinkAggregator[] calldata aggregators
117120
) public onlyDeployer returns (address relayerAddress) {
118121
if (address(deployedRelayers[rateFeedId]) != address(0)) {
119122
revert RelayerForFeedExists(rateFeedId);
120123
}
121124

122-
address expectedAddress = computedRelayerAddress(rateFeedId, rateFeedDescription, aggregators);
125+
address expectedAddress = computedRelayerAddress(rateFeedId, rateFeedDescription, maxTimestampSpread, aggregators);
123126
if (expectedAddress.code.length > 0) {
124127
revert ContractAlreadyExists(expectedAddress, rateFeedId);
125128
}
@@ -129,6 +132,7 @@ contract ChainlinkRelayerFactory is IChainlinkRelayerFactory, OwnableUpgradeable
129132
rateFeedId,
130133
rateFeedDescription,
131134
sortedOracles,
135+
maxTimestampSpread,
132136
aggregators
133137
);
134138

@@ -176,16 +180,19 @@ contract ChainlinkRelayerFactory is IChainlinkRelayerFactory, OwnableUpgradeable
176180
* has been upgraded since the last deployment of the relayer).
177181
* @param rateFeedId The rate feed ID for which the relayer will report.
178182
* @param rateFeedDescription Human-readable rate feed, which the relayer will report on, i.e. "CELO/USD".
183+
* @param maxTimestampSpread Max difference in milliseconds between the earliest and
184+
* latest timestamp of all aggregators in the price path.
179185
* @param aggregators Array of ChainlinkAggregator structs defining the price path.
180186
* @return relayerAddress The address of the newly deployed relayer contract.
181187
*/
182188
function redeployRelayer(
183189
address rateFeedId,
184190
string calldata rateFeedDescription,
191+
uint256 maxTimestampSpread,
185192
IChainlinkRelayer.ChainlinkAggregator[] calldata aggregators
186193
) external onlyDeployer returns (address relayerAddress) {
187194
removeRelayer(rateFeedId);
188-
return deployRelayer(rateFeedId, rateFeedDescription, aggregators);
195+
return deployRelayer(rateFeedId, rateFeedDescription, maxTimestampSpread, aggregators);
189196
}
190197

191198
/**
@@ -224,12 +231,15 @@ contract ChainlinkRelayerFactory is IChainlinkRelayerFactory, OwnableUpgradeable
224231
* @notice Computes the expected CREATE2 address for given relayer parameters.
225232
* @param rateFeedId The rate feed ID.
226233
* @param rateFeedDescription The human readable description of the reported rate feed.
234+
* @param maxTimestampSpread Max difference in milliseconds between the earliest and
235+
* latest timestamp of all aggregators in the price path.
227236
* @param aggregators Array of ChainlinkAggregator structs defining the price path.
228237
* @dev See https://eips.ethereum.org/EIPS/eip-1014.
229238
*/
230239
function computedRelayerAddress(
231240
address rateFeedId,
232241
string calldata rateFeedDescription,
242+
uint256 maxTimestampSpread,
233243
IChainlinkRelayer.ChainlinkAggregator[] calldata aggregators
234244
) public view returns (address) {
235245
bytes32 salt = _getSalt();
@@ -245,7 +255,7 @@ contract ChainlinkRelayerFactory is IChainlinkRelayerFactory, OwnableUpgradeable
245255
keccak256(
246256
abi.encodePacked(
247257
type(ChainlinkRelayerV1).creationCode,
248-
abi.encode(rateFeedId, rateFeedDescription, sortedOracles, aggregators)
258+
abi.encode(rateFeedId, rateFeedDescription, sortedOracles, maxTimestampSpread, aggregators)
249259
)
250260
)
251261
)

contracts/oracles/ChainlinkRelayerV1.sol

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,13 @@ contract ChainlinkRelayerV1 is IChainlinkRelayer {
8989
/// @notice The number of aggregators provided during construction 1 <= aggregatorCount <= 4.
9090
uint256 private immutable aggregatorCount;
9191

92+
/**
93+
* @notice Maximum timestamp deviation allowed between all report timestamps pulled
94+
* from the Chainlink aggregators.
95+
* @dev Only relevant when aggregatorCount > 1.
96+
*/
97+
uint256 public immutable maxTimestampSpread;
98+
9299
/**
93100
* @notice Human-readable description of the rate feed.
94101
* @dev Should only be used off-chain for easier debugging / UI generation,
@@ -102,6 +109,10 @@ contract ChainlinkRelayerV1 is IChainlinkRelayer {
102109
/// @notice Used when more than four aggregators are passed into the constructor.
103110
error TooManyAggregators();
104111

112+
/// @notice Used when a) there is more than 1 aggregator and the maxTimestampSpread is 0,
113+
/// OR b) when there is only 1 aggregator and the maxTimestampSpread is not 0.
114+
error InvalidMaxTimestampSpread();
115+
105116
/// @notice Used when a new price's timestamp is not newer than the most recent SortedOracles timestamp.
106117
error TimestampNotNew();
107118

@@ -110,7 +121,11 @@ contract ChainlinkRelayerV1 is IChainlinkRelayer {
110121

111122
/// @notice Used when a negative or zero price is returned by the Chainlink aggregator.
112123
error InvalidPrice();
113-
124+
/**
125+
* @notice Used when the spread between the earliest and latest timestamp
126+
* of the aggregators is above the maximum allowed.
127+
*/
128+
error TimestampSpreadTooHigh();
114129
/**
115130
* @notice Used when trying to recover from a lesser/greater revert and there are
116131
* too many existing reports in SortedOracles.
@@ -128,16 +143,20 @@ contract ChainlinkRelayerV1 is IChainlinkRelayer {
128143
* @param _rateFeedId ID of the rate feed this relayer instance relays for.
129144
* @param _rateFeedDescription The human-readable description of the reported rate feed.
130145
* @param _sortedOracles Address of the SortedOracles contract to relay to.
146+
* @param _maxTimestampSpread Max difference in milliseconds between the earliest and
147+
* latest timestamp of all aggregators in the price path.
131148
* @param _aggregators Array of ChainlinkAggregator structs defining the price path.
132149
*/
133150
constructor(
134151
address _rateFeedId,
135152
string memory _rateFeedDescription,
136153
address _sortedOracles,
154+
uint256 _maxTimestampSpread,
137155
ChainlinkAggregator[] memory _aggregators
138156
) {
139157
rateFeedId = _rateFeedId;
140158
sortedOracles = _sortedOracles;
159+
maxTimestampSpread = _maxTimestampSpread;
141160
rateFeedDescription = _rateFeedDescription;
142161

143162
aggregatorCount = _aggregators.length;
@@ -149,6 +168,10 @@ contract ChainlinkRelayerV1 is IChainlinkRelayer {
149168
revert TooManyAggregators();
150169
}
151170

171+
if ((aggregatorCount > 1 && _maxTimestampSpread == 0) || (aggregatorCount == 1 && _maxTimestampSpread != 0)) {
172+
revert InvalidMaxTimestampSpread();
173+
}
174+
152175
ChainlinkAggregator[] memory aggregators = new ChainlinkAggregator[](4);
153176
for (uint256 i = 0; i < _aggregators.length; i++) {
154177
if (_aggregators[i].aggregator == address(0)) {
@@ -183,7 +206,8 @@ contract ChainlinkRelayerV1 is IChainlinkRelayer {
183206
* @dev Performs checks on the timestamp, will revert if any fails:
184207
* - The most recent Chainlink timestamp should be strictly newer than the most
185208
* recent timestamp in SortedOracles.
186-
* - The oldest Chainlink timestamp should not be considered expired by SortedOracles.
209+
* - The most recent Chainlink timestamp should not be considered expired by SortedOracles.
210+
* - The spread between aggregator timestamps is less than the maxTimestampSpread.
187211
*/
188212
function relay() external {
189213
ISortedOraclesMin _sortedOracles = ISortedOraclesMin(sortedOracles);
@@ -201,13 +225,17 @@ contract ChainlinkRelayerV1 is IChainlinkRelayer {
201225
newestChainlinkTs = timestamp > newestChainlinkTs ? timestamp : newestChainlinkTs;
202226
}
203227

228+
if (newestChainlinkTs - oldestChainlinkTs > maxTimestampSpread) {
229+
revert TimestampSpreadTooHigh();
230+
}
231+
204232
uint256 lastReportTs = _sortedOracles.medianTimestamp(rateFeedId);
205233

206234
if (lastReportTs > 0 && newestChainlinkTs <= lastReportTs) {
207235
revert TimestampNotNew();
208236
}
209237

210-
if (isTimestampExpired(oldestChainlinkTs)) {
238+
if (isTimestampExpired(newestChainlinkTs)) {
211239
revert ExpiredTimestamp();
212240
}
213241

test/integration/ChainlinkRelayerIntegration.t.sol

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ contract ChainlinkRelayerIntegration_ReportAfterRedeploy is ChainlinkRelayerInte
108108

109109
vm.prank(owner);
110110
IChainlinkRelayer chainlinkRelayer0 = IChainlinkRelayer(
111-
relayerFactory.deployRelayer(rateFeedId, "cUSD/FOO", aggregatorList0)
111+
relayerFactory.deployRelayer(rateFeedId, "cUSD/FOO", 0, aggregatorList0)
112112
);
113113

114114
vm.prank(deployer);
@@ -121,7 +121,7 @@ contract ChainlinkRelayerIntegration_ReportAfterRedeploy is ChainlinkRelayerInte
121121

122122
vm.prank(owner);
123123
IChainlinkRelayer chainlinkRelayer1 = IChainlinkRelayer(
124-
relayerFactory.redeployRelayer(rateFeedId, "cUSD/FOO", aggregatorList1)
124+
relayerFactory.redeployRelayer(rateFeedId, "cUSD/FOO", 0, aggregatorList1)
125125
);
126126

127127
vm.prank(deployer);
@@ -158,7 +158,7 @@ contract ChainlinkRelayerIntegration_CircuitBreakerInteraction is ChainlinkRelay
158158
IChainlinkRelayer.ChainlinkAggregator[] memory aggregators = new IChainlinkRelayer.ChainlinkAggregator[](1);
159159
aggregators[0] = IChainlinkRelayer.ChainlinkAggregator(address(chainlinkAggregator), false);
160160
vm.prank(owner);
161-
chainlinkRelayer = IChainlinkRelayer(relayerFactory.deployRelayer(rateFeedId, "CELO/USD", aggregators));
161+
chainlinkRelayer = IChainlinkRelayer(relayerFactory.deployRelayer(rateFeedId, "CELO/USD", 0, aggregators));
162162

163163
vm.prank(deployer);
164164
sortedOracles.addOracle(rateFeedId, address(chainlinkRelayer));

0 commit comments

Comments
 (0)