Files
plm-lottery/BUGS.md
T

84 lines
4.4 KiB
Markdown
Raw Normal View History

# BUGS.md — open findings
Only the 2026-09-28 specification review (B-73 … B-100) still has open entries
(B-92, B-93, B-94, B-96). A finding is removed from this file once it is fixed, and
`git log --all --grep 'B-nn'` is the record of how it was closed. Per CLAUDE.md's
convention each entry gets its own commit with its own regression test, and the `B-nn`
marker goes in a comment next to the fix.
# Audit of 2026-09-28 — specification review (B-73 … B-98)
Fourth audit, run against the new specification in `docs/` rather than the code
alone: four independent reviews (coin/chain mechanics, money state machine,
DevOps, spec cross-validation), with the critical claims re-verified in code.
The target behaviour of every entry below is now written into `docs/` as
requirements marked `*Stato:* da implementare (B-nn)`; the Italian overview, the
design decisions and the implementation order are in
[docs/piano-correzioni.md](docs/piano-correzioni.md). The code has **not** been
changed yet. Same convention as above: one commit per entry, with its own
regression test, spec + code + test together.
Design decisions taken with the owner on 2026-09-28 (they change existing
decisions, see `docs/limiti-noti.md`):
- **Draw by commit-reveal.** The winner must not be computable by anyone who
controls block production, since PLM's hashrate (~22 TH/s) is rentable.
- **Never release funds on doubt.** A money-moving row is rolled back only on proof
(an explicit rejection, or its inputs confirmed-spent elsewhere), never on a
timeout or on one server's silence.
- **One pool address per round**, so a round's money, history and payout are
isolated from every other round's.
- **Deposits need 6 confirmations** (configurable), everything else keeps 1: the
deposit is the only input a user can double-spend.
- **Contain reorgs instead of pretending they don't happen:** detect them, pause
on deep ones, and pay out only what is verifiably on chain.
## Medium
### B-92 — No health, readiness or real audit browsing
`/health` always answers ok; nothing exposes Electrum state; the audit endpoint
returns only the latest 500 rows unfiltered; registration writes no audit row; the
access log records Caddy's IP; app logs go only to the file.
Fix: `GET /health/ready` (Electrum connected, tip age < 1800s, scheduler heartbeat <
30s) used as the container healthcheck; Electrum status in `/admin`; audit filters by
event type, round, user and time, with pagination; `user_registered` audit row with
index and address; uvicorn proxy headers from Caddy only; logs also to stdout.
### B-93 — Deploys and rollbacks
A deploy with an open SSE stream never shuts down gracefully (uvicorn waits, Docker
kills after 10s); an older image cannot start on a newer DB; images are untagged;
`SITE_ADDRESS` passed per command reverts to `localhost` on the next plain
`docker compose up`.
Fix: close SSE streams on shutdown, `stop_grace_period: 30s`, await task
cancellation; tag images per release; `SITE_ADDRESS` in `.env` with `${SITE_ADDRESS:?}`;
documented pre-deploy backup and downgrade procedure.
### B-94 — Account and endpoint abuse limits
`change-password` has no rate limit, no audit and no maximum length; login fields
have no length bounds; `POST /bug-reports` is anonymous and unlimited;
`GET /admin/users` has no pagination; admin, SSE and framework errors are bare
strings; static JS/CSS have no `Cache-Control`.
Fix: throttle and audit (`password_changed`) the password change, cap every
credential field at 256, rate-limit bug reports per IP, paginate the user list,
structured errors everywhere, `Cache-Control: no-cache` on static files.
### B-96 — Smaller state inconsistencies
A reduced full-balance withdrawal is not re-checked against the minimum; an abandoned
payout leaves `winner_amount_sats`/`fee_amount_sats` set; a promoted payout writes no
`payout_sent` and a promoted bet no `bet_placed`; the reconciler pings SSE for every
row it checks; concurrent refreshes of one user can collide on the unique outpoint
and end the Electrum session; `listener.client` is read outside the per-user lock;
a bet covered only by pending change gets `insufficient_balance` instead of
`balance_pending_confirmation`. A round waiting in `closing` on a bet that never
leaves the mempool gives no signal at all.
Fix: each as described, one regression test per item; for the `closing` wait, a
`closing_stalled` audit row every 1800s past the deadline and `closing_waiting_since`
on `GET /rounds/current` (spec: TX-29).