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

5 stars 5 forks source link

Function can be modifier #139

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

Function can be modifier

Low/Info issue submitted by petarP1998

Summary

The _onlyVault function in the StrategyPassiveManagerVelodrome contract can be converted into a modifier to enhance code readability and maintainability.

Vulnerability Detail

Currently, the _onlyVault function is implemented as a private view function. This approach requires explicit calls within the contract, which can be less intuitive and harder to follow compared to using a modifier. Modifiers provide a clear and standardized way to apply common checks across multiple functions, improving code clarity.

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

Impact

The current implementation does not pose a security risk but affects the readability and maintainability of the code. Using a modifier would make the contract easier to understand and reduce the likelihood of errors during future modifications or audits.

Code Snippet

function _onlyVault() private view {
    if (msg.sender != vault) revert NotVault();
}

Tool used

Manual Review

Recommendation

Refactor the _onlyVault function into a modifier named onlyVault. This change will improve the readability and maintainability of the contract by making access control checks more explicit and standardized.

Here is the suggested code for the modifier:

modifier onlyVault() { 
    if (msg.sender != vault) revert NotVault();
    _; 
}