Close the window where a bet pays into a round it was left out of (B-53)

place_bet commits its participant row as "building" before broadcasting (B-08's
two-phase write), while the scheduler flips the round "open" -> "closing" in one
transaction and counts in-flight participants in another. A bet whose deadline
check passed just before that flip could commit in between: the count saw zero,
so the round drew and paid out over the "confirmed" participants only, while the
bet confirmed normally and its sats landed in the pool address — credited to no
round, to no participant, with no refund path, silently improving the next
round's payout change.

Two locks on the same door:

- place_bet re-checks the deadline after building and signing (the first check
  happens before the UTXO scan, so a slow build could carry a bet past it), then
  commits the participant row behind a compare-and-set on the round's own row,
  UPDATE rounds ... WHERE status = 'open'. That UPDATE takes SQLite's write lock,
  so the two transactions can no longer interleave: either the bet commits first
  and the scheduler's in-flight count sees it, or the flip commits first and the
  guard matches zero rows and refuses the bet with round_closing before anything
  is broadcast. A write-snapshot conflict (OperationalError) is the same
  situation and gets the same answer. Nothing has been broadcast at that point,
  so the rollback releases the UTXOs and leaves no rows behind.

- _close_and_draw re-counts in-flight bets in the same session it snapshots the
  participants from, and returns with the round still "closing" if it finds any.
  Redundant given the CAS, and cheap: it fails safe and the next tick retries.

