Submission Details
Severity:
Entry Price Always Changes When Adding or Reducing Trade Size
Summary
The entry price is always recalculated when adding or reducing trade size. This behavior can lead to inaccurate profit and loss calculations, resulting in unfair financial outcomes for traders.
Vulnerability Details
The entry price is very important because it is a reference point for when the trader started the trade and it is used for calculating the profit or loss for both long and short positions. In the _fillOrder, there are only 2 options, either the position is cleared when a trader is closing the trader or the position is updated, this update changes the entry price.
The price been updated is the current price not the entry price of the trader.
function fillMarketOrder(
uint128 tradingAccountId,
uint128 marketId,
bytes calldata priceData
)
external
onlyMarketOrderKeeper(marketId)
{
// working data
FillMarketOrder_Context memory ctx;
// fetch storage slot for perp market's market order config
SettlementConfiguration.Data storage settlementConfiguration =
SettlementConfiguration.load(marketId, SettlementConfiguration.MARKET_ORDER_CONFIGURATION_ID);
// load trader's pending order; reverts if no pending order
MarketOrder.Data storage marketOrder = MarketOrder.loadExisting(tradingAccountId);
// enforce that keeper is filling the order for the correct marketId
if (marketId != marketOrder.marketId) {
revert Errors.OrderMarketIdMismatch(marketId, marketOrder.marketId);
}
// fetch storage slot for perp market
PerpMarket.Data storage perpMarket = PerpMarket.load(marketId);
// fetch storage slot for global config
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
// verifies provided price data following the configured settlement strategy
// returning the bid and ask prices
(ctx.bidX18, ctx.askX18) =
settlementConfiguration.verifyOffchainPrice(priceData, globalConfiguration.maxVerificationDelay);
// cache the order's size delta
ctx.sizeDeltaX18 = sd59x18(marketOrder.sizeDelta);
// cache the order side
ctx.isBuyOrder = ctx.sizeDeltaX18.gt(SD59x18_ZERO);
// buy order -> match against the ask price
// sell order -> match against the bid price
ctx.indexPriceX18 = ctx.isBuyOrder ? ctx.askX18 : ctx.bidX18;
// verify the provided price data against the verifier and ensure it's valid, then get the mark price
// based on the returned index price.
@> ctx.fillPriceX18 = perpMarket.getMarkPrice(ctx.sizeDeltaX18, ctx.indexPriceX18);
// perform the fill
_fillOrder(
tradingAccountId,
marketId,
SettlementConfiguration.MARKET_ORDER_CONFIGURATION_ID,
ctx.sizeDeltaX18,
@> ctx.fillPriceX18
);
// reset pending order details
marketOrder.clear();
}
function _fillOrder(
uint128 tradingAccountId,
uint128 marketId,
uint128 settlementConfigurationId,
SD59x18 sizeDeltaX18,
UD60x18 fillPriceX18
)
internal
virtual
{
// working data
FillOrderContext memory ctx;
// fetch storage slot for global config
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
// fetch storage slot for perp market's settlement config
SettlementConfiguration.Data storage settlementConfiguration =
SettlementConfiguration.load(marketId, settlementConfigurationId);
// cache settlement token
ctx.usdToken = globalConfiguration.usdToken;
// determine whether position is being increased or not
ctx.isIncreasing = Position.isIncreasing(tradingAccountId, marketId, sizeDeltaX18.intoInt256().toInt128());
// both markets and settlement can be disabled, however when this happens we want to:
// 1) allow open positions not subject to liquidation to decrease their size or close
// 2) prevent new positions from being opened & existing positions being increased
//
// the idea is to prevent a state where traders have open positions but are unable
// to reduce size or close even though they can still be liquidated; such a state
// would severly disadvantage traders
if (ctx.isIncreasing) {
// both checks revert if disabled
globalConfiguration.checkMarketIsEnabled(marketId);
settlementConfiguration.checkIsSettlementEnabled();
}
// load existing trading account; reverts for non-existent account
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
// fetch storage slot for perp market
PerpMarket.Data storage perpMarket = PerpMarket.load(marketId);
// enforce minimum trade size
perpMarket.checkTradeSize(sizeDeltaX18);
// get funding rates for this perp market
ctx.fundingRateX18 = perpMarket.getCurrentFundingRate();
ctx.fundingFeePerUnitX18 = perpMarket.getNextFundingFeePerUnit(ctx.fundingRateX18, fillPriceX18);
// update funding rates
perpMarket.updateFunding(ctx.fundingRateX18, ctx.fundingFeePerUnitX18);
// calculate order & settlement fees
ctx.orderFeeUsdX18 = perpMarket.getOrderFeeUsd(sizeDeltaX18, fillPriceX18);
ctx.settlementFeeUsdX18 = ud60x18(uint256(settlementConfiguration.fee));
// fetch storage slot for account's potential existing position in this market
Position.Data storage oldPosition = Position.load(tradingAccountId, marketId);
// int256 -> SD59x18
ctx.oldPositionSizeX18 = sd59x18(oldPosition.size);
{
// calculate required initial & maintenance margin for this trade
// and account's unrealized PNL
(
UD60x18 requiredInitialMarginUsdX18,
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 accountTotalUnrealizedPnlUsdX18
) = tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, sizeDeltaX18);
// check maintenance margin if:
// 1) position is not increasing AND
// 2) existing position is being decreased in size
//
// when a position is under the higher initial margin requirement but over the
// lower maintenance margin requirement, we want to allow the trader to decrease
// their losing position size before they become subject to liquidation
//
// but if the trader is opening a new position or increasing the size
// of their existing position we want to ensure they satisfy the higher
// initial margin requirement
ctx.shouldUseMaintenanceMargin = !ctx.isIncreasing && !ctx.oldPositionSizeX18.isZero();
ctx.requiredMarginUsdX18 =
ctx.shouldUseMaintenanceMargin ? requiredMaintenanceMarginUsdX18 : requiredInitialMarginUsdX18;
// reverts if the trader can't satisfy the appropriate margin requirement
tradingAccount.validateMarginRequirement(
ctx.requiredMarginUsdX18,
tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18),
ctx.orderFeeUsdX18.add(ctx.settlementFeeUsdX18)
);
}
// cache unrealized PNL from potential existing position in this market
// including accrued funding
ctx.pnlUsdX18 =
oldPosition.getUnrealizedPnl(fillPriceX18).add(oldPosition.getAccruedFunding(ctx.fundingFeePerUnitX18));
// create new position in working area
ctx.newPosition = Position.Data({
size: ctx.oldPositionSizeX18.add(sizeDeltaX18).intoInt256(),
lastInteractionPrice: fillPriceX18.intoUint128(),
lastInteractionFundingFeePerUnit: ctx.fundingFeePerUnitX18.intoInt256().toInt128()
});
// int256 -> SD59x18
ctx.newPositionSizeX18 = sd59x18(ctx.newPosition.size);
// enforce open interest and skew limits for target market and calculate
// new open interest and new skew
(ctx.newOpenInterestX18, ctx.newSkewX18) =
perpMarket.checkOpenInterestLimits(sizeDeltaX18, ctx.oldPositionSizeX18, ctx.newPositionSizeX18);
// update open interest and skew for this perp market
perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);
// update active markets for this account; may also trigger update
// to global config set of active accounts
tradingAccount.updateActiveMarkets(marketId, ctx.oldPositionSizeX18, ctx.newPositionSizeX18);
// if the position is being closed, clear old position data
if (ctx.newPositionSizeX18.isZero()) {
oldPosition.clear();
}
// otherwise we are opening a new position or modifying an existing position
else {
// revert if new position size is under the minimum for this market
if (ctx.newPositionSizeX18.abs().lt(sd59x18(int256(uint256(perpMarket.configuration.minTradeSizeX18))))) {
revert Errors.NewPositionSizeTooSmall();
}
// update trader's existing position in this market with new position data
@> oldPosition.update(ctx.newPosition);
}
// if trader's old position had positive pnl then credit that to the trader
if (ctx.pnlUsdX18.gt(SD59x18_ZERO)) {
ctx.marginToAddX18 = ctx.pnlUsdX18.intoUD60x18();
tradingAccount.deposit(ctx.usdToken, ctx.marginToAddX18);
// mint settlement tokens credited to trader; tokens are minted to
// address(this) since they have been credited to trader's deposited collateral
//
// NOTE: testnet only - this call will be updated once the Market Making Engine is finalized
LimitedMintingERC20(ctx.usdToken).mint(address(this), ctx.marginToAddX18.intoUint256());
}
// pay order/settlement fees and deduct collateral
// if trader's old position had negative pnl
tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: globalConfiguration.orderFeeRecipient,
settlementFeeRecipient: globalConfiguration.settlementFeeRecipient
}),
pnlUsdX18: ctx.pnlUsdX18.lt(SD59x18_ZERO) ? ctx.pnlUsdX18.abs().intoUD60x18() : UD60x18_ZERO,
orderFeeUsdX18: ctx.orderFeeUsdX18,
settlementFeeUsdX18: ctx.settlementFeeUsdX18
});
emit LogFillOrder(
msg.sender,
tradingAccountId,
marketId,
sizeDeltaX18.intoInt256(),
fillPriceX18.intoUint256(),
ctx.orderFeeUsdX18.intoUint256(),
ctx.settlementFeeUsdX18.intoUint256(),
ctx.pnlUsdX18.intoInt256(),
ctx.fundingFeePerUnitX18.intoInt256()
);
}
function update(Data storage self, Data memory newPosition) internal {
self.size = newPosition.size;
@> self.lastInteractionPrice = newPosition.lastInteractionPrice;
self.lastInteractionFundingFeePerUnit = newPosition.lastInteractionFundingFeePerUnit;
}
Impact
Inaccurate Profit/Loss Calculations: Traders may experience incorrect profit and loss calculations
Tools Used
Manual Review
Recommendations
Maintain Original Entry Price: Update the function to maintain the original entry price when adjusting the trade size.
Prize pool breakdown
Total prize
60,000 USDC
nSLOC
2,87821 USDC / LOC
50,000 USDC
5,500 USDC
4,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.