fix(bots): count only broadcast submits and explain skipped plans

[haydenshively]

Why

A BetterStack review of bot.liquidation.midnight (source 2607569, 14 days) found the bot looking healthy while doing nothing: ~1,780 ticks/hour, 0.19 ETH funded, 99.99% info. Since 2026-07-31 15:37 every tick reported liquidatable: 12, planned: 0 with every skip counter at zero — ~215,000 consecutive ticks — and no way to learn why without a debugger.

Three defects, all silent. Both liquidators had all three verbatim.

#	Defect	Production evidence
1	Two continues (in-flight, sizing refused) incremented no counter and logged nothing	12 liquidatable positions dropped every tick for 5 days
2	submitted counted submit calls, not broadcasts	2026-07-30 16:00–18:00: submitted: 2425, tx.sent: 0, tx.submit_failed: 2666
3	backoff.clear ran unconditionally after submit, wiping the attempt count so the delay never grew	7/30 16:00: 130 liquidatable, 2,197 planned, backoffSkipped: 0 for the whole hour — a hot loop burning a LiFi quote + a simulation + a send per position per block

What changed

submit reports whether it sent. PendingQueue.submit returns a SubmitOutcome (sent, or failed carrying a scope — 'position' or 'queue' — plus the exit it took, so adding a reason forces a scope decision in the type). Only sent counts submitted and clears a backoff. The TxSendError-with-nonce path still throws — the tick must abort rather than race the signer's cursor rollback.

Crucially, only the per-position failure (tx.submit_failed) backs a position off. tx.send_aborted, nonce.sync_failed and queue.nonce_hole are queue-wide refusals that reject every send that tick; attributing them to whichever position was in hand would suppress healthy positions for 2, 4, 8… blocks after the latch itself cleared.

Sizing explains itself. New planWithReason returns a discriminated reason plus a SizingTrace of the derived values (lif, effectiveDebt, cap, capEff, seizedAssets) — the ones an operator cannot compute by hand. plan() stays a thin facade, so the 236-line exact-bigint sizing test file is untouched and still passing. The tick logs plan.skipped with the inputs and the trace, at info for ordinary dust and warn for reasons that should be unreachable (a live assertion that eligibility and sizing have not diverged).

Bounded diagnostics. One line per position per block would be ~21k lines/hour/bot on a paid source. A new createBlockSampler in @repo/bot-kit is claimed once per tick and reset when nothing was skipped — so a quiet stretch never consumes the window (the first skip after any gap always reports), and a persistent condition settles to one coherent snapshot per ~5 min. createBalanceMonitor was refactored onto it rather than leaving two copies of the cadence pattern.

Counters that can't lie. New shape, ordered as the pipeline runs, with identities asserted in every tick test so a future stage added without a counter breaks a sum instead of silently dropping a position:

liquidatable === inflightSkipped + planSkipped + planned
planned      === cooledDown + backoffSkipped + noSwapPath + quoteFailed + ok + reverted
ok           === submitted + notSent

tick.end now carries complete and is emitted even when a submit aborts the tick — previously such a tick emitted nothing at all, so its counters vanished. The identities hold only for complete: true, which is documented.

Latent bug found while planning

When debt - maxDebt < badDebt < debt the RCF numerator goes negative, so maxRepaid, the cap and the derived seize all go negative. capBoundPlan's === 0n guard missed it and returned a plan with a negative seizedAssets (reproduced: -8146) that reverts opaquely once abi-encoded as uint256. The guard now discriminates on the raw cap: cap <= 0n → cap_not_positive at warn; a positive cap that floors to zero stays ordinary dust at info. (Splitting on capEff instead would have mislabelled every 1-wei cap with a non-zero margin as an impossible state.)

Also closed: an empty best slot could emit a (0, 0) plan that isBadDebtRealization misread as a write-off against a solvent position — now nothing_to_seize.

Corrected diagnosis

My first read of the incident listed bestCollateralPrice == 0 as a candidate cause. It cannot be: a zero price makes impliedRepaidUnits 0, so 0 <= effectiveDebt holds and plan() returns seizeWholeSlot. Under isLiquidatable the only reachable causes are the cap flooring to zero (dust) and the negative-cap path above.

