Incorrect Execution Fee Refund address on Failed Deposits or withdrawals in Strategy Vaults
The Strategy Vaults within the protocol use a two-step process for handling deposits/withdrawals via GMXv2. A `createDeposit()` transaction is followed by a callback function (afterDepositExecution() or afterDepositCancellation()) based on the transaction's success. In the event of a failed deposit due to vault health checks, the execution fee refund is mistakenly sent to the depositor instead of the keeper who triggers the deposit failure process.\\nThe protocol handles the `deposit` through the `deposit` function, which uses several parameters including an execution fee that refunds any excess fees.\\n```\\nfunction deposit(GMXTypes.DepositParams memory dp) external payable nonReentrant {\\n        GMXDeposit.deposit(_store, dp, false);\\n    }\\n\\nstruct DepositParams {\\n    // Address of token depositing; can be tokenA, tokenB or lpToken\\n    address token;\\n    // Amount of token to deposit in token decimals\\n    uint256 amt;\\n    // Minimum amount of shares to receive in 1e18\\n    uint256 minSharesAmt;\\n    // Slippage tolerance for adding liquidity; e.g. 3 = 0.03%\\n    uint256 slippage;\\n    // Execution fee sent to GMX for adding liquidity\\n    uint256 executionFee;\\n  }\\n```\\n\\nThe refund is intended for the message sender (msg.sender), which in the initial deposit case, is the depositor. This is established in the `GMXDeposit.deposit` function, where `self.refundee` is assigned to `msg.sender`.\\n```\\nfunction deposit(GMXTypes.Store storage self, GMXTypes.DepositParams memory dp, bool isNative) external {\\n        // Sweep any tokenA/B in vault to the temporary trove for future compouding and to prevent\\n        // it from being considered as part of depositor's assets\\n        if (self.tokenA.balanceOf(address(this)) > 0) {\\n            self.tokenA.safeTransfer(self.trove, self.tokenA.balanceOf(address(this)));\\n        }\\n        if (self.tokenB.balanceOf(address(this)) > 0) {\\n            self.tokenB.safeTransfer(self.trove, self.tokenB.balanceOf(address(this)));\\n        }\\n\\n        self.refundee = payable(msg.sender);\\n\\n  // rest of code\\n\\n  _dc.depositKey = GMXManager.addLiquidity(self, _alp);\\n\\n        self.depositCache = _dc;\\n\\n        emit DepositCreated(_dc.user, _dc.depositParams.token, _dc.depositParams.amt);\\n    }\\n```\\n\\nIf the deposit passes the GMX checks, the `afterDepositExecution` callback is triggered, leading to `vault.processDeposit()` to check the vault's health. A failure here updates the status to `GMXTypes.Status.Deposit_Failed`. The reversal process is then handled by the `processDepositFailure` function, which can only be called by keepers. They pay for the transaction's gas costs, including the execution fee.\\n```\\nfunction processDepositFailure(uint256 slippage, uint256 executionFee) external payable onlyKeeper {\\n        GMXDeposit.processDepositFailure(_store, slippage, executionFee);\\n    }\\n```\\n\\nIn `GMXDeposit.processDepositFailure`, `self.refundee` is not updated, resulting in any excess execution fees being incorrectly sent to the initial depositor, although the keeper paid for it.\\n```\\nfunction processDepositFailure(GMXTypes.Store storage self, uint256 slippage, uint256 executionFee) external {\\n        GMXChecks.beforeProcessAfterDepositFailureChecks(self);\\n\\n        GMXTypes.RemoveLiquidityParams memory _rlp;\\n\\n        // If current LP amount is somehow less or equal to amount before, we do not remove any liquidity\\n        if (GMXReader.lpAmt(self) <= self.depositCache.healthParams.lpAmtBefore) {\\n            processDepositFailureLiquidityWithdrawal(self);\\n        } else {\\n            // Remove only the newly added LP amount\\n            _rlp.lpAmt = GMXReader.lpAmt(self) - self.depositCache.healthParams.lpAmtBefore;\\n\\n            // If delta strategy is Long, remove all in tokenB to make it more\\n            // efficent to repay tokenB debt as Long strategy only borrows tokenB\\n            if (self.delta == GMXTypes.Delta.Long) {\\n                address[] memory _tokenASwapPath = new address[](1);\\n                _tokenASwapPath[0] = address(self.lpToken);\\n                _rlp.tokenASwapPath = _tokenASwapPath;\\n\\n                (_rlp.minTokenAAmt, _rlp.minTokenBAmt) = GMXManager.calcMinTokensSlippageAmt(\\n                    self, _rlp.lpAmt, address(self.tokenB), address(self.tokenB), slippage\\n                );\\n            } else {\\n                (_rlp.minTokenAAmt, _rlp.minTokenBAmt) = GMXManager.calcMinTokensSlippageAmt(\\n                    self, _rlp.lpAmt, address(self.tokenA), address(self.tokenB), slippage\\n                );\\n            }\\n\\n            _rlp.executionFee = executionFee;\\n\\n            // Remove liqudity\\n            self.depositCache.withdrawKey = GMXManager.removeLiquidity(self, _rlp);\\n        }\\n```\\n\\nThe same issue occurs in the `processWithdrawFailure` function where the excess fees will be sent to the initial user who called withdraw instead of the keeper.
The `processDepositFailure` and `processWithdrawFailure` functions must be modified to update `self.refundee` to the current executor of the function, which, in the case of deposit or withdraw failure, is the keeper.\\n```\\nfunction processDepositFailure(GMXTypes.Store storage self, uint256 slippage, uint256 executionFee) external {\\n        GMXChecks.beforeProcessAfterDepositFailureChecks(self);\\n\\n        GMXTypes.RemoveLiquidityParams memory _rlp;\\n\\n  self.refundee = payable(msg.sender);\\n\\n  // rest of code\\n        }\\n```\\n\\n```\\nfunction processWithdrawFailure(\\n    GMXTypes.Store storage self,\\n    uint256 slippage,\\n    uint256 executionFee\\n  ) external {\\n    GMXChecks.beforeProcessAfterWithdrawFailureChecks(self);\\n\\n self.refundee = payable(msg.sender);\\n\\n // rest of code\\n  }\\n```\\n
This flaw causes a loss of funds for the keepers, negatively impacting the vaults. Users also inadvertently receive extra fees that are rightfully owed to the keepers
```\\nfunction deposit(GMXTypes.DepositParams memory dp) external payable nonReentrant {\\n        GMXDeposit.deposit(_store, dp, false);\\n    }\\n\\nstruct DepositParams {\\n    // Address of token depositing; can be tokenA, tokenB or lpToken\\n    address token;\\n    // Amount of token to deposit in token decimals\\n    uint256 amt;\\n    // Minimum amount of shares to receive in 1e18\\n    uint256 minSharesAmt;\\n    // Slippage tolerance for adding liquidity; e.g. 3 = 0.03%\\n    uint256 slippage;\\n    // Execution fee sent to GMX for adding liquidity\\n    uint256 executionFee;\\n  }\\n```\\n