Corroborate deposit credits, not just external spends (B-59)

A candidate external spend has needed a quorum since B-29, but `value` and
`height` for a *credit* came from the single active connection and went straight
into utxo_events. One hostile or broken server could therefore inflate a user's
displayed balance with outpoints that don't exist. It never spends anyone else's
coins — a bet or withdrawal built on a phantom UTXO is refused at broadcast and
rolled back — but it wedges the balance display and burns build attempts, and on
a custodial platform a balance that isn't real is a support incident either way.
Balances move in both directions; both directions now need the same quorum.

corroborate_utxo_credit asks the other configured servers whether they report
the same outpoint, for the same amount, confirmed. The height itself isn't
compared: a server still catching up reports height 0 and simply doesn't agree,
which is the same answer, while two honest servers can't disagree on the height
of a genuinely confirmed outpoint.

refresh_user gains the phase that shape already implied: find_new_credit_
candidates (new, confirmed, not already held) inside the first session,
corroboration outside any session, then credit_confirmed_utxos over what
survived. Only new outpoints are corroborated — re-checking what we already hold
would open a connection to every other server on every refresh for an answer
that can no longer change anything.

A failed corroboration delays a credit, it never loses one: the next scripthash
notification or DepositReconciler sweep (300s) re-offers the same outpoint, and
the withholding is logged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

BUGS.md

@@ -40,21 +40,6 @@ remains the last prerequisite for running unattended.
 
 ## High — security
 
-### B-59 — deposit crediting is not corroborated, unlike external-spend detection
-
-`app/deposits/service.py:31-45`, `app/electrum/listener.py:401-433`.
-
-A candidate external spend is corroborated across the other configured servers
-before it can reduce a balance (B-29), but `value` and `height` for a *credit*
-are taken from the single active connection with no cross-check. A hostile or
-broken server can inflate a user's displayed balance with outpoints that do not
-exist. The blast radius is bounded — a bet or withdrawal built on a phantom UTXO
-is refused at broadcast and the rollback releases it — but it wedges the user's
-balance display and burns build attempts.
-
-Fix: either corroborate credits the same way (symmetry with B-29), or record the
-asymmetry explicitly as an accepted risk in CLAUDE.md with its bound stated.
-
 ### B-60 — `POST /admin/bug-reports/{id}/status` writes no audit entry
 
 `app/api/routes/admin.py:404-419`.

CLAUDE.md

@@ -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 — 296 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 — 302 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 296 tests
+python -m pytest                                      # all 302 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
 ```
@@ -140,7 +140,7 @@ One connection serves everything — deposit credits, broadcasts, confirmations,
 - **Bounded requests** (`_REQUEST_TIMEOUT_SECONDS` = 15s); a timeout tears the connection down. Unbounded waits used to hang `POST /bets` *while holding the per-user lock*, and could stall the confirmation poller permanently.
 - **The drop is observable**: `client.wait_closed()` resolves when the read loop dies, and `_run_once` races it against the notification consumers and a 60s `server.ping`. Without it the listener sat on queues nobody would ever fill while `listener.client` still looked alive.
 - **Headers are validated, not trusted** (`_apply_header`): the tip never regresses, a header must meet the difficulty target it claims, and a single-block advance must chain from the current tip's hash. Failure raises `HeaderValidationError`, which ends the session like a dropped connection and rotates away — that header is the draw's only entropy, so a fabricated one picks the winner.
-- **A quorum corroborates the two money-moving decisions** (`_corroborate_majority`, 10s per server, asking only the *other* endpoints — never the active one, which is what a MITM controls): `corroborate_header` before a block seeds the draw (B-28), `corroborate_utxo_spent` before a UTXO missing from one `listunspent` is written off as externally spent (B-29). No fallbacks configured → returns True (the accepted risk of an empty `ELECTRUM_FALLBACK_SERVERS`); nobody answers → returns **False**, since an unreachable network proves nothing.
+- **A quorum corroborates every money-moving decision** (`_corroborate_majority`, 10s per server, asking only the *other* endpoints — never the active one, which is what a MITM controls): `corroborate_header` before a block seeds the draw (B-28), `corroborate_utxo_spent` before a UTXO missing from one `listunspent` is written off as externally spent (B-29), and `corroborate_utxo_credit` before a new outpoint credits a balance — same outpoint, same amount, confirmed (B-59). Balances move in both directions, so both directions need the same quorum. No fallbacks configured → returns True (the accepted risk of an empty `ELECTRUM_FALLBACK_SERVERS`); nobody answers → returns **False**, since an unreachable network proves nothing. A failed credit corroboration only *delays*: `find_new_credit_candidates` re-offers the outpoint on the next refresh or `DepositReconciler` sweep.
 
 On reconnect `_subscribe_all_users` runs as its own task with bounded concurrency (`_RESUBSCRIBE_CONCURRENCY` = 20) rather than inline and serially — otherwise a large user base froze `tip_height`, and with it an in-flight draw, for the whole sweep (B-31); one user's failure is logged and skipped. `address_for_new_user` (called right after registration) is best-effort by design: on failure that address stays unsubscribed until the next reconnect or `DepositReconciler` sweep.
 
@@ -150,7 +150,7 @@ Diagrams: [platform-overview.mmd](flowchart/platform-overview.mmd), [round-lifec
 
 **REG** — on signup the server derives a P2WPKH address via BIP84 (`m/84'/coin'/0'/0/index`, one index per user) from the encrypted master xprv. Permanent, and doubles as deposit address, winnings address and withdrawal change address.
 
