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

5 stars 5 forks source link

Rhaydden - Incorrect Condition in `setDeviation` Function Causes Unnecessary Reverts When Setting Maximum Tick Deviation #51

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 5 months ago

Rhaydden

medium

Incorrect Condition in setDeviation Function Causes Unnecessary Reverts When Setting Maximum Tick Deviation

Summary

The setDeviation function in the StrategyPassiveManagerVelodrome contract incorrectly reverts when the maximum tick deviation is set to exactly four times the tick spacing. This behavior contradicts the intended functionality as described in the comment.

Vulnerability Detail

        // Require the deviation to be less than or equal to 4 times the tick spacing.

As per the the Natspec as seen above, the setDeviation function is designed to ensure that the maximum tick deviation does not exceed four times the tick spacing. Albeit, the current implementation uses a greater than or equal to (>=) comparison, which causes the function to revert even when the deviation is exactly four times the tick spacing. This mistake affects the ability to set the maximum tick deviation to the intended value, impacting the strategy's flexibility and efficiency.

Functions that query the tick deviation, such as moveTicks, deposit, and withdraw, rely on the maxTickDeviation value to determine if the current price is within an acceptable range. The incorrect condition in setDeviation can cause unnecessary reverts, preventing these functions from executing as intended. This can result in suboptimal rebalancing, failed deposits, and withdrawals, and more frequent reverts during calm period checks.

Impact

The incorrect condition in the setDeviation function limits the ability to set the maximum tick deviation to exactly four times the tick spacing. This restriction can lead to suboptimal strategy performance, failed deposits and withdrawals, and more frequent reverts during calm period checks. The overall intent of the strategy is compromised.

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L705-L712

Tool used

Manual Review

Recommendation

Update the condition in the setDeviation function to use a strictly greater than (>) comparison instead of greater than or equal to (>=). This ensures that the function only reverts if the deviation exceeds four times the tick spacing, allowing for the intended purpose which requires the deviation to be less than or equal to 4 times the tick spacing.

if (_maxDeviation > _tickDistance() * 4) revert InvalidInput();

Duplicate of #33