PriceFeed Manual Review Findings

PFD-01M: Potentially Unwanted Capability

Description:

The setSynthAddress permits the owner to overwrite a previously set synthKey.

Example:

Copy
65function setSynthAddress(string calldata _synth, IAggregatorV3Interface _address) external onlyOwner{
66    bytes32 __synth = stringToBytes32(_synth);
67    synthKey[__synth].synth = __synth;
68    synthKey[__synth].priceFeedAddress = _address;
69}

Recommendation:

We advise this trait to be evaluated and if deemed undesirable, a require check to be introduced ensuring that the synthKey[__synth] slot has not been set.

Alleviation:

A new member was introduced to the struct in question named exist that is meant to indicate whether a particular synth has been set. This does alleviate this exhibit, however, it is more efficient to instead check existing members of the synth struct that are not meant to be zero, such as the priceFeedAddress, instead of declaring a new struct variable as the latter consumes a lot of gas.

PFD-02M: Redundant Usage of Storage

Description:

The priceFeed contract level variable is temporarily utilized by the getLatestPrice function during which it is always overwritten before being utilized.

Example:

Copy
47function getLatestPrice(bytes32 _synth) external returns (int) {
48    priceFeed = IAggregatorV3Interface(synthKey[_synth].priceFeedAddress);
49    
50    (
51        uint80 roundID, 
52        int price,
53        uint startedAt,
54        uint timeStamp,
55        uint80 answeredInRound
56    ) = priceFeed.latestRoundData();
57    // If the round is not complete yet, timestamp is 0
58    require(timeStamp > 0, "Round not complete");
59    
60    synthKey[_synth]._price = uint256(price);
61    
62    return price;
63}

Recommendation:

We advise it to be omitted entirely and an in-memory variable to be utilized by the getLatestPrice function instead. Additionally, the type casting performed within it is redundant as priceFeedAddress is already of type IAggregatorV3Interface.

Alleviation:

The contract level variable declaration was commented out in favor of an in-memory declaration within the function it was being utilized in thereby alleviating this exhibit.