The first pass left 11 of the 24 entries with their remedy buried in prose and only 7 with a named test, while the header claimed a regression test for each. Every entry now follows one shape: symptom, root cause, *Proposed fix:*, *Test:* — with the handful that genuinely cannot be unit-tested (Docker image layout, browser behaviour) marked manual instead of left implied. A few fixes gained detail while being written out: B-14 needs the empty-token short-circuit kept ahead of compare_digest, B-15 breaks the current tests' short JWT secret, B-19 must not update tip_header_hex from a losing header, and B-20's column ends up holding the previous txid rather than the next one, so its name is backwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
35 KiB
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.
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 | Critical | Electrum client/listener | A dropped connection hangs the whole server permanently — no reconnect, no request timeout, no keepalive |
| B-02 | Critical | RBF / bets | A fee bump on a bet tx orphans RoundParticipant.bet_txid, wedging the round in closing forever |
| B-03 | Critical | Confirmation poller | One unresolvable txid aborts confirmation detection for every other pending tx |
| B-04 | Critical | Balance / UTXO state | No reconciliation path: a dropped tx freezes the spent UTXOs (and the user's funds) forever |
| 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 | High | PSBT builder | Change outputs below the dust limit are created, making the transaction unrelayable |
| B-07 | High | Bets / withdrawals | A broadcast rejection surfaces as HTTP 500, bypassing the structured error contract |
| B-08 | High | Bets | Broadcast happens before the bet is persisted: a commit failure loses the money with no record |
| B-09 | High | Rounds | Race allows two simultaneously-open rounds, which permanently blocks all future rounds |
| B-10 | High | Admin / audit | PUT /admin/config writes no audit-log entry |
| B-11 | High | Rounds API | The displayed jackpot is not the amount the winner receives |
| B-12 | High | Auth | Registration enforces no password or username validation, unlike change-password |
| B-13 | Medium | Auth | verify_password turns a malformed stored hash into a 500 instead of a 401 |
| B-14 | Medium | Admin auth | Admin token compared with != instead of a constant-time comparison |
| B-15 | Medium | Config | Empty JWT_SECRET / XPRV_ENCRYPTION_KEY are not rejected at startup |
| B-16 | Medium | Deployment | GET /guida is broken in Docker: docs/ is never copied into the image |
| B-17 | Medium | RBF | _find_change_output can shrink the recipient output instead of the change |
| B-18 | Medium | Scheduler | DB session held open across Electrum network calls during payout |
| B-19 | Medium | Electrum listener | tip_height is assigned without a monotonicity check (reorg) |
| B-20 | Low | RBF | PendingTransaction.replaced_by_txid is never written — dead column shown in the admin UI |
| B-21 | Low | Confirmation poller | Dead local pending_ids, plus attribute access on detached ORM objects |
| B-22 | Low | Frontend | Amounts rendered by raw division — floating-point artefacts visible to users |
| B-23 | Low | Frontend | Concurrent withLoading on the same button can leave it stuck on its loading label |
| B-24 | Low | API contract | The global exception handler answers with a bare-string detail |
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:
- The read loop's death never propagates.
connect()spawns_read_loopas a detached task. When the socket closes,readline()returnsb"", the loop breaks and the task completes — but_run_onceis blocked onasyncio.gather(self._consume_headers(...), self._consume_scripthash(...)), and both consumers are awaitingasyncio.Queue.get()on queues nobody will ever fill again. Thegathernever returns and never raises, so theexcept Exception+ backoff-reconnect logic inElectrumListener.run()is never reached. self.clientstays non-None. Every consumer of the client (RoundScheduler,ConfirmationPoller,RbfBumper,place_bet,request_withdrawal, and thelistener.client is Noneguards in the bet/withdrawal routes) therefore keeps treating a dead connection as live.request()has no timeout. It registers a future inself._pending, writes to a closed writer (drain()frequently does not raise on a half-closed socket) and thenawait future— which nothing will ever resolve, because_read_loopis gone. Note that_read_loop'sfinallyonly 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()inasyncio.wait_for(...)with an explicit timeout (10-15s) and pop the future fromself._pendingon timeout. - Add a keepalive task issuing
server.pingevery ~60s; a failed ping tears the connection down. - Make the read loop's termination observable: either include
self._read_taskin_run_once'sgather, or set anasyncio.Eventin_read_loop'sfinallythat_run_onceawaits alongside the consumers. On teardown, setself.client = Nonebefore 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.
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._tickcounts participants in"broadcast"before allowing the round to close, so the round stays inclosingindefinitely;- 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_drawonly selectsstatus == "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) 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.
B-03
One unresolvable txid stops confirmation detection for everything else
File: app/tx/confirmation.py:35-37
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), 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 needs).
Test: poll_once with two pending rows where the first raises must still confirm the
second and return 1.
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 setreplaced_by_txid/a failure reason; - clear
spent_txidon the UTXOs it consumed (they are identifiable by parsingraw_tx_hex's inputs); - call
recompute_balance; - roll the domain row back (delete the
RoundParticipant, mark theWithdrawalfailed) 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.
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:
- Malformed
fee_address→build_payout_transactioncallsscript.Script.from_address(fee_address), which raisesEmbitError._trigger_payoutonly catchesInsufficientFundsError, so the exception escapes through_close_and_drawup to theexcept ExceptioninRoundScheduler.run(). The round is left inpaying_outwith no retry (the payout-retry gap inCLAUDE.md), i.e. wedged. - Well-formed but foreign
fee_address(e.g. a Bitcoinbc1...) → 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 modeapp/wallet/address.pywas 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_addressmust passis_valid_plm_address;bet_amount_sats,round_duration_seconds,fee_rate_sat_vb,rbf_timeout_secondsstrictly positive, with sane upper bounds;round_cooldown_seconds,draw_animation_secondsnon-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.
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), 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.
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), 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.
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 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.
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 staysopenforever, never ticked, never closed. Sinceget_active_roundmatches 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.
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.
B-11
The displayed jackpot is not what the winner receives
File: app/api/routes/rounds.py:118-144
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:
- The real pool is
sum(p.bet_amount_sats), and each participant'sbet_amount_satsisrecipient_sats— the bet amount minus that bet's network fee (psbt_builder.build_signed_transaction). - 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. participant_countcounts everyRoundParticipantrow, including bets still inbroadcastthat 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.
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.
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.
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.
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.
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.
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.
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); 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 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).
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.
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 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.
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.
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.
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.
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.
Cross-cutting observation
B-01 through 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:
- B-01 — will happen within hours of deployment.
- B-03 — one bad row is enough, and it is silent.
- B-02 — happens on the first bump the RBF loop actually performs.
- B-06 — depends only on the user's UTXO shape.
- B-05 — happens on the first operator typo, and is unrecoverable in the foreign-address variant.
- B-04 + B-08 — the reconciliation layer both of them need.
- Everything else.