GMX V2 Vault

The withdraw function in the PerpetualVault contract allows users to specify a recipient when withdrawing collateral.

The internal _withdraw function is then called to withdraw funds from GMX if the position is open. Once withdrawn, the collateral tokens are transferred to the user-specified recipient address to complete the withdrawal flow. No other actions are permitted until this process is completed.

This allows a malicious actor to perform the following attack:

• Call the withdraw function with the recipient set to a blacklisted USDC address.
• The afterOrderExecution function in the GmxUtils contract will revert when trying to send the

collateral (USDC) to the blacklisted address.

• The flow variable will still be 3 (WITHDRAW) and _gmxLock set to true, preventing any further

actions.

This effectively causes a DOS attack by locking the contract in a state where no further actions can be taken.

Do not transfer tokens directly to the recipient, instead make the token claimable for them in a separate transaction and store their claimable balance in a mapping.

Gamma Team: The issue was resolved in commit a557f1d.

Proof of concept: PoC

A user is able to arbitrarily call settle and overwrite any order that is in the queue. By doing this, the user will not receive funds from their withdrawal.

An attacker could easily time this to repeatedly cause users to have their withdrawn funds locked in the PerpetualVault contract.

Implement access control to the function so that it can only be called from the PerpetualVault contract.

Gamma Team: The issue was resolved in commit b4022d5.

GMX allows users to set their callback contract in case of liquidation through the setSavedCallbackContract function.

function setSavedCallbackContract(address market,address callbackContract) external payable nonReentrant

This feature can be leveraged to bypass the validCallback function because the order is a liquidation which GMXUtils will not prevent.

An attacker would do so by executing the following attack:

• Attacker will set the callback contract to GMXUtils
• Open small highly leveraged position
• Attacker gets liquidated and the GMXUtils contracts afterOrderExecution will be called
• queue will be deleted, removing the pending order.
• PerpetualVault contracts afterOrderExecution will then be called and all conditionals will be skipped

leaving the protocol in a flow state

• Protocol is bricked as it is stuck in a flow state, and no pending deposits/withdrawals.

The result of this attack would be DoS of the protocol as well as a lost order from the deleted queue.

Validate that the liquidation is for the vault positionKey rather than any arbitrary position in the validCallback modifier.

Gamma Team: The issue was resolved in commit 57b3fae. 14

In the perpetual vault when withdrawing value from the vault there is no accounting for the negative PnL that a GMX perpetual position may have.

Instead the user’s decrease order contains the collateralDeltaAmount and sizeDeltaInUsd amounts that are proportional to the shares the user holds.

The sizeDeltaInUsd value will indeed realize the proportion of the negative PnL that the user owns, however this negative PnL amount will be deducted from the position’s collateral that remains, and not from the collateralDeltaAmount which will be rewarded to the withdrawer.

As a result withdrawers avoid losses and disburse them onto other vault holders.

Account for the PnL of a position that will be realized when computing the collateralDeltaAmount for a withdrawal.

Gamma Team: The issue was resolved in commit 94347b8.

The cancelDeposit function has an onlyKeeper modifier that restricts its execution to the KeeperProxy contract.

However, the KeeperProxy does not implement the necessary functionality to call this function, making it currently impossible to invoke.

The cancelDeposit function may be needed to cancel a reverting deposit to GMX, making it essential for the system's proper operation.

Implement a function in the KeeperProxy contract to call the cancelDeposit function.

Gamma Team: The issue was resolved in commit 24f5790.

The cancelDeposit function allows the protocol to cancel an ongoing deposit, resetting the state and deleting the deposit request.

However, it currently forgets to deduct the deposit request's amount from the totalDepositAmount state variable. This variable is used to ensure that the deposited amount within the protocol remains under the deposit cap.

Consequently, the missing deduction in the cancelDeposit function will lead to an overestimation of the total deposits. Every time cancelDeposit is called, the remaining possible deposit amount (maxDepositAmount - totalDepositAmount) will be incorrectly reduced by the cancelled amount.

This will incorrectly limit deposits, affecting the vault strategy and, in the worst case, could cause a DOS for the entire vault.

