From d528c5b475a7839593ae5cedd93fdea6e9f90d67 Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Mon, 27 Jul 2026 00:31:24 +0200 Subject: [PATCH] Let the system recover from a broadcast that never confirms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code treated a broadcast as final: money moved on-chain and the DB was updated on the assumption it would either confirm or be fee-bumped until it did. Neither is guaranteed, and every way that assumption broke was permanent (BUGS.md B-02, B-03, B-04, B-07, B-08, B-20, B-21). Persist before broadcasting. place_bet and request_withdrawal now write their rows in a "building" state and commit, then broadcast, then promote to broadcast/pending in a second commit. Before, a failure or crash between the broadcast and the commit left the coins irreversibly spent with no trace: no participant (so no entry in the draw), no pending row (so no RBF and no confirmation tracking), and the UTXOs not even marked spent, so the next bet would try to double-spend them. A refused broadcast now releases the reserved UTXOs, restores the balance, removes the participant (or marks the withdrawal failed), audit-logs it, and answers a translatable broadcast_failed — as 502, since the network refused it, not the caller, where it used to be an opaque 500. Reconcile what's in flight against the chain. New PendingTransactionReconciler (app/tx/reconcile.py, every 120s and once at startup) asks whether each non-terminal tx exists: present -> promote, gone -> mark failed with a reason, release the inputs, roll the domain row back, audit-log it. Grace periods differ by state (120s for "building", 6h for "pending", so the RBF bumper gets its attempts first). It is deliberately biased to inaction: only a server that positively doesn't know the tx counts as absent, and a transport failure never abandons anything, because releasing a UTXO whose tx is actually alive would invite a double-spend. Verified against the live server, which answers "No such mempool or blockchain transaction" for an unknown txid. Stop keying on a value that changes. An RBF bump changes the txid, and _on_bet_confirmed looked the participant up by bet_txid — so a bumped bet confirmed under a txid no participant carried, the row stayed "broadcast" forever, and the scheduler waited on it forever: the round could never close and the lottery stopped. Handlers now resolve by immutable ids (round_id/user_id, withdrawal_id), and bump_fee retargets every stored txid — bet_txid, Withdrawal.txid, Round.payout_txid and UtxoEvent.spent_txid — plus records the previous one in replaced_by_txid, which was never written at all. One bad row no longer blocks the rest. The confirmation poller's per-tx lookup is guarded: a txid the server can't resolve used to abort the whole pass, so nothing confirmed again until an operator intervened. It also selects plain columns instead of hydrating entities that outlive their session. Tests: 6 reconciler cases including "a broken connection must not release coins"; the bet-ordering test probes committed state from an independent session during the broadcast, and caught a real mistake in the first draft of this change (the _pending_transaction helper still hardcoded status="pending", so rows were born already-broadcast and would have got the 6-hour grace instead of 120s). Co-Authored-By: Claude Opus 5 --- app/api/routes/bets.py | 9 +- app/api/routes/withdrawals.py | 7 +- app/bets/confirmation.py | 18 ++- app/bets/service.py | 67 ++++++++- app/rounds/confirmation.py | 8 +- app/tx/broadcast.py | 47 +++++- app/tx/confirmation.py | 23 ++- app/tx/reconcile.py | 244 ++++++++++++++++++++++++++++++++ app/wallet/balance.py | 5 +- app/withdrawals/confirmation.py | 4 +- app/withdrawals/service.py | 87 ++++++++++-- tests/unit/test_bets.py | 69 +++++++++ tests/unit/test_broadcast.py | 69 +++++++++ tests/unit/test_confirmation.py | 91 ++++++++++++ tests/unit/test_reconcile.py | 233 ++++++++++++++++++++++++++++++ tests/unit/test_withdrawals.py | 43 +++++- 16 files changed, 988 insertions(+), 36 deletions(-) create mode 100644 app/tx/reconcile.py create mode 100644 tests/unit/test_reconcile.py diff --git a/app/api/routes/bets.py b/app/api/routes/bets.py index f671ff4..84a4f3d 100644 --- a/app/api/routes/bets.py +++ b/app/api/routes/bets.py @@ -36,7 +36,14 @@ async def create_bet( try: participant = await place_bet(session, listener.client, user) except BetError as exc: - raise from_api_error(status.HTTP_400_BAD_REQUEST, exc) from exc + # A rejected broadcast isn't the client's fault — it's the network + # refusing our transaction, so it answers 502 rather than 400 (B-07). + code = ( + status.HTTP_502_BAD_GATEWAY + if exc.code == "broadcast_failed" + else status.HTTP_400_BAD_REQUEST + ) + raise from_api_error(code, exc) from exc return BetResponse( round_id=participant.round_id, diff --git a/app/api/routes/withdrawals.py b/app/api/routes/withdrawals.py index 0927d98..bf4eee4 100644 --- a/app/api/routes/withdrawals.py +++ b/app/api/routes/withdrawals.py @@ -44,7 +44,12 @@ async def create_withdrawal( session, listener.client, user, body.external_address, body.amount_sats ) except WithdrawalError as exc: - raise from_api_error(status.HTTP_400_BAD_REQUEST, exc) from exc + code = ( + status.HTTP_502_BAD_GATEWAY # the network refused it, not the caller (B-07) + if exc.code == "broadcast_failed" + else status.HTTP_400_BAD_REQUEST + ) + raise from_api_error(code, exc) from exc return WithdrawalResponse( txid=withdrawal.txid, diff --git a/app/bets/confirmation.py b/app/bets/confirmation.py index 4162d9c..921417e 100644 --- a/app/bets/confirmation.py +++ b/app/bets/confirmation.py @@ -8,10 +8,24 @@ from app.tx.confirmation import register_handler async def _on_bet_confirmed(session: AsyncSession, pending: PendingTransaction) -> None: + """Resolved by (round_id, user_id) — the pair is unique per participant and, + unlike the txid, cannot change under us. Keying this on bet_txid meant an + RBF-bumped bet confirmed under a txid no participant carried, so the row stayed + "broadcast" forever and the round could never close (B-02). The txid is kept in + step by tx/broadcast.py too, but correctness here no longer depends on it.""" participant = await session.scalar( - select(RoundParticipant).where(RoundParticipant.bet_txid == pending.current_txid) + select(RoundParticipant).where( + RoundParticipant.round_id == pending.round_id, + RoundParticipant.user_id == pending.user_id, + ) ) - if participant is not None and participant.status == "broadcast": + if participant is None: + # Fall back to the txid for rows written before this changed, and for any + # pending row missing its round/user link. + participant = await session.scalar( + select(RoundParticipant).where(RoundParticipant.bet_txid == pending.current_txid) + ) + if participant is not None and participant.status in ("building", "broadcast"): participant.status = "confirmed" participant.confirmed_at = datetime.now(timezone.utc) diff --git a/app/bets/service.py b/app/bets/service.py index 0e819b6..572f7ef 100644 --- a/app/bets/service.py +++ b/app/bets/service.py @@ -64,12 +64,17 @@ async def place_bet(session: AsyncSession, client: ElectrumClient, user: User) - except InsufficientFundsError as exc: raise BetError(exc.code, str(exc)) from exc - await client.broadcast(built.raw_hex) - + # --- Phase 1: record the intent, *then* broadcast (B-08) -------------------- + # Broadcasting first meant a failure (or a crash) between the broadcast and the + # commit left the coins irreversibly spent on-chain with no trace in the DB: no + # participant, so no entry in the draw; no pending row, so no RBF and no + # confirmation tracking; and the UTXOs not even marked spent, so the next bet + # would try to double-spend them. Writing "building" rows first means the worst + # case is a row the reconciler (app/tx/reconcile.py) can resolve either way by + # asking the chain whether the tx exists. spent_by_key = {(u.txid, u.vout): u for u in unspent} for spent in built.spent_utxos: - row = spent_by_key[(spent.txid, spent.vout)] - row.spent_txid = built.txid + spent_by_key[(spent.txid, spent.vout)].spent_txid = built.txid await recompute_balance(session, user.id) broadcast_at = datetime.now(timezone.utc) @@ -79,10 +84,25 @@ async def place_bet(session: AsyncSession, client: ElectrumClient, user: User) - bet_amount_sats=built.recipient_sats, bet_txid=built.txid, broadcast_at=broadcast_at, - status="broadcast", + status="building", ) session.add(participant) - session.add(_pending_transaction(round_.id, user.id, built, config.fee_rate_sat_vb)) + pending = _pending_transaction(round_.id, user.id, built, config.fee_rate_sat_vb) + session.add(pending) + await session.commit() + + # --- Phase 2: broadcast, then promote both rows to their live state --------- + try: + await client.broadcast(built.raw_hex) + except Exception as exc: + # The node refused it (fee too low, dust, mempool conflict, or simply an + # unreachable server) — nothing is on-chain, so undo phase 1 completely and + # give the user a translatable failure instead of a bare 500 (B-07). + await _release_failed_bet(session, participant, pending, built, user.id, str(exc)) + raise BetError("broadcast_failed", f"the network refused the transaction: {exc}") from exc + + participant.status = "broadcast" + pending.status = "pending" await write_audit_log( session, "bet_placed", @@ -97,6 +117,35 @@ async def place_bet(session: AsyncSession, client: ElectrumClient, user: User) - return participant +async def _release_failed_bet( + session: AsyncSession, + participant: RoundParticipant, + pending: PendingTransaction, + built: BuiltTransaction, + user_id: int, + reason: str, +) -> None: + """Undo phase 1 after a failed broadcast: free the UTXOs the build reserved, drop + the two rows, and restore the balance. Same shape as what the reconciler does for + a tx that turns out never to have made it onto the chain.""" + for spent in built.spent_utxos: + row = await session.scalar( + select(UtxoEvent).where(UtxoEvent.txid == spent.txid, UtxoEvent.vout == spent.vout) + ) + if row is not None: + row.spent_txid = None + await session.delete(participant) + await session.delete(pending) + await recompute_balance(session, user_id) + await write_audit_log( + session, + "bet_broadcast_failed", + {"txid": built.txid, "reason": reason[:200]}, + user_id=user_id, + ) + await session.commit() + + def _pending_transaction( round_id: int, user_id: int, built: BuiltTransaction, fee_rate_sat_vb: int ) -> PendingTransaction: @@ -107,5 +156,9 @@ def _pending_transaction( current_txid=built.txid, fee_rate_sat_vb=fee_rate_sat_vb, raw_tx_hex=built.raw_hex, - status="pending", + # "building" until the broadcast succeeds — see place_bet's two phases. It + # matters which one this starts as: the reconciler gives a "building" row a + # short grace period (we may have died mid-broadcast) and a "pending" one a + # long one (a node accepted it once, so it deserves the RBF attempts first). + status="building", ) diff --git a/app/rounds/confirmation.py b/app/rounds/confirmation.py index 64660be..71d6978 100644 --- a/app/rounds/confirmation.py +++ b/app/rounds/confirmation.py @@ -8,7 +8,13 @@ from app.tx.confirmation import register_handler async def _on_payout_confirmed(session: AsyncSession, pending: PendingTransaction) -> None: - round_ = await session.scalar(select(Round).where(Round.payout_txid == pending.current_txid)) + # Resolved by round_id rather than by payout_txid: an RBF-bumped payout confirms + # under a different txid than the one first recorded (B-02). + round_ = None + if pending.round_id is not None: + round_ = await session.get(Round, pending.round_id) + if round_ is None: + round_ = await session.scalar(select(Round).where(Round.payout_txid == pending.current_txid)) if round_ is not None and round_.status == "paying_out": round_.status = "closed" # closed_at is what round_cooldown_seconds counts from (service.py's diff --git a/app/tx/broadcast.py b/app/tx/broadcast.py index b050b15..0df974e 100644 --- a/app/tx/broadcast.py +++ b/app/tx/broadcast.py @@ -9,7 +9,7 @@ from embit.finalizer import finalize_psbt from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker -from app.db.models import PendingTransaction, User +from app.db.models import PendingTransaction, Round, RoundParticipant, User, UtxoEvent, Withdrawal from app.electrum.client import ElectrumClient from app.rounds.config import get_round_config from app.wallet.hd import derive_pool_key, derive_user_key @@ -113,17 +113,60 @@ async def bump_fee(session: AsyncSession, client: ElectrumClient, pending: Pendi new_txid = final_tx.txid().hex() await client.broadcast(raw_hex) + old_txid = pending.current_txid + pending.replaced_by_txid = old_txid # points backwards: what current_txid replaced pending.current_txid = new_txid pending.raw_tx_hex = raw_hex pending.fee_rate_sat_vb = new_fee_rate pending.attempt_count += 1 pending.broadcast_at = datetime.now(timezone.utc) + await _retarget_txid_references(session, pending, old_txid, new_txid) await session.commit() - logger.info("bumped %s pending_transaction %s: %s -> %s", pending.kind, pending.id, pending.current_txid, new_txid) + logger.info("bumped %s pending_transaction %s: %s -> %s", pending.kind, pending.id, old_txid, new_txid) return new_txid +async def _retarget_txid_references( + session: AsyncSession, pending: PendingTransaction, old_txid: str, new_txid: str +) -> None: + """A bump changes the txid, and everything that recorded the old one has to + follow — otherwise the bumped tx confirms and nothing recognizes it (B-02). + + The worst case was the bet path: _on_bet_confirmed used to look the participant + up by bet_txid, so after a bump it found nothing, the participant stayed + "broadcast" forever, and the scheduler waited on it forever — the round could + never close and the lottery stopped. The handlers now key off immutable ids + (round_id/user_id, withdrawal_id) as well, so this update is about keeping the + stored txids *true* — for the admin UI, for the audit trail, and for + reconcile.py, which matches UtxoEvent.spent_txid against current_txid. + """ + if pending.kind == "bet": + participant = await session.scalar( + select(RoundParticipant).where( + RoundParticipant.round_id == pending.round_id, + RoundParticipant.user_id == pending.user_id, + ) + ) + if participant is not None: + participant.bet_txid = new_txid + elif pending.kind == "withdrawal" and pending.withdrawal_id is not None: + withdrawal = await session.get(Withdrawal, pending.withdrawal_id) + if withdrawal is not None: + withdrawal.txid = new_txid + elif pending.kind == "payout" and pending.round_id is not None: + round_ = await session.get(Round, pending.round_id) + if round_ is not None and round_.payout_txid == old_txid: + round_.payout_txid = new_txid + + # The UTXOs this tx spends are still the same UTXOs — only the id of the tx + # spending them changed. Keeping this in step is what lets reconcile.py tell + # "reserved by this pending tx" from "spent by something else". + spent = (await session.scalars(select(UtxoEvent).where(UtxoEvent.spent_txid == old_txid))).all() + for utxo in spent: + utxo.spent_txid = new_txid + + class RbfBumper: def __init__(self, session_factory: async_sessionmaker, get_client): self._session_factory = session_factory diff --git a/app/tx/confirmation.py b/app/tx/confirmation.py index 61c4c69..a7fdb91 100644 --- a/app/tx/confirmation.py +++ b/app/tx/confirmation.py @@ -26,14 +26,27 @@ def register_handler(kind: str, handler: ConfirmationHandler) -> None: async def poll_once(session_factory: async_sessionmaker, client: ElectrumClient) -> int: async with session_factory() as session: - pending = ( - await session.scalars(select(PendingTransaction).where(PendingTransaction.status == "pending")) + # Plain columns, not entities: nothing then outlives the session, so this + # can't break if expire_on_commit is ever turned on (B-21). + candidates = ( + await session.execute( + select( + PendingTransaction.id, PendingTransaction.current_txid, PendingTransaction.kind + ).where(PendingTransaction.status == "pending") + ) ).all() - pending_ids = [p.id for p in pending] confirmed = 0 - for pending_id, txid, kind in [(p.id, p.current_txid, p.kind) for p in pending]: - tx = await client.get_transaction(txid, verbose=True) + for pending_id, txid, kind in candidates: + try: + tx = await client.get_transaction(txid, verbose=True) + except Exception: + # One unresolvable txid must not stop the others: a tx the server no + # longer knows (dropped from the mempool, replaced) used to abort the + # whole pass, so nothing confirmed again until an operator intervened + # (B-03). Abandoning such a row is app/tx/reconcile.py's job, not ours. + logger.warning("could not check pending_transaction %s (txid %s)", pending_id, txid, exc_info=True) + continue if not tx or tx.get("confirmations", 0) < 1: continue async with session_factory() as session: diff --git a/app/tx/reconcile.py b/app/tx/reconcile.py new file mode 100644 index 0000000..32ddf79 --- /dev/null +++ b/app/tx/reconcile.py @@ -0,0 +1,244 @@ +"""Resolves in-flight transactions against the chain. + +Everything else in this codebase assumes a broadcast either confirms or gets +fee-bumped until it does. Neither is guaranteed: an RBF bump raises RbfError +whenever there's no change output big enough to absorb it (see tx/broadcast.py), +a node can drop a low-fee tx from its mempool, and the process can die between +building a transaction and broadcasting it. Without this module those cases were +permanent: `spent_txid` was set at build time and never cleared, so the coins +stayed spendable on-chain while the database considered them gone — the user's +balance simply lost them, with no path back short of editing the DB by hand +(B-04, and the "building" half of B-08). + +What it does, per PendingTransaction that isn't already terminal: + + * status "building" — we crashed (or were killed) between writing the row and + broadcasting. Ask the chain: if the tx is there after all, promote everything + to its live state; if it isn't, release the UTXOs and undo the intent. + * status "pending" — broadcast, still unconfirmed. Left alone until it has been + unconfirmed for `_ABANDON_AFTER_SECONDS`, since absence from one server's + mempool is not proof of death; only then is it abandoned like the above. + +Deliberately conservative: it never touches a tx the chain knows about, and the +grace period is long (multiples of the RBF timeout) so a slow-but-alive tx is +bumped by RbfBumper rather than abandoned here. +""" + +import asyncio +import logging +from collections.abc import Callable +from datetime import datetime, timedelta, timezone + +from embit.transaction import Transaction +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from app.audit.log import write_audit_log +from app.db.models import PendingTransaction, Round, RoundParticipant, UtxoEvent, Withdrawal +from app.electrum.client import ElectrumClient +from app.rounds.events import broadcaster +from app.wallet.balance import recompute_balance + +logger = logging.getLogger(__name__) + +_POLL_INTERVAL_SECONDS = 120 + +# A "building" row means we never got confirmation that the broadcast happened, so +# it only needs long enough to rule out a request still in flight. +_BUILDING_GRACE_SECONDS = 120 + +# A "pending" row was accepted by a node once. Give it a wide margin — the RBF +# bumper gets several attempts inside this window — before concluding it's gone. +_ABANDON_AFTER_SECONDS = 6 * 60 * 60 + + +class PendingTransactionReconciler: + def __init__(self, session_factory: async_sessionmaker, get_client: Callable[[], ElectrumClient | None]): + self._session_factory = session_factory + self._get_client = get_client + + async def run(self) -> None: + # Runs once promptly at startup: a crash mid-broadcast is exactly the case + # that leaves a "building" row, and the restart is when we can clear it. + while True: + client = self._get_client() + if client is not None: + try: + await reconcile_once(self._session_factory, client) + except asyncio.CancelledError: + raise + except Exception: + logger.exception("pending-transaction reconciliation failed") + await asyncio.sleep(_POLL_INTERVAL_SECONDS) + + +async def _tx_exists_on_chain(client: ElectrumClient, txid: str) -> bool: + """True if the server knows this txid at all (mempool or mined). An error reply + means "unknown", which is the answer we're looking for; a transport failure is + *not* — that raises, and the caller leaves the row alone until next time.""" + try: + tx = await client.get_transaction(txid, verbose=True) + except Exception as exc: + message = str(exc).lower() + if "missing" in message or "not found" in message or "no such" in message or "unknown" in message: + return False + raise + return bool(tx) + + +async def reconcile_once(session_factory: async_sessionmaker, client: ElectrumClient) -> int: + """Returns how many rows were resolved (promoted or abandoned).""" + now = datetime.now(timezone.utc) + async with session_factory() as session: + rows = ( + await session.scalars( + select(PendingTransaction).where(PendingTransaction.status.in_(("building", "pending"))) + ) + ).all() + candidates = [ + (row.id, row.status, row.current_txid) + for row in rows + if _is_due(row, now) + ] + + resolved = 0 + for row_id, status, txid in candidates: + try: + exists = await _tx_exists_on_chain(client, txid) + except Exception: + # Transport/server problem — say nothing about this tx and try again on + # the next pass rather than abandoning a tx that may be perfectly alive. + logger.warning("could not check pending_transaction %s (txid %s) against the chain", row_id, txid) + continue + + async with session_factory() as session: + row = await session.get(PendingTransaction, row_id) + if row is None or row.status != status: + continue # something else moved it while we were asking + if exists: + if row.status == "building": + await _promote(session, row) + resolved += 1 + else: + await _abandon(session, row, "not found on chain") + resolved += 1 + await session.commit() + broadcaster.publish() + + return resolved + + +def _is_due(row: PendingTransaction, now: datetime) -> bool: + grace = _BUILDING_GRACE_SECONDS if row.status == "building" else _ABANDON_AFTER_SECONDS + return now >= row.broadcast_at.replace(tzinfo=timezone.utc) + timedelta(seconds=grace) + + +async def _promote(session: AsyncSession, row: PendingTransaction) -> None: + """The tx did make it onto the chain before we died — finish what phase 2 of + place_bet/request_withdrawal would have done.""" + row.status = "pending" + if row.kind == "bet": + participant = await session.scalar( + select(RoundParticipant).where( + RoundParticipant.round_id == row.round_id, RoundParticipant.user_id == row.user_id + ) + ) + if participant is not None and participant.status == "building": + participant.status = "broadcast" + elif row.kind == "withdrawal" and row.withdrawal_id is not None: + withdrawal = await session.get(Withdrawal, row.withdrawal_id) + if withdrawal is not None and withdrawal.status == "building": + withdrawal.status = "broadcast" + await write_audit_log( + session, + "pending_tx_recovered", + {"pending_transaction_id": row.id, "kind": row.kind, "txid": row.current_txid}, + user_id=row.user_id, + round_id=row.round_id, + ) + logger.info("recovered %s pending_transaction %s: tx %s is on-chain", row.kind, row.id, row.current_txid) + + +async def _abandon(session: AsyncSession, row: PendingTransaction, reason: str) -> None: + """The tx is gone for good. Release whatever it reserved so the funds come back, + and roll the domain row back to something truthful.""" + row.status = "failed" + row.failure_reason = reason[:128] + + released = await _release_inputs(session, row) + + if row.kind == "bet": + participant = await session.scalar( + select(RoundParticipant).where( + RoundParticipant.round_id == row.round_id, RoundParticipant.user_id == row.user_id + ) + ) + if participant is not None and participant.status in ("building", "broadcast"): + # The bet never happened, so the user is not in this round. Removing the + # row also unblocks the scheduler, which waits for every non-confirmed + # participant before closing the round. + await session.delete(participant) + elif row.kind == "withdrawal" and row.withdrawal_id is not None: + withdrawal = await session.get(Withdrawal, row.withdrawal_id) + if withdrawal is not None and withdrawal.status in ("building", "broadcast"): + withdrawal.status = "failed" + withdrawal.txid = None + elif row.kind == "payout": + # Payout funds come from the pool address, which isn't tracked in + # utxo_events, so there's nothing to release. Clearing payout_txid leaves the + # round in "paying_out" with no tx attached, which is the state an operator + # (or a future payout-retry routine — still an open gap) can act on. + round_ = await session.get(Round, row.round_id) if row.round_id else None + if round_ is not None and round_.status == "paying_out": + round_.payout_txid = None + logger.error( + "round %s payout tx %s vanished — round needs operator attention", round_.id, row.current_txid + ) + + if row.user_id is not None: + await recompute_balance(session, row.user_id) + + await write_audit_log( + session, + "pending_tx_abandoned", + { + "pending_transaction_id": row.id, + "kind": row.kind, + "txid": row.current_txid, + "reason": reason, + "utxos_released": released, + }, + user_id=row.user_id, + round_id=row.round_id, + ) + logger.warning( + "abandoned %s pending_transaction %s (txid %s): %s — released %s UTXO(s)", + row.kind, + row.id, + row.current_txid, + reason, + released, + ) + + +async def _release_inputs(session: AsyncSession, row: PendingTransaction) -> int: + """Clear spent_txid on every UTXO this transaction consumed, so the balance + counts them again. The inputs come from the stored raw tx, which is kept current + across RBF bumps, so this works for a bumped tx too.""" + try: + tx = Transaction.parse(bytes.fromhex(row.raw_tx_hex)) + except Exception: + logger.exception("could not parse raw tx of pending_transaction %s; inputs not released", row.id) + return 0 + + released = 0 + for vin in tx.vin: + utxo = await session.scalar( + select(UtxoEvent).where(UtxoEvent.txid == vin.txid.hex(), UtxoEvent.vout == vin.vout) + ) + # Only release what this tx actually reserved: if another tx has since spent + # the same UTXO, its claim is the live one and must not be cleared. + if utxo is not None and utxo.spent_txid == row.current_txid: + utxo.spent_txid = None + released += 1 + return released diff --git a/app/wallet/balance.py b/app/wallet/balance.py index 24bc502..d911549 100644 --- a/app/wallet/balance.py +++ b/app/wallet/balance.py @@ -46,7 +46,10 @@ async def compute_pending_balance(session: AsyncSession, user: User) -> tuple[in select(PendingTransaction).where( PendingTransaction.user_id == user.id, PendingTransaction.kind.in_(("bet", "withdrawal")), - PendingTransaction.status == "pending", + # "building" as well as "pending": a building row's UTXOs are already + # marked spent (see place_bet's two phases), so leaving it out would + # make the displayed balance dip for the duration of the broadcast. + PendingTransaction.status.in_(("building", "pending")), ) ) ).all() diff --git a/app/withdrawals/confirmation.py b/app/withdrawals/confirmation.py index 04d098b..e111625 100644 --- a/app/withdrawals/confirmation.py +++ b/app/withdrawals/confirmation.py @@ -10,7 +10,9 @@ async def _on_withdrawal_confirmed(session: AsyncSession, pending: PendingTransa if pending.withdrawal_id is None: return withdrawal = await session.get(Withdrawal, pending.withdrawal_id) - if withdrawal is not None and withdrawal.status == "broadcast": + # "building" is reachable if we confirmed before the reconciler promoted the row + # (a crash between broadcast and commit — see app/tx/reconcile.py). + if withdrawal is not None and withdrawal.status in ("building", "broadcast"): withdrawal.status = "confirmed" withdrawal.confirmed_at = datetime.now(timezone.utc) diff --git a/app/withdrawals/service.py b/app/withdrawals/service.py index 45a9538..ff520d6 100644 --- a/app/withdrawals/service.py +++ b/app/withdrawals/service.py @@ -11,7 +11,12 @@ from app.rounds.events import broadcaster from app.wallet.address import is_valid_plm_address from app.wallet.balance import recompute_balance from app.wallet.hd import derive_user_key -from app.wallet.psbt_builder import InsufficientFundsError, Utxo, build_signed_transaction +from app.wallet.psbt_builder import ( + BuiltTransaction, + InsufficientFundsError, + Utxo, + build_signed_transaction, +) class WithdrawalError(ApiError): @@ -27,6 +32,17 @@ async def request_withdrawal( if not is_valid_plm_address(external_address): raise WithdrawalError("invalid_address", "not a valid PLM bech32 address") + # Withdrawing to your own deposit address is a no-op that costs a network fee, + # and it breaks two things that assume the recipient and the change are + # distinguishable by address: the RBF bump would shrink the recipient output + # instead of the change (tx/broadcast.py:_find_change_output), and + # compute_pending_balance would count the amount twice (B-17). + if external_address == user.address: + raise WithdrawalError( + "withdrawal_to_own_address", + "that is your own deposit address — withdraw to an external wallet instead", + ) + config = await get_round_config(session) if amount_sats < config.bet_amount_sats: raise WithdrawalError( @@ -60,8 +76,8 @@ async def request_withdrawal( except InsufficientFundsError as exc: raise WithdrawalError(exc.code, str(exc)) from exc - await client.broadcast(built.raw_hex) - + # Persist the intent before broadcasting, and only promote the rows once the + # network has accepted the tx — same two-phase shape as place_bet (B-08). spent_by_key = {(u.txid, u.vout): u for u in unspent} for spent in built.spent_utxos: spent_by_key[(spent.txid, spent.vout)].spent_txid = built.txid @@ -73,21 +89,32 @@ async def request_withdrawal( amount_requested_sats=amount_sats, amount_sent_sats=built.recipient_sats, txid=built.txid, - status="broadcast", + status="building", ) session.add(withdrawal) await session.flush() - session.add( - PendingTransaction( - kind="withdrawal", - withdrawal_id=withdrawal.id, - user_id=user.id, - current_txid=built.txid, - fee_rate_sat_vb=config.fee_rate_sat_vb, - raw_tx_hex=built.raw_hex, - status="pending", - ) + pending = PendingTransaction( + kind="withdrawal", + withdrawal_id=withdrawal.id, + user_id=user.id, + current_txid=built.txid, + fee_rate_sat_vb=config.fee_rate_sat_vb, + raw_tx_hex=built.raw_hex, + status="building", ) + session.add(pending) + await session.commit() + + try: + await client.broadcast(built.raw_hex) + except Exception as exc: + await _release_failed_withdrawal(session, withdrawal, pending, built, user.id, str(exc)) + raise WithdrawalError( + "broadcast_failed", f"the network refused the transaction: {exc}" + ) from exc + + withdrawal.status = "broadcast" + pending.status = "pending" await write_audit_log( session, "withdrawal_sent", @@ -99,3 +126,35 @@ async def request_withdrawal( await session.refresh(withdrawal) broadcaster.publish() # balance just went "pending" — nudge the dashboard to refetch return withdrawal + + +async def _release_failed_withdrawal( + session: AsyncSession, + withdrawal: Withdrawal, + pending: PendingTransaction, + built: BuiltTransaction, + user_id: int, + reason: str, +) -> None: + """Nothing reached the chain, so free the reserved UTXOs and restore the balance. + The Withdrawal row is kept (marked "failed") rather than deleted: unlike a bet, a + withdrawal is an instruction the user gave, and they should be able to see that it + didn't go through.""" + for spent in built.spent_utxos: + row = await session.scalar( + select(UtxoEvent).where(UtxoEvent.txid == spent.txid, UtxoEvent.vout == spent.vout) + ) + if row is not None: + row.spent_txid = None + withdrawal.status = "failed" + withdrawal.txid = None + pending.status = "failed" + pending.failure_reason = reason[:128] + await recompute_balance(session, user_id) + await write_audit_log( + session, + "withdrawal_broadcast_failed", + {"txid": built.txid, "reason": reason[:200]}, + user_id=user_id, + ) + await session.commit() diff --git a/tests/unit/test_bets.py b/tests/unit/test_bets.py index c3714ee..aa2886a 100644 --- a/tests/unit/test_bets.py +++ b/tests/unit/test_bets.py @@ -133,3 +133,72 @@ async def test_place_bet_rejects_after_timer_expires_even_if_still_open(session_ assert len(participants) == 0 round_ = (await session.scalars(select(Round))).one() assert round_.status == "open" # scheduler hasn't ticked — status is unchanged, only the check is deadline-aware + + +class RejectingElectrumClient: + """A node that refuses the transaction — fee too low, dust output, mempool + conflict, or simply an unreachable server.""" + + async def broadcast(self, raw_tx_hex: str) -> str: + raise RuntimeError("min relay fee not met") + + +async def test_failed_broadcast_leaves_nothing_behind(session_factory): + """B-07/B-08: the broadcast used to happen before anything was written, so a + rejection left the UTXOs marked spent with no rows to explain it, and the caller + got an opaque HTTP 500. Now it's a translatable error and a full rollback.""" + user_id = await _make_funded_user(session_factory, 4, 3_000_000_000) + + async with session_factory() as session: + user = await session.get(User, user_id) + with pytest.raises(BetError, match="refused"): + await place_bet(session, RejectingElectrumClient(), user) + + async with session_factory() as session: + utxo = (await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id))).one() + assert utxo.spent_txid is None # released, so the user can bet again + assert (await session.scalars(select(RoundParticipant))).all() == [] + assert (await session.scalars(select(PendingTransaction))).all() == [] + user = await session.get(User, user_id) + assert user.cached_balance_sats == 3_000_000_000 + events = [e.event_type for e in (await session.scalars(select(AuditLog))).all()] + assert "bet_broadcast_failed" in events + assert "bet_placed" not in events + + +async def test_failed_broadcast_reports_the_broadcast_failed_code(session_factory): + user_id = await _make_funded_user(session_factory, 5, 3_000_000_000) + + async with session_factory() as session: + user = await session.get(User, user_id) + try: + await place_bet(session, RejectingElectrumClient(), user) + assert False, "expected BetError" + except BetError as exc: + assert exc.code == "broadcast_failed" + + +async def test_bet_is_persisted_before_it_is_broadcast(session_factory): + """The ordering guarantee behind B-08: by the time the network call happens, the + rows already exist, so a crash there is recoverable rather than silent.""" + user_id = await _make_funded_user(session_factory, 6, 3_000_000_000) + seen: dict[str, object] = {} + + class ObservingClient: + async def broadcast(self, raw_tx_hex: str) -> str: + # Read committed state from an independent session, mid-broadcast. + async with session_factory() as probe: + seen["pending"] = [ + (p.kind, p.status) for p in (await probe.scalars(select(PendingTransaction))).all() + ] + seen["participants"] = [ + (p.status) for p in (await probe.scalars(select(RoundParticipant))).all() + ] + return "network-txid" + + async with session_factory() as session: + user = await session.get(User, user_id) + await place_bet(session, ObservingClient(), user) + + assert seen["pending"] == [("bet", "building")] + assert seen["participants"] == ["building"] diff --git a/tests/unit/test_broadcast.py b/tests/unit/test_broadcast.py index 072b483..3ccc33e 100644 --- a/tests/unit/test_broadcast.py +++ b/tests/unit/test_broadcast.py @@ -179,3 +179,72 @@ async def test_bump_fee_raises_when_no_change_output(session_factory): row = await session.get(PendingTransaction, pending_id) with pytest.raises(RbfError): await bump_fee(session, client, row) + + +async def test_bump_fee_retargets_every_stored_txid(session_factory): + """B-02/B-20: a bump changes the txid, and everything that recorded the old one + has to follow — the participant's bet_txid (whose staleness used to wedge the + round forever), the UTXO's spent_txid (which the reconciler matches on), and + replaced_by_txid, which was never written at all.""" + from app.db.models import Round, RoundParticipant, UtxoEvent + from app.wallet.hd import derive_user_address, derive_user_key + + signer = derive_user_key(0) + my_address = derive_user_address(0) + from_script = script.p2wpkh(signer.to_public()) + to_address = script.p2wpkh(_key(98).to_public()).address(network=PLM_MAINNET) + + utxo_amount = 150_000_000 + utxo_txid = "22" * 32 + built = build_signed_transaction( + signing_key=signer, + from_script=from_script, + utxos=[Utxo(utxo_txid, 0, utxo_amount)], + to_address=to_address, + amount_sats=10_000_000, + change_address=my_address, + fee_rate_sat_vb=1, + ) + + async with session_factory() as session: + user = User(username="bob", password_hash="x", derivation_index=0, address=my_address) + session.add(user) + session.add(Round(id=1, status="open")) + await session.flush() + session.add( + UtxoEvent( + user_id=user.id, txid=utxo_txid, vout=0, amount_sats=utxo_amount, + confirmed_height=5, spent_txid=built.txid, + ) + ) + session.add( + RoundParticipant( + round_id=1, user_id=user.id, bet_amount_sats=built.recipient_sats, + bet_txid=built.txid, status="broadcast", + ) + ) + pending = PendingTransaction( + kind="bet", round_id=1, user_id=user.id, current_txid=built.txid, fee_rate_sat_vb=1, + raw_tx_hex=built.raw_hex, status="pending", + broadcast_at=datetime.now(timezone.utc) - timedelta(seconds=1000), + ) + session.add(pending) + await session.commit() + pending_id = pending.id + + async with session_factory() as session: + row = await session.get(PendingTransaction, pending_id) + new_txid = await bump_fee(session, FakeClient({utxo_txid: utxo_amount}), row) + + async with session_factory() as session: + from sqlalchemy import select + + row = await session.get(PendingTransaction, pending_id) + assert row.current_txid == new_txid + assert row.replaced_by_txid == built.txid # points backwards at what it replaced + + participant = (await session.scalars(select(RoundParticipant))).one() + assert participant.bet_txid == new_txid + + utxo = (await session.scalars(select(UtxoEvent))).one() + assert utxo.spent_txid == new_txid diff --git a/tests/unit/test_confirmation.py b/tests/unit/test_confirmation.py index 24bf060..056f3d4 100644 --- a/tests/unit/test_confirmation.py +++ b/tests/unit/test_confirmation.py @@ -80,3 +80,94 @@ async def test_payout_confirmation_closes_round(session_factory): async with session_factory() as session: round_ = await session.get(Round, 3) assert round_.status == "closed" + + +class ExplodingClient: + """Answers for one txid and raises for the other — a tx the server no longer + knows (dropped from the mempool, replaced by a bump).""" + + def __init__(self, known: dict[str, int], exploding_txid: str): + self._known = known + self._exploding = exploding_txid + + async def get_transaction(self, txid: str, verbose: bool = False) -> dict: + if txid == self._exploding: + raise RuntimeError("missing transaction") + return {"confirmations": self._known.get(txid, 0)} + + +async def test_one_unresolvable_txid_does_not_block_the_others(session_factory): + """B-03: the lookup used to be unguarded, so a single unknown txid aborted the + whole pass — nothing confirmed again until an operator intervened, which in turn + meant no round could ever close.""" + async with session_factory() as session: + session.add(Round(id=10, status="open")) + session.add( + RoundParticipant(round_id=10, user_id=1, bet_amount_sats=1_000, bet_txid="good", status="broadcast") + ) + session.add( + PendingTransaction( + kind="bet", round_id=10, user_id=2, current_txid="gone", fee_rate_sat_vb=1, raw_tx_hex="00", + status="pending", + ) + ) + session.add( + PendingTransaction( + kind="bet", round_id=10, user_id=1, current_txid="good", fee_rate_sat_vb=1, raw_tx_hex="00", + status="pending", + ) + ) + await session.commit() + + confirmed = await poll_once(session_factory, ExplodingClient({"good": 1}, exploding_txid="gone")) + assert confirmed == 1 # the healthy one still got processed + + async with session_factory() as session: + participant = (await session.scalars(select(RoundParticipant).where(RoundParticipant.round_id == 10))).one() + assert participant.status == "confirmed" + rows = {p.current_txid: p.status for p in (await session.scalars(select(PendingTransaction))).all()} + assert rows["good"] == "confirmed" + assert rows["gone"] == "pending" # left for the reconciler to judge, not abandoned here + + +async def test_bet_confirms_after_an_rbf_bump_changed_the_txid(session_factory): + """B-02: the handler used to match on bet_txid, so a bumped bet confirmed under + a txid no participant carried — the participant stayed "broadcast" forever and + the round could never close. It now resolves by (round_id, user_id).""" + async with session_factory() as session: + session.add(Round(id=11, status="open")) + session.add( + RoundParticipant( + round_id=11, user_id=7, bet_amount_sats=1_000, bet_txid="old-txid", status="broadcast" + ) + ) + session.add( + PendingTransaction( + kind="bet", round_id=11, user_id=7, current_txid="bumped-txid", fee_rate_sat_vb=2, + raw_tx_hex="00", status="pending", replaced_by_txid="old-txid", + ) + ) + await session.commit() + + assert await poll_once(session_factory, FakeClient({"bumped-txid": 1})) == 1 + + async with session_factory() as session: + participant = (await session.scalars(select(RoundParticipant).where(RoundParticipant.round_id == 11))).one() + assert participant.status == "confirmed" + + +async def test_payout_confirms_after_an_rbf_bump_changed_the_txid(session_factory): + async with session_factory() as session: + session.add(Round(id=12, status="paying_out", payout_txid="old-payout")) + session.add( + PendingTransaction( + kind="payout", round_id=12, current_txid="bumped-payout", fee_rate_sat_vb=2, + raw_tx_hex="00", status="pending", + ) + ) + await session.commit() + + assert await poll_once(session_factory, FakeClient({"bumped-payout": 1})) == 1 + + async with session_factory() as session: + assert (await session.get(Round, 12)).status == "closed" diff --git a/tests/unit/test_reconcile.py b/tests/unit/test_reconcile.py new file mode 100644 index 0000000..3c757a3 --- /dev/null +++ b/tests/unit/test_reconcile.py @@ -0,0 +1,233 @@ +"""Regression tests for B-04 (and the "building" half of B-08): a transaction that +never made it onto the chain must give the coins back instead of freezing them.""" + +import pytest +from embit import script +from embit.transaction import Transaction, TransactionInput, TransactionOutput +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 AuditLog, PendingTransaction, RoundParticipant, User, UtxoEvent, Withdrawal +from app.tx.reconcile import reconcile_once + + +class UnknownTxClient: + """A server that doesn't know any of the txids it's asked about.""" + + async def get_transaction(self, txid: str, verbose: bool = False): + raise RuntimeError(f"missing transaction {txid}") + + +class KnownTxClient: + async def get_transaction(self, txid: str, verbose: bool = False): + return {"txid": txid, "confirmations": 0} + + +class BrokenClient: + """A transport failure — says nothing about whether the tx exists.""" + + async def get_transaction(self, txid: str, verbose: bool = False): + raise ConnectionResetError("connection reset") + + +@pytest.fixture +async def session_factory(): + 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() + + +# A real (unsigned) transaction spending one input, built rather than hand-written +# so it round-trips through Transaction.parse — that parse is how the reconciler +# discovers which UTXOs to release, so a fixture the parser rejects would test +# nothing. +_TX_INPUT_TXID = "11" * 32 +_RAW_TX = ( + Transaction( + vin=[TransactionInput(bytes.fromhex(_TX_INPUT_TXID), 0)], + vout=[ + TransactionOutput( + 999_000_000, script.Script.from_address("plm1q0xcqpzrky6eff2g52qdye53xkk9jxkvraxkkwd") + ) + ], + ) + .serialize() + .hex() +) + + +async def _seed_bet(session_factory, *, pending_status: str, participant_status: str, age_seconds: int): + from datetime import datetime, timedelta, timezone + + async with session_factory() as session: + user = User(username="u", password_hash="x", derivation_index=0, address="plm1qtest") + session.add(user) + await session.flush() + session.add( + UtxoEvent( + user_id=user.id, + txid=_TX_INPUT_TXID, + vout=0, + amount_sats=1_000_000_000, + confirmed_height=10, + spent_txid="betxid", + ) + ) + session.add( + RoundParticipant( + round_id=1, + user_id=user.id, + bet_amount_sats=999_000_000, + bet_txid="betxid", + status=participant_status, + ) + ) + session.add( + PendingTransaction( + kind="bet", + round_id=1, + user_id=user.id, + current_txid="betxid", + fee_rate_sat_vb=1, + raw_tx_hex=_RAW_TX, + status=pending_status, + broadcast_at=datetime.now(timezone.utc) - timedelta(seconds=age_seconds), + ) + ) + await session.commit() + return user.id + + +async def test_abandons_a_building_bet_and_gives_the_coins_back(session_factory): + """The crash-mid-broadcast case: the tx isn't on the chain, so the UTXO must be + released, the participant removed (they never entered the round) and the balance + restored. Before this existed, spent_txid stayed set forever and the user simply + lost the coins.""" + user_id = await _seed_bet( + session_factory, pending_status="building", participant_status="building", age_seconds=300 + ) + + resolved = await reconcile_once(session_factory, UnknownTxClient()) + assert resolved == 1 + + async with session_factory() as session: + utxo = (await session.scalars(select(UtxoEvent))).one() + assert utxo.spent_txid is None # spendable again + assert (await session.scalars(select(RoundParticipant))).all() == [] + row = (await session.scalars(select(PendingTransaction))).one() + assert row.status == "failed" + assert row.failure_reason + user = await session.get(User, user_id) + assert user.cached_balance_sats == 1_000_000_000 + events = [e.event_type for e in (await session.scalars(select(AuditLog))).all()] + assert "pending_tx_abandoned" in events + + +async def test_promotes_a_building_row_whose_tx_did_reach_the_chain(session_factory): + """We died after the broadcast, not before: the tx is real, so the rows must be + finished rather than rolled back.""" + await _seed_bet( + session_factory, pending_status="building", participant_status="building", age_seconds=300 + ) + + resolved = await reconcile_once(session_factory, KnownTxClient()) + assert resolved == 1 + + async with session_factory() as session: + row = (await session.scalars(select(PendingTransaction))).one() + assert row.status == "pending" + participant = (await session.scalars(select(RoundParticipant))).one() + assert participant.status == "broadcast" + utxo = (await session.scalars(select(UtxoEvent))).one() + assert utxo.spent_txid == "betxid" # still legitimately spent + + +async def test_leaves_a_young_building_row_alone(session_factory): + """A row written seconds ago may just be a broadcast still in flight.""" + await _seed_bet( + session_factory, pending_status="building", participant_status="building", age_seconds=5 + ) + + assert await reconcile_once(session_factory, UnknownTxClient()) == 0 + + async with session_factory() as session: + assert (await session.scalars(select(PendingTransaction))).one().status == "building" + + +async def test_leaves_a_recently_broadcast_pending_row_alone(session_factory): + """A broadcast tx gets a wide grace window — absence from one server's mempool + is not proof of death, and the RBF bumper should get its attempts first.""" + await _seed_bet( + session_factory, pending_status="pending", participant_status="broadcast", age_seconds=3600 + ) + + assert await reconcile_once(session_factory, UnknownTxClient()) == 0 + + +async def test_transport_failure_never_abandons_anything(session_factory): + """A dead connection says nothing about the transaction. Treating it as "gone" + would release coins for transactions that are perfectly alive.""" + await _seed_bet( + session_factory, pending_status="building", participant_status="building", age_seconds=300 + ) + + assert await reconcile_once(session_factory, BrokenClient()) == 0 + + async with session_factory() as session: + assert (await session.scalars(select(PendingTransaction))).one().status == "building" + assert (await session.scalars(select(UtxoEvent))).one().spent_txid == "betxid" + + +async def test_abandoned_withdrawal_is_marked_failed_and_kept(session_factory): + """Unlike a bet, a withdrawal is an instruction the user gave: the row stays so + they can see it didn't go through.""" + from datetime import datetime, timedelta, timezone + + async with session_factory() as session: + user = User(username="w", password_hash="x", derivation_index=1, address="plm1qtest2") + session.add(user) + await session.flush() + session.add( + UtxoEvent( + user_id=user.id, + txid=_TX_INPUT_TXID, + vout=0, + amount_sats=500_000_000, + confirmed_height=10, + spent_txid="wdtxid", + ) + ) + withdrawal = Withdrawal( + user_id=user.id, + external_address="plm1qexternal", + amount_requested_sats=400_000_000, + amount_sent_sats=399_000_000, + txid="wdtxid", + status="broadcast", + ) + session.add(withdrawal) + await session.flush() + session.add( + PendingTransaction( + kind="withdrawal", + withdrawal_id=withdrawal.id, + user_id=user.id, + current_txid="wdtxid", + fee_rate_sat_vb=1, + raw_tx_hex=_RAW_TX, + status="pending", + broadcast_at=datetime.now(timezone.utc) - timedelta(days=1), + ) + ) + await session.commit() + + assert await reconcile_once(session_factory, UnknownTxClient()) == 1 + + async with session_factory() as session: + withdrawal = (await session.scalars(select(Withdrawal))).one() + assert withdrawal.status == "failed" + assert withdrawal.txid is None + assert (await session.scalars(select(UtxoEvent))).one().spent_txid is None diff --git a/tests/unit/test_withdrawals.py b/tests/unit/test_withdrawals.py index dda3e48..e882cc2 100644 --- a/tests/unit/test_withdrawals.py +++ b/tests/unit/test_withdrawals.py @@ -4,7 +4,7 @@ from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from app.config import settings from app.db.base import Base -from app.db.models import PendingTransaction, User, UtxoEvent +from app.db.models import PendingTransaction, User, UtxoEvent, Withdrawal from app.wallet.hd import derive_user_address from app.withdrawals.service import WithdrawalError, request_withdrawal @@ -122,3 +122,44 @@ async def test_withdrawal_rejects_non_plm_address(session_factory, address): assert exc_info.value.code == "invalid_address" assert not client.broadcasted + + +async def test_withdrawal_to_own_address_is_rejected(session_factory): + """B-17: allowed before, and it broke two things that assume the recipient and + the change are distinguishable by address — the RBF bump would shrink the + recipient output, and compute_pending_balance counted the amount twice.""" + user_id = await _make_funded_user(session_factory, 8, 3_000_000_000) + client = FakeElectrumClient() + + async with session_factory() as session: + user = await session.get(User, user_id) + with pytest.raises(WithdrawalError, match="own deposit address"): + await request_withdrawal(session, client, user, user.address, 1_000_000_000) + + assert not client.broadcasted + async with session_factory() as session: + assert (await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id))).one().spent_txid is None + + +async def test_failed_broadcast_marks_the_withdrawal_failed_and_frees_the_coins(session_factory): + """B-07/B-08: the Withdrawal row is kept (unlike a bet) so the user can see the + instruction didn't go through, but the coins must come back.""" + user_id = await _make_funded_user(session_factory, 9, 3_000_000_000) + + class RejectingClient: + async def broadcast(self, raw_tx_hex: str) -> str: + raise RuntimeError("min relay fee not met") + + async with session_factory() as session: + user = await session.get(User, user_id) + external = derive_user_address(99) + with pytest.raises(WithdrawalError, match="refused"): + await request_withdrawal(session, RejectingClient(), user, external, 1_000_000_000) + + async with session_factory() as session: + withdrawal = (await session.scalars(select(Withdrawal))).one() + assert withdrawal.status == "failed" + assert withdrawal.txid is None + assert (await session.scalars(select(UtxoEvent).where(UtxoEvent.user_id == user_id))).one().spent_txid is None + user = await session.get(User, user_id) + assert user.cached_balance_sats == 3_000_000_000