plm-lottery/BUGS.md

# Known bugs and required fixes

Full-codebase audit performed on 2026-07-26 against commit `d4e0974` (branch `main`),
covering every Python module under `app/`, both static frontends, and the Docker/Caddy
deployment. The test suite was green at the time of the audit (79 passed), so **none of
the findings below are caught by the existing tests** — every one of them needs a
regression test alongside its fix.

> **Status: 23 of the 24 findings are fully fixed on `main`** as of 2026-07-27, and the
> 24th ([B-16](#b-16)) is fixed as far as the crash goes — shipping the user guide in the
> image was deliberately deferred. Every entry below keeps its original description — the bug, why it
> mattered, and how it was meant to be fixed — and carries a **Fixed:** note recording
> what was actually done and where the regression test lives. The suite grew from 79 to
> 137 tests. Two entries were fixed differently from the plan (B-15 validates at startup
> rather than in a Pydantic validator, B-09 gained a bounded retry); both say so and why.
> The fixes were then run in the real Docker deployment — see
> [Runtime verification](#runtime-verification-2026-07-27) for what that actually
> confirmed and, more importantly, what it did **not**.

Findings are ordered by severity. Each entry is self-contained and follows the same shape:
symptom, root cause with `file:line` references, a `*Proposed fix:*` block, and a `*Test:*`
block naming the regression test to add (a few are marked manual where an automated test
would be testing the Docker image layout or the browser, not the code). Items already listed
under "Known gaps / TODO" in `CLAUDE.md` are cross-referenced rather than repeated, except
where this audit found the gap to be worse than documented.

## Summary

| ID | Severity | Area | One-line |
|----|----------|------|----------|
| [B-01](#b-01) | Critical | Electrum client/listener | A dropped connection hangs the whole server permanently — no reconnect, no request timeout, no keepalive |
| [B-02](#b-02) | Critical | RBF / bets | A fee bump on a bet tx orphans `RoundParticipant.bet_txid`, wedging the round in `closing` forever |
| [B-03](#b-03) | Critical | Confirmation poller | One unresolvable txid aborts confirmation detection for every other pending tx |
| [B-04](#b-04) | Critical | Balance / UTXO state | No reconciliation path: a dropped tx freezes the spent UTXOs (and the user's funds) forever |
| [B-05](#b-05) | Critical | Admin config / payout | `fee_address` and numeric params are unvalidated; a bad value either wedges the payout or sends the commission to an unspendable script |
| [B-06](#b-06) | High | PSBT builder | Change outputs below the dust limit are created, making the transaction unrelayable |
| [B-07](#b-07) | High | Bets / withdrawals | A broadcast rejection surfaces as HTTP 500, bypassing the structured error contract |
| [B-08](#b-08) | High | Bets | Broadcast happens before the bet is persisted: a commit failure loses the money with no record |
| [B-09](#b-09) | High | Rounds | Race allows two simultaneously-`open` rounds, which permanently blocks all future rounds |
| [B-10](#b-10) | High | Admin / audit | `PUT /admin/config` writes no audit-log entry |
| [B-11](#b-11) | High | Rounds API | The displayed jackpot is not the amount the winner receives |
| [B-12](#b-12) | High | Auth | Registration enforces no password or username validation, unlike change-password |
| [B-13](#b-13) | Medium | Auth | `verify_password` turns a malformed stored hash into a 500 instead of a 401 |
| [B-14](#b-14) | Medium | Admin auth | Admin token compared with `!=` instead of a constant-time comparison |
| [B-15](#b-15) | Medium | Config | Empty `JWT_SECRET` / `XPRV_ENCRYPTION_KEY` are not rejected at startup |
| [B-16](#b-16) | Medium | Deployment | `GET /guida` is broken in Docker: `docs/` is never copied into the image (crash fixed; shipping the guide deferred — see entry) |
| [B-17](#b-17) | Medium | RBF | `_find_change_output` can shrink the recipient output instead of the change |
| [B-18](#b-18) | Medium | Scheduler | DB session held open across Electrum network calls during payout |
| [B-19](#b-19) | Medium | Electrum listener | `tip_height` is assigned without a monotonicity check (reorg) |
| [B-20](#b-20) | Low | RBF | `PendingTransaction.replaced_by_txid` is never written — dead column shown in the admin UI |
| [B-21](#b-21) | Low | Confirmation poller | Dead local `pending_ids`, plus attribute access on detached ORM objects |
| [B-22](#b-22) | Low | Frontend | Amounts rendered by raw division — floating-point artefacts visible to users |
| [B-23](#b-23) | Low | Frontend | Concurrent `withLoading` on the same button can leave it stuck on its loading label |
| [B-24](#b-24) | Low | API contract | The global exception handler answers with a bare-string `detail` |

---

## Runtime verification (2026-07-27)

The fixed code was built and run via `docker compose up -d --build` against mainnet. This
section separates what the running system **demonstrated** from what is still only covered
by unit tests — the distinction matters, because a green suite is not a working deployment.

**Caveat on the image that produced this evidence:** it was built while the `Dockerfile`
still carried `COPY docs ./docs`, which was subsequently reverted (see [B-16](#b-16)). The
running container therefore does *not* match the current Dockerfile: `/guida` answers 200
there, and will answer 404 after the next rebuild. Everything else below is unaffected.

### Confirmed at runtime

| Fix | Evidence from the live system |
|---|---|
| [B-15](#b-15) | The app serves traffic, so `validate_runtime_secrets()` passed against the real `.env` — and it is now on the startup path of every deploy. |
| [B-01](#b-01) (partly) | `app.main: Electrum endpoints (in rotation order): santantonio.sytes.net:50002` followed by `app.electrum.listener: Electrum connected to santantonio.sytes.net:50002` — the restructured session setup (`asyncio.wait` over the consumers, `wait_closed()` and the keepalive) connects and stays up against a real server. Only one endpoint is configured, so rotation itself is still only unit-tested. |
| [B-09](#b-09) | `ix_rounds_single_active` exists in the deployed DB and rounds keep opening and closing normally (196 rounds, one per ~98s cycle) — the index and the conflict-retry path do not interfere with normal round creation. |
| [B-04](#b-04) (schema) | The `failure_reason` column is present in the deployed DB; migration `8a1c4e7b2d90` applied cleanly to the live database. |
| [B-04](#b-04) (heuristic) | The one genuinely unknown piece — whether `_tx_exists_on_chain` recognizes a missing tx on *this* server — was tested directly: for a bogus txid the server answers `No such mempool or blockchain transaction`, which the substring check matches, so the function correctly returns `False`. The conservative direction is preserved: any message it does *not* recognize re-raises and leaves the row untouched. |
| [B-16](#b-16) | Confirmed as a bug that had really been firing, not a theoretical one: `logs/app.log` holds two pre-fix `ERROR [app.main] Unhandled error on GET /guida` entries (17:40 and 19:42 on 2026-07-26) ending in `RuntimeError: File at path docs/guida-utente.md does not exist.` |
| No regressions | Zero `ERROR`/`WARNING` lines since the restart on the fixed code. The only errors in the entire log predate it (the two `/guida` failures above, plus two unrelated `asyncio` entries from 2026-07-22). |

### Still unverified outside the test suite

Nothing has spent money since the restart: `pending_transactions` holds only rows that were
already `confirmed` beforehand, `withdrawals` is empty, and no `attempt_count` is above 1.
So these fixes have unit coverage and **no runtime evidence at all**:

- [B-08](#b-08)/[B-07](#b-07) — the two-phase write and the rollback-on-refusal need a real
  bet, and a genuinely rejected broadcast, to be seen working.
- [B-02](#b-02)/[B-20](#b-20) — needs an actual RBF bump (`rbf_timeout_seconds` must elapse
  with the tx unconfirmed).
- [B-03](#b-03)/[B-04](#b-04) (behaviour) — needs a transaction that really goes missing.
  The reconciler has been ticking every 120s with nothing to do, which is silent by design.
- [B-06](#b-06) — needs a UTXO set that produces sub-dust change.
- [B-17](#b-17) — needs a withdrawal attempt; the withdrawal path as a whole has still never
  been exercised against a live broadcast, which was already true before this audit.
- [B-22](#b-22)/[B-23](#b-23) — browser-side, verifiable only by using the UI.

### What remains open (not part of the 24)

Unchanged by this work, and still the reason this isn't unattended-safe:

- **Scheduler doesn't resume mid-flight rounds after a restart** — a round left in
  `closing`/`drawing`/`paying_out` when the process dies stays stuck. Transaction-level
  state now self-heals ([B-04](#b-04)); *round*-level state does not.
- **No automatic payout retry** — a failed payout is audit-logged (`payout_failed`, added by
  [B-05](#b-05)) and visible in `/admin`, but acting on it is manual.
- **RBF still handles only the single-change-output case** — the difference is that failure
  is no longer permanent: an unbumpable tx is eventually abandoned and its coins released.
- **`/guida` is not served in Docker** by current deliberate decision — the guide is being
  reworked; re-adding `COPY docs ./docs` is all it will take.
- **No rate limiting anywhere**, **admin auth is one shared token** with no per-operator
  identity, and **single-process assumptions** remain in the SSE broadcaster and the
  per-user locks.
- **No automated integration tests against a live Electrum connection** — the verification
  in this section was done by hand.

---

## Critical

### B-01

**A dropped Electrum connection hangs the entire server permanently**

*Files:* `app/electrum/client.py:31-63`, `app/electrum/client.py:85-99`, `app/electrum/listener.py:58-77`

Three independent defects compose into a single absorbing failure state:

1. **The read loop's death never propagates.** `connect()` spawns `_read_loop` as a
   detached task. When the socket closes, `readline()` returns `b""`, the loop breaks and
   the task completes — but `_run_once` is blocked on
   `asyncio.gather(self._consume_headers(...), self._consume_scripthash(...))`, and both
   consumers are awaiting `asyncio.Queue.get()` on queues nobody will ever fill again.
   The `gather` never returns and never raises, so the `except Exception` +
   backoff-reconnect logic in `ElectrumListener.run()` is never reached.
2. **`self.client` stays non-`None`.** Every consumer of the client
   (`RoundScheduler`, `ConfirmationPoller`, `RbfBumper`, `place_bet`,
   `request_withdrawal`, and the `listener.client is None` guards in the bet/withdrawal
   routes) therefore keeps treating a dead connection as live.
3. **`request()` has no timeout.** It registers a future in `self._pending`, writes to a
   closed writer (`drain()` frequently does not raise on a half-closed socket) and then
   `await future` — which nothing will ever resolve, because `_read_loop` is gone. Note
   that `_read_loop`'s `finally` only fails the futures that existed at the moment it
   died; every future created afterwards hangs forever.

There is also **no keepalive** (`server.ping`) anywhere. Electrum servers routinely drop
idle connections after ~10 minutes, so on a low-traffic instance this is close to
guaranteed rather than hypothetical.

*Observable consequences:* deposits stop being credited; the tip stops advancing, so a
round in `drawing` waits in `_wait_for_next_block` forever; `ConfirmationPoller` blocks
on its first `get_transaction` and stops polling entirely; a `POST /bets` request hangs
indefinitely **while holding that user's `UserLocks` entry**, so every later bet or
withdrawal from the same user deadlocks behind it.

*Reproduction:* start the app, kill the TCP connection out from under it
(`ss -K dst <electrum-host>`, or block the port with a firewall rule). No reconnect log
line is emitted, `/rounds/current` keeps reporting the last known `chain_tip_height`, and
a bet request never returns.

*Proposed fix:*
- Wrap every `request()` in `asyncio.wait_for(...)` with an explicit timeout (10-15s) and
  pop the future from `self._pending` on timeout.
- Add a keepalive task issuing `server.ping` every ~60s; a failed ping tears the
  connection down.
- Make the read loop's termination observable: either include `self._read_task` in
  `_run_once`'s `gather`, or set an `asyncio.Event` in `_read_loop`'s `finally` that
  `_run_once` awaits alongside the consumers. On teardown, set `self.client = None`
  *before* awaiting the reconnect backoff.

*Test:* a unit test with a fake stream that closes mid-session must observe
`listener.client is None` and a reconnect attempt; a test that `request()` raises
`ElectrumError`/`TimeoutError` rather than hanging when no response arrives.

**Fixed.** `app/electrum/client.py` now bounds every `request()` with
`asyncio.wait_for` (`_REQUEST_TIMEOUT_SECONDS = 15`), pops the orphaned future and tears
the connection down on timeout; `_read_loop`'s `finally` sets a `_closed` event, exposed
as `wait_closed()`; and `ping()` was added. `app/electrum/listener.py:_run_once` races the
two notification consumers against `wait_closed()` and a 60s keepalive with
`asyncio.wait(FIRST_COMPLETED)`, so a drop ends the session and `run()` reconnects.
`self.client` is cleared before the reconnect, so callers stop using a dead client.
Beyond the original finding, the listener now **rotates over a list of servers**
(`ELECTRUM_FALLBACK_SERVERS`, `parse_endpoints`): one dead server costs a single attempt
instead of an outage, and the backoff only sleeps once every server has had a turn.
Tests: `tests/unit/test_electrum_client.py` (timeout, `wait_closed`, pending-request
failure, endpoint parsing), `tests/unit/test_electrum_listener.py` (rotation, backoff
reset, no-endpoints case).

### B-02

**An RBF fee bump on a bet permanently wedges the round**

*Files:* `app/tx/broadcast.py:116-121`, `app/bets/confirmation.py:10-16`,
`app/rounds/scheduler.py:77-86`, `app/rounds/scheduler.py:92-98`

`bump_fee` rebuilds the transaction, which changes its txid, and updates
`PendingTransaction.current_txid` accordingly. Nothing updates the domain row's copy of
that txid: `RoundParticipant.bet_txid` is written exactly once, at
`app/bets/service.py:80`, and never again (verified by grepping every write to
`bet_txid`).

When the bumped transaction confirms, `_on_bet_confirmed` looks the participant up by
`RoundParticipant.bet_txid == pending.current_txid`, finds nothing, and returns silently.
Consequences, in order:

- the participant stays `status == "broadcast"` forever;
- `RoundScheduler._tick` counts participants in `"broadcast"` before allowing the round to
  close, so the round stays in `closing` **indefinitely**;
- since a new round cannot open while one is active (`get_active_round`), the entire
  lottery stops;
- even if the round were forced closed, `_close_and_draw` only selects
  `status == "confirmed"` participants, so that player would have paid the bet without
  entering the draw.

The same staleness affects `Withdrawal.txid`. It is less severe there, because
`_on_withdrawal_confirmed` keys off `pending.withdrawal_id`, so the status does update —
but the txid stored and shown to the user is the replaced one.

*Reproduction:* place a bet, let `rbf_timeout_seconds` elapse without confirmation so
`RbfBumper` bumps it, then let the bumped tx confirm. `round_participants.status` remains
`broadcast`; the round never leaves `closing`.

*Proposed fix:* make the bump update the domain row too. Either have `bump_fee` write
through to `RoundParticipant.bet_txid` / `Withdrawal.txid` (it already knows `kind`,
`user_id`, `round_id`, `withdrawal_id`), or — cleaner — stop keying confirmation handlers
on the txid at all: `_on_bet_confirmed` should resolve the participant via
`pending.round_id` + `pending.user_id`, which are immutable. Also set
`replaced_by_txid` (see [B-20](#b-20)) so the chain of replacements is auditable.

*Test:* bump a pending bet, then run the confirmation handler with the new txid and assert
the participant flips to `confirmed`.

**Fixed.** Both confirmation handlers now resolve their domain row by immutable
ids — `_on_bet_confirmed` by `(round_id, user_id)`, `_on_payout_confirmed` by `round_id` —
with the old txid lookup kept only as a fallback for pre-existing rows. Independently,
`bump_fee` calls a new `_retarget_txid_references()` that updates
`RoundParticipant.bet_txid`, `Withdrawal.txid`, `Round.payout_txid` **and**
`UtxoEvent.spent_txid` to the new txid, so no stored txid is left pointing at a
transaction that no longer exists. Tests:
`test_confirmation.py::test_bet_confirms_after_an_rbf_bump_changed_the_txid` (plus the
payout equivalent), `test_broadcast.py::test_bump_fee_retargets_every_stored_txid`.

### B-03

**One unresolvable txid stops confirmation detection for everything else**

*File:* `app/tx/confirmation.py:35-37`

```python
for pending_id, txid, kind in [...]:
    tx = await client.get_transaction(txid, verbose=True)   # not guarded
```

If a txid is no longer knowable by the server — dropped from the mempool, replaced by an
RBF bump whose old entry is still `pending`, or simply an Electrum server that answers
with an error — `ElectrumError` propagates out of `poll_once` and **aborts the loop before
the remaining pending transactions are checked**. `ConfirmationPoller.run` logs it and
retries every 10s, failing at the same row every time.

Because the poller is the single mechanism that confirms bets, payouts and withdrawals,
one stuck row means: no bet ever confirms again (so no round ever closes — see
[B-02](#b-02)), no payout ever completes, no withdrawal ever settles. Recovery requires
manual DB surgery.

*Proposed fix:* wrap the per-transaction lookup in `try/except Exception`, log at
`warning` with the txid, and `continue`. Optionally track consecutive failures per row so
a permanently-unknown tx can be escalated to an operator (which is also the hook
[B-04](#b-04) needs).

*Test:* `poll_once` with two pending rows where the first raises must still confirm the
second and return `1`.

**Fixed.** The per-transaction lookup in `poll_once` is wrapped in
`try/except Exception`: it logs the txid at warning level and continues, leaving the
judgement about a permanently-unknown tx to the reconciler ([B-04](#b-04)). Test:
`test_confirmation.py::test_one_unresolvable_txid_does_not_block_the_others`, which
asserts the healthy row still confirms and the unknown one is left `pending` rather than
abandoned here.

### B-04

**No reconciliation: a dropped transaction freezes the spent UTXOs forever**

*Files:* `app/bets/service.py:69-73`, `app/withdrawals/service.py:65-68`,
`app/wallet/balance.py:9-18`

`spent_txid` is set optimistically at broadcast time and is **never cleared anywhere in
the codebase**. `recompute_balance` only sums UTXOs with `spent_txid IS NULL`, so those
coins are permanently excluded from the user's balance.

If the transaction never confirms and eventually disappears from the mempool — which
`CLAUDE.md` already acknowledges is a realistic outcome, since the RBF path raises
`RbfError` and gives up whenever there is no change output large enough to absorb the
bump — the UTXOs remain perfectly spendable on-chain while the database considers them
gone. The user's funds are silently lost from their point of view.

More broadly, nothing in the system ever writes a terminal failure state: there is no
`status = "failed"` on `PendingTransaction`, `Withdrawal` or `RoundParticipant`, and no
startup routine that re-scans in-flight transactions against the chain. Combined with
`restart: unless-stopped` and the already-documented scheduler-resume gap, the system has
several states it can only be pulled out of by hand.

*Proposed fix:* add a reconciliation task (and run it once at startup) that, for every
`PendingTransaction` in `pending` older than some threshold, asks the chain whether the
tx exists. If it is gone:
- mark the row `failed`, and set `replaced_by_txid`/a failure reason;
- clear `spent_txid` on the UTXOs it consumed (they are identifiable by parsing
  `raw_tx_hex`'s inputs);
- call `recompute_balance`;
- roll the domain row back (delete the `RoundParticipant`, mark the `Withdrawal`
  `failed`) and audit-log the event.

*Test:* given a pending bet whose tx is unknown to the chain, the reconciler must restore
the user's balance and remove the participant.

**Fixed** by a new component, `app/tx/reconcile.py`
(`PendingTransactionReconciler`, started from the lifespan and running every 120s,
including once at startup). Per non-terminal `PendingTransaction` it asks the chain
whether the tx exists: a `building` row whose tx is there gets promoted, a row whose tx is
gone is marked `failed` with a `failure_reason` (new column), its inputs released
(`spent_txid` cleared, only where it still matches this row's txid), the balance
recomputed, and the domain row rolled back — participant deleted, withdrawal marked
`failed`, payout txid cleared. Grace periods differ by state (120s for `building`, 6h for
`pending`) and a transport failure never abandons anything. Tests:
`tests/unit/test_reconcile.py` (6 cases, including the "broken connection must not release
coins" one).

### B-05

**`fee_address` and the numeric config params are unvalidated**

*File:* `app/api/routes/admin.py:70-80`, consumed at `app/rounds/scheduler.py:162-198`

`PUT /admin/config` assigns whatever it is given straight onto the `RoundConfig` row. Two
distinct failure modes:

1. **Malformed `fee_address`** → `build_payout_transaction` calls
   `script.Script.from_address(fee_address)`, which raises `EmbitError`. `_trigger_payout`
   only catches `InsufficientFundsError`, so the exception escapes through
   `_close_and_draw` up to the `except Exception` in `RoundScheduler.run()`. The round is
   left in `paying_out` with no retry (the payout-retry gap in `CLAUDE.md`), i.e. wedged.
2. **Well-formed but foreign `fee_address`** (e.g. a Bitcoin `bc1...`) → this is *worse*,
   because it parses fine into a valid witness program. The payout is built, signed and
   broadcast, and the 30 % commission of every round lands on a script nobody holds the
   key for. This is exactly the failure mode `app/wallet/address.py` was written to
   prevent for user withdrawals; the check was never applied to the operator-supplied
   address.

Numeric fields are equally unguarded: `round_duration_seconds = 0` (round expires the
instant it opens), `fee_rate_sat_vb = 0` (fee-less transactions are never relayed, so
every bet/payout hangs and the whole pipeline stalls), negative `bet_amount_sats`,
`draw_animation_seconds` far larger than the round itself.

*Proposed fix:* validate in `RoundConfigUpdate` (Pydantic `field_validator` /
`Field(gt=0)`):
- `fee_address` must pass `is_valid_plm_address`;
- `bet_amount_sats`, `round_duration_seconds`, `fee_rate_sat_vb`, `rbf_timeout_seconds`
  strictly positive, with sane upper bounds;
- `round_cooldown_seconds`, `draw_animation_seconds` non-negative.

Independently, `_trigger_payout` should catch `Exception` around the build/broadcast,
audit-log the failure, and leave the round in a state a retry routine can pick up.

*Test:* `PUT /admin/config` with a `bc1...` fee address and with `fee_rate_sat_vb = 0`
must both return 422/400 and leave the stored config untouched.

**Fixed.** `RoundConfigUpdate` now validates: `fee_address` must pass
`is_valid_plm_address`, and every numeric field carries bounds (`fee_rate_sat_vb >= 1`,
`round_duration_seconds >= 30`, etc.). Separately `_trigger_payout` catches `Exception`
around the build/broadcast, logs it and writes a `payout_failed` audit entry so a stuck
round is visible in `/admin` rather than only in the log file. Automatic payout retry
remains an open pre-existing gap (documented in CLAUDE.md), unchanged by this fix. Test:
`test_admin.py::test_config_rejects_unusable_values`, parameterized over the `bc1…`
address, a broken checksum, `fee_rate_sat_vb=0`, `round_duration_seconds=0` and more.

---

## High

### B-06

**Dust change outputs are created**

*File:* `app/wallet/psbt_builder.py:98-99` and `:166-167`

Both builders use `if change > 0:` as the sole condition for adding the change output. Any
leftover below the P2WPKH dust threshold (~294 sat at the standard 3000 sat/kvB dust
relay fee) produces a transaction that relaying nodes reject outright as `dust`. The
resulting `ElectrumError` is unhandled (see [B-07](#b-07)), so the user gets an opaque
HTTP 500 and cannot bet or withdraw at all until their UTXO set happens to change.

*Proposed fix:* introduce a `DUST_LIMIT_SATS` constant; add the change output only when
`change >= DUST_LIMIT_SATS`, otherwise leave the remainder to the fee (and reflect that in
the returned `fee_sats`/`change_sats` so the accounting stays honest). Note the fee
estimate at `psbt_builder.py:86` already assumes two outputs unconditionally, so dropping
change does not underpay.

*Test:* build a transaction whose change lands at 100 sat and assert a single-output tx
with the remainder folded into the fee.

**Fixed.** `DUST_LIMIT_SATS = 294` added to `app/wallet/psbt_builder.py`; both
builders fold sub-dust change into the fee instead of creating the output, and refuse a
sub-dust recipient/winner/commission amount with a dedicated error code. Tests in
`test_psbt_builder.py`: dust change folded into the fee (asserting nothing vanishes —
inputs still equal outputs plus fee), change exactly at the limit still paid back, and a
dust-sized recipient refused.

### B-07

**Broadcast failures surface as HTTP 500**

*Files:* `app/bets/service.py:67`, `app/withdrawals/service.py:63`

`await client.broadcast(built.raw_hex)` is not guarded. Every node-side rejection —
fee below the relay minimum, dust output ([B-06](#b-06)), mempool conflict, non-standard
tx — raises `ElectrumError`, which the global handler in `app/main.py:74-77` turns into
`{"detail": "internal server error"}`. That bypasses the whole error contract documented
in `app/api/errors.py`: no `code`, nothing translatable, no actionable message.

*Proposed fix:* wrap the broadcast, and raise
`BetError("broadcast_failed", ...)` / `WithdrawalError("broadcast_failed", ...)` carrying
the node's message in `params`. Add `error.broadcast_failed` to all 7 languages in
`app/static/i18n.js` (per the i18n contract in `CLAUDE.md`). Answer 502/503 rather than
400, since the failure is not the client's fault.

*Test:* `place_bet` / `request_withdrawal` with a client stub whose `broadcast` raises must
raise `BetError`/`WithdrawalError` with code `broadcast_failed` — and must leave no
`spent_txid`, no participant and no pending row behind.

**Fixed.** Both services wrap the broadcast and raise
`BetError`/`WithdrawalError("broadcast_failed", …)`; the routes answer **502** rather than
400, since the network refused it, not the caller. `error.broadcast_failed` was added to
all 7 languages in `i18n.js` (along with `amount_below_dust_limit`,
`withdrawal_to_own_address`, `internal_error`, `guide_unavailable`) — key parity verified,
123 keys per language. Tests:
`test_bets.py::test_failed_broadcast_reports_the_broadcast_failed_code` and the withdrawal
equivalent.

### B-08

**Bets are broadcast before they are persisted**

*File:* `app/bets/service.py:67-94`

The order today is: broadcast → mutate `spent_txid` → insert `RoundParticipant` +
`PendingTransaction` → `commit()`. If the commit fails, or the process dies in that
window, the transaction is already irreversibly on the chain: the money has moved to the
pool address, but **no record of the bet exists** — no participant (so no entry in the
draw), no pending row (so no RBF, no confirmation tracking), and the UTXOs are not even
marked spent, so the next bet attempt will try to double-spend them.

The same shape exists in `request_withdrawal`, where the `Withdrawal` row is flushed
before broadcast but only committed after.

*Proposed fix:* persist the intent first. Insert the `PendingTransaction` (and the
`RoundParticipant`) in a `building`/`unbroadcast` state, commit, then broadcast, then flip
to `broadcast` in a second commit. A crash between the two leaves a row the
reconciliation task from [B-04](#b-04) can resolve against the chain in either direction.

*Test:* with a client stub that broadcasts successfully but a session whose second commit
raises, the `PendingTransaction` must still exist afterwards (in its pre-broadcast state)
rather than the bet vanishing entirely.

**Fixed.** Both `place_bet` and `request_withdrawal` are now two-phase: write
the rows in a `building` state and commit, *then* broadcast, then promote to
`broadcast`/`pending` in a second commit. A failed broadcast runs a release helper that
frees the reserved UTXOs, restores the balance, removes the participant (or marks the
withdrawal `failed`) and audit-logs it. A crash between the phases leaves a `building` row
for the reconciler ([B-04](#b-04)) to resolve either way. Tests:
`test_bets.py::test_bet_is_persisted_before_it_is_broadcast` — which probes committed state
from an independent session *during* the broadcast, and which caught a real mistake in the
first draft of this fix (the `_pending_transaction` helper still hardcoded
`status="pending"`, so rows were born already-broadcast and would have got the reconciler's
6-hour grace instead of 120s) — plus the two rollback tests.

### B-09

**Two rounds can be opened concurrently, blocking every future round**

*File:* `app/rounds/service.py:44-60`

`open_new_round_if_needed` performs a read (`get_active_round`) followed by an insert with
no lock and no database-level uniqueness guarantee, and it is called both from
`RoundScheduler._tick` and from **every** `place_bet`. Two concurrent callers can both
observe "no active round" and both insert a row with `status="open"`.

`get_active_round` then quietly hides the problem — `select(...).order_by(Round.id.desc())`
+ `scalar()` returns only the newest — while the older round stays `open` forever, never
ticked, never closed. Since `get_active_round` matches on status, no further round can
ever open once the newer one closes: the lottery stops for good.

*Proposed fix:* the simplest robust option is to make the `RoundScheduler` the only writer
and have `place_bet` merely read the active round (rejecting the bet if there is none —
the scheduler opens one within 5s anyway). Additionally add a DB guard: a partial unique
index over `status IN ('open','closing','drawing','paying_out')`, or an advisory/serialized
transaction around the read-then-insert. `get_active_round` should also log loudly if it
ever sees more than one row.

*Test:* two concurrent `open_new_round_if_needed` calls on separate sessions must yield
exactly one round.

**Fixed** at the database level: `ix_rounds_single_active`, a unique index over
the constant expression `(1)` restricted to the active statuses (model +
migration `8a1c4e7b2d90`, which first closes any pre-existing duplicates, keeping the
newest — verified against a DB seeded with two active rounds). `open_new_round_if_needed`
catches the resulting `IntegrityError`, rolls back and returns the winner's round.
**Deviation from the plan:** rather than making the scheduler the only writer (which would
have meant the first bet after a cooldown couldn't open a round), it keeps both callers and
adds a bounded retry — a conflict where nothing is active yet means the winner simply
hadn't committed, and a bet must not fail on that timing. `get_active_round` also logs
loudly if it ever sees more than one active round. Tests in `test_rounds_service.py`: the
race-recovery path (forced deterministically by making the first look miss the existing
round — real concurrency on a shared in-memory SQLite connection isn't isolated enough to
test this honestly), the DB refusing a second active round, and closed rounds coexisting
with an active one.

### B-10

**`PUT /admin/config` writes no audit-log entry**

*File:* `app/api/routes/admin.py:70-80`

`/admin/pause` and `/admin/resume` both call `write_audit_log`; the config update — which
can change `fee_address` (where 30 % of every pool goes), `bet_amount_sats` and the fee
rate — writes nothing. This contradicts `CLAUDE.md`, which states that `audit_log`
records *what* changed (only the *who* is documented as missing).

Secondary issue: `paused` is part of `_CONFIG_FIELDS`, so it can be toggled through
`PUT /config`, silently bypassing the deliberately audit-logged `pause`/`resume`
endpoints.

*Proposed fix:* write a `config_updated` audit entry containing the changed fields as
`{field: {"from": old, "to": new}}` — computed before assignment. Remove `paused` from
`_CONFIG_FIELDS` (keep it in the response model) so the pause switch has exactly one
audited path.

*Test:* `PUT /admin/config` changing `fee_address` must produce exactly one
`config_updated` audit row carrying both the old and the new value; `paused` must be
rejected (or ignored) on that endpoint.

**Fixed.** `PUT /admin/config` computes a before/after diff and writes a
`config_updated` audit entry (`{field: {"from": …, "to": …}}`), skipping no-op updates.
`paused` was removed from `_CONFIG_FIELDS` so the maintenance switch has exactly one
audited path (`/admin/pause`, `/admin/resume`); it remains in the response model. Tests:
`test_admin.py::test_config_update_is_audit_logged`, the no-op case, and
`test_pause_cannot_be_toggled_through_the_config_endpoint`.

### B-11

**The displayed jackpot is not what the winner receives**

*File:* `app/api/routes/rounds.py:118-144`

```python
pool_amount_sats = participant_count * config.bet_amount_sats
jackpot_sats = pool_amount_sats * 70 // 100
```

Three independent divergences from the amount actually paid out:

1. The real pool is `sum(p.bet_amount_sats)`, and each participant's `bet_amount_sats` is
   `recipient_sats` — the bet amount **minus that bet's network fee**
   (`psbt_builder.build_signed_transaction`).
2. The payout deducts its own network fee from the winner's 70 % share
   (`build_payout_transaction`), so the winner receives strictly less than 70 % of the
   pool.
3. `participant_count` counts every `RoundParticipant` row, including bets still in
   `broadcast` that may never confirm — and those are excluded from the draw and the pool
   by `_close_and_draw`.

Plus: it multiplies by the *current* `bet_amount_sats`, which an operator can change
mid-round, retroactively changing the advertised jackpot of a round already in progress.

The in-code comment explicitly promises "what's displayed should match what the winner
actually receives", which is the opposite of the behaviour.

*Proposed fix:* compute the pool from the participants' stored `bet_amount_sats`
(optionally restricted to `confirmed`), derive the 70 % share from that, and either
subtract an estimated payout fee or relabel the field as an estimate in the UI. Keep
`participant_count` as-is for display but consider exposing `confirmed_count` separately.

*Test:* with two confirmed participants whose stored `bet_amount_sats` is below the
configured `bet_amount_sats` (fee already deducted), `GET /rounds/current` must report a
`jackpot_sats` derived from the stored amounts, and must not change when
`RoundConfig.bet_amount_sats` is edited mid-round.

**Fixed.** `GET /rounds/current` now sums the participants' stored
`bet_amount_sats` (each already net of its own bet's fee) instead of multiplying
`participant_count` by the current configured amount. The remaining, unavoidable
imprecision — the payout tx's own fee, deducted from the winner's share and unknowable
until the payout is built — is documented in the code rather than silently promised away.
Test: `test_rounds_route.py::test_jackpot_comes_from_the_participants_actual_bets`, which
also asserts that editing `bet_amount_sats` mid-round no longer moves a running round's
jackpot.

### B-12

**Registration has no password or username validation**

*File:* `app/auth/routes.py:18-56`

`POST /users/me/change-password` enforces `_MIN_PASSWORD_LENGTH = 8`
(`app/api/routes/users.py:15`), and the frontend re-checks it. `POST /auth/register`
enforces nothing: an empty username and a one-character password are both accepted, and
there is no `minlength` on the registration inputs in `app/static/index.html` either. On a
custodial system holding real funds, this is the wrong default.

Secondary defect in the same handler: the `_MAX_REGISTER_RETRIES` loop catches *any*
`IntegrityError`. A username collision that slips past the pre-check (concurrent
registration) is therefore retried five times and finally reported as
`derivation_index_conflict`, which is misleading for both the user and the operator.

*Proposed fix:* validate `username` (length, allowed character set) and `password`
(shared minimum-length constant, reused by `users.py`) in `RegisterRequest`; mirror the
constraints in the HTML form. Inspect the `IntegrityError` and re-raise `username_taken`
when it is the username constraint that failed.

*Test:* `POST /auth/register` with an empty username and with a 3-character password must
both be rejected (422/400) and create no user; a registration racing an existing username
must answer `username_taken`, not `derivation_index_conflict`.

**Fixed.** `MIN_PASSWORD_LENGTH` moved to `app/auth/security.py` and is now
shared by registration and the password change, so the two can't drift. `RegisterRequest`
constrains username (3–32 chars, `[A-Za-z0-9_.-]`) and password; the HTML form mirrors it
with `minlength`/`pattern`/`required`, and `register()` pre-checks the length so the
failure is immediate and translated. The `IntegrityError` handler now distinguishes a
username collision (answers `username_taken`) from a derivation-index collision (retries).
Tests: `test_users.py::test_register_rejects_weak_credentials` (parameterized) and the
positive case.

---

## Medium

### B-13

**A malformed stored hash becomes a 500 instead of a 401**

*File:* `app/auth/security.py:16-20`

`verify_password` catches only `VerifyMismatchError`. Argon2 raises `InvalidHashError` for
a hash it cannot parse and `VerificationError` for other verification failures, both of
which escape as an unhandled 500.

*Proposed fix:* catch `argon2.exceptions.VerificationError` (the superclass of
`VerifyMismatchError`) plus `InvalidHashError` and return `False`, so an unusable stored
hash reads as "wrong password" rather than as a server fault.

*Test:* `verify_password("x", "not-a-hash")` returns `False`; `POST /auth/login` against a
user row with a corrupted `password_hash` answers 401.

**Fixed.** `verify_password` catches `VerificationError` (the superclass of
`VerifyMismatchError`) and `InvalidHashError` separately, returning `False` in both cases
and logging the unparseable-hash case as an error, since that one is a data problem worth
noticing. Tests: `test_security.py::test_verify_password_returns_false_for_an_unparseable_hash`,
plus one confirming a genuinely wrong password is still rejected.

### B-14

**Admin token compared non-constant-time**

*File:* `app/api/routes/admin.py:20-22`

`x_admin_token != settings.admin_token` is a short-circuiting comparison. This token gates
the private-key export endpoint, so it deserves a constant-time comparison.

*Proposed fix:* `secrets.compare_digest(x_admin_token, settings.admin_token)`, keeping the
existing "empty configured token means always deny" short-circuit *before* it (compare_digest
on two empty strings returns `True`).

*Test:* the existing admin auth tests still pass, plus one asserting an empty
`ADMIN_TOKEN` setting denies an empty `X-Admin-Token` header.

**Fixed.** `require_admin` uses `secrets.compare_digest`, with the
empty-configured-token check kept *ahead* of it — `compare_digest("", "")` returns `True`,
so the original order would have opened the admin panel on any instance without an
`ADMIN_TOKEN`. Covered by the existing admin auth tests.

### B-15

**Empty secrets are not rejected at startup**

*File:* `app/config.py:13-18`

`jwt_secret` and `xprv_encryption_key` both default to `""`. With an empty `JWT_SECRET`,
PyJWT raises `InvalidKeyError: HMAC key must not be empty` on every login and
registration — a 500 with no hint about the real cause (verified locally). With an empty
`XPRV_ENCRYPTION_KEY`, Fernet fails on the first key derivation instead. Either way the
container starts up healthy and only fails once a user touches the broken path.

*Proposed fix:* a Pydantic `field_validator` (or `model_validator`) on `Settings` rejecting
empty `jwt_secret`/`xprv_encryption_key`, with a minimum length on `jwt_secret` (32 bytes,
per the `InsecureKeyLengthWarning` PyJWT already emits in the test suite). The process must
refuse to boot instead of half-working. Note the tests currently rely on a short secret, so
they need updating alongside.

*Test:* constructing `Settings(jwt_secret="")` raises `ValidationError`.

**Fixed**, but **not as planned.** A Pydantic `field_validator` on `Settings`
would have run at import time in every module that reads config — including the whole test
suite, which has no `.env` — so a fresh clone would have failed at collection. Instead
`validate_runtime_secrets()` (in `app/config.py`) is called from the app's lifespan: the
server still refuses to serve half-configured, without coupling imports to a gitignored
file. It reports all problems at once and treats an empty `ADMIN_TOKEN` as non-fatal
(`require_admin` already denies everything, so the effect is a locked panel, not an open
one). Tests: `tests/unit/test_config.py` (6 cases, using `_env_file=None` so a developer's
real `.env` can't influence the result).

### B-16

**`GET /guida` is broken in Docker**

*Files:* `Dockerfile:38-42`, `app/main.py:98-103`

The image copies `pyproject.toml`, `app/`, `migrations/`, `alembic.ini` and `scripts/`.
`docs/` is never copied (it is not in `.dockerignore` — it is simply not `COPY`-ed), so
`FileResponse("docs/guida-utente.md")` raises inside the container and the help link in
the navbar returns a 500 in every real deployment.

*Proposed fix:* add `COPY docs ./docs` to the `Dockerfile` (treating the guide as a shipped
asset), or move the guide under `app/static/` and serve it from there. Whichever is chosen,
`GET /guida` should degrade to a 404 with a clear message rather than an unhandled
exception when the file is absent.

*Test:* not unit-testable as-is (it depends on the image layout) — verify with
`docker compose up -d --build && curl -k https://localhost/guida`, and add that check to
`docs/running-the-server.md`'s smoke list.

**Partially fixed, by explicit decision.** The crash is gone: `GET /guida` checks
the file exists and answers a structured 404 (`guide_unavailable`, translated in all 7
languages) with an error logged, instead of raising and returning a 500.

Shipping `docs/` in the image (`COPY docs ./docs`) was written and then **reverted at the
owner's request** — the user guide is going to be reworked first, so there is no point
baking the current one into the image. Net effect today: in Docker, `/guida` answers 404
rather than serving the guide, and the navbar link leads nowhere useful. That is a known,
accepted state, not an oversight; adding the `COPY` line is all it takes once the guide is
ready. Verification once it is: `docker compose up -d --build && curl -k https://localhost/guida`.

### B-17

**`_find_change_output` can shrink the recipient output**

*File:* `app/tx/broadcast.py:59-63`

It returns the *first* output whose address matches the sender's own address. Nothing
prevents a withdrawal to the user's own address (`request_withdrawal` never compares
`external_address` with `user.address`), in which case output 0 is the recipient and
output 1 is the change — and a fee bump would reduce the recipient output. The same case
makes `compute_pending_balance` (`app/wallet/balance.py:55-59`) count both outputs, so the
displayed pending balance double-counts the withdrawn amount.

*Proposed fix:* reject a withdrawal whose destination is the user's own address (a
distinct error code — it is a user mistake, not a system limit), and/or identify the
change output by index rather than by address, recording it on `PendingTransaction` at
build time.

*Test:* `request_withdrawal` to `user.address` is rejected; and given a hand-built tx with
two outputs to the same address, the bump reduces the *change* one (the last), leaving the
recipient amount untouched.

**Fixed.** `request_withdrawal` rejects a destination equal to the user's own
deposit address with its own error code — it was a no-op that cost a fee, and it was the
only way for the recipient and change outputs to be indistinguishable by address (which
would have made a fee bump shrink the recipient output, and made
`compute_pending_balance` count the amount twice). Test:
`test_withdrawals.py::test_withdrawal_to_own_address_is_rejected`, which also asserts no
UTXO was touched.

### B-18

**DB session held open across Electrum network calls during payout**

*File:* `app/rounds/scheduler.py:162-220`

`_trigger_payout` opens a session and, inside it, awaits `client.listunspent(...)` and
`client.broadcast(...)` before committing. On SQLite that holds the write lock for the
duration of two network round-trips (unbounded, per [B-01](#b-01)); on Postgres it becomes
a long-running transaction.

*Proposed fix:* restructure into three phases — read config/winner and close the session →
do the network work (`listunspent`, build, `broadcast`) → reopen a session to persist the
result and the `PendingTransaction`. This also makes the [B-05](#b-05) error handling
easier to place, since the failure-prone part is no longer inside a transaction.

*Test:* the existing payout tests must still pass; add one asserting no session is open
while the stub client's `broadcast` is being awaited (e.g. by having the stub attempt a
write through a second session).

**Fixed.** `_trigger_payout` is now explicitly three phases — read (session
closed), build+broadcast, persist — so no session is held across a network call. The
restructuring is also what made [B-05](#b-05)'s error handling easy to place, since the
failure-prone part is no longer inside a transaction. Covered by the existing scheduler
tests.

### B-19

**`tip_height` is assigned without a monotonicity check**

*File:* `app/electrum/listener.py:92-101`

`self.tip_height = header["height"]` accepts a lower height on a reorg. Since
`_wait_for_next_block` compares `self._listener.tip_height <= tip_at_close`, a regression
silently extends the wait by a block.

*Proposed fix:* full reorg handling is out of scope for v1 by explicit design decision, but
the assignment should be guarded: keep `max(self.tip_height, header["height"])` and log a
warning when a header arrives with a lower height, so the condition is at least visible in
the logs when it happens. Do not update `tip_header_hex` from a header that loses this
comparison, or height and hash would describe different blocks.

*Test:* feeding the consumer a header at height N then N-1 leaves `tip_height == N` and the
hash unchanged.

**Fixed.** Header handling moved into `_apply_header()`, which refuses a height
below the current tip, logs a warning when that happens, and applies height and hex
together — a losing header's hex must not be stored, since that hex is the draw's entropy
source and a mismatched pair would be worse than a stale one. Test:
`test_electrum_listener.py::test_tip_never_moves_backwards`.

## Low / hygiene

### B-20

**`replaced_by_txid` is never written**

*Files:* `app/db/models.py:119`, `app/tx/broadcast.py:116-120`,
`app/api/routes/admin.py:261`

`bump_fee` overwrites `current_txid` in place and never records what the old txid was. The
column exists, is selected, and is rendered in the admin "Transazioni pendenti" table,
where it therefore always shows `—`.

*Proposed fix:* set it in `bump_fee` (naturally part of the [B-02](#b-02) fix) so the
replacement chain is auditable. Note the column semantics are the reverse of what the name
suggests for an in-place update — it will hold the *previous* txid, so either rename it
(`previous_txid`, via a migration) or document the direction on the model.

*Test:* after a bump, the row's `replaced_by_txid` holds the pre-bump txid and
`current_txid` the new one.

**Fixed.** `bump_fee` records the pre-bump txid in `replaced_by_txid`. The
model now documents that the column points *backwards* despite its name (renaming it would
need a migration and would churn the admin UI; the direction is stated on the field
instead). Test: covered by
`test_broadcast.py::test_bump_fee_retargets_every_stored_txid`.

### B-21

**Dead code and detached-object access in the confirmation poller**

*File:* `app/tx/confirmation.py:29-35`

`pending_ids` (line 32) is computed and never used. Line 35 rebuilds the same tuples from
ORM objects whose session has already been closed; it works today only because
`expire_on_commit=False` and no commit intervened, so flipping that engine setting would
break it silently.

*Proposed fix:* drop `pending_ids` and select the plain columns
(`select(PendingTransaction.id, PendingTransaction.current_txid, PendingTransaction.kind)`)
instead of hydrating entities, so nothing outlives the session.

*Test:* covered by the existing `tests/unit/test_confirmation.py` — it must stay green with
`expire_on_commit=True` forced on the test session factory.

**Fixed.** `poll_once` selects plain columns
(`select(PendingTransaction.id, .current_txid, .kind)`) instead of hydrating entities, so
nothing outlives the session, and the unused `pending_ids` local is gone. Covered by the
existing (and new) `test_confirmation.py` cases.

### B-22

**Amounts rendered by raw division**

*Files:* `app/static/app.js` (multiple: `:295`, `:424`, `:660`, `:673`, `:680`),
`app/static/admin.js` (table cells)

Every amount is displayed as `sats / SATS_PER_PLM` with no formatting, so values like
`0.7000000000000001` are reachable in the balance, the jackpot and the admin tables.

*Proposed fix:* one `formatPlm(sats)` helper (fixed decimals, locale-aware grouping via
`Intl.NumberFormat` with the language already resolved by `i18n.js`), used by every display
site. Amounts sent *to* the server must keep going through `Math.round(x * SATS_PER_PLM)` —
the formatter is for display only.

*Test:* manual — with a 0.7 PLM jackpot and a 12345678.9 PLM balance, no artefacts and no
locale mismatch against the selected language.

**Fixed.** `formatPlm(sats)` added to `app.js` (and `fmtPlm` to `admin.js`),
using `Intl.NumberFormat` with the already-resolved language; every display site routes
through it. Input fields deliberately keep the raw value — a grouped, localized string
would break `parseFloat` — and amounts sent to the server still go through
`Math.round(x * SATS_PER_PLM)`. Verification is manual (browser rendering).

### B-23

**Concurrent `withLoading` on the same button**

*File:* `app/static/app.js:20-31`

`withLoading` snapshots `button.innerHTML` and restores it in `finally`. `refreshMe()` is
invoked from the SSE handler, the poll chain, `placeBet`, `withdraw` and `showDashboard`,
all sharing `#refresh-btn`. Two overlapping calls make the second snapshot the *loading*
label, which it then restores permanently — leaving the button stuck on
"Aggiornamento…".

*Proposed fix:* keep the in-flight promise on the element itself (e.g. a
`button._loadingPromise` / `WeakMap`); a second call either awaits the existing one or
returns immediately, so only the outermost call restores the markup. Also worth
de-duplicating the SSE burst: `onRoundServerEvent` fires three fetches per event, and the
broadcaster is generic, so every client reacts to every event.

*Test:* manual — trigger a bet while an SSE-driven `refreshMe()` is in flight and confirm
the refresh button returns to its icon+label state.

**Fixed.** `withLoading` keeps the in-flight promise in a `WeakMap` keyed by the
button: a nested call awaits the existing one and runs its own work without touching the
markup, so only the outermost call restores it. Verification is manual (browser
interaction).

### B-24

**The global exception handler breaks the error contract**

*File:* `app/main.py:74-77`

It answers `{"detail": "internal server error"}` — a bare string, while
`app/api/errors.py` documents `detail` as `{"code", "message", "params"}`. The frontend
tolerates a string (`apiErrorMessage` handles that case), but the contract should be
uniform.

*Proposed fix:* return `ApiError("internal_error", "internal server error").as_detail()` and
add `error.internal_error` to all 7 languages in `app/static/i18n.js`. Keep the response
body free of exception details — the traceback belongs in `logs/app.log` only.

*Test:* an endpoint stubbed to raise answers 500 with `detail.code == "internal_error"` and
no exception text in the body.

**Fixed.** The catch-all handler returns
`ApiError("internal_error", "internal server error").as_detail()`, and
`error.internal_error` was added to all 7 languages. Test:
`test_rounds_route.py::test_unhandled_errors_use_the_structured_detail_shape`, which also
asserts the exception text does not leak into the response body.

---

## Cross-cutting observation

[B-01](#b-01) through [B-04](#b-04) share one root cause: **the code treats a broadcast as
final and the network connection as never failing.** There is no reconciliation between
the database's view of the world and the chain's — no re-scan at startup, no terminal
`failed` state, no request timeouts, no "unstick" routine. Combined with the already
documented scheduler-resume gap and `restart: unless-stopped` in `docker-compose.yml`,
the system has several absorbing states from which only manual database edits recover.

Fixing that class of problem is more valuable than any individual item above. Suggested
order of work, by expected time-to-first-occurrence in production:

1. [B-01](#b-01) — will happen within hours of deployment.
2. [B-03](#b-03) — one bad row is enough, and it is silent.
3. [B-02](#b-02) — happens on the first bump the RBF loop actually performs.
4. [B-06](#b-06) — depends only on the user's UTXO shape.
5. [B-05](#b-05) — happens on the first operator typo, and is unrecoverable in the
   foreign-address variant.
6. [B-04](#b-04) + [B-08](#b-08) — the reconciliation layer both of them need.
7. Everything else.