Ensure that the cancelDeposit function deducts the cancelled deposits amount from the totalDepositAmount state variable.

Gamma Team: The issue was resolved in commit 5c40cff.

When the afterOrderExecution function is called, Gamma has the opportunity to collect funding fees for both the long and short tokens.

It is important for Gamma to claim these fees, otherwise they will remain in GXM. However, only the short funding fee is currently being claimed (tokens[0] = order.addresses.initialCollateralToken).

This issue means that some funding fees will go unclaimed, resulting in the expected yield not being distributed to users as intended.

Consider claiming both long and short funding fees.

Gamma Team: The issue was resolved in commit f2008b6.

The GmxUtils contract doesn’t account for Automatic Deleveraging (ADL) from GMX, which can partially or fully close profitable positions if pending profits exceed the market's threshold.

During an ADL event, the receiver variable is set to the account address, which in this case is the GmxUtils contract. Since the GmxUtils contract doesn’t set a savedCallbackContract (which is used as the callback contract for ADLs), the callback address will default to the zero address.

As a result, the ADL callback will not be triggered, meaning the ADL event won't be properly handled. This causes any funds sent to the GmxUtils contract to become stuck with no way to retrieve them.

Additionally, these funds will no longer be included in share calculations, leading to discrepancies in share distribution and vault accounting, as they aren’t in the PerpetualVault contract or the GMX position.

If the position is closed, further calls to GMX, such as withdrawal requests, could lead to a denial of service (DOS) because the PerpetualVault is incorrectly marked as open while the GMX position is closed, causing those calls to repeatedly fail.

The GmxUtils contract should set the savedCallbackContract so the afterOrderExecution function in the GmxUtils contract is called during ADLs or liquidations. Additionally the validCallback modifier should allow the AdlHandler to call the GmxUtils contract for the current positionKey. The afterOrderExecution function should also account for ADL calls from GMX.

This update should include a mechanism to transfer any funds sent to the GmxUtils contract due to ADL to the PerpetualVault contract. Additionally, the afterOrderExecution function in the PerpetualVault contract should check if the position is still open and handle the position decrease accordingly.

If the position is closed, it should adjust the flow to reflect that closure. It is also important to consider the various flows (deposit, signal change, withdraw, compound) that the vault can be in during the PerpetualVault callback.

Gamma Team: Will handle liquidation risk via offchain logic. 19

The afterOrderExecution function in the GmxUtils contract assumes that only one output token will be returned. However, GMX decrease position orders can output two tokens instead of a single token if the decrease position swap fails.

As a result, this scenario is not properly accounted for in the _handleReturn function, which is responsible for calculating the user's withdrawal amounts and updating global states. This can result in users receiving less than they should and can corrupt the accounting.

Update the afterOrderExecution function in the GmxUtils contract to read and pass down the secondaryOutputToken and secondaryOutputAmount in the same way as the outputToken and outputAmount.

Then, modify the _handleReturn function in the PerpetualVault contract to account for both tokens.

Gamma Team: The issue was resolved in commit f2008b6.

Proof of concept: PoC

During a liquidation, the receiver variable is set to the account address, which in this case is the GmxUtils contract. Since the GmxUtils contract doesn’t set a savedCallbackContract (which is used as the callback contract for liquidations), the callback address will default to the zero address.

As a result, the callback will not be triggered, meaning the afterOrderExecution function in the GmxUtils contract, which is meant to send the liquidated funds to the perpetualVault contract won’t be triggered. This causes the funds sent to the GmxUtils contract to become stuck with no way to retrieve them.

Additionally, interacting with the protocol after said liquidation can lead to DoS's and incorrect fund and share allocation. This is in part because liquidations will close the position which results in withdraw attempts failing when the settle order executes.

After the failed execution Gamma will retry the settle over and over failing each time. Deposits also present a special case where the deposit may underflow _totalAmount(marketPrices) could be less than amount: totalAmountBefore = _totalAmount(marketPrices) - amount;

The GmxUtils contract should set the savedCallbackContract so the afterOrderExecution function in the GmxUtils contract is called during ADLs or liquidations.

