plm-lottery/BUGS.md

BUGS.md — open findings

Two audits currently have open entries: 2026-08-03 (B-71, B-72, below) and the 2026-09-28 specification review (B-73 … B-98, at the end of the file).

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.

---

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. 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.

Critical

B-73 — The draw can be steered by whoever produces or relays blocks

The seed is the block hash alone, so a miner holding tickets can withhold blocks until one makes them win, and the active Electrum server can pick which real block is used (it only has to skip announcing one; corroboration checks that the block exists, not that it is the first after closing). The participant order uses the server-only broadcast_at with no tie-break, so the draw is not reproducible.

Fix (commit-reveal): at round open generate a 32-byte server_seed, store it encrypted (server_seed_enc, Fernet with the master-xprv key, so a DB copy or backup alone never reveals it; the plaintext column is written only at reveal), and publish only server_seed_hash = sha256(server_seed). On entering drawing, fix the participant list ordered by confirmed bet txid (ascending), publish participants_hash = sha256 of the txid:vout:address lines, and fix draw_height = tip_at_close + 1 (persisted, never moved). Wait until the chain has draw_confirmations (round_config, default 3, snapshotted at open) blocks from draw_height, fetch the header at draw_height from every server, require the quorum to agree (on disagreement wait and re-ask for the same height). Seed = sha256(server_seed || block_hash || participants_hash) as an integer, index = seed mod n. Reveal server_seed once the winner is persisted, and expose the full proof at GET /rounds/{id}/proof.

B-74 — Headers are accepted without a minimum-difficulty check, and a jump skips the linkage check

header_meets_its_own_target never compares bits against the chain's powLimit, so a header with bits=0x207fffff passes with no work. A multi-block jump is not linked to the current tip. A fake header at a huge height locks tip_height (it never regresses) and freezes every draw. Conversely an ordinary 1-block reorg kills the session on every server until the next block.

Fix: reject bits easier than powLimit; on any advance of more than one block fetch the intermediate headers (blockchain.block.headers) and validate the chain; on a linkage mismatch walk back up to 10 headers to find the fork point and follow the reorg instead of ending the session (reorg handling: B-85).

B-75 — Replies over 64 KiB kill the connection; the pool's history grows past that

asyncio.open_connection uses the default 64 KiB StreamReader limit; a longer reply line raises ValueError in _read_loop and closes the connection. The confirmation poller and the reconciler fetch the pool address's full history every cycle, which passes 64 KiB after ~800 transactions: from then on every 10s poll drops the only connection and payouts never confirm. A few hundred dust outputs sent to any public address trigger the same thing on purpose.

Fix: 16 MiB reader limit; an oversized or malformed reply is a request error, not a dead connection; UTXOs below DUST_LIMIT_SATS are ignored for crediting and selection; per-round pool addresses (B-80) keep every address's history bounded.

B-76 — An ambiguous broadcast is rolled back as if refused

place_bet/request_withdrawal release everything on any broadcast exception, including the 15s timeout and a drop after the write. The tx may have propagated: the bet's sats land in the pool with no participant, the withdrawal leaves while its row says failed.

Fix: only an explicit JSON-RPC rejection can release, and only after verifying it: blockchain.transaction.get(txid) is asked of the active server and every fallback (10s each). Any server returning the tx → accepted; every responder denying it, with the active server and the quorum minimum among them → released; too few answers → uncertain (below). Matching "already known / already in mempool" messages is only a shortcut that skips the check, so an unrecognised message can never release a tx that is on the network (spec: TX-04). A timeout or disconnect leaves the rows building for the reconciler and the API answers 202 with the row in building. After the active server accepts, the raw tx is also sent to every fallback (best effort).

B-77 — Only the latest txid is tracked, and bump_fee broadcasts before it persists

The poller and the reconciler match current_txid only (replaced_by_txid keeps one step and nothing reads it). If an earlier version of a bumped tx confirms, it is never seen; after 6h the row is abandoned — a confirmed bet's participant deleted, a sent withdrawal marked failed, a paid payout rebuilt. bump_fee broadcasts in phase 2 and persists in phase 3, unconditionally, so a crash or a database is locked in between, or an abandon that ran meanwhile, desynchronises DB and chain.

