low

Precision loss when calculating the health factor

Reward

Total

5.75 USDC

0.37 USDC
0.37 USDC
0.37 USDC
0.37 USDC
0.37 USDC
0.37 USDC
0.37 USDC
0.37 USDC
0.37 USDC
0.37 USDC
0.37 USDC
Selected
0.52 USDC
0.37 USDC
0.37 USDC
0.37 USDC
Selected Submission

Precision loss when calculating the health factor

Severity

Low Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L330

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L331

Summary

The calculation of the health factor in the _calculateHealthFactor() suffers from a rounding down issue, resulting in a small precision loss that can be improved.

Vulnerability Details

Division before multiplication can lead to rounding down issue since Solidity has no fixed-point numbers. Consider the calculation of the health factor in the _calculateHealthFactor(), the function does the division (by LIQUIDATION_PRECISION) before the multiplication (by 1e18). Hence, the computed result can suffer from the rounding down issue, resulting in a small precision loss.

    function _calculateHealthFactor(uint256 totalDscMinted, uint256 collateralValueInUsd)
        internal
        pure
        returns (uint256)
    {
        if (totalDscMinted == 0) return type(uint256).max;
@>      uint256 collateralAdjustedForThreshold = (collateralValueInUsd * LIQUIDATION_THRESHOLD) / LIQUIDATION_PRECISION; //@audit Division (by LIQUIDATION_PRECISION) before multiplication
@>      return (collateralAdjustedForThreshold * 1e18) / totalDscMinted; //@audit Multiplication (by 1e18) after division
    }

Impact

The computed result of the _calculateHealthFactor() can suffer from the rounding down issue. However, the impact can be considerably low since the denominator, LIQUIDATION_PRECISION, is a constant of 100. Anyway, there can be a way to improve the calculation for better precision loss (see the Recommendations section).

Tools Used

Manual Review

Recommendations

I recommend improving the affected formula by taking multiplications before divisions to prevent truncation, as shown below.

    function _calculateHealthFactor(uint256 totalDscMinted, uint256 collateralValueInUsd)
        internal
        pure
        returns (uint256)
    {
        if (totalDscMinted == 0) return type(uint256).max;
-       uint256 collateralAdjustedForThreshold = (collateralValueInUsd * LIQUIDATION_THRESHOLD) / LIQUIDATION_PRECISION;
-       return (collateralAdjustedForThreshold * 1e18) / totalDscMinted;

+       return (collateralValueInUsd * LIQUIDATION_THRESHOLD * 1e18) / (LIQUIDATION_PRECISION * totalDscMinted);
    }