Zaros Part 1

Lines of code

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L153

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L281

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L392-L393

Impact

When filling an order in _fillOrder the markPrice is used to calculate the NextFundingFeePerUnit for a market and thereby the fees other users need to pay/receive for holding a position. Because the markPrice is influenced by the size and direction of the position which is filled this leads to wrong fees which are payed/erned in the market.

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.
The first stap to update fundingFeePerUnit when an order is filled is to calculate the currentFundingRate :

This CurrentFundingRate as well as the makrPrice are than taken to calculate the nextFundingFeePerUnit:

For this, the pendingFundingFeePerUnit is calculated by getting the avgFundingRate since the last update and multiplying it with the provided markPriceX18 and the time passed in days (ProportionalElapsedSinceLastFunding):

Based on the explanation above it is obvious, that the provided price for the calculation has a direct impact on the fees all users of the market will need to pay/will receive for holding their positions.

In the current implementation for calculating the pendingFundingFeePerUnit when an order is filled, the markPrice is used, which is an average of the price in the market before and after the position is adjusted:

The issue arises from the fact that the average price (markPrice) is influenced by the size in change for the position that is filled(skewDelta). This in turn influences the fees everyone needs to pay in the market. But the fees users should pay/receive until the moment the position is filled should be fixed, no matter what the size and direction of the next interaction with the market is. Therefore, not the markPrice should be used for calculating the fundingFeePerUnit but the priceBeforeDelta.

Example: (previous values are 0 for simplicity)

Bob has a SHORT position in market A of the size of 10 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 = 10ETH * 8,84 USD/ETH = 88,4 USD

If now a position of the size of 200 ETH is filled, the markPrice ends up being:

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 fees Bob earned get reduced from 88,4 USD to 87,5 USD.

newFeesEarned = 10ETH * 8,75 USD/ETH = 87,5 USD

Recommended Mitigation Steps

When filling a position and calculating the new fundingFeePerUnit for the market, make sure to use the priceBeforeDelta instead of the markPrice to ensure that all accumulated fees are reflected properly in the fundingFeePerUnit variable