Liquidity Management

Description:

GMX has a maximum cap of negative price impact on decrease orders. If a trade is closed with a price impact higher than this percentage, then the additional impact would become claimable after a few days.

In the current implementation of PerpetualVault::claimCollateralRebates function, only collateralToken is added to the tokens array and sent to GMX's ExchangeRouter::claimCollateral function as a parameter.

In the code snippet below we see that only one type of token is added to the tokens array:

This is a problem, because negative price impact above the maximum value can occur in both long and short token. During MarketDecrease order execution, if the negative price impact is above the maximum limit, GMX adds the number of tokens above the limit to the claimable collateral pool and after few days it becomes claimable and the owner of the position who experienced the negative price impact can call ExchangeRouter::claimCollateral function to receive the rebate. During the claiming process, this function in GMX's MarketUtils gets called:

Notice, that this function validates market token balance of both long and short tokens, which means that negative price impact can be claimed both in long token and short token of the market (e.g. WETH and USDC).

The complete rebate claiming execution flow looks like this:

PerpetualVault::claimCollateralRebates -> GmxProxy::claimCollateralRebates -> ExchangeRouter::claimCollateral -> MarketUtils::claimCollateral (calculates amountToBeClaimed and transfers out the tokens) -> MarketUtils::validateMarketTokenBalance (ensures market token balance of both long and short token is enough, else - reverts)

Impact:

If GMX holds some of the claimable collateral in the form of long tokens (e.g. WETH), Gamma's current implementation will not claim it. The unclaimed long tokens will stay in GMX’s claimable collateral pool until properly requested. This means Gamma is leaving some rebates unclaimed, leading to inefficiencies.

Proof of concept:

In DecreasePositionCollateralUtils::processCollateral function in lines 433 - 473, we can see proof that negative price impact over the maximum limit can occur and be paid in both long and short tokens:

Source: https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/position/DecreasePositionCollateralUtils.sol#L433

Note, that:

1. values.priceImpactDiffUsd is the difference between maximum negative price impact and the actual price impact that was calculated (Proof: https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/position/DecreasePositionCollateralUtils.sol#L91)

2. Paying for price impact difference occurs in DecreasePositionCollateralUtils::payForCost function: https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/position/DecreasePositionCollateralUtils.sol#L521

3. MarketUtils::incrementClaimableCollateralAmount function is used for increasing the collateral amount the owner of the position can claim, and as we can see, it is called 2 times in the above code snippet, one time for collateral token, second time for secondaryOutputToken (e.g. WETH).

For further proof, take a look at this comment from lines 476 - 482 in DecreasePositionCollateralUtils::processCollateral function:

Source: https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/position/DecreasePositionCollateralUtils.sol#L476

Recommended Mitigation:

Change the length of both arrays to 2, add the index token to the tokens array and add market to the second slot in markets array, similar to the setup before calling claimFundingFees in GmxProxy::afterOrderExecution function.

Example fix: