GMXPerpetualDEXLongVault Manual Review Findings

GMX-01M: Significant Centralization of Contract Configuration

Type	Severity	Location
Centralization Concern		GMXPerpetualDEXLongVault.sol:L321, L329, L337, L345, L353, L362, L370

Description:

The referenced functions permit the GMXPerpetualDEXLongVault contract to be arbitrarily configured by its owner.

Example:

Copy
317/**
318  * Update vault config struct
319  * @param _vaultConfig Vault config struct
320*/
321function updateVaultConfig(VaultConfig memory _vaultConfig) external onlyOwner {
322  vaultConfig = _vaultConfig;
323}
324
325/**
326  * Update management fee per second of vault
327  * @param _mgmtFeePerSecond Management fee in 1e18
328*/
329function updateMgmtFeePerSecond(uint256 _mgmtFeePerSecond) external onlyOwner {
330  mgmtFeePerSecond = _mgmtFeePerSecond;
331}
332
333/**
334  * Update performance fee of vault
335  * @param _perfFee Performance fee in 1e18
336*/
337function updatePerfFee(uint256 _perfFee) external onlyOwner {
338  perfFee = _perfFee;
339}
340
341/**
342  * Update vault's reader contract
343  * @param _reader Reader address
344*/
345function updateReader(IGMXPerpetualDEXLongReader _reader) external onlyOwner {
346  reader = _reader;
347}
348
349/**
350  * Update vault's manager contract
351  * @param _manager Manager address
352*/
353function updateManager(IGMXPerpetualDEXLongManager _manager) external onlyOwner {
354  manager = _manager;
355}
356
357/**
358  * Approve or revoke address to be a keeper for this vault
359  * @param _keeper Keeper address
360  * @param _approval Boolean to approve keeper or not
361*/
362function updateKeeper(address _keeper, bool _approval) external onlyOwner {
363  keepers[_keeper] = _approval;
364}
365
366/**
367  * Update max capacity value
368  * @param _capacityValue Capacity value in 1e18
369*/
370function updateCapacity(uint256 _capacityValue) external onlyOwner {
371  capacityValue = _capacityValue;
372}

Recommendation:

We advise the functions to be invoke-able by a decentralized entity, preferably sitting behind a timelock as the present implementation permits sensitive variables such as mgmtFeePerSecond to be adjusted at will (GMXPerpetualDEXLongVault::updateMgmtFeePerSecond).

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The Steadefi team has stated that they will implement a path to decentralization for all the deployed contracts within their ecosystem by utilizing a combination of a governance mechanism and a Timelock contract. As such, we consider this exhibit alleviated based on the fact that the Steadefi team will implement a decentralization path in the near future.

GMX-02M: Improper Tokenomic Share System

Type	Severity	Location
Logical Fault		GMXPerpetualDEXLongVault.sol:L254, L256

Description:

The share system of GMXPerpetualDEXLongVault is incorrect as the shares it mints to a deposit are proportionate to the equity the user mints. This causes the system to provide an abnormal amount of shares for small deposits when the system's targetLeverage has been adjusted.

Impact:

After multiple discussions with the Steadefi team and extensive test suites and documentation on their end, we concluded that the impact of this exhibit is inexistent as the funds that can ultimately be withdrawn from the vaules remain the same.

In any case, the Steadefi team remediated this issue by ensuring that an adjut

Example:

Copy
254uint256 _equityChange = (reader.assetValueWithPrice(glpPrice) - reader.debtValue()) - equityBefore;
255// calculate shares to users
256uint256 sharesToUser = valueToShares(_equityChange, equityBefore);
257
258_mint(msg.sender, sharesToUser);

Recommendation:

We advise adjustments of the targetLeverage to be accompanied by a rebalance operation in the same call, ensuring that the equity and debt of the vault at any given point in time reflects the desired targetLeverage.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The Steadefi team adjusted the updateVaultConfig function to accept multiple new arguments and perform a rebalance operation after the configuration has been updated, alleviating this exhibit in full.

GMX-03M: Inexplicable Presence of Variable

Type	Severity	Location
Logical Fault		GMXPerpetualDEXLongVault.sol:L143, L164, L174, L189, L206, L302

Description:

The _isNative variable remains unutilized throughout the codebase (i.e. in GMXPerpetualDEXLongVault::_withdraw) yet it remains in the GMXPerpetualDEXLongVault::deposit, GMXPerpetualDEXLongVault::withdraw, and GMXPerpetualDEXLongVault::emergencyWithdraw functions.

Impact:

Presently, the functions referenced mislead its users as they may expect to receive native funds when it is impossible to do so. If interface conformity is desirable, we advise interface implementations that are directly overridden to be declared within the codebase instead.

Example:

Copy
294/**
295  * Follow-on function from _withdraw(), called after manager does work(); withdraws balance of tokenA/B to user
296  * @param _token Token address
297  * @param _withdrawAmt Withdraw amt in 1e18
298*/
299function _withdraw(
300  address _token,
301  uint256 _withdrawAmt,
302  bool /*_isNative*/
303) internal {
304  IERC20(_token).safeTransfer(msg.sender, _withdrawAmt);
305}

Recommendation:

We advise it to be omitted from the codebase safely or adequately supported by its internal functions, either of which we consider an adequate remediation to this exhibit.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The _isNative variable has been removed throughout the contract as advised, alleviating this exhibit and increasing the legibility of the codebase.

GMX-04M: Incorrect payable Function Attribute

Type	Severity	Location
Language Specific		GMXPerpetualDEXLongVault.sol:L143

Description:

The GMXPerpetualDEXLongVault::deposit function is incorrectly marked as payable when it does not handle native funds.

Impact:

As the payable attribute permits native funds to be sent to the contract within a GMXPerpetualDEXLongVault::deposit transaction, users will lose any native funds they send within such a transaction.

Example:

Copy
139/**
140  * Deposits asset into vault and mint svToken to user
141  * @param _amt Amount of asset tokens to deposit in token decimals
142 */
143function deposit(uint256 _amt, bool /*_isNative*/) external payable nonReentrant whenNotPaused {
144  _mintMgmtFee();
145
146  token.safeTransferFrom(msg.sender, address(this), _amt);
147  token.safeTransfer(address(manager), _amt);
148
149  uint256 sharesToUser = _deposit(_amt, reader.tokenValue(address(token), _amt));
150
151  emit Deposit(
152    msg.sender,
153    address(token),
154    _amt,
155    sharesToUser
156  );
157}

Recommendation:

We advise the payable attribute to be omitted as it presently causes funds to be permanently locked in the contract when transferred alongside a GMXPerpetualDEXLongVault::deposit call.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The payable attribute has been properly removed from the GMXPerpetualDEXLongVault::deposit function, preventing native funds from ever being locked within the contract by the function's misuse.

GMX-05M: Inexistent Protection Against Paused Invocation

Type	Severity	Location
Logical Fault		GMXPerpetualDEXLongVault.sol:L381

Description:

The GMXPerpetualDEXLongVault::rebalance function can be invoked when the contract is paused, minting a management fee incorrectly.

Impact:

A management fee collected during the system's emergency state is undesirable and can be harmful to existing users of the contract who have not withdrawn their shares yet via the GMXPerpetualDEXLongVault::emergencyWithdraw function.

Example:

Copy
381function rebalance(
382  ManagerAction _action,
383  uint256 _lpAmt,
384  uint256 _borrowTokenAmt,
385  uint256 _repayTokenAAmt
386) external onlyKeeper {
387  _mintMgmtFee();
388
389  uint256 svTokenValueBefore = svTokenValue();
390
391  manager.work(
392    _action,
393    _lpAmt,
394    _borrowTokenAmt,
395    _repayTokenAAmt
396  );
397
398  emit Rebalance(svTokenValueBefore, svTokenValue());
399}

Recommendation:

We advise the function to be guarded by the whenNotPaused modifier, ensuring that a management fee is collected solely when the contract is in operation.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The GMXPerpetualDEXLongVault::rebalance function properly prevents invocations when the protocol is paused via the whenNotPaused modifier, alleviating this exhibit.

GMX-06M: Inexistent Slippage Protections

Type	Severity	Location
Logical Fault		GMXPerpetualDEXLongVault.sol:L149, L172

Description:

The GMXPerpetualDEXLongVault::deposit and GMXPerpetualDEXLongVault::withdraw functions do not apply any form of slippage protection, rendering all depositors and withdrawers prone to slippage related attacks (such as sandwich attacks).

Impact:

The current mechanisms of the contract render the users prone to arbitrage attacks, causing them to receive less shares and less funds than they expect from their deposits and withdrawals. This is especially prevalent in GMXPerpetualDEXLongVault whereby the total supply of the vault is continuously increased via GMXPerpetualDEXLongVault::_mintMgmtFee.

Example:

Copy
139/**
140  * Deposits asset into vault and mint svToken to user
141  * @param _amt Amount of asset tokens to deposit in token decimals
142 */
143function deposit(uint256 _amt, bool /*_isNative*/) external payable nonReentrant whenNotPaused {
144  _mintMgmtFee();
145
146  token.safeTransferFrom(msg.sender, address(this), _amt);
147  token.safeTransfer(address(manager), _amt);
148
149  uint256 sharesToUser = _deposit(_amt, reader.tokenValue(address(token), _amt));
150
151  emit Deposit(
152    msg.sender,
153    address(token),
154    _amt,
155    sharesToUser
156  );
157}

Recommendation:

We advise both functions to introduce a new variable representing the minimum sharesToUser and minimum withdrawAmt respectively that they wish to accept as a result of their GMXPerpetualDEXLongVault::deposit / GMXPerpetualDEXLongVault::withdraw transaction.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

Both functions have had a minimum share / withdrawal amount argument introduced, ensuring that the user is able to control the slippage they render themselves prone to.

GMX-07M: Precision Loss of Convoluted Calculations

Type	Severity	Location
Mathematical Operations		GMXPerpetualDEXLongVault.sol:L241-L245

Description:

The referenced calculations are redundantly convoluted as the _depositValue of GMXPerpetualDEXLongVault::_deposit is equal to GMXPerpetualDEXLongReader::tokenValue with the _amt as an input. As such, the borrowTokenAmt can be calculated by multiplying _amt by vaultConfig.targetLeverage - (1 * SAFE_MULTIPLIER) and dividing by SAFE_MULTIPLIER, rendering all other calculations redundant. Additionally, the current vaultConfig.targetLeverage is utilized in calculations within parenthesis (()) that cause it to incur significant precision loss.

Impact:

The current calculations cause a significant precision loss that will affect how the vaultConfig.targetLeverage is set in effect within the GMXPerpetualDEXLongVault contract.

Example:

Copy
234function _deposit(uint256 _amt, uint256 _depositValue) internal returns (uint256) {
235  uint256 glpPrice = reader.glpPrice();
236  uint256 equityBefore = reader.assetValueWithPrice(glpPrice) - reader.debtValue();
237
238  require(_amt > 0, "Amt must be > 0");
239  require(equityBefore + _depositValue <= capacityValue, "Exceeded capacity");
240
241   uint256 borrowTokenAmt = _depositValue
242    * ((vaultConfig.targetLeverage - (1 * SAFE_MULTIPLIER)) / SAFE_MULTIPLIER )
243    * 1 * SAFE_MULTIPLIER
244    / priceOracle.consult(address(token))
245    / reader.to18ConversionFactor(address(token));

Recommendation:

We advise the calculation to increase in legibility and be made optimal according to the simplifications outlined in the description of this exhibit.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The borrowTokenAmt value is optimally calculated in the latest iteration of the codebase, achieving the maximum precision possible and significantly increasing its legibility.