Fix: a pending_transaction_txids table holding every txid (and raw tx) a row ever had; a confirmation of any of them confirms the row and retargets every reference to it. The replacement is written (as a candidate) before it is broadcast; phase 3 updates with WHERE status='pending' AND current_txid=<old> and treats zero rows as a conflict to reconcile.

B-78 — Abandonment rests on one server's silence

reconcile_once abandons when the active server's get_history lacks the txid — after 120s for building rows, which a restart onto a different server makes likely, and all at once after a forward clock jump. It re-checks status but not the txid, so it races the bumper. A failed row is never looked at again.

Fix: abandon only on proof — (a) an input confirmed-spent by a different txid (quorum), or (b) no server of the quorum knows any txid of the row and a rebroadcast of the latest raw tx to all servers is explicitly refused for missing/spent inputs. A fee-related refusal is not proof (bump instead). The re-check includes current_txid. failed rows stay watched for 14 days; a late confirmation writes pending_tx_resurrected and pauses the lottery for the operator.

B-79 — A payout normally has no change output, so it cannot be fee-bumped

The pool normally holds exactly the round's bet outputs, the payout spends them all for exactly pool_amount_sats, so there is no change and bump_fee can never help. A payout stuck in a mempool is also never abandoned (the server still knows it): the round stays in paying_out and no new round opens.

