Deviation calculation inconsistent

Summary

Deviation checked in two places in code base, but differently.

Vulnerability Detail

In SimplePriceFeedStrategy function getAveragePriceIfDeviation and getMedianPriceIfDeviation have following deviation check (snippent for average price, but same for median):

// Check the deviation of the minimum from the average
uint256 minPrice = sortedPrices[0];
if (((averagePrice - minPrice) * 10000) / averagePrice > deviationBps) return averagePrice;

// Check the deviation of the maximum from the average
uint256 maxPrice = sortedPrices[sortedPrices.length - 1];
if (((maxPrice - averagePrice) * 10000) / averagePrice > deviationBps) return averagePrice;

Note that first check calculates it as (max - min) / max, but second as (max - min) / min

But in deviation library always (max - min) / max):

    function isDeviating(
        uint256 value0_,
        uint256 value1_,
        uint256 deviationBps_,
        uint256 deviationMax_
    ) internal pure returns (bool) {
        return
            (value0_ < value1_)
                ? _isDeviating(value1_, value0_, deviationBps_, deviationMax_)
                : _isDeviating(value0_, value1_, deviationBps_, deviationMax_);
    }

    function _isDeviating(
        uint256 value0_,
        uint256 value1_,
        uint256 deviationBps_,
        uint256 deviationMax_
    ) internal pure returns (bool) {
        return ((value0_ - value1_) * deviationMax_) / value0_ > deviationBps_;
    }

Impact

I think this is not intended behavior as admins would need to keep in mind two deviation models when setting parameters. Better be consistent.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/strategies/SimplePriceFeedStrategy.sol#L197-L203

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/libraries/Deviation.sol#L42-L70

Tool used

Manual Review

Recommendation

Change deviation check in SimplePriceFeedStrategy to always be (max - min) / max)

Duplicate of #193

sherlock-audit / 2023-11-olympus-judging

evilakela - Deviation calculation inconsistent #208