fix(perps): accept omitted Lighter fill context

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

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 = {

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

packages/perps-controller/src/types/lighter-types.ts

@@ -618,14 +618,15 @@ 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. */
   takerFee?: number | string;
   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;
 };
 
 /**

packages/perps-controller/src/utils/lighterAdapter.ts

@@ -277,35 +277,43 @@ 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: {
   isBuy: boolean;
   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';
     }
     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,

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(

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 },