Additionally, consider handling the liquidation case where after funds are sent back to the perpetualVault there are specific actions taken to get the protocol to a state where DOS's will not occur. This includes updating the state so that Gamma has the position as closed. It also includes preparing the state for any upcoming or incoming deposits/withdrawals.

Set positionIsClosed to true and curPositionKey to bytes32(0) in afterOrderExecution(). Also, configure the callback via setSavedCallback(), and expect the call for liquidations to originate from GMX’s LiquidationHandler contract in validCallback().

Gamma Team: Liquidation issue; will handle liquidation risk via offchain logic. 21

When creating an increase order for a deposit the amountIn is assigned to the collateralToken balance of the vault, however the user may have only deposited a portion of these tokens.

Indeed when a user receives shares for their deposited amount, the minted shares are computed only based on the depositInfo[counter].amount and does not include any additional balance that may have been used to create the order.

However the user pays fees for the balance which was not a direct part of their deposit. A malicious actor may leverage this to cause the afterOrderExecution execution to revert by forcing the feeAmount to be larger than the depositInfo[counter].amount.

Consider the following example:

• The vault holds a 5x Long position in GMX
• User A deposits the minimum deposit amount of 10 USDC
• The positionFeeFactor fee in GMX is 30 basis points
• User A then donates 990 USDC to the vault before the next action is run
• The keeper runs the next action and initiates a GMX increase order for $1,000 * 5x leverage =

$5,000 sizeDeltaUsd

• The order is executed and the order fee is 0.003 * 5,000 = $15
• The $15 fee is larger than the user’s actual deposit amount which is stored in the

depositInfo[counter].amount

• An underflow occurs when attempting to compute the value to mint based on:

depositInfo[counter].amount - feeAmount = 10 - 15

As a result the active increase order is not recorded as completing in Gamma’s vault and the deposit flow is never completed, locking all user deposits. A malicious actor only needs to spend $1,000 in this example and could profit off of this activity by short-selling the GAMMA token and spreading negative press about the loss of funds for users.

Consider only charging the fee that the user’s deposit amount would directly be responsible for when determining the user’s shares amount. Otherwise consider only creating an increase order for the amount of USDC that the user deposited rather than the entire vault balance.

Gamma Team: The issue was resolved in commit 97159d6.

In the afterOrderExecution function the amount of shares minted to a depositor is dependent on the sizeDeltaUsd of the order. However this method ignores price impact which the vault position experiences due to the increase order.

This allows depositors to receive more shares of the vault than they ought to when their order is negatively impacted.

For example:

• Vault is 2x long on GMX with $100,000 sizeInUsd
• PnL is currently 0 and the vault position is worth $50,000
• User A deposits with $5,000 and the sizeInUsd for their order is $10,000
• User A’s order is negatively impacted such that it experiences a $1,000 loss relative to the current

market price

• Ignoring fees, User A is credited with their 5,000 collateralToken deposit for share calculations
• The vault worth excluding the trader’s collateral is $49,000 as the negative price impact is

attributed

• The trader receives $5,000/$49,000 ~= 20.41% of the share supply
• Instead the trader should have received $4,000/$50,000 ~= 18% of the share supply by rightfully

charging the impact to the depositor

Compute the amount of price impact that the depositor experienced by comparing the sizeInUsd of the order to the sizeInTokens increase of the position with respect to the current market prices in GMX.Then deduct this amount from the user’s credited deposit value when calling the _mint function.

Gamma Team: The issue was resolved in commit cb4ae42.

Guardian Team: The share calculation has been adjusted to account for the difference between the balance before and after. This modification will more accurately distribute shares, although there may still be a disproportionate distribution of shares in certain cases due to price fluctuations 23 between creation and execution.

During a liquidation, the afterOrderExecution function in the GmxUtils contract is responsible for sending the queue.tokenIn balance from the GmxUtils contract to the PerpetualVault contract.

The problem is because the queue.tokenIn variable is set during the createOrder function’s specifically for a MarketIncrease order and is deleted in the afterOrderExecution or afterOrderCancellation functions.

