IllIllI - Users can get impact pool discounts while also increasing the virtual impact pool skew

[sherlock-admin]

IllIllI

medium

Users can get impact pool discounts while also increasing the virtual impact pool skew

Summary

Virtual impacts will remain consistently negative, even if most pools' impacts are balanced

Vulnerability Detail

applyDeltaToPoolAmount()/applyDeltaToVirtualInventoryForPositions() always modify the virtual impact, even if the virtual impact wasn't consulted

Impact

Imagine a different stablecoin is one of the tokens in each of 5 markets where the other side is Eth, all five markes are part of the same virtual market, and each of the markets' swap imbalance is such that users will get a discount for swapping from Eth into the stablecoin, and the global virtual swap balance for 'stable'/Eth is flat. Users will keep swapping from Eth into the stable, which will push the individual pool towards being flat, but will push the virtual swap inventory for stables to be more and more negative. As soon as one of the five markets becomes flat, any user trying to swap Eth into a stable will suddenly be hit with the huge virtual swap impact imbalance

Code Snippet

applyDeltaToPoolAmount() unconditionally updates the virtual inventory for swaps:

// File: gmx-synthetics/contracts/market/MarketUtils.sol : MarketUtils.applyDeltaToPoolAmount()   #1

711 @>         applyDeltaToVirtualInventoryForSwaps(
712                dataStore,
713                eventEmitter,
714                market,
715                token,
716                delta
717:           );

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/market/MarketUtils.sol#L711-L717

but the actual price impact skips the virtual impact if one of the tokens doesn't have virtual inventory:

// File: gmx-synthetics/contracts/pricing/SwapPricingUtils.sol : SwapPricingUtils.getPriceImpactUsd()   #2

119 @>         if (!hasVirtualInventoryTokenA || !hasVirtualInventoryTokenB) {
120                return priceImpactUsd;
121:           }

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/pricing/SwapPricingUtils.sol#L119-L121

or if the impact was positive:

// File: gmx-synthetics/contracts/pricing/SwapPricingUtils.sol : SwapPricingUtils.getPriceImpactUsd()   #3

96             // the virtual price impact calculation is skipped if the price impact
97             // is positive since the action is helping to balance the pool
98             //
99             // in case two virtual pools are unbalanced in a different direction
100            // e.g. pool0 has more WNT than USDC while pool1 has less WNT
101            // than USDT
102            // not skipping the virtual price impact calculation would lead to
103            // a negative price impact for any trade on either pools and would
104            // disincentivise the balancing of pools
105:@>         if (priceImpactUsd >= 0) { return priceImpactUsd; }

https://github.com/sherlock-audit/2023-04-gmx/blob/main/gmx-synthetics/contracts/pricing/SwapPricingUtils.sol#L91-L104

Virtual position impacts have the same issue

Tool used

Manual Review

Recommendation

Don't adjust the virtual inventory for swaps if the virtual inventory wasn't consulted when calculating the impact.

[xvi10]

fixed in gmx-io/gmx-synthetics@1b35173

virtual swap price impact should be tracked by virtualInventoryForSwapsKey(virtualMarketId, isLongToken) instead of virtualInventoryForSwapsKey(virtualMarketId, token)

this would allow similar markets e.g. ETH/USDC, ETH/USDT to be associated together