No new error code — a bet refused this way is exactly the "round is closing"
case the user already sees.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-03 22:06:21 +02:00
co-authored by Claude Opus 5
parent 025754c860
commit 64f62291d2
6 changed files with 212 additions and 75 deletions
+8 -68
View File
@@ -4,10 +4,14 @@ Third full-codebase audit, opened after the 2026-07-26 (B-01 … B-24) and
2026-07-27 (B-25 … B-49) lists were emptied. Numbering continues from the last
fixed finding, B-51.
Nothing in this list is fixed yet — it is the analysis pass only. Per CLAUDE.md's
convention each entry gets its own commit with its own regression test, and the
`B-nn` marker goes in a comment next to the fix so `git log --all --grep 'B-nn'`
finds it later.
The list opened at B-52 … B-72 and holds only what is still **open**: a finding is
removed from this file once it is fixed. Per CLAUDE.md's convention each entry gets
its own commit with its own regression test, and the `B-nn` marker goes in a comment
next to the fix so `git log --all --grep 'B-nn'` finds it later.
Already fixed and removed: B-52 (a round past ~50 participants deadlocked the
payout — `025754c`), B-53 (a bet could pay into the pool of a round it was left
out of — `PLACEHOLDER`).
State of the tree at audit time: 264 unit tests, all passing; `tests/integration/`
still empty; withdrawal and the RBF bump path still never live-broadcast.
@@ -28,70 +32,6 @@ Severity is about consequence, not likelihood:
## Critical
### B-52 — a round with more than ~50 participants deadlocks the platform permanently — **FIXED**
`app/wallet/psbt_builder.py:42` (`MAX_TX_INPUTS = 50`),
`app/rounds/scheduler.py:349-373`.
Every confirmed bet leaves exactly one UTXO on the pool address, and the payout's
selection target is `winner_share + commission`, i.e. the whole pool — so it needs
*all* n bet UTXOs as inputs. At n ≥ 51 `select_utxos` raises `too_many_inputs`,
`_trigger_payout` records `payout_failed`, and `_retry_payout_if_due` re-attempts
every 60 s forever. The round stays `paying_out`, so `open_new_round_if_needed`
never opens another round: the lottery halts, the pool is unspendable through the
normal path, and the only way out is a manual consolidation with the pool key.
CLAUDE.md presents B-48's input cap purely as a fragmented *user* address problem.
The pool case is structural rather than an edge case: participant count alone
causes it, with the default bet amount and no unusual deposit pattern.
**Fixed** by moving the limit from where it was *discovered* to where it can still be
*enforced*:
- `select_utxos` takes the cap as a parameter. Bets and withdrawals keep
`MAX_TX_INPUTS = 50` (a user-protection limit: the fee comes out of the amount
they are moving); the payout uses the new `MAX_PAYOUT_TX_INPUTS = 500`, since the
pool's UTXO count is just the number of bets and the fee comes out of a 70% share
of the whole pool. 500 inputs is ~34 kvB, about a third of the 100 kvB relay
standardness budget; signing that many costs ~0.4 s of event loop, once per round,
in a background task.
- `place_bet` refuses the bet past `MAX_PARTICIPANTS_PER_ROUND = 400` with a new
`round_full` error (400, translated into all 7 languages), counting every
participant row rather than only the confirmed ones. The cap sits below the input
cap so the payout keeps headroom for pool change accumulated by earlier rounds.
The invariant is now "a round can always be paid out", enforced before any of the
401st player's money moves. A round already wedged with 51499 participants pays out
by itself on the next `_retry_payout_if_due` tick.
Not addressed, and deliberately so: periodic pool consolidation, which is what would
be needed to go beyond this order of magnitude (see the audit discussion — it needs a
new PendingTransaction kind, must not run mid-round, and would force the payout math
to tolerate a pool short of its exact target). Raising these two constants covers
anything up to ~1400 participants first.
### B-53 — a bet can pay into the pool and still be left out of the draw
`app/bets/service.py:80-92`, `app/rounds/scheduler.py:101-129`.
`place_bet` commits its participant row as `building` *before* broadcasting
(the deliberate two-phase write of B-08). The scheduler counts in-flight
participants in one session and then reads the `confirmed` participants in a
second, separate session. A bet that passed `round_accepts_bets` just before the
deadline can commit its phase-1 row *between* those two queries: the count saw
zero, so the round draws and pays out, while the new row — not yet `confirmed`
is excluded from `participants`. The bet then confirms normally and its sats land
in the pool address, credited to no round and to no participant. There is no
refund path, and the money silently improves the *next* round's payout change.
The window is one task switch wide, but both queries do real DB I/O, so it is
reachable rather than theoretical.
Fix directions: re-check the in-flight count inside the same transaction that
snapshots the participants (and abort the close if it is non-zero), or make the
deadline authoritative at the row level so a bet cannot commit against a round
whose timer has expired.
### (not new) `drawing` does not resume after a restart
Already tracked as an accepted gap in CLAUDE.md's "Known gaps", not re-numbered
+3 -3
View File
@@ -8,7 +8,7 @@ The user communicates in Italian in chat — reply to them in Italian. Everythin
## Project status
All 10 stages of the original build order are code-complete and unit-tested — 253 tests, all under `tests/unit/` (`tests/integration/` is an empty package). Beyond them: Docker + Caddy deployment, admin dashboard (`/admin`), static test UI (`/`), pending-inclusive balance display, an SSE push channel layered over the original polling, self-service password change + admin password reset, and the reconciliation/corroboration machinery below.
All 10 stages of the original build order are code-complete and unit-tested — 275 tests, all under `tests/unit/` (`tests/integration/` is an empty package). Beyond them: Docker + Caddy deployment, admin dashboard (`/admin`), static test UI (`/`), pending-inclusive balance display, an SSE push channel layered over the original polling, self-service password change + admin password reset, and the reconciliation/corroboration machinery below.
Verified on mainnet with real money: registration + address derivation, deposit crediting (1-conf), a real 10 PLM bet (broadcast → confirmed → change credited back), and one full round cycle (close → draw on a real block hash → 70/30 payout with sat math checked against the broadcast tx → confirmation → close → next round auto-opened). **Withdrawal and the RBF bump path have never been exercised against a live broadcast** — unit-tested only.
@@ -33,7 +33,7 @@ PYTHONPATH=. python scripts/decrypt_master_key.py # ops recovery: decrypt+pr
PYTHONPATH=. python scripts/encrypt_master_key.py # ops bootstrap: import an externally-generated xprv (--overwrite to replace)
PYTHONPATH=. python scripts/electrum_smoke_test.py # manual check: connect, handshake, subscribe to headers, print the tip
python -m pytest # all 253 tests
python -m pytest # all 275 tests
python -m pytest tests/unit/test_hd.py # one file
python -m pytest tests/unit/test_hd.py::test_derivation_is_deterministic # one test
```
@@ -155,7 +155,7 @@ Diagrams: [platform-overview.mmd](flowchart/platform-overview.mmd), [round-lifec
**PLAY** — fixed cost, **at most one active bet per user**, and **at most `MAX_PARTICIPANTS_PER_ROUND` (400) players per round** — past that the bet is refused with `round_full` and the player waits for the next round (B-52: the payout must spend one pool UTXO per bet, so a round is only ever allowed to grow to what a single payout tx can drain). PSBT user-address → pool-address, always with a **change output back to the same user address** (a user's balance must never exactly equal the bet). Fee ~1 sat/vB, **deducted from the bet amount**. No confirmation within the timeout → RBF bump and rebroadcast.
**DRAW** — configurable timer (default 600s):
- *Bet cutoff is the round's own deadline* (`opened_at + round_duration_seconds`), **not** the DB status: `place_bet` calls `rounds/service.round_accepts_bets`, which rejects once the deadline passes even while `status` is still `"open"` (the 5s scheduler tick can lag behind it). Once a round leaves `open`, no new bets either, and no new round opens until this one is fully `closed`.
- *Bet cutoff is the round's own deadline* (`opened_at + round_duration_seconds`), **not** the DB status: `place_bet` calls `rounds/service.round_accepts_bets`, which rejects once the deadline passes even while `status` is still `"open"` (the 5s scheduler tick can lag behind it). Once a round leaves `open`, no new bets either, and no new round opens until this one is fully `closed`. The deadline is checked twice — on arrival and again after the transaction is built — and the participant row is then committed behind a **compare-and-set on the round row** (`UPDATE rounds ... WHERE status = 'open'`, B-53): the scheduler flips `open``closing` in a transaction of its own and only counts in-flight bets afterwards, so without the CAS a bet could commit in between, be excluded from the draw (only `confirmed` participants are drawn) and still have its sats land in the pool with no refund path. Its mirror image on the scheduler side is `_close_and_draw` re-counting in-flight bets in the same session it snapshots the participants from.
- *"Yellow light":* closing **waits for every already-broadcast bet to confirm** before drawing, so a bet in flight at the boundary isn't lost (`building` counts as in-flight; what bounds the wait is the reconciler eventually abandoning a bet that never confirms).
- *Algorithm* (deliberately simple, meant to be replaced): first block confirmed after closing — corroborated by the other servers first, and on failure the draw waits for a *further* block and writes a `draw_header_corroboration_failed` audit entry rather than stalling silently — hash as seed, `index = seed mod participant_count` over participants ordered by **broadcast timestamp** (also the tie-break when two bets land in the same block). Equal probability for everyone, regardless of amount.
- *Payout* is signed with the pool key; its **fee comes out of the winner's 70%**, leaving the 30% fee share intact. Same timeout → RBF → rebroadcast pattern.
+44 -3
View File
@@ -1,12 +1,13 @@
from datetime import datetime, timezone
from embit import script
from sqlalchemy import func, select
from sqlalchemy import func, select, update
from sqlalchemy.exc import OperationalError
from sqlalchemy.ext.asyncio import AsyncSession
from app.api.errors import ApiError
from app.audit.log import write_audit_log
from app.db.models import PendingTransaction, RoundParticipant, User, UtxoEvent
from app.db.models import PendingTransaction, Round, RoundParticipant, User, UtxoEvent
from app.electrum.client import ElectrumClient
from app.rounds.config import get_round_config
from app.rounds.events import broadcaster
@@ -26,6 +27,11 @@ class BetError(ApiError):
pass
class _RoundClosedDuringBuild(Exception):
"""Internal signal (B-53): the round stopped accepting bets while this one was
being built. Never leaves place_bet — it becomes a `round_closing` BetError."""
async def place_bet(session: AsyncSession, client: ElectrumClient, user: User) -> RoundParticipant:
round_ = await open_new_round_if_needed(session)
if round_ is None:
@@ -115,7 +121,42 @@ async def place_bet(session: AsyncSession, client: ElectrumClient, user: User) -
session.add(participant)
pending = _pending_transaction(round_.id, user.id, built, config.fee_rate_sat_vb)
session.add(pending)
await session.commit()
# B-53: the deadline check at the top of this function happened before the UTXO
# scan and the signing above, so re-check it here against the clock as it is now —
# a slow build must not sneak a bet past the round's deadline.
#
# And then the part the clock can't cover: a compare-and-set on the round's own
# row, in the *same* transaction as the participant insert. The scheduler flips
# "open" -> "closing" in a transaction of its own and only counts in-flight
# participants afterwards, so without this a bet could commit its "building" row
# in between and be paid into the pool while the round drew and paid out without
# it — money credited to no round, no participant and no refund path. The UPDATE
# takes SQLite's write lock, so the two transactions can no longer interleave:
# either this commits first and the scheduler's subsequent in-flight count sees
# the row, or the flip commits first and this matches zero rows and refuses the
# bet before anything is broadcast.
try:
if not round_accepts_bets(round_, config.round_duration_seconds):
raise _RoundClosedDuringBuild
guard = await session.execute(
update(Round)
.where(Round.id == round_.id, Round.status == "open")
.values(status="open")
.execution_options(synchronize_session=False)
)
if guard.rowcount != 1:
raise _RoundClosedDuringBuild
await session.commit()
except (_RoundClosedDuringBuild, OperationalError) as exc:
# OperationalError here is SQLite's write-snapshot conflict: the round row
# changed under us, which is the same situation as the guard matching nothing.
# Nothing has been broadcast yet, so the rollback undoes phase 1 entirely —
# the UTXOs stay unspent and no participant row survives.
await session.rollback()
raise BetError(
"round_closing", "the current round is closing, please try again shortly"
) from exc
# --- Phase 2: broadcast, then promote both rows to their live state ---------
try:
+21
View File
@@ -120,6 +120,27 @@ class RoundScheduler:
async with self._session_factory() as session:
round_ = await session.get(Round, round_id)
# B-53: re-check for in-flight bets in the *same* session that snapshots
# the participants. _tick's check ran in a session of its own, so a bet
# committing its "building" row in between was counted by neither: the
# round drew and paid out without it, while its sats still landed in the
# pool. place_bet's compare-and-set on the round row is what makes that
# window unreachable; this is the cheap second lock on the same door, and
# it fails safe — the round stays "closing" and the next tick retries.
pending_count = await session.scalar(
select(func.count())
.select_from(RoundParticipant)
.where(
RoundParticipant.round_id == round_id,
RoundParticipant.status.in_(("building", "broadcast")),
)
)
if pending_count:
logger.info(
"round %s: %s bet(s) still in flight at close time, waiting", round_id, pending_count
)
return
participants = (
await session.scalars(
select(RoundParticipant)
+100
View File
@@ -328,3 +328,103 @@ async def test_bet_is_persisted_before_it_is_broadcast(session_factory):
assert seen["pending"] == [("bet", "building")]
assert seen["participants"] == ["building"]
# --- B-53: a bet must never pay into the pool of a round it was left out of ------
async def _assert_bet_left_no_trace(session_factory, user_id: int, balance_before: int) -> None:
async with session_factory() as session:
utxo = (await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id))).one()
assert utxo.spent_txid is None # nothing reserved, so the user can bet next round
assert (await session.scalars(select(RoundParticipant))).all() == []
assert (await session.scalars(select(PendingTransaction))).all() == []
user = await session.get(User, user_id)
assert user.cached_balance_sats == balance_before # the rollback undid the recompute too
async def test_place_bet_refuses_when_the_round_closed_between_the_check_and_the_commit(
session_factory, monkeypatch
): # B-53
"""The scheduler flips "open" -> "closing" in a transaction of its own and only
then counts in-flight bets. A bet whose deadline check passed just before that
flip must not be able to commit its participant row afterwards: it would be
excluded from the draw (only "confirmed" participants are drawn) while its sats
still landed in the pool address — credited to no round, with no refund path.
round_accepts_bets is forced to pass so the refusal can only come from the
compare-and-set on the round row, which is the part that survives the race the
wall-clock check cannot see."""
user_id = await _make_funded_user(session_factory, 40, 3_000_000_000)
client = FakeElectrumClient()
async with session_factory() as session:
round_ = await open_new_round_if_needed(session)
await session.commit()
round_id = round_.id
monkeypatch.setattr("app.bets.service.round_accepts_bets", lambda *args, **kwargs: True)
async with session_factory() as session:
# What the scheduler's own tick would have committed a moment earlier.
(await session.get(Round, round_id)).status = "closing"
await session.commit()
async with session_factory() as session:
user = await session.get(User, user_id)
balance_before = user.cached_balance_sats
with pytest.raises(BetError) as excinfo:
await place_bet(session, client, user)
assert excinfo.value.code == "round_closing"
assert not client.broadcasted # refused before any money moved
await _assert_bet_left_no_trace(session_factory, user_id, balance_before)
async def test_place_bet_rechecks_the_deadline_after_building_the_transaction(
session_factory, monkeypatch
): # B-53
"""The first deadline check happens before the UTXO scan and the signing, so a
slow build could carry a bet past the round's deadline. It is re-checked against
the clock as it is at commit time."""
user_id = await _make_funded_user(session_factory, 41, 3_000_000_000)
client = FakeElectrumClient()
async with session_factory() as session:
await open_new_round_if_needed(session)
await session.commit()
checks: list[bool] = []
def _accepts_then_expires(*args, **kwargs) -> bool:
checks.append(True)
return len(checks) == 1 # open when the bet arrived, expired by the time it was built
monkeypatch.setattr("app.bets.service.round_accepts_bets", _accepts_then_expires)
async with session_factory() as session:
user = await session.get(User, user_id)
balance_before = user.cached_balance_sats
with pytest.raises(BetError) as excinfo:
await place_bet(session, client, user)
assert len(checks) == 2 # the re-check really ran
assert excinfo.value.code == "round_closing"
assert not client.broadcasted
await _assert_bet_left_no_trace(session_factory, user_id, balance_before)
async def test_place_bet_still_succeeds_while_the_round_is_open(session_factory): # B-53
"""The guard must not refuse the normal path: an open, in-time round still takes
bets, and the round's status is left untouched by the compare-and-set."""
user_id = await _make_funded_user(session_factory, 42, 3_000_000_000)
client = FakeElectrumClient()
async with session_factory() as session:
user = await session.get(User, user_id)
participant = await place_bet(session, client, user)
assert participant.status == "broadcast"
async with session_factory() as session:
round_ = (await session.scalars(select(Round))).one()
assert round_.status == "open"
+36 -1
View File
@@ -7,7 +7,7 @@ from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
from app.config import settings
from app.db.base import Base
from app.db.models import AuditLog, PendingTransaction, Round, RoundConfig, User
from app.db.models import AuditLog, PendingTransaction, Round, RoundConfig, RoundParticipant, User
from app.rounds.scheduler import RoundScheduler, _reserved_payout_outpoints
from app.wallet.psbt_builder import MAX_TX_INPUTS
@@ -493,3 +493,38 @@ async def test_wait_for_next_block_logs_a_stall_audit_entry_past_the_threshold(s
entries = (await session.scalars(select(AuditLog).where(AuditLog.event_type == "draw_stalled"))).all()
assert len(entries) == 1
assert entries[0].round_id == 1
async def test_close_and_draw_waits_when_a_bet_appears_after_the_tick_check(session_factory): # B-53
"""_tick counts in-flight bets in a session of its own, so a "building" row that
commits between that count and the participant snapshot used to be invisible to
both: the round drew and paid out without the bet, while its sats still landed in
the pool. _close_and_draw re-checks in the same session it snapshots from, and
must leave the round in "closing" for the next tick rather than draw."""
async with session_factory() as session:
session.add(RoundConfig(fee_address=""))
session.add(Round(status="closing", opened_at=datetime.now(timezone.utc)))
await session.commit()
round_ = (await session.scalars(select(Round))).one()
session.add(
RoundParticipant(
round_id=round_.id,
user_id=1,
bet_amount_sats=1_000_000_000,
bet_txid="ab" * 32,
status="building", # committed a moment after _tick counted zero
)
)
await session.commit()
round_id = round_.id
scheduler = RoundScheduler(session_factory, FakeListener())
await scheduler._close_and_draw(round_id)
async with session_factory() as session:
round_ = await session.get(Round, round_id)
assert round_.status == "closing" # not drawn, and not closed as participant-less
assert round_.winner_user_id is None
events = [e.event_type for e in (await session.scalars(select(AuditLog))).all()]
assert "round_closed" not in events
assert "winner_drawn" not in events