Weird-pass review of pink_direct.py + exec_router.py + unified spec vs industry practice. Ranked findings, each grounded (cited sources + nautilus/FIX/OMS internals): - H1 float money math (PINK) — against consensus; ALREADY fixed by rebuild's Decimal contract - H2 crash-recovery state in /tmp — durable+atomic path needed - H3 shared VST account + symbol-membership ownership — sub-account isolation is the fix - H4 clientOrderId must be unique PER ATTEMPT — gap in my contract.py (raw request_id seed) - H5 WS-primary without explicit seqnum gap detection — add forced REST resync - M1 bare except-pass swallows; M2 dead-man-stop orphan hard-gate; M3 L673 sign bug Plus a balance section: where PINK/spec are AT/ABOVE best practice (don't 'fix' these). Net: spec sound (deviations are additions); real deviations are PINK-side; float already fixed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
11 KiB
Execution layer — adversarial best-practice audit (the "weird pass")
From: Fable, 2026-07-14. Scope: pink_direct.py + exec_router.py +
SPEC_UNIFIED_EXEC_LAYER_20260714.md vs industry / best-of-breed execution practice.
Method: current-web grounding (cited) + best-of-breed internals (nautilus_trader —
which this repo vendors — FIX ClOrdID discipline, professional OMS drop-copy patterns,
Hummingbot in-flight-order tracking). Ranked most-severe first. Nothing here is a
BLUE/PINK edit order — it is the port's correctness checklist + escalations for the operator.
HIGH
H1 — float for all money/price/size math (PINK). Against near-universal consensus.
pink_direct.py casts everything to float: _slot_to_position_dict (L55-78), capital,
size, PnL, fees throughout. IEEE-754 cannot represent most decimal money exactly; error
accumulates across many fills — and PnL/fee/capital is precisely where verification is
expensive (doctrine anti-pattern #4). Industry consensus is blunt: never use float/double
for monetary math; use arbitrary-precision decimal or integer minor units (rounding every op
can be made exact, but a decimal type is the safe default). The irony: this system
sits on nautilus_trader, which ships fixed-precision Price/Quantity/Money value
types — and pink_direct.py casts them straight to raw float at the boundary.
- Mitigation PINK already has: the K≈E reconcile gate re-anchors capital to exchange
truth on every balance-bearing
ACCOUNT_UPDATE(L726-732), so drift is bounded and caught, not unbounded. That is a backstop, not correctness. - Verdict: real deviation, bounded in practice. The Unified spec +
contract.pyalready fix it — everything isDecimalat the boundary (contract §2, done). The port must NOT re-inherit float. This finding validates the rebuild direction.
MEDIUM-HIGH
H2 — crash-recovery state lives in /tmp. Against durable-state practice.
_KERNEL_STATE_PATH = /tmp/.pink_kernel_state.json (L87) holds crash recovery + fee
calibration + session-to-session continuity. /tmp is volatile (cleared on reboot /
tmpwatch), world-traversable, and racy. For a subsystem whose entire purpose is surviving
a crash, persisting its recovery tape to the one directory guaranteed to be wiped is
self-defeating. Best practice: a durable app-state dir ($XDG_STATE_HOME, /var/lib/...),
0600 perms, atomic write (tmp-file + rename) so a crash mid-write can't corrupt it.
The append-only-log school goes further: the event log is the source of truth, snapshots
merely accelerate recovery. Port: exec_unified persists working-order state to a
durable, atomically-written path — never /tmp.
H3 — shared VST account + symbol-membership fill ownership. Architectural foot-gun.
_fill_is_ours (L588-611) attributes fills on an account shared by PINK / PRODGREEN / BLUE /
manual, using symbol membership because BingX WS does not echo our clientOrderId.
Best practice is unambiguous: isolate strategies by sub-account (or dedicated API key) —
never multiplex independent trading systems on one account; margin and fills cross-
contaminate. Symbol membership is fragile precisely where it matters: two systems trading
the same symbol on the shared account misattribute each other's fills. The reseed-on-
ACCOUNT_UPDATE bounds the damage, but the root (shared account) is the deviation.
- Escalation: the real bug is the venue not echoing
clientOrderIdon WS. That should be pushed to BingX and/or worked around with a per-strategy sub-account. This is the correct long-term fix, not a smarter membership heuristic.
H4 — clientOrderId idempotency on retry (SPEC + my contract.py gap).
Industry rule (and FIX ClOrdID law): a unique key per ATTEMPT, never reused; a retry
after an INDETERMINATE submit needs a new venue id (exchanges reject duplicate
clientOrderId) while preserving linkage to the parent (FIX OrigClOrdID). TTL of the dedup
key must exceed the retry window.
- PINK does this right: retry builds
new_tid,intent_id=new_tid(L1210-1212) — fresh id per retry. - My
contract.pydoes NOT yet:client_order_id_seedreturns the rawrequest_id(contract.py). If two attempts derive the same venue id, the retry-after-INDETERMINATE collides and is rejected — the exact failure the idempotency is meant to prevent. Fix: the dialect must mintu-<request_id>-<attempt_n>(fresh per attempt, parent-linked) AND enforce venue charset/length limits (BingX caps clientOrderId length). Add anattemptcounter to the working-order state; the seed alone is insufficient.
H5 — WS-primary / REST-failover without explicit sequence-gap detection.
PINK starts the WS account stream as primary with poll failover inside it (L386-389). Best practice: WS is a delta channel; the REST snapshot is the reconciliation source of truth; seqnum gaps must be detected and force a REST resync; the append-only event log is the ultimate truth.
- PINK partially aligns:
pump_venue_eventsreconciles viavenue.reconcile()and the kernel dedups byseen_event_ids/_last_settled_pnl(L1236-1237) — that IS event-log- style idempotent application. Good. - Gap: no visible WS sequence-number gap detection; the SNAPSHOT-burst barrier is still a spec TODO (§19). The 5 s own-fill hot-window (L1055, L1323) is a time-based proxy for "WS may lag REST" — a heuristic where seqnum + REST-snapshot reconciliation is the rigorous mechanism. Port: add explicit order-stream gap detection → forced REST resync; keep the hot-window as belt-and-braces, not the primary guard.
MEDIUM
M1 — bare except Exception: pass swallowing. The project's own named house-demon.
Multiple silent swallows: venue back-ref (L351), own-fill symbol seed (L370), asset-picker
observe (L1428), and others. Some are correctly fail-safe with logging ("Hz read failure
must never affect trading", L881 — that one logs). The intent is right (telemetry/Hz
failure must never touch trading), but a bare pass in a hot loop hides the bug that
eventually bites — and the repo's TESTING_DOCTRINE explicitly names "silent failure is the
house specialty." Best practice: catch the specific exception, always log at ≥debug with
context. Port: fail-safe guards keep the guard, lose the silence — log every swallow.
M2 — dead-man's-stop orphan + flip risk (already spec-flagged; make it a HARD gate).
Attaching STOP_MARKET at ~2× software SL (spec §6/§10) is best practice for silent-death
survival — but two known foot-guns: (a) the attached stop must auto-cancel on position
close or it orphans (the proto-PINK hell the spec §6 already flags); (b) it must be
reduceOnly/closePosition so a fire cannot open a reverse position. Verdict: the
spec flags (a) as a VST-verify TODO — good, but it must be a hard pre-live gate with a
mutation-tested assertion, not a checklist line. ProtectiveSpec.reduce_only=True (my
contract) covers (b); keep it enforced.
M3 — live sign inconsistency pink_direct.py:673 (# negative = rebate).
Contradicts today's Q1 finding (BingX commission negative = debit/cost). If
bingx_user_stream passes the raw venue sign into event.fee, the K-fold books a cost as
income. Bounded by the K≈E gate + abs() in calibration (L549), so likely not bleeding —
but it is a real sign inconsistency. Not fixing (BLUE/PINK untouchable); the port's §12
friction telemetry must get the sign right at its own boundary. Already logged in the port
inventory §4.2.
LOW
L1 — hardcoded cadences / magic numbers (1 s TTL, 5 s window, 15 % dev, 1e-9).
Best practice + the spec's own NFR: named constants with provenance, cadence injected. PINK
partially violates; the port fixes it (_constants.py + injected clock). Minor.
L2 — "GTX is the ONLY certified fill-improvement technique" — conservative, not wrong.
Industry also uses pegged/midpoint/iceberg/adaptive-limit. The spec is honestly scoping to the only empirically-certified technique for THIS venue/history — disciplined, not a deviation. The tier ladder (T2+) leaves room for the others. No action.
Where PINK / the spec are AT or ABOVE best practice (balance — do not "fix" these)
- "Unknown is never flat" / INDETERMINATE triage (spec §5, L5 doctrine): matches best- practice execution-truth handling — never synthesize a REJECT; let the E-feed FILL settle. This is better than many retail bots, which optimistically assume failure = flat.
- Idempotent replace = cancel-confirm → place (no blind amend) (spec §5): FIX-grade.
- Telemetry off the exec path, lossless side-lane (spec §12,
b46ebd2lesson): textbook — no I/O in the hot loop. - Fail-safe requote gate; venue position is the truth (
_exec_safe_to_requote, L1049): fail-closed on ambiguity is exactly right ("a skipped entry is safe, a doubled one is not"). - Three price truths never conflated (mark = geometry, OB-top = execution, last = parity; spec §9): matches how professional risk/exec systems separate mark vs trade price.
seen_event_idsidempotent event application (L1236): the append-only-log dedup discipline the sources call the ultimate source of truth.- New
contract.pyusesDecimalend-to-end: directly fixes H1 for the port.
Net verdict
The spec is sound; its deviations (H4 clientOrderId-per-attempt, H5 seqnum gaps, M2 hard-gate the stop) are additions, not corrections. The real deviations are PINK-side (H1 float, H2 /tmp, H3 shared account, M1 silent swallows) — and the single most important one (float money) is already fixed by the rebuild's Decimal contract. The audit thus argues for the port, and hands it a concrete checklist.
Sources
- Idempotency / clientOrderId: Token Metrics, Coinbase idempotency, George Tsiokos — idempotent orders
- Float vs decimal money: Modern Treasury — floats don't work for cents, DZone — never use float for money, evanjones.ca — you can, with rounding
- WS vs REST reconciliation / seqnum: FIX vs REST vs WebSocket, Coinbase Advanced Trade WS, Kraken WS FAQ