Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Inconsistent Execution Fee in PerpetualVault Deposit Function

mahdi811

Summary

In the PerpetualVault.sol contract, the deposit function charges an ExecutionFee only when the vault has an active position (positionIsClosed == false). When the vault has no active position (positionIsClosed == true), users are not required to pay any ExecutionFee. This behavior leads to an unfair distribution of costs among users, depending on the state of the vault at the time of deposit, which can create a sense of unfairness or impact user trust.

Vulnerability Details

  • Location: PerpetualVault.sol FIle, deposit function

  • Relevant Code:

    if (positionIsClosed) {
    MarketPrices memory prices;
    _mint(counter, amount, false, prices);
    _finalize(hex'');
    } else {
    _payExecutionFee(counter, true);
    nextAction.selector = NextActionSelector.INCREASE_ACTION;
    nextAction.data = abi.encode(beenLong);
    }

Explanation: When positionIsClosed == true, the deposit is immediately processed by calling _mint and _finalize, and no call to _payExecutionFee is made. However, when positionIsClosed == false, the user must call _payExecutionFee, which transfers the ExecutionFee to the GmxProxy. This fee is used to compensate Keepers for increasing the GMX position.

Reason: The difference lies in the complexity of operations. The no-position state does not require asynchronous interaction with GMX, while the active-position state requires Keepers and order execution on GMX.

Impact

  • Financial Impact: Users depositing during an active position incur higher costs (transaction gas + ExecutionFee) compared to users depositing when the position is closed (only transaction gas). This difference can range from a few dollars to a significant amount when tx.gasprice is high.

  • User Experience: The inequality in costs may reduce user trust or cause dissatisfaction, especially if users are unaware of this behavior.

Tools Used

  • Manual Analysis: Review of the source code of PerpetualVault.sol and the logic of the deposit function.

  • Foundry: For writing the Proof of Concept and simulating contract behavior.

Poc

// SPDX-License-Identifier: GPL-3.0-only
pragma solidity ^0.8.4;
​
import "forge-std/Test.sol";
import "../src/PerpetualVault.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract MockCollateralToken is ERC20 {
constructor() ERC20("Mock USDC", "USDC") {
_mint(msg.sender, 1_000_000 * 10**6); // 1M USDC with 6 decimals
}
}
​
contract PerpetualVaultTest is Test {
PerpetualVault vault;
MockCollateralToken usdc;
address userA = address(0xA);
address userB = address(0xB);
address mockGmxProxy = address(0xC);
address mockVaultReader = address(0xD);
​
function setUp() public {
usdc = new MockCollateralToken();
vault = new PerpetualVault();
vault.initialize(
address(0x1), // mock market
address(this), // keeper
address(this), // treasury
mockGmxProxy,
mockVaultReader,
1000 * 10**6, // minDepositAmount = 1000 USDC
100_000 * 10**6, // maxDepositAmount = 100,000 USDC
20000 // leverage = 2x (BASIS_POINTS_DIVISOR = 10000)
);
​
// Setup users with USDC
vm.deal(userA, 1 ether);
vm.deal(userB, 1 ether);
usdc.transfer(userA, 5000 * 10**6); // 5000 USDC
usdc.transfer(userB, 5000 * 10**6); // 5000 USDC
}
​
function testDepositNoExecutionFeeWhenPositionClosed() public {
vm.startPrank(userA);
usdc.approve(address(vault), 2000 * 10**6);
vault.deposit{value: 0}(2000 * 10**6); // No ETH sent
vm.stopPrank();
​
// Check shares minted without ExecutionFee
(, uint256 sharesA,,,) = vault.depositInfo(1);
assertEq(sharesA, 2000 * 10**8); // Initial shares = amount * 1e8
assertEq(address(vault).balance, 0); // No ETH received
}
​
function testDepositWithExecutionFeeWhenPositionOpen() public {
// Simulate an open position
vm.prank(address(this));
vault.setVaultState(FLOW.NONE, 0, true, keccak256("mockPosition"), false, false, vault.nextAction());
​
vm.startPrank(userB);
usdc.approve(address(vault), 2000 * 10**6);
uint256 minFee = vault.getExecutionGasLimit(true) * tx.gasprice; // Approx ExecutionFee
vault.deposit{value: minFee}(2000 * 10**6); // Send ETH for fee
vm.stopPrank();
​
// Check ETH transferred to GmxProxy
assertEq(address(mockGmxProxy).balance, minFee);
(, uint256 sharesB,,,) = vault.depositInfo(1);
assertEq(sharesB, 0); // Shares not minted yet, deferred to next action
}
}

PoC Explanation:

  • Test 1: User A deposits when positionIsClosed == true and does not send any ETH (as ExecutionFee). Shares are immediately allocated.

  • Test 2: User B deposits when positionIsClosed == false and must pay an ExecutionFee (ETH). Shares are not yet allocated and deferred to INCREASE_ACTION.

  • Result: User A incurs lower costs compared to User B.

Recommendations

  1. Charge a Fixed Fee: Always charge an ExecutionFee from users, regardless of positionIsClosed. In the closed state, this fee can be transferred to the treasury:

_payExecutionFee(counter, true);
if (positionIsClosed) {
MarketPrices memory prices;
_mint(counter, amount, false, prices);
_finalize(hex'');
} else {
nextAction.selector = NextActionSelector.INCREASE_ACTION;
nextAction.data = abi.encode(beenLong);
}

  1. Transparency in Documentation: Clearly explain in the documentation or UI that ExecutionFee is only charged during an open position, and the reason is the cost of Keepers and GMX.

  2. Distribute Costs from Profits: Use vault profits (e.g., positive funding fees) to cover Keeper costs, reducing the need for ExecutionFee from new users.

  3. Optional Minimum Fee: Allow users to optionally pay a fee during positionIsClosed == true, which can be used as a reserve for future operations.

These changes can enhance fairness and improve the user experience while maintaining the technical logic of the contract.

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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