berndartmueller - Incorrect position value calculation due to using mark (perp market price) instead of index (spot) price

[github-actions[bot]]

berndartmueller

medium

Incorrect position value calculation due to using mark (perp market price) instead of index (spot) price

Summary

The PerpDepository contract uses the mark price to calculate the position value. This contrasts Perp's AccountBalance.getTotalPositionValue function and internal liquidation condition evaluation, which uses the index price.

Vulnerability Detail

Mark price is the price of the derivative (perpetual future), whereas the index price is the price of the real asset (spot price). Funding payments are the key mechanism used in perpetual futures to keep the mark price close to the index price.

The PerpDepository contract calculates the unrealized (negative) PnL to determine if a rebalancing or quote minting is necessary. The getUnrealizedPnl function calls the internal getPositionValue function, which uses the mark price (via the Uniswap V3 pool) to calculate the position value.

This contrasts Perp's AccountBalance.getTotalPositionValue function, which uses the index TWAP price. Liquidation conditions are also based on the index price (see docs).

Consider the following scenario:

The mark price of the assetToken = WETH is 1_000e6 USDC, while the index price is 1_500e6 USDC. The PerpDepository.getUnrealizedPnl function would calculate a positive PnL, as the short position, based on the mark price, is worth more than the collateral. However, based on the index price, the PnL would be already negative and in need of a rebalancing.

Impact

In certain market conditions, the PerpDepository contract could allow or disallow rebalancing or quote minting when it should not.

For example, having a negative PnL potentially leads to a depegg from its USD parity.

Code Snippet

integrations/perp/PerpDepository.sol#L709

/// @notice Returns the current size of the short position in quote amount.
/// @return Position size
function getPositionValue() public view returns (uint256) {
    uint256 markPrice = getMarkPriceTwap(15);
    int256 positionSize = IAccountBalance(clearingHouse.getAccountBalance())
        .getTakerPositionSize(address(this), market);
    return markPrice.mulWadUp(_abs(positionSize));
}

function getMarkPriceTwap(uint32 twapInterval)
    public
    view
    returns (uint256)
{
    IExchange exchange = IExchange(clearingHouse.getExchange());
    uint256 markPrice = exchange
        .getSqrtMarkTwapX96(market, twapInterval)
        .formatSqrtPriceX96ToPriceX96()
        .formatX96ToX10_18();
    return markPrice;
}

Perp's AccountBalance.sol#L377

function getTotalPositionValue(address trader, address baseToken) public view override returns (int256) {
    int256 positionSize = getTotalPositionSize(trader, baseToken);
    return _getPositionValue(baseToken, positionSize);
}

Perp's AccountBalance.sol#L474

function _getPositionValue(address baseToken, int256 positionSize) internal view returns (int256) {
    if (positionSize == 0) return 0;

    uint256 indexTwap = _getReferencePrice(baseToken);
    // both positionSize & indexTwap are in 10^18 already
    // overflow inspection:
    // only overflow when position value in USD(18 decimals) > 2^255 / 10^18
    return positionSize.mulDiv(indexTwap.toInt256(), 1e18);
}

Tool used

Manual Review

Recommendation

Consider using Perp's AccountBalance.getTotalPositionValue function to calculate the current size of the short position.

[WarTech9]

@acamill any thoughts on this?

[acamill]

That makes sense, this is related to the known issues of spread arbitrage between spot and perp prices. It’s working similarly to the Solana implementation in that regard? Or I overlook something. Over time the spread differences ended up toward 0 on Solana

[IAm0x52]

Would be interested to hear feedback but it seems to me that using either index or mark price is irrelevant. The goal of rebalancing is just to prevent the protocol from being liquidated and doesn't affect the delta neutrality of the vault. It wants to keep the total short WETH exposure the same as the long (held) WETH exposure, which doesn't change with the USD price.

Both index and mark have their benefits and downsides. As mentioned in this report using the mark price can lead to unnecessary rebalancing but mark price is a better indication of the liquidity available on the exchange itself, which at the end of the day is what is being bought/sold to keep the vault delta neutral.

Personally I don't see either value as superior or significant risk associated with using one over the other.

[WarTech9]

it's probably better to use the index price to calculate the position value to align more with the way Perpetual Protocol accounts for collateral value but this is low to medium priority.

[hrishibhat]

Based on the comments above there doesn't seem to be a clear loss of funds. Considering this issue a low.