Impermanent Loss & Arbitrage due to incorrect twap rounding

Summary

Incorrect price deviation threshold can be set due to the twap() function not handling the case where ticksCummulative delta is negative leading to more arbitrage losses than it should.

Vulnerability Detail

The twap() function 'StrategyPassiveManagerVelodrome.sol' does not round correctly when ticksCummulativeDelta is negative

    function twap() public view returns (int56 twapTick) {
        uint32[] memory secondsAgo = new uint32[](2);
        secondsAgo[0] = uint32(twapInterval);
        secondsAgo[1] = 0;

        (int56[] memory tickCuml,) = IUniswapV3Pool(pool).observe(secondsAgo);
        twapTick = (tickCuml[1] - tickCuml[0]) / int32(twapInterval);
        ///@audit ^ 
    }

as can be seen from velodrome oracle lib -

if (tickCumulativesDelta < 0 && (tickCumulativesDelta % secondsAgo != 0)) arithmeticMeanTick--;

this is how the twap should handle in case tickCuml[1] - tickCuml[0] is negative, i.e it should return 1 tick lower. But this case is not handled properly neither in velo strategy nor in the already deployed uniswap strategy. Which in turn results in more arbitrage and impermanent losses for the LPs than it should, as this tick is important in defining price deviation.

Impact

LPs are more susceptible to arbitrage loss

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L741-L748

https://github.com/velodrome-finance/slipstream/blob/4fe6f626f850bd03390af18fd84a56de29adcc5b/contracts/periphery/libraries/OracleLibrary.sol#L34C1-L36C104

Tool used

Manual Review

Recommendation

Follow OracleLib.sol from velo & also update the uniswap strategy twap function

    function twap() public view returns (int56 twapTick) {
        uint32[] memory secondsAgo = new uint32[](2);
        secondsAgo[0] = uint32(twapInterval);
        secondsAgo[1] = 0;

        (int56[] memory tickCuml,) = IUniswapV3Pool(pool).observe(secondsAgo);
        tickCumlD = tickCuml[1] - tickCuml[0];
        twapTick = tickCumlD / int32(twapInterval);
        if(tickCumlD <0 && (tickCumlD % twapInterval != 0 )) twapTick--;
        ///@audit mitigation ^
    }

Duplicate of #95

sherlock-audit / 2024-05-beefy-cowcentrated-liquidity-manager-judging

Kalyan-Singh - Impermanent Loss & Arbitrage due to incorrect twap rounding #85