Steadefi
Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Depositing to the GMX POOl will return sub-optimal return if the Pool is imbalanced

tripathi

Summary

Whenever A user deposits tokens to vault, vault create a leverage position depending[delta or delta neutral] in the GMX POOl. performing a proportional deposit is not optimal in every case and depositng tokens in such case will result in fewer LP tokens due to sub optimal trade. Eventually leading to a loss of gain for the strategy vault

Vulnerability Details

Alice deposits token A() into the vault to make Delta.Neutral position

File: GMXVault.sol
​
function deposit(GMXTypes.DepositParams memory dp) external payable nonReentrant {
GMXDeposit.deposit(_store, dp, false);
}

vault refer to deposit to GMXDeposit library to execute the further logic

File: GMXDeposit.sol
​
function deposit(
GMXTypes.Store storage self,
GMXTypes.DepositParams memory dp,
bool isNative
) external {
[................]
// Borrow assets and create deposit in GMX
(
uint256 _borrowTokenAAmt,
uint256 _borrowTokenBAmt
) = GMXManager.calcBorrow(self, _dc.depositValue);
​
[.........]
}

https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXDeposit.sol#L54

which calls calcBorrow on GMXManager Library for borrowing assets and making the position IN GMX POOL

File: GMXManager.sol
​
/**
* @notice Calculate amount of tokenA and tokenB to borrow
* @param self GMXTypes.Store
* @param depositValue USD value in 1e18
*/
function calcBorrow(
GMXTypes.Store storage self,
uint256 depositValue
) external view returns (uint256, uint256) {
// Calculate final position value based on deposit value
uint256 _positionValue = depositValue * self.leverage / SAFE_MULTIPLIER;
​
// Obtain the value to borrow
uint256 _borrowValue = _positionValue - depositValue;
​
uint256 _tokenADecimals = IERC20Metadata(address(self.tokenA)).decimals();
uint256 _tokenBDecimals = IERC20Metadata(address(self.tokenB)).decimals();
uint256 _borrowLongTokenAmt;
uint256 _borrowShortTokenAmt;
​
[....................]
​
// If delta is neutral, borrow appropriate amount in long token to hedge, and the rest in short token
if (self.delta == GMXTypes.Delta.Neutral) {
// Get token weights in LP, e.g. 50% = 5e17
(uint256 _tokenAWeight,) = GMXReader.tokenWeights(self);
​
// Get value of long token (typically tokenA)
uint256 _longTokenWeightedValue = _tokenAWeight * _positionValue / SAFE_MULTIPLIER;
​
// Borrow appropriate amount in long token to hedge
_borrowLongTokenAmt = _longTokenWeightedValue * SAFE_MULTIPLIER
/ GMXReader.convertToUsdValue(self, address(self.tokenA), 10**(_tokenADecimals))
/ (10 ** (18 - _tokenADecimals));
​
// Borrow the shortfall value in short token
_borrowShortTokenAmt = (_borrowValue - _longTokenWeightedValue) * SAFE_MULTIPLIER
/ GMXReader.convertToUsdValue(self, address(self.tokenB), 10**(_tokenBDecimals))
/ (10 ** (18 - _tokenBDecimals));
}
[.....................]
}

https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXManager.sol#L70

Here it consider the current reserve ratio of the pool and deposits in the same ratio.

While GMX docs clearly state that If deposits try to create balance in the pool[depositing in such way which will make actual token weight of index Token towards the TOKEN_WEIGHT defined in the pool] will get benefit technically more LP tokens and oppose to this less LP token if current deposits imbalance the Pool reserve the ratio
Reference

Even Curve pools work in the same way. Depositer get benefit if they try to balance the pool reserve making them optimal

Impact

It is clear that Weight of index token will not be always near equal to the Defined Total_Weight of the Pool.
So if the pool is imbalanced Depositing into GMXPool will not give optimal returns( resulting in fewer LP token), eventually leading to the loss of gain for the depositers affecting net APR

Tools Used

Manual Review

Recommendations

consider implementing check and if the pool is imablanced depositing(making levearge position) towards balancing the Index Token's weight will give optimal returns[extra LP tokens ]

Updates

Lead Judging Commences

hans Auditor
almost 2 years ago
hans Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

Suboptimal deposit/withdraw on GMX

More of a design choice. Leaving open for now for the sponsor's review.

tripathi Submitter
almost 2 years ago
hans Auditor
almost 2 years ago
Steadefi Lead Judge
almost 2 years ago
hans Auditor
almost 2 years ago
tripathi Submitter
almost 2 years ago
hans Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Design choice
tripathi Submitter
almost 2 years ago

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.