The KeeperProxy
contract has sequencerUptimeFeed
checks in place to assert if the sequencer on Arbitrum
is running, but these checks are not implemented correctly. Since the protocol implements some checks for the sequencerUptimeFeed
status, it should implement all of the checks.
The following implemented check in _validatePrice
is useless if its called in an invalid round
as startedAt
will be 0
, the arithmetic operation block.timestamp - startedAt
will result in a value greater than GRACE_PERIOD_TIME
(which is hardcoded to be 3600) i.e block.timestamp = 1719739032, so 1719739032 - 0 = 1719739032 which is bigger than 3600. The code won't revert.
The chainlink docs say that sequencerUptimeFeed
can return a 0 value for startedAt
if it is called during an "invalid round".
startedAt: This timestamp indicates when the sequencer changed status. This timestamp returns
0
if a round is invalid. When the sequencer comes back up after an outage, wait for theGRACE_PERIOD_TIME
to pass before accepting answers from the data feed. SubtractstartedAt
fromblock.timestamp
and revert the request if the result is less than theGRACE_PERIOD_TIME
.
Please note that an "invalid round" is described to mean there was a problem updating the sequencer's status, possibly due to network issues or problems with data from oracles, and is shown by a startedAt
time of 0 and answer
is 0.
Imagine a case where a round starts, at the beginning startedAt
is recorded to be 0, and answer
, the initial status is set to be 0
. Note that docs say that if answer = 0
, sequencer is up, if equals to 1
, sequencer is down. But in this case here, answer
and startedAt
can be 0
initially, till after all data is gotten from oracles and update is confirmed then the values are reset to the correct values that show the correct status of the sequencer.
From these explanations and information, it can be seen that startedAt
value is a second value that should be used in the check for if a sequencer is down/up or correctly updated. The checks in _validatePrice
will allow for sucessfull calls in an invalid round because reverts dont happen if answer == 0
and startedAt == 0
thus defeating the purpose of having a sequencerFeed
check to assert the status of the sequencerFeed
on L2 i.e if it is up/down/active or if its status is actually confirmed to be either.
Inadequate checks to confirm the correct status of the sequencerUptimeFeed
in KeeperProxy
contract will cause _validatePrice
to not revert even when the sequencer uptime feed is not updated or is called in an invalid round.
Manual Review
Add the following check in the _validatePrice
function.
startedAt is only 0 when contract is not initialized on Arbitrum, but it is already initialized on Arbitrum. startedAt is sufficient for the protocol, it does not need roundID. Current documentation of Chainlink does not have this sentence: “This timestamp returns `0` if a round is invalid.“
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.