isLong
when no open positions exist.The function getPositionInfo() in VaultReader.sol contract by default sets isLong
to true
when there no open position with associated key (sizeInTokens = 0);
This value not reflects the actual status of position, potentially leading to data inconsistencies in UI representations and downstream applications.
In the getPositionInfo
function, when sizeInTokens
== 0, the function returns a PositionData
struct with default values:
The isLong
flag is hardcoded to true without any consideration of the position's actual direction.
Also these functions: getPositionInfo()
, getNegativeFundingFeeAmount()
, getPnl()
have incorrect naming of variable which handle returned value from getPositionSizeInUsd()
:
It is essential to use sizeInUsd
. Although it does not involve any calculations, using sizeInUsd
is the preferred approach.
-UI/UX confusion for users viewing closed position details;
-Potential issues in integrated protocols that rely on function getPositionInfo()
;
Manual code review
There are several possible solutions to improve this behavior:
Remove the hardcoded True
and set's isLong
to false
:
Add a new field bool isActive
to the new structure to explicitly indicate that the position is inactive:
The function willPositionCollateralBeInsufficient()
in VaultReader
incorrectly returns true
(indicating insufficient collateral) when called for positions that no longer exist (sizeInUsd
== 0). This creates misleading signals for users/offchain keeper attempting to add more collateral with closed positions. Even if keeper has other health check logic for open positions, users/their contracts still can receives incorrect result.
The function unconditionally returns true
if the position has sizeInUsd
== 0, regardless of the actual collateral state. This is fundamentally incorrect because closed positions (size = 0) cannot have insufficient collateral by definition (collateral is removed when positions close).
Also these functions: getPositionInfo()
, getNegativeFundingFeeAmount()
, getPnl()
have incorrect naming of variable which handle returned value from getPositionSizeInUsd()
:
It is essential to use sizeInUsd
. Although it does not involve any calculations, using sizeInUsd
is the preferred approach.
Users/contracts calling this function for closed positions (e.g., to check if they can re-open a position) will receive a false "insufficient collateral" signal. This could lead to unnecessary actions like depositing extra collateral or abandoning valid operations depending of how Gamma offchain script reacts.
Manual code analysis
Also another approach is to return a structure that explicitly indicates whether the position is closed.
False
A critical inconsistency has been identified in the VaultReader
contract where the getPositionInfo()
function consistently returns false for the isLong
parameter, even when the position is actually long. This contradicts the data returned by direct calls to GMXReader
with identical input parameters, which correctly returns true
for long positions.
The VaultReader.getPositionInfo() function incorrectly reports position sides (long/short) despite using the same underlying GMXReader contract and dataStore:
Foundry test:
//just add to PerpetualVault.t.sol:
But direct calls to GMXReader.getPositionInfo() with identical parameters correctly return true
for long positions.
Misleading position information for users and integrating protocols as well gamma off-chain script.
Manual review
Consider to add direct validation of position side: bool isLongPosition = Position.isLong(dataStore, key);
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.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.