Comet Custom WBTC Price Feed Audit.md

Comet Custom WBTC Price Feed Audit

Performed by OpenZeppelin

Date: May 30, 2023

We were asked to audit the compound-protocol/comet repository at commit d9c2f85, which is part of PR 737. The only file in scope was:

• contracts/pricefeeds/WBTCPriceFeed.sol

Overview

Compoundv3, referred to hereafter as Comet, uses Chainlink as its source of asset prices. Currently, it prices WBTC using Chainlink's BTC price feed. The idea being that the price of BTC should be close to the price of WBTC and generally that is true. But in the case of market panic or catastrophe, there is a chance that the price of WBTC could be drastically different, creating issues or distortions in the protocol that could risk user funds. The newly proposed price feed involves pricing WBTC more accurately in those cases, using Chainlink's WBTC/BTC price feed in conjunction with the BTC/USD price feed to create a WBTC/USD pricing function.

Of course, the purported gains in accuracy of the pricing function depend on the resilience of the Chainlink network of aggregators and operators to provide accurate data. Both of the price feeds to be integrated are described by Chainlink as "Verified Feeds", the highest grade of quality that they assign. However, we recommend independently monitoring the health of these feeds to ensure they meet the standards set forth by the Compound community.

Consider monitoring the number of aggregators and node operators that are healthy and participating in these price feeds.

Findings

Low

L01 - Stale Oracle Prices Not Handled

In WBTCPriceFeed.sol, the latestRoundData function calls out to the Chainlink data feeds for WBTC/BTC and BTC/USD. Should either feed fail to update in a timely manner, the prices in the Comet market would become detached from reality.

Consider implementing a method for determining if price data is stale (e.g., perhaps two hours old) and determine what special handling it would require in the market.

Update: Acknowledged, not resolved. The Compound team stated:

Stale price data is definitely a risk that should be properly handled. Such a change would likely have to be introduced in a larger effort to re-design and improve Comet’s oracle system.

Note

N01 - Unused named return variables

Named return variables are a way to declare variables that are meant to be used within a function body for the purpose of being returned as the function's output. They are an alternative to explicit in-line return statements.

In WBTCPriceFeed.sol, there are multiple instances of unused named return variables:

• The roundId return variable in the latestRoundData function
• The answer return variable in the latestRoundData function
• The startedAt return variable in the latestRoundData function
• The updatedAt return variable in the latestRoundData function
• The answeredInRound return variable in the latestRoundData function

Consider either using or removing any unused named return variables.

Update: Resolved in pull request #737 at commit bde60e1.

N02 - Constant not using UPPER_CASE format

There are several constant and immutable variables not declared using UPPER_CASE format.

According to the Solidity Style Guide, constants should be named with all capital letters with underscores separating words. For better readability and understanding, consider following this convention.

Update: Acknowledged, not resolved. The Compound team stated:

The Comet repo has chosen to stylistically define immutables as if they were state variables, so the same convention will be used here to remain consistent with the rest of the contracts. We also decided to keep lowercase formatting for the constants version and description because the Chainlink AggregatorV3Interface defines lowercase setter functions for them.