Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

Liquidation with assets under 18 decimals #384

Closed pedrovalido closed 1 year ago

pedrovalido commented 1 year ago

[H-2] Partial Liquidations won't be possible for vaults with the collateral asset of decimals < 18

Description

The liquidate function of Fuji's borrowing vault has the following code:

uint256 price = oracle.getPriceOf(debtAsset(), asset(), _debtDecimals);
uint256 discountedPrice = Math.mulDiv(price, LIQUIDATION_PENALTY, 1e18);
gainedShares = convertToShares(Math.mulDiv(debt, liquidationFactor, discountedPrice));

The purpose of gainedShares is to calculate the number of shares a liquidator should receive for paying an owner's debt. Ideally, it should be done like this:

debtToCover = debt * liquidationFactor / 1e18

If 1 * 10 ** assetDecimals is worth this much price,
then how many assets are equivalent to debtToCover?

1 * 10 ** assetDecimals = price
? = debtToCover

gainedAssets = debtToCover * 10 ** assetDecimals / price
                         = debt * liquidationFactor * 10 ** assetDecimals / price * 1e18

However, Fuji's current implementation needs to be corrected since it ignores both offsets. Math.mulDiv(debt, liquidationFactor, discountedPrice);

gainedAssets = debt * liquidationFactor / price

Nearly all widely used ERC20s have decimals ≤ 18. There won't be any issue for tokens with decimals = 18 since the numerator and denominator would cancel each other out. However, for all tokens with decimals < 18, Fuji would overestimate the gained shares and hence would assign gainedShares equal to the total balance of the owner due to the following check:

if (gainedShares > existingShares) {
      gainedShares = existingShares;
}

If the concerned liquidation is whole, liquidation will go untroubled.

However, if the liquidation is partial, then it would revert while burning owner shares since you cannot burn all owner shares until all the owner's debt is paid.

Remediation to consider

Consider applying proper offsets 10 18 / 10 assetDecimals for gained asset calculation.

pedrovalido commented 1 year ago

check aave and angle