code-423n4 / 2021-06-gro-findings

0 stars 1 forks source link

Loss of precision #95

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Vulnerability Details

In ._totalAssetsEmergency there's a loss of precision in the assets computation due to first truncating by the chainlink decimal factor and then multiplying by DEFAULT_DECIMALS_FACTOR afterwards.

assets = assets.mul(price).div(CHAINLINK_PRICE_DECIMAL_FACTOR);
assets = assets.mul(DEFAULT_DECIMALS_FACTOR).div(decimals);

Impact

There can be a loss of precision in the computations.

Recommended Mitigation Steps

Multiply by DEFAULT_DECIMALS_FACTOR first before dividing by CHAINLINK_PRICE_DECIMAL_FACTOR

kitty-the-kat commented 2 years ago

No any precision loss. The 1st line is the token amount, the 2nd line is the dollar amount.

ghoul-sol commented 2 years ago

Technically warden is right but I doubt there will be any precision loss in practice. I'll keep it as non-critical.