-**DEP** — the listener subscribes to the user's scripthash; balance is credited after **1 confirmation**, with the 1-conf reorg risk knowingly accepted and no rollback logic. `deposits/service.py` also detects UTXOs that vanished (spent outside the platform — corroborated per B-29 first) and *reinstates* ones that reappear.
+**DEP** — the listener subscribes to the user's scripthash; balance is credited after **1 confirmation** and only once the other servers corroborate the outpoint and its amount (B-59), with the 1-conf reorg risk knowingly accepted and no rollback logic. `deposits/service.py` also detects UTXOs that vanished (spent outside the platform — corroborated per B-29 first) and *reinstates* ones that reappear.
 
 **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.
 

app/deposits/service.py

@@ -11,6 +11,32 @@ from app.wallet.balance import recompute_balance
 logger = logging.getLogger(__name__)
 
 
+async def find_new_credit_candidates(session: AsyncSession, user_id: int, entries: list[dict]) -> list[dict]:
+    """The subset of `entries` that would actually credit something: confirmed
+    (height > 0, per the Electrum convention where <= 0 means mempool) and not
+    already recorded.
+
+    Split out from credit_confirmed_utxos so the caller
+    (electrum/listener.py:refresh_user) can corroborate each *new* outpoint
+    against the other configured servers before any of it is written (B-59) —
+    the mirror image of what B-29 already required before a balance may go
+    *down*. Only new ones: corroborating outpoints already credited would open a
+    connection to every other server on every refresh, for an answer that can no
+    longer change what we hold.
+    """
+    existing_keys = {
+        (txid, vout)
+        for txid, vout in (
+            await session.execute(select(UtxoEvent.txid, UtxoEvent.vout).where(UtxoEvent.user_id == user_id))
+        ).all()
+    }
+    return [
+        entry
+        for entry in entries
+        if entry["height"] > 0 and (entry["tx_hash"], entry["tx_pos"]) not in existing_keys
+    ]
+
+
 async def credit_confirmed_utxos(session: AsyncSession, user_id: int, entries: list[dict]) -> int:
     """Insert utxo_events for newly-confirmed entries from an Electrum
     `listunspent` response (idempotent on txid+vout), refresh the user's cached

app/electrum/listener.py

@@ -8,6 +8,7 @@ from sqlalchemy.ext.asyncio import async_sessionmaker
 from app.db.models import User
 from app.deposits.service import (
     credit_confirmed_utxos,
+    find_new_credit_candidates,
     find_utxos_missing_from,
     mark_utxos_spent_externally,
     reinstate_reappeared_utxos,
@@ -381,6 +382,51 @@ class ElectrumListener:
 
         return await self._corroborate_majority(_ask, lambda agrees: agrees, f"outpoint {txid}:{vout}")
 
+    async def corroborate_utxo_credit(self, scripthash: str, txid: str, vout: int, value: int) -> bool:
+        """B-59: the mirror of corroborate_utxo_spent, for money going the other
+        way. A candidate external spend could not reduce a balance without a
+        quorum, but `value` and `height` for a *credit* were taken from the single
+        active connection and written straight to the DB — so one hostile or broken
+        server could inflate a user's displayed balance with outpoints that don't
+        exist. That never spends anyone else's coins (a bet or withdrawal built on
+        a phantom UTXO is refused at broadcast and rolled back), but it wedges the
+        balance display and burns build attempts, and on a custodial platform a
+        balance that isn't real is a support incident either way.
+
+        Agreement means: this server also reports the outpoint as unspent, for the
+        same amount, and considers it confirmed. The height itself is not compared —
+        a server still catching up reports the entry at height 0 and simply doesn't
+        agree, which is the same answer, while for a genuinely confirmed outpoint
+        two honest servers cannot disagree on the height anyway.
+
+        A failure here delays a credit, it never loses one: the next scripthash
+        notification or DepositReconciler sweep (300s) retries it, and a deposit is
+        only credited once the quorum agrees.
+        """
+
+        async def _ask(endpoint: ElectrumEndpoint) -> bool | None:
+            client = self._client_factory(endpoint)
+            try:
+                await asyncio.wait_for(client.connect(), timeout=_CORROBORATION_TIMEOUT_SECONDS)
+                entries = await asyncio.wait_for(
+                    client.listunspent(scripthash), timeout=_CORROBORATION_TIMEOUT_SECONDS
+                )
+                return any(
+                    e.get("tx_hash") == txid
+                    and e.get("tx_pos") == vout
+                    and e.get("value") == value
+                    and (e.get("height") or 0) > 0
+                    for e in entries
+                )
+            except Exception:
+                return None
+            finally:
+                await client.close()
+
+        return await self._corroborate_majority(
+            _ask, lambda agrees: agrees, f"credit of {value} sats at {txid}:{vout}"
+        )
+
     async def _consume_headers(self, queue: asyncio.Queue) -> None:
         while True:
             params = await queue.get()
@@ -400,20 +446,41 @@ class ElectrumListener:
 
     async def refresh_user(self, user_id: int, scripthash: str) -> None:
         """Three phases, so no DB session is held across a network call (B-18),
