diff --git a/BUGS.md b/BUGS.md index 3905fab..b418e47 100644 --- a/BUGS.md +++ b/BUGS.md @@ -40,24 +40,6 @@ remains the last prerequisite for running unattended. ## Low — documentation and consistency drift -### B-68 — CLAUDE.md and README describe a state the code has moved past - -- CLAUDE.md's tech-stack line still says JWT has **no revocation (B-34)**; `token_version` - implements exactly that revocation (`app/db/models.py:29`, `app/auth/dependencies.py:26`). -- "Known gaps" still says **no rate limiting anywhere (B-33)**; login and registration - are throttled (`app/auth/rate_limit.py`). What is genuinely still unthrottled is - bets, withdrawals and the admin endpoints — that is the claim worth keeping. -- "Known gaps" still says **`/report-bug` is a placeholder**; it is fully implemented - and translated, with admin triage. Only `/guida` is still a stub. -- Test counts are stale in three places: 264 actual, CLAUDE.md says 253 twice, - README says 232. -- The code map omits `app/auth/rate_limit.py`, `app/api/client_ip.py` and - `app/api/routes/bug_reports.py`. -- README links `flowchart.mmd`, which does not exist (the diagrams live in - `flowchart/`), describes `docs/running-the-server.md` as covering "local venv vs. - Docker" (that workflow was removed in B-44), and links the anchor - `CLAUDE.md#tech-stack-mvp`, which no longer exists. - ### B-69 — stale in-code comments - `app/tx/reconcile.py:206` calls the payout retry "a future payout-retry routine — diff --git a/CLAUDE.md b/CLAUDE.md index d8d1177..8dbfe87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ The user communicates in Italian in chat — reply to them in Italian. Everythin ## Project status -All 10 stages of the original build order are code-complete and unit-tested — 313 tests, all under `tests/unit/` (`tests/integration/` is an empty package). Beyond them: Docker + Caddy deployment, admin dashboard (`/admin`), static test UI (`/`), pending-inclusive balance display, an SSE push channel layered over the original polling, self-service password change + admin password reset, and the reconciliation/corroboration machinery below. +All 10 stages of the original build order are code-complete and unit-tested — 345 tests, all under `tests/unit/` (`tests/integration/` is an empty package). Beyond them: Docker + Caddy deployment, admin dashboard (`/admin`), static test UI (`/`), pending-inclusive balance display, an SSE push channel layered over the original polling, self-service password change + admin password reset, and the reconciliation/corroboration machinery below. Verified on mainnet with real money: registration + address derivation, deposit crediting (1-conf), a real 10 PLM bet (broadcast → confirmed → change credited back), and one full round cycle (close → draw on a real block hash → 70/30 payout with sat math checked against the broadcast tx → confirmation → close → next round auto-opened). **Withdrawal and the RBF bump path have never been exercised against a live broadcast** — unit-tested only. @@ -33,7 +33,7 @@ PYTHONPATH=. python scripts/decrypt_master_key.py # ops recovery: decrypt+pr PYTHONPATH=. python scripts/encrypt_master_key.py # ops bootstrap: import an externally-generated xprv (--overwrite to replace) PYTHONPATH=. python scripts/electrum_smoke_test.py # manual check: connect, handshake, subscribe to headers, print the tip -python -m pytest # all 313 tests +python -m pytest # all 345 tests python -m pytest tests/unit/test_hd.py # one file python -m pytest tests/unit/test_hd.py::test_derivation_is_deterministic # one test ``` @@ -61,7 +61,7 @@ The `Caddyfile` sends baseline security headers — HSTS, `X-Content-Type-Option - Python 3.12+, FastAPI, SQLAlchemy 2 async + Alembic, SQLite via aiosqlite, `embit` for keys/PSBT/tx parsing. - **PLM access via the Electrum protocol only** (no full node/P2P). Dev bootstrap server: `santantonio.sytes.net:50002` (SSL). -- Auth: Argon2 hashing + JWT (HS256, 24h, **no revocation** — B-34). Argon2 costs tens of ms of CPU per call by design, so every async caller goes through `hash_password_async`/`verify_password_async` (`run_in_threadpool`, B-55) — inline it froze the whole process, background tasks included, for the duration of every login. The sync pair stays for tests and scripts. +- Auth: Argon2 hashing + JWT (HS256, 24h). Tokens **are** revocable (B-34): the token carries a `tv` claim, `User.token_version` is bumped by a self-service password change and by an admin reset, and `get_current_user`/`get_optional_user` reject any token whose `tv` no longer matches — so changing the password invalidates every session issued before it, instead of leaving them valid for up to `jwt_expire_minutes`. A token predating the claim decodes as `tv = 0`, which is what a migrated user starts at, so the deploy didn't log everyone out. Argon2 costs tens of ms of CPU per call by design, so every async caller goes through `hash_password_async`/`verify_password_async` (`run_in_threadpool`, B-55) — inline it froze the whole process, background tasks included, for the duration of every login. The sync pair stays for tests and scripts. - Secrets: master xprv Fernet-encrypted at rest, encryption key in an env var (never in the DB or git). `validate_runtime_secrets()` (`app/config.py`, called from the lifespan — deliberately *not* a `Settings` validator, so imports and tests need no real secrets) makes the server **refuse to serve** if `JWT_SECRET` < 32 chars or `XPRV_ENCRYPTION_KEY` is empty. An empty `ADMIN_TOKEN` is deliberately non-fatal: `require_admin` then denies everything, i.e. a locked panel, not an open one. - **Operational config lives in the DB, not in env vars**: every business/round parameter is one row of `round_config` (`app/rounds/config.py`), editable live from `/admin` — no redeploy, no restart. Defaults for a fresh instance are column defaults on `RoundConfig` (`app/db/models.py`), *not* `app/config.py`. Only secrets and infra wiring (master key, JWT secret, Electrum hosts, admin token, DB URL) stay in `.env`, since those need a restart anyway. @@ -108,8 +108,8 @@ Source of truth: the `PalladiumWallet` repo — [ChainProfiles.cs](../PalladiumW | Package | Contents | |---|---| | `app/main.py` | entry point: lifespan starts the six background tasks, mounts the routers and `app/static/` | -| `app/api/routes/` | `admin`, `bets`, `withdrawals`, `rounds` (incl. SSE), `users`, `qr`; `app/api/errors.py` holds the error contract | -| `app/auth/` | routes (register/login), Argon2 + JWT (`security.py`), `get_current_user`/`get_optional_user` | +| `app/api/routes/` | `admin`, `bets`, `withdrawals`, `rounds` (incl. SSE), `users`, `qr`, `bug_reports`; `app/api/errors.py` holds the error contract and `app/api/client_ip.py` the trusted-peer extraction every IP-keyed control uses | +| `app/auth/` | routes (register/login), Argon2 + JWT (`security.py`), `get_current_user`/`get_optional_user`, login/registration throttling (`rate_limit.py`) | | `app/db/` | `models.py` (all tables + the active-round index), engine/session factories | | `app/wallet/` | HD derivation + WIF export (`hd.py`), PLM network constants, address/scripthash, balance math, `psbt_builder.py` (build/sign bet, withdrawal, payout; `select_utxos`) | | `app/electrum/` | `client.py` (JSON-RPC, endpoint parsing, timeouts), `listener.py` (the one connection: rotation, keepalive, header validation, corroboration, deposit crediting) | @@ -210,7 +210,8 @@ Everything that spends money is written **before** it is broadcast and resolved Two static SPAs served directly by FastAPI (`main.py` mounts `app/static/` and adds routes for `/admin`, `/guida`, `/report-bug`) — no build step, no framework, no bundler, `Cache-Control: no-store`. - **`/`** — end-user test UI: register/login, then a navbar dashboard with four panels (Deposito with a QR from `GET /qr/{address}`, Bet, Prelievo, Profilo — account info + self-service password change via `POST /users/me/change-password`), above a persistent round-status card and the chain-status bar with the language switcher. -- **`/admin`** — gated by a token screen (not a login: just `X-Admin-Token` vs `ADMIN_TOKEN`), then five sections each backed by its own `/admin/*` endpoint: Parametri (`RoundConfig` + the Manutenzione card), Utenti (list, WIF privkey export, password reset — both audit-logged), Round, Transazioni pendenti, Audit log; plus a live Electrum/tip-height pill. **Deliberately not linked from `/`** in either direction. +- **`/admin`** — gated by a token screen (not a login: just `X-Admin-Token` vs `ADMIN_TOKEN`), then six sections each backed by its own `/admin/*` endpoint: Parametri (`RoundConfig` + the Manutenzione card), Utenti (list, WIF privkey export, password reset — both audit-logged), Round, Transazioni pendenti, Audit log, Bug report (triage `open` → `read` → `resolved`, audit-logged `bug_report_status_changed`); plus a live Electrum/tip-height pill. **Deliberately not linked from `/`** in either direction. +- **`/report-bug`** — standalone page (no navbar, own language switcher), reachable logged-in or logged-out: `POST /bug-reports` stores the report with the submitter attached when there is one, `GET /bug-reports/mine` is the reporter-side status view for the logged-in case, and `/admin`'s Bug report section is the triage end. Both talk to the same JSON API; there's no admin/user API split beyond `require_admin`. @@ -245,9 +246,9 @@ Accepted **by design** — distinct from the audit findings above (all fixed), w - **`drawing` doesn't resume after a restart.** `_tick()` handles `open`, `closing` and `paying_out` (the last via `_retry_payout_if_due`); nothing re-enters `_wait_for_next_block` after a crash. That wait is unbounded by design (the draw's entropy genuinely depends on a future block) but no longer silent — past `_DRAW_STALL_THRESHOLD_SECONDS` it logs progress and writes a `draw_stalled` audit entry, and `GET /rounds/current`'s `draw_waiting_since` surfaces it live (B-36). Restart-resumption itself remains the last prerequisite for running unattended. - **RBF handles one shape only**: a single change output, back to the tx's own sender, big enough to absorb the increase. No extra-input fallback, and none would help the case that used to hurt (an amount equal to the whole input total leaves no other UTXO to add) — which is why `build_signed_transaction` now guarantees a change output of at least `DUST_LIMIT_SATS` instead (B-62): a withdrawal for the full balance moves a dust limit less, a bet from a balance equal to the bet is refused with `balance_leaves_no_change`. What's left is a bump whose *delta* exceeds an otherwise-fine change output, which still raises `RbfError`; that tx is eventually abandoned and its UTXOs released. - **Withdrawal and RBF bump are unit-tested but never live-broadcast** (failure and rollback branches included — still not a real network). -- **No user-facing history.** `GET /users/me/last-round-result` covers exactly one case (the reveal backstop above). Admin has `/admin/rounds`, `/admin/pending-transactions`, `/admin/audit-log`; a user has no equivalent — a failed withdrawal leaves a `failed` row they can never see, which argues for closing this. +- **No user-facing history of rounds or transactions.** `GET /users/me/last-round-result` covers exactly one case (the reveal backstop above) and `GET /bug-reports/mine` one more (the reporter's own reports). Admin has `/admin/rounds`, `/admin/pending-transactions`, `/admin/audit-log`; a user has no equivalent — a failed withdrawal leaves a `failed` row they can never see, which argues for closing this. - **Admin auth is one shared bearer token** (`ADMIN_TOKEN`) with no per-admin identity: `audit_log` records *what* changed (config edits as `config_updated`, with before/after) but never *who* did it. It gates the user list, privkey export, password resets and history, so a leak is high-blast-radius. - **No rate limiting on bet, withdrawal, admin or SSE.** Login has a per-username + per-IP failure throttle and registration a per-IP quota of 5 accounts/hour (`app/auth/rate_limit.py`: `RateLimiter` for failed guesses at a secret, `RollingQuota` for "how many of these may one source create" — B-33, B-58); everything else is unlimited. -- **`/guida` and `/report-bug` are placeholders** (`app/static/guida.html`, `report-bug.html`) — links work, content is "coming soon". +- **`/guida` is a placeholder** (`app/static/guida.html`) — the link works, the content is "coming soon". `/report-bug` is *not*: it is fully implemented and translated, with admin triage (see Frontends). - **No integration tests against a live Electrum connection.** `tests/integration/` is empty; live verification has all been manual (`scripts/electrum_smoke_test.py`, ad hoc scripts, real mainnet txs). - **Single-process assumptions**: the SSE broadcaster and the per-user locks are in-process only. A multi-worker deployment needs a shared channel and a DB/Redis lock. The round-uniqueness invariant is *not* in this category — it's a DB index. diff --git a/README.md b/README.md index cc21160..dbe5720 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,9 @@ domain). ## Documentation - [CLAUDE.md](CLAUDE.md) — architecture, commands, domain decisions, known gaps (for anyone/anything working on the code) -- [flowchart.mmd](flowchart.mmd) — the source-of-truth flow diagram the implementation follows node-by-node +- [flowchart/](flowchart/) — the source-of-truth flow diagrams the implementation follows node-by-node: [platform-overview.mmd](flowchart/platform-overview.mmd) (the 5-phase flow) and [round-lifecycle.mmd](flowchart/round-lifecycle.mmd) (the round/draw lifecycle) - [docs/setup.md](docs/setup.md) — one-time setup (secrets, master key, migrations) -- [docs/running-the-server.md](docs/running-the-server.md) — how to launch it (local venv vs. Docker+Caddy, dev vs. production TLS) +- [docs/running-the-server.md](docs/running-the-server.md) — how to launch it with Docker+Caddy (dev vs. production TLS) - [docs/guida-utente.md](docs/guida-utente.md) — end-user guide to the test UI (Italian) - [docs/guida-admin.md](docs/guida-admin.md) — admin dashboard guide (Italian) @@ -51,7 +51,7 @@ domain). Python (FastAPI, SQLAlchemy async + Alembic, Argon2 + JWT auth), Electrum protocol for PLM network access (no full node), Docker + Caddy for -deployment. See [CLAUDE.md](CLAUDE.md#tech-stack-mvp) for the complete list +deployment. See [CLAUDE.md](CLAUDE.md#tech-stack) for the complete list and the reasoning behind each choice. ## Testing @@ -61,7 +61,7 @@ python -m pytest # all tests python -m pytest tests/unit/test_hd.py # one file ``` -232 unit tests cover HD derivation, PSBT building, the Electrum client, bets, +345 unit tests cover HD derivation, PSBT building, the Electrum client, bets, deposits, withdrawals, the round/draw engine, RBF fee-bumping, admin config, the pending-inclusive balance calculation, and the SSE push channel. No automated integration tests against a live Electrum connection — mainnet diff --git a/tests/unit/test_docs_current.py b/tests/unit/test_docs_current.py new file mode 100644 index 0000000..e1ba467 --- /dev/null +++ b/tests/unit/test_docs_current.py @@ -0,0 +1,91 @@ +"""B-68: CLAUDE.md and README must describe the code as it is now. + +The audit found both files still asserting things the code had moved past — +JWT "no revocation" after token_version implemented exactly that, /report-bug +"a placeholder" after it shipped with admin triage, three stale test counts, a +code map missing three modules, and README links to a file and an anchor that +no longer exist. None of that is catchable by reading the code, so it is +pinned here instead. +""" + +import re +import subprocess +import sys +from pathlib import Path + +import pytest + + +_ROOT = Path(__file__).resolve().parents[2] +CLAUDE_MD = (_ROOT / "CLAUDE.md").read_text(encoding="utf-8") +README = (_ROOT / "README.md").read_text(encoding="utf-8") + + +def _collected_test_count() -> int: + result = subprocess.run( + [sys.executable, "-m", "pytest", "--collect-only", "-q"], + cwd=_ROOT, + capture_output=True, + text=True, + timeout=300, + ) + match = re.search(r"(\d+) tests? collected", result.stdout) + assert match, f"could not parse the collection summary:\n{result.stdout[-2000:]}" + return int(match.group(1)) + + +def test_documented_test_counts_match_reality(): + actual = _collected_test_count() + documented = [int(n) for n in re.findall(r"(\d+) tests\b", CLAUDE_MD)] + documented += [int(n) for n in re.findall(r"(\d+) unit tests\b", README)] + assert documented, "no test count found in CLAUDE.md or README — did the wording change?" + for count in documented: + assert count == actual, ( + f"docs claim {count} tests, the suite collects {actual} — " + "update the counts in CLAUDE.md (twice) and README.md" + ) + + +@pytest.mark.parametrize( + "stale", + [ + "no revocation", # token_version implements it (app/auth/dependencies.py) + "`/report-bug` are placeholders", # /report-bug shipped, only /guida is a stub + ], +) +def test_claude_md_has_no_stale_claims(stale): + assert stale not in CLAUDE_MD + + +@pytest.mark.parametrize( + "module", + ["rate_limit.py", "client_ip.py", "bug_reports"], +) +def test_code_map_covers_every_package_member(module): + code_map = CLAUDE_MD.split("## Code map", 1)[1].split("## Background tasks", 1)[0] + assert module in code_map + + +@pytest.mark.parametrize( + "link", + ["(flowchart.mmd)", "CLAUDE.md#tech-stack-mvp"], +) +def test_readme_has_no_dead_links(link): + assert link not in README + + +def test_readme_relative_links_resolve(): + for target in re.findall(r"\]\(([^)#]+)(?:#[^)]*)?\)", README): + if target.startswith(("http://", "https://", "mailto:")): + continue + assert (_ROOT / target).exists(), f"README links {target}, which does not exist" + + +def test_claude_md_anchors_into_itself_resolve(): + headings = { + re.sub(r"[^a-z0-9 -]", "", line.lstrip("# ").lower()).replace(" ", "-") + for line in CLAUDE_MD.splitlines() + if line.startswith("#") + } + for anchor in re.findall(r"\(CLAUDE\.md#([a-z0-9-]+)\)", README + CLAUDE_MD): + assert anchor in headings, f"anchor #{anchor} matches no CLAUDE.md heading"