[High] Trade confirmation can submit a different worst-fill bound than the value reviewed by the user

[Bayyan16]

Summary

The trade confirmation modal can display one worst-fill price to the user while the submitted transaction uses a different limitPriceE6.

OrderTicket calculates a fresh worstFillPriceE6 when the confirmation modal is opened and stores it in confirmSnapshot. However, the confirmed value is not forwarded through the original submission path.

The confirmation callback forwards only the snapshotted position size. trade() is consequently called without an explicit limitPriceE6, causing useTrade to derive a new limit from the latest live-market price at the time of submission.

If the market moves between opening the confirmation modal and pressing Confirm, the protection bound submitted for execution can differ from the bound the user reviewed.

Severity

High — Trade confirmation integrity

This is not classified as Critical because the existing fallback still derives a non-zero slippage limit. However, the submitted price-protection parameter is not guaranteed to match the value explicitly displayed to and confirmed by the user.

Affected Branch

playground

Affected Components

• app/components/trade/OrderTicket.tsx
• app/hooks/useTrade.ts
• app/components/trade/TradeConfirmationModal.tsx

Preconditions

The issue can occur when:

1. A user has a funded trading account.
2. A valid market and LP are available.
3. The user enters a valid Long or Short order.
4. The live mark changes after the confirmation modal opens but before the transaction is submitted.

The price movement does not need to be extreme. Any update that changes the derived slippage bound can produce a mismatch.

Technical Details

When the user opens the confirmation modal, OrderTicket performs a fresh price read and calculates a worst-fill bound:

const freshPriceE6 = getLivePriceSnapshot(slabAddress).priceE6;

let worstFillPriceE6 = 0n;

try {
  worstFillPriceE6 =
    freshPriceE6 && freshPriceE6 > 0n
      ? computeLimitPriceE6({
          markE6: freshPriceE6,
          size: signedSize,
        })
      : 0n;
} catch {
  worstFillPriceE6 = 0n;
}

setConfirmSnapshot({
  positionSize,
  marginNative,
  estimatedLiqPrice: afterLiqPrice,
  tradingFee: fee,
  worstFillPriceE6,
});

The modal then displays confirmSnapshot.worstFillPriceE6 to the user.

However, the original handleTrade() submission path accepts only the snapshotted position size:

async function handleTrade(snapshotSize?: bigint) {
  const effectiveSize = snapshotSize ?? positionSize;

  // ...

  const size =
    direction === "short"
      ? -effectiveSize
      : effectiveSize;

  const sig = await withTransientRetry(
    async () =>
      trade({
        lpIdx,
        userIdx: userAccount!.idx,
        size,
      }),
    {
      maxRetries: 2,
      delayMs: 3000,
    },
  );
}

The reviewed worstFillPriceE6 is not included in the parameters passed to trade().

Inside useTrade, an omitted limitPriceE6 triggers a second calculation using the latest live mark:

const { priceE6: livePriceE6 } =
  getLivePriceSnapshot(slabAddress);

const effectiveLimitPriceE6 =
  params.limitPriceE6 !== undefined
    ? params.limitPriceE6
    : computeLimitPriceE6({
        markE6: livePriceE6 ?? 0n,
        size: params.size,
      });

Therefore, the confirmation modal and the transaction builder can use price snapshots taken at different times.

Vulnerable Flow

T0: User clicks the trade button
    ↓
OrderTicket reads live mark A
    ↓
OrderTicket calculates worst-fill bound A
    ↓
Confirmation modal displays bound A
    ↓
Market price changes while the modal remains open
    ↓
User presses Confirm
    ↓
Confirmation callback forwards only positionSize
    ↓
trade() receives no explicit limitPriceE6
    ↓
useTrade reads newer live mark B
    ↓
useTrade calculates worst-fill bound B
    ↓
Transaction is submitted with bound B

The user reviewed bound A, but the transaction can be submitted with bound B.

Expected Behavior

The worst-fill price shown in the confirmation modal should be treated as part of the confirmed transaction snapshot.

After the user presses Confirm:

displayed worstFillPriceE6
=
confirmed worstFillPriceE6
=
submitted limitPriceE6

A later live-price update must not silently replace the already-reviewed bound.

Actual Behavior

Only the snapshotted position size is forwarded from the confirmation modal.

The reviewed worstFillPriceE6 is discarded, and useTrade derives a new value from the latest live mark.

As a result:

displayed worstFillPriceE6
may not equal
submitted limitPriceE6

User Impact

The confirmation modal represents the final transaction review step before wallet approval. Users reasonably expect the displayed worst-fill value to describe the price protection applied to the transaction they are confirming.

Because the reviewed value is not bound to submission, the application can submit execution parameters that were not shown in the modal.

Potential impact includes:

• execution under a worst-fill bound the user did not review;
• inconsistent confirmation and transaction semantics;
• reduced confidence in the accuracy of the confirmation modal;
• increased exposure during fast-moving or volatile markets;
• unexpected differences between displayed protection and submitted protection.