If a liquidation occurs when there is no MarketIncrease order in progress, the queue.tokenIn variable will be set to the zero address, causing the transfer of funds during liquidation to fail.

As a result, these funds in the GmxUtils contract become stuck with no way to retrieve them, leading to losses for users, as they are not included in the PerpetualVault or GMX position and will not be accounted for in share calculations.

The outputToken should be used from the eventData instead, similar to how it's done in the afterOrderExecution function. This ensures that the correct token address is used.

Gamma Team: Liquidation issue; will handle liquidation risk via offchain logic

GMX can cap negative price impact on decrease orders, leaving the excess amount in a claimable collateral pool that must be manually claimed using the ExchangeRouter.claimCollateral function.

This scenario is not currently handled, meaning that if it occurs during a user withdrawal, the user could receive less than they should have.

Additionally, the GmxUtils contract does not currently implement the claimCollateral function, which means those funds cannot be retrieved, leading to potential losses for users.

Update the GmxUtils contract to implement the claimCollateral function, ensuring that any excess funds due to capped negative price impact can be claimed and properly accounted for.

Additionally, modify the withdrawal process to handle this scenario, ensuring users receive the full amount they are entitled to, even when negative price impact is capped.

Gamma Team: The issue was resolved in commit 018ab42.

When an attempted order is canceled by GMX, the protocol chooses to handle this scenario by attempting to retry the previously attempted action.

However, there are numerous reasons why the transaction may fail again. For instance, if the reserve ratio or open interest is out of balance for a market, if a market or action is disabled, or if GMX triggers Auto Deleveraging.

A malicious user could even force this to occur by depositing, withdrawing, or swapping on GMX to attain one of these states. When this occurs Gamma’s flow will be stuck in its current state, and no other actions will be executable.

Reset the transaction flow, and require the user or keeper to reattempt the action again. In the case of deposits, make sure that amount is refunded.

Also consider adding additional admin privileged functions to terminate a flow, similar to cancelDeposit().

Gamma Team: The issue was resolved in commit 15f3967.

Guardian Team: There may still be DOS when actions are forced to repeatedly retry. Consider implementing the entire recommendation.

A malicious actor may observe that Gamma currently has a high leverage and intentionally push this leverage up to force the PerpetualVault position to be liquidated. A user may do this by creating large deposits and then initiating withdrawals for those deposits as soon as possible to levy the MarketDecrease order position fee on the vault position.

Users do not pay for the MarketDecrease order position fee and thus this fee is deducted from the remaining position thus changing the leverage of the remaining position. Once this has been repeated several times the vault position will have a very high leverage which makes it liquidatable almost instantaneously.

The malicious actor can profit off of this liquidation by setting a limit order which can only be triggered once the liquidation has occurred and the priceImpact allows for immediate profit.

Adjust the initialCollateralDelta amount for the Position fees that will be experienced for the MarketDecrease order.

This way the withdrawer takes on the burden of the fee and the vault position leverage cannot be manipulated.

Gamma Team: The issue was resolved in commit f03e60e.

In the cancelDeposit function there is no accounting for the swapProgressData, this causes several issues when a deposit is cancelled after a composite swap through Paraswap and GMX where the GMX swap failed.

Firstly the transfer of collateral tokens is likely to fail as part of the deposited collateral tokens will have been swapped to the index token and are recorded in the swapProgressData.swapped value.

safeTransfer is not used for this transfer and therefore depending on the collateralToken used, users could lose their funds as a result.

Secondly the swapProgressData.swapped value is not cleared when the deposit occurs, therefore the next deposit will be credited with receiving these swapped tokens.

Therefore if the vault did have enough collateral tokens to pay out the user, this swapped value would then be double counted and awarded to the next depositor at the expense of previous vault shareholders.

Account for the swapProgressData.swapped value in the cancelDeposit function such that if the swapProgressData.swapped value is nonzero that amount of index tokens is transferred to the user and deducted from the amount of collateral tokens transferred.

Then clear the swapProgressData at the end of the function. Additionally, use safeTransfer in the cancelDeposit function.

