From e8fdea038938f18c78374fa1b01930e22236e9fd Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Mon, 27 Jul 2026 10:25:07 +0200 Subject: [PATCH] Corroborate an external spend before marking a UTXO gone (B-29) detect_external_spends marked a UTXO spent_txid='external-spend' irreversibly the moment it was missing from one listunspent reply, on one server, with no way to undo it. A rotated-to server that's broken or behind, or an empty reply, could zero a user's balance permanently. Split into three functions in deposits/service.py: find_utxos_missing_ from (read-only candidate detection, and refuses to flag anything at all when listunspent comes back entirely empty for a funded address - that reads as a broken response, not a full sweep), mark_utxos_spent_ externally (persistence only, once a candidate is already confirmed), and reinstate_reappeared_utxos (undoes the mark if the outpoint reappears as unspent later). ElectrumListener gains corroborate_utxo_spent, sharing the same majority-quorum logic corroborate_header already uses for B-28: before a candidate is marked, the other configured servers are asked whether they also see it as spent. _refresh_user now reads candidates, then corroborates each one with no DB session held open across those network calls (same shape as B-18/B-25), then persists. Suite grows from 165 to 176 tests. BUGS.md moves B-29 to Previously fixed. --- BUGS.md | 34 ++----- app/deposits/service.py | 107 +++++++++++++++---- app/electrum/listener.py | 128 +++++++++++++++++------ tests/unit/test_deposits.py | 121 ++++++++++++++++++++-- tests/unit/test_electrum_listener.py | 147 +++++++++++++++++++++++++++ 5 files changed, 453 insertions(+), 84 deletions(-) diff --git a/BUGS.md b/BUGS.md index 429d36b..738bbb3 100644 --- a/BUGS.md +++ b/BUGS.md @@ -1,17 +1,17 @@ # 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-29 … B-49. B-25 through B-28 are fixed (see "Previously -fixed" below) — no Critical-severity finding remains open; the other 21 are High/Medium/Low. +7 medium, 8 low), listed below as B-30 … B-49. B-25 through B-29 are fixed (see "Previously +fixed" below) — no Critical-severity finding remains open; the other 20 are High/Medium/Low. 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 four fixes so far brought the suite from -139 to 165). +coverage — every fix lands with a regression test (the five fixes so far brought the suite from +139 to 176). 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. -Outgoing transactions reconcile; deposits do not (B-29). The payout phase is now fully -recoverable; the "drawing" phase (waiting on a block) still has no equivalent -resume-after-restart or stall visibility (B-36). +The payout phase is now fully recoverable; the "drawing" phase (waiting on a block) still has +no equivalent resume-after-restart or stall visibility (B-36), and there's still no periodic +deposit-side reconciler independent of scripthash notifications (B-30). 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 @@ -21,23 +21,6 @@ single-process assumptions, no user-facing history, etc.), see "Known gaps / TOD ## High -### B-29 — `detect_external_spends` is irreversible and trusts a single response - -`deposits/service.py:87` marks `spent_txid = "external-spend"` for any UTXO missing from the -current `listunspent`. There is **no path to undo it**: `credit_confirmed_utxos` skips -`(txid, vout)` keys that already exist, regardless of their spent status (`:19-31`). - -One incomplete `listunspent` — a rotated-to server that is broken or behind, an empty reply on -error, or a reorg — permanently and silently zeroes a user's balance, recoverable only by -editing the database. Crediting is idempotent and conservative; debiting is neither, and it -acts on a single reply from a single unauthenticated server. - -**Proposed fix.** Treat a missing outpoint as *evidence*, not proof. Require the same UTXO to -be absent across N consecutive refreshes (or confirm the spend by looking up the outpoint's -spending tx) before marking it, and skip the whole pass when `listunspent` returns empty for -an address the DB believes is funded. Make the mark reversible: re-crediting should clear -`spent_txid` when the sentinel value is present and the outpoint reappears as unspent. - ### B-30 — No deposit-side reconciler: one missed subscription means deposits are never credited `electrum/listener.py:61-67` (`address_for_new_user`) fires `asyncio.create_task(...)` without @@ -301,9 +284,10 @@ already does. - **B-26** — a payout failure or a process restart could wedge a round in `paying_out` forever - **B-27** — an RBF bump reset the reconciler's own abandon clock, so a repeatedly-bumped tx was never abandoned - **B-28** — a hostile Electrum server (or a MITM) could single-handedly pick the round's winner +- **B-29** — a UTXO absent from one server's `listunspent` was marked spent immediately, irreversibly, on a single unauthenticated reply See git history for the fix-by-fix breakdown (commits `f13f685`, `50a43ae`, `933760e`, and the -B-28 fix). Suite grew from 139 to 165 tests over the four. +B-28/B-29 fixes). Suite grew from 139 to 176 tests over the five. 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, diff --git a/app/deposits/service.py b/app/deposits/service.py index 9c3cd34..4b88aae 100644 --- a/app/deposits/service.py +++ b/app/deposits/service.py @@ -1,3 +1,5 @@ +import logging + from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -6,6 +8,8 @@ from app.db.models import UtxoEvent from app.rounds.events import broadcaster from app.wallet.balance import recompute_balance +logger = logging.getLogger(__name__) + async def credit_confirmed_utxos(session: AsyncSession, user_id: int, entries: list[dict]) -> int: """Insert utxo_events for newly-confirmed entries from an Electrum @@ -59,18 +63,64 @@ async def credit_confirmed_utxos(session: AsyncSession, user_id: int, entries: l _EXTERNAL_SPEND_SENTINEL = "external-spend" -async def detect_external_spends(session: AsyncSession, user_id: int, entries: list[dict]) -> int: - """Mirror of credit_confirmed_utxos: catches a UTXO leaving the address - through a transaction this platform never built (e.g. someone spending it - directly with the raw privkey, bypassing /withdrawals entirely). +async def reinstate_reappeared_utxos(session: AsyncSession, user_id: int, entries: list[dict]) -> int: + """The reverse of a mark applied by find_utxos_missing_from/ + mark_utxos_spent_externally (B-29): if an outpoint we'd previously flagged as + spent outside the platform reappears as unspent in a later listunspent, undo + the mark instead of leaving it permanent no matter what the chain says + afterwards. Cheap and purely DB-side — always safe to run on every refresh. + """ + current_keys = {(entry["tx_hash"], entry["tx_pos"]) for entry in entries} - Everything the platform itself spends (bets, withdrawals, payouts) sets - spent_txid at broadcast time, before the tx ever reaches the chain — so by - the time an Electrum refresh runs, an outpoint still marked unspent in our - own DB that Electrum no longer reports as unspent was never on our radar. - entries is this address's current `listunspent`; anything in our unspent - set but missing from it left the address some other way. Returns the - number of UTXOs newly marked spent. + marked_rows = ( + await session.scalars( + select(UtxoEvent).where( + UtxoEvent.user_id == user_id, UtxoEvent.spent_txid == _EXTERNAL_SPEND_SENTINEL + ) + ) + ).all() + + reinstated = 0 + for row in marked_rows: + if (row.txid, row.vout) not in current_keys: + continue + row.spent_txid = None + await write_audit_log( + session, + "utxo_external_spend_reinstated", + {"txid": row.txid, "vout": row.vout, "amount_sats": row.amount_sats}, + user_id=user_id, + ) + reinstated += 1 + + if reinstated: + await session.flush() + await recompute_balance(session, user_id) + await session.commit() + broadcaster.publish() + + return reinstated + + +async def find_utxos_missing_from(session: AsyncSession, user_id: int, entries: list[dict]) -> list[UtxoEvent]: + """Candidates for an external spend (B-29): unspent UTXOs the DB believes this + user still holds that are absent from `entries`, this address's current + listunspent. Everything the platform itself spends (bets, withdrawals, + payouts) sets spent_txid at broadcast time, before the tx ever reaches the + chain — so an outpoint still marked unspent in our own DB that Electrum no + longer reports as unspent was never on our own radar. + + Returning a row here is *not* proof it was actually spent — only that this one + server's reply no longer lists it. A single broken, behind, or malicious + server could otherwise zero a user's balance on one bad reply, which is why + the caller (electrum/listener.py:_refresh_user) must independently + corroborate each candidate against other configured servers before treating + it as genuine, rather than this function marking anything itself. + + An entirely empty `entries` for an address the DB believes is funded returns + no candidates at all: it would otherwise flag every one of this user's UTXOs + as missing from a single reply, which is a strong sign of an incomplete or + broken response rather than N independent spends landing in the same refresh. """ current_keys = {(entry["tx_hash"], entry["tx_pos"]) for entry in entries} @@ -80,9 +130,32 @@ async def detect_external_spends(session: AsyncSession, user_id: int, entries: l ) ).all() - newly_spent = 0 - for row in unspent_rows: - if (row.txid, row.vout) in current_keys: + if not entries and unspent_rows: + logger.warning( + "listunspent for user_id=%s returned no entries at all while %s UTXO(s) are still recorded " + "unspent — treating this as an incomplete response rather than a full external sweep", + user_id, + len(unspent_rows), + ) + return [] + + return [row for row in unspent_rows if (row.txid, row.vout) not in current_keys] + + +async def mark_utxos_spent_externally(session: AsyncSession, user_id: int, utxo_ids: list[int]) -> int: + """Applies the external-spend sentinel to UTXOs the caller has already + corroborated against other servers (B-29) — this function does no + verification of its own, only persistence, so it never runs with a session + held open across the network calls that verification needs. + + Re-checks each row is still unspent before applying the mark: something else + may have resolved it (a legitimate platform spend, or a prior refresh) between + when the caller read the candidate list and finished corroborating it. + """ + marked = 0 + for utxo_id in utxo_ids: + row = await session.get(UtxoEvent, utxo_id) + if row is None or row.spent_txid is not None: continue row.spent_txid = _EXTERNAL_SPEND_SENTINEL await write_audit_log( @@ -91,12 +164,12 @@ async def detect_external_spends(session: AsyncSession, user_id: int, entries: l {"txid": row.txid, "vout": row.vout, "amount_sats": row.amount_sats}, user_id=user_id, ) - newly_spent += 1 + marked += 1 - if newly_spent: + if marked: await session.flush() await recompute_balance(session, user_id) await session.commit() broadcaster.publish() - return newly_spent + return marked diff --git a/app/electrum/listener.py b/app/electrum/listener.py index 327462b..f83daf0 100644 --- a/app/electrum/listener.py +++ b/app/electrum/listener.py @@ -6,7 +6,12 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import async_sessionmaker from app.db.models import User -from app.deposits.service import credit_confirmed_utxos, detect_external_spends +from app.deposits.service import ( + credit_confirmed_utxos, + find_utxos_missing_from, + mark_utxos_spent_externally, + reinstate_reappeared_utxos, +) from app.electrum.client import ElectrumClient, ElectrumEndpoint from app.electrum.scripthash import address_to_scripthash from app.rounds.draw import ( @@ -24,10 +29,10 @@ logger = logging.getLogger(__name__) # difference between noticing the drop in a minute and never noticing it at all. _PING_INTERVAL_SECONDS = 60 -# B-28: how long to wait for any *one* other server's answer when corroborating the -# draw's block header. Shorter than the standard request timeout since this is a -# supplementary check across several servers at once — a single slow fallback -# shouldn't hold up the others. +# How long to wait for any *one* other server's answer when corroborating the +# draw's block header (B-28) or a candidate external spend (B-29). Shorter than +# the standard request timeout since this is a supplementary check across several +# servers at once — a single slow fallback shouldn't hold up the others. _CORROBORATION_TIMEOUT_SECONDS = 10 @@ -223,27 +228,49 @@ class ElectrumListener: self.tip_height = height self.tip_header_hex = header_hex - async def corroborate_header(self, height: int, expected_hash: str) -> bool: - """B-28: independently ask every *other* configured server for the header - at `height` and require a strict majority of the ones that actually answer - to agree with `expected_hash` — the hash our own active connection - reported — before the draw (rounds/scheduler.py:_wait_for_next_block) treats - it as trustworthy entropy. Without this, a single hostile server (or a MITM - on the one active connection) can single-handedly decide who wins every - round; this raises the bar to controlling a majority of the configured - servers. + async def _corroborate_majority( + self, + ask: Callable[[ElectrumEndpoint], "asyncio.Future"], + agrees: Callable[[object], bool], + description: str, + ) -> bool: + """Shared quorum logic behind corroborate_header (B-28) and + corroborate_utxo_spent (B-29): ask every *other* configured server (never + the currently active one — that's exactly what a hostile server or a MITM + would control) and require a strict majority of the ones that actually + answer to agree, via `agrees`, with what our own connection reported. - Returns True if there are no other servers configured at all — a - single-endpoint deployment has nothing to corroborate against, and accepted - that risk when ELECTRUM_FALLBACK_SERVERS was left empty (see CLAUDE.md). - Also returns False (never silently "passes") if none of the other servers - could be reached at all, since an unreachable network answers nothing about - whether the header is genuine. + Returns True with no other servers configured — nothing to corroborate + against, a risk accepted when ELECTRUM_FALLBACK_SERVERS was left empty + (see CLAUDE.md). Returns False (never silently "passes") if none of the + others could be reached, since an unreachable network proves nothing + either way. """ others = [endpoint for endpoint in self._endpoints if endpoint != self.current_endpoint] if not others: return True + results = await asyncio.gather(*(ask(endpoint) for endpoint in others)) + responded = [result for result in results if result is not None] + if not responded: + logger.warning( + "could not corroborate %s with any of %s other configured server(s)", description, len(others) + ) + return False + + agreements = sum(1 for result in responded if agrees(result)) + return agreements * 2 > len(responded) + + async def corroborate_header(self, height: int, expected_hash: str) -> bool: + """B-28: is `expected_hash` — the header our own active connection + reported for `height` — corroborated by other configured servers before + the draw (rounds/scheduler.py:_wait_for_next_block) treats it as + trustworthy entropy? Without this, a single hostile server (or a MITM on + the one active connection) can single-handedly decide who wins every + round; this raises the bar to controlling a majority of the configured + servers. See _corroborate_majority for the shared quorum logic. + """ + async def _ask(endpoint: ElectrumEndpoint) -> str | None: client = self._client_factory(endpoint) try: @@ -260,18 +287,32 @@ class ElectrumListener: finally: await client.close() - results = await asyncio.gather(*(_ask(endpoint) for endpoint in others)) - responded = [block_hash for block_hash in results if block_hash is not None] - if not responded: - logger.warning( - "could not corroborate block %s header with any of %s other configured server(s)", - height, - len(others), - ) - return False + return await self._corroborate_majority(_ask, lambda block_hash: block_hash == expected_hash, f"block {height} header") - agreements = sum(1 for block_hash in responded if block_hash == expected_hash) - return agreements * 2 > len(responded) + async def corroborate_utxo_spent(self, scripthash: str, txid: str, vout: int) -> bool: + """B-29: before deposits/service.py's find_utxos_missing_from candidates + are treated as genuinely spent outside the platform, ask the other + configured servers whether *they* also no longer report this outpoint as + unspent. A single broken, behind, or malicious server could otherwise zero + a user's balance on one incomplete listunspent reply. See + _corroborate_majority for the shared quorum logic. + """ + + 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 + ) + still_unspent = any(e.get("tx_hash") == txid and e.get("tx_pos") == vout for e in entries) + return not still_unspent # True = this server agrees the outpoint is gone + except Exception: + return None + finally: + await client.close() + + return await self._corroborate_majority(_ask, lambda agrees: agrees, f"outpoint {txid}:{vout}") async def _consume_headers(self, queue: asyncio.Queue) -> None: while True: @@ -291,12 +332,35 @@ class ElectrumListener: await self._refresh_user(user_id, scripthash) 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 + candidate external spends against other servers (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) - spent_externally = await detect_external_spends(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) + ] + + confirmed_ids = [ + utxo_id + for utxo_id, txid, vout in candidates + if await self.corroborate_utxo_spent(scripthash, txid, vout) + ] + + spent_externally = 0 + if confirmed_ids: + async with self._session_factory() as session: + spent_externally = await mark_utxos_spent_externally(session, user_id, confirmed_ids) + if credited: logger.info("credited %s new UTXO(s) for user_id=%s", credited, user_id) + if reinstated: + logger.info("reinstated %s previously-flagged UTXO(s) for user_id=%s", reinstated, user_id) if spent_externally: logger.warning("%s UTXO(s) spent outside the platform for user_id=%s", spent_externally, user_id) diff --git a/tests/unit/test_deposits.py b/tests/unit/test_deposits.py index 299ec9c..d744916 100644 --- a/tests/unit/test_deposits.py +++ b/tests/unit/test_deposits.py @@ -4,7 +4,12 @@ from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from app.db.base import Base from app.db.models import AuditLog, User, UtxoEvent -from app.deposits.service import credit_confirmed_utxos, detect_external_spends +from app.deposits.service import ( + credit_confirmed_utxos, + find_utxos_missing_from, + mark_utxos_spent_externally, + reinstate_reappeared_utxos, +) @pytest.fixture @@ -55,14 +60,66 @@ async def test_idempotent_on_repeated_notification(session_factory, user_id): assert user.cached_balance_sats == 7_000_000 -async def test_external_spend_marks_utxo_spent_and_corrects_balance(session_factory, user_id): - entries = [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] +# --- B-29: detecting a UTXO spent outside the platform is now a three-step, +# corroborate-before-you-mark process, split across find_utxos_missing_from +# (read-only candidate detection), the caller's own corroboration against other +# servers (electrum/listener.py, not exercised here), and mark_utxos_spent_ +# externally (persistence only, once a candidate is already confirmed). --------- + + +async def test_find_utxos_missing_from_returns_the_missing_candidate(session_factory, user_id): + async with session_factory() as session: + await credit_confirmed_utxos( + session, user_id, [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] + ) + + # A different outpoint present in this refresh — our own tracked one is + # genuinely absent from it, not just from an entirely empty reply. + other_entries = [{"tx_hash": "99" * 32, "tx_pos": 0, "height": 100, "value": 1_000_000}] + async with session_factory() as session: + candidates = await find_utxos_missing_from(session, user_id, other_entries) + assert len(candidates) == 1 + assert candidates[0].txid == "dd" * 32 + assert candidates[0].spent_txid is None # read-only: nothing is marked yet + + +async def test_find_utxos_missing_from_returns_nothing_when_present(session_factory, user_id): + entries = [{"tx_hash": "ee" * 32, "tx_pos": 0, "height": 100, "value": 3_000_000}] async with session_factory() as session: await credit_confirmed_utxos(session, user_id, entries) async with session_factory() as session: - spent = await detect_external_spends(session, user_id, []) - assert spent == 1 + candidates = await find_utxos_missing_from(session, user_id, entries) + assert candidates == [] + user = await session.get(User, user_id) + assert user.cached_balance_sats == 3_000_000 + + +async def test_find_utxos_missing_from_skips_a_totally_empty_response(session_factory, user_id): + """B-29: an entirely empty listunspent for a funded address reads as an + incomplete/broken response, not proof of a full external sweep — it would + otherwise flag every UTXO of this user as missing from one bad reply.""" + async with session_factory() as session: + await credit_confirmed_utxos( + session, user_id, [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] + ) + + async with session_factory() as session: + candidates = await find_utxos_missing_from(session, user_id, []) + assert candidates == [] + + +async def test_mark_utxos_spent_externally_marks_and_corrects_balance(session_factory, user_id): + async with session_factory() as session: + await credit_confirmed_utxos( + session, user_id, [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] + ) + + async with session_factory() as session: + utxo = (await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id))).one() + marked = await mark_utxos_spent_externally(session, user_id, [utxo.id]) + assert marked == 1 + user = await session.get(User, user_id) assert user.cached_balance_sats == 0 @@ -73,13 +130,57 @@ async def test_external_spend_marks_utxo_spent_and_corrects_balance(session_fact assert any(e.event_type == "utxo_spent_externally" for e in audit_events) -async def test_no_spend_detected_when_utxo_still_unspent(session_factory, user_id): +async def test_mark_utxos_spent_externally_skips_an_already_resolved_row(session_factory, user_id): + """Something else (a legitimate platform spend, or a prior refresh) may have + resolved the row between the caller reading the candidate list and finishing + corroboration — mark_utxos_spent_externally must not clobber that.""" + async with session_factory() as session: + await credit_confirmed_utxos( + session, user_id, [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] + ) + utxo = (await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id))).one() + utxo_id = utxo.id + utxo.spent_txid = "some-real-platform-txid" + await session.commit() + + async with session_factory() as session: + marked = await mark_utxos_spent_externally(session, user_id, [utxo_id]) + assert marked == 0 + utxo = await session.get(UtxoEvent, utxo_id) + assert utxo.spent_txid == "some-real-platform-txid" # untouched + + +async def test_reinstate_reappeared_utxos_clears_the_mark_and_restores_balance(session_factory, user_id): + async with session_factory() as session: + await credit_confirmed_utxos( + session, user_id, [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] + ) + utxo_id = (await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id))).one().id + + async with session_factory() as session: + await mark_utxos_spent_externally(session, user_id, [utxo_id]) + + # The outpoint reappears as unspent in a later refresh. + entries = [{"tx_hash": "dd" * 32, "tx_pos": 0, "height": 100, "value": 20_000_000}] + async with session_factory() as session: + reinstated = await reinstate_reappeared_utxos(session, user_id, entries) + assert reinstated == 1 + + user = await session.get(User, user_id) + assert user.cached_balance_sats == 20_000_000 + + utxo = await session.get(UtxoEvent, utxo_id) + assert utxo.spent_txid is None + + audit_events = [e.event_type for e in (await session.scalars(select(AuditLog))).all()] + assert "utxo_external_spend_reinstated" in audit_events + + +async def test_reinstate_reappeared_utxos_ignores_unmarked_rows(session_factory, user_id): entries = [{"tx_hash": "ee" * 32, "tx_pos": 0, "height": 100, "value": 3_000_000}] async with session_factory() as session: await credit_confirmed_utxos(session, user_id, entries) async with session_factory() as session: - spent = await detect_external_spends(session, user_id, entries) - assert spent == 0 - user = await session.get(User, user_id) - assert user.cached_balance_sats == 3_000_000 + reinstated = await reinstate_reappeared_utxos(session, user_id, entries) + assert reinstated == 0 diff --git a/tests/unit/test_electrum_listener.py b/tests/unit/test_electrum_listener.py index 2e18730..9d4568a 100644 --- a/tests/unit/test_electrum_listener.py +++ b/tests/unit/test_electrum_listener.py @@ -11,9 +11,11 @@ import asyncio import struct import pytest +from sqlalchemy import select from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from app.db.base import Base +from app.db.models import User, UtxoEvent from app.electrum.client import ElectrumEndpoint from app.electrum.listener import ElectrumListener from app.rounds.draw import HeaderValidationError, header_hex_to_block_hash, header_meets_its_own_target @@ -304,3 +306,148 @@ async def test_corroborate_header_false_when_nobody_responds(session_factory): listener = ElectrumListener(factory, session_factory, _ENDPOINTS) assert await listener.corroborate_header(100, "deadbeef") is False + + +# --- B-29: a UTXO absent from our own connection's listunspent must be +# corroborated by other configured servers before it's treated as genuinely spent +# outside the platform. ------------------------------------------------------------ + + +async def _listunspent_client_factory(responses: dict[str, object]): + """Builds a client_factory whose fake clients answer listunspent per-endpoint: + a list of entries to report as unspent, `None` to simulate an unreachable + server (fails at listunspent), or an Exception instance to simulate a connect + failure.""" + + class _FakeClient: + def __init__(self, answer): + self._answer = answer + + async def connect(self): + if isinstance(self._answer, Exception): + raise self._answer + + async def listunspent(self, scripthash): + if self._answer is None: + raise ConnectionRefusedError("unreachable") + return self._answer + + async def close(self): + pass + + def factory(endpoint): + return _FakeClient(responses[endpoint.host]) + + return factory + + +async def test_corroborate_utxo_spent_true_with_no_other_servers_configured(session_factory): + single = [ElectrumEndpoint("only.example", 50002, True)] + listener = ElectrumListener(lambda endpoint: None, session_factory, single) + assert await listener.corroborate_utxo_spent("scripthash", "dd" * 32, 0) is True + + +async def test_corroborate_utxo_spent_true_when_others_agree_its_gone(session_factory): + factory = await _listunspent_client_factory({"first.example": [], "second.example": [], "third.example": []}) + listener = ElectrumListener(factory, session_factory, _ENDPOINTS) + + assert await listener.corroborate_utxo_spent("scripthash", "dd" * 32, 0) is True + + +async def test_corroborate_utxo_spent_false_when_majority_still_see_it_unspent(session_factory): + still_there = [{"tx_hash": "dd" * 32, "tx_pos": 0}] + factory = await _listunspent_client_factory( + {"first.example": [], "second.example": still_there, "third.example": still_there} + ) + listener = ElectrumListener(factory, session_factory, _ENDPOINTS) + + assert await listener.corroborate_utxo_spent("scripthash", "dd" * 32, 0) is False + + +async def test_corroborate_utxo_spent_false_when_nobody_responds(session_factory): + factory = await _listunspent_client_factory( + {"first.example": [], "second.example": None, "third.example": ConnectionRefusedError("down")} + ) + listener = ElectrumListener(factory, session_factory, _ENDPOINTS) + + assert await listener.corroborate_utxo_spent("scripthash", "dd" * 32, 0) is False + + +class _ActiveClient: + """Stands in for `self.client`, the listener's one active connection — + _refresh_user only ever calls listunspent on it.""" + + def __init__(self, entries: list[dict]): + self._entries = entries + + async def listunspent(self, scripthash): + return self._entries + + +async def _seed_funded_user(session_factory, *, username: str, address: str) -> int: + from app.wallet.balance import recompute_balance + + async with session_factory() as session: + user = User(username=username, password_hash="x", derivation_index=0, address=address) + session.add(user) + await session.commit() + session.add( + UtxoEvent(user_id=user.id, txid="dd" * 32, vout=0, amount_sats=20_000_000, confirmed_height=100) + ) + await recompute_balance(session, user.id) + await session.commit() + return user.id + + +# An unrelated outpoint present alongside our own connection's listunspent reply — +# keeps `entries` non-empty so find_utxos_missing_from's "entirely empty response" +# guard doesn't swallow these tests; our own tracked UTXO is still genuinely +# absent from it. +_UNRELATED_ENTRY = [{"tx_hash": "99" * 32, "tx_pos": 0, "height": 100, "value": 1_000_000}] + + +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="plm1qtest") + + others_factory = await _listunspent_client_factory( + {"first.example": [], "second.example": [], "third.example": []} + ) + listener = ElectrumListener(others_factory, session_factory, _ENDPOINTS) + listener.client = _ActiveClient(_UNRELATED_ENTRY) # our own connection no longer sees the UTXO either + + await listener._refresh_user(user_id, "scripthash") + + async with session_factory() as session: + utxo = ( + await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id, UtxoEvent.txid == "dd" * 32)) + ).one() + assert utxo.spent_txid == "external-spend" + user = await session.get(User, user_id) + # The original 20_000_000 is spent; the unrelated entry the "active" + # connection also reported gets freshly credited alongside it. + assert user.cached_balance_sats == 1_000_000 + + +async def test_refresh_user_does_not_mark_when_corroboration_fails(session_factory): + """The single most important case: our own connection alone reporting the + UTXO missing must not be enough — before B-29 this zeroed the balance on one + bad reply.""" + user_id = await _seed_funded_user(session_factory, username="carol", address="plm1qtest2") + + still_there = [{"tx_hash": "dd" * 32, "tx_pos": 0}] + others_factory = await _listunspent_client_factory( + {"first.example": [], "second.example": still_there, "third.example": still_there} + ) + listener = ElectrumListener(others_factory, session_factory, _ENDPOINTS) + listener.client = _ActiveClient(_UNRELATED_ENTRY) + + await listener._refresh_user(user_id, "scripthash") + + async with session_factory() as session: + utxo = ( + await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id, UtxoEvent.txid == "dd" * 32)) + ).one() + assert utxo.spent_txid is None + user = await session.get(User, user_id) + # Untouched, plus the unrelated entry credited alongside it. + assert user.cached_balance_sats == 21_000_000