Distinguish a pending-only balance from a truly insufficient one (B-37)

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>
This commit is contained in:
2026-07-27 14:23:00 +02:00
co-authored by Claude Sonnet 5
parent 7fa26df104
commit 0b44fe632e
5 changed files with 64 additions and 30 deletions
+8 -26
View File
@@ -1,42 +1,23 @@
# Known bugs # Known bugs
A second full-codebase audit on 2026-07-27 found **25 further issues** (4 critical, 6 high, 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-33 … B-49. B-25 through B-36 are fixed (see "Previously 7 medium, 8 low), listed below as B-33 … B-49. B-25 through B-37 are fixed (see "Previously
fixed" below) — no Critical-severity finding remains open; the other 13 are High/Medium/Low. fixed" below) — no Critical-severity finding remains open; the other 12 are High/Medium/Low.
The 139-test suite was green at the time of the audit, so none of these were caught by existing 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 (the twelve fixes so far brought the suite coverage — every fix lands with a regression test (the thirteen fixes so far brought the suite
from 139 to 200). from 139 to 201).
The recurring pattern across the open findings is worth stating once: the code is rigorous The recurring pattern across the open findings 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. about the failure modes that have actually been hit, and silent about the ones that have not.
The payout phase is now fully recoverable; the "drawing" phase (waiting on a block) is now The payout phase is now fully recoverable; the "drawing" phase (waiting on a block) is now
observable (B-36) but still has no equivalent resume-after-restart — see "Known gaps / TODO" observable (B-36) but still has no equivalent resume-after-restart — see "Known gaps / TODO"
in [CLAUDE.md](CLAUDE.md). in [CLAUDE.md](CLAUDE.md), which is also where other by-design limitations (single-shared-token
admin auth, single-process assumptions, no user-facing history, etc.) are documented.
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
[CLAUDE.md](CLAUDE.md).
--- ---
## Medium ## Medium
### B-37 — Displayed balance and spendable balance diverge, and the error does not explain it
After a bet the change is unconfirmed, so `cached_balance_sats` ≈ 0 while the UI shows
`pending_balance_sats` (the real figure). A withdrawal attempted right after validates against
**confirmed** UTXOs (`withdrawals/service.py:54-60`) and answers `insufficient_balance`.
The user sees "1.000 PLM" on screen and is told they have no funds. The mechanism is a
documented design decision, but the error does not distinguish "you don't have the money" from
"your money is waiting to confirm" — two very different situations for whoever reads it.
**Proposed fix.** A distinct error code (e.g. `balance_pending_confirmation`) raised when the
requested amount is covered by `pending_balance_sats` but not by the confirmed balance,
carrying the pending amount in `params`, plus its `error.*` entry in all 7 languages. The
withdrawal form should also cap/hint the max against the confirmed balance rather than the
displayed one.
### B-38 — The 500-subscriber SSE cap is a zero-cost DoS of the realtime feature ### B-38 — The 500-subscriber SSE cap is a zero-cost DoS of the realtime feature
`GET /rounds/stream` requires no authentication and each connection takes a slot on a `GET /rounds/stream` requires no authentication and each connection takes a slot on a
@@ -174,9 +155,10 @@ already does.
- **B-34** — password change/reset didn't invalidate already-issued JWTs, so a stolen token survived a change meant to lock it out - **B-34** — password change/reset didn't invalidate already-issued JWTs, so a stolen token survived a change meant to lock it out
- **B-35** — API timestamps round-tripped as naive datetimes, so the frontend parsed them as local time instead of UTC - **B-35** — API timestamps round-tripped as naive datetimes, so the frontend parsed them as local time instead of UTC
- **B-36** — a stalled draw wait had no timeout, no log, and no audit trail, so a frozen round showed nothing in `/admin` - **B-36** — a stalled draw wait had no timeout, no log, and no audit trail, so a frozen round showed nothing in `/admin`
- **B-37** — a withdrawal covered by unconfirmed change answered "insufficient balance" instead of distinguishing it from actually having no funds
See git history for the fix-by-fix breakdown (commits `f13f685`, `50a43ae`, `933760e`, and the See git history for the fix-by-fix breakdown (commits `f13f685`, `50a43ae`, `933760e`, and the
B-28/B-29/B-30/B-31/B-32/B-33/B-34/B-35/B-36 fixes). Suite grew from 139 to 200 tests over the twelve. B-28/B-29/B-30/B-31/B-32/B-33/B-34/B-35/B-36/B-37 fixes). Suite grew from 139 to 201 tests over the thirteen.
A full-codebase audit on 2026-07-26 (commit `d4e0974`) found 24 bugs across every Python 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, module under `app/`, both static frontends, and the Docker/Caddy deployment — 5 critical,
+2 -2
View File
@@ -12,7 +12,7 @@ All 10 stages of the original build order are code-complete and unit-tested —
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. 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.
**Read [BUGS.md](BUGS.md) before trusting any behaviour here.** Two audits: 2026-07-26 found 24 bugs (5 critical), all fixed; 2026-07-27 found 25 more (B-25 … B-49), of which **13 are still open** — no Critical or High remains, only Medium/Low: no WAL/`busy_timeout` under five concurrent SQLite writer tasks (B-39), a 500-subscriber SSE cap that doubles as a cheap DoS of the realtime feature (B-38), among others. BUGS.md is the live open list with a proposed fix per finding; "Known gaps" at the end of this file is for limitations accepted **by design** instead. Don't fix a BUGS.md item silently as a side effect of other work — each fix lands with its own regression test. **Read [BUGS.md](BUGS.md) before trusting any behaviour here.** Two audits: 2026-07-26 found 24 bugs (5 critical), all fixed; 2026-07-27 found 25 more (B-25 … B-49), of which **12 are still open** — no Critical or High remains, only Medium/Low: no WAL/`busy_timeout` under five concurrent SQLite writer tasks (B-39), a 500-subscriber SSE cap that doubles as a cheap DoS of the realtime feature (B-38), among others. BUGS.md is the live open list with a proposed fix per finding; "Known gaps" at the end of this file is for limitations accepted **by design** instead. Don't fix a BUGS.md item silently as a side effect of other work — each fix lands with its own regression test.
Before writing code, read the "Architecture" section below in full plus the diagrams in [flowchart/](flowchart/): [platform-overview.mmd](flowchart/platform-overview.mmd) (the 5-phase flow) and [round-lifecycle.mmd](flowchart/round-lifecycle.mmd) (the round/draw lifecycle). Every node **and edge label** (conditions, retries, loops) is a behaviour that must be implemented as described. Regenerate the companion PDFs with `flowchart/render-pdf.sh <file>.mmd` after editing either. Before writing code, read the "Architecture" section below in full plus the diagrams in [flowchart/](flowchart/): [platform-overview.mmd](flowchart/platform-overview.mmd) (the 5-phase flow) and [round-lifecycle.mmd](flowchart/round-lifecycle.mmd) (the round/draw lifecycle). Every node **and edge label** (conditions, retries, loops) is a behaviour that must be implemented as described. Regenerate the companion PDFs with `flowchart/render-pdf.sh <file>.mmd` after editing either.
@@ -174,7 +174,7 @@ At 120s blocks that's ~46 min worst case (last bet confirms right at the dead
`place_bet`/`request_withdrawal` select whole UTXOs (`select_utxos`, largest-first) and mark each `spent_txid` at broadcast time, long before any confirmation. `cached_balance_sats` (`recompute_balance`) sums only confirmed, unspent UTXOs, so right after a bet it understates the real balance by the whole unconfirmed change — often far more than the amount actually moving. `place_bet`/`request_withdrawal` select whole UTXOs (`select_utxos`, largest-first) and mark each `spent_txid` at broadcast time, long before any confirmation. `cached_balance_sats` (`recompute_balance`) sums only confirmed, unspent UTXOs, so right after a bet it understates the real balance by the whole unconfirmed change — often far more than the amount actually moving.
`compute_pending_balance` (`app/wallet/balance.py`) fixes the *displayed* number without changing what's spendable: it decodes the raw tx of every in-flight (`pending`) bet/withdrawal for the user and adds back the outputs paying to the user's own address. `GET /users/me` returns both — `balance_sats` (confirmed only; still what withdrawal-max and spend logic use, since only confirmed UTXOs are spendable) and `pending_balance_sats` + `has_pending` (what the UI shows: green when settled, amber while pending). The gap is user-visible and currently under-explained in errors (B-37). `compute_pending_balance` (`app/wallet/balance.py`) fixes the *displayed* number without changing what's spendable: it decodes the raw tx of every in-flight (`pending`) bet/withdrawal for the user and adds back the outputs paying to the user's own address. `GET /users/me` returns both — `balance_sats` (confirmed only; still what withdrawal-max and spend logic use, since only confirmed UTXOs are spendable) and `pending_balance_sats` + `has_pending` (what the UI shows: green when settled, amber while pending). A withdrawal whose amount is covered by the pending-inclusive balance but not the confirmed one gets `balance_pending_confirmation` instead of a flat `insufficient_balance` (B-37), so the error doesn't contradict what the user is looking at.
## Real-time updates (SSE) ## Real-time updates (SSE)
+7
View File
@@ -121,6 +121,7 @@ const TRANSLATIONS = {
'error.round_closing': 'The current round is closing, please try again shortly.', 'error.round_closing': 'The current round is closing, please try again shortly.',
'error.already_betting': 'You already have an active bet in the current round.', 'error.already_betting': 'You already have an active bet in the current round.',
'error.insufficient_balance': 'Insufficient balance.', 'error.insufficient_balance': 'Insufficient balance.',
'error.balance_pending_confirmation': 'You have {pending_plm} PLM pending confirmation — it is not spendable yet.',
'error.amount_below_network_fee': 'The amount is too small to cover the network fee.', 'error.amount_below_network_fee': 'The amount is too small to cover the network fee.',
'error.invalid_address': 'Not a valid PLM address (it must start with plm1q…).', 'error.invalid_address': 'Not a valid PLM address (it must start with plm1q…).',
'error.amount_below_minimum': 'The minimum withdrawal amount is {minimum_plm} PLM.', 'error.amount_below_minimum': 'The minimum withdrawal amount is {minimum_plm} PLM.',
@@ -259,6 +260,7 @@ const TRANSLATIONS = {
'error.round_closing': 'Il round corrente si sta chiudendo, riprova tra poco.', 'error.round_closing': 'Il round corrente si sta chiudendo, riprova tra poco.',
'error.already_betting': 'Hai già una bet attiva nel round corrente.', 'error.already_betting': 'Hai già una bet attiva nel round corrente.',
'error.insufficient_balance': 'Saldo insufficiente.', 'error.insufficient_balance': 'Saldo insufficiente.',
'error.balance_pending_confirmation': 'Hai {pending_plm} PLM in attesa di conferma — non ancora disponibili per la spesa.',
'error.amount_below_network_fee': "L'importo è troppo basso per coprire la fee di rete.", 'error.amount_below_network_fee': "L'importo è troppo basso per coprire la fee di rete.",
'error.invalid_address': 'Indirizzo PLM non valido (deve iniziare con plm1q…).', 'error.invalid_address': 'Indirizzo PLM non valido (deve iniziare con plm1q…).',
'error.amount_below_minimum': "L'importo minimo di prelievo è {minimum_plm} PLM.", 'error.amount_below_minimum': "L'importo minimo di prelievo è {minimum_plm} PLM.",
@@ -397,6 +399,7 @@ const TRANSLATIONS = {
'error.round_closing': 'La ronda actual se está cerrando, inténtalo de nuevo en un momento.', 'error.round_closing': 'La ronda actual se está cerrando, inténtalo de nuevo en un momento.',
'error.already_betting': 'Ya tienes una apuesta activa en la ronda actual.', 'error.already_betting': 'Ya tienes una apuesta activa en la ronda actual.',
'error.insufficient_balance': 'Saldo insuficiente.', 'error.insufficient_balance': 'Saldo insuficiente.',
'error.balance_pending_confirmation': 'Tienes {pending_plm} PLM pendientes de confirmación — todavía no se pueden gastar.',
'error.amount_below_network_fee': 'El importe es demasiado pequeño para cubrir la comisión de red.', 'error.amount_below_network_fee': 'El importe es demasiado pequeño para cubrir la comisión de red.',
'error.invalid_address': 'Dirección PLM no válida (debe empezar por plm1q…).', 'error.invalid_address': 'Dirección PLM no válida (debe empezar por plm1q…).',
'error.amount_below_minimum': 'El importe mínimo de retiro es {minimum_plm} PLM.', 'error.amount_below_minimum': 'El importe mínimo de retiro es {minimum_plm} PLM.',
@@ -535,6 +538,7 @@ const TRANSLATIONS = {
'error.round_closing': 'Le round en cours est en train de se fermer, réessayez dans un instant.', 'error.round_closing': 'Le round en cours est en train de se fermer, réessayez dans un instant.',
'error.already_betting': 'Vous avez déjà une mise active dans le round en cours.', 'error.already_betting': 'Vous avez déjà une mise active dans le round en cours.',
'error.insufficient_balance': 'Solde insuffisant.', 'error.insufficient_balance': 'Solde insuffisant.',
'error.balance_pending_confirmation': 'Vous avez {pending_plm} PLM en attente de confirmation — pas encore disponibles.',
'error.amount_below_network_fee': 'Le montant est trop faible pour couvrir les frais de réseau.', 'error.amount_below_network_fee': 'Le montant est trop faible pour couvrir les frais de réseau.',
'error.invalid_address': 'Adresse PLM invalide (elle doit commencer par plm1q…).', 'error.invalid_address': 'Adresse PLM invalide (elle doit commencer par plm1q…).',
'error.amount_below_minimum': 'Le montant minimum de retrait est de {minimum_plm} PLM.', 'error.amount_below_minimum': 'Le montant minimum de retrait est de {minimum_plm} PLM.',
@@ -673,6 +677,7 @@ const TRANSLATIONS = {
'error.round_closing': 'Die laufende Runde wird gerade geschlossen, bitte versuche es gleich erneut.', 'error.round_closing': 'Die laufende Runde wird gerade geschlossen, bitte versuche es gleich erneut.',
'error.already_betting': 'Du hast bereits eine aktive Wette in der laufenden Runde.', 'error.already_betting': 'Du hast bereits eine aktive Wette in der laufenden Runde.',
'error.insufficient_balance': 'Nicht genügend Guthaben.', 'error.insufficient_balance': 'Nicht genügend Guthaben.',
'error.balance_pending_confirmation': 'Sie haben {pending_plm} PLM, die noch auf Bestätigung warten — noch nicht verfügbar.',
'error.amount_below_network_fee': 'Der Betrag ist zu klein, um die Netzwerkgebühr zu decken.', 'error.amount_below_network_fee': 'Der Betrag ist zu klein, um die Netzwerkgebühr zu decken.',
'error.invalid_address': 'Keine gültige PLM-Adresse (sie muss mit plm1q… beginnen).', 'error.invalid_address': 'Keine gültige PLM-Adresse (sie muss mit plm1q… beginnen).',
'error.amount_below_minimum': 'Der Mindestauszahlungsbetrag beträgt {minimum_plm} PLM.', 'error.amount_below_minimum': 'Der Mindestauszahlungsbetrag beträgt {minimum_plm} PLM.',
@@ -811,6 +816,7 @@ const TRANSLATIONS = {
'error.round_closing': 'Текущий раунд закрывается, повторите попытку чуть позже.', 'error.round_closing': 'Текущий раунд закрывается, повторите попытку чуть позже.',
'error.already_betting': 'У вас уже есть активная ставка в текущем раунде.', 'error.already_betting': 'У вас уже есть активная ставка в текущем раунде.',
'error.insufficient_balance': 'Недостаточно средств.', 'error.insufficient_balance': 'Недостаточно средств.',
'error.balance_pending_confirmation': 'У вас есть {pending_plm} PLM, ожидающих подтверждения — они пока недоступны для расходования.',
'error.amount_below_network_fee': 'Сумма слишком мала, чтобы покрыть комиссию сети.', 'error.amount_below_network_fee': 'Сумма слишком мала, чтобы покрыть комиссию сети.',
'error.invalid_address': 'Некорректный адрес PLM (он должен начинаться с plm1q…).', 'error.invalid_address': 'Некорректный адрес PLM (он должен начинаться с plm1q…).',
'error.amount_below_minimum': 'Минимальная сумма вывода — {minimum_plm} PLM.', 'error.amount_below_minimum': 'Минимальная сумма вывода — {minimum_plm} PLM.',
@@ -949,6 +955,7 @@ const TRANSLATIONS = {
'error.round_closing': '当前回合正在结束,请稍后重试。', 'error.round_closing': '当前回合正在结束,请稍后重试。',
'error.already_betting': '你在当前回合已有一笔有效下注。', 'error.already_betting': '你在当前回合已有一笔有效下注。',
'error.insufficient_balance': '余额不足。', 'error.insufficient_balance': '余额不足。',
'error.balance_pending_confirmation': '您有 {pending_plm} PLM 待确认 —— 尚不可用于支出。',
'error.amount_below_network_fee': '金额太小,不足以支付网络手续费。', 'error.amount_below_network_fee': '金额太小,不足以支付网络手续费。',
'error.invalid_address': 'PLM 地址无效(必须以 plm1q… 开头)。', 'error.invalid_address': 'PLM 地址无效(必须以 plm1q… 开头)。',
'error.amount_below_minimum': '最低提现金额为 {minimum_plm} PLM。', 'error.amount_below_minimum': '最低提现金额为 {minimum_plm} PLM。',
+17 -2
View File
@@ -9,7 +9,7 @@ from app.electrum.client import ElectrumClient
from app.rounds.config import get_round_config from app.rounds.config import get_round_config
from app.rounds.events import broadcaster from app.rounds.events import broadcaster
from app.wallet.address import is_valid_plm_address from app.wallet.address import is_valid_plm_address
from app.wallet.balance import recompute_balance from app.wallet.balance import compute_pending_balance, recompute_balance
from app.wallet.hd import derive_user_key from app.wallet.hd import derive_user_key
from app.wallet.psbt_builder import ( from app.wallet.psbt_builder import (
BuiltTransaction, BuiltTransaction,
@@ -56,7 +56,22 @@ async def request_withdrawal(
select(UtxoEvent).where(UtxoEvent.user_id == user.id, UtxoEvent.spent_txid.is_(None)) select(UtxoEvent).where(UtxoEvent.user_id == user.id, UtxoEvent.spent_txid.is_(None))
) )
).all() ).all()
if sum(u.amount_sats for u in unspent) < amount_sats: confirmed_sats = sum(u.amount_sats for u in unspent)
if confirmed_sats < amount_sats:
# B-37: cached_balance_sats (== confirmed_sats here) can understate the real
# balance by a whole unconfirmed change output right after a bet/withdrawal —
# the UI shows pending_balance_sats instead (compute_pending_balance), which
# can cover an amount this check would otherwise reject as flatly
# "insufficient". Distinguish "you don't have the money" from "your money
# hasn't confirmed yet" so the error doesn't contradict what the user is
# looking at on screen.
pending_inclusive_sats, has_pending = await compute_pending_balance(session, user)
if has_pending and pending_inclusive_sats >= amount_sats:
raise WithdrawalError(
"balance_pending_confirmation",
"the requested amount is covered by your pending balance, which has not confirmed yet",
pending_sats=pending_inclusive_sats - confirmed_sats,
)
raise WithdrawalError("insufficient_balance", "insufficient balance", required_sats=amount_sats) raise WithdrawalError("insufficient_balance", "insufficient balance", required_sats=amount_sats)
user_key = derive_user_key(user.derivation_index) user_key = derive_user_key(user.derivation_index)
+30
View File
@@ -2,6 +2,7 @@ import pytest
from sqlalchemy import select from sqlalchemy import select
from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
from app.bets.service import place_bet
from app.config import settings from app.config import settings
from app.db.base import Base from app.db.base import Base
from app.db.models import PendingTransaction, User, UtxoEvent, Withdrawal from app.db.models import PendingTransaction, User, UtxoEvent, Withdrawal
@@ -99,6 +100,35 @@ async def test_withdrawal_rejects_insufficient_balance(session_factory):
await request_withdrawal(session, client, user, EXTERNAL_ADDRESS, BET_AMOUNT_SATS) await request_withdrawal(session, client, user, EXTERNAL_ADDRESS, BET_AMOUNT_SATS)
async def test_withdrawal_distinguishes_pending_from_truly_insufficient_balance(session_factory):
"""B-37: right after a bet, cached_balance_sats is ~0 because the whole funding
UTXO was spent as input and the change hasn't confirmed yet — but the UI shows
the pending-inclusive balance (compute_pending_balance), which does cover a
withdrawal of this size. The error must say "not confirmed yet", not flatly
"insufficient balance", or it contradicts what the user is looking at."""
user_id = await _make_funded_user(session_factory, 4, 3_000_000_000)
bet_client = FakeElectrumClient()
async with session_factory() as session:
user = await session.get(User, user_id)
await place_bet(session, bet_client, user)
async with session_factory() as session:
user = await session.get(User, user_id)
assert user.cached_balance_sats == 0 # the whole funding UTXO was spent as input
withdraw_client = FakeElectrumClient()
with pytest.raises(WithdrawalError) as exc_info:
# Above the withdrawal minimum (BET_AMOUNT_SATS) and covered by the
# unconfirmed change (~1_999_800_000 sats), but not by the (zero)
# confirmed balance.
await request_withdrawal(session, withdraw_client, user, EXTERNAL_ADDRESS, 1_500_000_000)
assert exc_info.value.code == "balance_pending_confirmation"
assert exc_info.value.params["pending_sats"] > 0
assert not withdraw_client.broadcasted
@pytest.mark.parametrize( @pytest.mark.parametrize(
"address", "address",
[ [