Gamma Team: cancelDeposit function is allowed to call when runNextAction() fails. Not used to call after runNextAction.

In GMX if a decrease order is deemed to leave the remaining position with insufficient collateral for its open interest, then the initialCollateralDelta is reassigned to 0.

This will result in withdrawals which are deemed to leave behind insufficient collateral via the willPositionCollateralBeSufficient check in receiving no collateral tokens out from GMX.

Therefore, withdrawers which are affected by this case lose all collateral and will only receive a portion of any profit which was gained from the position.

Validate that the willPositionCollateralBeSufficient check in GMX will not be triggered upon creating withdrawals from GMX, and if it is triggered upon execution of a decrease order for a withdrawal, adjust the user’s shares such that they can claim their portion of the collateral that did not come out of the decrease order.

Gamma Team: The issue was resolved in commit 75723b7.

Guardian Team: Only assign the sizeDeltaInUsd to the entire position size if the willCollateralBeSufficient check is false. And additionally be sure to carefully account for the user only receiving the funds out of the order which they should when this happens.

The GmxUtils contract uses the gExchangeRouter and reader contracts, saving their addresses as constants during deployment.

However, it is recommended by GMX to allow these addresses to be changeable (not immutable) and to provide setter functions.

These contracts are currently used in essential functions by the protocol, meaning if they were to be changed, the protocol will not function correctly until the addresses are updated through an upgrade.

Reference GMX's datastore and use the address that is stored their for applicable GMX addresses.

Gamma Team: We use TransparentProxy so we can upgrade it later. We will take account for that later.

Orders can become stuck halting protocol functionality when order execution is disabled. When an order is created GMX will check that the create order feature is enabled.If it is not, it will revert.

However, during order creation there is no check that the order execution feature will be disabled.

This means that when order execution is disabled Gamma orders will still successfully be created but will not execute due to the feature validation in _executeOrder until the feature is enabled again. Halting protocol functionality for a period of time.

Before sending an order to GMX check that the execution feature is enabled.

Gamma Team: The issue was resolved in commit 52e272d.

The _check function in the KeeperProxy contract checks the price difference between the given price and the Chainlink price.

The issue is that the Chainlink price is fetched using the latestAnswer function, which is deprecated and should no longer be used reference.

The _check function should instead use the latestRoundData function for price retrieval, as it allows for additional validations to ensure that the price data is current and reliable.

Refer to the Chainlink documentation for implementation details.

Gamma Team: The issue was resolved in commit c863800.

The _check function in the KeeperProxy contract retrieves the Chainlink price; however, the price is fetched and used without performing any validation on the price or the sequencer status.

Chainlink recommends following certain security practices, such as checking for stale or invalid prices.

Additionally, when using Chainlink with L2 chains like Arbitrum, it is crucial to check whether the L2 Sequencer is down.

Add validation checks to ensure the price retrieved by the _check function is not stale or invalid Ref.

Also, implement sequencer feed checks to confirm the L2 Sequencer is operational before using the price data.

Follow the Chainlink example for these checks as outlined in their documentation.

Gamma Team: The issue was resolved in commit c863800.

In withdraw(), _payExecutionFee() is called, which validates the msg.value is large enough to cover GMX’s execution cost.

However, if a user goes to withdraw when there are no open positions, there will be no calls made to GMX.

Additionally, when calls to GMX are made, any excess execution fee refund by GMX is not returned to the user. This can occur when a user calls deposit() or withdraw().

Move the _payExecutionFee call into the curPositionKey != bytes32(0) case for withdrawing.

Additionally, implement functionality to refund excess GMX execution fees to the creator of the deposit or withdrawal.

Gamma Team: The issue was resolved in commit 1087920.

GMX keepers will not always execute orders in a reasonable amount of time. For reasons outside of the creators control.

With no functionality to manually cancel these orders an order can be sitting for a prolonged period of time halting any other order operations.

Consider adding functionality for keepers to cancel orders after a certain amount of time.

Gamma Team: The issue was resolved in commit 15f3967.

In the getExecutionGasLimit function the ESTIMATED_GAS_FEE_BASE_AMOUNT key is used to estimate the gas.

