Nadin - LMPVault.sol does not match EIP4626 because of preview functions.

[sherlock-admin2]

Nadin

medium

LMPVault.sol does not match EIP4626 because of preview functions.

Summary

• According to docs:

Q: Is the code/contract expected to comply with any EIPs? Are there specific assumptions around adhering to those EIPs that Watsons should be aware of?
src/vault/LMPVault.sol should be 4626 compatible

• But current implementation of preview function in LMPVault.sol : previewDeposit(), previewRedeem(), previewMint() are not EIP4626 compliant according to docs: https://eips.ethereum.org/EIPS/eip-4626

Vulnerability Detail

• Take a look at : LMPVault.sol#previewDeposit() , LMPVault.sol#previewMint() and LMPVault.sol#previewRedeem()

File: LMPVault.sol
328:    function previewDeposit(uint256 assets) public view virtual returns (uint256 shares) {
        shares = _convertToShares(assets, Math.Rounding.Down);
    }
---SKIP---
362:    function previewMint(uint256 shares) public view virtual returns (uint256 assets) {
        assets = _convertToAssets(shares, Math.Rounding.Up);
    }
---SKIP---
372:    function previewRedeem(uint256 shares) public view virtual override returns (uint256) {
        return _convertToAssets(shares, Math.Rounding.Down);
    }

• LMPVault.sol#_convertToAssets() and LMPVault.sol#_convertToShares is called in those functions.

File: LMPVault.sol
595:    function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256 assets) {
        uint256 supply = totalSupply();
        assets = (supply == 0) ? shares : shares.mulDiv(totalAssets(), supply, rounding);
    }
---SKIP---
587:    function _convertToShares(uint256 assets, Math.Rounding rounding) internal view virtual returns (uint256 shares) {
        uint256 supply = totalSupply();


        // slither-disable-next-line incorrect-equality
        shares = (assets == 0 || supply == 0) ? assets : assets.mulDiv(supply, totalAssets(), rounding);
    }

• According to EIP4626: previewDeposit(), previewRedeem(), previewMint() must include fee in returned value:

1. previewDeposit "MUST be inclusive of deposit fees. Integrators should be aware of the existence of deposit fees."
2. previewRedeem "MUST be inclusive of withdrawal fees. Integrators should be aware of the existence of withdrawal fees."
3. previewMint "MUST be inclusive of deposit fees. Integrators should be aware of the existence of deposit fees."

• As we see current implementation of preview functions in LMPVault.sol does not match EIP4626.

Impact

previewDeposit(), previewRedeem() and previewMint() are not exactly return the same value as expected. Since fees are
involving with it.

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVault.sol#L328-L330
https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVault.sol#L362-L364
https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVault.sol#L372-L374

Tool used

Manual Review and https://eips.ethereum.org/EIPS/eip-4626.

Recommendation

Use standared ERC4626 contract to implement those functions.

Duplicate of #577

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

Trumpero commented:

invalid, no impact. Fee will be minted for the sink so won't affect the shares/assets in preview function