-        same shape as _trigger_payout: read what's needed, corroborate any
+        same shape as _trigger_payout: read what's needed, corroborate every
-        candidate external spends against other servers (B-29), then persist.
+        balance-moving candidate against other servers — new credits (B-59) as
+        well as candidate external spends (B-29) — then persist.
         """
         assert self.client is not None
         entries = await self.client.listunspent(scripthash)
 
         async with self._session_factory() as session:
-            credited = await credit_confirmed_utxos(session, user_id, entries)
+            credit_candidates = await find_new_credit_candidates(session, user_id, entries)
             reinstated = await reinstate_reappeared_utxos(session, user_id, entries)
             candidates = [
                 (row.id, row.txid, row.vout)
                 for row in await find_utxos_missing_from(session, user_id, entries)
             ]
 
+        corroborated_credits = [
+            entry
+            for entry in credit_candidates
+            if await self.corroborate_utxo_credit(
+                scripthash, entry["tx_hash"], entry["tx_pos"], entry["value"]
+            )
+        ]
+
+        credited = 0
+        if corroborated_credits:
+            async with self._session_factory() as session:
+                credited = await credit_confirmed_utxos(session, user_id, corroborated_credits)
+        if len(corroborated_credits) < len(credit_candidates):
+            logger.warning(
+                "%s new UTXO(s) for user_id=%s not corroborated by the other servers — "
+                "not credited yet, will retry on the next refresh",
+                len(credit_candidates) - len(corroborated_credits),
+                user_id,
+            )
+
         confirmed_ids = [
             utxo_id
             for utxo_id, txid, vout in candidates

tests/unit/test_deposits.py

@@ -184,3 +184,26 @@ async def test_reinstate_reappeared_utxos_ignores_unmarked_rows(session_factory,
     async with session_factory() as session:
         reinstated = await reinstate_reappeared_utxos(session, user_id, entries)
         assert reinstated == 0
+
+
+async def test_find_new_credit_candidates_skips_unconfirmed_and_already_known(session_factory, user_id):  # B-59
+    """What the caller has to corroborate before crediting: only entries that would
+    actually write something. Re-corroborating what we already hold would open a
+    connection to every other server on every refresh, for an answer that can no
+    longer change anything."""
+    from app.deposits.service import find_new_credit_candidates
+
+    async with session_factory() as session:
+        await credit_confirmed_utxos(
+            session, user_id, [{"tx_hash": "aa" * 32, "tx_pos": 0, "height": 100, "value": 1_000}]
+        )
+
+    entries = [
+        {"tx_hash": "aa" * 32, "tx_pos": 0, "height": 100, "value": 1_000},  # already credited
+        {"tx_hash": "bb" * 32, "tx_pos": 0, "height": 0, "value": 2_000},  # still in the mempool
+        {"tx_hash": "cc" * 32, "tx_pos": 1, "height": 101, "value": 3_000},  # genuinely new
+    ]
+    async with session_factory() as session:
+        candidates = await find_new_credit_candidates(session, user_id, entries)
+
+    assert [(c["tx_hash"], c["tx_pos"]) for c in candidates] == [("cc" * 32, 1)]

tests/unit/test_electrum_listener.py

@@ -446,8 +446,14 @@ _UNRELATED_ENTRY = [{"tx_hash": "99" * 32, "tx_pos": 0, "height": 100, "value":
 async def test_refresh_user_marks_a_utxo_spent_once_others_corroborate_it(session_factory):
     user_id = await _seed_funded_user(session_factory, username="bob", address="plm1q0xcqpzrky6eff2g52qdye53xkk9jxkvraxkkwd")
 
+    # The others agree the tracked UTXO is gone, and agree about the unrelated
+    # entry — which B-59 now requires before that one may be credited.
     others_factory = await _listunspent_client_factory(
-        {"first.example": [], "second.example": [], "third.example": []}
+        {
+            "first.example": _UNRELATED_ENTRY,
+            "second.example": _UNRELATED_ENTRY,
+            "third.example": _UNRELATED_ENTRY,
+        }
     )
     listener = ElectrumListener(others_factory, session_factory, _ENDPOINTS)
     listener.client = _ActiveClient(_UNRELATED_ENTRY)  # our own connection no longer sees the UTXO either
@@ -471,7 +477,7 @@ async def test_refresh_user_does_not_mark_when_corroboration_fails(session_facto
     bad reply."""
     user_id = await _seed_funded_user(session_factory, username="carol", address="plm1qtest2")
 
