costInEuros calculation will incur precision loss due to division before multiplication

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L220-L221

Summary

In LiquidationPool::distributeAssets(), costInEuros calculation result will be rounded down due to division before multiplication.

Vulnerability Details

Since exchange rates are involved, round downs will always occur. Here's code for quick reference.

uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
    * _hundredPC / _collateralRate;

Impact

These little losses will pile up in proportion the number of users and longevity of the contract.

Tools Used

Manual Review

Recommendations

Change the code to this such that multiplication is done first before division.

-- uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
--    * _hundredPC / _collateralRate;

++ uint256 costInEuros = (_portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) * _hundredPC) / (uint256(priceEurUsd) * _collateralRate);

or this

-- uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
--    * _hundredPC / _collateralRate;

++ uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) * _hundredPC / uint256(priceEurUsd) / _collateralRate;