The issue affects both Long and Short orders because the derived bound depends on the signed position size and the live mark used during calculation.

Manual Reproduction

1. Open the playground trading interface.
2. Connect a funded wallet.
3. Select an active market with available liquidity.
4. Enter a valid Long or Short order.
5. Click the main trade button to open the confirmation modal.
6. Record the displayed worst-fill price.
7. Keep the confirmation modal open while the market price updates.
8. Press Confirm.
9. Inspect the parameters passed to useTrade.trade() or decode the submitted trade instruction.
10. Compare the submitted limitPriceE6 with the value previously shown in the confirmation modal.

Result

The confirmation modal contains a snapshotted worstFillPriceE6, but the original call to trade() does not include it. useTrade calculates a new bound from the latest live mark instead.

Automated PoC

The following PoC uses the production computeLimitPriceE6() implementation to model the exact vulnerable flow:

• a worst-fill bound is calculated for the confirmation modal;
• the submitted parameters omit limitPriceE6, matching the original OrderTicket call;
• the live mark changes before submission;
• useTrade fallback behavior calculates a different effective bound.

Create:

app/__tests__/lib/confirmedWorstFillMismatch.poc.test.ts

with the following content:

import { describe, expect, it } from "vitest";

import { computeLimitPriceE6 } from "../../lib/slippage";

interface TradeParams {
  lpIdx: number;
  userIdx: number;
  size: bigint;
  limitPriceE6?: bigint;
}

describe("PoC: confirmed worst-fill bound is not bound to submission", () => {
  it("submits a newly derived bound when the live mark changes after confirmation", () => {
    const positionSize = 2_000_000n;

    // Price used when OrderTicket opens the confirmation modal.
    const confirmationMarkE6 = 100_000_000n;

    // Price available later when useTrade builds the transaction.
    const laterSubmissionMarkE6 = 101_000_000n;

    const reviewedWorstFillPriceE6 = computeLimitPriceE6({
      markE6: confirmationMarkE6,
      size: positionSize,
    });

    // Mirrors the vulnerable OrderTicket submission:
    // trade({ lpIdx, userIdx, size })
    //
    // confirmSnapshot.worstFillPriceE6 is not forwarded.
    const submittedParams: TradeParams = {
      lpIdx: 3,
      userIdx: 7,
      size: positionSize,
    };

    expect(submittedParams.limitPriceE6).toBeUndefined();

    // Mirrors useTrade fallback behavior when limitPriceE6 is omitted.
    const effectiveSubmittedLimitPriceE6 =
      submittedParams.limitPriceE6 !== undefined
        ? submittedParams.limitPriceE6
        : computeLimitPriceE6({
            markE6: laterSubmissionMarkE6,
            size: submittedParams.size,
          });

    // Confirms that both values are valid slippage bounds.
    expect(reviewedWorstFillPriceE6).toBeGreaterThan(0n);
    expect(effectiveSubmittedLimitPriceE6).toBeGreaterThan(0n);

    // Bug: the submitted protection is not the value the user reviewed.
    expect(effectiveSubmittedLimitPriceE6).not.toBe(
      reviewedWorstFillPriceE6,
    );
  });

  it("also affects short orders", () => {
    const positionSize = -2_000_000n;
    const confirmationMarkE6 = 100_000_000n;
    const laterSubmissionMarkE6 = 99_000_000n;

    const reviewedWorstFillPriceE6 = computeLimitPriceE6({
      markE6: confirmationMarkE6,
      size: positionSize,
    });

    const submittedParams: TradeParams = {
      lpIdx: 3,
      userIdx: 7,
      size: positionSize,
    };

    const effectiveSubmittedLimitPriceE6 =
      submittedParams.limitPriceE6 !== undefined
        ? submittedParams.limitPriceE6
        : computeLimitPriceE6({
            markE6: laterSubmissionMarkE6,
            size: submittedParams.size,
          });

    expect(reviewedWorstFillPriceE6).toBeGreaterThan(0n);
    expect(effectiveSubmittedLimitPriceE6).toBeGreaterThan(0n);

    expect(effectiveSubmittedLimitPriceE6).not.toBe(
      reviewedWorstFillPriceE6,
    );
  });
});

Run the PoC from the app directory:

pnpm exec vitest run \
  __tests__/lib/confirmedWorstFillMismatch.poc.test.ts

Expected PoC Output

✓ PoC: confirmed worst-fill bound is not bound to submission
  ✓ submits a newly derived bound when the live mark changes after confirmation
  ✓ also affects short orders

Test Files  1 passed
Tests       2 passed

A passing PoC demonstrates that the value calculated during confirmation can differ from the effective value derived during submission when the explicit confirmed limit is omitted.

Source-Flow Verification

The vulnerable source chain can also be confirmed directly:

grep -nE \
  "confirmSnapshot|worstFillPriceE6|async function handleTrade|trade\\(" \
  app/components/trade/OrderTicket.tsx

Verify that:

