Lose due to rounding. Use more precise library for mathematical operations

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L226-L227

Vulnerability details

Impact

The mulDivDown function, assumed to be from FixedPointMathLib, is likely designed to multiply two numbers and then divide the result, rounding down any remainders. This rounding down can result in minor discrepancies when converting between assets and shares.

The convertToShares function may lead to precision errors during its calculations, potentially causing users to receive fewer shares than they should.

Proof of Concept

Scenario: Consider a hypothetical situation where the division should ideally result in 10.9999. Solidity's default behavior will round this down to 10. Now, if this value is being used in subsequent calculations, or if it's being multiplied by a large number, the resulting value can be significantly less than expected, leading to potential asset losses for users

Tools Used

foundry

Recommended Mitigation Steps

Recommendation: Utilize the ABDK Math 64.64 library to ensure high precision during calculations.

Fixed code snippet:

function _convertToAssets(uint256 shares) internal view virtual returns (uint256 assets, uint256 rdpxAmount) {
    uint256 supply = totalSupply;
    return
        (supply == 0)
            ? (shares, 0)
            : (
                shares.mulDiv(totalCollateral(), supply),
                shares.mulDiv(_rdpxCollateral, supply)
            );
}

Implement a more precise method of division. The ABDK Math 64.64 library can be utilized for this purpose due to its enhanced precision. Ensure division does not result in zero assets. Introduce checks to ensure that rounding doesn't produce zero assets

How it Works: By using a library like ABDK Math 64.64, division and multiplication operations are handled with more precision, which can prevent the assets from becoming zero in most cases. The new require statement checks if the assets are greater than a defined minimum (MIN_ASSETS), ensuring that even if there's a rounding error, the function won't revert unless the assets are below this minimum threshold.

FixedPointMathLib is less known and accurate library than ABDK Math.

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as low quality report

[bytes032]

solmate's lib is perfectly fine

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid