Precision loss when calculating the health factor

[codehawks-bot]

Precision loss when calculating the health factor

Severity

Low Risk

Relevant GitHub Links

2023-07-foundry-defi-stablecoin/src/DSCEngine.sol

Line 330 in d1c5501

uint256 collateralAdjustedForThreshold = (collateralValueInUsd * LIQUIDATION_THRESHOLD) / LIQUIDATION_PRECISION;

2023-07-foundry-defi-stablecoin/src/DSCEngine.sol

Line 331 in d1c5501

return (collateralAdjustedForThreshold * 1e18) / totalDscMinted;

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
    }

• Division (by LIQUIDATION_PRECISION) before multiplication:

  2023-07-foundry-defi-stablecoin/src/DSCEngine.sol

  Line 330 in d1c5501

  uint256 collateralAdjustedForThreshold = (collateralValueInUsd * LIQUIDATION_THRESHOLD) / LIQUIDATION_PRECISION;

• Multiplication (by 1e18) after division:

  2023-07-foundry-defi-stablecoin/src/DSCEngine.sol

  Line 331 in d1c5501

  return (collateralAdjustedForThreshold * 1e18) / totalDscMinted;

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);
    }