_release_failed_bet and _release_failed_withdrawal restored the balance, freed
the reserved UTXOs and (for a bet) removed the participant without calling
broadcaster.publish(), so every dashboard kept showing the phantom bet and the
reduced balance until its next poll — while the success path and the
reconciler's own abandon path both published.
The two regression tests pre-open the round before subscribing: place_bet opens
one itself, and that publish() would otherwise satisfy the assertion whether or
not the rollback published anything.
select_utxos had no ceiling on input count, so an address fragmented into many
small deposits built an ever-larger transaction whose fee — deducted from the
amount being moved — eroded the bet's share of the pool or the withdrawn amount,
and past a few hundred inputs stopped being standard at all.
MAX_TX_INPUTS (50) now bounds the selection. Reaching the cap without covering
the target is reported as its own "too_many_inputs" code, distinct from having
no funds, with the cap carried in the error params for the 7 translations. The
payout path records the same distinction in its payout_failed audit reason.
Both held arbitrary-length data (a raw signed transaction hex, an audit
payload) in a bare String, which SQLAlchemy compiles to VARCHAR with no
length. SQLite and PostgreSQL accept that; other backends like MySQL
require a length on VARCHAR and would reject it. Add a migration
(verified upgrade/downgrade/upgrade round-trip, and confirmed with
`alembic check` that it leaves no further diff against the models).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
secrets.compare_digest raises TypeError instead of returning False when a
str argument contains non-ASCII characters, turning a bad admin token into
an unhandled 500 instead of the expected 403. Encode both sides before
comparing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
/admin/rounds and /admin/audit-log accepted any limit, including -1 (which
SQLite treats as "no limit"), and /admin/pending-transactions had no limit
at all -- it grows without end. Add Query(default=..., ge=1, le=500) to all
three, plus an optional status filter on pending-transactions.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CLAUDE.md declares the server always runs via Docker (dev and prod
alike) with no supported bare-uvicorn workflow, but README's Quick
start and docs/running-the-server.md's "Locale / venv" section still
documented running uvicorn directly — a leftover from before that
policy was adopted. Rewrite both to a single Docker-only path and
update CLAUDE.md's own note about it.
Verified docker compose run --rm app python scripts/generate_master_key.py
against a real build/run to confirm the Quick start's Docker commands
actually work as documented.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Caddy adds none of these on its own. Add HSTS, X-Content-Type-Options,
X-Frame-Options, Referrer-Policy and a CSP scoped to default-src
'self' plus the one external asset (Google Fonts). script-src/style-src
need 'unsafe-inline' because both SPAs rely on inline onclick handlers
and style="" attributes throughout — removing those is a separate,
larger refactor.
Validated with `caddy validate` and a live container curl check.
Adds a static regression test asserting the header directives stay
present in the Caddyfile, since nothing else in the Python suite
exercises it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
They enumerate the entire API surface, admin endpoints included, to
anyone who requests them. Gate them behind a new ENABLE_API_DOCS
setting (off by default) and update README/docs and BUGS.md/CLAUDE.md
open-bug counts accordingly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
poll_once and reconcile.py's existence check both called
blockchain.transaction.get(txid, verbose=True). Several Electrum server
implementations and versions reject the verbose flag outright
("verbose transactions are currently unsupported"), which would have
meant no confirmations and no reconciliation ever running against such
a server, read as a plain transport error. reconcile.py additionally
decided whether to abandon a transaction - releasing its funds - by
substring-matching the error text ("missing", "not found", ...), which
only works against ElectrumX's specific wording.
Both now ask blockchain.scripthash.get_history for the address that
owns every input of the transaction (a user's own address for a
bet/withdrawal, the pool address for a payout) and look for the txid in
the result: present with height > 0 means confirmed, present with
height <= 0 means still in the mempool, absent means the server
doesn't know it. get_history is a plain, universally-supported Electrum
method, and "not in the list" replaces the old substring-matching
entirely - no more guessing at error wording to decide whether to
release funds. History is cached per scripthash within one pass, since
every "payout" row shares the same pool address.
New app/tx/pending_address.py factors out own_address_for (the
address derivation was previously duplicated informally inside
tx/broadcast.py's signing context) so confirmation.py and reconcile.py
share one definition instead of two that could compute different
addresses for the same row.
tests/unit/test_confirmation.py and test_reconcile.py needed real User
rows and a master-key bootstrap they didn't have before, since address
derivation is now exercised for real rather than assumed. Suite grows
from 217 to 222 tests. BUGS.md moves B-41 to Previously fixed - no
Medium-severity finding remains open.
bump_fee issued one get_transaction per input (up to 15s each) and
then a broadcast, all with the caller's DB session held open - exactly
the pattern already fixed elsewhere for the same reason (B-18's
_trigger_payout, B-31's refresh_user). Also, _prevout_amount computed
a prevout's satoshi value via round(value_coins * 100_000_000) on a
float the server reported, in a codebase that is otherwise strictly
integer-satoshi.
bump_fee now takes a session_factory and a pending_id instead of a
live session and row, with three phases: read what's needed (the
signing key, current fee rate, raw tx) and close the session before
any network call; do the chain reads, signing and broadcast with no
session open; reopen a session only to persist the outcome.
_prevout_amount now asks for the raw (non-verbose) transaction and
reads embit's parsed TransactionOutput.value directly - already an
exact integer, no float conversion involved at all.
A pending_transaction that's no longer "pending" by the time bump_fee
actually runs (it confirmed in the meantime, a normal race) is now a
quiet no-op returning None, rather than being folded into RbfBumper's
error-logging path.
Suite grows from 214 to 217 tests. BUGS.md moves B-40 to Previously
fixed, and trims its own now-stale claim that bump_fee still depended
on verbose=True (B-41) - it no longer does.
create_async_engine had no connect_args and there was no PRAGMA
anywhere in the repo. SQLite's default rollback-journal mode lets a
writer block every reader for the duration of its transaction, and a
second writer arriving while one is already active fails immediately
with "database is locked" rather than waiting at all - realistic given
five concurrent background tasks (scheduler, confirmation poller, RBF
bumper, two reconcilers) plus every HTTP handler share one file, and
nothing previously handled that error.
app/db/base.py now registers a "connect" event on the engine that sets
journal_mode=WAL, synchronous=NORMAL and a 5-second busy_timeout on
every new DBAPI connection - applied only when the dialect is sqlite,
so a future PostgreSQL DATABASE_URL is unaffected. WAL lets readers and
writers proceed without blocking each other, and busy_timeout gives a
second writer a real window to wait instead of failing instantly.
Left out: an explicit application-level retry wrapper for "database is
locked" in the background loops, the other half of the proposed fix -
busy_timeout already gives SQLite itself several seconds to resolve
writer-vs-writer contention before ever raising, and every background
loop already catches and logs an unhandled exception before its next
scheduled tick, which is itself a retry, just not an immediate one.
Suite grows from 211 to 214 tests. BUGS.md moves B-39 to Previously
fixed.
GET /rounds/stream capped concurrent subscribers with one global
counter (MAX_SUBSCRIBERS=500): anyone opening 500 connections degraded
every other user to polling. The comment called it a defensive cap;
it was actually the vector, since nothing stopped a single source from
exhausting it alone.
RoundEventBroadcaster now also tracks subscribers per client IP,
capped much lower (MAX_SUBSCRIBERS_PER_IP=5). Past that cap, opening
one more stream evicts that same IP's own oldest connection (woken via
a new EVICTED sentinel so the SSE generator closes it promptly) rather
than refusing the new one or letting one abusive IP crowd out unrelated
clients under the old global-only cap. The global cap stays as a
backstop against overall resource exhaustion regardless of source.
Extracted client_ip() (X-Forwarded-For, since Caddy reverse-proxies
every request) out of auth/routes.py into app/api/client_ip.py so the
login/registration throttles (B-33) and this new per-IP cap share one
definition instead of two that could drift apart.
Not implemented: enforcing the connection cap at the Caddy layer
itself, which the proposed fix also suggested - the standard Caddy
image this project uses has no such directive without a third-party
module, and building a custom image felt like a bigger, separate change
than this fix warranted.
Suite grows from 201 to 211 tests. BUGS.md moves B-38 to Previously
fixed.
request_withdrawal validated against confirmed UTXOs only and answered
a flat insufficient_balance even when the requested amount was covered
by the pending-inclusive balance the UI actually shows (unconfirmed
change from a recent bet/withdrawal) — contradicting what the user was
looking at on screen. Raise balance_pending_confirmation instead when
compute_pending_balance covers the amount, carrying the pending sats
in params, with its error.* string in all 7 languages.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_wait_for_next_block had no timeout, no log, and no audit entry: a
connection that stopped advancing the tip left a round silently frozen
in "drawing" with nothing in /admin to explain why. Log progress
periodically, write a draw_stalled audit entry past a threshold (a few
block-time multiples), and surface the wait via a new Round.drawing_started_at
column, exposed as draw_waiting_since in GET /rounds/current.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
SQLite/aiosqlite returns DateTime columns as naive even though every
value is written in UTC, so a bare .isoformat() dropped the offset and
the frontend's new Date() parsed it as local time. Add a shared
isoformat_utc() helper and use it at every call site that was missing
the fix already applied ad hoc in rounds.py.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Neither self-service password change nor the admin reset invalidated
already-issued JWTs — a 24h-lifetime token stayed valid regardless, so
a stolen token (or an attacker who already had the old password) kept
working past a password change meant to lock them out. The admin reset
exists precisely for the "account compromised" case and didn't evict
the attacker at all.
Add User.token_version (migration 943dbd74d983), embedded in every JWT
as a "tv" claim and checked against the DB on every request in
get_current_user/get_optional_user; a mismatch reads as session_expired.
Both change-password and the admin reset bump it. change-password hands
back a freshly minted token so the caller's own session keeps working
instead of being logged out by its own request; the admin reset does
not, since that session isn't the one making the call.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
POST /auth/login had no rate limiting, no lockout, no delay — a patient
distributed attack could brute-force a password against an enumerable
username list on a custodial wallet, where a guessed password means
withdrawing someone's funds.
Add per-username and per-IP throttling with exponential backoff
(app/auth/rate_limit.py), keyed on app.state like UserLocks rather than
a module global so each app instance gets isolated throttle state.
Unknown-user and wrong-password already shared one response path, so no
enumeration oracle there. Registration is throttled per-IP too, which
also bounds how many accounts one IP can spin up (B-31).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bump_fee computed fee_delta as new_fee - old_fee, falling back to a
flat 1-satoshi bump whenever that came out zero or negative - which
happened whenever old_fee (the actual fee paid, from real prevout
amounts) already exceeded the naive target, e.g. because dust change
had been folded into the original fee (psbt_builder.py's
DUST_LIMIT_SATS handling). A 1-satoshi total increase is nowhere near
BIP125 rule 4's minimum (the replacement must pay at least the
incremental relay fee rate times its own vsize more than what it
replaces), so the node rejected it every time - and since bump_fee
raised before touching `pending`, the next tick retried with identical
parameters every 30 seconds, forever. Separately, the fee rate climbed
by 1 sat/vB every bump with no ceiling.
fee_delta is now max(target_fee - old_fee, vsize * the incremental
relay rate) - always at least the relay-mandated minimum regardless of
what the naive arithmetic produces. pending.fee_rate_sat_vb is set to
the actual resulting rate rather than the naive target, so a later
bump's arithmetic starts from what's really being paid instead of
drifting from it. Once a transaction reaches MAX_FEE_RATE_SAT_VB (a
new constant, 10,000 sat/vB, shared with RoundConfig.fee_rate_sat_vb's
existing admin-facing bound so the two can't drift apart - the same
reason MIN_PASSWORD_LENGTH is shared elsewhere) bump_fee refuses to
bump further; the reconciler abandons it if it never confirms (B-27)
instead of this retrying forever.
Suite grows from 185 to 187 tests. BUGS.md moves B-32 to Previously
fixed.
_run_once awaited _subscribe_all_users() inline, before starting the
header/scripthash consumer tasks, and that method subscribed one user
at a time. At thousands of users that's thousands of sequential
round-trips during which nothing else ran: tip_height was frozen and
an in-flight draw's _wait_for_next_block made zero progress for the
entire resubscribe - a reconnect (which the listener already treats as
routine, not exceptional) could stall the lottery for minutes.
_subscribe_all_users now fans out with bounded concurrency
(asyncio.Semaphore, 20 at a time) instead of a serial loop, and one
user's failure no longer stops the rest. _run_once now starts it as
its own background task, created after the consumer tasks rather than
awaited before them, so tip updates and already-subscribed users'
notifications keep flowing throughout - its own completion is
deliberately not raced against the session-ending tasks (unlike them,
it's expected to finish normally), and its failure is logged the same
way address_for_new_user's background task is (B-30).
Left out: decoupling the listunspent refresh from the subscribe call
itself (the third part of the proposed fix) - the periodic
DepositReconciler (B-30) already provides a backstop for a slow or
delayed initial refresh, so the added complexity wasn't worth it here.
Suite grows from 182 to 185 tests, including an end-to-end test
against _run_once proving a new tip is processed while a slow
resubscribe is still in flight. BUGS.md moves B-31 to Previously
fixed.
Deposits were credited exclusively by scripthash-change notifications,
with nothing re-verifying a user's balance against the chain if a
subscription was ever silently lost. address_for_new_user's subscribe
was fire-and-forget: the task wasn't retained, so it could be
garbage-collected mid-flight, and any failure (including self.client
turning None between the check and the task running) vanished into
asyncio's default unretrieved-exception handler instead of being
logged anywhere. On an otherwise healthy, long-lived connection there
may be no reconnect for days to re-subscribe everyone, so a user in
that state never saw their deposits.
address_for_new_user now retains the task and logs its exception if
it fails. New app/deposits/reconcile.py adds DepositReconciler, a
periodic sweep (every 5 minutes, gated on the Electrum client being
connected, same shape as tx/reconcile.py) that round-robins over every
user and calls the listener's own refresh_user (renamed from
_refresh_user since it's now called from outside the class) - so the
notification-driven and periodic paths can never behave differently.
Deliberately sweeps every user rather than only ones missing from the
in-memory scripthash map, since that map can't tell "never subscribed"
apart from "subscribed, but the server stopped delivering
notifications for it". Wired into app/main.py's lifespan alongside the
other three background reconcilers.
Suite grows from 176 to 182 tests. BUGS.md moves B-30 to Previously
fixed.
detect_external_spends marked a UTXO spent_txid='external-spend'
irreversibly the moment it was missing from one listunspent reply, on
one server, with no way to undo it. A rotated-to server that's broken
or behind, or an empty reply, could zero a user's balance permanently.
Split into three functions in deposits/service.py: find_utxos_missing_
from (read-only candidate detection, and refuses to flag anything at
all when listunspent comes back entirely empty for a funded address -
that reads as a broken response, not a full sweep), mark_utxos_spent_
externally (persistence only, once a candidate is already confirmed),
and reinstate_reappeared_utxos (undoes the mark if the outpoint
reappears as unspent later).
ElectrumListener gains corroborate_utxo_spent, sharing the same
majority-quorum logic corroborate_header already uses for B-28: before
a candidate is marked, the other configured servers are asked whether
they also see it as spent. _refresh_user now reads candidates, then
corroborates each one with no DB session held open across those
network calls (same shape as B-18/B-25), then persists.
Suite grows from 165 to 176 tests. BUGS.md moves B-29 to Previously
fixed.
A single hostile Electrum server, or a MITM on the one active
connection, could fabricate the block header the draw's entropy comes
from and so pick the winner of every round: headers were accepted with
no proof-of-work check and no link to the previous tip.
app/rounds/draw.py adds header_meets_its_own_target (rejects a header
whose hash doesn't satisfy the difficulty target it claims) and
header_prev_hash. electrum/listener.py's _apply_header now rejects a
header failing either check by raising HeaderValidationError, which
ends the session the same way a dropped connection would so the
listener rotates to the next configured server.
ElectrumListener gains corroborate_header: before the draw uses a
block, it's independently checked against the other configured servers
and needs a majority to agree. rounds/scheduler.py's
_wait_for_next_block now calls this and, on failure, logs why and
waits for a further block instead of ever using an uncorroborated
header.
Certificate/hostname verification stays disabled, so this doesn't
cover an attacker able to MITM every configured server at once -
BUGS.md notes that as not covered.
Suite grows from 151 to 165 tests. BUGS.md moves B-28 to Previously
fixed.
bump_fee (tx/broadcast.py) used to overwrite PendingTransaction.
broadcast_at on every fee bump, but reconcile.py's abandon-after-N-
hours grace period is measured from that same column. A transaction
successfully bumped every rbf_timeout_seconds (900s by default) but
never mined reset that clock before it could ever reach the 6-hour
abandon window, so it was never abandoned: its UTXOs never returned to
the user, and if it was a bet the round stayed in "closing"
indefinitely.
PendingTransaction gains a last_broadcast_at column (migration
861e76aaf34c, backfilled from broadcast_at for existing rows before
the NOT NULL constraint is applied). broadcast_at is now never
rewritten after creation, so reconcile.py's _is_due keeps measuring
from the first broadcast unchanged. bump_fee updates last_broadcast_at
instead, and should_bump now reads last_broadcast_at rather than
broadcast_at — correct, since whether another bump is due should reset
after every bump, unlike the reconciler's abandon check, which must
not.
BUGS.md moves B-27 to "Previously fixed" with the fix description; the
suite grows from 148 to 151 tests, including a direct proof that a tx
bumped a minute ago but first broadcast 7 hours ago still gets
abandoned.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_trigger_payout used to run exactly once, from _close_and_draw. Any
failure after that point — no Electrum client, insufficient pool
UTXOs, a missing fee_address, a rejected broadcast — wedged the round
in paying_out forever, and every one of those early returns except the
generic exception handler logged nothing at all: /admin showed a
stalled round with no explanation. A process restart while paying_out
hit the same dead end.
_tick now handles status == "paying_out": it calls the new
_retry_payout_if_due, which re-invokes _trigger_payout unless the most
recent payout_failed audit entry for the round is younger than
_PAYOUT_RETRY_INTERVAL_SECONDS (60s) — throttled so a persistently
broken payout (e.g. no fee_address set yet) doesn't retry, and re-log
a failure, on every 5-second tick. Every early return in
_trigger_payout now calls _log_payout_failure with a reason string, so
that throttle always has something to check against and /admin always
shows why a round is stuck.
This is safe to fire on a restart too, because B-25 already made
_trigger_payout idempotent (it no-ops if a non-terminal payout
PendingTransaction already exists) and persists before broadcasting —
so a round found paying_out at startup, whatever state its payout was
actually in, gets retried the same way. That closes the paying_out
half of the "scheduler doesn't resume mid-flight rounds after a
restart" gap in CLAUDE.md; the drawing/block-wait half is untouched
(see BUGS.md B-36).
BUGS.md moves B-26 to "Previously fixed" with the fix description; the
suite grows from 143 to 148 tests.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_trigger_payout used to broadcast the payout transaction and only
afterwards write payout_txid and its PendingTransaction. A crash in
that window (docker-compose.yml auto-restarts on crash) left a payout
on-chain with zero record: the round stuck in paying_out, nothing for
the reconciler to resolve, and a manual retry that would have paid the
winner a second time. This mirrors B-08, which already fixed the same
gap for place_bet/request_withdrawal.
_trigger_payout now has four phases: read, build (network read only,
no write), persist the intent as a PendingTransaction(kind="payout",
status="building") and commit, then broadcast and promote to
"pending". A rejected broadcast now leaves that "building" row for
tx/reconcile.py to resolve — its existing building/pending handling
already covers a payout kind correctly, including clearing
payout_txid on abandonment, so reconcile.py needed no changes.
Since pool UTXOs aren't tracked in utxo_events and so can never be
reserved/released the way a user's own UTXOs are, two guards go along
with the two-phase write: _trigger_payout now refuses to build a
second payout for a round that already has a non-terminal
PendingTransaction, and the payout builder excludes any UTXO already
referenced by any non-terminal payout transaction
(_reserved_payout_outpoints) so a stale payout from an earlier round
the reconciler hasn't abandoned yet can't be double-spent by a fresh
attempt.
This makes a payout retry safe; making one happen automatically is
B-26, still open. BUGS.md moves B-25 to "Previously fixed" with the
fix description; the suite grows from 139 to 143 tests.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
credit_confirmed_utxos only ever credited new UTXOs; a UTXO spent by
something other than the app's own bet/withdrawal/payout flow (e.g. someone
using the raw derived privkey directly) never got its spent_txid set, so
cached_balance_sats kept counting it forever. detect_external_spends mirrors
the same listunspent refresh in the other direction: anything still marked
unspent in our DB but missing from the address's current unspent set gets
spent_txid="external-spend", an audit_log entry, and an immediate balance
recompute — wired into the same ElectrumListener._refresh_user call that
already runs on every scripthash notification and on listener (re)connect.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A dropped connection used to hang the whole platform permanently, and three
defects composed to do it (BUGS.md B-01):
The read loop's death was invisible. When the socket closed, _read_loop broke out
and finished, but _run_once was blocked on gather() over two notification
consumers waiting on queues nobody would ever fill again — it never returned and
never raised, so the reconnect-with-backoff logic was unreachable.
client.wait_closed() now resolves when the loop ends for any reason, and
_run_once races it against the consumers and a keepalive with
asyncio.wait(FIRST_COMPLETED).
Nothing had a timeout. request() registered a future, wrote to a half-closed
socket (drain() often doesn't raise) and awaited a reply that would never come.
That hung a POST /bets *while holding the per-user lock*, and could stop the
confirmation poller for good. Every request is now bounded at 15s, and a timeout
tears the connection down rather than leaving a server that owes us a reply in
rotation.
There was no keepalive, so on a quiet instance the normal way this connection
dies is an idle-timeout drop by the server (~10 minutes for many). A server.ping
every 60s makes that observable within a minute.
listener.client is also cleared before reconnecting, so callers stop treating a
dead connection as live.
On top of the finding, the listener now rotates over a list of servers:
ELECTRUM_FALLBACK_SERVERS holds comma-separated host:port[:notls] extras, tried
after the primary. Everything the platform does goes through this one connection
— deposit credits, broadcasts, confirmations, the chain tip the draw waits on —
which made a single hardcoded server its biggest point of failure. A failed or
dropped session moves to the next server immediately and only sleeps on the
backoff once every server has had a turn, so one dead server costs one attempt
instead of an outage, while a genuinely offline network still backs off. A
malformed entry fails at startup, not during the outage when the fallback is what
you need.
Also fixes B-19: header handling refuses a height below the current tip and
applies height and hex together, since _wait_for_next_block waits for
tip_height > tip_at_close (a regression silently added a block to the draw's
wait) and that hex is the draw's entropy source, so a mismatched pair would be
worse than a stale one.
Verified in the live deployment: the log shows the endpoint list, then "Electrum
connected to santantonio.sytes.net:50002", and the connection holds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Startup validation (B-15). An empty JWT_SECRET makes PyJWT raise InvalidKeyError
on every login and an empty XPRV_ENCRYPTION_KEY makes Fernet fail on the first
key derivation — either way the container came up looking healthy and broke the
moment a real user touched it. validate_runtime_secrets() reports every problem
at once and is called from the lifespan (wired in the next commit).
Deviation from the plan in BUGS.md, which proposed a Pydantic field_validator:
Settings is constructed at import time by every module that reads config,
including the test suite, which has no .env and no business holding real secrets
— a validator there would fail a fresh clone at collection. At startup the
guarantee that matters is unchanged (the server won't serve traffic
half-configured) without coupling imports to a gitignored file. An empty
ADMIN_TOKEN is deliberately non-fatal: require_admin already denies everything,
so the effect is a locked panel, not an open one.
Unhandled errors answer the documented shape (B-24). The catch-all returned a
bare-string `detail` while app/api/errors.py documents
{"code", "message", "params"}, leaving clients to special-case exactly the
responses they understand least. It now returns internal_error in that shape,
with the exception text staying in logs/app.log and out of the response body.
GET /guida no longer crashes (B-16). It reads docs/guida-utente.md, which the
Dockerfile doesn't ship, so in every real deployment that navbar link was a 500 —
confirmed in the deployed log, which holds two of them from earlier today ending
in "RuntimeError: File at path docs/guida-utente.md does not exist." It now
checks the file and answers a structured 404 (guide_unavailable) with an error
logged. Shipping docs/ in the image was written and then reverted on request: the
guide is being reworked first, so /guida answers 404 in Docker for now, which is
an accepted state rather than an oversight.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fee_address was the dangerous one (B-05). PUT /admin/config assigned whatever it
was given, and a well-formed address from another chain (bc1...) parses fine as a
witness program — so every round's 30% commission would be signed and broadcast
to a script nobody holds the key for. A malformed one instead wedged the payout
with an unhandled EmbitError. It now has to pass is_valid_plm_address, the same
check user withdrawals already had. Numeric fields got bounds too:
fee_rate_sat_vb=0 produces transactions no node relays, which stalls bets,
payouts and withdrawals alike, and round_duration_seconds=0 expires a round the
instant it opens.
Config changes are audit-logged (B-10). /pause and /resume were logged but a
config edit wasn't, so the most sensitive setting in the system could be changed
without leaving any trace — contradicting CLAUDE.md, which says audit_log records
what changed. The entry carries a before/after diff per field, computed before
assignment, and no-op updates write nothing. `paused` was removed from
_CONFIG_FIELDS so the maintenance switch has exactly one audited path; it stays
in the response model.
Admin token comparison is constant-time (B-14), with the empty-token check kept
*ahead* of it: compare_digest("", "") returns True, so the obvious ordering would
have opened the panel on any instance without an ADMIN_TOKEN.
Registration input (B-12). It accepted an empty username and a one-character
password while /users/me/change-password demanded 8 — an odd place to be lenient
on a custodial system holding real funds. MIN_PASSWORD_LENGTH moved to
auth/security.py so both share it, and the username is constrained to 3-32 chars
of [A-Za-z0-9_.-]. The IntegrityError handler also distinguishes a username
collision (answers username_taken) from a derivation-index one (retries): a
concurrent duplicate username used to be retried five times and then reported as
derivation_index_conflict, which told the user the wrong thing.
verify_password (B-13) catches VerificationError and InvalidHashError, not just
VerifyMismatchError, so an unparseable stored hash reads as "wrong password"
instead of a 500 — logged as an error, since that one is a data problem.
guida-admin.md gains a table of the audit events worth watching, including
payout_failed, which needs manual intervention.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round opening (B-09). open_new_round_if_needed now handles the IntegrityError
from ix_rounds_single_active (previous commit) by rolling back and using the
winner's round. Deviation from the plan in BUGS.md, which proposed making the
scheduler the only writer: that would mean the first bet after a cooldown
couldn't open a round, so both callers stay and a bounded retry was added
instead — a conflict where nothing is active yet just means the winner 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 rather than silently picking
the newest.
The jackpot (B-11). It was participant_count * the *current* bet_amount_sats,
which overstated the pool (each stored bet is already net of that bet's network
fee) and silently rewrote the advertised jackpot of a round in progress whenever
an operator edited the bet amount. It now sums the participants' stored
bet_amount_sats. The remaining 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 promised away, since the comment there claimed exactness.
Payout (B-05, B-18). _trigger_payout is split into read / build+broadcast /
persist, so no DB session is held across a network call (on SQLite that meant
holding the write lock for two unbounded round-trips). That restructuring is also
what makes the error handling placeable: it now catches Exception around the
chain work and writes a payout_failed audit entry, where a malformed fee_address
used to raise EmbitError all the way to the scheduler's catch-all, leaving the
round stuck in paying_out with nothing recorded about why. Automatic payout retry
remains an open gap.
The scheduler also counts "building" participants as in-flight when deciding
whether a round may close, matching the two-phase bet write.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`if change > 0` created a change output for any leftover at all. Below the
P2WPKH dust threshold (294 sat: the output's 31 vbytes plus the 67 needed to
spend it, at the 3000 sat/kvB dust relay fee) relaying nodes reject the whole
transaction, so the bet or withdrawal failed at broadcast with an error the user
could do nothing about — and which arrived as a 500 (BUGS.md B-06).
Sub-dust change now goes to the fee in both builders, and a sub-dust
recipient/winner/commission amount is refused up front with its own error code.
The fee estimate already assumed two outputs, so dropping one never underpays.
Cross-checked against PalladiumWallet, the source of truth for PLM parameters:
it delegates to NBitcoin's TransactionBuilder (same 294 sat threshold) and has
an explicit test — Un_resto_sotto_la_soglia_dust_viene_assorbito_nella_fee —
asserting the same behaviour, so both the value and the semantics match the
reference implementation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The code treated a broadcast as final: money moved on-chain and the DB was
updated on the assumption it would either confirm or be fee-bumped until it
did. Neither is guaranteed, and every way that assumption broke was permanent
(BUGS.md B-02, B-03, B-04, B-07, B-08, B-20, B-21).
Persist before broadcasting. place_bet and request_withdrawal now write their
rows in a "building" state and commit, then broadcast, then promote to
broadcast/pending in a second commit. Before, a failure or crash between the
broadcast and the commit left the coins irreversibly spent with no trace: no
participant (so no entry in the draw), no pending row (so no RBF and no
confirmation tracking), and the UTXOs not even marked spent, so the next bet
would try to double-spend them. A refused broadcast now releases the reserved
UTXOs, restores the balance, removes the participant (or marks the withdrawal
failed), audit-logs it, and answers a translatable broadcast_failed — as 502,
since the network refused it, not the caller, where it used to be an opaque 500.
Reconcile what's in flight against the chain. New PendingTransactionReconciler
(app/tx/reconcile.py, every 120s and once at startup) asks whether each
non-terminal tx exists: present -> promote, gone -> mark failed with a reason,
release the inputs, roll the domain row back, audit-log it. Grace periods differ
by state (120s for "building", 6h for "pending", so the RBF bumper gets its
attempts first). It is deliberately biased to inaction: only a server that
positively doesn't know the tx counts as absent, and a transport failure never
abandons anything, because releasing a UTXO whose tx is actually alive would
invite a double-spend. Verified against the live server, which answers "No such
mempool or blockchain transaction" for an unknown txid.
Stop keying on a value that changes. An RBF bump changes the txid, and
_on_bet_confirmed looked the participant up by bet_txid — so a bumped bet
confirmed under a txid no participant carried, the row stayed "broadcast"
forever, and the scheduler waited on it forever: the round could never close and
the lottery stopped. Handlers now resolve by immutable ids (round_id/user_id,
withdrawal_id), and bump_fee retargets every stored txid — bet_txid,
Withdrawal.txid, Round.payout_txid and UtxoEvent.spent_txid — plus records the
previous one in replaced_by_txid, which was never written at all.
One bad row no longer blocks the rest. The confirmation poller's per-tx lookup
is guarded: a txid the server can't resolve used to abort the whole pass, so
nothing confirmed again until an operator intervened. It also selects plain
columns instead of hydrating entities that outlive their session.
Tests: 6 reconciler cases including "a broken connection must not release coins";
the bet-ordering test probes committed state from an independent session during
the broadcast, and caught a real mistake in the first draft of this change (the
_pending_transaction helper still hardcoded status="pending", so rows were born
already-broadcast and would have got the 6-hour grace instead of 120s).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
embit's Script.from_address accepts a well-formed bech32 address from any
chain: a Bitcoin bc1... parses into a perfectly valid witness program. So a
withdrawal to a BTC address built, signed and broadcast normally on PLM, and
the funds landed on a script nobody holds the key for — silently, with no
error anywhere. A malformed address fared slightly better only in that it
crashed the request with an unhandled 500.
is_valid_plm_address checks the HRP as well as the parse, and runs first in
request_withdrawal, before a single UTXO is touched. It matches what the
withdrawal form already told the user (bech32 plm1q... only).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Frontend dashboards previously found out about state changes only on their
next poll tick (up to 15s, or 3s during a draw) — this adds a push channel
so updates land as soon as they happen instead.
- app/rounds/events.py: a small in-process pub/sub (RoundEventBroadcaster).
The message carries no payload — it's just a "something changed, go
refetch" signal, so it needs no auth and no knowledge of who's allowed to
see what; personalization stays entirely in the existing REST endpoints.
- GET /rounds/stream: an SSE endpoint exposing that channel, with keep-alive
comments so it survives idle periods behind a reverse proxy, and a
defensive MAX_SUBSCRIBERS cap (well above the ~100 concurrent users
expected) — past it, the endpoint returns 503 instead of opening a stream,
and callers just keep working off polling.
- publish() calls added at every point that actually changes what a
dashboard would want to know: new round opened, round status transitions
(closing/drawing/paying_out/closed), a bet or withdrawal broadcast, any
pending tx confirming (bet/withdrawal/payout), a deposit credited, and a
new block tip arriving (the exact moment the "drawing" phase is waiting on).
- Caddyfile: excludes /rounds/stream from gzip encoding, since compression
would buffer output and defeat the point of a live stream.
Single-process only by design for now (no cross-worker fan-out) and the
notification is a generic broadcast rather than a per-user channel — both
are deliberate scope decisions for the current ~100-user, single-container
deployment, not oversights. Polling is left fully in place as a fallback;
this is purely additive.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Balance display: place_bet/request_withdrawal spend whole UTXOs and mark
them spent at broadcast time, well before confirmation, so the confirmed-only
balance could drop by far more than the amount actually moving. Add
compute_pending_balance() (app/wallet/balance.py) to fold the unconfirmed
change from in-flight bet/withdrawal PendingTransactions back in; GET
/users/me now returns pending_balance_sats + has_pending, and the frontend
shows it colored green (settled) or amber (still pending) instead of the
confirmed-only figure.
Round outcome display: the win/lose reveal and the "pagamento al vincitore
in corso" status were fighting over the same UI slot, and the reveal broke
across a page refresh. Now:
- The round-status box (generic phase progress) and the personal win/lose
box are independent and can both be visible at once.
- The win/lose box only renders for users who actually played in that round
(new user_played field on GET /rounds/current, via a new optional-auth
dependency so the endpoint stays usable logged-out).
- The reveal delay is anchored to the round's server-provided closes_at
instead of a client-side "first seen" timestamp, so repeated reloads can't
reset it, and the revealed result is persisted in localStorage so it
survives a refresh even after the round has fully closed.
- GET /users/me/last-round-result is a durable DB-backed backstop for
players who miss the live window entirely (backgrounded tab, offline).
Also hardens the frontend polling loop: call() now times out instead of
hanging forever, and a session-epoch counter stops an in-flight request from
a previous login from resurrecting a duplicate poll loop after logout.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
RoundConfig.min_amount_sats was an independently-configurable floor that
could drift out of sync with bet_amount_sats for no real reason (deposits
never had a server-side minimum anyway). Drop the field and enforce
amount_sats >= config.bet_amount_sats directly in request_withdrawal.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The round timer relied on each client's own wall clock, so two browsers with
skewed local clocks showed different countdowns for the same round; the
server now also returns server_time so the frontend can correct for clock
skew. Also drop out-of-order /rounds/current responses (multiple independent
triggers could resolve late and revert the UI to a stale drawing/result
state) and prune per-round bookkeeping maps on round transitions.
Separately, place_bet only checked status == "open", leaving a window (up to
the scheduler's 5s tick interval) after a round's timer hit zero where a new
bet could still be accepted. place_bet now checks the round's own deadline
directly (round_accepts_bets), acting as an immediate "yellow light" for new
entries while still letting already-broadcast bets confirm before the round
closes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Session hardening: / and /admin now respond with Cache-Control: no-store, and
both pages re-derive their auth state on pageshow (event.persisted) as a
safety net against bfcache showing a stale logged-in/out view across
back/forward navigation. The user page also syncs logout across tabs via the
storage event, since localStorage is shared but in-memory JS state isn't.
Password recovery: admin gets a "Reset" button per user (POST
/admin/users/{id}/reset-password) that generates and sets a new password,
shown once — passwords are Argon2-hashed and can never be recovered, only
replaced. Users get self-service password change (POST
/users/me/change-password, requires the current password) under a new
Profilo tab, alongside read-only account info (username, address, balance,
join date).
Round display robustness: the user dashboard now refreshes immediately on
tab visibility change (background tabs get their timers throttled hard),
shows an explicit "connessione persa" state after repeated failed polls
instead of silently freezing on stale data, and polls faster both right when
the countdown hits zero and through the gap where the round is past its
deadline but still waiting for in-flight bets to confirm before the server
actually closes it.
RoundConfig gets a paused flag toggled via new POST /admin/pause and
/admin/resume endpoints (audit-logged, surfaced as a "Manutenzione" card in
the admin Parametri view). Pausing only stops the *next* round from opening
once the current one closes — rounds/service.py:open_new_round_if_needed
still lets an in-progress round finish, draw, and pay out its winner
normally. GET /rounds/current exposes lottery_paused so the user page shows
a maintenance banner (even while logged out) instead of silently going idle.
Also replaces the user dashboard's stacked account-bar card + bento-grid
menu with a single sticky navbar (identity row + Deposito/Bet/Prelievo
tabs), and moves the page content into a dedicated .app-shell container so
the navbar itself can span full width.
RoundConfig gains round_duration_seconds, round_cooldown_seconds,
min_amount_sats, fee_rate_sat_vb and rbf_timeout_seconds (plus a
hardcoded default for the pre-existing bet_amount_sats) as column
defaults on the model itself — get_round_config no longer seeds from
Settings at all. Every call site that read these from settings
(scheduler, bets, withdrawals, rounds service/route, RBF bumper) now
reads the DB-backed RoundConfig instead.
app/config.py now holds only true env-driven infra/secrets (database
URL, Electrum connection, master key, JWT, admin token) — no business
parameter has an env var anymore, matching an explicit decision to drop
the "seed from settings" indirection entirely rather than keep a env
fallback nobody should rely on.
Migration backfills the existing round_config row via server_default
(matching the old settings defaults) then drops the default, so future
rows go through the ORM/model defaults instead of a stale constant.
Tests updated: should_bump's timeout_seconds is now required (no
settings fallback); test_scheduler.py seeds a RoundConfig row directly
instead of monkeypatching settings; test_withdrawals.py and
test_rounds_service.py use local constants mirroring the model
defaults instead of reading them off settings.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GET /admin/users lists id/username/address/balance_sats/created_at.
GET /admin/users/{id}/privkey derives and returns that user's raw WIF
private key, for manual intervention (e.g. sweeping funds back if
something's stuck) — this is already a custodial system, the server
holds the master key everything is derived from, so this doesn't grant
a new capability, just exposes an existing one through the API. Every
access is audit-logged (admin_privkey_accessed).
Also fixes a pre-existing test-isolation bug in test_admin.py's client
fixture: app.db.session.get_session had `from app.db.base import
AsyncSessionLocal`, a one-time reference copy at first import — later
tests reassigning db_base.AsyncSessionLocal never reached it, so any
test mixing direct DB writes with router calls silently read/wrote
against a stale, possibly-disposed engine from whichever test ran
first. Fixed by also rebinding app.db.session.AsyncSessionLocal in the
fixture on every run.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
open_new_round_if_needed now withholds opening the next round until
ROUND_COOLDOWN_SECONDS (default 30) have passed since the previous
round's closed_at, returning None in that window instead of a Round.
Without this, the next round opened within one scheduler tick (~5s) of
the previous payout confirming — not enough time for a player to
notice the round they were in actually resolved.
Callers updated: the scheduler treats None as "nothing to do this
tick", and place_bet raises a "try again shortly" BetError instead of
crashing on a None round.
Not in the original flowchart — a deliberate UX addition on top of it,
documented as such in CLAUDE.md.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bearer-token-gated admin endpoints to read/update the DB-backed
operational config (fee_address, bet_amount_sats) without a redeploy,
plus a lightweight audit log writer for round/payout/config events.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Builds and broadcasts a user->external-address PSBT with change back to
the user's own address, fee deducted from the withdrawn amount, and
registers the confirmation handler that marks the withdrawal confirmed.
Shares the per-user lock with bets so a build never races a spend from
the same UTXO set.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Periodic scheduler (configurable round duration) that closes a round
only once all broadcast bets confirm, waits for the next block after
closing, draws a winner via block-hash-seeded modulo over participants
ordered by broadcast time, triggers the 70/30 payout, and only opens the
next round once that payout confirms. Draw logic is isolated in
draw.py as a deliberately simple, replaceable component.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Places the fixed-cost bet into the current round: builds and broadcasts
the user->pool PSBT with change back to the user's own address, enforces
at most one active bet per user, and registers the confirmation handler
that marks a bet confirmed and adds the participant to the round.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Credits a user's internal balance once a UTXO on their deposit address
reaches 1 confirmation, keeping utxo_events as the source of truth and
cached_balance_sats as a derived read cache.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Shared per-user locking to serialize bet/withdrawal PSBT builds
(tx/locks.py), a confirmation poller for pending outgoing transactions,
and the timeout->fee-bump->rebroadcast loop used by bets, payouts and
withdrawals alike (tx/broadcast.py).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Argon2 password hashing, JWT session issuing/verification
(auth/security.py), register/login routes, the bearer-token
get_current_user dependency, and GET /users/me for address + balance.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Minimal Electrum protocol client (client.py) plus a scripthash
subscription listener (listener.py) that watches user deposit addresses
for confirmed UTXOs, with the address->scripthash conversion helper and
a manual smoke-test script against the dev bootstrap server.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
davide
