davideandClaude Opus 5
526a649c8b
The flowchart's WITHDRAW node (E1) stated that a withdrawal cannot happen together with a bet in progress. The code only serializes the two *builds* through the per-user lock: a withdrawal is accepted while a bet is still unconfirmed, as long as confirmed, unspent UTXOs cover it. CLAUDE.md makes every node of the diagrams binding, so one of the two had to move, and it is the diagram. The hazard the node was reaching for is the two transactions picking the same UTXO, and that is already excluded twice: app/tx/locks.py keeps the builds from overlapping, and select_utxos skips anything already marked spent_txid. What the node forbade on top of that is spending untouched, confirmed money — so implementing it as written would freeze a user's whole balance for a block after every bet and protect nothing. E1 now describes the real rule, and CLAUDE.md's per-user-lock paragraph states it is the only exclusion between the two. Regenerated the A4/A3 PDFs (gitignored, so not in this commit). The regression test is behavioural, not a wording check: it funds a user with two confirmed UTXOs, bets (taking the larger), and asserts the withdrawal goes through on the other one with the bet still unconfirmed and neither transaction spending the other's input. A second test keeps the diagram from drifting back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
68 lines
3.2 KiB
Markdown
68 lines
3.2 KiB
Markdown
# BUGS.md — audit of 2026-08-03
|
|
|
|
Third full-codebase audit, opened after the 2026-07-26 (B-01 … B-24) and
|
|
2026-07-27 (B-25 … B-49) lists were emptied. Numbering continues from the last
|
|
fixed finding, B-51.
|
|
|
|
The list opened at B-52 … B-72 and holds only what is still **open**: a finding is
|
|
removed from this file once it is fixed, and is not listed here afterwards. 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, so
|
|
`git log --all --grep 'B-nn'` is the record of how any closed finding was closed.
|
|
|
|
State of the tree at audit time: 264 unit tests, all passing; `tests/integration/`
|
|
still empty; withdrawal and the RBF bump path still never live-broadcast.
|
|
|
|
Verified as *not* broken while looking for these: i18n key parity (146 identical
|
|
keys across all 7 languages), HTML escaping of every user-controlled value in
|
|
`admin.js`, the Alembic chain (linear, single head, matching `models.py`),
|
|
strictly-integer satoshi arithmetic everywhere, and `.gitignore` coverage of
|
|
secrets/DB/logs (nothing sensitive is tracked in git).
|
|
|
|
Severity is about consequence, not likelihood:
|
|
**critical** = money stuck or lost, or the lottery stops;
|
|
**high** = a security control that does not hold;
|
|
**medium** = wrong behaviour with a bounded blast radius;
|
|
**low** = drift between documentation and code.
|
|
|
|
---
|
|
|
|
## Critical
|
|
|
|
### (not new) `drawing` does not resume after a restart
|
|
|
|
Already tracked as an accepted gap in CLAUDE.md's "Known gaps", not re-numbered
|
|
here. Worth restating in context: with `restart: unless-stopped` on the app
|
|
container, this is the one state that gets stuck with money in play, and it
|
|
remains the last prerequisite for running unattended.
|
|
|
|
---
|
|
|
|
## Low — documentation and consistency drift
|
|
|
|
### B-71 — `.env` points `MASTER_KEY_PATH` at a second copy of the master key
|
|
|
|
CLAUDE.md's deployment section prescribes pointing `MASTER_KEY_PATH` at the
|
|
host-side `./data/keys/master.xprv.enc` so the venv scripts and the container read
|
|
one file. `.env` instead sets `./master.xprv.enc`, and both files now exist in the
|
|
working tree (both gitignored). They were verified during this audit to decrypt to
|
|
the *same* xprv, so nothing has diverged yet — but `scripts/decrypt_master_key.py`
|
|
reads a different file from the one the container uses, and a future
|
|
`generate_master_key.py --overwrite` would split them silently, with an ops
|
|
recovery path that then reports the wrong key.
|
|
|
|
The same duplication exists for the database (`./plm_lottery.db` next to
|
|
`data/db/plm_lottery.db`), which is less dangerous but equally confusing.
|
|
|
|
Fix: set `MASTER_KEY_PATH=./data/keys/master.xprv.enc` in `.env`, delete the stray
|
|
root copy once confirmed redundant, and state the same for `DATABASE_URL`.
|
|
|
|
### B-72 — `docs/setup.md` still frames setup as "locally or via Docker"
|
|
|
|
`docs/setup.md:1-10` lists Python as a prerequisite "for the local/venv workflow"
|
|
and describes the master-key step as local *or* Docker, while B-44 made Docker the
|
|
only supported way to run the server (`docs/running-the-server.md` and README were
|
|
updated, this file was not). The venv genuinely is needed for tests, migrations and
|
|
the key scripts — the wording just needs to say that instead of implying a second
|
|
way to run the server.
|