From e8a8a87704514dd6b2e60d0dc5ef975a5d0945a4 Mon Sep 17 00:00:00 2001 From: 9qeklajc Date: Wed, 26 Aug 2026 22:51:09 +0200 Subject: [PATCH] chore: trim redundant comments --- .env.example | 2 +- docs/adr/001-cashu-payment-safety.md | 21 ---------------- routstr/core/admin.py | 4 +-- routstr/core/db.py | 5 ---- routstr/core/exceptions.py | 2 -- routstr/core/settings.py | 4 --- routstr/lightning.py | 5 +--- routstr/mint.py | 1 - routstr/payment/lnurl.py | 25 ++++--------------- routstr/upstream/auto_topup.py | 13 ++-------- routstr/upstream/base.py | 10 +------- routstr/upstream/ppqai.py | 8 ++---- routstr/wallet.py | 21 ++-------------- tests/integration/conftest.py | 1 - .../integration/test_ppq_auto_topup_claim.py | 2 -- .../integration/test_wallet_authentication.py | 3 --- tests/integration/test_wallet_melt_restart.py | 2 -- .../unit/test_lnurl_amount_and_destination.py | 2 -- tests/unit/test_lnurl_melt_timeout.py | 1 - tests/unit/test_refund_no_retry.py | 2 -- 20 files changed, 15 insertions(+), 119 deletions(-) delete mode 100644 docs/adr/001-cashu-payment-safety.md diff --git a/.env.example b/.env.example index 5ad7a74d..265ca2c4 100644 --- a/.env.example +++ b/.env.example @@ -33,7 +33,7 @@ ROUTSTR_SECRET_KEY= # DATABASE_POOL_PRE_PING=false # Warn when a checkout is held this many seconds. # DATABASE_POOL_HOLD_WARN_SECONDS=10 -# Seconds a file-backed SQLite writer waits for the write lock (default: 30). +# SQLite write-lock timeout, in seconds. # DATABASE_BUSY_TIMEOUT=30 # SQLite serialises writes; increasing its pool can trade pool timeouts for # "database is locked" errors rather than increasing write throughput. diff --git a/docs/adr/001-cashu-payment-safety.md b/docs/adr/001-cashu-payment-safety.md deleted file mode 100644 index a2ed627f..00000000 --- a/docs/adr/001-cashu-payment-safety.md +++ /dev/null @@ -1,21 +0,0 @@ -# ADR-001: Cashu payment safety boundaries - -## Status - -Accepted - -## Context - -Cashu proofs are bearer instruments. Retrying quote creation, refund delivery, or a dispatched Lightning melt can duplicate side effects or spend proofs whose outcome is still unknown. Mint transport failures and concurrent workers also need one shared policy. - -## Decision - -- Treat account, invoice, quote, melt, token-delivery, and refund creation as non-idempotent unless an upstream idempotency key is available. -- A dispatched melt with an unknown outcome keeps a durable quote-linked proof reservation until later reconciliation confirms a terminal state. An immediate `unpaid` observation after transport loss is not terminal. -- Size melts from the quote amount, reserve, and exact proof input fees within the caller's gross budget; do not use recursive send selection for melt planning. -- Apply mint transport/rate cooldowns centrally and permit only explicit reconciliation probes during cooldown. -- Auto-topups require fresh threshold confirmation, durable per-provider claims/cooldown, atomic spend-cap checks, and owner-only funds. - -## Consequences - -Transient failures can delay payouts/topups rather than risk duplicate payment. Operators may need to reconcile ambiguous claims. Tests must cover restart, concurrency, and partial-stream failures at these boundaries. diff --git a/routstr/core/admin.py b/routstr/core/admin.py index 2a4c7fb7..36a495a4 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -1367,9 +1367,7 @@ async def initiate_provider_topup( else {} ) - # This POST creates a Cashu mint quote upstream. Without an - # idempotency key, retrying a timeout or 5xx can create a - # second invoice while abandoning the first. + # Quote creation is unsafe to retry without idempotency. resp = await client.post( f"{clean_url}/v1/balance/lightning/invoice", json=request_json, diff --git a/routstr/core/db.py b/routstr/core/db.py index 76539c45..14cc669e 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -39,11 +39,6 @@ def create_db_engine(database_url: str = DATABASE_URL) -> AsyncEngine: options: dict[str, int | float | bool] = {"pool_pre_ping": pool_pre_ping} connect_args: dict[str, object] = {} if is_sqlite and not is_memory_sqlite: - # SQLite's default busy_timeout is only 5s, and aiosqlite does not set - # one of its own. Without this, concurrent payment-settlement writes - # across the pooled engine wait just 5s, then raise - # sqlite3.OperationalError: database is locked. Give writers a real - # chance to acquire the single SQLite write lock. connect_args["timeout"] = settings.database_busy_timeout if not is_memory_sqlite: options.update( diff --git a/routstr/core/exceptions.py b/routstr/core/exceptions.py index 7e319a94..a096455c 100644 --- a/routstr/core/exceptions.py +++ b/routstr/core/exceptions.py @@ -40,8 +40,6 @@ async def http_exception_handler(request: Request, exc: Exception) -> JSONRespon path = request.url.path # 4xx is client behaviour; the uvicorn access log already records it. - # Retryable mint outages are expected dependency failures, not application - # faults, so keep them visible without flooding the error stream. if status_code >= 500: error_type = None if isinstance(detail, dict): diff --git a/routstr/core/settings.py b/routstr/core/settings.py index 8f4caed8..7190f058 100644 --- a/routstr/core/settings.py +++ b/routstr/core/settings.py @@ -148,10 +148,6 @@ class Settings(BaseSettings): database_pool_hold_warn_seconds: float = Field( default=10.0, gt=0, env="DATABASE_POOL_HOLD_WARN_SECONDS" ) - # SQLite busy_timeout (seconds): how long a writer waits on a locked DB - # before raising "database is locked". Defaults to SQLite's 5s in stock - # aiosqlite; raise it so concurrent payment-settlement writes can queue - # instead of erroring. Referenced only by create_db_engine for SQLite. database_busy_timeout: float = Field( default=30.0, gt=0, env="DATABASE_BUSY_TIMEOUT" ) diff --git a/routstr/lightning.py b/routstr/lightning.py index 358e8515..13c6b321 100644 --- a/routstr/lightning.py +++ b/routstr/lightning.py @@ -225,8 +225,7 @@ async def _request_mint_with_fallback( lambda: wallet.request_mint(amount_sats), op_name="request_mint_invoice", mint_url=mint_url, - # Response loss may leave a valid quote at the mint. Creating a - # second quote is not a safe retry without an idempotency key. + # Quote creation is unsafe to retry without idempotency. retry_timeouts=False, retry_on_rate_limit=False, ) @@ -479,8 +478,6 @@ async def check_invoice_payment( await session.commit() mint_url = settlement.mint_url or settings.primary_mint - # Quote status is remote state and does not inspect local proofs. - # _mint_invoice_quote loads proofs exactly when settlement needs them. wallet = await get_wallet(mint_url, "sat", load_proofs=False) try: mint_status = await run_mint_operation( diff --git a/routstr/mint.py b/routstr/mint.py index 8948bd4f..b09b497c 100644 --- a/routstr/mint.py +++ b/routstr/mint.py @@ -239,7 +239,6 @@ def mint_cooldown_reason(mint_url: str) -> str | None: def is_mint_transport_error(error: BaseException) -> bool: - """Return whether an exception chain contains a mint transport failure.""" current: BaseException | None = error seen: set[int] = set() while current is not None and id(current) not in seen: diff --git a/routstr/payment/lnurl.py b/routstr/payment/lnurl.py index 3e391f8b..35f29f2a 100644 --- a/routstr/payment/lnurl.py +++ b/routstr/payment/lnurl.py @@ -107,7 +107,6 @@ async def _fetch_lnurl_json( def _contains_mint_transport_error(error: BaseException) -> bool: - """Detect transport failures wrapped by the Cashu wallet implementation.""" seen: set[int] = set() current: BaseException | None = error while current is not None and id(current) not in seen: @@ -321,15 +320,10 @@ async def raw_send_to_lnurl( f"({min_sendable_sat} - {max_sendable_sat} {unit})" ) - # Start at the caller's gross budget and converge downward from the mint's - # exact reserve plus NUT-02 input fees. Starting below the budget with a - # percentage heuristic silently underpays even when the exact fees are tiny. final_amount = amount_msat selected_proofs: list[Proof] | None = None - # Fee reserves can change with the invoice amount. Each quote reduces the - # candidate by its exact shortfall, so this bounded fixed-point search keeps - # the largest amount the gross budget can fund without Cashu coin selection. + # Find the largest amount covered by the budget after reserve and input fees. for _ in range(8): if final_amount < lnurl_data["min_sendable"]: raise LNURLError("Cashu melt fees leave no payable LNURL amount") @@ -340,7 +334,7 @@ async def raw_send_to_lnurl( lambda: wallet.melt_quote(invoice=bolt11_invoice), op_name="lnurl_melt_quote", mint_url=str(wallet.url), - # Creating another quote after response loss only abandons the first. + # Quote creation is unsafe to retry without idempotency. retry_timeouts=False, ) @@ -392,10 +386,7 @@ async def raw_send_to_lnurl( raise if not _contains_mint_transport_error(error): raise - # Cashu 0.20 clears melt reservations before wrapping transport errors - # in a plain Exception. Restore the durable melt association before - # asking for quote state so these proofs cannot be spent again while - # the Lightning outcome is unknown. + # Cashu clears reservations on transport errors despite an unknown outcome. try: await wallet.set_reserved_for_melt( proofs, reserved=True, quote_id=melt_quote_resp.quote @@ -414,8 +405,6 @@ async def raw_send_to_lnurl( if melt_state == MeltQuoteState.paid: return final_amount if melt_state == MeltQuoteState.unpaid: - # A direct unpaid response is authoritative: the mint rejected the melt - # and Cashu has already cleared its quote-linked reservation. await wallet.set_reserved_for_send(proofs, reserved=False) raise LNURLError("Cashu mint confirmed that the melt was unpaid") @@ -425,8 +414,7 @@ async def raw_send_to_lnurl( op_name="reconcile_lnurl_melt_quote", mint_url=str(wallet.url), retry_timeouts=False, - # One direct state lookup is required to reconcile the just-dispatched - # melt even though its transport failure opened the mint cooldown. + # Reconciliation must bypass the cooldown opened by this failure. allow_during_cooldown=True, ) except Exception as reconciliation_error: @@ -438,10 +426,7 @@ async def raw_send_to_lnurl( if quote is not None and quote.state == MeltQuoteState.paid: return final_amount if quote is not None and quote.state == MeltQuoteState.unpaid: - # Reaching reconciliation means melt was dispatched and either lost its - # response or returned pending. A just-dispatched quote can briefly read - # UNPAID before transitioning. Cashu clears the reservation while - # refreshing that state, so restore it and require later reconciliation. + # A just-dispatched quote can briefly report unpaid before transitioning. try: await wallet.set_reserved_for_melt( proofs, reserved=True, quote_id=melt_quote_resp.quote diff --git a/routstr/upstream/auto_topup.py b/routstr/upstream/auto_topup.py index 64d7d380..a73bdecf 100644 --- a/routstr/upstream/auto_topup.py +++ b/routstr/upstream/auto_topup.py @@ -366,9 +366,7 @@ async def _check_and_topup(row: UpstreamProviderRow) -> None: try: async with wallet_operation_guard(): - # The cap, owner-liability check, proof reservation, and outgoing - # audit row share one wallet mutation scope. The audit row must be - # durable before another worker can recheck the rolling cap. + # Keep the spend cap and audit mutation in one wallet lock. spent_24h_sats = await _routstr_spent_last_24h_sats() if spent_24h_sats + amount > ROUTSTR_MAX_DAILY_TOPUP_SATS: raise ValueError("Routstr auto top-up daily spend cap reached") @@ -420,9 +418,6 @@ async def _check_and_topup(row: UpstreamProviderRow) -> None: await _release_routstr_claim(row, operation_id) return - # The audit row and SENT claim committed together before this network call, - # so a worker crash cannot make reconciliation treat reserved proofs as an - # unspent CLAIMED attempt. result = await provider.topup(token) if "error" in result: @@ -713,7 +708,6 @@ async def _persist_routstr_token_and_mark_sent( amount: int, mint_url: str, ) -> None: - """Commit the bearer-token audit row and SENT claim atomically.""" state_id = _routstr_state_id(row) async with create_session() as session: state = await session.get(CashuTransaction, state_id) @@ -1123,8 +1117,6 @@ async def _set_ppq_state_terminal( .values( collected=collected, swept=swept, - # For successful payments this timestamps the durable cooldown, - # not merely when the original claim was created. created_at=int(time.time()) if collected else CashuTransaction.created_at, ) ) @@ -1513,8 +1505,7 @@ async def _check_and_topup_ppq(row: UpstreamProviderRow, settings: dict) -> None if balance >= threshold_usd: return - # A single stale/partial balance response must never create an invoice. - # Read the uncached endpoint again and require independent agreement. + # Require two low-balance reads before creating an invoice. confirmed_balance = await provider.get_balance() if ( confirmed_balance is None diff --git a/routstr/upstream/base.py b/routstr/upstream/base.py index e0f0a4f9..8606f201 100644 --- a/routstr/upstream/base.py +++ b/routstr/upstream/base.py @@ -1076,9 +1076,6 @@ class BaseUpstreamProvider: ) ) except Exception: - # Preserve the original stream exception. If the database - # cannot even be opened/read, stale-reservation cleanup is - # the only safe recovery path. logger.exception( "Fallback stream billing recovery could not access the database", extra={"key_hash": key.hashed_key[:8] + "..."}, @@ -1551,9 +1548,6 @@ class BaseUpstreamProvider: ) ) except Exception: - # Preserve the original stream exception. If the database - # cannot even be opened/read, stale-reservation cleanup is - # the only safe recovery path. logger.exception( "Fallback Responses billing recovery could not access the database", extra={"key_hash": key.hashed_key[:8] + "..."}, @@ -3686,9 +3680,7 @@ class BaseUpstreamProvider: ) try: - # send_token may perform an irreversible Cashu swap to make exact - # denominations. A blanket retry after response loss can dispatch - # a second swap, so this call is intentionally single-attempt. + # Token creation may swap proofs, so it is unsafe to retry. refund_token = await send_token(amount, unit=unit, mint_url=mint) except Exception as error: logger.error( diff --git a/routstr/upstream/ppqai.py b/routstr/upstream/ppqai.py index d373d8e8..0eec7bfe 100644 --- a/routstr/upstream/ppqai.py +++ b/routstr/upstream/ppqai.py @@ -24,7 +24,7 @@ _PPQ_CIRCUIT_COOLDOWN_SECONDS = 30.0 class PPQCircuitOpenError(RuntimeError): - """PPQ safe reads are suppressed until one probe is allowed.""" + pass @dataclass @@ -51,12 +51,10 @@ async def _safe_read_request( headers: dict[str, str], json: dict[str, object] | None = None, ) -> httpx.Response: - """Retry safe reads, then open one process-local circuit per PPQ origin.""" state = _ppq_circuits.setdefault(_ppq_origin(url), _PPQCircuitState()) loop = asyncio.get_running_loop() if state.loop is not loop: - # Runtime uses one long-lived loop; pytest and some embedded hosts do - # not. Preserve circuit state while replacing a loop-bound lock. + # Locks cannot be reused across event loops. state.lock = asyncio.Lock() state.loop = loop async with state.lock: @@ -323,8 +321,6 @@ class PPQAIUpstreamProvider(BaseUpstreamProvider): return models except Exception: - # The base refresh handler preserves the last good model cache when - # fetching raises; [] would look like a valid empty catalog. raise async def on_upstream_error_redirect( diff --git a/routstr/wallet.py b/routstr/wallet.py index 24072c74..0c0ecc7c 100644 --- a/routstr/wallet.py +++ b/routstr/wallet.py @@ -154,7 +154,6 @@ class Wallet(_CashuWallet): *, force_refresh: bool = False, ) -> None: - """Load metadata once per mint URL, then hydrate unit wallets locally.""" mint_url = str(self.url) lock = _mint_metadata_load_locks.setdefault(mint_url, asyncio.Lock()) async with lock: @@ -171,8 +170,6 @@ class Wallet(_CashuWallet): await self.load_mint_info(reload=False) return except Exception: - # An empty/stale local cache is not authoritative. Fall - # through to one remote refresh under the per-mint lock. pass await self.load_mint_keysets(force_old_keysets) @@ -593,7 +590,6 @@ async def send_token(amount: int, unit: str, mint_url: str | None = None) -> str async def send_token_from_owner_locked( amount: int, unit: str, mint_url: str | None = None ) -> str: - """Create an owner-funded token while the caller holds the wallet guard.""" _, token = await _send_locked(amount, unit, mint_url, owner_only=True) return token @@ -1866,10 +1862,7 @@ async def _credit_balance_locked( _wallets: dict[str, Wallet] = {} -# Proofs are local SQLite state and need a short refresh window because another -# worker process can reserve or spend them. Mint metadata is remote, shared by -# every operation on a wallet, and changes far less often; refreshing it on the -# proof cadence caused repeated /keysets, /keys, and /info requests. +# Proofs require a shorter refresh interval than remote mint metadata. _wallet_last_load: dict[str, float] = {} _wallet_last_mint_load: dict[str, float] = {} _wallet_load_locks: dict[str, asyncio.Lock] = {} @@ -1883,13 +1876,6 @@ async def get_wallet( force_reload: bool = False, load_proofs: bool = True, ) -> Wallet: - """Return a cached wallet, refreshing remote and local state independently. - - ``load=False`` remains the fully offline path. Quote-only callers can use - ``load_proofs=False``: mint metadata is initialized when needed, but local - proofs are not re-read when the operation cannot spend or inspect them. - ``force_reload`` still refreshes every requested layer immediately. - """ global _wallets, _wallet_last_load, _wallet_last_mint_load, _wallet_load_locks id = f"{mint_url}_{unit}" lock = _wallet_load_locks.setdefault(id, asyncio.Lock()) @@ -1925,7 +1911,6 @@ async def get_wallet( or now - last_proof_load >= _WALLET_PROOF_RELOAD_MIN_INTERVAL_SECONDS ): - # cashu's load_proofs is local SQLite I/O, not a mint call. await run_mint_operation( lambda: _wallets[id].load_proofs(reload=True), op_name="load_proofs", @@ -2005,9 +1990,7 @@ async def _get_supported_mint_units(mint_url: str) -> list[str]: if cached is not None and now < cached[0]: return cached[1] - # One full remote metadata load populates Cashu's shared SQLite keyset - # cache. Discover all advertised units from that cache instead of issuing a - # separate /keysets request before each unit wallet loads. + # A metadata load populates Cashu's shared keyset cache for all units. wallet = await get_wallet( mint_url, settings.primary_mint_unit, diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 8dcd1876..7848e38c 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -73,7 +73,6 @@ from routstr.mint import MintRateGuard # noqa: E402 @pytest.fixture(autouse=True) def isolate_mint_rate_guards() -> Iterator[None]: - """Do not let one integration test's simulated outage poison the next.""" MintRateGuard._guards.clear() yield MintRateGuard._guards.clear() diff --git a/tests/integration/test_ppq_auto_topup_claim.py b/tests/integration/test_ppq_auto_topup_claim.py index 82c33c6f..05b99c03 100644 --- a/tests/integration/test_ppq_auto_topup_claim.py +++ b/tests/integration/test_ppq_auto_topup_claim.py @@ -313,8 +313,6 @@ async def test_ppq_payment_audit_row_is_visible_and_survives_next_claim( assert audit["collected"] is True assert "lnbc-secret-invoice" not in audit["token"] - # The durable cooldown blocks an immediate duplicate, then the claim lock - # can be reused without overwriting audit history after it expires. assert await _claim_ppq_topup(_row()) is None async with create_session() as session: state = await session.get(CashuTransaction, _ppq_state_id_for_provider(1)) diff --git a/tests/integration/test_wallet_authentication.py b/tests/integration/test_wallet_authentication.py index 2054f0af..26407701 100644 --- a/tests/integration/test_wallet_authentication.py +++ b/tests/integration/test_wallet_authentication.py @@ -114,8 +114,6 @@ async def test_api_key_generation_invalid_token( async def test_duplicate_token_handling( integration_client: AsyncClient, testmint_wallet: Any, db_snapshot: Any ) -> None: - """Concurrent duplicate redemption credits once and deterministically replays.""" - amount = 500 token = await testmint_wallet.mint_tokens(amount) integration_client.headers["Authorization"] = f"Bearer {token}" @@ -134,7 +132,6 @@ async def test_duplicate_token_handling( assert api_key1 == api_key2 assert balance1 == balance2 == amount * 1000 - # The real DB contains one logical credit. A later replay is also mutation-free. await db_snapshot.capture() replay = await integration_client.get("/v1/wallet/info") assert replay.status_code == 200 diff --git a/tests/integration/test_wallet_melt_restart.py b/tests/integration/test_wallet_melt_restart.py index 6c28963c..9d75f542 100644 --- a/tests/integration/test_wallet_melt_restart.py +++ b/tests/integration/test_wallet_melt_restart.py @@ -95,7 +95,6 @@ async def test_melt_recovery_is_findable_by_quote_after_restart( async def test_paid_reconciliation_invalidates_recovered_proofs_after_restart( tmp_path: Path, ) -> None: - """Actual Wallet.get_melt_quote consumes quote-linked proofs after restart.""" wallet = await _wallet(tmp_path) await _seed_ambiguous_melt(wallet) @@ -143,7 +142,6 @@ async def test_send_style_reservation_would_not_be_reconcilable( async def test_unpaid_reconciliation_releases_recovered_proofs_after_restart( tmp_path: Path, ) -> None: - """Actual Wallet.get_melt_quote releases proofs after an unpaid answer.""" wallet = await _wallet(tmp_path) await _seed_ambiguous_melt(wallet) diff --git a/tests/unit/test_lnurl_amount_and_destination.py b/tests/unit/test_lnurl_amount_and_destination.py index 53cb13df..97737cf7 100644 --- a/tests/unit/test_lnurl_amount_and_destination.py +++ b/tests/unit/test_lnurl_amount_and_destination.py @@ -28,7 +28,6 @@ LNURL_DATA = { "max_sendable": 100_000_000, } -# The exact plan spends the 1000 sat gross budget as 999 sat + 1 sat reserve. EXPECTED_QUOTE_SAT = 999 @@ -142,7 +141,6 @@ async def test_raw_send_to_lnurl_accepts_exact_invoice() -> None: @pytest.mark.asyncio async def test_raw_send_to_lnurl_msat_unit_compares_in_wallet_unit() -> None: - # 1_000_000 msat gross minus the exact 1 msat reserve leaves 999_999 msat. wallet, proofs = _wallet() proofs[0].amount = 1_000_000 wallet.melt_quote = AsyncMock( diff --git a/tests/unit/test_lnurl_melt_timeout.py b/tests/unit/test_lnurl_melt_timeout.py index c517cbe2..ede3f5af 100644 --- a/tests/unit/test_lnurl_melt_timeout.py +++ b/tests/unit/test_lnurl_melt_timeout.py @@ -32,7 +32,6 @@ LNURL_DATA = { } -# Exact planning pays 999 sat from a 1000 sat budget with a 1 sat reserve. QUOTE_AMOUNT_SAT = 999 diff --git a/tests/unit/test_refund_no_retry.py b/tests/unit/test_refund_no_retry.py index f176755d..26ab0916 100644 --- a/tests/unit/test_refund_no_retry.py +++ b/tests/unit/test_refund_no_retry.py @@ -1,5 +1,3 @@ -"""Refund token issuance must not repeat an ambiguous Cashu swap.""" - from unittest.mock import AsyncMock, patch import httpx