However with the upgrade of GMX V2.1 the ESTIMATED_GAS_FEE_BASE_AMOUNT_V2_1 key is now used to validate the executionFee base amount upon creating an order.

Use the updated ESTIMATED_GAS_FEE_BASE_AMOUNT_V2_1 key in the getExecutionGasLimit function.

Gamma Team: The issue was resolved in commit b4022d5.

It is possible for a liquidation to send a long token from GMX. For instance, a liquidation can occur when the position was in profit, but fees caused a liquidation.

Alternatively, if GMX failed to swap into the collateral token, you can receive the long token. If this were to occur, the token would be stuck within GMXUtils.sol, and unrescuable.

Check if the long token was sent with a liquidation, and if so transfer it to PerpVault.sol. Additionally, consider adding an admin privileged function to rescue unexpected token transfers.

Gamma Team: Liquidation issue; will handle liquidation risk via offchain logic

In the deposit and withdraw functions the _payExecutionFee function is called regardless of if the action will require a GMX order.

For example, if the vault is 1x long and a paraswap swap will be used to execute the swap, the execution fee for a GMX swap is still collected.

This unnecessarily charges the user for an action that will not occur and offers them no refund in the event that a normal swap is used.

Consider refunding the executionFee to the user if a GMX action is not performed on a swap.

Gamma Team: The issue was resolved in commit 8a10486.

Guardian Team: If the intention is to only refund the execution fee when it is unused then consider refunding the full execution fee when there is sufficient funds.

The protocol only allows one action to be processed at a time and depending on the action and circumstances each execution can take up to minutes to be finalized.

As a result a malicious actor could intentionally create many small positions when the vault has no open position, and then when the vault has an open GMX position start triggering withdrawals from each account to DoS the protocol for an extended period.

If each action takes 1 minute to process and the minimumDeposit amount is $10 then an attacker can DoS the protocol for one hour with $600 + gas. The attacker would recoup the majority of this upfront cost after they withdraw all shares.

This cost may be profitable if the actor were a GM holder wanting to keep Gamma in a poor position for longer to reap PnL gains.

Additionally a short-seller may be able to profit from such a DoS, spreading word of the trapped funds.

Consider batching deposits and withdrawals to GMX to avoid DoS’s triggered by small individual accounts. Otherwise assign the minimum deposit value such that this attack is unprofitable.

Gamma Team: Will set minDeposit at $1,000 upon initialization.

setPerpVault() will initialize perpVault, and then will revert anytime it is called again. If a user monitors deployment and calls setPerpVault(), they will be able to set perpVault.

This will give a user control over execution flow for numerous function calls, if not dealt with. You could redeploy the contracts again, but you would be at risk of the same attack repeating itself.

Make setPerpVault() an access controlled function.

Gamma Team: Acknowledged. There's no reason that the attacker does that action.

In the GMXUtils contract, when creating orders for GMX the Oracle price count is not accounted for when determining the size of the executionFee.

However in GMX V2.1 an additional amount of executionFee is necessary to account for the number of oracle prices to execute the order.

This logic is seen here:

https://github.com/gmx-io/gmx-synthetics/blob/1938e365dc009342aa288aa6b42fc1fd3cd9e45d/c

ontracts/gas/GasUtils.sol#L212

Include the oracle price count estimated costs in the execution fee sent to GMX.

Gamma Team: The issue was resolved in commit b4022d5.

When the outputted amount is in both the long and the short underlying token, the amount variable will be less than expected.

Causing the governance fee to be less than expected or potentially no fee when a position was actually in profit.

Consider accounting for the long token when determining how much profit a user is in for governance fees.

Or Document to members of the governance team that some profitable positions may generate less yield than expected.

Gamma Team: The issue was resolved in commit f2008b6.

When computing the feeAmount which a user pays the forPositiveImpact is hardcoded as false, therefore even if a user’s order positively impacted the GM market skew and received a lower fee rate the user has to pay the higher rate when Gamma accounts for their orders fee.

This behavior is likely more trouble to fix than it is worth, but be aware of this disagreement between the fees charged by the two systems and clearly document this for users.