Verification

• pnpm test after rebasing onto current main — 1,729 pass. The 3 local failures are credential-gated: midnight-liquidation/test/fork/{liquidation,queue} and market-making/test/e2e/setup-check, all requiring RPC_URL_8453; GitHub Actions is authoritative for those suites.
• Telemetry-focused Vitest suites — 157 pass across bot-kit and both liquidators.
• pnpm -r run typecheck passed for all workspace packages.
• pnpm lint passed with 0 warnings / 0 errors, pnpm format made no changes, pnpm knip passed, and git diff --check passed.
• Market-making Node playground suites on macOS — 24 pass, 91 platform skips, 0 fail.
• Mutation-tested, not just green. 16 mutants reintroducing each bug — all killed. Two rounds were needed, and both found real test gaps rather than confirming the tests were fine:
  • The first pass had 3 survivors, two of them on bug 3 itself. The mutant moved clear before record, so a single tick still ended up backed off and my assertion passed. What the bug actually destroys is the accumulated attempt count, so a test now pins that the delay grows (seed at block 1, fail at 100 → a 4-block wait, not 2).
  • A later pass over the simplification found nothing pinned that a post-maturity cap uses the post-writeoff debt (debt - badDebt) rather than the gross debt — a pre-existing hole, since sizing against gross debt would over-repay and revert on-chain (Panic 0x11). Now asserted by equivalence, so the test does not re-derive the arithmetic it checks.
  • One mutant is excluded as provably equivalent: seizedAssets <= 0n → === 0n cannot change behavior because the cap guard runs first. Documented in-code as belt-and-braces rather than tested.

Acceptance is ultimately a production query — the stuck 12 are live. After deploy, plan.skipped on source 2607569 answers the original question directly; expect seize_rounds_to_zero with a dust cap.

Deliberately out of scope

• BetterStack dashboards. Redefining submitted changes the meaning of any chart or alert built on it (sources 2607564 / 2607569), and the new fields need the dashboard metrics-collection step before they are queryable. Needs a follow-up pass.
• Low signer-balance warning. 800 of the 7/30 failures were gas required exceeds allowance (0) — a funding problem createBalanceMonitor logs but never warns on.
• The 7/30 sim-ok → send-revert divergence itself (1,861 × "reverted for an unknown reason"). This PR removes the amplification and the misreporting; root-causing a one-hour event on state that no longer exists is not in scope.
• getBaseFee() in both submit wrappers still aborts a tick, now visible as tick.end{complete:false} + tick.error. The fix is hoisting the fee read to once per tick (also N RPC calls → 1), not widening SubmitOutcome.
• breaking the loop on the three latched refusals, where no later submit can succeed this tick.
• plan.built fires before the cooldown and backoff gates, so a plannable but permanently backed-off position logs it every block — pre-existing. Sampling it would lose the audit record for real liquidations; the fix is reordering the gates.

Notes for review

• Three commits: the implementation, a self-review pass for simplicity and comment density, and a final review-finding remediation. The self-review collapses sampler rationale repeated across four files, hoists effectiveDebt/wholeSlotRepaid out of both sizing mode branches into one shared ModeStage, and trims docs that restated an adjacent table.

• No new Error subclass: the liquidators' precedent is result unions (QuoteOutcome, SimulateResult), and the one genuine failure (TxSendError) already is a named subclass and correctly stays a throw.

• sizing/plan.ts converted wholesale to arrow consts per the CLAUDE.md rule, rather than leaving a mixed-style file.

• Rebased directly onto main after refactor(repo): drop bun for vitest, node, and esbuild #130 merged; the obsolete migration commits are no longer part of this PR.

• Conflict watch: fix(bot-kit): stamp pending txs with the broadcast-time head #116 also edits packages/bot-kit/src/queue/pending-queue.ts.

• TIB addenda (not edits, per docs/GUIDANCE.md) added to both bot TIBs, reconciling tick.end drift in both directions — backoffSkipped/cooledDown shipped undocumented, badRoute documented but never implemented.

🤖 Generated with Claude Code

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d4d253585

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".