Fix: every payout carries a bump reserve output of max(DUST_LIMIT_SATS, estimated_vsize × 10 sat/vB), taken from the commission share and paid to the pool reserve address (m/84'/746'/0'/1/0, the old pool address); RBF shrinks that output. rounds_can_open refuses a configuration in which a round of 1 or of MAX_PARTICIPANTS_PER_ROUND participants could not cover winner fee + reserve + dust commission, and PUT /admin/config refuses a bet amount or fee rate that would fail the same check (spec: RND-40).

B-80 — One pool address for every round

All bets of all rounds pay one address: its history grows forever (B-75), stray money and other rounds' money mix with this round's, and a restored backup replays a round against whatever the pool holds now.

Fix: each round gets its own pool address m/84'/746'/0'/2/<round_id>, stored on the round (rounds.pool_address); bets pay it, the payout spends only its UTXOs. Rounds opened before this change keep the old address. Withdrawals to any pool, reserve or fee address are refused.

B-81 — drawing wedges on any exception, not only on restart

The participant snapshot and tip_at_close live only in memory; _tick skips drawing. Any exception during the wait (database is locked writing an audit row, a malformed header) leaves the round in drawing forever.

Fix: persist draw_height, participants_hash and each participant's draw_position when entering drawing; _tick re-enters the wait for a round in drawing, idempotently. Together with B-73's fixed height and committed seed, a resumed draw always yields the same winner.

B-82 — A deposit reorged out after a bet leaves the round unpayable

Deposits are credited at 1 confirmation. A user can bet, then (if they lost) double-spend the deposit the bet spent with a private chain: the bet disappears but its participant stays confirmed and counted, and the payout fails "insufficient pool UTXOs" forever with every other player's money stuck. Immature coinbase outputs are also credited at 1 confirmation (maturity is 120).

Fix: deposits credited at deposit_confirmations (round_config, default 6), coinbase outputs at 120. Before building a payout the round address's UTXOs are verified with the quorum; participants whose bet outpoint is gone are marked reorged_out and the 70/30 split is computed on what is actually there. If the winner's own bet is gone the payout is held (payout_blocked) and the lottery pauses.

High

B-83 — Confirmations and reinstatement trust the active server alone

poll_once marks a tx confirmed on one server's height > 0, and reinstate_reappeared_utxos restores a UTXO on its word, so a hostile active server can report a never-relayed payout as confirmed (winner unpaid) or a phantom bet. Broadcasts go to the active server only.

Fix: a confirmation is accepted once the quorum returns a valid merkle proof (blockchain.transaction.get_merkle) against a header we hold; reinstatement needs the same quorum as a credit (B-59); broadcasts fan out (B-76).

B-84 — The quorum can be one server, or none, silently

An empty ELECTRUM_FALLBACK_SERVERS (the .env.example default) disables every corroboration; with fallbacks, the majority is over whoever answers, so one reachable fallback decides. TLS certificates are not verified.

Fix: at least 2 fallbacks and at least 2 responders for every corroboration, or the server refuses to start (override ALLOW_NO_QUORUM=true, development only); optional certificate pinning per endpoint (host:port:pin=<sha256>).

B-85 — Reorgs are neither followed nor surfaced

Nothing detects a reorg; a 1-block reorg is treated as a hostile server (B-74).

Fix: the listener keeps the last 10 headers; a fork is followed to its fork point, a chain_reorg_detected audit row records depth and heights, and a depth ≥ 2 pauses the lottery (paused_reason = 'reorg') until the operator resumes it.

B-86 — A connected but stalled server is never replaced

ping keeps succeeding while the server stops following the chain; nothing compares tip progress with the other servers, and a restart goes back to the primary.

Fix: if the tip has not advanced for 360s and any other server reports a higher tip, rotate. GET /health/ready and /admin expose connection state, endpoint and last-header age (B-92).

B-87 — The RBF bump can leave a dust change output

bump_fee only requires change > fee delta. A 294-sat change (full-balance withdrawal) becomes ~153 sat after the minimum bump and every replacement is refused by relay, every 30s.

Fix: when a bump would leave change below DUST_LIMIT_SATS, the change output is dropped and its value goes to the fee (the replacement is then final); RbfError and every refused bump write a rbf_bump_failed audit row.

B-89 — Key problems surface only at first use

Startup checks only that the key file exists and XPRV_ENCRYPTION_KEY is non-empty; a wrong key or a different xprv is found at the first registration or payout, and a different xprv silently moves the pool address. The scripts accept any xprv and write files 0644.

Fix: at startup decrypt the key and check the pool reserve address and a sample of stored user addresses; refuse to serve on mismatch. The scripts refuse an xprv whose derived addresses differ from the DB's and write 0600.

B-90 — Restoring a backup is unsafe

A restored DB reopens rounds that were already drawn and paid, and reassigns the derivation indexes of users registered after the backup to new users — who then receive the old users' funds.

Fix: START_PAUSED=true starts with the lottery paused; at startup the next 20 indexes past the highest assigned are checked for on-chain history and, if any has some, registration is refused until the operator reconciles (restore_gap_detected); per-round addresses (B-80) and B-82's pre-payout check make a replayed round unpayable instead of payable twice.

B-91 — Host and supply-chain hardening

The container runs as root, .env and key files are world-readable, dependencies are unpinned >= and base images float, secrets sit in env vars visible to docker inspect.

Fix: non-root USER, 0600/0700 permissions, a hash-pinned lock file, digest-pinned base images, the Fernet key mounted as a file rather than an env var.

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 < 360s, 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-95 — Round parameters read live instead of snapshotted

Only duration and cooldown are snapshotted at open; bet amount, fee_address and the new draw_confirmations are read live, so an edit mid-round changes the round in progress (e.g. redirects its commission).

Fix: snapshot bet_amount_sats, fee_address and draw_confirmations on the round at open. Optional per-round cap max_round_pool_sats (round_config, 0 = disabled): once reached, further bets get round_full.

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).

B-97 — Migrations can silently drop the expression indexes

batch_alter_table on SQLite rebuilds the table and drops ix_rounds_single_active and ix_users_username_lower with only a warning; autogenerate ignores them; no test runs the migrations.

Fix: a test that runs alembic upgrade head on an empty DB and asserts both indexes exist; any batch migration on those tables recreates them explicitly.

B-98 — Coin selection stops at the first covering UTXO, refusing a payable bet

Found while writing the spec on 2026-09-28 (medium). select_utxos returns as soon as the selected total covers the target. When that happens exactly (a 10 PLM coin for a 10 PLM bet), the change is 0: the bet is refused with balance_leaves_no_change and a withdrawal is reduced by DUST_LIMIT_SATS, even though a second coin would have left a proper change output.

Fix: select until the total covers target + DUST_LIMIT_SATS (still largest-first, still capped at max_inputs); only when every allowed coin together cannot reach it fall back to today's behaviour (refuse the bet, reduce the withdrawal). Spec: docs/scommesse-e-prelievi.md BET-24.