Gamma Team: This behavior is likely more trouble to fix than it’s worth, so we will let users know about that. Always pay fee for negative price impact case.

Throughout the contracts the boolean comparison == true is used to check if a boolean value is true. However the boolean value itself can be used to save gas.

Remove all instances of == true.

Gamma Team: The issue was resolved in commit a3f8827.

A strict acceptable price can lead to a run failing. It would fail because of the position that is being closed is large enough that the price impact would exceed the threshold. This can happen naturally as the positions get larger and larger. This can also happen as an attack.

Since a user would be able to know roughly when a run is going to occur they can either open a position right before Gamma which would increase the price impact gamma will face. Or the attacker could withdraw from GMX, with less capital in the market newly created positions will face greater price impact.

By preventing the run from executing the rest of the protocol will not be able to be used for a period of time. And more importantly the protocol will be forced to hold a position that does not align with their strategy since they won’t be able to close or change it.

On runs consider passing in an acceptablePrice. This flexibility would allow position changes despite market conditions.

Also consider allowing the acceptable price to be changed when the afterOrderCancellation is called. Preventing situations where the order repeatedly fails.

Gamma Team: The issue was resolved in commit 5f387ce.

The cancelDeposit function allows the protocol to cancel a deposit request, but it currently doesn’t fully remove the deposit as it forgets to remove the entry from the userDeposits mapping.

As a result, the getUserDeposits function, which retrieves all deposit IDs for a user, will still include the cancelled deposit.

Ensure the cancelled deposit is removed from the userDeposits mapping, similar to how it is handled in the _burn function.

Gamma Team: The issue was resolved in commit 5c40cff.

The cancelDeposit function allows the protocol to cancel a deposit request, and the user's deposited funds are then sent back to the user.

However, the funds are currently transferred using the transfer method instead of the safeTransfer method, which is used elsewhere in the code.

This could be problematic, as some tokens may not revert on failure and instead return false, leading to undetected transfer failures.

Replace the transfer method with safeTransfer in the cancelDeposit function to ensure that token transfers are properly handled and any potential failures are detected.

Gamma Team: The issue was resolved in commit 1087920.

It is possible to exceed the deposit cap by donating USDC to the vault after a deposit, these funds would essentially be donated to the vault, but if there were a single large holder they would be donating to themselves and surpassing the deposit cap.

Be aware of this possibility and ensure vaults are made up of a diverse number of depositors.

Gamma Team: The issue was resolved in commit b4022d5.

If a user, who holds the entirety of the shares, withdraws while a position is open, it will lead to the GMX position being closed.

This will cause curPositionKey to be set to bytes32(0), but positionIsClosed will not be updated in the withdrawal flow.

This will lead to unexpected issues when calling run(), as the current state will not match the expected state.

Check if the position has been fully closed in the withdrawal flow. If that is the case, set positionIsClosed to true so there are no discrepancies between curPositionKey and positionIsClosed.

Gamma Team: Acknowledged.

If the current action is a settle then in the afterOrderExecution the condition to be met is based on sizeDelta and output amount.

This relies on strict equality and may not be future-proof for any GMX changes to how outputAmount is calculated.

If this case occurred and the settle condition was not met, the execution flow will enter the withdrawal case directly.

Consequently, the user will get the settle amount, instead of their withdrawal amount. Additionally the shares from the withdrawal will be burned but the position will be relatively unchanged.

Add a SETTLE to the Flow enum and use that in the afterOrderExecution callback.

Gamma Team: The issue was resolved in commit 565919f.

In the PerpetualVault when the vault is in a 1x long state deposits are only valued based upon the proportion of index tokens they contribute to the vault.

However upon withdrawals the user receives their shares proportional value of the collateral tokens in the vault.

As a result if there are any leftover collateral tokens in the vault from unswapped funding fees or for any other reason, these collateral tokens can be siphoned by an arbitraguer.

Consider including the collateral token balance value when computing the shares to issue for a deposit when the vault is in a 1x long state.

Gamma Team: The issue was resolved in commit f2008b6.