VIS-2 / taobank-04-24

0 stars 0 forks source link

Chainlink price feed good practices #4

Open DanailYordanov opened 5 months ago

DanailYordanov commented 5 months ago

Impact

Severity: High Likelihood: Low

Context

ChainlinkPriceOracle::price()

Description

Given that the protocol will be deployed to Arbitrum, it's essential to implement checks for the sequencer being down. Consider the scenario where the sequencer experiences downtime — this has occurred in the past and may happen again in the future. When the sequencer resumes operation and oracles update their prices, all price movements that occurred during the downtime are applied simultaneously. If these movements are significant, they may lead to chaos, with borrowers rushing to save their positions while liquidators rush to liquidate them. Since liquidations are primarily handled by bots, borrowers are likely to face mass liquidations, which is unfair to them as they couldn't act on their positions during the L2 downtime. Hence, it would be ideal if the protocol provides borrowers with a grace period once the sequencer returns.

Chainlink provides a dedicated feed for sequencer uptimes. This feed continuously updates sequencer uptime from L1 to L2. When the sequencer goes down, the Chainlink sequencer update is pushed to the delayed inbox. Since all transactions in the delayed inbox are executed before any others once the sequencer resumes operation, your contract would consider a grace period.

Aave V3 follows a similar approach as described in their technical paper, bearing in mind that heavily undercollateralized (0.95 < HF < 1) positions may still experience liquidations even during the grace period.

Recommendation

Familiarize yourself with the Aave V3 solution to the problem. Example code snippet:

(
    /*uint80 roundID*/,
    int256 answer,
    uint256 startedAt,
    /*uint256 updatedAt*/,
    /*uint80 answeredInRound*/
) = sequencerUptimeFeed.latestRoundData();

// Answer == 0: Sequencer is up
// Answer == 1: Sequencer is down
bool isSequencerUp = answer == 0;
if (!isSequencerUp) {
    revert SequencerDown();
}

// Ensure the grace period has passed after the
// sequencer is back up.
uint256 timeSinceUp = block.timestamp - startedAt;
if (timeSinceUp <= GRACE_PERIOD_TIME) {
    revert GracePeriodNotOver();
}

Furthermore, there is a possibility in the future that access to Chainlink oracles will be restricted to paying customers (Chainlink is currently subsidized and has no monetization model in place for its most used feeds). Hence, it's advisable to surround the latestRoundData() call in a try-catch statement to gracefully recover from errors. Additionally, having a fallback option for oracles is recommended to avoid a single point of failure. Secondary oracle options such as Uniswap TWAP for assets with good liquidity on Uniswap or another off-chain oracle like Tellor can be used.