-    still_there = [{"tx_hash": "dd" * 32, "tx_pos": 0}]
+    still_there = [{"tx_hash": "dd" * 32, "tx_pos": 0}] + _UNRELATED_ENTRY
     others_factory = await _listunspent_client_factory(
         {"first.example": [], "second.example": still_there, "third.example": still_there}
     )
@@ -630,3 +636,95 @@ async def test_run_once_keeps_consuming_headers_while_resubscribing(session_fact
     finally:
         await client.close()
         await run_once_task
+
+
+# --- B-59: a credit must clear the same quorum a debit already had to (B-29) -----
+
+_PHANTOM = [{"tx_hash": "77" * 32, "tx_pos": 0, "height": 200, "value": 5_000_000}]
+
+
+async def test_corroborate_utxo_credit_true_when_others_report_the_same_outpoint(session_factory):
+    factory = await _listunspent_client_factory(
+        {"first.example": _PHANTOM, "second.example": _PHANTOM, "third.example": _PHANTOM}
+    )
+    listener = ElectrumListener(factory, session_factory, _ENDPOINTS)
+
+    assert await listener.corroborate_utxo_credit("scripthash", "77" * 32, 0, 5_000_000) is True
+
+
+async def test_corroborate_utxo_credit_false_when_the_amount_differs(session_factory):
+    """Agreement is on the amount too, not just on the outpoint existing — the
+    inflated number is the whole point of the attack."""
+    smaller = [{"tx_hash": "77" * 32, "tx_pos": 0, "height": 200, "value": 1_000}]
+    factory = await _listunspent_client_factory(
+        {"first.example": smaller, "second.example": smaller, "third.example": _PHANTOM}
+    )
+    listener = ElectrumListener(factory, session_factory, _ENDPOINTS)
+
+    assert await listener.corroborate_utxo_credit("scripthash", "77" * 32, 0, 5_000_000) is False
+
+
+async def test_corroborate_utxo_credit_false_when_others_call_it_unconfirmed(session_factory):
+    """height <= 0 is Electrum's "still in the mempool" — a server that hasn't seen
+    the block yet doesn't corroborate a 1-conf credit."""
+    mempool = [{"tx_hash": "77" * 32, "tx_pos": 0, "height": 0, "value": 5_000_000}]
+    factory = await _listunspent_client_factory(
+        {"first.example": mempool, "second.example": mempool, "third.example": _PHANTOM}
+    )
+    listener = ElectrumListener(factory, session_factory, _ENDPOINTS)
+
+    assert await listener.corroborate_utxo_credit("scripthash", "77" * 32, 0, 5_000_000) is False
+
+
+async def test_corroborate_utxo_credit_false_when_nobody_responds(session_factory):
+    factory = await _listunspent_client_factory(
+        {"first.example": None, "second.example": None, "third.example": ConnectionRefusedError("down")}
+    )
+    listener = ElectrumListener(factory, session_factory, _ENDPOINTS)
+
+    assert await listener.corroborate_utxo_credit("scripthash", "77" * 32, 0, 5_000_000) is False
+
+
+async def test_refresh_user_does_not_credit_a_utxo_only_our_own_server_reports(session_factory):
+    """The mirror of B-29's headline case: before this, one hostile or broken
+    server could inflate a user's displayed balance with an outpoint that doesn't
+    exist. The credit is withheld, not lost — the next refresh retries it."""
+    user_id = await _seed_funded_user(session_factory, username="dave", address="plm1qtest3")
+
+    others_factory = await _listunspent_client_factory(
+        {"first.example": [], "second.example": [], "third.example": []}
+    )
+    listener = ElectrumListener(others_factory, session_factory, _ENDPOINTS)
+    # Our own connection reports the tracked UTXO plus a phantom one nobody else has.
+    listener.client = _ActiveClient(
+        [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] + _PHANTOM
+    )
+
+    await listener.refresh_user(user_id, "scripthash")
+
+    async with session_factory() as session:
+        assert (
+            await session.scalars(select(UtxoEvent).where(UtxoEvent.txid == "77" * 32))
+        ).all() == []
+        user = await session.get(User, user_id)
+        assert user.cached_balance_sats == 20_000_000  # unchanged, not inflated
+
+
+async def test_refresh_user_credits_once_the_others_corroborate(session_factory):
+    user_id = await _seed_funded_user(session_factory, username="erin", address="plm1qtest4")
+
+    # first.example is the active endpoint, which _corroborate_majority never asks —
+    # the quorum here is second + third.
+    others_factory = await _listunspent_client_factory(
+        {"first.example": [], "second.example": _PHANTOM, "third.example": _PHANTOM}
+    )
+    listener = ElectrumListener(others_factory, session_factory, _ENDPOINTS)
+    listener.client = _ActiveClient(
+        [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] + _PHANTOM
+    )
+
+    await listener.refresh_user(user_id, "scripthash")
+
+    async with session_factory() as session:
+        user = await session.get(User, user_id)
+        assert user.cached_balance_sats == 25_000_000