From 7394b10e75cc908f790a3e935b7be8b1dcbc839c Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Fri, 24 Jul 2026 21:29:21 +0200 Subject: [PATCH 1/8] fix-concurent-issue --- routstr/core/db.py | 104 ++++++- routstr/core/settings.py | 3 + routstr/lightning.py | 143 +++++---- routstr/proxy.py | 15 + routstr/wallet.py | 274 +++++++++--------- .../test_lightning_invoice_constraints.py | 144 ++++++++- tests/unit/test_db_pool_config.py | 83 ++++++ tests/unit/test_fee_payout_crash_safety.py | 30 +- tests/unit/test_fetch_all_balances.py | 174 ++++++++++- tests/unit/test_periodic_payout.py | 148 +++++++--- tests/unit/test_proxy_session_lifecycle.py | 43 +++ tests/unit/test_refund_sweep.py | 38 +++ 12 files changed, 952 insertions(+), 247 deletions(-) create mode 100644 tests/unit/test_db_pool_config.py create mode 100644 tests/unit/test_proxy_session_lifecycle.py diff --git a/routstr/core/db.py b/routstr/core/db.py index 16cfedfd..ab556cc5 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -12,7 +12,8 @@ from typing import AsyncGenerator from alembic import command from alembic.config import Config from alembic.util.exc import CommandError -from sqlalchemy import Index, UniqueConstraint, case, delete, or_ +from sqlalchemy import Index, UniqueConstraint, case, delete, event, or_ +from sqlalchemy.engine import make_url from sqlalchemy.exc import IntegrityError, OperationalError from sqlalchemy.ext.asyncio.engine import create_async_engine from sqlalchemy.orm import aliased @@ -26,7 +27,82 @@ logger = get_logger(__name__) DATABASE_URL = os.environ.get("DATABASE_URL", "sqlite+aiosqlite:///keys.db") -engine = create_async_engine(DATABASE_URL, echo=False) # echo=True for debugging SQL +def _env_bool(name: str, default: bool) -> bool: + raw = os.environ.get(name) + if raw is None: + return default + normalized = raw.strip().lower() + if normalized in {"1", "true", "yes", "on"}: + return True + if normalized in {"0", "false", "no", "off"}: + return False + raise ValueError(f"{name} must be a boolean") + + +def _engine_options(database_url: str) -> dict[str, int | float | bool]: + """Build a bounded pool configuration, preserving SQLite memory semantics.""" + options: dict[str, int | float | bool] = { + "pool_pre_ping": _env_bool("DATABASE_POOL_PRE_PING", True) + } + url = make_url(database_url) + is_memory_sqlite = url.get_backend_name() == "sqlite" and url.database in { + None, + "", + ":memory:", + } + if is_memory_sqlite: + return options + + pool_size = int(os.environ.get("DATABASE_POOL_SIZE", "5")) + max_overflow = int(os.environ.get("DATABASE_MAX_OVERFLOW", "0")) + pool_timeout = float(os.environ.get("DATABASE_POOL_TIMEOUT", "5")) + pool_recycle = int(os.environ.get("DATABASE_POOL_RECYCLE", "1800")) + if pool_size < 1: + raise ValueError("DATABASE_POOL_SIZE must be at least 1") + if max_overflow < 0: + raise ValueError("DATABASE_MAX_OVERFLOW cannot be negative") + if pool_timeout <= 0: + raise ValueError("DATABASE_POOL_TIMEOUT must be positive") + if pool_recycle < 0: + raise ValueError("DATABASE_POOL_RECYCLE cannot be negative") + options.update( + pool_size=pool_size, + max_overflow=max_overflow, + pool_timeout=pool_timeout, + pool_recycle=pool_recycle, + ) + return options + + +engine = create_async_engine(DATABASE_URL, echo=False, **_engine_options(DATABASE_URL)) + +_POOL_HOLD_WARN_SECONDS = float(os.environ.get("DATABASE_POOL_HOLD_WARN_SECONDS", "10")) +if _POOL_HOLD_WARN_SECONDS <= 0: + raise ValueError("DATABASE_POOL_HOLD_WARN_SECONDS must be positive") + + +@event.listens_for(engine.sync_engine, "checkout") +def _record_pool_checkout( + dbapi_connection: object, connection_record: object, proxy: object +) -> None: + connection_record.info["routstr_checked_out_at"] = time.monotonic() # type: ignore[attr-defined] + + +@event.listens_for(engine.sync_engine, "checkin") +def _record_pool_checkin(dbapi_connection: object, connection_record: object) -> None: + checked_out_at = connection_record.info.pop("routstr_checked_out_at", None) # type: ignore[attr-defined] + if checked_out_at is None: + return + held_seconds = time.monotonic() - checked_out_at + if held_seconds >= _POOL_HOLD_WARN_SECONDS: + logger.warning( + "Database connection held longer than threshold", + extra={ + "held_seconds": round(held_seconds, 3), + "threshold_seconds": _POOL_HOLD_WARN_SECONDS, + "pool_status": engine.pool.status(), + }, + ) class ApiKey(SQLModel, table=True): # type: ignore @@ -727,6 +803,30 @@ async def balances_for_mint_and_unit( return result.one() or 0 +async def balances_by_mint_and_unit( + db_session: AsyncSession, +) -> dict[tuple[str, str], int]: + """Return all user liabilities in one query, grouped by mint and unit.""" + query = ( + select( + col(ApiKey.refund_mint_url), + col(ApiKey.refund_currency), + func.sum(ApiKey.balance), + ) + .where( + col(ApiKey.refund_mint_url).is_not(None), + col(ApiKey.refund_currency).is_not(None), + ) + .group_by(col(ApiKey.refund_mint_url), col(ApiKey.refund_currency)) + ) + result = await db_session.exec(query) + return { + (mint_url, unit): int(balance or 0) + for mint_url, unit, balance in result.all() + if mint_url is not None and unit is not None + } + + async def init_db() -> None: """Initializes the database and creates tables if they don't exist.""" async with engine.begin() as conn: diff --git a/routstr/core/settings.py b/routstr/core/settings.py index a0b5d05c..991c7554 100644 --- a/routstr/core/settings.py +++ b/routstr/core/settings.py @@ -41,6 +41,9 @@ class Settings(BaseSettings): receive_ln_address: str = Field(default="", env="RECEIVE_LN_ADDRESS") primary_mint: str = Field(default="", env="PRIMARY_MINT_URL") primary_mint_unit: str = Field(default="sat", env="PRIMARY_MINT_UNIT") + mint_operation_concurrency: int = Field( + default=4, ge=1, env="MINT_OPERATION_CONCURRENCY" + ) # Lightning payout configuration # Minimum available balance (in satoshis) before profit is paid out over diff --git a/routstr/lightning.py b/routstr/lightning.py index b0bbc63b..30aa7918 100644 --- a/routstr/lightning.py +++ b/routstr/lightning.py @@ -5,7 +5,7 @@ import time from fastapi import APIRouter, Depends, Header, HTTPException from pydantic import BaseModel, Field -from sqlmodel import col, select +from sqlmodel import col, select, update from sqlmodel.ext.asyncio.session import AsyncSession from .core.db import ApiKey, LightningInvoice, create_session, get_session @@ -222,62 +222,114 @@ async def recover_invoice( async def check_invoice_payment( invoice: LightningInvoice, session: AsyncSession ) -> None: + minted = False + invoice_id = invoice.id + invoice_purpose = invoice.purpose try: + # A preceding invoice lookup starts a transaction. End it before the + # potentially slow mint request so it cannot pin a pool connection. + await session.commit() + wallet = await get_wallet(settings.primary_mint, "sat") - mint_status = await wallet.get_mint_quote(invoice.payment_hash) + if not mint_status.paid: + return - if mint_status.paid: - invoice.status = "paid" - invoice.paid_at = int(time.time()) + # The mint enforces single-use quotes, so a concurrent checker that + # races us here fails inside wallet.mint rather than double-minting. + await wallet.mint(invoice.amount_sats, quote_id=invoice.payment_hash) + minted = True - if invoice.purpose == "create": - api_key = await create_api_key_from_invoice(invoice, session) - invoice.api_key_hash = api_key.hashed_key - elif invoice.purpose == "topup" and invoice.api_key_hash: - await topup_api_key_from_invoice(invoice, session) + if invoice_purpose == "create": + api_key = await _create_api_key_record(invoice, session) + invoice.api_key_hash = api_key.hashed_key + elif invoice_purpose == "topup": + await _credit_topup_record(invoice, session) - await session.commit() - - logger.info( - "Lightning invoice paid", - extra={ - "invoice_id": invoice.id, - "amount_sats": invoice.amount_sats, - "purpose": invoice.purpose, - "api_key_hash": invoice.api_key_hash[:8] + "..." - if invoice.api_key_hash - else None, - }, + # Conditional transition guards against double-credit: the credit + # above and this status flip commit atomically, and a lost race + # rolls both back. + paid_at = int(time.time()) + finalized = await session.exec( # type: ignore[call-overload] + update(LightningInvoice) + .where( + col(LightningInvoice.id) == invoice_id, + col(LightningInvoice.status) == "pending", ) + .values( + status="paid", + paid_at=paid_at, + api_key_hash=invoice.api_key_hash, + ) + ) + if finalized.rowcount != 1: + await session.rollback() + await session.refresh(invoice) + return + await session.commit() + invoice.status = "paid" + invoice.paid_at = paid_at + logger.info( + "Lightning invoice paid", + extra={ + "invoice_id": invoice_id, + "amount_sats": invoice.amount_sats, + "purpose": invoice_purpose, + "api_key_hash": invoice.api_key_hash[:8] + "..." + if invoice.api_key_hash + else None, + }, + ) except Exception as e: + await session.rollback() + if minted: + logger.critical( + "Invoice mint succeeded but DB finalization failed; reconciliation required", + extra={"invoice_id": invoice_id, "purpose": invoice_purpose}, + ) logger.error(f"Failed to check invoice payment: {e}") +async def _create_api_key_record( + invoice: LightningInvoice, session: AsyncSession +) -> ApiKey: + dummy_token = f"invoice-{invoice.id}-{invoice.payment_hash}" + hashed_key = hashlib.sha256(dummy_token.encode()).hexdigest() + api_key = ApiKey( + hashed_key=hashed_key, + balance=invoice.amount_sats * 1000, + refund_currency="sat", + refund_mint_url=settings.primary_mint, + balance_limit=invoice.balance_limit, + balance_limit_reset=invoice.balance_limit_reset, + validity_date=invoice.validity_date, + ) + session.add(api_key) + await session.flush() + return api_key + + +async def _credit_topup_record( + invoice: LightningInvoice, session: AsyncSession +) -> None: + if not invoice.api_key_hash: + raise ValueError("No API key associated with topup invoice") + credited = await session.exec( # type: ignore[call-overload] + update(ApiKey) + .where(col(ApiKey.hashed_key) == invoice.api_key_hash) + .values(balance=col(ApiKey.balance) + invoice.amount_sats * 1000) + ) + if credited.rowcount != 1: + raise ValueError("Associated API key not found") + + async def create_api_key_from_invoice( invoice: LightningInvoice, session: AsyncSession ) -> ApiKey: wallet = await get_wallet(settings.primary_mint, "sat") await wallet.mint(invoice.amount_sats, quote_id=invoice.payment_hash) - - dummy_token = f"invoice-{invoice.id}-{invoice.payment_hash}" - hashed_key = hashlib.sha256(dummy_token.encode()).hexdigest() - - api_key = ApiKey( - hashed_key=hashed_key, - balance=invoice.amount_sats * 1000, # Convert to msats - refund_currency="sat", - refund_mint_url=settings.primary_mint, - balance_limit=invoice.balance_limit, - balance_limit_reset=invoice.balance_limit_reset, - validity_date=invoice.validity_date, - ) - - session.add(api_key) - await session.flush() - - return api_key + return await _create_api_key_record(invoice, session) async def topup_api_key_from_invoice( @@ -285,16 +337,7 @@ async def topup_api_key_from_invoice( ) -> None: wallet = await get_wallet(settings.primary_mint, "sat") await wallet.mint(invoice.amount_sats, quote_id=invoice.payment_hash) - - if not invoice.api_key_hash: - raise ValueError("No API key associated with topup invoice") - - api_key = await session.get(ApiKey, invoice.api_key_hash) - if not api_key: - raise ValueError("Associated API key not found") - - api_key.balance += invoice.amount_sats * 1000 # Convert to msats - await session.flush() + await _credit_topup_record(invoice, session) INVOICE_WATCH_INTERVAL_SECONDS = 5 diff --git a/routstr/proxy.py b/routstr/proxy.py index c0b8794f..46596077 100644 --- a/routstr/proxy.py +++ b/routstr/proxy.py @@ -1,4 +1,5 @@ import asyncio +import inspect import json from typing import Any @@ -220,6 +221,20 @@ _API_PATH_PREFIXES = ( @proxy_router.api_route("/{path:path}", methods=["GET", "POST"], response_model=None) async def proxy( request: Request, path: str, session: AsyncSession = Depends(get_session) +) -> Response | StreamingResponse: + """Run proxy setup in a short request session, never across response streaming.""" + try: + return await _proxy(request, path, session) + finally: + # FastAPI yield dependencies normally close after the response body is + # sent. Close explicitly so a long stream cannot retain DB resources. + close_result = session.close() + if inspect.isawaitable(close_result): + await close_result + + +async def _proxy( + request: Request, path: str, session: AsyncSession ) -> Response | StreamingResponse: # GET requests must hit a known API prefix; otherwise return a 404 (HTML # for browsers, JSON for API clients). POST requests are always forwarded diff --git a/routstr/wallet.py b/routstr/wallet.py index dd92d913..24b11fe9 100644 --- a/routstr/wallet.py +++ b/routstr/wallet.py @@ -836,16 +836,20 @@ async def fetch_all_balances( if units is None: units = ["sat", "msat"] - async def fetch_balance( - session: db.AsyncSession, mint_url: str, unit: str - ) -> BalanceDetail: + async with db.create_session() as session: + user_balances = await db.balances_by_mint_and_unit(session) + + mint_check_limit = asyncio.Semaphore(settings.mint_operation_concurrency) + + async def fetch_balance(mint_url: str, unit: str) -> BalanceDetail: try: - wallet = await get_wallet(mint_url, unit) - proofs = get_proofs_per_mint_and_unit( - wallet, mint_url, unit, not_reserved=True - ) - proofs = await slow_filter_spend_proofs(proofs, wallet) - user_balance = await db.balances_for_mint_and_unit(session, mint_url, unit) + async with mint_check_limit: + wallet = await get_wallet(mint_url, unit) + proofs = get_proofs_per_mint_and_unit( + wallet, mint_url, unit, not_reserved=True + ) + proofs = await slow_filter_spend_proofs(proofs, wallet) + user_balance = user_balances.get((mint_url, unit), 0) if unit == "sat": user_balance = user_balance // 1000 proofs_balance = sum(proof.amount for proof in proofs) @@ -878,16 +882,8 @@ async def fetch_all_balances( if settings.primary_mint and settings.primary_mint not in mint_urls: mint_urls.append(settings.primary_mint) - # Create tasks for all mint/unit combinations - async with db.create_session() as session: - tasks = [ - fetch_balance(session, mint_url, unit) - for mint_url in mint_urls - for unit in units - ] - - # Run all tasks concurrently - balance_details = list(await asyncio.gather(*tasks)) + tasks = [fetch_balance(mint_url, unit) for mint_url in mint_urls for unit in units] + balance_details = list(await asyncio.gather(*tasks)) # Calculate totals total_wallet_balance_sats = 0 @@ -936,56 +932,56 @@ async def periodic_payout() -> None: mint_urls.append(settings.primary_mint) async with db.create_session() as session: - for mint_url in mint_urls: - for unit in ["sat", "msat"]: - # Isolate failures per mint/unit so one slow or failing - # mint does not abort payout for every other mint/unit. - try: - wallet = await get_wallet(mint_url, unit) - proofs = get_proofs_per_mint_and_unit( - wallet, mint_url, unit, not_reserved=True + user_balances = await db.balances_by_mint_and_unit(session) + + for mint_url in mint_urls: + for unit in ["sat", "msat"]: + # Isolate failures per mint/unit so one slow or failing + # mint does not abort payout for every other mint/unit. + try: + wallet = await get_wallet(mint_url, unit) + proofs = get_proofs_per_mint_and_unit( + wallet, mint_url, unit, not_reserved=True + ) + proofs = await slow_filter_spend_proofs(proofs, wallet) + await asyncio.sleep(5) + user_balance = user_balances.get((mint_url, unit), 0) + if unit == "sat": + user_balance = user_balance // 1000 + proofs_balance = sum(proof.amount for proof in proofs) + available_balance = proofs_balance - user_balance + # Threshold is configured in sats; convert for msat wallets. + min_amount = ( + settings.min_payout_sat + if unit == "sat" + else settings.min_payout_sat * 1000 + ) + if available_balance > min_amount: + amount_received = await raw_send_to_lnurl( + wallet, + proofs, + settings.receive_ln_address, + unit, + amount=available_balance, ) - proofs = await slow_filter_spend_proofs(proofs, wallet) - await asyncio.sleep(5) - user_balance = await db.balances_for_mint_and_unit( - session, mint_url, unit - ) - if unit == "sat": - user_balance = user_balance // 1000 - proofs_balance = sum(proof.amount for proof in proofs) - available_balance = proofs_balance - user_balance - # Threshold is configured in sats; convert for msat wallets. - min_amount = ( - settings.min_payout_sat - if unit == "sat" - else settings.min_payout_sat * 1000 - ) - if available_balance > min_amount: - amount_received = await raw_send_to_lnurl( - wallet, - proofs, - settings.receive_ln_address, - unit, - amount=available_balance, - ) - logger.info( - "Payout sent successfully", - extra={ - "mint_url": mint_url, - "unit": unit, - "balance": available_balance, - "amount_received": amount_received, - }, - ) - except Exception as e: - logger.error( - f"Error sending payout: {type(e).__name__}", + logger.info( + "Payout sent successfully", extra={ - "error": str(e), "mint_url": mint_url, "unit": unit, + "balance": available_balance, + "amount_received": amount_received, }, ) + except Exception as e: + logger.error( + f"Error sending payout: {type(e).__name__}", + extra={ + "error": str(e), + "mint_url": mint_url, + "unit": unit, + }, + ) except Exception as e: logger.error( f"Error in periodic payout cycle: {type(e).__name__}", @@ -1004,39 +1000,49 @@ async def _refund_sweep_once(cutoff: int) -> None: results = await session.exec(stmt) refunds = results.all() - for refund in refunds: - try: - await recieve_token(refund.token) - refund.swept = True - session.add(refund) + for refund in refunds: + try: + await recieve_token(refund.token) + async with db.create_session() as session: + await session.exec( # type: ignore[call-overload] + update(db.CashuTransaction) + .where( + col(db.CashuTransaction.id) == refund.id, + col(db.CashuTransaction.swept) == False, # noqa: E712 + ) + .values(swept=True) + ) + await session.commit() + logger.info( + "Swept uncollected refund", + extra={ + "id": refund.id, + "amount": refund.amount, + "unit": refund.unit, + }, + ) + except Exception as e: + error_msg = str(e).lower() + if "already spent" in error_msg: + async with db.create_session() as session: + await session.exec( # type: ignore[call-overload] + update(db.CashuTransaction) + .where(col(db.CashuTransaction.id) == refund.id) + .values(collected=True) + ) + await session.commit() logger.info( - "Swept uncollected refund", + "Refund already spent (client collected), marking swept", + extra={"id": refund.id}, + ) + else: + logger.warning( + "Failed to sweep refund", extra={ "id": refund.id, - "amount": refund.amount, - "unit": refund.unit, + "error": str(e), }, ) - except Exception as e: - error_msg = str(e).lower() - if "already spent" in error_msg: - refund.collected = True - session.add(refund) - logger.info( - "Refund already spent (client collected), marking swept", - extra={ - "id": refund.id, - }, - ) - else: - logger.warning( - "Failed to sweep refund", - extra={ - "id": refund.id, - "error": str(e), - }, - ) - await session.commit() async def refund_sweep_once() -> None: @@ -1083,52 +1089,52 @@ async def periodic_routstr_fee_payout() -> None: continue accumulated_sats = fee.accumulated_msats // 1000 - if accumulated_sats >= ROUTSTR_FEE_DEFAULT_PAYOUT: - wallet = await get_wallet(settings.primary_mint, "sat") - proofs = get_proofs_per_mint_and_unit( - wallet, settings.primary_mint, "sat", not_reserved=True - ) - paid_msats = accumulated_sats * 1000 - payout_checkpointed = await db.reset_routstr_fee( - session, paid_msats - ) - if not payout_checkpointed: - logger.warning("Routstr fee payout was already claimed") - continue + if accumulated_sats < ROUTSTR_FEE_DEFAULT_PAYOUT: + continue + paid_msats = accumulated_sats * 1000 + payout_checkpointed = await db.reset_routstr_fee(session, paid_msats) + if not payout_checkpointed: + logger.warning("Routstr fee payout was already claimed") + continue - try: - amount_received = await raw_send_to_lnurl( - wallet, - proofs, - ROUTSTR_LN_ADDRESS, - "sat", - amount=accumulated_sats, - ) - except Exception: - logger.critical( - "Routstr fee payout outcome is unknown; manual reconciliation required", - extra={"payout_in_progress_msats": paid_msats}, - exc_info=True, - ) - continue + wallet = await get_wallet(settings.primary_mint, "sat") + proofs = get_proofs_per_mint_and_unit( + wallet, settings.primary_mint, "sat", not_reserved=True + ) + try: + amount_received = await raw_send_to_lnurl( + wallet, + proofs, + ROUTSTR_LN_ADDRESS, + "sat", + amount=accumulated_sats, + ) + except Exception: + logger.critical( + "Routstr fee payout outcome is unknown; manual reconciliation required", + extra={"payout_in_progress_msats": paid_msats}, + exc_info=True, + ) + continue - payout_completed = await db.complete_routstr_fee_payout( - session, paid_msats - ) - if not payout_completed: - logger.critical( - "Routstr fee payout sent but checkpoint was not completed", - extra={"payout_in_progress_msats": paid_msats}, - ) - continue + async with db.create_session() as session: + payout_completed = await db.complete_routstr_fee_payout( + session, paid_msats + ) + if not payout_completed: + logger.critical( + "Routstr fee payout sent but checkpoint was not completed", + extra={"payout_in_progress_msats": paid_msats}, + ) + continue - logger.info( - "Routstr fee payout sent", - extra={ - "accumulated_sats": accumulated_sats, - "amount_received": amount_received, - }, - ) + logger.info( + "Routstr fee payout sent", + extra={ + "accumulated_sats": accumulated_sats, + "amount_received": amount_received, + }, + ) except Exception as e: logger.error( f"Error in Routstr fee payout: {type(e).__name__}", diff --git a/tests/integration/test_lightning_invoice_constraints.py b/tests/integration/test_lightning_invoice_constraints.py index a26b9083..8c8e15d9 100644 --- a/tests/integration/test_lightning_invoice_constraints.py +++ b/tests/integration/test_lightning_invoice_constraints.py @@ -9,10 +9,12 @@ Covers two things: from __future__ import annotations +import asyncio import time -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest +from sqlalchemy.ext.asyncio import AsyncEngine from sqlmodel.ext.asyncio.session import AsyncSession from routstr.core.db import ApiKey, LightningInvoice @@ -48,6 +50,7 @@ def mock_wallet_mint() -> object: # Persistence # --------------------------------------------------------------------------- + @pytest.mark.asyncio async def test_invoice_persists_balance_limit( integration_session: AsyncSession, @@ -92,6 +95,7 @@ async def test_invoice_persists_validity_date( # Propagation to ApiKey # --------------------------------------------------------------------------- + @pytest.mark.asyncio async def test_created_key_receives_balance_limit( integration_session: AsyncSession, @@ -141,6 +145,144 @@ async def test_created_key_receives_validity_date( assert stored_key.validity_date == expiry +@pytest.mark.asyncio +async def test_payment_check_releases_connection_during_mint_quote( + integration_engine: AsyncEngine, +) -> None: + invoice = _make_invoice(id="inv_slow_quote", status="pending", paid_at=None) + async with AsyncSession(integration_engine, expire_on_commit=False) as setup: + setup.add(invoice) + await setup.commit() + + async with AsyncSession(integration_engine, expire_on_commit=False) as session: + stored = await session.get(LightningInvoice, invoice.id) + assert stored is not None + + async def quote_status(*args: object, **kwargs: object) -> MagicMock: + assert integration_engine.pool.checkedout() == 0 # type: ignore[attr-defined] + return MagicMock(paid=False) + + wallet = MagicMock() + wallet.get_mint_quote = AsyncMock(side_effect=quote_status) + with patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)): + from routstr.lightning import check_invoice_payment + + await check_invoice_payment(stored, session) + + +@pytest.mark.asyncio +async def test_concurrent_payment_checks_mint_and_credit_invoice_once( + integration_engine: AsyncEngine, +) -> None: + invoice = _make_invoice(id="inv_concurrent", status="pending", paid_at=None) + async with AsyncSession(integration_engine, expire_on_commit=False) as setup: + setup.add(invoice) + await setup.commit() + + wallet = MagicMock() + wallet.get_mint_quote = AsyncMock(return_value=MagicMock(paid=True)) + + mint_calls = 0 + + async def single_use_mint(*args: object, **kwargs: object) -> list[object]: + # Real mints enforce single-use quotes: the second concurrent minter + # gets rejected at the mint, mirroring cashu quote semantics. + nonlocal mint_calls + mint_calls += 1 + call_number = mint_calls + await asyncio.sleep(0.05) + if call_number > 1: + raise Exception("quote already issued") + return [] + + wallet.mint = AsyncMock(side_effect=single_use_mint) + + async with ( + AsyncSession(integration_engine, expire_on_commit=False) as first, + AsyncSession(integration_engine, expire_on_commit=False) as second, + ): + first_invoice = await first.get(LightningInvoice, invoice.id) + second_invoice = await second.get(LightningInvoice, invoice.id) + assert first_invoice is not None + assert second_invoice is not None + + with patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)): + from routstr.lightning import check_invoice_payment + + await asyncio.gather( + check_invoice_payment(first_invoice, first), + check_invoice_payment(second_invoice, second), + ) + + async with AsyncSession(integration_engine, expire_on_commit=False) as verify: + stored_invoice = await verify.get(LightningInvoice, invoice.id) + assert stored_invoice is not None + assert stored_invoice.status == "paid" + assert stored_invoice.api_key_hash is not None + stored_key = await verify.get(ApiKey, stored_invoice.api_key_hash) + assert stored_key is not None + assert stored_key.balance == invoice.amount_sats * 1000 + + +@pytest.mark.asyncio +async def test_failed_mint_keeps_invoice_pending_for_retry( + integration_engine: AsyncEngine, +) -> None: + invoice = _make_invoice(id="inv_mint_failure", status="pending", paid_at=None) + async with AsyncSession(integration_engine, expire_on_commit=False) as setup: + setup.add(invoice) + await setup.commit() + + wallet = MagicMock() + wallet.get_mint_quote = AsyncMock(return_value=MagicMock(paid=True)) + wallet.mint = AsyncMock(side_effect=TimeoutError("mint unavailable")) + async with AsyncSession(integration_engine, expire_on_commit=False) as session: + stored = await session.get(LightningInvoice, invoice.id) + assert stored is not None + with patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)): + from routstr.lightning import check_invoice_payment + + await check_invoice_payment(stored, session) + + async with AsyncSession(integration_engine, expire_on_commit=False) as verify: + stored = await verify.get(LightningInvoice, invoice.id) + assert stored is not None + assert stored.status == "pending" + + +@pytest.mark.asyncio +async def test_post_mint_db_failure_keeps_invoice_pending_for_reconciliation( + integration_engine: AsyncEngine, +) -> None: + invoice = _make_invoice(id="inv_finalize_failure", status="pending", paid_at=None) + async with AsyncSession(integration_engine, expire_on_commit=False) as setup: + setup.add(invoice) + await setup.commit() + + wallet = MagicMock() + wallet.get_mint_quote = AsyncMock(return_value=MagicMock(paid=True)) + wallet.mint = AsyncMock(return_value=[]) + async with AsyncSession(integration_engine, expire_on_commit=False) as session: + stored = await session.get(LightningInvoice, invoice.id) + assert stored is not None + with ( + patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)), + patch( + "routstr.lightning._create_api_key_record", + AsyncMock(side_effect=RuntimeError("database unavailable")), + ), + ): + from routstr.lightning import check_invoice_payment + + await check_invoice_payment(stored, session) + + assert wallet.mint.await_count == 1 + async with AsyncSession(integration_engine, expire_on_commit=False) as verify: + stored = await verify.get(LightningInvoice, invoice.id) + assert stored is not None + assert stored.status == "pending" + + @pytest.mark.asyncio async def test_created_key_without_constraints_has_none_fields( integration_session: AsyncSession, diff --git a/tests/unit/test_db_pool_config.py b/tests/unit/test_db_pool_config.py new file mode 100644 index 00000000..ccfba575 --- /dev/null +++ b/tests/unit/test_db_pool_config.py @@ -0,0 +1,83 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +from routstr.core import db +from routstr.core.db import _engine_options + + +def test_engine_options_use_bounded_fail_fast_pool(monkeypatch) -> None: # type: ignore[no-untyped-def] + monkeypatch.delenv("DATABASE_POOL_SIZE", raising=False) + monkeypatch.delenv("DATABASE_MAX_OVERFLOW", raising=False) + monkeypatch.delenv("DATABASE_POOL_TIMEOUT", raising=False) + monkeypatch.delenv("DATABASE_POOL_RECYCLE", raising=False) + monkeypatch.delenv("DATABASE_POOL_PRE_PING", raising=False) + + options = _engine_options("postgresql+asyncpg://db/routstr") + + assert options == { + "pool_size": 5, + "max_overflow": 0, + "pool_timeout": 5.0, + "pool_recycle": 1800, + "pool_pre_ping": True, + } + + +def test_engine_options_are_operator_configurable(monkeypatch) -> None: # type: ignore[no-untyped-def] + monkeypatch.setenv("DATABASE_POOL_SIZE", "12") + monkeypatch.setenv("DATABASE_MAX_OVERFLOW", "3") + monkeypatch.setenv("DATABASE_POOL_TIMEOUT", "2.5") + monkeypatch.setenv("DATABASE_POOL_RECYCLE", "900") + monkeypatch.setenv("DATABASE_POOL_PRE_PING", "false") + + assert _engine_options("sqlite+aiosqlite:///keys.db") == { + "pool_size": 12, + "max_overflow": 3, + "pool_timeout": 2.5, + "pool_recycle": 900, + "pool_pre_ping": False, + } + + +@pytest.mark.parametrize( + ("name", "value"), + [ + ("DATABASE_POOL_SIZE", "0"), + ("DATABASE_MAX_OVERFLOW", "-1"), + ("DATABASE_POOL_TIMEOUT", "0"), + ("DATABASE_POOL_RECYCLE", "-1"), + ("DATABASE_POOL_PRE_PING", "maybe"), + ], +) +def test_engine_options_reject_invalid_values( + monkeypatch: pytest.MonkeyPatch, + name: str, + value: str, +) -> None: + monkeypatch.setenv(name, value) + with pytest.raises(ValueError): + _engine_options("postgresql+asyncpg://db/routstr") + + +def test_pool_observer_warns_when_connection_is_held_too_long( + monkeypatch: pytest.MonkeyPatch, +) -> None: + record = SimpleNamespace(info={}) + monotonic = MagicMock(side_effect=[100.0, 112.5]) + monkeypatch.setattr(db.time, "monotonic", monotonic) + monkeypatch.setattr(db, "_POOL_HOLD_WARN_SECONDS", 10.0) + + with patch.object(db.logger, "warning") as warning: + db._record_pool_checkout(None, record, None) + db._record_pool_checkin(None, record) + + warning.assert_called_once() + assert warning.call_args.kwargs["extra"]["held_seconds"] == 12.5 + + +def test_memory_sqlite_keeps_dialect_static_pool(monkeypatch) -> None: # type: ignore[no-untyped-def] + monkeypatch.delenv("DATABASE_POOL_PRE_PING", raising=False) + + assert _engine_options("sqlite+aiosqlite://") == {"pool_pre_ping": True} diff --git a/tests/unit/test_fee_payout_crash_safety.py b/tests/unit/test_fee_payout_crash_safety.py index 0939baa4..a1b18edb 100644 --- a/tests/unit/test_fee_payout_crash_safety.py +++ b/tests/unit/test_fee_payout_crash_safety.py @@ -1,6 +1,4 @@ import asyncio -from collections.abc import AsyncIterator -from contextlib import asynccontextmanager from types import SimpleNamespace from unittest.mock import AsyncMock, Mock, patch @@ -13,9 +11,19 @@ from routstr import wallet from routstr.core import db -@asynccontextmanager -async def _session_context(session: Mock) -> AsyncIterator[Mock]: - yield session +class _SessionContext: + def __init__(self, session: Mock) -> None: + self.session = session + + async def __aenter__(self) -> Mock: + return self.session + + async def __aexit__(self, *args: object) -> None: + return None + + +def _session_context(session: Mock) -> _SessionContext: + return _SessionContext(session) @pytest.mark.asyncio @@ -77,7 +85,9 @@ async def test_fee_payout_checkpoints_before_sending() -> None: "routstr.wallet.asyncio.sleep", AsyncMock(side_effect=[None, asyncio.CancelledError()]), ), - patch("routstr.wallet.db.create_session", return_value=_session_context(session)), + patch( + "routstr.wallet.db.create_session", return_value=_session_context(session) + ), patch("routstr.wallet.db.get_routstr_fee", AsyncMock(return_value=fee)), patch("routstr.wallet.db.reset_routstr_fee", side_effect=checkpoint), patch("routstr.wallet.db.complete_routstr_fee_payout", side_effect=complete), @@ -107,7 +117,9 @@ async def test_fee_payout_does_not_retry_an_unresolved_checkpoint() -> None: "routstr.wallet.asyncio.sleep", AsyncMock(side_effect=[None, asyncio.CancelledError()]), ), - patch("routstr.wallet.db.create_session", return_value=_session_context(session)), + patch( + "routstr.wallet.db.create_session", return_value=_session_context(session) + ), patch("routstr.wallet.db.get_routstr_fee", AsyncMock(return_value=fee)), patch("routstr.wallet.db.reset_routstr_fee", AsyncMock()) as checkpoint, patch("routstr.wallet.get_wallet", AsyncMock()) as get_wallet, @@ -141,7 +153,9 @@ async def test_fee_payout_keeps_checkpoint_when_send_outcome_is_unknown() -> Non "routstr.wallet.asyncio.sleep", AsyncMock(side_effect=[None, asyncio.CancelledError()]), ), - patch("routstr.wallet.db.create_session", return_value=_session_context(session)), + patch( + "routstr.wallet.db.create_session", return_value=_session_context(session) + ), patch("routstr.wallet.db.get_routstr_fee", AsyncMock(return_value=fee)), patch("routstr.wallet.db.reset_routstr_fee", AsyncMock(return_value=True)), patch("routstr.wallet.db.complete_routstr_fee_payout", complete), diff --git a/tests/unit/test_fetch_all_balances.py b/tests/unit/test_fetch_all_balances.py index 433e84d4..1d69e31e 100644 --- a/tests/unit/test_fetch_all_balances.py +++ b/tests/unit/test_fetch_all_balances.py @@ -1,7 +1,13 @@ +import asyncio +from collections.abc import AsyncGenerator from contextlib import asynccontextmanager +from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch import pytest +from sqlalchemy.ext.asyncio import create_async_engine +from sqlmodel import SQLModel +from sqlmodel.ext.asyncio.session import AsyncSession from routstr.wallet import fetch_all_balances @@ -26,8 +32,10 @@ def _patches( # type: ignore[no-untyped-def] AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_for_mint_and_unit", - AsyncMock(return_value=user_balance_msats), + "routstr.wallet.db.balances_by_mint_and_unit", + AsyncMock( + return_value={("http://primary:3338", "sat"): user_balance_msats} + ), ), patch("routstr.wallet.db.create_session", _fake_session), ] @@ -38,8 +46,9 @@ async def test_fetch_all_balances_falls_back_to_primary_mint() -> None: """With empty cashu_mints, balances are still fetched for primary_mint.""" from routstr.core.settings import settings - with patch.object(settings, "cashu_mints", []), patch.object( - settings, "primary_mint", "http://primary:3338" + with ( + patch.object(settings, "cashu_mints", []), + patch.object(settings, "primary_mint", "http://primary:3338"), ): for p in _patches(proof_amount=1000): p.start() @@ -54,13 +63,159 @@ async def test_fetch_all_balances_falls_back_to_primary_mint() -> None: assert total_wallet == 1000 +@pytest.mark.asyncio +async def test_fetch_all_balances_closes_db_session_before_concurrent_mint_io() -> None: + """Slow mint checks must never run while the balance DB session is open.""" + from routstr.core.settings import settings + + session_open = False + mint_calls = 0 + + @asynccontextmanager + async def tracked_session(): # type: ignore[no-untyped-def] + nonlocal session_open + session_open = True + try: + yield MagicMock() + finally: + session_open = False + + async def slow_filter(proofs, wallet): # type: ignore[no-untyped-def] + nonlocal mint_calls + assert session_open is False + mint_calls += 1 + await asyncio.sleep(0) + return proofs + + with ( + patch.object(settings, "cashu_mints", ["http://one:3338", "http://two:3338"]), + patch.object(settings, "primary_mint", "http://one:3338"), + patch("routstr.wallet.db.create_session", tracked_session), + patch( + "routstr.wallet.db.balances_by_mint_and_unit", + AsyncMock(return_value={}), + create=True, + ), + patch("routstr.wallet.get_wallet", AsyncMock(return_value=MagicMock())), + patch( + "routstr.wallet.get_proofs_per_mint_and_unit", + MagicMock(return_value=[MagicMock(amount=1)]), + ), + patch( + "routstr.wallet.slow_filter_spend_proofs", + AsyncMock(side_effect=slow_filter), + ), + ): + details, *_ = await fetch_all_balances(units=["sat", "msat"]) + + assert mint_calls == 4 + assert all("error" not in detail for detail in details) + + +@pytest.mark.asyncio +async def test_fetch_all_balances_bounds_parallel_mint_checks() -> None: + """A slow mint fleet cannot create an unbounded external-I/O fan-out.""" + from routstr.core.settings import settings + + active = 0 + peak = 0 + + async def slow_filter(proofs, wallet): # type: ignore[no-untyped-def] + nonlocal active, peak + active += 1 + peak = max(peak, active) + await asyncio.sleep(0.01) + active -= 1 + return proofs + + with ( + patch.object( + settings, + "cashu_mints", + [f"http://mint-{index}:3338" for index in range(8)], + ), + patch.object(settings, "primary_mint", ""), + patch.object(settings, "mint_operation_concurrency", 2), + patch("routstr.wallet.db.create_session", _fake_session), + patch( + "routstr.wallet.db.balances_by_mint_and_unit", + AsyncMock(return_value={}), + ), + patch("routstr.wallet.get_wallet", AsyncMock(return_value=MagicMock())), + patch( + "routstr.wallet.get_proofs_per_mint_and_unit", + MagicMock(return_value=[]), + ), + patch( + "routstr.wallet.slow_filter_spend_proofs", + AsyncMock(side_effect=slow_filter), + ), + ): + details, *_ = await fetch_all_balances(units=["sat"]) + + assert len(details) == 8 + assert peak == 2 + + +@pytest.mark.asyncio +async def test_slow_mints_do_not_exhaust_a_single_connection_pool( + tmp_path: Path, +) -> None: + """Concurrent slow balance refreshes release the sole DB connection promptly.""" + from routstr.core.settings import settings + + engine = create_async_engine( + f"sqlite+aiosqlite:///{tmp_path / 'pool-pressure.db'}", + pool_size=1, + max_overflow=0, + pool_timeout=0.2, + ) + async with engine.begin() as connection: + await connection.run_sync(SQLModel.metadata.create_all) + + @asynccontextmanager + async def single_pool_session() -> AsyncGenerator[AsyncSession, None]: + async with AsyncSession(engine, expire_on_commit=False) as session: + yield session + + async def slow_filter(proofs, wallet): # type: ignore[no-untyped-def] + await asyncio.sleep(0.3) + return proofs + + try: + with ( + patch.object(settings, "cashu_mints", ["http://slow:3338"]), + patch.object(settings, "primary_mint", "http://slow:3338"), + patch.object(settings, "mint_operation_concurrency", 1), + patch("routstr.wallet.db.create_session", single_pool_session), + patch("routstr.wallet.get_wallet", AsyncMock(return_value=MagicMock())), + patch( + "routstr.wallet.get_proofs_per_mint_and_unit", + MagicMock(return_value=[]), + ), + patch( + "routstr.wallet.slow_filter_spend_proofs", + AsyncMock(side_effect=slow_filter), + ), + ): + results = await asyncio.gather( + *(fetch_all_balances(units=["sat"]) for _ in range(6)) + ) + + assert all("error" not in result[0][0] for result in results) + assert engine.pool.checkedout() == 0 # type: ignore[attr-defined] + finally: + await engine.dispose() + + @pytest.mark.asyncio async def test_fetch_all_balances_reports_liability_when_wallet_is_empty() -> None: """An empty wallet must not hide outstanding user liabilities.""" from routstr.core.settings import settings - with patch.object(settings, "cashu_mints", []), patch.object( - settings, "primary_mint", "http://primary:3338" + with ( + patch.object(settings, "cashu_mints", []), + patch.object(settings, "primary_mint", "http://primary:3338"), ): for p in _patches(proof_amount=0, user_balance_msats=5000): p.start() @@ -84,9 +239,10 @@ async def test_fetch_all_balances_no_duplicate_primary_mint() -> None: """primary_mint already in cashu_mints is not inspected twice.""" from routstr.core.settings import settings - with patch.object( - settings, "cashu_mints", ["http://primary:3338"] - ), patch.object(settings, "primary_mint", "http://primary:3338"): + with ( + patch.object(settings, "cashu_mints", ["http://primary:3338"]), + patch.object(settings, "primary_mint", "http://primary:3338"), + ): for p in _patches(proof_amount=1000): p.start() try: diff --git a/tests/unit/test_periodic_payout.py b/tests/unit/test_periodic_payout.py index 54ebadb4..77d0a7cf 100644 --- a/tests/unit/test_periodic_payout.py +++ b/tests/unit/test_periodic_payout.py @@ -59,23 +59,28 @@ async def test_periodic_payout_includes_primary_mint_not_in_cashu_mints() -> Non get_wallet = AsyncMock(return_value=MagicMock()) raw_send = AsyncMock(return_value=1000) - with patch.object(settings, "cashu_mints", []), patch.object( - settings, "primary_mint", "http://primary:3338" - ), patch.object(settings, "receive_ln_address", "owner@ln.tld"), patch.object( - settings, "payout_interval_seconds", _INTERVAL - ), patch.object(settings, "min_payout_sat", 10), patch( - "routstr.wallet.asyncio.sleep", _one_cycle_sleep() - ), patch("routstr.wallet.db.create_session", _fake_session), patch( - "routstr.wallet.get_wallet", get_wallet - ), patch( - "routstr.wallet.get_proofs_per_mint_and_unit", - MagicMock(return_value=[MagicMock(amount=100_000)]), - ), patch( - "routstr.wallet.slow_filter_spend_proofs", - AsyncMock(side_effect=lambda proofs, wallet: proofs), - ), patch( - "routstr.wallet.db.balances_for_mint_and_unit", AsyncMock(return_value=0) - ), patch("routstr.wallet.raw_send_to_lnurl", raw_send): + with ( + patch.object(settings, "cashu_mints", []), + patch.object(settings, "primary_mint", "http://primary:3338"), + patch.object(settings, "receive_ln_address", "owner@ln.tld"), + patch.object(settings, "payout_interval_seconds", _INTERVAL), + patch.object(settings, "min_payout_sat", 10), + patch("routstr.wallet.asyncio.sleep", _one_cycle_sleep()), + patch("routstr.wallet.db.create_session", _fake_session), + patch("routstr.wallet.get_wallet", get_wallet), + patch( + "routstr.wallet.get_proofs_per_mint_and_unit", + MagicMock(return_value=[MagicMock(amount=100_000)]), + ), + patch( + "routstr.wallet.slow_filter_spend_proofs", + AsyncMock(side_effect=lambda proofs, wallet: proofs), + ), + patch( + "routstr.wallet.db.balances_by_mint_and_unit", AsyncMock(return_value={}) + ), + patch("routstr.wallet.raw_send_to_lnurl", raw_send), + ): with pytest.raises(_LoopBreak): await periodic_payout() @@ -84,6 +89,58 @@ async def test_periodic_payout_includes_primary_mint_not_in_cashu_mints() -> Non assert raw_send.await_count >= 1 +@pytest.mark.asyncio +async def test_periodic_payout_releases_session_before_slow_mint_send() -> None: + """The DB connection is returned before the external LNURL call starts.""" + from routstr.core.settings import settings + + session_open = False + sends_completed = 0 + + @asynccontextmanager + async def tracked_session(): # type: ignore[no-untyped-def] + nonlocal session_open + session_open = True + try: + yield MagicMock() + finally: + session_open = False + + async def raw_send(*args: object, **kwargs: object) -> int: + nonlocal sends_completed + assert session_open is False + sends_completed += 1 + return 1000 + + with ( + patch.object(settings, "cashu_mints", []), + patch.object(settings, "primary_mint", "http://primary:3338"), + patch.object(settings, "receive_ln_address", "owner@ln.tld"), + patch.object(settings, "payout_interval_seconds", _INTERVAL), + patch.object(settings, "min_payout_sat", 10), + patch("routstr.wallet.asyncio.sleep", _one_cycle_sleep()), + patch("routstr.wallet.db.create_session", tracked_session), + patch("routstr.wallet.get_wallet", AsyncMock(return_value=MagicMock())), + patch( + "routstr.wallet.get_proofs_per_mint_and_unit", + MagicMock(return_value=[MagicMock(amount=100_000)]), + ), + patch( + "routstr.wallet.slow_filter_spend_proofs", + AsyncMock(side_effect=lambda proofs, wallet: proofs), + ), + patch( + "routstr.wallet.db.balances_by_mint_and_unit", + AsyncMock(return_value={}), + ), + patch("routstr.wallet.raw_send_to_lnurl", AsyncMock(side_effect=raw_send)), + ): + with pytest.raises(_LoopBreak): + await periodic_payout() + + assert sends_completed == 2 + + @pytest.mark.asyncio async def test_periodic_payout_isolates_failing_mint() -> None: """A failing mint does not prevent payout for the other mints.""" @@ -97,23 +154,28 @@ async def test_periodic_payout_isolates_failing_mint() -> None: get_wallet = AsyncMock(side_effect=_get_wallet) raw_send = AsyncMock(return_value=1000) - with patch.object( - settings, "cashu_mints", ["http://bad:3338", "http://good:3338"] - ), patch.object(settings, "primary_mint", "http://good:3338"), patch.object( - settings, "receive_ln_address", "owner@ln.tld" - ), patch.object(settings, "payout_interval_seconds", _INTERVAL), patch.object( - settings, "min_payout_sat", 10 - ), patch("routstr.wallet.asyncio.sleep", _one_cycle_sleep()), patch( - "routstr.wallet.db.create_session", _fake_session - ), patch("routstr.wallet.get_wallet", get_wallet), patch( - "routstr.wallet.get_proofs_per_mint_and_unit", - MagicMock(return_value=[MagicMock(amount=100_000)]), - ), patch( - "routstr.wallet.slow_filter_spend_proofs", - AsyncMock(side_effect=lambda proofs, wallet: proofs), - ), patch( - "routstr.wallet.db.balances_for_mint_and_unit", AsyncMock(return_value=0) - ), patch("routstr.wallet.raw_send_to_lnurl", raw_send): + with ( + patch.object(settings, "cashu_mints", ["http://bad:3338", "http://good:3338"]), + patch.object(settings, "primary_mint", "http://good:3338"), + patch.object(settings, "receive_ln_address", "owner@ln.tld"), + patch.object(settings, "payout_interval_seconds", _INTERVAL), + patch.object(settings, "min_payout_sat", 10), + patch("routstr.wallet.asyncio.sleep", _one_cycle_sleep()), + patch("routstr.wallet.db.create_session", _fake_session), + patch("routstr.wallet.get_wallet", get_wallet), + patch( + "routstr.wallet.get_proofs_per_mint_and_unit", + MagicMock(return_value=[MagicMock(amount=100_000)]), + ), + patch( + "routstr.wallet.slow_filter_spend_proofs", + AsyncMock(side_effect=lambda proofs, wallet: proofs), + ), + patch( + "routstr.wallet.db.balances_by_mint_and_unit", AsyncMock(return_value={}) + ), + patch("routstr.wallet.raw_send_to_lnurl", raw_send), + ): with pytest.raises(_LoopBreak): await periodic_payout() @@ -134,15 +196,15 @@ async def test_periodic_payout_handles_session_creation_failure() -> None: create_session = MagicMock(side_effect=RuntimeError("db unavailable")) logger = MagicMock() - with patch.object(settings, "cashu_mints", ["http://mint:3338"]), patch.object( - settings, "primary_mint", "http://mint:3338" - ), patch.object(settings, "receive_ln_address", "owner@ln.tld"), patch.object( - settings, "payout_interval_seconds", _INTERVAL - ), patch( - "routstr.wallet.asyncio.sleep", _one_cycle_sleep() - ), patch( - "routstr.wallet.db.create_session", create_session - ), patch("routstr.wallet.logger", logger): + with ( + patch.object(settings, "cashu_mints", ["http://mint:3338"]), + patch.object(settings, "primary_mint", "http://mint:3338"), + patch.object(settings, "receive_ln_address", "owner@ln.tld"), + patch.object(settings, "payout_interval_seconds", _INTERVAL), + patch("routstr.wallet.asyncio.sleep", _one_cycle_sleep()), + patch("routstr.wallet.db.create_session", create_session), + patch("routstr.wallet.logger", logger), + ): with pytest.raises(_LoopBreak): await periodic_payout() diff --git a/tests/unit/test_proxy_session_lifecycle.py b/tests/unit/test_proxy_session_lifecycle.py new file mode 100644 index 00000000..5d0416d5 --- /dev/null +++ b/tests/unit/test_proxy_session_lifecycle.py @@ -0,0 +1,43 @@ +from collections.abc import AsyncIterator +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from fastapi.responses import StreamingResponse + +from routstr import proxy as proxy_module + + +@pytest.mark.asyncio +async def test_proxy_closes_request_session_before_returning_response() -> None: + """Route completion must release DB resources before response delivery.""" + request = MagicMock() + request.method = "GET" + request.headers = {"accept": "application/json"} + request.url.path = "/not-an-api-route" + request.state.request_id = "test-request" + session = AsyncMock() + + response = await proxy_module.proxy(request, "not-an-api-route", session=session) + + assert response.status_code == 404 + session.close.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_proxy_session_is_closed_before_first_stream_chunk() -> None: + request = MagicMock() + session = AsyncMock() + + async def stream() -> AsyncIterator[bytes]: + session.close.assert_awaited_once() + yield b"chunk" + + upstream_response = StreamingResponse(stream()) + with patch("routstr.proxy._proxy", AsyncMock(return_value=upstream_response)): + response = await proxy_module.proxy( + request, "v1/chat/completions", session=session + ) + + assert isinstance(response, StreamingResponse) + chunks = [chunk async for chunk in response.body_iterator] + assert chunks == [b"chunk"] diff --git a/tests/unit/test_refund_sweep.py b/tests/unit/test_refund_sweep.py index e82c2111..64b36efc 100644 --- a/tests/unit/test_refund_sweep.py +++ b/tests/unit/test_refund_sweep.py @@ -1,4 +1,5 @@ from collections.abc import AsyncIterator +from contextlib import asynccontextmanager from pathlib import Path from unittest.mock import AsyncMock, patch @@ -39,6 +40,43 @@ async def _load( return {row.token: row for row in result.all()} +@pytest.mark.asyncio +async def test_refund_sweep_releases_db_session_during_token_redemption( + session_factory: async_sessionmaker[AsyncSession], +) -> None: + await _insert( + session_factory, + CashuTransaction( + token="eligible", amount=1, unit="sat", type="out", created_at=800 + ), + ) + session_open = False + + @asynccontextmanager + async def tracked_session() -> AsyncIterator[AsyncSession]: + nonlocal session_open + async with session_factory() as session: + session_open = True + try: + yield session + finally: + session_open = False + + async def receive_token(token: str) -> None: + assert token == "eligible" + assert session_open is False + + with ( + patch("routstr.wallet.db.create_session", tracked_session), + patch("routstr.wallet.settings.refund_sweep_ttl_seconds", 100), + patch("routstr.wallet.time.time", return_value=1000), + patch("routstr.wallet.recieve_token", AsyncMock(side_effect=receive_token)), + ): + await refund_sweep_once() + + assert (await _load(session_factory))["eligible"].swept is True + + @pytest.mark.asyncio async def test_refund_sweep_only_processes_expired_eligible_outgoing_tokens( session_factory: async_sessionmaker[AsyncSession], From 1b09639265de7b9957f76e7bd4553afb41152505 Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Fri, 24 Jul 2026 21:44:11 +0200 Subject: [PATCH 2/8] docs: document DB pool and mint concurrency env vars in .env.example --- .env.example | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.env.example b/.env.example index 8ff04b35..8c3e2657 100644 --- a/.env.example +++ b/.env.example @@ -22,6 +22,13 @@ ROUTSTR_SECRET_KEY= # Database # DATABASE_URL=sqlite+aiosqlite:///keys.db +# Keep total pool capacity across all workers below the database connection limit. +# DATABASE_POOL_SIZE=5 +# DATABASE_MAX_OVERFLOW=0 +# DATABASE_POOL_TIMEOUT=5 +# DATABASE_POOL_RECYCLE=1800 +# DATABASE_POOL_PRE_PING=true +# DATABASE_POOL_HOLD_WARN_SECONDS=10 # Node Information # NAME=My Routstr Node @@ -31,6 +38,7 @@ ROUTSTR_SECRET_KEY= # RELAYS="wss://relay.damus.io,wss://relay.nostr.band,wss://eden.nostr.land,wss://relay.routstr.com" # ENABLE_ANALYTICS_SHARING=true # CASHU_MINTS="https://mint.minibits.cash/Bitcoin,https://mint.cubabitcoin.org,https://ecashmint.otrta.me" +# MINT_OPERATION_CONCURRENCY=4 # RECEIVE_LN_ADDRESS= # Custom Pricing Configuration From 2410a4a6ce0729a6e9f0bb471cb17a51adb4a7cf Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Fri, 24 Jul 2026 22:14:57 +0200 Subject: [PATCH 3/8] fix: address review findings on payout liability staleness, sweep races, and cancellation safety - periodic_payout: fetch liability per mint/unit right before computing available balance, so a concurrent top-up can only shrink the payout - refund sweep: atomically claim each refund before redeeming; release the claim on failure so retries still happen and concurrent sweeps cannot misreport a sweep as client-collected - check_invoice_payment: catch BaseException so task cancellation after a successful mint still emits the reconciliation alert - tests: DB-guard race test where both mints succeed (exactly one credit); pool_size=1 test proving the fee payout releases its connection during the external send --- routstr/lightning.py | 11 +++- routstr/wallet.py | 50 ++++++++++++----- .../test_lightning_invoice_constraints.py | 55 +++++++++++++++++++ tests/unit/test_fee_payout_crash_safety.py | 49 +++++++++++++++++ tests/unit/test_periodic_payout.py | 29 +++++++--- 5 files changed, 168 insertions(+), 26 deletions(-) diff --git a/routstr/lightning.py b/routstr/lightning.py index 30aa7918..5bba7593 100644 --- a/routstr/lightning.py +++ b/routstr/lightning.py @@ -281,13 +281,20 @@ async def check_invoice_payment( else None, }, ) - except Exception as e: - await session.rollback() + except BaseException as e: + # BaseException so task cancellation (e.g. client disconnect) after a + # successful mint still triggers the reconciliation alert. + try: + await asyncio.shield(session.rollback()) + except Exception: + logger.warning("Rollback failed during invoice check cleanup") if minted: logger.critical( "Invoice mint succeeded but DB finalization failed; reconciliation required", extra={"invoice_id": invoice_id, "purpose": invoice_purpose}, ) + if not isinstance(e, Exception): + raise logger.error(f"Failed to check invoice payment: {e}") diff --git a/routstr/wallet.py b/routstr/wallet.py index 24b11fe9..d61bbd4c 100644 --- a/routstr/wallet.py +++ b/routstr/wallet.py @@ -931,9 +931,6 @@ async def periodic_payout() -> None: if settings.primary_mint and settings.primary_mint not in mint_urls: mint_urls.append(settings.primary_mint) - async with db.create_session() as session: - user_balances = await db.balances_by_mint_and_unit(session) - for mint_url in mint_urls: for unit in ["sat", "msat"]: # Isolate failures per mint/unit so one slow or failing @@ -945,7 +942,14 @@ async def periodic_payout() -> None: ) proofs = await slow_filter_spend_proofs(proofs, wallet) await asyncio.sleep(5) - user_balance = user_balances.get((mint_url, unit), 0) + # Fetch the liability AFTER the proofs snapshot and the + # settle delay: a concurrent top-up then only inflates + # the liability, shrinking the payout — never sending + # customer-backed funds as profit. + async with db.create_session() as session: + user_balance = await db.balances_for_mint_and_unit( + session, mint_url, unit + ) if unit == "sat": user_balance = user_balance // 1000 proofs_balance = sum(proof.amount for proof in proofs) @@ -1001,18 +1005,24 @@ async def _refund_sweep_once(cutoff: int) -> None: refunds = results.all() for refund in refunds: + # Claim the refund atomically before redeeming so a concurrent sweep + # cannot redeem the same token and misreport it as client-collected. + async with db.create_session() as session: + claim = await session.exec( # type: ignore[call-overload] + update(db.CashuTransaction) + .where( + col(db.CashuTransaction.id) == refund.id, + col(db.CashuTransaction.swept) == False, # noqa: E712 + col(db.CashuTransaction.collected) == False, # noqa: E712 + ) + .values(swept=True) + ) + await session.commit() + if claim.rowcount != 1: + continue + try: await recieve_token(refund.token) - async with db.create_session() as session: - await session.exec( # type: ignore[call-overload] - update(db.CashuTransaction) - .where( - col(db.CashuTransaction.id) == refund.id, - col(db.CashuTransaction.swept) == False, # noqa: E712 - ) - .values(swept=True) - ) - await session.commit() logger.info( "Swept uncollected refund", extra={ @@ -1024,11 +1034,13 @@ async def _refund_sweep_once(cutoff: int) -> None: except Exception as e: error_msg = str(e).lower() if "already spent" in error_msg: + # We held the claim, so nobody else swept it: the client + # really collected the token. async with db.create_session() as session: await session.exec( # type: ignore[call-overload] update(db.CashuTransaction) .where(col(db.CashuTransaction.id) == refund.id) - .values(collected=True) + .values(collected=True, swept=False) ) await session.commit() logger.info( @@ -1036,6 +1048,14 @@ async def _refund_sweep_once(cutoff: int) -> None: extra={"id": refund.id}, ) else: + # Release the claim so the next sweep retries this refund. + async with db.create_session() as session: + await session.exec( # type: ignore[call-overload] + update(db.CashuTransaction) + .where(col(db.CashuTransaction.id) == refund.id) + .values(swept=False) + ) + await session.commit() logger.warning( "Failed to sweep refund", extra={ diff --git a/tests/integration/test_lightning_invoice_constraints.py b/tests/integration/test_lightning_invoice_constraints.py index 8c8e15d9..a7fa1384 100644 --- a/tests/integration/test_lightning_invoice_constraints.py +++ b/tests/integration/test_lightning_invoice_constraints.py @@ -299,3 +299,58 @@ async def test_created_key_without_constraints_has_none_fields( assert stored_key.balance_limit is None assert stored_key.balance_limit_reset is None assert stored_key.validity_date is None + + +@pytest.mark.asyncio +async def test_db_guard_credits_once_when_both_mints_succeed( + integration_engine: AsyncEngine, +) -> None: + """Even if the mint fails to enforce single-use quotes and both racers + mint successfully, the conditional status update must credit exactly once.""" + key = ApiKey(hashed_key="race-key", balance=1_000) + invoice = _make_invoice( + id="inv_db_guard", + status="pending", + paid_at=None, + purpose="topup", + api_key_hash="race-key", + ) + async with AsyncSession(integration_engine, expire_on_commit=False) as setup: + setup.add(key) + setup.add(invoice) + await setup.commit() + + wallet = MagicMock() + wallet.get_mint_quote = AsyncMock(return_value=MagicMock(paid=True)) + + async def always_succeeding_mint(*args: object, **kwargs: object) -> list[object]: + await asyncio.sleep(0.05) + return [] + + wallet.mint = AsyncMock(side_effect=always_succeeding_mint) + + async with ( + AsyncSession(integration_engine, expire_on_commit=False) as first, + AsyncSession(integration_engine, expire_on_commit=False) as second, + ): + first_invoice = await first.get(LightningInvoice, invoice.id) + second_invoice = await second.get(LightningInvoice, invoice.id) + assert first_invoice is not None + assert second_invoice is not None + + with patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)): + from routstr.lightning import check_invoice_payment + + await asyncio.gather( + check_invoice_payment(first_invoice, first), + check_invoice_payment(second_invoice, second), + ) + + assert wallet.mint.await_count == 2 + async with AsyncSession(integration_engine, expire_on_commit=False) as verify: + stored_invoice = await verify.get(LightningInvoice, invoice.id) + assert stored_invoice is not None + assert stored_invoice.status == "paid" + stored_key = await verify.get(ApiKey, "race-key") + assert stored_key is not None + assert stored_key.balance == 1_000 + invoice.amount_sats * 1000 diff --git a/tests/unit/test_fee_payout_crash_safety.py b/tests/unit/test_fee_payout_crash_safety.py index a1b18edb..19b95ec3 100644 --- a/tests/unit/test_fee_payout_crash_safety.py +++ b/tests/unit/test_fee_payout_crash_safety.py @@ -1,4 +1,6 @@ import asyncio +from collections.abc import AsyncGenerator +from contextlib import asynccontextmanager from types import SimpleNamespace from unittest.mock import AsyncMock, Mock, patch @@ -172,3 +174,50 @@ async def test_fee_payout_keeps_checkpoint_when_send_outcome_is_unknown() -> Non complete.assert_not_awaited() critical.assert_called_once() + + +@pytest.mark.asyncio +async def test_fee_payout_releases_db_connection_during_send(tmp_path: object) -> None: + """With pool_size=1, the payout must not hold a connection while the + external LNURL send is in flight, or the completion step would starve.""" + engine = create_async_engine( + f"sqlite+aiosqlite:///{tmp_path}/payout.db", pool_size=1, max_overflow=0 + ) + async with engine.begin() as connection: + await connection.run_sync(SQLModel.metadata.create_all) + async with AsyncSession(engine) as session: + session.add(db.RoutstrFee(id=1, accumulated_msats=5_000_000)) + await session.commit() + + @asynccontextmanager + async def create_session() -> AsyncGenerator[AsyncSession, None]: + async with AsyncSession(engine, expire_on_commit=False) as session: + yield session + + async def send(*_args: object, **_kwargs: object) -> int: + assert engine.pool.checkedout() == 0 # type: ignore[attr-defined] + return 5 + + try: + with ( + patch("routstr.auth.ROUTSTR_FEE_DEFAULT_PAYOUT", 1), + patch("routstr.auth.ROUTSTR_FEE_PAYOUT_INTERVAL_SECONDS", 1), + patch("routstr.auth.ROUTSTR_LN_ADDRESS", "fees@example.com"), + patch( + "routstr.wallet.asyncio.sleep", + AsyncMock(side_effect=[None, asyncio.CancelledError()]), + ), + patch("routstr.wallet.db.create_session", create_session), + patch("routstr.wallet.get_wallet", AsyncMock(return_value=Mock())), + patch("routstr.wallet.get_proofs_per_mint_and_unit", return_value=[]), + patch("routstr.wallet.raw_send_to_lnurl", side_effect=send), + ): + with pytest.raises(asyncio.CancelledError): + await wallet.periodic_routstr_fee_payout() + + async with AsyncSession(engine) as session: + fee = await db.get_routstr_fee(session) + assert fee.payout_in_progress_msats == 0 + assert fee.total_paid_msats == 5_000_000 + finally: + await engine.dispose() diff --git a/tests/unit/test_periodic_payout.py b/tests/unit/test_periodic_payout.py index 77d0a7cf..c1768dc7 100644 --- a/tests/unit/test_periodic_payout.py +++ b/tests/unit/test_periodic_payout.py @@ -77,7 +77,7 @@ async def test_periodic_payout_includes_primary_mint_not_in_cashu_mints() -> Non AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_by_mint_and_unit", AsyncMock(return_value={}) + "routstr.wallet.db.balances_for_mint_and_unit", AsyncMock(return_value=0) ), patch("routstr.wallet.raw_send_to_lnurl", raw_send), ): @@ -130,8 +130,8 @@ async def test_periodic_payout_releases_session_before_slow_mint_send() -> None: AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_by_mint_and_unit", - AsyncMock(return_value={}), + "routstr.wallet.db.balances_for_mint_and_unit", + AsyncMock(return_value=0), ), patch("routstr.wallet.raw_send_to_lnurl", AsyncMock(side_effect=raw_send)), ): @@ -172,7 +172,7 @@ async def test_periodic_payout_isolates_failing_mint() -> None: AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_by_mint_and_unit", AsyncMock(return_value={}) + "routstr.wallet.db.balances_for_mint_and_unit", AsyncMock(return_value=0) ), patch("routstr.wallet.raw_send_to_lnurl", raw_send), ): @@ -190,7 +190,7 @@ async def test_periodic_payout_isolates_failing_mint() -> None: @pytest.mark.asyncio async def test_periodic_payout_handles_session_creation_failure() -> None: - """A db.create_session failure is logged and the payout loop continues.""" + """A db.create_session failure is logged per mint/unit and the loop continues.""" from routstr.core.settings import settings create_session = MagicMock(side_effect=RuntimeError("db unavailable")) @@ -203,14 +203,25 @@ async def test_periodic_payout_handles_session_creation_failure() -> None: patch.object(settings, "payout_interval_seconds", _INTERVAL), patch("routstr.wallet.asyncio.sleep", _one_cycle_sleep()), patch("routstr.wallet.db.create_session", create_session), + patch("routstr.wallet.get_wallet", AsyncMock(return_value=MagicMock())), + patch( + "routstr.wallet.get_proofs_per_mint_and_unit", + MagicMock(return_value=[MagicMock(amount=100_000)]), + ), + patch( + "routstr.wallet.slow_filter_spend_proofs", + AsyncMock(side_effect=lambda proofs, wallet: proofs), + ), patch("routstr.wallet.logger", logger), ): with pytest.raises(_LoopBreak): await periodic_payout() - create_session.assert_called_once() - logger.error.assert_called_once() + # The liability session is opened per mint/unit (sat + msat), and each + # failure is isolated to its own iteration rather than aborting the cycle. + assert create_session.call_count == 2 + assert logger.error.call_count == 2 message = logger.error.call_args.args[0] extra = logger.error.call_args.kwargs["extra"] - assert message == "Error in periodic payout cycle: RuntimeError" - assert extra == {"error": "db unavailable"} + assert message == "Error sending payout: RuntimeError" + assert extra["error"] == "db unavailable" From 7b0ade398741aada5f58f9712892a9fe5a21e36b Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Fri, 24 Jul 2026 22:26:25 +0200 Subject: [PATCH 4/8] fix: stop AsyncSession pool exhaustion in balance reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetch_all_balances shared a single AsyncSession across the tasks it ran with asyncio.gather. AsyncSession is not safe for concurrent use: the overlapping queries raised "concurrent operations are not permitted" and left connections wedged until the QueuePool was exhausted, after which admin/API endpoints returned 500/502 until a container restart. Read all outstanding user liabilities up front in one short-lived session with a single grouped query (balances_by_mint_and_unit), then run the per-mint balance checks concurrently with no session in scope. A failure reading liabilities now degrades gracefully — the page still reports the known wallet custody and blanks only the unknowable user/owner split, tagging each mint with the error — instead of 500-ing the whole page. periodic_payout reads each liability fresh, immediately before the payout decision, rather than from a single pre-loop snapshot: the per-mint round trip is slow, and a user top-up during the cycle would otherwise let a later mint/unit act on a stale-low liability and over-send funds owed to users (related to the payout-safety concern in issue #611). Co-Authored-By: Claude Opus 4.8 --- routstr/core/db.py | 35 ++- routstr/wallet.py | 92 +++++--- tests/unit/test_balances_by_mint_and_unit.py | 100 +++++++++ tests/unit/test_fetch_all_balances.py | 219 ++++++++++++++++++- tests/unit/test_periodic_payout.py | 61 +++++- 5 files changed, 464 insertions(+), 43 deletions(-) create mode 100644 tests/unit/test_balances_by_mint_and_unit.py diff --git a/routstr/core/db.py b/routstr/core/db.py index 16cfedfd..ab263be4 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -717,14 +717,37 @@ async def complete_routstr_fee_payout( return result.rowcount == 1 -async def balances_for_mint_and_unit( - db_session: AsyncSession, mint_url: str, unit: str -) -> int: - query = select(func.sum(ApiKey.balance)).where( - ApiKey.refund_mint_url == mint_url, ApiKey.refund_currency == unit +async def balances_by_mint_and_unit( + db_session: AsyncSession, mint_urls: list[str], units: list[str] +) -> dict[tuple[str, str], int]: + """Sum outstanding user balances (msats) grouped by (mint_url, unit). + + One query for every requested mint/unit pair. Pairs with no matching rows + are absent from the mapping — callers should default those to 0. + """ + if not mint_urls or not units: + return {} + query = ( + select( + ApiKey.refund_mint_url, + ApiKey.refund_currency, + func.sum(ApiKey.balance), + ) + .where( + col(ApiKey.refund_mint_url).in_(mint_urls), + col(ApiKey.refund_currency).in_(units), + ) + .group_by(col(ApiKey.refund_mint_url), col(ApiKey.refund_currency)) ) result = await db_session.exec(query) - return result.one() or 0 + # IN(...) filters out NULL mint_url/currency at the SQL level, so the group + # keys are never None at runtime; the guard also narrows the nullable + # columns from ``str | None`` to ``str`` for the typed return. + return { + (mint_url, unit): total or 0 + for mint_url, unit, total in result.all() + if mint_url is not None and unit is not None + } async def init_db() -> None: diff --git a/routstr/wallet.py b/routstr/wallet.py index dd92d913..99b77e5c 100644 --- a/routstr/wallet.py +++ b/routstr/wallet.py @@ -837,7 +837,7 @@ async def fetch_all_balances( units = ["sat", "msat"] async def fetch_balance( - session: db.AsyncSession, mint_url: str, unit: str + mint_url: str, unit: str, user_balance: int ) -> BalanceDetail: try: wallet = await get_wallet(mint_url, unit) @@ -845,7 +845,6 @@ async def fetch_all_balances( wallet, mint_url, unit, not_reserved=True ) proofs = await slow_filter_spend_proofs(proofs, wallet) - user_balance = await db.balances_for_mint_and_unit(session, mint_url, unit) if unit == "sat": user_balance = user_balance // 1000 proofs_balance = sum(proof.amount for proof in proofs) @@ -878,40 +877,66 @@ async def fetch_all_balances( if settings.primary_mint and settings.primary_mint not in mint_urls: mint_urls.append(settings.primary_mint) - # Create tasks for all mint/unit combinations - async with db.create_session() as session: - tasks = [ - fetch_balance(session, mint_url, unit) - for mint_url in mint_urls - for unit in units - ] + # Read all outstanding user liabilities up front in one short-lived DB + # session and a single grouped query. AsyncSession is not safe for + # concurrent use, so the session must NOT be shared across the gathered + # mint checks below (that raises "concurrent operations are not permitted" + # and can wedge connections until the pool is exhausted). A DB failure here + # must still degrade gracefully rather than 500 the whole balances page. + user_balances: dict[tuple[str, str], int] = {} + liabilities_error: str | None = None + try: + async with db.create_session() as session: + user_balances = await db.balances_by_mint_and_unit( + session, mint_urls, units + ) + except Exception as e: + logger.error(f"Error reading user balances: {e}") + liabilities_error = str(e) - # Run all tasks concurrently - balance_details = list(await asyncio.gather(*tasks)) + # Run the per-mint balance checks concurrently — no DB session involved. + tasks = [ + fetch_balance(mint_url, unit, user_balances.get((mint_url, unit), 0)) + for mint_url in mint_urls + for unit in units + ] + balance_details = list(await asyncio.gather(*tasks)) - # Calculate totals + # Compute totals BEFORE tagging any liability-read failure. A failed + # liability read does not invalidate custody, so the known wallet balance + # must still be summed; only the per-user split is unknowable. A per-mint + # fetch failure (``error`` already set inside fetch_balance) means custody + # for that mint is genuinely unknown, so those details are skipped. total_wallet_balance_sats = 0 total_user_balance_sats = 0 - for detail in balance_details: - if not detail.get("error"): - # Convert to sats for total calculation - unit = detail["unit"] - proofs_balance_sats = ( - detail["wallet_balance"] - if unit == "sat" - else detail["wallet_balance"] // 1000 - ) - user_balance_sats = ( + if detail.get("error"): + continue + unit = detail["unit"] + total_wallet_balance_sats += ( + detail["wallet_balance"] + if unit == "sat" + else detail["wallet_balance"] // 1000 + ) + if liabilities_error is None: + total_user_balance_sats += ( detail["user_balance"] if unit == "sat" else detail["user_balance"] // 1000 ) - total_wallet_balance_sats += proofs_balance_sats - total_user_balance_sats += user_balance_sats - - owner_balance = total_wallet_balance_sats - total_user_balance_sats + if liabilities_error is None: + owner_balance = total_wallet_balance_sats - total_user_balance_sats + else: + # Liabilities are unknown: report custody truthfully but never claim any + # of it as owner profit (owner = wallet - unknown liabilities). Surface + # the failure on each detail without discarding a more specific per-mint + # error, and blank the unknowable per-user/owner split. + owner_balance = 0 + for detail in balance_details: + detail["user_balance"] = 0 + detail["owner_balance"] = 0 + detail.setdefault("error", liabilities_error) return ( balance_details, @@ -935,9 +960,10 @@ async def periodic_payout() -> None: if settings.primary_mint and settings.primary_mint not in mint_urls: mint_urls.append(settings.primary_mint) + units = ["sat", "msat"] async with db.create_session() as session: for mint_url in mint_urls: - for unit in ["sat", "msat"]: + for unit in units: # Isolate failures per mint/unit so one slow or failing # mint does not abort payout for every other mint/unit. try: @@ -947,9 +973,17 @@ async def periodic_payout() -> None: ) proofs = await slow_filter_spend_proofs(proofs, wallet) await asyncio.sleep(5) - user_balance = await db.balances_for_mint_and_unit( - session, mint_url, unit + # Read the liability fresh, right before deciding the + # payout. The mint round-trip above is slow; reading + # it once before the loop would let a user top-up + # during the cycle go unseen, so a later (mint, unit) + # would act on a stale-low liability and over-send + # funds owed to users. The loop is sequential, so + # reusing this session per iteration is safe. + balances = await db.balances_by_mint_and_unit( + session, [mint_url], [unit] ) + user_balance = balances.get((mint_url, unit), 0) if unit == "sat": user_balance = user_balance // 1000 proofs_balance = sum(proof.amount for proof in proofs) diff --git a/tests/unit/test_balances_by_mint_and_unit.py b/tests/unit/test_balances_by_mint_and_unit.py new file mode 100644 index 00000000..bb1b3c81 --- /dev/null +++ b/tests/unit/test_balances_by_mint_and_unit.py @@ -0,0 +1,100 @@ +"""Real-DB coverage for db.balances_by_mint_and_unit. + +Verifies the grouped liability query used by fetch_all_balances: it sums +balances per (mint_url, unit), filters to the requested mints/units, excludes +NULL mint/currency rows, and returns nothing for empty inputs. +""" + +from typing import AsyncGenerator + +import pytest +from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine +from sqlalchemy.pool import StaticPool +from sqlmodel import SQLModel +from sqlmodel.ext.asyncio.session import AsyncSession + +from routstr.core.db import ApiKey, balances_by_mint_and_unit + + +def _make_engine() -> AsyncEngine: + return create_async_engine( + "sqlite+aiosqlite://", + poolclass=StaticPool, + connect_args={"check_same_thread": False}, + ) + + +@pytest.fixture +async def session() -> "AsyncGenerator[AsyncSession, None]": + engine = _make_engine() + async with engine.begin() as conn: + await conn.run_sync(SQLModel.metadata.create_all) + db_session = AsyncSession(engine, expire_on_commit=False) + try: + yield db_session + finally: + await db_session.close() + await engine.dispose() + + +async def _add_key( + session: AsyncSession, + hashed_key: str, + balance: int, + mint_url: str | None, + currency: str | None, +) -> None: + session.add( + ApiKey( + hashed_key=hashed_key, + balance=balance, + refund_mint_url=mint_url, + refund_currency=currency, + ) + ) + await session.commit() + + +@pytest.mark.asyncio +async def test_sums_and_groups_by_mint_and_unit(session: AsyncSession) -> None: + await _add_key(session, "a", 1000, "http://m1", "sat") + await _add_key(session, "b", 500, "http://m1", "sat") + await _add_key(session, "c", 7000, "http://m1", "msat") + await _add_key(session, "d", 200, "http://m2", "sat") + + result = await balances_by_mint_and_unit( + session, ["http://m1", "http://m2"], ["sat", "msat"] + ) + + assert result[("http://m1", "sat")] == 1500 + assert result[("http://m1", "msat")] == 7000 + assert result[("http://m2", "sat")] == 200 + + +@pytest.mark.asyncio +async def test_filters_out_unrequested_mints_and_units(session: AsyncSession) -> None: + await _add_key(session, "a", 1000, "http://wanted", "sat") + await _add_key(session, "b", 999, "http://other", "sat") + await _add_key(session, "c", 888, "http://wanted", "usd") + + result = await balances_by_mint_and_unit(session, ["http://wanted"], ["sat"]) + + assert result == {("http://wanted", "sat"): 1000} + + +@pytest.mark.asyncio +async def test_excludes_rows_with_null_mint_or_currency(session: AsyncSession) -> None: + await _add_key(session, "a", 1000, "http://m1", "sat") + await _add_key(session, "b", 4242, None, None) + + result = await balances_by_mint_and_unit(session, ["http://m1"], ["sat"]) + + assert result == {("http://m1", "sat"): 1000} + + +@pytest.mark.asyncio +async def test_empty_inputs_return_empty_mapping(session: AsyncSession) -> None: + await _add_key(session, "a", 1000, "http://m1", "sat") + + assert await balances_by_mint_and_unit(session, [], ["sat"]) == {} + assert await balances_by_mint_and_unit(session, ["http://m1"], []) == {} diff --git a/tests/unit/test_fetch_all_balances.py b/tests/unit/test_fetch_all_balances.py index 433e84d4..977c2100 100644 --- a/tests/unit/test_fetch_all_balances.py +++ b/tests/unit/test_fetch_all_balances.py @@ -1,4 +1,7 @@ -from contextlib import asynccontextmanager +import asyncio +from collections.abc import AsyncIterator, Callable +from contextlib import AbstractAsyncContextManager, asynccontextmanager +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -11,9 +14,70 @@ async def _fake_session(): # type: ignore[no-untyped-def] yield MagicMock() -def _patches( # type: ignore[no-untyped-def] - proof_amount: int = 1000, user_balance_msats: int = 0 -): +class _TrackingSession: + """Stand-in for ``AsyncSession`` that records concurrent use. + + Real ``AsyncSession`` is not safe for concurrent use: if a second + coroutine issues a query while an ``await``-ed query is still in flight + on the same session it raises ``"This session is provisioning a new + connection; concurrent operations are not permitted"`` and can leave a + connection wedged. + + Every awaited query method (``exec``, ``execute``, ``scalar``, ``get`` …) + is resolved through ``__getattr__`` to the same tracked coroutine, so the + double detects concurrent use regardless of which session API the code + under test happens to call — it counts how many coroutines are inside a + query at once and remembers the peak. + """ + + def __init__(self) -> None: + self._in_flight = 0 + self.max_concurrency = 0 + self.query_calls = 0 + + def __getattr__(self, name: str): # type: ignore[no-untyped-def] + async def _tracked(*args: object, **kwargs: object) -> MagicMock: + self.query_calls += 1 + self._in_flight += 1 + self.max_concurrency = max(self.max_concurrency, self._in_flight) + try: + # Yield to the event loop so any concurrently-scheduled task + # sharing this session can enter a query before we exit. + await asyncio.sleep(0.01) + result = MagicMock() + result.one.return_value = 0 + result.all.return_value = [] + return result + finally: + self._in_flight -= 1 + + return _tracked + + +def _tracking_session_factory() -> tuple[ + Callable[[], AbstractAsyncContextManager[_TrackingSession]], + list[_TrackingSession], +]: + """Patch replacement for ``db.create_session`` that records every session. + + Each ``async with db.create_session()`` gets a fresh ``_TrackingSession``, + appended to ``sessions`` so the test can assert that *no single* session + was ever used concurrently — true whether the fix precomputes balances in + one query (one session, used serially) or opens a session per task. + """ + sessions: list[_TrackingSession] = [] + + @asynccontextmanager + async def _factory() -> "AsyncIterator[_TrackingSession]": + session = _TrackingSession() + sessions.append(session) + yield session + + return _factory, sessions + + +def _mint_patches(proof_amount: int = 1000): # type: ignore[no-untyped-def] + """Patches for the mint/proof side of ``fetch_all_balances`` (no DB).""" proof = MagicMock(amount=proof_amount) return [ patch("routstr.wallet.get_wallet", AsyncMock(return_value=MagicMock())), @@ -25,9 +89,21 @@ def _patches( # type: ignore[no-untyped-def] "routstr.wallet.slow_filter_spend_proofs", AsyncMock(side_effect=lambda proofs, wallet: proofs), ), + ] + + +def _patches( # type: ignore[no-untyped-def] + proof_amount: int = 1000, user_balance_msats: int = 0 +): + async def _grouped( # type: ignore[no-untyped-def] + session: object, mint_urls: list[str], units: list[str] + ): + return {(m, u): user_balance_msats for m in mint_urls for u in units} + + return _mint_patches(proof_amount) + [ patch( - "routstr.wallet.db.balances_for_mint_and_unit", - AsyncMock(return_value=user_balance_msats), + "routstr.wallet.db.balances_by_mint_and_unit", + AsyncMock(side_effect=_grouped), ), patch("routstr.wallet.db.create_session", _fake_session), ] @@ -79,6 +155,137 @@ async def test_fetch_all_balances_reports_liability_when_wallet_is_empty() -> No assert owner == -5 +@pytest.mark.asyncio +async def test_fetch_all_balances_does_not_use_one_session_concurrently() -> None: + """A single AsyncSession must never be shared across the gathered tasks. + + ``fetch_all_balances`` fans out one balance lookup per (mint, unit) with + ``asyncio.gather``. Passing one session into all of them makes concurrent + coroutines issue queries on the same AsyncSession — unsafe, and the source + of the observed "concurrent operations are not permitted" error that + cascades into connection-pool exhaustion. + + We drive it with several mint/unit combinations and a session whose + ``exec`` records peak concurrency. If any session is entered by more than + one coroutine at a time the finding is present. + """ + from routstr.core.settings import settings + + factory, sessions = _tracking_session_factory() + + # NB: db.balances_by_mint_and_unit is intentionally NOT patched here — we + # let the real helper run so it issues a query on the session, which is + # what exercises (and detects) concurrent session use. + patches = _mint_patches() + [ + patch("routstr.wallet.db.create_session", factory), + ] + + with patch.object( + settings, "cashu_mints", ["http://mint-a:3338", "http://mint-b:3338"] + ), patch.object(settings, "primary_mint", "http://mint-a:3338"): + for p in patches: + p.start() + try: + await fetch_all_balances(units=["sat", "msat"]) + finally: + patch.stopall() + + # The liabilities must be read in exactly ONE short-lived session with ONE + # query, before the concurrent mint fan-out — not a session per gathered + # task, and never a session shared across tasks. Asserting the exact shape + # (rather than a peak-concurrency counter, which cannot trip once no session + # crosses the gather) makes the guarantee explicit and catches a regression + # that reintroduces per-task DB access. + assert len(sessions) == 1, ( + f"expected exactly one create_session() for the up-front liability read, " + f"got {len(sessions)}" + ) + assert sessions[0].query_calls == 1, ( + f"expected a single grouped liability query, got " + f"{sessions[0].query_calls} — per-(mint,unit) querying has returned" + ) + # And that single session was never entered concurrently. + assert sessions[0].max_concurrency <= 1 + + +@pytest.mark.asyncio +async def test_fetch_all_balances_degrades_when_liability_read_fails() -> None: + """A DB failure reading liabilities must not 500 the whole balances page. + + A failed liability read does not invalidate custody, so the *known* wallet + balance is still reported (both per-mint and in the wallet total). Only the + unknowable per-user/owner split is blanked, and owner is reported as 0 so + the custody is never claimed as owner profit. + """ + from routstr.core.settings import settings + + patches = _mint_patches(proof_amount=1000) + [ + patch( + "routstr.wallet.db.balances_by_mint_and_unit", + AsyncMock(side_effect=RuntimeError("db pool exhausted")), + ), + patch("routstr.wallet.db.create_session", _fake_session), + ] + + with patch.object(settings, "cashu_mints", []), patch.object( + settings, "primary_mint", "http://primary:3338" + ): + for p in patches: + p.start() + try: + details, total_wallet, total_user, owner = await fetch_all_balances( + units=["sat"] + ) + finally: + patch.stopall() + + assert details[0]["error"] == "db pool exhausted" + assert details[0]["user_balance"] == 0 + assert details[0]["owner_balance"] == 0 + # Custody is known and must not be hidden by the liability-read failure. + assert details[0]["wallet_balance"] == 1000 + assert total_wallet == 1000 + # The user/owner split is unknown: blanked, and never claimed as profit. + assert total_user == 0 + assert owner == 0 + + +@pytest.mark.asyncio +async def test_liability_error_does_not_clobber_a_specific_mint_error() -> None: + """A per-mint fetch failure keeps its specific error under a liability error. + + When the up-front liability read fails, every detail is tagged so the UI + shows liabilities are unknown — but a detail that already failed at the mint + level carries a more specific message, which must not be overwritten by the + generic liability-read error. + """ + from routstr.core.settings import settings + + patches: list[Any] = [ + patch( + "routstr.wallet.get_wallet", + AsyncMock(side_effect=RuntimeError("mint down")), + ), + patch( + "routstr.wallet.db.balances_by_mint_and_unit", + AsyncMock(side_effect=RuntimeError("db pool exhausted")), + ), + patch("routstr.wallet.db.create_session", _fake_session), + ] + + with patch.object(settings, "cashu_mints", []), patch.object( + settings, "primary_mint", "http://primary:3338" + ): + for p in patches: + p.start() + try: + details, *_ = await fetch_all_balances(units=["sat"]) + finally: + patch.stopall() + + assert details[0]["error"] == "mint down" + + @pytest.mark.asyncio async def test_fetch_all_balances_no_duplicate_primary_mint() -> None: """primary_mint already in cashu_mints is not inspected twice.""" diff --git a/tests/unit/test_periodic_payout.py b/tests/unit/test_periodic_payout.py index 54ebadb4..0ce944b6 100644 --- a/tests/unit/test_periodic_payout.py +++ b/tests/unit/test_periodic_payout.py @@ -74,7 +74,7 @@ async def test_periodic_payout_includes_primary_mint_not_in_cashu_mints() -> Non "routstr.wallet.slow_filter_spend_proofs", AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_for_mint_and_unit", AsyncMock(return_value=0) + "routstr.wallet.db.balances_by_mint_and_unit", AsyncMock(return_value={}) ), patch("routstr.wallet.raw_send_to_lnurl", raw_send): with pytest.raises(_LoopBreak): await periodic_payout() @@ -112,7 +112,7 @@ async def test_periodic_payout_isolates_failing_mint() -> None: "routstr.wallet.slow_filter_spend_proofs", AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_for_mint_and_unit", AsyncMock(return_value=0) + "routstr.wallet.db.balances_by_mint_and_unit", AsyncMock(return_value={}) ), patch("routstr.wallet.raw_send_to_lnurl", raw_send): with pytest.raises(_LoopBreak): await periodic_payout() @@ -126,6 +126,63 @@ async def test_periodic_payout_isolates_failing_mint() -> None: assert raw_send.await_count == 2 # good mint paid for both units +@pytest.mark.asyncio +async def test_periodic_payout_reads_liability_fresh_per_iteration() -> None: + """The liability must be read fresh inside the loop, after the mint round-trip. + + Reading all liabilities once *before* the loop lets a user top-up during the + slow, per-mint payout cycle go unseen: a later (mint, unit) then computes + ``available_balance = fresh_proofs - stale_liability`` and can over-send + funds owed to users. The read must happen per (mint, unit), after the inner + ``asyncio.sleep(5)``, so it reflects the balance at payout time. + """ + from routstr.core.settings import settings + + events: list[str] = [] + + async def _sleep(seconds: float) -> None: + if seconds == _INTERVAL: + events.append("interval") + if events.count("interval") >= 2: + raise _LoopBreak() + else: + events.append(f"sleep{int(seconds)}") + + async def _liability( + session: Any, mint_urls: list[str], units: list[str] + ) -> dict[tuple[str, str], int]: + events.append("liability") + return {} + + with patch.object(settings, "cashu_mints", ["http://m:3338"]), patch.object( + settings, "primary_mint", "http://m:3338" + ), patch.object(settings, "receive_ln_address", "owner@ln.tld"), patch.object( + settings, "payout_interval_seconds", _INTERVAL + ), patch.object(settings, "min_payout_sat", 10), patch( + "routstr.wallet.asyncio.sleep", _sleep + ), patch("routstr.wallet.db.create_session", _fake_session), patch( + "routstr.wallet.get_wallet", AsyncMock(return_value=MagicMock()) + ), patch( + "routstr.wallet.get_proofs_per_mint_and_unit", + MagicMock(return_value=[MagicMock(amount=100_000)]), + ), patch( + "routstr.wallet.slow_filter_spend_proofs", + AsyncMock(side_effect=lambda proofs, wallet: proofs), + ), patch( + "routstr.wallet.db.balances_by_mint_and_unit", + AsyncMock(side_effect=_liability), + ), patch("routstr.wallet.raw_send_to_lnurl", AsyncMock(return_value=1000)): + with pytest.raises(_LoopBreak): + await periodic_payout() + + # One fresh liability read per (mint, unit) pair (sat + msat), not a single + # pre-loop snapshot shared across the whole cycle. + assert events.count("liability") == 2 + # And the first read happens only after the first inner mint sleep, proving + # it is read inside the loop at payout time rather than before it. + assert events.index("liability") > events.index("sleep5") + + @pytest.mark.asyncio async def test_periodic_payout_handles_session_creation_failure() -> None: """A db.create_session failure is logged and the payout loop continues.""" From a2cedd676934fc73e405c0d9aee57ff1d736b845 Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Fri, 24 Jul 2026 22:26:57 +0200 Subject: [PATCH 5/8] feat: make the DB connection pool env-configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add DATABASE_POOL_SIZE / DATABASE_MAX_OVERFLOW / DATABASE_POOL_TIMEOUT, consumed like every other typed env var through the pydantic Settings (constrained Fields, defaults 5/10/30 matching SQLAlchemy's own baseline so leaving them unset is behaviour-neutral). An out-of-range or non-integer value fails validation and refuses to boot — the same fail-loud behaviour a malformed DATABASE_URL already has — rather than silently starting up misconfigured. create_db_engine sizes the pool from these and logs the effective values at startup so they can be confirmed from the boot output during an incident. In-memory SQLite (StaticPool, which rejects the pool kwargs) is detected and built without them. pool_pre_ping is deliberately not exposed: the default backend is a local SQLite file with no network peer to drop idle connections, so it would add a SELECT 1 per checkout for no benefit — and it detects dead connections, not the live-but-wedged ones behind the exhaustion this series addresses. These knobs are infrastructure the node needs before it can open a DB session, so they can never be sourced from the DB (chicken-and-egg). A new ENV_ONLY_FIELDS set keeps them out of the persisted settings blob and stops a DB value from shadowing env in both SettingsService.initialize and .update, so env stays authoritative. Co-Authored-By: Claude Opus 4.8 --- .env.example | 32 ++++++++ routstr/core/db.py | 45 ++++++++++- routstr/core/settings.py | 42 +++++++++- tests/unit/test_db_pool_config.py | 129 ++++++++++++++++++++++++++++++ tests/unit/test_settings.py | 62 ++++++++++++++ 5 files changed, 305 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_db_pool_config.py diff --git a/.env.example b/.env.example index 8ff04b35..fba99b56 100644 --- a/.env.example +++ b/.env.example @@ -23,6 +23,38 @@ ROUTSTR_SECRET_KEY= # Database # DATABASE_URL=sqlite+aiosqlite:///keys.db +# Database tuning (advanced — you almost certainly do not need these) +# --------------------------------------------------------------------------- +# These size the SQLAlchemy connection pool. The defaults below match +# SQLAlchemy's own baseline, so leaving them unset changes nothing. Touch them +# ONLY if the logs show the pool running out of connections: +# +# QueuePool limit of size 5 overflow 10 reached, connection timed out, timeout 30.00 +# +# The effective values are printed at startup ("Database pool configured: ...") +# so you can confirm what is actually in effect while diagnosing an incident. +# An invalid value (non-integer, or a pool size below 1) stops the node at boot +# with a clear error rather than starting up misconfigured. +# +# DATABASE_POOL_SIZE Connections kept open in the pool. (default 5) +# DATABASE_MAX_OVERFLOW Extra connections opened under load, (default 10) +# beyond pool_size, then discarded. +# DATABASE_POOL_TIMEOUT Seconds a request waits for a free (default 30) +# connection before failing with the error above. +# +# Note for the default SQLite backend: SQLite serialises writes behind a single +# database-level lock, so a bigger pool does NOT buy more write throughput — it +# only lets more readers run at once (WAL mode) and trades pool-timeout errors +# for "database is locked" errors. If you exhaust the pool on SQLite, the cause +# is almost always connections being held too long, not the pool being too +# small. These knobs come into their own if you point DATABASE_URL at a +# networked database (e.g. Postgres), where connections genuinely run in +# parallel; there, size the pool against the server's max_connections. +# +# DATABASE_POOL_SIZE=5 +# DATABASE_MAX_OVERFLOW=10 +# DATABASE_POOL_TIMEOUT=30 + # Node Information # NAME=My Routstr Node # DESCRIPTION=Fast AI API access with Bitcoin payments diff --git a/routstr/core/db.py b/routstr/core/db.py index ab263be4..2c9b5438 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -13,7 +13,9 @@ from alembic import command from alembic.config import Config from alembic.util.exc import CommandError from sqlalchemy import Index, UniqueConstraint, case, delete, or_ +from sqlalchemy.engine import make_url from sqlalchemy.exc import IntegrityError, OperationalError +from sqlalchemy.ext.asyncio import AsyncEngine from sqlalchemy.ext.asyncio.engine import create_async_engine from sqlalchemy.orm import aliased from sqlmodel import Field, Relationship, SQLModel, col, func, select, update @@ -26,7 +28,48 @@ logger = get_logger(__name__) DATABASE_URL = os.environ.get("DATABASE_URL", "sqlite+aiosqlite:///keys.db") -engine = create_async_engine(DATABASE_URL, echo=False) # echo=True for debugging SQL +def create_db_engine(database_url: str = DATABASE_URL) -> AsyncEngine: + """Build the async engine, sizing the connection pool from ``settings``. + + Pool sizing comes from the pydantic ``Settings`` (validated at boot like + every other typed env var), so an out-of-range value refuses to start rather + than running misconfigured. Defaults match SQLAlchemy's own baseline + (5 / 10 / 30s); operators only need to tune them when logs show + ``QueuePool limit ... reached, connection timed out`` — see the "Database + tuning" section of ``.env.example``. The effective config is logged so it can + be confirmed from the boot output during an incident. + + In-memory SQLite is a special case: it uses ``StaticPool``, which rejects + the pool-sizing kwargs (passing them raises ``TypeError``), and there is no + pool to size anyway — so those URLs are built without them. + """ + from .settings import settings + + url = make_url(database_url) + is_memory_sqlite = url.get_backend_name() == "sqlite" and url.database in ( + None, + "", + ":memory:", + ) + if is_memory_sqlite: + logger.info("Database pool: in-memory SQLite, using default StaticPool") + return create_async_engine(database_url, echo=False) + + logger.info( + f"Database pool configured: pool_size={settings.database_pool_size} " + f"max_overflow={settings.database_max_overflow} " + f"pool_timeout={settings.database_pool_timeout}s", + ) + return create_async_engine( # echo=True for debugging SQL + database_url, + echo=False, + pool_size=settings.database_pool_size, + max_overflow=settings.database_max_overflow, + pool_timeout=settings.database_pool_timeout, + ) + + +engine = create_db_engine() class ApiKey(SQLModel, table=True): # type: ignore diff --git a/routstr/core/settings.py b/routstr/core/settings.py index a0b5d05c..404ce8d1 100644 --- a/routstr/core/settings.py +++ b/routstr/core/settings.py @@ -100,6 +100,14 @@ class Settings(BaseSettings): refund_cache_ttl_seconds: int = Field(default=3600, env="REFUND_CACHE_TTL_SECONDS") refund_sweep_ttl_seconds: int = Field(default=604800, env="REFUND_SWEEP_TTL_SECONDS") + # Database connection-pool sizing (advanced). Defaults match SQLAlchemy's own + # baseline, so unset is behaviour-neutral — see the "Database tuning" section + # of .env.example. A pool_size of 0 would mean an unbounded pool in + # SQLAlchemy, so it is rejected (ge=1); max_overflow=0 (no overflow) is valid. + database_pool_size: int = Field(default=5, ge=1, env="DATABASE_POOL_SIZE") + database_max_overflow: int = Field(default=10, ge=0, env="DATABASE_MAX_OVERFLOW") + database_pool_timeout: int = Field(default=30, ge=1, env="DATABASE_POOL_TIMEOUT") + # Logging log_level: str = Field(default="INFO", env="LOG_LEVEL") enable_console_logging: bool = Field(default=True, env="ENABLE_CONSOLE_LOGGING") @@ -144,10 +152,25 @@ def _normalize_settings_data(data: dict[str, Any]) -> dict[str, Any]: # ``routstr.core.vault``. SECRET_FIELDS = frozenset({"admin_password", "nsec"}) +# Infrastructure the node needs *before* it can open a DB session — so it can +# never be configured from the DB (chicken-and-egg) and stays env-only. Unlike +# secrets (owned by bootstrap), these are excluded so the DB settings blob can +# neither store nor shadow them; env is always authoritative. +ENV_ONLY_FIELDS = frozenset( + {"database_pool_size", "database_max_overflow", "database_pool_timeout"} +) + +_NON_PERSISTED_FIELDS = SECRET_FIELDS | ENV_ONLY_FIELDS + def _strip_secret_fields(data: dict[str, Any]) -> dict[str, Any]: - """Return a copy of ``data`` without any secret fields (for persistence).""" - return {k: v for k, v in data.items() if k not in SECRET_FIELDS} + """Return a copy of ``data`` without secret or env-only fields. + + Both are kept out of the persisted settings blob: secrets for confidentiality, + env-only fields (e.g. DB pool sizing) because they must never be sourced from + the database. + """ + return {k: v for k, v in data.items() if k not in _NON_PERSISTED_FIELDS} def _apply_to_live_settings(data: dict[str, Any]) -> None: @@ -327,7 +350,13 @@ class SettingsService: valid_fields = set(env_resolved.dict().keys()) merged_dict: dict[str, Any] = dict(env_resolved.dict()) merged_dict.update( - {k: v for k, v in db_json.items() if v not in (None, "", [], {}) and k in valid_fields} + { + k: v + for k, v in db_json.items() + if v not in (None, "", [], {}) + and k in valid_fields + and k not in ENV_ONLY_FIELDS + } ) merged_dict = Settings(**merged_dict).dict() @@ -394,8 +423,13 @@ class SettingsService: ) ) await db_session.commit() - # Update in-place + # Update in-place. Env-only fields (e.g. DB pool sizing) are never + # applied here: the engine pool is already built at boot from env, + # so letting an update mutate the live value would only make it + # diverge from the running pool. for k, v in candidate.dict().items(): + if k in ENV_ONLY_FIELDS: + continue setattr(settings, k, v) cls._current = settings return settings diff --git a/tests/unit/test_db_pool_config.py b/tests/unit/test_db_pool_config.py new file mode 100644 index 00000000..03473024 --- /dev/null +++ b/tests/unit/test_db_pool_config.py @@ -0,0 +1,129 @@ +"""Coverage for env-configurable DB connection-pool sizing. + +Pool sizing comes from the pydantic ``Settings`` (DATABASE_POOL_SIZE / +DATABASE_MAX_OVERFLOW / DATABASE_POOL_TIMEOUT), matching how every other typed +env var is consumed. Defaults equal SQLAlchemy's own baseline so unset is +behaviour-neutral; a bad value fails validation and refuses to boot (like a +malformed DATABASE_URL); the effective config is logged at engine creation; and +pool_pre_ping is never enabled (the default DB is a local SQLite file with no +network peer to drop idle connections). In-memory SQLite uses StaticPool, which +rejects the pool kwargs, so those URLs are built without them. +""" + +import logging + +import pytest +from pydantic.v1 import ValidationError +from sqlalchemy.ext.asyncio import AsyncEngine +from sqlalchemy.pool import QueuePool, StaticPool + +from routstr.core.db import create_db_engine +from routstr.core.settings import Settings, settings + +# A file URL keeps the pool an AsyncAdaptedQueuePool (which honours pool_size); +# engine creation is lazy, so no connection is opened and no file is created. +_URL = "sqlite+aiosqlite:///./_pool_config_test.db" + + +def _pool(engine: AsyncEngine) -> QueuePool: + pool = engine.sync_engine.pool + assert isinstance(pool, QueuePool) + return pool + + +def test_pool_defaults_match_sqlalchemy_baseline() -> None: + """With defaults, the pool keeps SQLAlchemy's 5 / 10 / 30 baseline.""" + pool = _pool(create_db_engine(_URL)) + assert pool.size() == 5 + assert pool._max_overflow == 10 + assert pool._timeout == 30 + + +def test_pool_config_reads_settings_overrides() -> None: + """create_db_engine sizes the pool from the Settings values.""" + with pytest.MonkeyPatch.context() as mp: + mp.setattr(settings, "database_pool_size", 3) + mp.setattr(settings, "database_max_overflow", 7) + mp.setattr(settings, "database_pool_timeout", 12) + + pool = _pool(create_db_engine(_URL)) + + assert pool.size() == 3 + assert pool._max_overflow == 7 + assert pool._timeout == 12 + + +@pytest.mark.parametrize("url", ["sqlite+aiosqlite://", "sqlite+aiosqlite:///:memory:"]) +def test_in_memory_sqlite_skips_pool_sizing(url: str) -> None: + """In-memory SQLite uses StaticPool, which rejects pool kwargs. + + Passing pool_size/max_overflow/pool_timeout to such a URL raises TypeError, + so create_db_engine must not send them — the engine must build cleanly. + """ + engine = create_db_engine(url) + assert isinstance(engine.sync_engine.pool, StaticPool) + + +def test_pool_does_not_enable_pre_ping() -> None: + """pre_ping stays off: the local SQLite file has no peer to drop connections.""" + assert _pool(create_db_engine(_URL))._pre_ping is False + + +def test_engine_creation_logs_effective_pool_config( + caplog: pytest.LogCaptureFixture, +) -> None: + """The effective pool config is logged so operators can confirm it at boot.""" + # The routstr loggers set propagate=False, so caplog's root handler never + # sees the record — attach the capture handler to this logger directly. + db_logger = logging.getLogger("routstr.core.db") + db_logger.addHandler(caplog.handler) + db_logger.setLevel(logging.INFO) + try: + with pytest.MonkeyPatch.context() as mp: + mp.setattr(settings, "database_pool_size", 4) + mp.setattr(settings, "database_max_overflow", 9) + mp.setattr(settings, "database_pool_timeout", 15) + create_db_engine(_URL) + finally: + db_logger.removeHandler(caplog.handler) + + logged = " ".join(record.getMessage() for record in caplog.records) + assert "pool_size" in logged + assert "4" in logged and "9" in logged and "15" in logged + + +# --- Validation happens in Settings (pydantic), like every other typed env var. +# A bad value raises ValidationError when Settings is built at boot, refusing to +# start rather than running misconfigured. + + +def test_non_integer_pool_size_rejected_at_boot( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("DATABASE_POOL_SIZE", "twenty") + with pytest.raises(ValidationError): + Settings() + + +@pytest.mark.parametrize("value", ["0", "-3"]) +def test_out_of_range_pool_size_rejected_at_boot( + monkeypatch: pytest.MonkeyPatch, value: str +) -> None: + """pool_size below 1 is rejected: 0 would mean an unbounded pool.""" + monkeypatch.setenv("DATABASE_POOL_SIZE", value) + with pytest.raises(ValidationError): + Settings() + + +def test_negative_pool_timeout_rejected_at_boot( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("DATABASE_POOL_TIMEOUT", "0") + with pytest.raises(ValidationError): + Settings() + + +def test_zero_max_overflow_is_allowed(monkeypatch: pytest.MonkeyPatch) -> None: + """max_overflow=0 (no overflow) is a valid choice, not a rejection trigger.""" + monkeypatch.setenv("DATABASE_MAX_OVERFLOW", "0") + assert Settings().database_max_overflow == 0 diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index a553b6d4..d85b52f7 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -62,6 +62,68 @@ def test_payout_settings_have_sensible_defaults() -> None: assert s.payout_interval_seconds == 900 +@pytest.mark.asyncio +async def test_database_pool_fields_are_env_only_not_persisted( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """DB pool sizing is infrastructure the node needs *before* it can read the + DB, so it can never be configured from the DB — it must never be written to + the settings blob, and a stale/injected DB value must never shadow env. + """ + monkeypatch.setenv("DATABASE_POOL_SIZE", "7") + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with AsyncSession(engine, expire_on_commit=False) as session: + s = await SettingsService.initialize(session) + + # The env value is live for runtime consumers... + assert s.database_pool_size == 7 + # ...but pool sizing is never written to the settings blob. + blob = await _read_settings_blob(session) + assert "database_pool_size" not in blob + assert "database_max_overflow" not in blob + assert "database_pool_timeout" not in blob + + # Even a stale blob that somehow carries a pool value must not win: env + # stays authoritative on the next initialize. + await session.exec( # type: ignore + text("UPDATE settings SET data = :d WHERE id = 1").bindparams( + d=json.dumps({"database_pool_size": 99}) + ) + ) + await session.commit() + again = await SettingsService.initialize(session) + assert again.database_pool_size == 7 + + +@pytest.mark.asyncio +async def test_update_does_not_apply_env_only_fields_to_live_settings( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """DB pool sizing is env-only: a settings update must neither persist it nor + mutate the live value. The engine pool is already built at boot from env, so + a UI/API update carrying a pool value must not make the live setting diverge + from the running pool. + """ + monkeypatch.delenv("DATABASE_POOL_SIZE", raising=False) + monkeypatch.setattr(settings, "database_pool_size", 5) + + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + async with AsyncSession(engine, expire_on_commit=False) as session: + await SettingsService.initialize(session) + await SettingsService.update( + {"database_pool_size": 99, "name": "PoolTweaker"}, session + ) + + # A non-env-only field still updates normally... + assert settings.name == "PoolTweaker" + # ...but the env-only pool size stays at the boot value. + assert settings.database_pool_size == 5 + # ...and it is never written to the settings blob. + blob = await _read_settings_blob(session) + assert "database_pool_size" not in blob + + @pytest.mark.parametrize( "field,bad_value", [ From 1131c2d58382f5449d4a809bee3154e439c24264 Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Fri, 24 Jul 2026 23:41:01 +0200 Subject: [PATCH 6/8] test: keep dynamic settings validation mypy-safe --- tests/unit/test_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index b51ec172..965be8e6 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -86,7 +86,7 @@ def test_database_pool_settings_reject_invalid_values( field: str, bad_value: int ) -> None: with pytest.raises(ValidationError): - Settings(**{field: bad_value}) + Settings.parse_obj({field: bad_value}) @pytest.mark.asyncio From ff55788e2d4a4b297239119f5264dd1c79cbec26 Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Sun, 26 Jul 2026 12:44:37 +0200 Subject: [PATCH 7/8] fix: address PR 634 review feedback --- .env.example | 12 +- routstr/core/admin.py | 10 +- routstr/core/db.py | 97 ++++--- routstr/core/settings.py | 15 +- routstr/lightning.py | 119 +++++---- routstr/wallet.py | 240 ++++++++++++------ .../test_lightning_invoice_constraints.py | 101 +++++++- tests/unit/test_admin_transactions.py | 35 +++ tests/unit/test_balances_by_mint_and_unit.py | 17 +- tests/unit/test_db_pool_config.py | 48 +++- tests/unit/test_fee_payout_crash_safety.py | 50 ++++ tests/unit/test_periodic_payout.py | 17 +- tests/unit/test_refund_sweep.py | 111 ++++++++ tests/unit/test_settings.py | 8 +- 14 files changed, 664 insertions(+), 216 deletions(-) create mode 100644 tests/unit/test_admin_transactions.py diff --git a/.env.example b/.env.example index ba556919..9f0c0bbc 100644 --- a/.env.example +++ b/.env.example @@ -23,14 +23,14 @@ ROUTSTR_SECRET_KEY= # Database # DATABASE_URL=sqlite+aiosqlite:///keys.db # Pool controls are validated at boot, sourced only from the environment, and -# logged at startup. Defaults intentionally bound capacity and fail quickly so -# leaked connections are visible instead of hidden by overflow connections. -# Keep total capacity across all workers below the database connection limit. +# logged at startup. Keep total capacity across all workers below the database +# connection limit. Pre-ping is automatic for networked backends; SQLite may +# explicitly opt in if desired. # DATABASE_POOL_SIZE=5 -# DATABASE_MAX_OVERFLOW=0 -# DATABASE_POOL_TIMEOUT=5 +# DATABASE_MAX_OVERFLOW=10 +# DATABASE_POOL_TIMEOUT=30 # DATABASE_POOL_RECYCLE=1800 -# DATABASE_POOL_PRE_PING=true +# DATABASE_POOL_PRE_PING=false # Warn when a checkout is held this many seconds. # DATABASE_POOL_HOLD_WARN_SECONDS=10 # SQLite serialises writes; increasing its pool can trade pool timeouts for diff --git a/routstr/core/admin.py b/routstr/core/admin.py index 66a1d288..74a94668 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -1669,12 +1669,18 @@ async def get_transactions_api( ) total = count_result.one() - stmt = base.order_by(col(CashuTransaction.created_at).desc()).offset(offset).limit(limit) + stmt = ( + base.order_by(col(CashuTransaction.created_at).desc()) + .offset(offset) + .limit(limit) + ) results = await session.exec(stmt) transactions = results.all() return { - "transactions": [tx.dict() for tx in transactions], + "transactions": [ + tx.dict(exclude={"sweep_started_at"}) for tx in transactions + ], "total": total, } diff --git a/routstr/core/db.py b/routstr/core/db.py index 0ee04933..9e0c5973 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -22,6 +22,7 @@ from sqlmodel import Field, Relationship, SQLModel, col, func, select, update from sqlmodel.ext.asyncio.session import AsyncSession from .logging import get_logger +from .settings import settings logger = get_logger(__name__) @@ -29,18 +30,13 @@ DATABASE_URL = os.environ.get("DATABASE_URL", "sqlite+aiosqlite:///keys.db") def create_db_engine(database_url: str = DATABASE_URL) -> AsyncEngine: - """Build an async engine from validated, environment-only pool settings.""" - from .settings import settings - + """Build and instrument an async engine from environment-only settings.""" url = make_url(database_url) - is_memory_sqlite = url.get_backend_name() == "sqlite" and url.database in { - None, - "", - ":memory:", - } - options: dict[str, int | float | bool] = { - "pool_pre_ping": settings.database_pool_pre_ping, - } + backend = url.get_backend_name() + is_sqlite = backend == "sqlite" + is_memory_sqlite = is_sqlite and url.database in {None, "", ":memory:"} + pool_pre_ping = settings.database_pool_pre_ping or not is_sqlite + options: dict[str, int | float | bool] = {"pool_pre_ping": pool_pre_ping} if not is_memory_sqlite: options.update( pool_size=settings.database_pool_size, @@ -52,44 +48,45 @@ def create_db_engine(database_url: str = DATABASE_URL) -> AsyncEngine: logger.info( "Database pool configured", extra={ - "database_url_backend": url.get_backend_name(), + "database_url_backend": backend, "in_memory_sqlite": is_memory_sqlite, **options, }, ) - return create_async_engine(database_url, echo=False, **options) + created_engine = create_async_engine(database_url, echo=False, **options) + hold_warn_seconds = settings.database_pool_hold_warn_seconds + + def record_pool_checkout( + dbapi_connection: object, connection_record: object, proxy: object + ) -> None: + connection_record.info["routstr_checked_out_at"] = time.monotonic() # type: ignore[attr-defined] + + def record_pool_checkin( + dbapi_connection: object, connection_record: object + ) -> None: + checked_out_at = connection_record.info.pop( # type: ignore[attr-defined] + "routstr_checked_out_at", None + ) + if checked_out_at is None: + return + held_seconds = time.monotonic() - checked_out_at + if held_seconds >= hold_warn_seconds: + logger.warning( + "Database connection held longer than threshold", + extra={ + "held_seconds": round(held_seconds, 3), + "threshold_seconds": hold_warn_seconds, + "pool_status": created_engine.pool.status(), + }, + ) + + event.listen(created_engine.sync_engine, "checkout", record_pool_checkout) + event.listen(created_engine.sync_engine, "checkin", record_pool_checkin) + return created_engine engine = create_db_engine() -from .settings import settings as _runtime_settings # noqa: E402 - -_POOL_HOLD_WARN_SECONDS = _runtime_settings.database_pool_hold_warn_seconds - - -@event.listens_for(engine.sync_engine, "checkout") -def _record_pool_checkout( - dbapi_connection: object, connection_record: object, proxy: object -) -> None: - connection_record.info["routstr_checked_out_at"] = time.monotonic() # type: ignore[attr-defined] - - -@event.listens_for(engine.sync_engine, "checkin") -def _record_pool_checkin(dbapi_connection: object, connection_record: object) -> None: - checked_out_at = connection_record.info.pop("routstr_checked_out_at", None) # type: ignore[attr-defined] - if checked_out_at is None: - return - held_seconds = time.monotonic() - checked_out_at - if held_seconds >= _POOL_HOLD_WARN_SECONDS: - logger.warning( - "Database connection held longer than threshold", - extra={ - "held_seconds": round(held_seconds, 3), - "threshold_seconds": _POOL_HOLD_WARN_SECONDS, - "pool_status": engine.pool.status(), - }, - ) - class ApiKey(SQLModel, table=True): # type: ignore __tablename__ = "api_keys" @@ -284,7 +281,10 @@ async def release_stale_reservations( if released: logger.warning( "Released stale reservations", - extra={"released_reservations": released, "max_age_seconds": max_age_seconds}, + extra={ + "released_reservations": released, + "max_age_seconds": max_age_seconds, + }, ) return released @@ -783,6 +783,19 @@ async def complete_routstr_fee_payout( return result.rowcount == 1 +async def balance_for_mint_and_unit( + db_session: AsyncSession, mint_url: str, unit: str +) -> int: + """Return the user liability for one mint and unit in millisatoshis.""" + result = await db_session.exec( + select(func.sum(ApiKey.balance)).where( + col(ApiKey.refund_mint_url) == mint_url, + col(ApiKey.refund_currency) == unit, + ) + ) + return int(result.one() or 0) + + async def balances_by_mint_and_unit( db_session: AsyncSession, mint_urls: list[str], units: list[str] ) -> dict[tuple[str, str], int]: diff --git a/routstr/core/settings.py b/routstr/core/settings.py index 34c6d9c1..0caffd16 100644 --- a/routstr/core/settings.py +++ b/routstr/core/settings.py @@ -106,14 +106,17 @@ class Settings(BaseSettings): default=900, gt=0, env="REFUND_SWEEP_CLAIM_TIMEOUT_SECONDS" ) - # Database connection-pool controls (advanced). Keep the default pool - # bounded and fail quickly: increasing it can hide leaked connections and - # makes SQLite lock contention worse. These fields are env-only below. + # Database connection-pool controls (advanced). Capacity defaults match + # SQLAlchemy's established queue-pool behavior. Pre-ping is enabled by the + # engine factory for networked backends; SQLite can explicitly opt in. + # These fields are env-only below. database_pool_size: int = Field(default=5, ge=1, env="DATABASE_POOL_SIZE") - database_max_overflow: int = Field(default=0, ge=0, env="DATABASE_MAX_OVERFLOW") - database_pool_timeout: float = Field(default=5.0, gt=0, env="DATABASE_POOL_TIMEOUT") + database_max_overflow: int = Field(default=10, ge=0, env="DATABASE_MAX_OVERFLOW") + database_pool_timeout: float = Field( + default=30.0, gt=0, env="DATABASE_POOL_TIMEOUT" + ) database_pool_recycle: int = Field(default=1800, ge=0, env="DATABASE_POOL_RECYCLE") - database_pool_pre_ping: bool = Field(default=True, env="DATABASE_POOL_PRE_PING") + database_pool_pre_ping: bool = Field(default=False, env="DATABASE_POOL_PRE_PING") database_pool_hold_warn_seconds: float = Field( default=10.0, gt=0, env="DATABASE_POOL_HOLD_WARN_SECONDS" ) diff --git a/routstr/lightning.py b/routstr/lightning.py index 5bba7593..554925f2 100644 --- a/routstr/lightning.py +++ b/routstr/lightning.py @@ -225,48 +225,84 @@ async def check_invoice_payment( minted = False invoice_id = invoice.id invoice_purpose = invoice.purpose + invoice_amount_sats = invoice.amount_sats + invoice_payment_hash = invoice.payment_hash + finalized_api_key_hash = invoice.api_key_hash try: # A preceding invoice lookup starts a transaction. End it before the # potentially slow mint request so it cannot pin a pool connection. await session.commit() + # Do not redeem a paid top-up quote if its target has already been + # pruned. This validation owns a short-lived session and releases its + # connection before any mint I/O starts. + if invoice_purpose == "topup": + if not finalized_api_key_hash: + raise ValueError("No API key associated with topup invoice") + async with create_session() as validation_session: + target = await validation_session.get(ApiKey, finalized_api_key_hash) + if target is None: + logger.error( + "Topup invoice target API key was not found; skipping mint", + extra={"invoice_id": invoice_id}, + ) + return + wallet = await get_wallet(settings.primary_mint, "sat") - mint_status = await wallet.get_mint_quote(invoice.payment_hash) + mint_status = await wallet.get_mint_quote(invoice_payment_hash) if not mint_status.paid: return # The mint enforces single-use quotes, so a concurrent checker that # races us here fails inside wallet.mint rather than double-minting. - await wallet.mint(invoice.amount_sats, quote_id=invoice.payment_hash) + await wallet.mint(invoice_amount_sats, quote_id=invoice_payment_hash) minted = True - if invoice_purpose == "create": - api_key = await _create_api_key_record(invoice, session) - invoice.api_key_hash = api_key.hashed_key - elif invoice_purpose == "topup": - await _credit_topup_record(invoice, session) + # Paid finalization owns a fresh session. The API/watcher session is + # never rolled back by this function, so its invoice and sibling ORM + # objects remain usable after a DB failure or lost CAS race. + async with create_session() as finalization_session: + if invoice_purpose == "create": + api_key = await _create_api_key_record(invoice, finalization_session) + finalized_api_key_hash = api_key.hashed_key + elif invoice_purpose == "topup": + await _credit_topup_record(invoice, finalization_session) - # Conditional transition guards against double-credit: the credit - # above and this status flip commit atomically, and a lost race - # rolls both back. - paid_at = int(time.time()) - finalized = await session.exec( # type: ignore[call-overload] - update(LightningInvoice) - .where( - col(LightningInvoice.id) == invoice_id, - col(LightningInvoice.status) == "pending", + # Conditional transition guards against double-credit: the credit + # above and this status flip commit atomically, and a lost race + # rolls both back in the owned finalization session. + paid_at = int(time.time()) + finalized = await finalization_session.exec( # type: ignore[call-overload] + update(LightningInvoice) + .where( + col(LightningInvoice.id) == invoice_id, + col(LightningInvoice.status) == "pending", + ) + .values( + status="paid", + paid_at=paid_at, + api_key_hash=finalized_api_key_hash, + ) ) - .values( - status="paid", - paid_at=paid_at, - api_key_hash=invoice.api_key_hash, - ) - ) - if finalized.rowcount != 1: - await session.rollback() - await session.refresh(invoice) - return - await session.commit() + if finalized.rowcount != 1: + await finalization_session.rollback() + committed_invoice = await finalization_session.get( + LightningInvoice, invoice_id + ) + await finalization_session.commit() + if committed_invoice is not None: + # A concurrent finalizer won the CAS. Publish only the + # state observed from the database after ending the owned + # read transaction; never refresh the caller's session. + invoice.api_key_hash = committed_invoice.api_key_hash + invoice.status = committed_invoice.status + invoice.paid_at = committed_invoice.paid_at + return + await finalization_session.commit() + + # Only publish finalized values to the caller-owned object after the + # owned transaction has committed successfully. + invoice.api_key_hash = finalized_api_key_hash invoice.status = "paid" invoice.paid_at = paid_at @@ -274,20 +310,17 @@ async def check_invoice_payment( "Lightning invoice paid", extra={ "invoice_id": invoice_id, - "amount_sats": invoice.amount_sats, + "amount_sats": invoice_amount_sats, "purpose": invoice_purpose, - "api_key_hash": invoice.api_key_hash[:8] + "..." - if invoice.api_key_hash + "api_key_hash": finalized_api_key_hash[:8] + "..." + if finalized_api_key_hash else None, }, ) except BaseException as e: # BaseException so task cancellation (e.g. client disconnect) after a - # successful mint still triggers the reconciliation alert. - try: - await asyncio.shield(session.rollback()) - except Exception: - logger.warning("Rollback failed during invoice check cleanup") + # successful mint still triggers the reconciliation alert. Any rollback + # belongs to create_session(), never to the caller-owned session. if minted: logger.critical( "Invoice mint succeeded but DB finalization failed; reconciliation required", @@ -331,22 +364,6 @@ async def _credit_topup_record( raise ValueError("Associated API key not found") -async def create_api_key_from_invoice( - invoice: LightningInvoice, session: AsyncSession -) -> ApiKey: - wallet = await get_wallet(settings.primary_mint, "sat") - await wallet.mint(invoice.amount_sats, quote_id=invoice.payment_hash) - return await _create_api_key_record(invoice, session) - - -async def topup_api_key_from_invoice( - invoice: LightningInvoice, session: AsyncSession -) -> None: - wallet = await get_wallet(settings.primary_mint, "sat") - await wallet.mint(invoice.amount_sats, quote_id=invoice.payment_hash) - await _credit_topup_record(invoice, session) - - INVOICE_WATCH_INTERVAL_SECONDS = 5 INVOICE_WATCH_BATCH_LIMIT = 100 diff --git a/routstr/wallet.py b/routstr/wallet.py index ac2fda3d..e2a20a7c 100644 --- a/routstr/wallet.py +++ b/routstr/wallet.py @@ -33,6 +33,22 @@ _CashuMintInfo.model_rebuild(force=True) logger = get_logger(__name__) +def _sats_to_msats(amount: int) -> int: + return amount * 1000 + + +def _msats_to_sats(amount: int) -> int: + return amount // 1000 + + +def _mints_to_inspect() -> list[str]: + """Return configured mints plus the primary mint, without duplicates.""" + mint_urls = list(settings.cashu_mints) + if settings.primary_mint and settings.primary_mint not in mint_urls: + mint_urls.append(settings.primary_mint) + return mint_urls + + class MintConnectionError(Exception): """The mint could not be reached (network transport failure). @@ -305,10 +321,10 @@ def _net_minted_amount(amount_msat: int, token_unit: str, fees: int) -> int: Convert the token value minus fees (given in the token unit) into an amount in the primary mint's unit. """ - fee_msat = fees * 1000 if token_unit == "sat" else fees + fee_msat = _sats_to_msats(fees) if token_unit == "sat" else fees remaining_msat = amount_msat - fee_msat if settings.primary_mint_unit == "sat": - return int(remaining_msat // 1000) + return _msats_to_sats(remaining_msat) return int(remaining_msat) @@ -361,7 +377,7 @@ async def _calculate_swap_amount( melt fees and NUT-02 input fees on the foreign mint. """ if settings.primary_mint_unit == "sat": - receive_amount = amount_msat // 1000 + receive_amount = _msats_to_sats(amount_msat) else: receive_amount = amount_msat @@ -395,7 +411,7 @@ async def _calculate_swap_amount( logger.info( "swap_to_primary_mint: fee estimation result", extra={ - "token_amount_sat": amount_msat // 1000, + "token_amount_sat": _msats_to_sats(amount_msat), "estimated_fee": total_fees, "estimated_fee_unit": token_unit, "input_fees": input_fees, @@ -434,7 +450,7 @@ async def swap_to_primary_mint( token_amount = token_obj.amount if token_obj.unit == "sat": - amount_msat = token_amount * 1000 + amount_msat = _sats_to_msats(token_amount) elif token_obj.unit == "msat": amount_msat = token_amount else: @@ -598,7 +614,10 @@ async def swap_to_primary_mint( # advance the counter so the next request derives fresh secrets. logger.warning( "swap_to_primary_mint: outputs already signed — recovering orphaned proofs", - extra={"mint_quote_id": mint_quote.quote, "minted_amount": minted_amount}, + extra={ + "mint_quote_id": mint_quote.quote, + "minted_amount": minted_amount, + }, ) try: for keyset_id in primary_wallet.keysets: @@ -683,7 +702,7 @@ async def credit_balance( ) if unit == "sat": - amount = amount * 1000 + amount = _sats_to_msats(amount) logger.info( "credit_balance: Converted to msat", extra={"amount_msat": amount} ) @@ -838,9 +857,7 @@ async def fetch_all_balances( # Received tokens are stored against primary_mint even when cashu_mints is # empty, so include it in both the liability query and mint fan-out. - mint_urls: list[str] = list(settings.cashu_mints) - if settings.primary_mint and settings.primary_mint not in mint_urls: - mint_urls.append(settings.primary_mint) + mint_urls = _mints_to_inspect() user_balances: dict[tuple[str, str], int] = {} liabilities_error: str | None = None @@ -865,7 +882,7 @@ async def fetch_all_balances( proofs = await slow_filter_spend_proofs(proofs, wallet) user_balance = user_balances.get((mint_url, unit), 0) if unit == "sat": - user_balance = user_balance // 1000 + user_balance = _msats_to_sats(user_balance) proofs_balance = sum(proof.amount for proof in proofs) result: BalanceDetail = { @@ -900,13 +917,13 @@ async def fetch_all_balances( total_wallet_balance_sats += ( detail["wallet_balance"] if unit == "sat" - else detail["wallet_balance"] // 1000 + else _msats_to_sats(detail["wallet_balance"]) ) if liabilities_error is None: total_user_balance_sats += ( detail["user_balance"] if unit == "sat" - else detail["user_balance"] // 1000 + else _msats_to_sats(detail["user_balance"]) ) if liabilities_error is None: @@ -938,9 +955,7 @@ async def periodic_payout() -> None: # Include the primary mint even if it is not listed in cashu_mints, # matching fetch_all_balances(); otherwise primary-mint funds never # auto-payout. - mint_urls: list[str] = list(settings.cashu_mints) - if settings.primary_mint and settings.primary_mint not in mint_urls: - mint_urls.append(settings.primary_mint) + mint_urls = _mints_to_inspect() for mint_url in mint_urls: for unit in ["sat", "msat"]: @@ -953,24 +968,47 @@ async def periodic_payout() -> None: ) proofs = await slow_filter_spend_proofs(proofs, wallet) await asyncio.sleep(5) - # Fetch the liability AFTER the proofs snapshot and the - # settle delay: a concurrent top-up then only inflates - # the liability, shrinking the payout — never sending - # customer-backed funds as profit. + except Exception as e: + logger.error( + f"Error sending payout: {type(e).__name__}", + extra={ + "error": str(e), + "mint_url": mint_url, + "unit": unit, + }, + ) + continue + + # Fetch the liability AFTER the proofs snapshot and the + # settle delay: a concurrent top-up then only inflates the + # liability, shrinking the payout — never sending + # customer-backed funds as profit. + try: async with db.create_session() as session: - balances = await db.balances_by_mint_and_unit( - session, [mint_url], [unit] + user_balance = await db.balance_for_mint_and_unit( + session, mint_url, unit ) - user_balance = balances.get((mint_url, unit), 0) + except Exception as e: + logger.error( + f"Error in periodic payout cycle: {type(e).__name__}", + extra={ + "error": str(e), + "mint_url": mint_url, + "unit": unit, + }, + ) + continue + + try: if unit == "sat": - user_balance = user_balance // 1000 + user_balance = _msats_to_sats(user_balance) proofs_balance = sum(proof.amount for proof in proofs) available_balance = proofs_balance - user_balance # Threshold is configured in sats; convert for msat wallets. min_amount = ( settings.min_payout_sat if unit == "sat" - else settings.min_payout_sat * 1000 + else _sats_to_msats(settings.min_payout_sat) ) if available_balance > min_amount: amount_received = await raw_send_to_lnurl( @@ -1005,20 +1043,24 @@ async def periodic_payout() -> None: ) -async def _set_refund_sweep_state(refund_id: str, **values: object) -> None: +async def _set_refund_sweep_state( + refund_id: str, + *, + predicates: tuple[typing.Any, ...] = (), + **values: object, +) -> int: async with db.create_session() as session: - await session.exec( # type: ignore[call-overload] + result = await session.exec( # type: ignore[call-overload] update(db.CashuTransaction) - .where(col(db.CashuTransaction.id) == refund_id) + .where(col(db.CashuTransaction.id) == refund_id, *predicates) .values(**values) ) await session.commit() + return int(result.rowcount or 0) async def _refund_sweep_once(cutoff: int) -> None: - claim_cutoff = ( - int(time.time()) - settings.refund_sweep_claim_timeout_seconds - ) + claim_cutoff = int(time.time()) - settings.refund_sweep_claim_timeout_seconds claim_available = col(db.CashuTransaction.sweep_started_at).is_(None) | ( col(db.CashuTransaction.sweep_started_at) < claim_cutoff ) @@ -1036,69 +1078,109 @@ async def _refund_sweep_once(cutoff: int) -> None: for refund in refunds: reclaimed_stale_claim = refund.sweep_started_at is not None claim_started_at = int(time.time()) - async with db.create_session() as session: - claim = await session.exec( # type: ignore[call-overload] - update(db.CashuTransaction) - .where( - col(db.CashuTransaction.id) == refund.id, - col(db.CashuTransaction.swept) == False, # noqa: E712 - col(db.CashuTransaction.collected) == False, # noqa: E712 - claim_available, - ) - .values(sweep_started_at=claim_started_at) - ) - await session.commit() - if claim.rowcount != 1: + claimed = await _set_refund_sweep_state( + refund.id, + predicates=( + col(db.CashuTransaction.swept) == False, # noqa: E712 + col(db.CashuTransaction.collected) == False, # noqa: E712 + claim_available, + ), + sweep_started_at=claim_started_at, + ) + if claimed != 1: continue + claim_owned = col(db.CashuTransaction.sweep_started_at) == claim_started_at + redeemed = False try: await recieve_token(refund.token) - await _set_refund_sweep_state( - refund.id, swept=True, sweep_started_at=None - ) - logger.info( - "Swept uncollected refund", - extra={ - "id": refund.id, - "amount": refund.amount, - "unit": refund.unit, - }, + redeemed = True + finalized = await _set_refund_sweep_state( + refund.id, + predicates=(claim_owned,), + swept=True, + sweep_started_at=None, ) + if finalized == 1: + logger.info( + "Swept uncollected refund", + extra={ + "id": refund.id, + "amount": refund.amount, + "unit": refund.unit, + }, + ) + else: + logger.critical( + "Refund token swept after claim ownership changed; manual reconciliation required", + extra={"id": refund.id}, + ) except BaseException as e: + if redeemed: + # The token is already in the node wallet. Retain the claim so + # a stale retry classifies "already spent" as swept, never as a + # client collection. + logger.critical( + "Refund token swept but checkpoint was not completed; manual reconciliation required", + extra={"id": refund.id}, + exc_info=isinstance(e, Exception), + ) + if not isinstance(e, Exception): + raise + continue + error_msg = str(e).lower() if isinstance(e, Exception) and "already spent" in error_msg: if reclaimed_stale_claim: # A prior worker may have redeemed the token and crashed # before finalizing. Treat the ambiguous stale claim as a # completed sweep rather than misreporting client collection. - await _set_refund_sweep_state( - refund.id, swept=True, sweep_started_at=None + updated = await _set_refund_sweep_state( + refund.id, + predicates=(claim_owned,), + swept=True, + sweep_started_at=None, ) else: - await _set_refund_sweep_state( + updated = await _set_refund_sweep_state( refund.id, + predicates=(claim_owned,), collected=True, swept=False, sweep_started_at=None, ) - logger.info( - "Refund token was already spent", - extra={ - "id": refund.id, - "reclaimed_stale_claim": reclaimed_stale_claim, - }, - ) + if updated == 1: + logger.info( + "Refund token was already spent", + extra={ + "id": refund.id, + "reclaimed_stale_claim": reclaimed_stale_claim, + }, + ) + else: + logger.warning( + "Refund claim ownership changed before spent-token checkpoint", + extra={"id": refund.id}, + ) else: - # Cancellation is a known-no-send outcome at this boundary, so - # release the lease under shield and let a later sweep retry. - await asyncio.shield( - _set_refund_sweep_state(refund.id, sweep_started_at=None) + # Cancellation or a transient pre-redemption failure is a + # known-no-send outcome, so release the lease for a later retry. + released = await asyncio.shield( + _set_refund_sweep_state( + refund.id, + predicates=(claim_owned,), + sweep_started_at=None, + ) ) if not isinstance(e, Exception): raise logger.warning( "Failed to sweep refund", - extra={"id": refund.id, "error": str(e)}, + extra={ + "id": refund.id, + "error": str(e), + "claim_released": released == 1, + }, ) @@ -1145,10 +1227,10 @@ async def periodic_routstr_fee_payout() -> None: ) continue - accumulated_sats = fee.accumulated_msats // 1000 + accumulated_sats = _msats_to_sats(fee.accumulated_msats) if accumulated_sats < ROUTSTR_FEE_DEFAULT_PAYOUT: continue - paid_msats = accumulated_sats * 1000 + paid_msats = _sats_to_msats(accumulated_sats) # Wallet/proof preparation cannot send funds, so do it before the # durable checkpoint. A preparation failure must not strand an @@ -1180,10 +1262,20 @@ async def periodic_routstr_fee_payout() -> None: ) continue - async with db.create_session() as session: - payout_completed = await db.complete_routstr_fee_payout( - session, paid_msats + try: + async with db.create_session() as session: + payout_completed = await db.complete_routstr_fee_payout( + session, paid_msats + ) + except BaseException as e: + logger.critical( + "Routstr fee payout sent but checkpoint was not completed", + extra={"payout_in_progress_msats": paid_msats}, + exc_info=isinstance(e, Exception), ) + if not isinstance(e, Exception): + raise + continue if not payout_completed: logger.critical( "Routstr fee payout sent but checkpoint was not completed", diff --git a/tests/integration/test_lightning_invoice_constraints.py b/tests/integration/test_lightning_invoice_constraints.py index a7fa1384..b4e12a7f 100644 --- a/tests/integration/test_lightning_invoice_constraints.py +++ b/tests/integration/test_lightning_invoice_constraints.py @@ -3,8 +3,8 @@ Covers two things: - The three constraint fields (balance_limit, balance_limit_reset, validity_date) are persisted on LightningInvoice and survive a DB round-trip. -- create_api_key_from_invoice propagates those fields to the created ApiKey, - so the constraints are actually enforced when the key is used. +- The production-path API-key record helper propagates those fields to the + created ApiKey, so the constraints are actually enforced when the key is used. """ from __future__ import annotations @@ -14,11 +14,12 @@ import time from unittest.mock import AsyncMock, MagicMock, patch import pytest +from sqlalchemy import inspect from sqlalchemy.ext.asyncio import AsyncEngine from sqlmodel.ext.asyncio.session import AsyncSession from routstr.core.db import ApiKey, LightningInvoice -from routstr.lightning import create_api_key_from_invoice +from routstr.lightning import _create_api_key_record def _make_invoice(**kwargs: object) -> LightningInvoice: @@ -104,7 +105,7 @@ async def test_created_key_receives_balance_limit( integration_session.add(invoice) await integration_session.flush() - api_key = await create_api_key_from_invoice(invoice, integration_session) + api_key = await _create_api_key_record(invoice, integration_session) await integration_session.commit() stored_key = await integration_session.get(ApiKey, api_key.hashed_key) @@ -120,7 +121,7 @@ async def test_created_key_receives_balance_limit_reset( integration_session.add(invoice) await integration_session.flush() - api_key = await create_api_key_from_invoice(invoice, integration_session) + api_key = await _create_api_key_record(invoice, integration_session) await integration_session.commit() stored_key = await integration_session.get(ApiKey, api_key.hashed_key) @@ -137,7 +138,7 @@ async def test_created_key_receives_validity_date( integration_session.add(invoice) await integration_session.flush() - api_key = await create_api_key_from_invoice(invoice, integration_session) + api_key = await _create_api_key_record(invoice, integration_session) await integration_session.commit() stored_key = await integration_session.get(ApiKey, api_key.hashed_key) @@ -148,6 +149,7 @@ async def test_created_key_receives_validity_date( @pytest.mark.asyncio async def test_payment_check_releases_connection_during_mint_quote( integration_engine: AsyncEngine, + patched_db_engine: None, ) -> None: invoice = _make_invoice(id="inv_slow_quote", status="pending", paid_at=None) async with AsyncSession(integration_engine, expire_on_commit=False) as setup: @@ -173,6 +175,7 @@ async def test_payment_check_releases_connection_during_mint_quote( @pytest.mark.asyncio async def test_concurrent_payment_checks_mint_and_credit_invoice_once( integration_engine: AsyncEngine, + patched_db_engine: None, ) -> None: invoice = _make_invoice(id="inv_concurrent", status="pending", paid_at=None) async with AsyncSession(integration_engine, expire_on_commit=False) as setup: @@ -227,6 +230,7 @@ async def test_concurrent_payment_checks_mint_and_credit_invoice_once( @pytest.mark.asyncio async def test_failed_mint_keeps_invoice_pending_for_retry( integration_engine: AsyncEngine, + patched_db_engine: None, ) -> None: invoice = _make_invoice(id="inv_mint_failure", status="pending", paid_at=None) async with AsyncSession(integration_engine, expire_on_commit=False) as setup: @@ -251,20 +255,62 @@ async def test_failed_mint_keeps_invoice_pending_for_retry( @pytest.mark.asyncio -async def test_post_mint_db_failure_keeps_invoice_pending_for_reconciliation( +async def test_missing_topup_target_is_rejected_before_mint( integration_engine: AsyncEngine, + patched_db_engine: None, ) -> None: - invoice = _make_invoice(id="inv_finalize_failure", status="pending", paid_at=None) + invoice = _make_invoice( + id="inv_missing_topup_target", + status="pending", + paid_at=None, + purpose="topup", + api_key_hash="pruned-key", + ) async with AsyncSession(integration_engine, expire_on_commit=False) as setup: setup.add(invoice) await setup.commit() + get_wallet = AsyncMock() + async with AsyncSession(integration_engine, expire_on_commit=False) as session: + stored = await session.get(LightningInvoice, invoice.id) + assert stored is not None + with patch("routstr.lightning.get_wallet", get_wallet): + from routstr.lightning import check_invoice_payment + + await check_invoice_payment(stored, session) + + get_wallet.assert_not_awaited() + async with AsyncSession(integration_engine) as verify: + stored = await verify.get(LightningInvoice, invoice.id) + assert stored is not None + assert stored.status == "pending" + + +@pytest.mark.asyncio +async def test_post_mint_db_failure_keeps_invoice_pending_for_reconciliation( + integration_engine: AsyncEngine, + patched_db_engine: None, +) -> None: + invoice = _make_invoice(id="inv_finalize_failure", status="pending", paid_at=None) + sibling = _make_invoice( + id="inv_finalize_failure_sibling", + bolt11="lnbc1000n1sibling", + payment_hash="cafebabe" * 8, + status="pending", + paid_at=None, + ) + async with AsyncSession(integration_engine, expire_on_commit=False) as setup: + setup.add_all([invoice, sibling]) + await setup.commit() + wallet = MagicMock() wallet.get_mint_quote = AsyncMock(return_value=MagicMock(paid=True)) wallet.mint = AsyncMock(return_value=[]) async with AsyncSession(integration_engine, expire_on_commit=False) as session: stored = await session.get(LightningInvoice, invoice.id) + stored_sibling = await session.get(LightningInvoice, sibling.id) assert stored is not None + assert stored_sibling is not None with ( patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)), patch( @@ -276,6 +322,15 @@ async def test_post_mint_db_failure_keeps_invoice_pending_for_reconciliation( await check_invoice_payment(stored, session) + stored_state = inspect(stored) + sibling_state = inspect(stored_sibling) + assert stored_state is not None + assert sibling_state is not None + assert stored_state.expired is False + assert sibling_state.expired is False + assert stored.status == "pending" + assert stored_sibling.id == sibling.id + assert wallet.mint.await_count == 1 async with AsyncSession(integration_engine, expire_on_commit=False) as verify: stored = await verify.get(LightningInvoice, invoice.id) @@ -291,7 +346,7 @@ async def test_created_key_without_constraints_has_none_fields( integration_session.add(invoice) await integration_session.flush() - api_key = await create_api_key_from_invoice(invoice, integration_session) + api_key = await _create_api_key_record(invoice, integration_session) await integration_session.commit() stored_key = await integration_session.get(ApiKey, api_key.hashed_key) @@ -304,6 +359,7 @@ async def test_created_key_without_constraints_has_none_fields( @pytest.mark.asyncio async def test_db_guard_credits_once_when_both_mints_succeed( integration_engine: AsyncEngine, + patched_db_engine: None, ) -> None: """Even if the mint fails to enforce single-use quotes and both racers mint successfully, the conditional status update must credit exactly once.""" @@ -315,9 +371,15 @@ async def test_db_guard_credits_once_when_both_mints_succeed( purpose="topup", api_key_hash="race-key", ) + sibling = _make_invoice( + id="inv_db_guard_sibling", + bolt11="lnbc1000n1race-sibling", + payment_hash="01234567" * 8, + status="pending", + paid_at=None, + ) async with AsyncSession(integration_engine, expire_on_commit=False) as setup: - setup.add(key) - setup.add(invoice) + setup.add_all([key, invoice, sibling]) await setup.commit() wallet = MagicMock() @@ -334,8 +396,10 @@ async def test_db_guard_credits_once_when_both_mints_succeed( AsyncSession(integration_engine, expire_on_commit=False) as second, ): first_invoice = await first.get(LightningInvoice, invoice.id) + first_sibling = await first.get(LightningInvoice, sibling.id) second_invoice = await second.get(LightningInvoice, invoice.id) assert first_invoice is not None + assert first_sibling is not None assert second_invoice is not None with patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)): @@ -346,6 +410,21 @@ async def test_db_guard_credits_once_when_both_mints_succeed( check_invoice_payment(second_invoice, second), ) + first_state = inspect(first_invoice) + sibling_state = inspect(first_sibling) + second_state = inspect(second_invoice) + assert first_state is not None + assert sibling_state is not None + assert second_state is not None + assert first_state.expired is False + assert sibling_state.expired is False + assert second_state.expired is False + assert first_invoice.id == invoice.id + assert first_sibling.id == sibling.id + assert second_invoice.id == invoice.id + assert first_invoice.status == "paid" + assert second_invoice.status == "paid" + assert wallet.mint.await_count == 2 async with AsyncSession(integration_engine, expire_on_commit=False) as verify: stored_invoice = await verify.get(LightningInvoice, invoice.id) diff --git a/tests/unit/test_admin_transactions.py b/tests/unit/test_admin_transactions.py new file mode 100644 index 00000000..22508fd6 --- /dev/null +++ b/tests/unit/test_admin_transactions.py @@ -0,0 +1,35 @@ +from contextlib import asynccontextmanager +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from routstr.core.admin import get_transactions_api +from routstr.core.db import CashuTransaction + + +@pytest.mark.asyncio +async def test_transactions_api_excludes_internal_sweep_claim_timestamp() -> None: + transaction = CashuTransaction( + token="cashu-token", + amount=10, + unit="sat", + type="out", + sweep_started_at=123, + ) + count_result = MagicMock() + count_result.one.return_value = 1 + transactions_result = MagicMock() + transactions_result.all.return_value = [transaction] + session = MagicMock() + session.exec = AsyncMock(side_effect=[count_result, transactions_result]) + + @asynccontextmanager + async def create_session(): # type: ignore[no-untyped-def] + yield session + + with patch("routstr.core.admin.create_session", create_session): + response = await get_transactions_api() + + assert response["total"] == 1 + assert response["transactions"][0]["token"] == "cashu-token" + assert "sweep_started_at" not in response["transactions"][0] diff --git a/tests/unit/test_balances_by_mint_and_unit.py b/tests/unit/test_balances_by_mint_and_unit.py index bb1b3c81..248c43a8 100644 --- a/tests/unit/test_balances_by_mint_and_unit.py +++ b/tests/unit/test_balances_by_mint_and_unit.py @@ -13,7 +13,11 @@ from sqlalchemy.pool import StaticPool from sqlmodel import SQLModel from sqlmodel.ext.asyncio.session import AsyncSession -from routstr.core.db import ApiKey, balances_by_mint_and_unit +from routstr.core.db import ( + ApiKey, + balance_for_mint_and_unit, + balances_by_mint_and_unit, +) def _make_engine() -> AsyncEngine: @@ -92,6 +96,17 @@ async def test_excludes_rows_with_null_mint_or_currency(session: AsyncSession) - assert result == {("http://m1", "sat"): 1000} +@pytest.mark.asyncio +async def test_scalar_balance_for_one_mint_and_unit(session: AsyncSession) -> None: + await _add_key(session, "a", 1000, "http://m1", "sat") + await _add_key(session, "b", 500, "http://m1", "sat") + await _add_key(session, "c", 9000, "http://m1", "msat") + await _add_key(session, "d", 700, "http://m2", "sat") + + assert await balance_for_mint_and_unit(session, "http://m1", "sat") == 1500 + assert await balance_for_mint_and_unit(session, "http://missing", "sat") == 0 + + @pytest.mark.asyncio async def test_empty_inputs_return_empty_mapping(session: AsyncSession) -> None: await _add_key(session, "a", 1000, "http://m1", "sat") diff --git a/tests/unit/test_db_pool_config.py b/tests/unit/test_db_pool_config.py index a2838621..8f5d0367 100644 --- a/tests/unit/test_db_pool_config.py +++ b/tests/unit/test_db_pool_config.py @@ -1,4 +1,3 @@ -from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest @@ -43,17 +42,44 @@ async def test_memory_sqlite_keeps_static_pool( await engine.dispose() -def test_pool_observer_warns_when_connection_is_held_too_long( +def test_non_sqlite_backend_enables_pre_ping_automatically( monkeypatch: pytest.MonkeyPatch, ) -> None: - record = SimpleNamespace(info={}) - monotonic = MagicMock(side_effect=[100.0, 112.5]) - monkeypatch.setattr(db.time, "monotonic", monotonic) - monkeypatch.setattr(db, "_POOL_HOLD_WARN_SECONDS", 10.0) + monkeypatch.setattr(settings, "database_pool_pre_ping", False) + fake_engine = MagicMock() - with patch.object(db.logger, "warning") as warning: - db._record_pool_checkout(None, record, None) - db._record_pool_checkin(None, record) + with ( + patch.object(db, "create_async_engine", return_value=fake_engine) as factory, + patch.object(db.event, "listen") as listen, + ): + created = create_db_engine("postgresql+asyncpg://user:pass@db/node") - warning.assert_called_once() - assert warning.call_args.kwargs["extra"]["held_seconds"] == 12.5 + assert created is fake_engine + assert factory.call_args.kwargs["pool_pre_ping"] is True + assert listen.call_count == 2 + + +@pytest.mark.asyncio +async def test_every_created_engine_warns_for_long_checkouts( + monkeypatch: pytest.MonkeyPatch, tmp_path: object +) -> None: + monkeypatch.setattr(settings, "database_pool_hold_warn_seconds", 0.0) + monkeypatch.setattr(settings, "database_pool_pre_ping", False) + first = create_db_engine(f"sqlite+aiosqlite:///{tmp_path}/first.db") + second = create_db_engine(f"sqlite+aiosqlite:///{tmp_path}/second.db") + + try: + with patch.object(db.logger, "warning") as warning: + async with first.connect() as connection: + await connection.exec_driver_sql("SELECT 1") + async with second.connect() as connection: + await connection.exec_driver_sql("SELECT 1") + + assert warning.call_count == 2 + assert all( + call.kwargs["extra"]["threshold_seconds"] == 0.0 + for call in warning.call_args_list + ) + finally: + await first.dispose() + await second.dispose() diff --git a/tests/unit/test_fee_payout_crash_safety.py b/tests/unit/test_fee_payout_crash_safety.py index 7a0cd94a..0768db16 100644 --- a/tests/unit/test_fee_payout_crash_safety.py +++ b/tests/unit/test_fee_payout_crash_safety.py @@ -252,6 +252,56 @@ async def test_fee_payout_keeps_checkpoint_when_send_outcome_is_unknown() -> Non critical.assert_called_once() +@pytest.mark.asyncio +@pytest.mark.parametrize("failure_site", ["session", "completion"]) +async def test_fee_payout_completion_failures_use_sent_checkpoint_alert( + failure_site: str, +) -> None: + session = Mock() + fee = SimpleNamespace( + accumulated_msats=5_000, + payout_in_progress_msats=0, + payout_started_at=None, + ) + completion = AsyncMock() + if failure_site == "session": + create_session = Mock( + side_effect=[ + _session_context(session), + _session_context(session), + RuntimeError("pool unavailable"), + ] + ) + else: + create_session = Mock(return_value=_session_context(session)) + completion.side_effect = RuntimeError("checkpoint unavailable") + + with ( + patch("routstr.auth.ROUTSTR_FEE_DEFAULT_PAYOUT", 1), + patch("routstr.auth.ROUTSTR_FEE_PAYOUT_INTERVAL_SECONDS", 1), + patch("routstr.auth.ROUTSTR_LN_ADDRESS", "fees@example.com"), + patch( + "routstr.wallet.asyncio.sleep", + AsyncMock(side_effect=[None, asyncio.CancelledError()]), + ), + patch("routstr.wallet.db.create_session", create_session), + patch("routstr.wallet.db.get_routstr_fee", AsyncMock(return_value=fee)), + patch("routstr.wallet.db.reset_routstr_fee", AsyncMock(return_value=True)), + patch("routstr.wallet.db.complete_routstr_fee_payout", completion), + patch("routstr.wallet.get_wallet", AsyncMock(return_value=Mock())), + patch("routstr.wallet.get_proofs_per_mint_and_unit", return_value=[]), + patch("routstr.wallet.raw_send_to_lnurl", AsyncMock(return_value=5)), + patch("routstr.wallet.logger.critical") as critical, + ): + with pytest.raises(asyncio.CancelledError): + await wallet.periodic_routstr_fee_payout() + + critical.assert_called_once() + assert critical.call_args.args[0] == ( + "Routstr fee payout sent but checkpoint was not completed" + ) + + @pytest.mark.asyncio async def test_fee_payout_releases_db_connection_during_send(tmp_path: object) -> None: """With pool_size=1, the payout must not hold a connection while the diff --git a/tests/unit/test_periodic_payout.py b/tests/unit/test_periodic_payout.py index 69d13ee8..953f06cb 100644 --- a/tests/unit/test_periodic_payout.py +++ b/tests/unit/test_periodic_payout.py @@ -77,8 +77,8 @@ async def test_periodic_payout_includes_primary_mint_not_in_cashu_mints() -> Non AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_by_mint_and_unit", - AsyncMock(return_value={}), + "routstr.wallet.db.balance_for_mint_and_unit", + AsyncMock(return_value=0), ), patch("routstr.wallet.raw_send_to_lnurl", raw_send), ): @@ -131,8 +131,8 @@ async def test_periodic_payout_releases_session_before_slow_mint_send() -> None: AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_by_mint_and_unit", - AsyncMock(return_value={}), + "routstr.wallet.db.balance_for_mint_and_unit", + AsyncMock(return_value=0), ), patch("routstr.wallet.raw_send_to_lnurl", AsyncMock(side_effect=raw_send)), ): @@ -173,8 +173,8 @@ async def test_periodic_payout_isolates_failing_mint() -> None: AsyncMock(side_effect=lambda proofs, wallet: proofs), ), patch( - "routstr.wallet.db.balances_by_mint_and_unit", - AsyncMock(return_value={}), + "routstr.wallet.db.balance_for_mint_and_unit", + AsyncMock(return_value=0), ), patch("routstr.wallet.raw_send_to_lnurl", raw_send), ): @@ -220,10 +220,11 @@ async def test_periodic_payout_handles_session_creation_failure() -> None: await periodic_payout() # The liability session is opened per mint/unit (sat + msat), and each - # failure is isolated to its own iteration rather than aborting the cycle. + # DB failure retains the cycle-specific alert wording while remaining + # isolated to its own iteration. assert create_session.call_count == 2 assert logger.error.call_count == 2 message = logger.error.call_args.args[0] extra = logger.error.call_args.kwargs["extra"] - assert message == "Error sending payout: RuntimeError" + assert message == "Error in periodic payout cycle: RuntimeError" assert extra["error"] == "db unavailable" diff --git a/tests/unit/test_refund_sweep.py b/tests/unit/test_refund_sweep.py index 3a8a68e2..b43c3152 100644 --- a/tests/unit/test_refund_sweep.py +++ b/tests/unit/test_refund_sweep.py @@ -9,6 +9,7 @@ from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from sqlmodel import SQLModel, select from sqlmodel.ext.asyncio.session import AsyncSession +from routstr import wallet from routstr.core.db import CashuTransaction from routstr.wallet import refund_sweep_once @@ -185,6 +186,116 @@ async def test_refund_sweep_releases_claim_on_cancellation( assert refund.sweep_started_at is None +@pytest.mark.asyncio +async def test_checkpoint_failure_retains_claim_and_stale_retry_records_sweep( + session_factory: async_sessionmaker[AsyncSession], +) -> None: + await _insert( + session_factory, + CashuTransaction( + token="checkpoint-failure", + amount=1, + unit="sat", + type="out", + created_at=800, + ), + ) + real_set_state = wallet._set_refund_sweep_state + + async def fail_swept_checkpoint( + refund_id: str, + *, + predicates: tuple[object, ...] = (), + **values: object, + ) -> int: + if values.get("swept") is True: + raise RuntimeError("checkpoint unavailable") + return await real_set_state(refund_id, predicates=predicates, **values) + + with ( + patch("routstr.wallet.db.create_session", side_effect=session_factory), + patch("routstr.wallet.settings.refund_sweep_ttl_seconds", 100), + patch("routstr.wallet.settings.refund_sweep_claim_timeout_seconds", 200), + patch("routstr.wallet.time.time", return_value=1000), + patch( + "routstr.wallet.recieve_token", AsyncMock(return_value=(1, "sat", "mint")) + ), + patch( + "routstr.wallet._set_refund_sweep_state", + side_effect=fail_swept_checkpoint, + ), + patch("routstr.wallet.logger.critical") as critical, + ): + await refund_sweep_once() + + retained = (await _load(session_factory))["checkpoint-failure"] + assert retained.swept is False + assert retained.collected is False + assert retained.sweep_started_at == 1000 + critical.assert_called_once() + + with ( + patch("routstr.wallet.db.create_session", side_effect=session_factory), + patch("routstr.wallet.settings.refund_sweep_ttl_seconds", 100), + patch("routstr.wallet.settings.refund_sweep_claim_timeout_seconds", 200), + patch("routstr.wallet.time.time", return_value=1300), + patch( + "routstr.wallet.recieve_token", + AsyncMock(side_effect=RuntimeError("token already spent")), + ), + ): + await refund_sweep_once() + + recovered = (await _load(session_factory))["checkpoint-failure"] + assert recovered.swept is True + assert recovered.collected is False + assert recovered.sweep_started_at is None + + +@pytest.mark.asyncio +@pytest.mark.parametrize("redemption_succeeds", [True, False]) +async def test_expired_worker_cannot_overwrite_or_release_newer_claim( + session_factory: async_sessionmaker[AsyncSession], + redemption_succeeds: bool, +) -> None: + await _insert( + session_factory, + CashuTransaction( + token="reclaimed", + amount=1, + unit="sat", + type="out", + created_at=800, + ), + ) + + async def replace_claim(_token: str) -> tuple[int, str, str]: + async with session_factory() as session: + result = await session.exec( + select(CashuTransaction).where(CashuTransaction.token == "reclaimed") + ) + transaction = result.one() + transaction.sweep_started_at = 1100 + session.add(transaction) + await session.commit() + if not redemption_succeeds: + raise RuntimeError("mint unavailable") + return (1, "sat", "mint") + + with ( + patch("routstr.wallet.db.create_session", side_effect=session_factory), + patch("routstr.wallet.settings.refund_sweep_ttl_seconds", 100), + patch("routstr.wallet.time.time", return_value=1000), + patch("routstr.wallet.recieve_token", AsyncMock(side_effect=replace_claim)), + ): + await refund_sweep_once() + + reclaimed = (await _load(session_factory))["reclaimed"] + assert reclaimed.swept is False + assert reclaimed.collected is False + assert reclaimed.sweep_started_at == 1100 + + @pytest.mark.asyncio async def test_refund_sweep_recovers_stale_claim_without_misreporting_collection( session_factory: async_sessionmaker[AsyncSession], diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 965be8e6..98a9e24f 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -62,13 +62,13 @@ def test_payout_settings_have_sensible_defaults() -> None: assert s.payout_interval_seconds == 900 -def test_database_pool_defaults_are_bounded_and_fail_fast() -> None: +def test_database_pool_defaults_match_sqlalchemy_capacity() -> None: s = Settings() assert s.database_pool_size == 5 - assert s.database_max_overflow == 0 - assert s.database_pool_timeout == 5.0 + assert s.database_max_overflow == 10 + assert s.database_pool_timeout == 30.0 assert s.database_pool_recycle == 1800 - assert s.database_pool_pre_ping is True + assert s.database_pool_pre_ping is False assert s.database_pool_hold_warn_seconds == 10.0 From 5ea5024608d0f5e4ec8d1c02179d9fb69a68134c Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Wed, 29 Jul 2026 22:50:33 +0200 Subject: [PATCH 8/8] resolve review comments --- routstr/core/db.py | 3 +- routstr/lightning.py | 60 +++++++++++----- routstr/wallet.py | 40 +++++------ .../test_lightning_invoice_constraints.py | 59 +++++++++++++-- tests/unit/test_fee_payout_crash_safety.py | 39 ++++++++++ tests/unit/test_refund_sweep.py | 72 ++++++++++++++++--- 6 files changed, 217 insertions(+), 56 deletions(-) diff --git a/routstr/core/db.py b/routstr/core/db.py index 9e0c5973..12dcc659 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -382,7 +382,8 @@ class LightningInvoice(SQLModel, table=True): # type: ignore description: str = Field(description="Invoice description") payment_hash: str = Field(description="Payment hash for tracking", unique=True) status: str = Field( - default="pending", description="pending, paid, expired, cancelled" + default="pending", + description="pending, paid, expired, cancelled, reconciliation_required", ) api_key_hash: str | None = Field( default=None, description="Associated API key hash for topup operations" diff --git a/routstr/lightning.py b/routstr/lightning.py index 554925f2..d696403a 100644 --- a/routstr/lightning.py +++ b/routstr/lightning.py @@ -5,6 +5,7 @@ import time from fastapi import APIRouter, Depends, Header, HTTPException from pydantic import BaseModel, Field +from sqlalchemy.orm.attributes import set_committed_value from sqlmodel import col, select, update from sqlmodel.ext.asyncio.session import AsyncSession @@ -233,25 +234,46 @@ async def check_invoice_payment( # potentially slow mint request so it cannot pin a pool connection. await session.commit() + wallet = await get_wallet(settings.primary_mint, "sat") + mint_status = await wallet.get_mint_quote(invoice_payment_hash) + if not mint_status.paid: + return + # Do not redeem a paid top-up quote if its target has already been # pruned. This validation owns a short-lived session and releases its - # connection before any mint I/O starts. + # connection before mint redemption starts. if invoice_purpose == "topup": if not finalized_api_key_hash: raise ValueError("No API key associated with topup invoice") async with create_session() as validation_session: target = await validation_session.get(ApiKey, finalized_api_key_hash) - if target is None: - logger.error( - "Topup invoice target API key was not found; skipping mint", - extra={"invoice_id": invoice_id}, - ) - return - - wallet = await get_wallet(settings.primary_mint, "sat") - mint_status = await wallet.get_mint_quote(invoice_payment_hash) - if not mint_status.paid: - return + if target is None: + terminal = await validation_session.exec( # type: ignore[call-overload] + update(LightningInvoice) + .where( + col(LightningInvoice.id) == invoice_id, + col(LightningInvoice.status) == "pending", + ) + .values(status="reconciliation_required") + ) + await validation_session.commit() + if terminal.rowcount == 1: + set_committed_value( + invoice, "status", "reconciliation_required" + ) + else: + committed_invoice = await validation_session.get( + LightningInvoice, invoice_id + ) + if committed_invoice is not None: + set_committed_value( + invoice, "status", committed_invoice.status + ) + logger.critical( + "Paid topup invoice target API key was not found; reconciliation required", + extra={"invoice_id": invoice_id}, + ) + return # The mint enforces single-use quotes, so a concurrent checker that # races us here fails inside wallet.mint rather than double-minting. @@ -294,17 +316,19 @@ async def check_invoice_payment( # A concurrent finalizer won the CAS. Publish only the # state observed from the database after ending the owned # read transaction; never refresh the caller's session. - invoice.api_key_hash = committed_invoice.api_key_hash - invoice.status = committed_invoice.status - invoice.paid_at = committed_invoice.paid_at + set_committed_value( + invoice, "api_key_hash", committed_invoice.api_key_hash + ) + set_committed_value(invoice, "status", committed_invoice.status) + set_committed_value(invoice, "paid_at", committed_invoice.paid_at) return await finalization_session.commit() # Only publish finalized values to the caller-owned object after the # owned transaction has committed successfully. - invoice.api_key_hash = finalized_api_key_hash - invoice.status = "paid" - invoice.paid_at = paid_at + set_committed_value(invoice, "api_key_hash", finalized_api_key_hash) + set_committed_value(invoice, "status", "paid") + set_committed_value(invoice, "paid_at", paid_at) logger.info( "Lightning invoice paid", diff --git a/routstr/wallet.py b/routstr/wallet.py index e2a20a7c..aed99f92 100644 --- a/routstr/wallet.py +++ b/routstr/wallet.py @@ -1116,12 +1116,12 @@ async def _refund_sweep_once(cutoff: int) -> None: extra={"id": refund.id}, ) except BaseException as e: - if redeemed: - # The token is already in the node wallet. Retain the claim so - # a stale retry classifies "already spent" as swept, never as a - # client collection. + if redeemed or isinstance(e, TokenConsumedError): + # The token was spent, or the redemption outcome is known to be + # post-spend. Retain the claim so a stale retry classifies + # "already spent" as swept, never as a client collection. logger.critical( - "Refund token swept but checkpoint was not completed; manual reconciliation required", + "Refund token spent but sweep checkpoint was not completed; manual reconciliation required", extra={"id": refund.id}, exc_info=isinstance(e, Exception), ) @@ -1163,25 +1163,17 @@ async def _refund_sweep_once(cutoff: int) -> None: extra={"id": refund.id}, ) else: - # Cancellation or a transient pre-redemption failure is a - # known-no-send outcome, so release the lease for a later retry. - released = await asyncio.shield( - _set_refund_sweep_state( - refund.id, - predicates=(claim_owned,), - sweep_started_at=None, - ) + # Once redemption starts, an exception cannot prove the token + # was not spent (for example, a melt may land before the + # response is lost). Retain the claim so a stale retry treats + # an "already spent" result as a completed sweep. + logger.critical( + "Refund token redemption outcome is unknown; retaining sweep claim for reconciliation", + extra={"id": refund.id, "error": str(e)}, + exc_info=isinstance(e, Exception), ) if not isinstance(e, Exception): raise - logger.warning( - "Failed to sweep refund", - extra={ - "id": refund.id, - "error": str(e), - "claim_released": released == 1, - }, - ) async def refund_sweep_once() -> None: @@ -1254,12 +1246,14 @@ async def periodic_routstr_fee_payout() -> None: "sat", amount=accumulated_sats, ) - except Exception: + except BaseException as e: logger.critical( "Routstr fee payout outcome is unknown; manual reconciliation required", extra={"payout_in_progress_msats": paid_msats}, - exc_info=True, + exc_info=isinstance(e, Exception), ) + if not isinstance(e, Exception): + raise continue try: diff --git a/tests/integration/test_lightning_invoice_constraints.py b/tests/integration/test_lightning_invoice_constraints.py index b4e12a7f..079954ce 100644 --- a/tests/integration/test_lightning_invoice_constraints.py +++ b/tests/integration/test_lightning_invoice_constraints.py @@ -254,6 +254,40 @@ async def test_failed_mint_keeps_invoice_pending_for_retry( assert stored.status == "pending" +@pytest.mark.asyncio +async def test_unpaid_topup_does_not_query_target_key( + integration_engine: AsyncEngine, + patched_db_engine: None, +) -> None: + invoice = _make_invoice( + id="inv_unpaid_topup", + status="pending", + paid_at=None, + purpose="topup", + api_key_hash="target-key", + ) + async with AsyncSession(integration_engine, expire_on_commit=False) as setup: + setup.add(invoice) + await setup.commit() + + wallet = MagicMock() + wallet.get_mint_quote = AsyncMock(return_value=MagicMock(paid=False)) + create_session = MagicMock(side_effect=RuntimeError("target lookup should not run")) + async with AsyncSession(integration_engine, expire_on_commit=False) as session: + stored = await session.get(LightningInvoice, invoice.id) + assert stored is not None + with ( + patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)), + patch("routstr.lightning.create_session", create_session), + ): + from routstr.lightning import check_invoice_payment + + await check_invoice_payment(stored, session) + + wallet.get_mint_quote.assert_awaited_once_with(invoice.payment_hash) + create_session.assert_not_called() + + @pytest.mark.asyncio async def test_missing_topup_target_is_rejected_before_mint( integration_engine: AsyncEngine, @@ -265,25 +299,36 @@ async def test_missing_topup_target_is_rejected_before_mint( paid_at=None, purpose="topup", api_key_hash="pruned-key", + expires_at=int(time.time()) - 1, ) async with AsyncSession(integration_engine, expire_on_commit=False) as setup: setup.add(invoice) await setup.commit() - get_wallet = AsyncMock() + wallet = MagicMock() + wallet.get_mint_quote = AsyncMock(return_value=MagicMock(paid=True)) + wallet.mint = AsyncMock() async with AsyncSession(integration_engine, expire_on_commit=False) as session: stored = await session.get(LightningInvoice, invoice.id) assert stored is not None - with patch("routstr.lightning.get_wallet", get_wallet): - from routstr.lightning import check_invoice_payment + with ( + patch("routstr.lightning.get_wallet", AsyncMock(return_value=wallet)), + patch("routstr.lightning.logger.critical") as critical, + ): + from routstr.lightning import get_invoice_status - await check_invoice_payment(stored, session) + response = await get_invoice_status(invoice.id, session) - get_wallet.assert_not_awaited() + assert response.status == "reconciliation_required" + assert stored.status == "reconciliation_required" + assert stored not in session.dirty + critical.assert_called_once() + + wallet.mint.assert_not_awaited() async with AsyncSession(integration_engine) as verify: stored = await verify.get(LightningInvoice, invoice.id) assert stored is not None - assert stored.status == "pending" + assert stored.status == "reconciliation_required" @pytest.mark.asyncio @@ -424,6 +469,8 @@ async def test_db_guard_credits_once_when_both_mints_succeed( assert second_invoice.id == invoice.id assert first_invoice.status == "paid" assert second_invoice.status == "paid" + assert first_invoice not in first.dirty + assert second_invoice not in second.dirty assert wallet.mint.await_count == 2 async with AsyncSession(integration_engine, expire_on_commit=False) as verify: diff --git a/tests/unit/test_fee_payout_crash_safety.py b/tests/unit/test_fee_payout_crash_safety.py index 0768db16..ae7d0788 100644 --- a/tests/unit/test_fee_payout_crash_safety.py +++ b/tests/unit/test_fee_payout_crash_safety.py @@ -252,6 +252,45 @@ async def test_fee_payout_keeps_checkpoint_when_send_outcome_is_unknown() -> Non critical.assert_called_once() +@pytest.mark.asyncio +async def test_fee_payout_cancellation_during_send_alerts_and_propagates() -> None: + session = Mock() + fee = SimpleNamespace( + accumulated_msats=5_000, + payout_in_progress_msats=0, + payout_started_at=None, + ) + complete = AsyncMock() + + with ( + patch("routstr.auth.ROUTSTR_FEE_DEFAULT_PAYOUT", 1), + patch("routstr.auth.ROUTSTR_FEE_PAYOUT_INTERVAL_SECONDS", 1), + patch("routstr.auth.ROUTSTR_LN_ADDRESS", "fees@example.com"), + patch("routstr.wallet.asyncio.sleep", AsyncMock(return_value=None)), + patch( + "routstr.wallet.db.create_session", return_value=_session_context(session) + ), + patch("routstr.wallet.db.get_routstr_fee", AsyncMock(return_value=fee)), + patch("routstr.wallet.db.reset_routstr_fee", AsyncMock(return_value=True)), + patch("routstr.wallet.db.complete_routstr_fee_payout", complete), + patch("routstr.wallet.get_wallet", AsyncMock(return_value=Mock())), + patch("routstr.wallet.get_proofs_per_mint_and_unit", return_value=[]), + patch( + "routstr.wallet.raw_send_to_lnurl", + AsyncMock(side_effect=asyncio.CancelledError()), + ), + patch("routstr.wallet.logger.critical") as critical, + ): + with pytest.raises(asyncio.CancelledError): + await wallet.periodic_routstr_fee_payout() + + complete.assert_not_awaited() + critical.assert_called_once() + assert critical.call_args.args[0] == ( + "Routstr fee payout outcome is unknown; manual reconciliation required" + ) + + @pytest.mark.asyncio @pytest.mark.parametrize("failure_site", ["session", "completion"]) async def test_fee_payout_completion_failures_use_sent_checkpoint_alert( diff --git a/tests/unit/test_refund_sweep.py b/tests/unit/test_refund_sweep.py index b43c3152..f2937936 100644 --- a/tests/unit/test_refund_sweep.py +++ b/tests/unit/test_refund_sweep.py @@ -130,14 +130,17 @@ async def test_refund_sweep_only_processes_expired_eligible_outgoing_tokens( @pytest.mark.asyncio @pytest.mark.parametrize( - ("error", "collected"), + ("error", "collected", "claim_started_at"), [ - (RuntimeError("token already spent"), True), - (RuntimeError("mint unavailable"), False), + (RuntimeError("token already spent"), True, None), + (RuntimeError("mint unavailable"), False, 1000), ], ) -async def test_refund_sweep_records_terminal_but_not_transient_failures( - session_factory: async_sessionmaker[AsyncSession], error: Exception, collected: bool +async def test_refund_sweep_records_spent_and_unknown_outcomes_safely( + session_factory: async_sessionmaker[AsyncSession], + error: Exception, + collected: bool, + claim_started_at: int | None, ) -> None: await _insert( session_factory, @@ -156,11 +159,64 @@ async def test_refund_sweep_records_terminal_but_not_transient_failures( refund = (await _load(session_factory))["refund"] assert refund.collected is collected assert refund.swept is False - assert refund.sweep_started_at is None + assert refund.sweep_started_at == claim_started_at @pytest.mark.asyncio -async def test_refund_sweep_releases_claim_on_cancellation( +async def test_post_spend_failure_retains_claim_and_stale_retry_records_sweep( + session_factory: async_sessionmaker[AsyncSession], +) -> None: + await _insert( + session_factory, + CashuTransaction( + token="post-spend-failure", + amount=1, + unit="sat", + type="out", + created_at=800, + ), + ) + with ( + patch("routstr.wallet.db.create_session", side_effect=session_factory), + patch("routstr.wallet.settings.refund_sweep_ttl_seconds", 100), + patch("routstr.wallet.settings.refund_sweep_claim_timeout_seconds", 200), + patch("routstr.wallet.time.time", return_value=1000), + patch( + "routstr.wallet.recieve_token", + AsyncMock( + side_effect=wallet.TokenConsumedError( + "Mint on primary failed after successful melt" + ) + ), + ), + ): + await refund_sweep_once() + + retained = (await _load(session_factory))["post-spend-failure"] + assert retained.swept is False + assert retained.collected is False + assert retained.sweep_started_at == 1000 + + with ( + patch("routstr.wallet.db.create_session", side_effect=session_factory), + patch("routstr.wallet.settings.refund_sweep_ttl_seconds", 100), + patch("routstr.wallet.settings.refund_sweep_claim_timeout_seconds", 200), + patch("routstr.wallet.time.time", return_value=1300), + patch( + "routstr.wallet.recieve_token", + AsyncMock(side_effect=RuntimeError("token already spent")), + ), + ): + await refund_sweep_once() + + recovered = (await _load(session_factory))["post-spend-failure"] + assert recovered.swept is True + assert recovered.collected is False + assert recovered.sweep_started_at is None + + +@pytest.mark.asyncio +async def test_refund_sweep_retains_claim_on_cancellation_during_redemption( session_factory: async_sessionmaker[AsyncSession], ) -> None: await _insert( @@ -183,7 +239,7 @@ async def test_refund_sweep_releases_claim_on_cancellation( refund = (await _load(session_factory))["cancelled"] assert refund.swept is False - assert refund.sweep_started_at is None + assert refund.sweep_started_at == 1000 @pytest.mark.asyncio