wise-foundation / lending-audit

5 stars 4 forks source link

[OHR-02C] Inefficient Invocation of Chainlink Related Functions #151

Open vm06007 opened 1 year ago

vm06007 commented 1 year ago

OHR-02C: Inefficient Invocation of Chainlink Related Functions

Type Severity Location
Gas Optimization OracleHelper.sol:L87, L103, L107, L119

Description:

The highlighted argument to the OracleHelper::_getLatestAggregatorRoundId, OracleHelper::_getPhaseId, and OracleHelper::_getRoundTimestamp functions is inefficient as it will always be "converted" to its Chainlink oracle via the priceFeed[_tokenAddress] lookup from storage.

Example:

/**
 * @dev Fetches timestamp of a byteshifted
 * aggregatorRound with specific phaseId.
 */
function _getRoundTimestamp(
    address _tokenAddress,
    uint16 _phaseId,
    uint80 _aggregatorRoundId
)
    internal
    view
    returns (uint256)
{
    (
        ,
        ,
        ,
        uint256 timestamp,
    ) = priceFeed[_tokenAddress].getRoundData(
            getRoundIdByByteShift(
                _phaseId,
                _aggregatorRoundId
            )
        );

    return timestamp;
}

/**
 * @dev Determines info for the heartbeat update
 * mechanism for chainlink oracles, roundIds.
 */
function _getLatestAggregatorRoundId(
    address _tokenAddress
)
    internal
    view
    returns (uint80)
{
    (   uint80 roundId,
        ,
        ,
        ,
    ) = priceFeed[_tokenAddress].latestRoundData();

    return roundId;
}

/**
 * @dev Determines info for the heartbeat update
 * mechanism for chainlink oracles, shifted roundIds.
 */
function getRoundIdByByteShift(
    uint16 _phaseId,
    uint80 _aggregatorRoundId
)
    public
    pure
    returns (uint80)
{
    return uint80(
        uint256(_phaseId) << 64 | _aggregatorRoundId
    );
}

/**
 * @dev Routing phaseId from chainLink.
 * Returns phaseId by passing underlying token address.
 */
function _getPhaseId(
    address _tokenAddress
)
    internal
    view
    returns (uint16)
{
    return priceFeed[_tokenAddress].phaseId();
}

/**
 * @dev Routing latest round data from chainLink.
 * Returns latestRoundData by passing underlying token address.
 */
function latestRoundData(
    address _tokenAddress
)
    public
    view
    returns (uint256 upd)
{
    (   ,
        ,
        ,
        upd
        ,
    ) = priceFeed[_tokenAddress].latestRoundData();
}

Recommendation:

We advise the oracle to be queried once in the OracleHelper::_recalibratePreview function and to be passed in as an oracle to the highlighted functions, optimizing the gas cost of the function significantly.

vm06007 commented 1 year ago

All these functions are used only _recalibratePreview which is only used in _recalibrate -> recalibrate which means it is expected to be called only once by the team when setting the oracle feed initially. If it costs a little bit more thats okay it is not meant to be called more than once per life-cycle most likely anyway unless something changes. This function would be called relatively rarely if at all. Calling _getRoundTimestamp to only get one value in the loop is still needed to be separate. No change intended based on the suggestion. Not worth the rewrite. Still nice to have separate functions that would return separate values.