1. confirmSnapshot contains worstFillPriceE6.
2. The modal displays the snapshot.
3. handleTrade() originally accepts only snapshotSize.
4. The trade() call contains lpIdx, userIdx, and size.
5. The call does not contain limitPriceE6.

Then inspect the fallback:

grep -nE \
  "limitPriceE6|effectiveLimitPriceE6|getLivePriceSnapshot" \
  app/hooks/useTrade.ts

Verify that an undefined params.limitPriceE6 causes the limit to be recalculated from the latest live mark.

Root Cause

The confirmation snapshot is incomplete at the submission boundary.

Although worstFillPriceE6 is included in confirmSnapshot, the confirmation callback treats only positionSize as authoritative.

The reviewed bound is used for presentation but not propagated as an execution parameter.

This separates:

confirmation state

from:

transaction state

for a submit-critical slippage-protection value.

Recommended Fix

The confirmation callback should preserve the complete snapshot before clearing modal state and forward both:

• positionSize
• worstFillPriceE6

handleTrade() should accept the confirmed bound and submit it as the explicit limitPriceE6.

Example:

async function handleTrade(
  snapshotSize?: bigint,
  snapshotLimitPriceE6?: bigint,
) {
  const effectiveSize = snapshotSize ?? positionSize;

  // ...

  const size =
    direction === "short"
      ? -effectiveSize
      : effectiveSize;

  await trade({
    lpIdx,
    userIdx: userAccount!.idx,
    size,
    ...(snapshotLimitPriceE6 !== undefined &&
    snapshotLimitPriceE6 > 0n
      ? { limitPriceE6: snapshotLimitPriceE6 }
      : {}),
  });
}

The modal confirmation callback should copy the snapshot before resetting it:

onConfirm={() => {
  const snapshot = confirmSnapshot;

  setShowConfirmModal(false);
  setConfirmSnapshot(null);

  if (!snapshot) return;

  void handleTrade(
    snapshot.positionSize,
    snapshot.worstFillPriceE6,
  );
}}

Required Regression Coverage

The fix should include tests proving that:

1. The exact worst-fill bound shown to the user is submitted as limitPriceE6.
2. A later live-price-derived bound cannot overwrite the reviewed bound.
3. Long and Short directions preserve the confirmed value.
4. Missing, zero, or invalid confirmed values preserve the existing live-price fallback.
5. The confirmation snapshot is copied before modal state is cleared.
6. Existing callers that do not provide a confirmed bound remain backward-compatible.

Suggested Regression Assertions

expect(submittedParams).toEqual({
  lpIdx: 3,
  userIdx: 7,
  size: 2_000_000n,
  limitPriceE6: reviewedWorstFillPriceE6,
});

expect(submittedParams.limitPriceE6).toBe(
  reviewedWorstFillPriceE6,
);

expect(submittedParams.limitPriceE6).not.toBe(
  laterLivePriceBoundE6,
);

Security and Integrity Considerations

The issue does not bypass wallet authorization and does not directly alter the on-chain slippage formula.

The integrity problem occurs before signing: the UI presents one protection value, while the application can construct the transaction using another.

For transaction-confirmation interfaces, submit-critical values should be immutable after review unless the UI explicitly invalidates the confirmation and requires the user to review updated values again.

Scope of the Fix

The fix should remain limited to the client-side confirmation and trade-submission flow.

It should not require changes to:

• the on-chain Percolator program;
• the TradeCpi instruction layout;
• the slippage calculation formula;
• wallet signing logic;
• oracle update logic;
• backend APIs;
• dependency manifests or lockfiles.

Impact After Fix

After the fix, the flow becomes:

OrderTicket calculates bound A
→ modal displays bound A
→ user confirms bound A
→ OrderTicket forwards bound A
→ trade receives limitPriceE6: bound A
→ useTrade preserves the explicit value
→ submitted transaction uses bound A

This restores consistency between the value reviewed by the user and the price-protection parameter submitted for execution.

[dcccrypto]

Appears already fixed on playground — flagging so this can be closed if a maintainer agrees.

On `playground`, `app/components/trade/OrderTicket.tsx` already threads the reviewed bound through: handleTrade(snapshotSize?, snapshotLimitPriceE6?), with onConfirm forwarding snapshot.worstFillPriceE6 and submission routed via a bindConfirmedLimitPrice(...) helper.

I am not closing it myself since it is not my issue and I may be missing context. But it is worth checking: I have now examined five open HIGH issues on this repo (#2416, #2414, #2362, #2364, #2371) and all five are already fixed on playground, which is where development actually lands (recent PRs #2442/#2436/#2434 all target it; #2431/#2432 merged there).

The tracker appears to be lagging the branch, so the open-HIGH count materially overstates outstanding work. I wasted several cycles writing fixes for defects that were already solved — see #2440.

[Bayyan16]

Verified. #2363 was merged into playground, and the current confirmation flow forwards snapshot.worstFillPriceE6 through handleTrade and bindConfirmedLimitPrice, preserving the exact bound reviewed by the user. The current regression tests cover later live-price changes as well. Closing this issue as completed.