Zaros Part 1

Summary

Traders are charged higher fees when providing liquidity

Vulnerability Details

UD60x18 feeBps = isSkewGtZero != isBuyOrder && !skew.isZero()
? ud60x18(self.configuration.orderFees.makerFee)
: ud60x18(self.configuration.orderFees.takerFee);

This condition is intended to determine when to apply the maker fee, but it doesn't cover all cases correctly.

Let's break down the condition:

1. isSkewGtZero != isBuyOrder: This checks if the skew direction is opposite to the order direction.

2. !skew.isZero(): This ensures the skew is not zero.

The issue is that this condition doesn't correctly identify all situations where a maker fee should be applied.

Negative Skew (short-heavy market):

• Current skew: -100

• isSkewGtZero = false

a) Buy order (isBuyOrder = true):

• isSkewGtZero != isBuyOrder is true

• Result: Maker fee applied (correct)

b) Sell order (isBuyOrder = false):

• isSkewGtZero != isBuyOrder is false

• Result: Taker fee applied (incorrect - should be maker fee)

The error occurs in case b. When the market is short-heavy (negative skew) and a sell order is placed, it should be charged a maker fee because it's reducing the market imbalance. However, the current condition incorrectly applies a taker fee.
https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/PerpMarket.sol#L194C13-L196C66

Impact

In scenarios like this, traders are charged higher fees (taker fee instead of maker fee) when they're actually providing liquidity to the market. This discourages beneficial market-making behavior.

Tools Used

Manual Review

Recommendations

UD60x18 feeBps = (isSkewGtZero && !isBuyOrder || !isSkewGtZero && isBuyOrder) && !skew.isZero()
? ud60x18(self.configuration.orderFees.makerFee)
: ud60x18(self.configuration.orderFees.takerFee);