Levandor

Auto-Trade 3c-2 ROUND-2 Validation — Bug/DRY/Redundancy/Gap Lenses

7 min read

Second independent validation pass over the completed pmv2 auto-trade 3c-2 (live submit) build, run through bug / DRY / redundancy / gap lenses — deliberately NOT the trade-safety lens that round-1 used. It found real issues the safety reviews missed, the worst of them capital-trapping. Final re-review of round-2 → CONFIRMED SAFE FOR STAGED ARM, all round-1 invariants intact. This note records what round-2 found; the consolidated arm runbook lives in autotrade-3c2-complete-handoff.

The lesson (most important takeaway)

A flow can be structurally trade-safe yet BROKEN at a vendored value-validation boundary the safety lens never checks. The round-1 review reasoned about lock order, reservation races, exposure caps, exit authority, reconcile truth — and signed off “SAFE FOR STAGED ARM.” But that verdict was premised on the SELL path actually working, and it did not: every real fractional-share exit failed at rs-clob-client-v2’s lot-size guard inside Amount::shares, far below the safety machinery. A different lens (does each value survive the vendored constructor?) found in minutes what the safety lens structurally could not see. When you vendor a value-validating SDK, audit every value you hand across that boundary against the SDK’s own accept/reject rules — separately from auditing the flow.

B1 — BLOCKER (capital-trapping): the SELL path was non-functional

B1 — every real fractional-share position was unsellable

Symptom: auto_sell reverted the order row back to open on a build_order error; the position never sold — for both auto-exit and the manual /autotrade sell. Capital trapped until on-chain resolution. Root cause: the vendored rs-clob-client-v2 Amount::shares(value) rejects any share amount whose value.normalize().scale() > LOT_SIZE_SCALE (=2) — Polymarket shares trade in 0.01 lots (types/mod.rs:193-200, enforced again in order_builder.rs:25). auto_sell passed the RAW on-chain fill straight into build_order → Amount::shares. The project’s locked test vector is 7.6219 shares (scale 4) → rejected → build_order errors → auto_sell reverts the row to open. So every fractional-share position with scale > 2 was un-exitable. Why round-1 missed it: the prior re-review checked assert_sell_amounts (the project’s own pure pre-POST assertion) but not the vendored lot-size guard one layer deeper in Amount::shares. The assertion passed; the constructor it fed still rejected.

Fix (commit e93dde1): quantize the sell size to the lot scale once and use that single value everywhere:

sell_shares = filled_size.trunc_with_scale(2)

sell_shares is then used consistently for build_order + quantize_sell(expected_shares) + assert_sell_amounts — so the value handed to Amount::shares always has scale ≤ 2 and the pure assertion agrees with it. Sub-0.01-share dust is held to resolution (the function returns before mark_selling, so a dust position never enters selling and never tries to POST an un-representable order).

For Agents — BUY is unaffected

The BUY path does not hit this guard. Amount::usdc allows scale ≤ 6, and the market-order builder has no lot gate. So the lot-size landmine is SELL-only. The fix is therefore scoped to the sell quantization; no BUY change.

Quick reference — Polymarket lot/precision rules at the vendored boundary

  • Shares (Amount::shares): scale must be ≤ 2 (0.01 lot). Truncate with trunc_with_scale(2) before building a SELL.
  • USDC (Amount::usdc): scale ≤ 6.
  • Constants live in rs-clob-client-v2 types/mod.rs:193-200; the share guard is re-applied in order_builder.rs:25.
  • The on-chain CTF fill (balanceOf) is full-precision — do not feed it raw to any share-amount constructor.

2E — Important (false risk-limit): max_copies_per_event never enforced

max_copies_per_event was loaded from the DB, validated, and displayed in /autotrade status — but never enforced. Different cohort members entering the same event were each copied up to the GLOBAL max_open cap; there was no per-event ceiling. The operator saw a configured per-event limit that did nothing.

Fix (commit 84e3e34): a per-event_slug COUNT of live non-terminal rows inside the reserve config-lock transaction, rejecting once the count reaches the cap. 0 = unlimited (intentionally divergent from max_open where 0 blocks all entries). Full mechanics in autotrade-max-copies-per-event.

2C / I3 / 1D — Important gaps (commit d4a2a85)

