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