Skip to content

adr: 0016 title: "Prevent and surface un-closeable zero-price CFD positions" status: accepted implementation-status: implemented date: 2026-06-03 implemented-date: 2026-06-03 implemented-in: - ftl-backend bug/cfd-zero-price-stuck-position deciders: [@amalkrsihna] affects-specs: [cfd-pricing-and-wallet, amm-pricing] affects-code: - ftl-backend/internal/redis/lua/position_open.lua - ftl-backend/internal/positions/service.go - ftl-backend/internal/positions/handlers.go - ftl-backend/migrations/000037_add_cfd_positions_price_checks.up.sql supersedes: null superseded-by: null

ADR-0016: Prevent and surface un-closeable zero-price CFD positions

Context

A player on the dev02 stack hit a position that could not be closed: the UI showed openPrice = 0 and marginRequired = 0, and pressing Close returned a generic "internal error" banner. The position was permanently stuck.

Root-cause analysis traced a chain of missing guards, each of which had to hold for the bug to manifest:

  1. position_open.lua truthy bug. The instrument-state guard read local base_price = live_bp or static_bp; if not base_price then ... end. In Lua, 0 is truthy, so if not base_price does not fire when base_price = 0. A zero base flowed through to server_price = (0 + k_mod*0) = 0 and margin_required = (0 * lot * CONTRACT_SIZE) / LEVERAGE = 0. The free-margin check (free_margin < margin_required ⇒ equity < 0) passes for any solvent user, so the open succeeded at price 0.

  2. No Go-side validation. service.Open passed result.OpenPrice straight into the cfd_positions INSERT with no > 0 check.

  3. No database constraint. cfd_positions.open_price and instruments.base_price had no CHECK (... > 0), so a zero row persisted.

  4. Close maps to a generic 500. On close, position_close.lua correctly returns corrupt_position_state (when open_price <= 0) or instrument_missing (when the instrument hash is gone from Redis). But neither string was handled in luaErrorToGoErr, so they became untyped errors.New(s), and closeErrorStatus fell through to its default branch — HTTP 500 "internal error". The user had no actionable signal and the position stayed stuck.

The upstream trigger for base_price = 0 is a cold-Redis / pre-first-tick race: EnsureRedisState seeds the instrument hash with HSETNX base_price <value>, and if the seeded value (or a freshly-seeded DB row) is 0 and the live ticker has not yet written live_base_price, the open prices at 0.

Decision

Close the bug at every layer it slipped through, and convert the silent 500 into an actionable client response.

  1. position_open.lua — reject a non-positive base price: if not base_price or base_price <= 0 then return {error='instrument not found'}. Add a belt-and-suspenders if server_price <= 0 guard after pricing.

  2. service.Open — reject result.OpenPrice <= 0 before the PG INSERT, returning the new typed ErrCorruptPositionState.

  3. Typed errors + HTTP mapping — add ErrCorruptPositionState and ErrInstrumentMissing; map corrupt_position_state and instrument_missing in luaErrorToGoErr; in closeErrorStatus surface them as 409 ("position is in an invalid state; contact support") and 503 ("instrument data unavailable; retry shortly") instead of 500.

  4. Migration 000037 — CHECK (open_price > 0) on cfd_positions and CHECK (base_price > 0) on instruments, both added NOT VALID.

The NOT VALID choice is deliberate (see Alternatives). A NOT VALID CHECK is enforced for every new or updated row immediately — which is the actual fix, no new stuck positions can be written — but Postgres does not scan existing rows. This makes the migration safe for an automated migrate-up even if a staging/prod environment still holds legacy zero-price rows. Those are remediated out-of-band (force-closed at zero PnL with closed_by='auto_exit_ft') and the constraint can later be promoted with VALIDATE CONSTRAINT.

Consequences

  • Positive: A zero-price position can no longer be created — guarded in Lua, in Go, and in the schema.
  • Positive: Any pre-existing stuck position now returns a specific 409 with a contact-support message instead of a dead-end 500. The player understands the state and support has a typed signal to act on.
  • Positive: The DB constraint is a durable backstop independent of the application code path (covers the outbox drainer and any future writer).
  • Neutral: NOT VALID leaves legacy rows unvalidated until an ops remediation + VALIDATE CONSTRAINT. Acceptable — new writes are fully guarded.
  • Risk: If a genuine instrument legitimately has base_price = 0 (it should not), opens on it now 404 instead of creating a junk position. That is the intended behaviour.

Alternatives considered

Alternative A: Add the CHECK constraints as VALIDATED (immediate full scan)

Add CHECK (open_price > 0) without NOT VALID, so Postgres validates all existing rows at migration time.

Rejected. ADD CONSTRAINT with immediate validation fails the entire migration if even one legacy zero-price row exists, which would break the automated migrate-up in the deploy pipeline on any environment that already has a stuck position. NOT VALID guards new rows (the fix) without coupling the schema change to data cleanup. Remediation + VALIDATE CONSTRAINT is a separate, non-blocking step.

Alternative B: Fix only the Lua guard

Add base_price <= 0 to position_open.lua and stop there.

Rejected. Defence in depth matters here because the failure was silent and player-facing. The Lua guard prevents new bad opens, but the Go guard and DB constraint protect against other write paths (outbox drainer, future callers), and the error-mapping change is what turns the existing stuck positions from an opaque 500 into an actionable 409. The bug existed because each layer trusted the one before it; the fix restores a guard at each layer.

Alternative C: Auto-heal stuck positions on close

When close hits corrupt_position_state, silently force-close the position at zero PnL instead of returning an error.

Rejected for now. Auto-healing on the hot close path hides a data-integrity problem and could mask a future regression. A 409 with a support path keeps the event visible. A batch remediation job (the documented ops step) is the right place to clean up the small, finite set of legacy rows — not the per-request close handler.

Verification

  • New unit tests lock luaErrorToGoErr("corrupt_position_state") ⇒ ErrCorruptPositionState and closeErrorStatus(ErrCorruptPositionState) ⇒ 409 (never the generic "internal error").
  • On dev02 Postgres, an INSERT ... open_price = 0 is rejected with SQLSTATE 23514 (check_violation) after migration 000037.
  • dev02 had zero offender rows at migration time; no remediation was needed there.