Three settle / exit-cursor / reconcile gaps, all fixed in one commit:

  • 2C — settle silently held a position that should have exited. settle_open_positions now alarms when it closes an open row that still carried exit_pending — i.e. a cohort exit signal was queued but the exit never completed, so the position rode all the way to resolution. Previously this was a silent ride-to-resolution; now the operator is told.
  • I3 — the exit cursor permanently DROPPED an event. The exits cursor advanced to the last-success detected_ts, so any other event sharing that exact timestamp was skipped forever (a > cursor on a non-unique key loses ties). Fixed with a forward-only resume_cursor: on halt, resume at first_failed − 1µs, so no same-timestamp sibling is lost.
  • 1D — reconcile’s unbounded CTF sweep ran ahead of entries. Reconcile did a per-row CTF balanceOf sweep with no limit on every tick, before entries — a growing position table meant an ever-longer on-chain sweep delaying (and competing with) entry processing each tick. Now capped at LIMIT 25 oldest rows per tick, with a visibility notify so the operator can see the sweep is bounded/working.

DRY extractions (commit efc69bc) + a self-inflicted regression (0f06ec5)

The DRY pass extracted collecting_notify / AutotradeDeps::market() / record_fill_then_open(FillOutcome) and removed dead mark_dry_run / load_autotrade_order (see autotrade-dry-refactors-2026-06-16 for the full inventory).

The DRY extraction introduced a visibility regression — caught by the FINAL re-review

collecting_notify (commit efc69bc) broke /autotrade sell’s reply. run_sell drained its collected messages via Arc::try_unwrap, but the extracted notify closure still held a second Arc clone → try_unwrap always failed → the drained vec was always empty → the manual /autotrade sell Telegram reply swallowed the SOLD / REJECTED / TIMEOUT result. Scope: visibility only — the sell itself executed correctly; only the operator’s reply was blank. CI stayed green because no test asserted the reply content. Fix (commit 0f06ec5): drain via lock() instead of Arc::try_unwrap, plus a regression test that asserts the reply is non-empty. Meta-lesson: a behavior-preserving refactor that touches an Arc/ownership-drain pattern can silently change runtime behavior the type-checker and a green CI both miss. The fix that swallowed output is exactly the kind of thing only a from-scratch re-read (not a diff review) catches.

Outcome

A final independent re-review of round-2 → CONFIRMED SAFE FOR STAGED ARM. All round-1 invariants intact. Gate green: 334 polymarket-fetch + 87 polymarket-data tests.

For Agents — round-2 commit ledger

IssueSeverityCommitOne-liner
B1BLOCKER (capital-trapping)e93dde1sell_shares = filled_size.trunc_with_scale(2); dust held to resolution; BUY unaffected
2EImportant (false risk-limit)84e3e34per-event_slug COUNT in the reserve config-lock tx; 0 = unlimited
2C / I3 / 1DImportant (gaps)d4a2a85settle alarms held exit_pending; forward-only resume_cursor (first_failed−1µs); reconcile LIMIT 25 + notify
DRYrefactorefc69bccollecting_notify / market() / record_fill_then_open; dead-code removal
regressionself-inflicted, visibility0f06ec5DRY collecting_notify blanked the /autotrade sell reply (Arc::try_unwrap on a still-cloned Arc) → lock()-drain + regression test

Known deferrals (unchanged from round-1)

These were carried by round-1 and remain accepted after round-2:

  • fee_usd = 0 — PostOrderResponse exposes no fee; cost understates / realized PnL overstates (fail-safe bias). Calibrate or wire order_trades fees.
  • Cap-breach reads as a benign “dedup” Skip — exposure-saturation is indistinguishable from a genuine dedup at the operator surface (visibility gap, not a correctness bug).
  • is_resolution_only unwired — still carries #[allow(dead_code)].
  • A timed-out-but-landed SELL books pnl = 0 (loud “verify manually” alarm) — manual-bookkeeping action item.
  • ~30-min REST trigger latency — the first-mover trigger is REST-polled (mean ~15 min detection lag), likely eroding the first-mover edge; a real-time on-chain OrderFilled WSS trigger is recommended before scaling past tiny caps.

Graph View

Backlinks