Zaros Part 1

Impact

Because the wrong fundingFeePerUnit is used to calculate the accruedFunding in the function
getAccountMarginRequirementUsdAndUnrealizedPnlUsd, beside other issues, an account can be liquidated even though it should not be liquidatable.

Proof of Concept

The fundingRate is an indicator of how much a holder of a long/ short position must pay/earns for holding his position. If the funding rate is positive, meaning that the marketPrice is above the assets price, holders of long positions need to pay a fee to holders of short positions and vice versa. The fees that need to be paid per unit of position size are tracked in the variable fundingFeePerUnit.
When calling getAccountMarginRequirementUsdAndUnrealizedPnlUsd three values for the account are determined: the requiredInitialMarginUsdX18, the requiredMaintenanceMargin and the accountTotalUnrealizedPnlUsd. The accountTotalUnrealizedPnlUsd consist of the unrealizedPnl of the position and its AccruedFunding.

The accruedFunding is supposed to reflect the above-mentioned fees.
To determine those fees associated with the position, the fundingFeePerUnit is used. It is determined by calling getNextFundingFeePerUnit using the currentFundingRate and the markPrice:

Based on the explanation above it is obvious, that the provided price for calculating the getNextFundingFeePerUnit has a direct impact on calculated AccruedFunding.

In the current implementation the markPrice is used to calculate the accruedFunding for a position. To get the markPrice, it is simulated to close the entire position by using the negative value of the position size as skewDelta:

Than the markPrice is calculated as the average of the price in the market before and after the position is closed:

The issue arises from the fact that the average price (markPrice) is influenced by the size of the position in question (skewDelta). This in turn influences the accumulatedFunding of the position.
But the fees a position accumulates should be the same, no matter what the size or direction of the next (simulated) interaction with the market is. Therefore, not the markPrice should be used for calculating the fundingFeePerUnit but the priceBeforeDelta. Because using the wrong price to calculate the accruedFunding can lead to a result that is lower than it should be, this can, beside other thinks lead to the liquidation of an account which should not be liquidatable.

Example: (previous values are 0 for simplicity)

Bob has a SHORT position in market A of the size of 200 ETH. He holds the position for 1 day, no one interacts with the market for 1 day.
The current skew of the market is 100 (short earns fees, long pays fees), the skewScale is 10.000 and the oraclePrice is 3.500USD/ETH. This results in a priceBeforeDelta of:

priceBeforeDelta = 3.500 USD/ETH + (3.500 USD/ETH * 100/10.000 )= 3.535 USD/ETH

The fundingRate for the period is 0,5% (not influenced by change in positions) which results in a fundingFeePerUnit of:

fundingFeePerUnit = fundingRate/2 * priceBeforeDelta = 0,5%/2 * 3.535 USD/ETH = 8,84 USD/ETH

Therefore, the fees Bob should earn during this period are:

feesEarned = 200ETH * 8,84 USD/ETH = 1768 USD

If the admin now calls checkLiquidatableAccounts and thereby getAccountMarginRequirementUsdAndUnrealizedPnlUsd the accruedFunding is determined by simulating the closer of the 200 ETH position which results in the following markPrice:

priceBeforeDelta = 3.500 USD/ETH + (3.500 USD/ETH * 100/10.000) = 3.535 USD/ETH

priceAfterDelta = 3.500 USD/ETH + (3.500 USD/ETH * (-100/10.000)) = 3.465 USD/ETH

markPrice = (3535 + 3465)/2 = 3500 USD/ETH

Using this markPrice to calculate the fundingFeePerUnit turns out to be:

fundingFeePerUnit = fundingRate/2 * markPrice = 0,5%/2 * 3.500 USD/ETH = 8,75 USD/ETH

Therefore, the accruedFunding Bobs position has earned, according to the calculation is 1750 USD instead of the actual 1768 USD.

newFeesEarned = 200ETH * 8,75 USD/ETH = 1750 USD

Since the accruedFunding directly determines if an account is liquidatable or not, this can lead to the unrightfully liquidations of the account. Because of that, not only the user will need to pay the liquidation fee but he will also lose the difference in accruedFunds since it is never paid out to him

Recommended Mitigation Steps

When calculating the accruedFunding of a position, calculating the new fundingFeePerUnit for the market useing the priceBeforeDelta instead of the markPrice. This ensure that all accumulated fees are reflected properly and no account is liquidated even it should not be liquidated.