fix(monitor): stop reporting v17 markets as a passing conservation check (#347)

[dcccrypto]

Addresses #347 — the false signal. Deliberately does not claim to implement the v17 invariant; see Scope below.

The bug

MonitorService is the keeper's only off-chain fund-conservation tripwire. Its own header states the purpose: if the on-chain SPL balance falls below what the program thinks is in the vault, funds have leaked and we alert immediately.

For v17 markets it bailed out before that check and recorded:

ok: true,                 // reports HEALTHY without checking anything
vaultTokenBalance: "0",
engineVault: "0",
shortfall: "0",

Since the v17 cutover every production market is v17, so the invariant never ran anywhere — and /health's invariants block actively showed green for a check that was never computed.

The ADL-staleness skip beside it is legitimate (ExecuteAdl was removed in v17). This one isn't: v17 vaults still hold SPL tokens and the program still tracks accounting; only the source field changed. The code answered "the old formula doesn't apply" with "report healthy".

The change

Makes "unevaluated" representable, and reports it:

ok: boolean | null   // null = NOT EVALUATED, no conclusion available

Balances become null rather than "0" — a dashboard reading shortfall: "0" concludes "checked, nothing missing", which is the same false green wearing a different hat.

Scope — what this does NOT do

It does not implement the v17 invariant. Deriving it (insurance fund + Σ portfolio capital/pnl backing vs getTokenAccountBalance) needs the v17 accounting model, and getting it subtly wrong would emit false shortfall alerts on a fund-leak detector — which trains on-call to ignore it, arguably worse than silence.

That work stays open on #347 and wants someone with the v17 accounting model (anchor/SDK side). What this PR fixes is the part that is unambiguously wrong today: a monitor claiming a result it never computed.

Why it's safe to change the type

/health embeds monitorService.getStatus() verbatim and does not gate its status code on invariants[].ok — I checked before touching it, since flipping every v17 market to non-true could otherwise have turned the endpoint red fleet-wide. src/index.ts is the only consumer outside this file.

Verification

The existing v17 test asserted ok: true, codifying the bug — updated, with a comment saying so. Added the two regressions the issue asks for.

Checked against the original monitor.ts, not a hand-rolled mutation:

ORIGINAL false-green code + tests → 3 of 4 FAILED  ✅
fixed code + tests                → 4 passed

• Full keeper suite: 977 passed, 0 failures
• npx tsc --noEmit clean

Touches only src/services/monitor.ts + its test — no collision with the open PRs on liquidation.ts / oracle.ts / crank.ts.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Market monitoring now clearly reports when v17 conservation checks have not been evaluated.
  • Unevaluated v17 checks no longer appear as successful or display fabricated zero-balance values.
  • Monitoring results include clearer information when a conservation check is unavailable.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 64042124-402d-4842-80fb-d4850d83b3b4

📥 Commits

Reviewing files that changed from the base of the PR and between 1d2cb11 and da6d693.

📒 Files selected for processing (2)

• src/services/monitor.ts
• tests/services/monitor.test.ts

---

📝 Walkthrough

Walkthrough

The monitor now represents the v17 conservation invariant as unevaluated, using nullable status and balance fields. Tests verify that v17 markets are not reported as passing and do not receive fabricated zero values.

Changes

V17 invariant correction

Layer / File(s)	Summary
Nullable v17 invariant reporting src/services/monitor.ts	MarketInvariantResult supports nullable invariant fields, and v17 markets are recorded with ok: null, null balances, and an expanded unchecked reason.
V17 invariant regression coverage tests/services/monitor.test.ts	Tests assert the v17 invariant is explicitly unchecked and that its balance fields remain null.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• Issue 347 — The change replaces the hard-coded passing v17 invariant with ok: null and adds regression coverage.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: v17 markets are no longer reported as passing the conservation check when it is not evaluated.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/347-v17-invariant-not-false-green

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[dcccrypto]

Closing as superseded — #394 (0817b66) implemented this independently.

src/services/monitor.ts on main now carries the fix this PR was for, with the reasoning spelled out in the same terms:

#347: `null` exists because this used to report `true` for v17 markets
without checking anything. Since the v17 cutover that is every production
market, so the only fund-conservation tripwire in the codebase was emitting
...
"UNCHECKED — v17 market account: the legacy engine.vault invariant does not apply
 and no v17 equivalent is implemented yet (#347). This is NOT a passing check."

Verified rather than assumed: I merged this branch into current main and diffed the result — net effect zero, no files changed. It is a literal no-op now.

Closing rather than leaving a no-op PR in the queue. #390 and #391 stay open — both still apply (#390 +195 lines, and the CI docker job on main is still gated if: github.ref == 'refs/heads/main', which is exactly what #391 fixes). Full keeper suite with both merged: 1036 passed, 0 failed.