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

5 stars 5 forks source link

Redundancy_In_Checks #140

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

Redundancy_In_Checks

Low/Info issue submitted by petarP1998

Summary

In the StratFeeManagerInitializable contract, there is redundancy in the implementation of the _isPaused function and the _whenStrategyNotPaused function. Both functions perform the same checks to determine if the strategy is paused.

Vulnerability Detail

The _isPaused function and the _whenStrategyNotPaused function both check whether the strategy is paused by evaluating the paused() function and the factory.globalPause() function. This duplication of logic results in redundant code, which can be simplified.

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/StratFeeManagerInitializable.sol#L77-L86

Impact

Redundant code can make the contract harder to read and maintain. It can also increase the potential for bugs if one of the duplicated pieces of code is modified in the future without making corresponding changes to the other.

Code Snippet

function _whenStrategyNotPaused() internal view {
    if (paused() || factory.globalPause()) revert StrategyPaused();
}

function _isPaused() internal view returns (bool) {
    return paused() || factory.globalPause();
}

Tool used

Manual Review

Recommendation

Use the _isPaused function and its logic directly in _whenStrategyNotPaused.

The refactored code would be:

function _whenStrategyNotPaused() internal view {
    if (_isPaused()) revert StrategyPaused();
}