diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index a61e1277ab2..81b930dd8c9 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** `OrderFill.pnl` is optional when the venue omits realized PnL. Consumers must preserve missing amounts as unknown when aggregating or displaying fills; only a reported `'0'` is zero ([#10605](https://github.com/MetaMask/core/pull/10605)) + +### Fixed + +- Accept Lighter trades that omit position-sign flags, preserve omitted account PnL as unknown, and reject known reductions without realized PnL. Retain side-only fill directions when lifecycle context is ambiguous ([#10605](https://github.com/MetaMask/core/pull/10605)) + ## [19.0.0] ### Added diff --git a/packages/perps-controller/src/services/LighterClientService.ts b/packages/perps-controller/src/services/LighterClientService.ts index 888061db332..fd0734a1143 100644 --- a/packages/perps-controller/src/services/LighterClientService.ts +++ b/packages/perps-controller/src/services/LighterClientService.ts @@ -275,8 +275,8 @@ const TradeStruct = type({ makerFee: optional(NonNegativeFinancialNumberStruct), takerPositionSizeBefore: NonNegativeDecimalStringStruct, makerPositionSizeBefore: NonNegativeDecimalStringStruct, - takerPositionSignChanged: boolean(), - makerPositionSignChanged: boolean(), + takerPositionSignChanged: optional(boolean()), + makerPositionSignChanged: optional(boolean()), }); const ResponseStructs = { diff --git a/packages/perps-controller/src/types/index.ts b/packages/perps-controller/src/types/index.ts index c36124e89e7..38d1bed141c 100644 --- a/packages/perps-controller/src/types/index.ts +++ b/packages/perps-controller/src/types/index.ts @@ -1234,7 +1234,8 @@ export type OrderFill = { side: string; // Normalized order side ('buy' or 'sell') size: string; // Fill size price: string; // Fill price - pnl: string; // PNL + /** Realized PnL reported by the venue; absent means unknown, never zero. */ + pnl?: string; direction: string; // Direction of the fill fee: string; // Fee paid feeToken: string; // Fee token symbol diff --git a/packages/perps-controller/src/types/lighter-types.ts b/packages/perps-controller/src/types/lighter-types.ts index 1d4cf982cc9..9f85def2997 100644 --- a/packages/perps-controller/src/types/lighter-types.ts +++ b/packages/perps-controller/src/types/lighter-types.ts @@ -618,7 +618,7 @@ export type LighterWsTrade = { bidAccountId: number; isMakerAsk: boolean; timestamp: number; - /** Realized pnl per side — same wire shape as the REST trade payload. */ + /** Realized pnl per side; may be omitted on opens or adds, as in REST trades. */ askAccountPnl?: string; bidAccountPnl?: string; /** Fees, present when nonzero; unit unproven — see LighterRestTrade. */ @@ -626,6 +626,7 @@ export type LighterWsTrade = { makerFee?: number | string; takerPositionSizeBefore?: string; makerPositionSizeBefore?: string; + /** Whether the side's position sign changed; omission is unknown. */ takerPositionSignChanged?: boolean; makerPositionSignChanged?: boolean; }; @@ -657,9 +658,9 @@ export type LighterRestTrade = { bidAccountId: number; isMakerAsk: boolean; timestamp: number; - /** Realized pnl for the ask-side account, signed USDC; may be omitted on opens from flat. */ + /** Realized pnl for the ask-side account, signed USDC; may be omitted on opens or adds. */ askAccountPnl?: string; - /** Realized pnl for the bid-side account, signed USDC; may be omitted on opens from flat. */ + /** Realized pnl for the bid-side account, signed USDC; may be omitted on opens or adds. */ bidAccountPnl?: string; /** * Taker/maker fees, present when nonzero. The official model types them @@ -671,9 +672,9 @@ export type LighterRestTrade = { /** Position size (absolute) of each side before the trade executed. */ takerPositionSizeBefore: string; makerPositionSizeBefore: string; - /** Whether the side's position sign changed (crossed or left zero). */ - takerPositionSignChanged: boolean; - makerPositionSignChanged: boolean; + /** Whether the side's position sign changed (crossed or left zero); omission is unknown. */ + takerPositionSignChanged?: boolean; + makerPositionSignChanged?: boolean; }; /** diff --git a/packages/perps-controller/src/utils/lighterAdapter.ts b/packages/perps-controller/src/utils/lighterAdapter.ts index c139a4669ce..fa2475c1cb9 100644 --- a/packages/perps-controller/src/utils/lighterAdapter.ts +++ b/packages/perps-controller/src/utils/lighterAdapter.ts @@ -277,7 +277,7 @@ export function adaptAccountStateFromLighterUserStats( * @param context.size - Trade size (base units, absolute). * @param context.positionBefore - Our side's absolute position size before. * @param context.signChanged - Whether our side's position sign changed. - * @param context.pnl - Realized pnl for our side. + * @param context.pnl - Realized pnl for our side, or undefined when omitted. * @returns Client-facing direction string. */ export function deriveLighterFillDirection(context: { @@ -285,16 +285,24 @@ export function deriveLighterFillDirection(context: { size: number; positionBefore: number; signChanged: boolean | undefined; - pnl: number; + pnl: number | undefined; }): string { const { isBuy, size, positionBefore, signChanged, pnl } = context; - if (!Number.isFinite(positionBefore) || signChanged === undefined) { + if (!Number.isFinite(positionBefore)) { return isBuy ? 'Buy' : 'Sell'; } if (positionBefore === 0) { return isBuy ? 'Open Long' : 'Open Short'; } - if (signChanged) { + // A flat baseline proves an open. On an existing position, nonzero + // realized PnL proves reduction; otherwise an omitted flag is ambiguous. + if (signChanged === undefined && (pnl === undefined || pnl === 0)) { + return isBuy ? 'Buy' : 'Sell'; + } + if ( + signChanged || + (signChanged === undefined && pnl !== undefined && pnl !== 0) + ) { // Crossed past zero → flipped; landed exactly on zero → full close. if (size > positionBefore * 1.000001) { return isBuy ? 'Short > Long' : 'Long > Short'; @@ -302,10 +310,10 @@ export function deriveLighterFillDirection(context: { return isBuy ? 'Close Short' : 'Close Long'; } // Partial fill on an existing position: realized pnl proves it reduced. - if (pnl !== 0) { + if (pnl !== undefined && pnl !== 0) { return isBuy ? 'Close Short' : 'Close Long'; } - // Zero-pnl partial with no sign change is genuinely ambiguous from this + // Zero or unknown pnl with no sign change is ambiguous from this // payload (a break-even partial close and an add both fit): fall back to // the side-only vocabulary instead of asserting Open without evidence. return isBuy ? 'Buy' : 'Sell'; @@ -412,8 +420,8 @@ export function adaptFillFromLighterTrade( const fee = '0'; const isBuy = !accountIsAsk; // The account's maker/taker role selects the matching pre-trade position - // context. Missing context is rejected above instead of becoming neutral - // history that looks authoritative. + // context. Omitted sign-change flags retain an unknown direction for + // existing positions; malformed supplied context still fails closed. const positionBeforeRaw = accountIsMaker ? trade.makerPositionSizeBefore : trade.takerPositionSizeBefore; @@ -425,22 +433,22 @@ export function adaptFillFromLighterTrade( positionBefore === null || !Number.isFinite(positionBefore) || positionBefore < 0 || - typeof signChanged !== 'boolean' + (signChanged !== undefined && typeof signChanged !== 'boolean') ) { throw new Error( `${LIGHTER_DATA_INTEGRITY_PREFIX} trade ${trade.tradeId} is missing valid position context`, ); } - const accountPnl = accountIsAsk ? trade.askAccountPnl : trade.bidAccountPnl; - // The venue omits PnL on opens from flat. Only a validated zero pre-trade - // position proves zero realized PnL; never assume it for an existing position. - const pnl = - accountPnl === undefined && positionBefore === 0 ? '0' : accountPnl; - const parsedPnl = parseLighterStrictDecimal(pnl); + const pnl = accountIsAsk ? trade.askAccountPnl : trade.bidAccountPnl; + // Omitted amounts remain unknown. A changed sign on an existing position + // proves reduction, so its realized PnL must be supplied by the venue. + const parsedPnl = + pnl === undefined ? undefined : parseLighterStrictDecimal(pnl); if ( - typeof pnl !== 'string' || parsedPnl === null || - !Number.isFinite(parsedPnl) + (pnl === undefined && positionBefore !== 0 && signChanged === true) || + (pnl !== undefined && + (typeof pnl !== 'string' || !Number.isFinite(parsedPnl))) ) { throw new Error( `${LIGHTER_DATA_INTEGRITY_PREFIX} trade ${trade.tradeId} is missing valid account pnl`, @@ -474,7 +482,7 @@ export function adaptFillFromLighterTrade( size: trade.size, price: trade.price, // The venue reports realized pnl per side of the trade. - pnl, + ...(pnl === undefined ? {} : { pnl }), direction, ...(startPosition === undefined ? {} : { startPosition }), fee, diff --git a/packages/perps-controller/tests/src/services/LighterClientService.test.ts b/packages/perps-controller/tests/src/services/LighterClientService.test.ts index 461d5af9f44..9a10d620dc1 100644 --- a/packages/perps-controller/tests/src/services/LighterClientService.test.ts +++ b/packages/perps-controller/tests/src/services/LighterClientService.test.ts @@ -346,6 +346,49 @@ describe('LighterClientService', () => { }); describe('getTrades', () => { + it.each([ + { taker_position_sign_changed: undefined }, + { maker_position_sign_changed: undefined }, + { + taker_position_sign_changed: undefined, + maker_position_sign_changed: undefined, + ask_account_pnl: undefined, + bid_account_pnl: undefined, + }, + ])('accepts omitted optional trade fields %j', async (omitted) => { + fetchMock.mockResolvedValue( + mockJsonResponse({ + code: 200, + trades: [{ ...VALID_TRADE_WIRE, ...omitted }], + }), + ); + + const response = await buildService().getTrades(28, 'auth-token', { + limit: 50, + }); + + expect(response.trades).toHaveLength(1); + expect(response.trades[0].tradeId).toBe(VALID_TRADE_WIRE.trade_id); + }); + + it.each([null, 'false', 0])( + 'rejects malformed supplied maker sign flag %s', + async (flag) => { + fetchMock.mockResolvedValue( + mockJsonResponse({ + code: 200, + trades: [ + { ...VALID_TRADE_WIRE, maker_position_sign_changed: flag }, + ], + }), + ); + + await expect( + buildService().getTrades(28, 'auth-token', { limit: 50 }), + ).rejects.toThrow('Invalid Lighter venue data'); + }, + ); + it('encodes cursor, from, and market filters', async () => { fetchMock.mockResolvedValue( mockJsonResponse({ code: 200, next_cursor: 'next', trades: [] }), @@ -399,8 +442,8 @@ describe('LighterClientService', () => { ['negative position magnitude', { taker_position_size_before: '-0.1' }], ['missing maker role', { is_maker_ask: undefined }], [ - 'missing position sign context', - { taker_position_sign_changed: undefined }, + 'malformed position sign context', + { taker_position_sign_changed: 'false' }, ], ])('rejects %s in a trade payload', async (_label, override) => { fetchMock.mockResolvedValue( diff --git a/packages/perps-controller/tests/src/utils/lighterAdapter.test.ts b/packages/perps-controller/tests/src/utils/lighterAdapter.test.ts index c4cf5ce6b36..9fd837940e0 100644 --- a/packages/perps-controller/tests/src/utils/lighterAdapter.test.ts +++ b/packages/perps-controller/tests/src/utils/lighterAdapter.test.ts @@ -342,6 +342,70 @@ describe('lighterAdapter', () => { makerPositionSignChanged: true, }; + describe('omitted venue fill context', () => { + it.each([true, false])( + 'normalizes omitted fields for maker=%s', + (isMaker) => { + for (const isAsk of [true, false]) { + for (const [before, size, pnl, expected] of [ + ['0', '1', undefined, isAsk ? 'Open Short' : 'Open Long'], + ['2', '1', undefined, isAsk ? 'Sell' : 'Buy'], + ['2', '1', '0', isAsk ? 'Sell' : 'Buy'], + ['2', '1', '-1', isAsk ? 'Close Long' : 'Close Short'], + ['2', '2', '1', isAsk ? 'Close Long' : 'Close Short'], + ['2', '3', '-1', isAsk ? 'Long > Short' : 'Short > Long'], + ] as const) { + const trade = { + ...REAL_TRADE, + isMakerAsk: isAsk === isMaker, + size, + makerPositionSizeBefore: isMaker ? before : '9', + takerPositionSizeBefore: isMaker ? '9' : before, + makerPositionSignChanged: undefined, + takerPositionSignChanged: undefined, + askAccountPnl: isAsk ? pnl : '123', + bidAccountPnl: isAsk ? '123' : pnl, + }; + + const fill = adaptFillFromLighterTrade( + trade, + 'SOL', + isAsk ? 28 : 7, + ); + + expect(fill.direction).toBe(expected); + expect(fill.pnl).toBe(pnl); + expect(Object.hasOwn(fill, 'pnl')).toBe(pnl !== undefined); + } + } + }, + ); + + it.each([null, 'false', 0])( + 'rejects malformed supplied sign flag %s', + (flag) => { + for (const isMaker of [true, false]) { + const trade = { + ...REAL_TRADE, + isMakerAsk: isMaker, + makerPositionSignChanged: flag, + takerPositionSignChanged: flag, + }; + + expect(() => + adaptFillFromLighterTrade( + trade as unknown as Parameters< + typeof adaptFillFromLighterTrade + >[0], + 'SOL', + 28, + ), + ).toThrow('Invalid Lighter venue data'); + } + }, + ); + }); + it('adapts the real venue payload: a taker sell of the full position is Close Long', () => { const fill = adaptFillFromLighterTrade(REAL_TRADE, 'SOL', 28); expect(fill).toMatchObject({ @@ -549,14 +613,44 @@ describe('lighterAdapter', () => { ); }); - it('rejects missing account pnl for an existing position', () => { - expect(() => - adaptFillFromLighterTrade( - { ...REAL_TRADE, askAccountPnl: undefined }, - 'SOL', - 28, - ), - ).toThrow('Invalid Lighter venue data'); + it.each([true, false])( + 'rejects missing reduction pnl for maker=%s', + (isMaker) => { + for (const isAsk of [true, false]) { + for (const size of ['0.133', '0.2']) { + const trade = { + ...REAL_TRADE, + size, + isMakerAsk: isAsk === isMaker, + makerPositionSizeBefore: isMaker ? '0.133' : '0', + takerPositionSizeBefore: isMaker ? '0' : '0.133', + makerPositionSignChanged: isMaker, + takerPositionSignChanged: !isMaker, + askAccountPnl: isAsk ? undefined : '123', + bidAccountPnl: isAsk ? '123' : undefined, + }; + + expect(() => + adaptFillFromLighterTrade(trade, 'SOL', isAsk ? 28 : 7), + ).toThrow('is missing valid account pnl'); + } + } + }, + ); + + it('preserves unknown pnl with a supplied unchanged-sign flag', () => { + const fill = adaptFillFromLighterTrade( + { + ...REAL_TRADE, + askAccountPnl: undefined, + takerPositionSignChanged: false, + }, + 'SOL', + 28, + ); + + expect(fill.direction).toBe('Sell'); + expect(fill).not.toHaveProperty('pnl'); }); it.each([ @@ -579,7 +673,8 @@ describe('lighterAdapter', () => { const fill = adaptFillFromLighterTrade(trade, 'SOL', isAsk ? 28 : 7); - expect(fill).toMatchObject({ pnl: '0', direction, startPosition: '0' }); + expect(fill).toMatchObject({ direction, startPosition: '0' }); + expect(fill).not.toHaveProperty('pnl'); }, ); @@ -602,7 +697,7 @@ describe('lighterAdapter', () => { }, ); - it('accepts omitted counterparty pnl while requiring the selected account pnl', () => { + it('uses the selected participant pnl independently of the counterparty', () => { expect( adaptFillFromLighterTrade( { ...REAL_TRADE, bidAccountPnl: undefined },