Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Liquidation providers will receive less assets on redeem in spite of perps market's gain

glightspeed2

Summary

When non-usdc and non-usdz asset is deposited to market, it is calculated as market and its connected vaults' realized debt. This will lead to serious miscalculation and malfunction of the protocol.

Vulnerability Details

When user's order results in negative PnL or when user's position is getting liquidated, perps engine deposits part of user's collateral asset to market making engine via CreditDelegationBranch.depositCreditForMarket. Essential part of the logic is as follows:

if (collateralAddr == usdToken) {
// if the deposited collateral is USD Token, it reduces the market's realized debt
market.updateNetUsdTokenIssuance(unary(amountX18.intoSD59x18()));
} else {
if (collateralAddr == usdc) {
market.settleCreditDeposit(address(0), amountX18);
} else {
// deposits the received collateral to the market to be distributed to vaults
// to be settled in the future
market.depositCredit(collateralAddr, amountX18);
}
}

Before we dive into each if-else branches, let's recall how market's total debt is calculated in the current implementation:

, where

(*)

  • If collateral asset is usd token:

In this case, market.netUsdTokenIssuanceis decreased by deposited amount. Thus, market's total debt will be decreased by usd token deposit amount

  • If collateral asset is usdc


Below is market.settleCreditDeposit source code

function settleCreditDeposit(Data storage self, address settledAsset, UD60x18 netUsdcReceivedX18) internal {
// removes the credit deposit asset that has just been settled for usdc
self.creditDeposits.remove(settledAsset);
​
// calculate the usdc that has been accumulated per usd of credit delegated to the market
UD60x18 addedUsdcPerCreditShareX18 = netUsdcReceivedX18.div(ud60x18(self.totalDelegatedCreditUsd));
​
// add the usdc acquired to the accumulated usdc credit variable
self.usdcCreditPerVaultShare =
ud60x18(self.usdcCreditPerVaultShare).add(addedUsdcPerCreditShareX18).intoUint128();
​
// deduct the amount of usdc credit from the realized debt per vault share, so we don't double count it
self.realizedDebtUsdPerVaultShare = sd59x18(self.realizedDebtUsdPerVaultShare).sub(
addedUsdcPerCreditShareX18.intoSD59x18()
).intoInt256().toInt128();
}

Since settledAsset == address(0), it does not affect market's totalDebt directly. But it increases usdcCreditPerVaultShareand decreases realizedDebtUsdPerVaultSharewhich all effectively decrease connected vault's debts

  • If collateral is non-usdz, non-usdc asset

According to Market.depositCredit:

function depositCredit(Data storage self, address asset, UD60x18 amountX18) internal {
AssetToAmountMap.update(self.creditDeposits, asset, amountX18, true);
}

market's credit deposit value is increased by deposit amount. And from the formula (*), market's realizedDebtUsdis increased!

This does not make sense in every perspect, including:

  • trader's negative PnL and liquidation are considered as house (market) win in all the other codebase. So, normal asset deposits should be considered as credit as well

  • usdc deposit and usdz deposit are considered as market credit, while equivalent worth of collateral asset being considered as market debt doesn't really make sense

Impact

Market's debt is distributed to connected vaults. When connected vault's debt is increased, indexTokenSwapRateis decreased. So the conclusion is:

Liquidation providers will receive less assets on redeem in spite of perps market's gain.

Tools Used

Manual Review

Recommendations

Consider the following change when calculating market's realized debt usd

- realizedDebtUsdX18 = creditDepositsValueUsdX18.intoSD59x18().add(sd59x18(self.netUsdTokenIssuance));
+ realizedDebtUsdX18 = sd59x18(self.netUsdTokenIssuance).sub(creditDepositsValueUsdX18.intoSD59x18());
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Market::getRealizedDebtUsd incorrectly adds creditDeposits to netUsdTokenIssuance when calculating debt, causing accounting errors because credit deposits should reduce debt

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.