fix(trade): submit the worst-fill bound the user actually reviewed (GH#2362)

app/__tests__/lib/gh2362-submitted-limit-price.test.ts

@@ -0,0 +1,59 @@
+/**
+ * GH#2362 — the trade confirmation modal could display one worst-fill bound
+ * while the submitted transaction used another.
+ *
+ * TradeForm computes `worstFillPriceE6` when the modal opens and shows it to
+ * the user as the binding slippage limit, but the confirm callback forwarded
+ * only the position size. `trade()` was therefore called without an explicit
+ * `limitPriceE6`, so `useTrade` re-derived one from the live mark at submit
+ * time — any price move between opening the modal and pressing Confirm changed
+ * the protection bound the user had approved.
+ *
+ * The zero case is the trap. On-chain, `limit_price_e6 == 0` is a "no limit"
+ * sentinel that skips the slippage check entirely, and the modal's snapshot is
+ * `0n` whenever its computation failed. Blindly forwarding the snapshot would
+ * therefore convert a mismatched bound into NO bound.
+ */
+
+import { describe, it, expect } from "vitest";
+import { resolveSubmittedLimitPriceE6 } from "@/lib/submitted-limit-price";
+
+describe("GH#2362 submitted worst-fill bound", () => {
+  it("forwards the reviewed bound so the tx matches what the user approved", () => {
+    // The core fix: what was displayed is what gets submitted.
+    expect(resolveSubmittedLimitPriceE6(1_234_567n)).toBe(1_234_567n);
+  });
+
+  it("forwards the reviewed bound unchanged even if the market has since moved", () => {
+    // Staleness is the POINT — the user approved this number, and the tx should
+    // fail rather than execute at a bound they never saw.
+    const reviewed = 98_765_432n;
+    expect(resolveSubmittedLimitPriceE6(reviewed)).toBe(reviewed);
+  });
+
+  it("does NOT forward a 0n snapshot — that would disable slippage protection", () => {
+    // 0n means the modal's computation failed, not "no limit wanted".
+    // Returning undefined makes useTrade derive a fresh non-zero limit.
+    expect(resolveSubmittedLimitPriceE6(0n)).toBeUndefined();
+  });
+
+  it("does not forward a negative bound", () => {
+    expect(resolveSubmittedLimitPriceE6(-1n)).toBeUndefined();
+    expect(resolveSubmittedLimitPriceE6(-1_000_000n)).toBeUndefined();
+  });
+
+  it("returns undefined when there is no snapshot at all", () => {
+    // Non-confirm code paths (mock mode, direct calls) keep the old behaviour.
+    expect(resolveSubmittedLimitPriceE6(undefined)).toBeUndefined();
+  });
+
+  it("preserves bigint precision for large bounds", () => {
+    // Must not round-trip through Number.
+    const huge = 9_007_199_254_740_993n; // Number.MAX_SAFE_INTEGER + 2
+    expect(resolveSubmittedLimitPriceE6(huge)).toBe(huge);
+  });
+
+  it("treats the smallest positive bound as forwardable", () => {
+    expect(resolveSubmittedLimitPriceE6(1n)).toBe(1n);
+  });
+});

app/components/trade/TradeForm.tsx

@@ -28,6 +28,7 @@ import { useMarketInfo } from "@/hooks/useMarketInfo";
 import { formatTokenAmount, formatUsdPriceE6 } from "@/lib/format";
 import { useClosePosition } from "@/hooks/useClosePosition";
 import { saveEntryPrice, getEntryPrice, getEntryLeverage, clearEntryPrice } from "@/lib/entry-price";
+import { resolveSubmittedLimitPriceE6 } from "@/lib/submitted-limit-price";
 import { isSentinelValue } from "@/lib/health";
 import { DepositWithdrawCard } from "@/components/trade/DepositWithdrawCard";
 import { useInitUser } from "@/hooks/useInitUser";
@@ -458,7 +459,7 @@ export const TradeForm: FC<{ slabAddress: string }> = ({ slabAddress }) => {
   const needsDeposit = connected && userAccount && capital === 0n;
   const canTrade = connected && userAccount && capital > 0n && !lpUnderfunded;
 
-  async function handleTrade(snapshotSize?: bigint) {
+  async function handleTrade(snapshotSize?: bigint, snapshotLimitPriceE6?: bigint) {
     // Use the snapshotted size from the confirmation modal so the submitted
     // trade matches what the user reviewed, even if the live price moved
     // between modal-open and confirm. Fall back to live size if no snapshot
@@ -482,8 +483,23 @@ export const TradeForm: FC<{ slabAddress: string }> = ({ slabAddress }) => {
     setTradePhase("submitting");
     try {
       const size = direction === "short" ? -effectiveSize : effectiveSize;
+      // GH#2362: submit the worst-fill bound the user actually reviewed. The
+      // modal already displays this value as the binding slippage limit, but it
+      // was never forwarded, so useTrade re-derived one from the live mark at
+      // submit time — a market move between opening the modal and pressing
+      // Confirm silently changed the protection bound out from under the user.
+      // See resolveSubmittedLimitPriceE6 for why a 0n snapshot is NOT forwarded.
+      const submittedLimitPriceE6 = resolveSubmittedLimitPriceE6(snapshotLimitPriceE6);
+      const tradeParams: { lpIdx: number; userIdx: number; size: bigint; limitPriceE6?: bigint } = {
+        lpIdx,
+        userIdx: userAccount!.idx,
+        size,
+      };
+      if (submittedLimitPriceE6 !== undefined) {
+        tradeParams.limitPriceE6 = submittedLimitPriceE6;
+      }
       const sig = await withTransientRetry(
-        async () => trade({ lpIdx, userIdx: userAccount!.idx, size }),
+        async () => trade(tradeParams),
         { maxRetries: 2, delayMs: 3000 },
       );
       setTradePhase("confirming");
@@ -1048,9 +1064,12 @@ export const TradeForm: FC<{ slabAddress: string }> = ({ slabAddress }) => {
           decimals={decimals}
           onConfirm={() => {
             const snapSize = confirmSnapshot.positionSize;
+            // GH#2362: carry the reviewed worst-fill bound through to submission
+            // so the transaction is protected at the price the user approved.
+            const snapLimitPriceE6 = confirmSnapshot.worstFillPriceE6;
             setShowConfirmModal(false);
             setConfirmSnapshot(null);
-            handleTrade(snapSize);
+            handleTrade(snapSize, snapLimitPriceE6);
           }}
           onCancel={() => { setShowConfirmModal(false); setConfirmSnapshot(null); }}
         />

app/lib/submitted-limit-price.ts

@@ -0,0 +1,29 @@
+/**
+ * GH#2362 — which worst-fill bound actually gets submitted.
+ *
+ * The trade confirmation modal computes a `worstFillPriceE6` and shows it to
+ * the user as the binding slippage limit. That reviewed value must be the one
+ * the transaction carries; previously it was dropped and `useTrade` re-derived
+ * a bound from the live mark at submit time, so any price move between opening
+ * the modal and pressing Confirm silently changed the protection the user had
+ * approved.
+ *
+ * The subtlety is the zero case. The on-chain handler treats
+ * `limit_price_e6 == 0` as a "no limit" sentinel and skips the slippage check
+ * entirely (percolator.rs::handle_trade_cpi); `useTrade` deliberately preserves
+ * an explicit `0n` as an opt-in escape hatch for keeper/bot paths. But the
+ * modal's snapshot is *also* `0n` whenever its computation threw or no live
+ * price was available. Forwarding that zero would convert "the bound might not
+ * match what you saw" into "there is no bound at all" — strictly worse than the
+ * bug being fixed.
+ *
+ * So: forward a positive reviewed bound, and otherwise return undefined, which
+ * makes `useTrade` derive a fresh non-zero limit exactly as it did before.
+ */
+export function resolveSubmittedLimitPriceE6(
+  reviewedWorstFillPriceE6: bigint | undefined,
+): bigint | undefined {
+  if (reviewedWorstFillPriceE6 === undefined) return undefined;
+  if (reviewedWorstFillPriceE6 <= 0n) return undefined;
+  return reviewedWorstFillPriceE6;
+}