Gas_Optimization

[sherlock-admin3]

Gas_Optimization

Low/Info issue submitted by petarP1998

Summary

The StrategyPassiveManagerVelodrome::_checkAmounts function contains redundant code that calculates amount0 and amount1 using LiquidityAmounts.getAmountsForLiquidity. This calculation is already performed in _addLiquidity. Therefore, it can be removed from _checkAmounts and instead, the values can be passed as parameters.

Vulnerability Detail

The code snippet below from the _checkAmounts function redundantly calculates amount0 and amount1:

(uint256 amount0, uint256 amount1) = LiquidityAmounts
    .getAmountsForLiquidity(
        sqrtPrice(),
        TickMath.getSqrtRatioAtTick(_tickLower),
        TickMath.getSqrtRatioAtTick(_tickUpper),
        _liquidity
    );

This block is unnecessary since the amounts are already computed in the _addLiquidity function.

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

Impact

Removing the redundant code improves the efficiency of the smart contract by eliminating unnecessary calculations, thus reducing gas costs. It also simplifies the code, making it easier to read and maintain.

Code Snippet

function _checkAmounts(
    uint128 _liquidity,
    int24 _tickLower,
    int24 _tickUpper
) private view returns (bool) {
    (uint256 amount0, uint256 amount1) = LiquidityAmounts
        .getAmountsForLiquidity(
            sqrtPrice(),
            TickMath.getSqrtRatioAtTick(_tickLower),
            TickMath.getSqrtRatioAtTick(_tickUpper),
            _liquidity
        );

    if (amount0 == 0 || amount1 == 0) return false;
    else return true;
}

Tool used

Manual Review

Recommendation

Refactor the _checkAmounts function to remove the redundant code block. Instead, pass the already computed amount0 and amount1 as parameters from _addLiquidity to _checkAmounts. This will enhance the contract's efficiency and readability.

This will look like this:

function _addLiquidity() private {
    _whenStrategyNotPaused();

    (uint256 bal0, uint256 bal1) = balancesOfThis();

    int24 mainLower = positionMain.tickLower;
    int24 mainUpper = positionMain.tickUpper;
    int24 altLower = positionAlt.tickLower;
    int24 altUpper = positionAlt.tickUpper;

    // Then we fetch how much liquidity we get for adding at the main position ticks with our token balances.
    uint160 sqrtprice = sqrtPrice();
    uint128 liquidity = LiquidityAmounts.getLiquidityForAmounts(
        sqrtprice,
        TickMath.getSqrtRatioAtTick(mainLower),
        TickMath.getSqrtRatioAtTick(mainUpper),
        bal0,
        bal1
    );

    (uint256 amount0, uint256 amount1) = LiquidityAmounts
        .getAmountsForLiquidity(
            sqrtprice,
            TickMath.getSqrtRatioAtTick(mainLower),
            TickMath.getSqrtRatioAtTick(mainUpper),
            liquidity
        );

    bool amountsOk = _checkAmounts(amount0, amount1);
    ...
}

function _checkAmounts(
    uint256 amount0,
    uint256 amount1
) private pure returns (bool) {
    if (amount0 == 0 || amount1 == 0) return false;
    else return true;
}

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/beefyfinance/cowcentrated-contracts/pull/21/files

[spacegliderrrr]

Fixed. _checkAmounts now takes the already calculated values and only checks if both of them are non-zero.

[sherlock-admin2]

The Lead Senior Watson signed off on the fix.