Minor Inefficiencies in _deposit Function

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0xfe48c86ee5fde1eaae0770dab56021baee6c579c2aa94c61a309134d82982c86
Severity: low

Description:
Description
In the _deposit function of the EthMultiVault contract, there are two minor inefficiencies:

1. Unnecessary Comparison: The function checks if totalAssetsDelta <= 0, which is redundant for unsigned integers.
2. Unused Variable: A variable totalSharesDelta is created but is identical to sharesForReceiver, making it unnecessary.

While these issues don't pose security risks, they represent opportunities for gas optimization and code clarity.

Attack Scenario
These issues don't present direct attack vectors, but they have the following implications:

1. The unnecessary comparison with zero slightly increases gas costs for every deposit operation.
2. The unused variable adds complexity to the code without providing any benefit, potentially confusing future developers or auditors.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

function _deposit(address receiver, uint256 id, uint256 value) internal returns (uint256) {
    if (previewDeposit(value, id) == 0) {
        revert Errors.MultiVault_DepositOrWithdrawZeroShares();
    }

    (uint256 totalAssetsDelta, uint256 sharesForReceiver, uint256 userAssetsAfterTotalFees, uint256 entryFee) =
        getDepositSharesAndFees(value, id);

    // Corrected: Remove unnecessary comparison with zero
    if (totalAssetsDelta == 0) {
        revert Errors.MultiVault_InsufficientDepositAmountToCoverFees();
    }

    // Corrected: Remove unnecessary variable
    // set vault totals (assets and shares)
    _setVaultTotals(id, vaults[id].totalAssets + totalAssetsDelta, vaults[id].totalShares + sharesForReceiver);

    // mint `sharesOwed` shares to sender factoring in fees
    _mint(receiver, id, sharesForReceiver);

    emit Deposited(
        msg.sender,
        receiver,
        vaults[id].balanceOf[receiver],
        userAssetsAfterTotalFees,
        sharesForReceiver,
        entryFee,
        id
    );

    return sharesForReceiver;
}

[mihailo-maksa]

The report suggests two improvements to the _deposit function: removing an unnecessary comparison and an unused variable.

Label: minor

Comment:
The report contains two suggested fixes to improve the gas efficiency of the _deposit function:

1. Unnecessary Comparison: The function checks if totalAssetsDelta <= 0, which is redundant for unsigned integers.
   • Our Comment: This is correct; the check totalAssetsDelta == 0 will suffice, as suggested by the reporter.
2. Unused Variable: The variable totalSharesDelta is identical to sharesForReceiver and thus unnecessary.
   • Our Comment: The totalSharesDelta variable was added for clarity, and we can consider removing it or adding a comment instead. The reporter is technically correct, but this change is not yet certain.

Comment on the issue:
The suggested fixes are valid and will be considered for improving gas efficiency and code clarity. We appreciate the feedback.