davide/plm-lottery
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Persist the payout before broadcasting it (B-25)

Browse Source 
_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>
This commit is contained in:
davide
2026-07-27 09:19:53 +02:00
co-authored by Claude Sonnet 5
parent 43d2321e0f
commit f13f6850b7
3 changed files with 342 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
BUGS.md
+49 -33
View File
@@ -1,20 +1,21 @@
# Known bugs
A second full-codebase audit on 2026-07-27 found **25 further issues** (4 critical, 6 high,
7 medium, 8 low), listed below as B-25 … B-49. **All of them are open.** The 139-test suite
is green, so none of these are caught by existing coverage — every fix should land with a
regression test.
7 medium, 8 low), listed below as B-25 … B-49. B-25 is fixed as of 2026-07-27; the other 24
are open. The 139-test suite was green at the time of the audit, so none of these were caught
by existing coverage — every fix lands with a regression test (B-25's four tests brought the
suite to 143).
The recurring pattern across B-25, B-26, B-27, B-29 and B-36 is worth stating once: the code
is rigorous about the failure modes that have actually been hit, and silent about the ones
that have not. Outgoing transactions reconcile; deposits do not. Bets and withdrawals write
their intent before broadcasting; the payout does not. Broadcast failures are audit-logged;
*pre*-broadcast failures (no client, insufficient pool funds) are not.
The recurring pattern across B-26, B-27, B-29 and B-36 is worth stating once: the code is
rigorous about the failure modes that have actually been hit, and silent about the ones that
have not. Outgoing transactions reconcile; deposits do not. Broadcast failures are
audit-logged; *pre*-broadcast failures (no client, insufficient pool funds) are not.
**Single highest priority: make `paying_out` a recoverable, idempotent state** (record before
broadcast + startup resume + retry). That closes B-25, B-26 and half of the already-known
"scheduler doesn't resume" gap — i.e. every way the lottery currently stops and cannot
restart on its own.
**Highest remaining priority: make `paying_out` fully recoverable, not just idempotent.**
B-25 made a payout retry *safe* (persisted before broadcast, guarded against double-spend);
B-26 is what would make a retry actually *happen* automatically. Together with the
already-known "scheduler doesn't resume" gap, that's every way the lottery currently stops
and cannot restart on its own.
For limitations that are accepted by design rather than bugs (single-shared-token admin auth,
single-process assumptions, no user-facing history, etc.), see "Known gaps / TODO" in
@@ -24,27 +25,6 @@ single-process assumptions, no user-facing history, etc.), see "Known gaps / TOD
## Critical
### B-25 — The payout has no two-phase write, unlike bets and withdrawals
`rounds/scheduler.py:214` broadcasts, and only afterwards (`:229-251`) writes `payout_txid`
and the `PendingTransaction`. A crash in that window — and `docker-compose.yml` sets
`restart: unless-stopped`, so a crash means an automatic restart — leaves a payout on-chain
with **no record at all**: the round is stuck in `paying_out`, the reconciler has nothing to
resolve, and a manual retry would pay the winner a second time (pool UTXOs are not tracked in
`utxo_events`, so nothing reserves them).
This is exactly what B-08 fixed for `place_bet`/`request_withdrawal`; the same fix was never
applied to the path that moves the most money.
**Proposed fix.** Mirror the bet path: write `Round.payout_txid` plus a
`PendingTransaction(kind="payout", status="building")` and commit *before*
`client.broadcast()`, then promote to `"pending"` after. Teach
`reconcile.py:_promote`/`_abandon` to handle a `building` payout (promote if the tx is on
chain, otherwise clear `payout_txid` and leave the round for the retry routine). Since pool
UTXOs are invisible to `utxo_events`, `_abandon` cannot release them — so the payout builder
must additionally refuse to spend an outpoint already referenced by a non-terminal payout
`PendingTransaction`, which is what makes a retry safe against double-paying.
### B-26 — A transient failure at payout time wedges the lottery permanently
`rounds/scheduler.py:166-169`: if `listener.client is None` when `_trigger_payout` starts, it
@@ -383,6 +363,42 @@ already does.
## Previously fixed
### B-25 — The payout has no two-phase write, unlike bets and withdrawals
`rounds/scheduler.py` used to broadcast the payout and only afterwards write `payout_txid`
and the `PendingTransaction`. A crash in that window — and `docker-compose.yml` sets
`restart: unless-stopped`, so a crash means an automatic restart — left a payout on-chain
with **no record at all**: the round stuck in `paying_out`, the reconciler with nothing to
resolve, and a manual retry that would pay the winner a second time (pool UTXOs are not
tracked in `utxo_events`, so nothing reserved them).
This was exactly what B-08 fixed for `place_bet`/`request_withdrawal`; the same fix had never
been applied to the path that moves the most money.
**Fixed:** `_trigger_payout` (`rounds/scheduler.py`) now has four phases instead of three —
read, *build* (network read only, no write), *persist the intent as `PendingTransaction(kind=
"payout", status="building")` and commit*, then broadcast and promote to `"pending"`. A
broadcast rejection now leaves that `"building"` row behind for the existing reconciler
(`tx/reconcile.py`) to resolve — its generic `building`/`pending` handling already covered a
`payout` kind correctly (including clearing `payout_txid` on abandonment), so no changes were
needed there.
Two guards were added alongside the two-phase write, since pool UTXOs are invisible to
`utxo_events` and so can never be released/reserved the way a user's own UTXOs are:
`_trigger_payout` now refuses to build a second payout for a round that already has a
non-terminal `PendingTransaction(kind="payout")`, and the payout builder excludes any UTXO
already referenced by *any* non-terminal payout transaction (`_reserved_payout_outpoints`) —
not just this round's — so a stale payout from an earlier round that the reconciler hasn't
abandoned yet can't be double-spent by a fresh attempt. `should_bump`/reconciler retry timing
around a fee-bumped payout is unaffected by this fix (see B-27, still open).
This makes a payout retry *safe*; it does not yet make one *automatic* — that is B-26, still
open. Regression tests: `tests/unit/test_scheduler.py`
(`test_trigger_payout_persists_before_broadcasting`,
`test_trigger_payout_broadcast_failure_leaves_a_recoverable_row`,
`test_trigger_payout_skips_when_already_in_flight`,
`test_reserved_payout_outpoints_excludes_utxos_claimed_by_a_stale_payout`).
A full-codebase audit on 2026-07-26 (commit `d4e0974`) found 24 bugs across every Python
module under `app/`, both static frontends, and the Docker/Caddy deployment — 5 critical,
7 high, 7 medium, 5 low. All 24 were fixed and verified against the current code on
app/rounds/scheduler.py
+96 -22
View File
@@ -3,8 +3,9 @@ import logging
from datetime import datetime, timedelta, timezone
from embit import script
from embit.transaction import Transaction
from sqlalchemy import func, select
from sqlalchemy.ext.asyncio import async_sessionmaker
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
from app.audit.log import write_audit_log
from app.db.models import PendingTransaction, Round, RoundParticipant, User
@@ -161,8 +162,18 @@ class RoundScheduler:
return self._listener.tip_height, header_hex_to_block_hash(self._listener.tip_header_hex)
async def _trigger_payout(self, round_id: int) -> None:
"""Three phases, so no DB session is held across a network call (B-18): read
what's needed, do the chain work, then persist the outcome."""
"""Four phases, so no DB session is held across a network call (B-18): read
what's needed, build the tx, persist the intent, then broadcast.
The persist happens *before* the broadcast (B-25) — the same two-phase shape
as place_bet/request_withdrawal (B-08): a crash between building the payout
and recording it used to leave money on-chain with zero trace in the DB (no
payout_txid, no PendingTransaction), so a manual retry would have paid the
winner a second time. Now the worst case is a "building" PendingTransaction
the reconciler (app/tx/reconcile.py) can resolve either way by asking the
chain whether the tx exists, exactly like it already does for bets and
withdrawals.
"""
client = self._listener.client
if client is None:
logger.error("round %s payout deferred: not connected", round_id)
@@ -171,6 +182,25 @@ class RoundScheduler:
# --- Phase 1: read (session closed before any network I/O) ---------------
async with self._session_factory() as session:
round_ = await session.get(Round, round_id)
already_in_flight = await session.scalar(
select(PendingTransaction).where(
PendingTransaction.round_id == round_id,
PendingTransaction.kind == "payout",
PendingTransaction.status.in_(("building", "pending")),
)
)
if already_in_flight is not None:
# A payout for this round is already building or broadcast — this
# must not build a second one, or a retry (manual, or a future
# automatic one) would pay the winner twice. Confirmation/
# reconciliation already owns resolving that row.
logger.info(
"round %s payout already in flight (pending_transaction %s), skipping",
round_id,
already_in_flight.id,
)
return
reserved_outpoints = await _reserved_payout_outpoints(session)
config = await get_round_config(session)
fee_address = config.fee_address
fee_rate = config.fee_rate_sat_vb
@@ -192,13 +222,17 @@ class RoundScheduler:
winner_share = pool_amount_sats * 70 // 100
commission_share = pool_amount_sats - winner_share # remainder from rounding goes to fees
# --- Phase 2: build and broadcast ----------------------------------------
# --- Phase 2: build (network read only, no DB write yet) -----------------
try:
pool_key = derive_pool_key()
pool_script_obj = script.p2wpkh(pool_key.to_public())
pool_address = pool_script_obj.address(network=PLM_MAINNET)
entries = await client.listunspent(address_to_scripthash(pool_address))
utxos = [Utxo(e["tx_hash"], e["tx_pos"], e["value"]) for e in entries if e["height"] > 0]
utxos = [
Utxo(e["tx_hash"], e["tx_pos"], e["value"])
for e in entries
if e["height"] > 0 and (e["tx_hash"], e["tx_pos"]) not in reserved_outpoints
]
built = build_payout_transaction(
signing_key=pool_key,
@@ -211,36 +245,54 @@ class RoundScheduler:
change_address=pool_address,
fee_rate_sat_vb=fee_rate,
)
await client.broadcast(built.raw_hex)
except InsufficientFundsError:
logger.exception("round %s payout failed: insufficient pool UTXOs", round_id)
return
except Exception:
# Anything else — a malformed fee_address (EmbitError), a rejected
# broadcast, a dead connection. This used to escape all the way to
# run()'s catch-all, which logged it without recording anything, leaving
# no trace of *why* the round was stuck (B-05). The round stays in
# "paying_out" either way: automatic payout retry is still an open gap.
logger.exception("round %s payout failed", round_id)
# Anything else — a malformed fee_address (EmbitError) or similar. This
# used to escape all the way to run()'s catch-all, which logged it
# without recording anything, leaving no trace of *why* the round was
# stuck (B-05). The round stays in "paying_out" either way: automatic
# payout retry is still an open gap.
logger.exception("round %s payout build failed", round_id)
await self._log_payout_failure(round_id, winner_user_id)
return
# --- Phase 3: persist -----------------------------------------------------
# --- Phase 3: persist the intent, *then* broadcast (B-25) -----------------
async with self._session_factory() as session:
round_ = await session.get(Round, round_id)
round_.winner_amount_sats = built.winner_sats
round_.fee_amount_sats = built.commission_sats
round_.payout_txid = built.txid
session.add(
PendingTransaction(
kind="payout",
round_id=round_id,
current_txid=built.txid,
fee_rate_sat_vb=fee_rate,
raw_tx_hex=built.raw_hex,
status="pending",
)
pending = PendingTransaction(
kind="payout",
round_id=round_id,
current_txid=built.txid,
fee_rate_sat_vb=fee_rate,
raw_tx_hex=built.raw_hex,
# "building" until the broadcast succeeds, exactly like place_bet's
# two phases — see the reconciler, which gives this a short grace
# period before asking the chain whether it made it out after all.
status="building",
)
session.add(pending)
await session.commit()
pending_id = pending.id
# --- Phase 4: broadcast, then promote the pending row --------------------
try:
await client.broadcast(built.raw_hex)
except Exception:
# The row stays "building": the reconciler will ask the chain about it
# and, finding nothing, abandon it and clear payout_txid (B-25) — instead
# of the round being stuck with a payout_txid that never went anywhere.
logger.exception("round %s payout broadcast failed", round_id)
await self._log_payout_failure(round_id, winner_user_id)
return
async with self._session_factory() as session:
pending = await session.get(PendingTransaction, pending_id)
pending.status = "pending"
await write_audit_log(
session,
"payout_sent",
@@ -267,3 +319,25 @@ class RoundScheduler:
await session.commit()
except Exception:
logger.exception("could not record the payout failure of round %s", round_id)
async def _reserved_payout_outpoints(session: AsyncSession) -> set[tuple[str, int]]:
"""Pool UTXOs already claimed by a payout that hasn't resolved yet — this
round's own in-flight payout (guarded against separately in _trigger_payout) or
a stale one from an earlier round the reconciler hasn't abandoned yet (B-25).
These must be excluded from selection, or a retry would double-spend the same
coins into two payouts before the reconciler gets a chance to release them."""
rows = (
await session.scalars(
select(PendingTransaction).where(
PendingTransaction.kind == "payout",
PendingTransaction.status.in_(("building", "pending")),
)
)
).all()
reserved: set[tuple[str, int]] = set()
for row in rows:
tx = Transaction.parse(bytes.fromhex(row.raw_tx_hex))
for vin in tx.vin:
reserved.add((vin.txid.hex(), vin.vout))
return reserved
tests/unit/test_scheduler.py
+197 -2
View File
@@ -4,9 +4,10 @@ import pytest
from sqlalchemy import select
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 Round, RoundConfig
from app.rounds.scheduler import RoundScheduler
from app.db.models import AuditLog, PendingTransaction, Round, RoundConfig, User
from app.rounds.scheduler import RoundScheduler, _reserved_payout_outpoints
class FakeListener:
@@ -52,3 +53,197 @@ async def test_tick_closes_round_with_no_participants_once_due(session_factory,
async with session_factory() as session:
round_ = (await session.scalars(select(Round))).one()
assert round_.status == "closed"
# --- B-25: the payout must be persisted before it is broadcast, like bets/withdrawals ---
# A real, reusable PLM bech32 address so build_payout_transaction's
# script.Script.from_address(...) succeeds — this is not a value the scheduler
# validates itself (that's the admin panel's job for fee_address), it just needs to
# actually decode.
_WINNER_ADDRESS = "plm1q0xcqpzrky6eff2g52qdye53xkk9jxkvraxkkwd"
_FEE_ADDRESS = "plm1q5x25wd6q463mfhckjraaedgjg0lyu73qfcj43n"
_POOL_AMOUNT_SATS = 10_000_000_000 # 100 PLM
class FakePayoutClient:
def __init__(self, entries, *, fail_broadcast=False):
self._entries = entries
self._fail_broadcast = fail_broadcast
self.broadcasted: list[str] = []
async def listunspent(self, scripthash):
return self._entries
async def broadcast(self, raw_tx_hex):
if self._fail_broadcast:
raise RuntimeError("node rejected the transaction")
self.broadcasted.append(raw_tx_hex)
return "network-txid"
class FakePayoutListener:
def __init__(self, client):
self.client = client
@pytest.fixture
async def payout_session_factory(tmp_path, monkeypatch):
"""Same master-key bootstrap as test_broadcast.py's fixture: _trigger_payout
needs a real pool key to sign with."""
monkeypatch.setattr(settings, "master_key_path", str(tmp_path / "master.xprv.enc"))
monkeypatch.setattr(
settings,
"xprv_encryption_key",
__import__("cryptography.fernet", fromlist=["Fernet"]).Fernet.generate_key().decode(),
)
from app.wallet import hd
hd._account_key = None
hd.generate_master_key()
engine = create_async_engine("sqlite+aiosqlite:///:memory:")
async with engine.begin() as conn:
await conn.run_sync(Base.metadata.create_all)
yield async_sessionmaker(engine, expire_on_commit=False)
await engine.dispose()
hd._account_key = None
async def _seed_paying_out_round(session_factory, round_id: int = 1) -> int:
async with session_factory() as session:
winner = User(username="winner", password_hash="x", derivation_index=0, address=_WINNER_ADDRESS)
session.add(winner)
await session.flush()
session.add(RoundConfig(fee_address=_FEE_ADDRESS, fee_rate_sat_vb=1))
session.add(
Round(
id=round_id,
status="paying_out",
pool_amount_sats=_POOL_AMOUNT_SATS,
winner_user_id=winner.id,
)
)
await session.commit()
return winner.id
async def test_trigger_payout_persists_before_broadcasting(payout_session_factory):
"""The happy path: payout_txid and a PendingTransaction must exist once the
broadcast succeeds, promoted from "building" to "pending" — the two-phase write
that used to be missing entirely (B-25)."""
await _seed_paying_out_round(payout_session_factory)
entries = [{"tx_hash": "33" * 32, "tx_pos": 0, "height": 10, "value": _POOL_AMOUNT_SATS + 100_000}]
client = FakePayoutClient(entries)
scheduler = RoundScheduler(payout_session_factory, FakePayoutListener(client))
await scheduler._trigger_payout(1)
assert client.broadcasted
async with payout_session_factory() as session:
round_ = await session.get(Round, 1)
assert round_.payout_txid is not None
assert round_.winner_amount_sats and round_.fee_amount_sats
pending = (await session.scalars(select(PendingTransaction))).one()
assert pending.kind == "payout"
assert pending.status == "pending"
assert pending.current_txid == round_.payout_txid
events = [e.event_type for e in (await session.scalars(select(AuditLog))).all()]
assert "payout_sent" in events
async def test_trigger_payout_broadcast_failure_leaves_a_recoverable_row(payout_session_factory):
"""Before B-25, a broadcast rejection here left nothing behind — no payout_txid,
no PendingTransaction — because everything was persisted only after the
broadcast. Now the intent is already durable, so the reconciler has something to
resolve instead of the round being stuck with zero trace of what was attempted."""
await _seed_paying_out_round(payout_session_factory)
entries = [{"tx_hash": "44" * 32, "tx_pos": 0, "height": 10, "value": _POOL_AMOUNT_SATS + 100_000}]
client = FakePayoutClient(entries, fail_broadcast=True)
scheduler = RoundScheduler(payout_session_factory, FakePayoutListener(client))
await scheduler._trigger_payout(1)
assert not client.broadcasted
async with payout_session_factory() as session:
round_ = await session.get(Round, 1)
assert round_.payout_txid is not None # durable, even though the broadcast failed
pending = (await session.scalars(select(PendingTransaction))).one()
assert pending.kind == "payout"
assert pending.status == "building" # not lost — the reconciler resolves this
assert pending.current_txid == round_.payout_txid
events = [e.event_type for e in (await session.scalars(select(AuditLog))).all()]
assert "payout_failed" in events
async def test_trigger_payout_skips_when_already_in_flight(payout_session_factory):
"""A second call for a round that already has a non-terminal payout
PendingTransaction must not build (and broadcast) another one — that would pay
the winner twice."""
winner_id = await _seed_paying_out_round(payout_session_factory)
async with payout_session_factory() as session:
round_ = await session.get(Round, 1)
round_.payout_txid = "already-sent-txid"
session.add(
PendingTransaction(
kind="payout",
round_id=1,
current_txid="already-sent-txid",
fee_rate_sat_vb=1,
raw_tx_hex="00",
status="pending",
)
)
await session.commit()
entries = [{"tx_hash": "55" * 32, "tx_pos": 0, "height": 10, "value": _POOL_AMOUNT_SATS + 100_000}]
client = FakePayoutClient(entries)
scheduler = RoundScheduler(payout_session_factory, FakePayoutListener(client))
await scheduler._trigger_payout(1)
assert not client.broadcasted
async with payout_session_factory() as session:
assert (await session.scalars(select(PendingTransaction))).all() # still just the one seeded
rows = (await session.scalars(select(PendingTransaction))).all()
assert len(rows) == 1
assert rows[0].current_txid == "already-sent-txid"
async def test_reserved_payout_outpoints_excludes_utxos_claimed_by_a_stale_payout(payout_session_factory):
"""A payout still "building"/"pending" for some round — most plausibly a stale
one the reconciler hasn't abandoned yet — must keep its inputs off the table for
a fresh payout attempt, or the same pool coins could be spent twice."""
from embit import script
from embit.transaction import Transaction, TransactionInput, TransactionOutput
reserved_txid = "66" * 32
raw_tx = (
Transaction(
vin=[TransactionInput(bytes.fromhex(reserved_txid), 2)],
vout=[TransactionOutput(1_000_000, script.Script.from_address(_WINNER_ADDRESS))],
)
.serialize()
.hex()
)
async with payout_session_factory() as session:
session.add(
PendingTransaction(
kind="payout",
round_id=99,
current_txid="stale-payout-txid",
fee_rate_sat_vb=1,
raw_tx_hex=raw_tx,
status="building",
)
)
await session.commit()
reserved = await _reserved_payout_outpoints(session)
assert reserved == {(reserved_txid, 2)}