diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b0600ba3..e4764f90 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -52,6 +52,46 @@ jobs: .coverage retention-days: 30 + postgres-compat: + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: routstr + POSTGRES_PASSWORD: routstr + POSTGRES_DB: routstr + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install uv + uses: astral-sh/setup-uv@v3 + + - name: Set up Python + run: uv python install 3.12 + + - name: Install dependencies + run: uv sync --python 3.12 --dev + + - name: Run PostgreSQL compatibility tests + env: + UPSTREAM_BASE_URL: "http://test" + UPSTREAM_API_KEY: "test" + ROUTSTR_TEST_POSTGRES_URL: "postgresql+asyncpg://routstr:routstr@127.0.0.1:5432/routstr" + run: | + uv run --python 3.12 pytest tests/integration/test_postgres_compatibility.py \ + --verbose --tb=short + ui-build: runs-on: ubuntu-latest diff --git a/docs/provider/configuration.md b/docs/provider/configuration.md index 6a47854d..e153e3e0 100644 --- a/docs/provider/configuration.md +++ b/docs/provider/configuration.md @@ -200,7 +200,7 @@ Use environment variables for: | `ADMIN_PASSWORD` | Legacy seed for the dashboard password (otherwise generated + logged on first start) | (auto-generated) | | `ROUTSTR_SECRET_KEY` | Master key encrypting node secrets at rest. Auto-generated to a key file if unset | (auto-generated) | | `ROUTSTR_SECRET_KEY_FILE` | Path to the generated key file (used when `ROUTSTR_SECRET_KEY` is unset) | `routstr_secret.key` beside the database | -| `DATABASE_URL` | Database connection string | `sqlite+aiosqlite:///keys.db` | +| `DATABASE_URL` | Database connection string. SQLite or PostgreSQL, e.g. `postgresql+asyncpg://user:pass@host:5432/routstr` | `sqlite+aiosqlite:///keys.db` | | `NAME` | Node display name | `ARoutstrNode` | | `DESCRIPTION` | Node description | `A Routstr Node` | | `NPUB` | Nostr public key (bech32) | — | @@ -264,6 +264,12 @@ decrypted and must be re-entered — there is no rotation. To keep the key off t data volume, set `ROUTSTR_SECRET_KEY` explicitly (an env value always takes precedence over the file). See also [Deployment](deployment.md). +**On PostgreSQL, set the key explicitly.** There is no local database file to sit +beside, so the generated key lands in the working directory. If that directory +isn't persisted, the key is gone on the next start and a node with a stored nsec +refuses to boot with `ROUTSTR_SECRET_KEY is not set`. Set `ROUTSTR_SECRET_KEY`, or +point `ROUTSTR_SECRET_KEY_FILE` at a persistent path. + --- ## Models diff --git a/routstr/core/admin.py b/routstr/core/admin.py index 365e2f68..09d63bc8 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -99,8 +99,8 @@ async def get_temporary_balances_api( if search: pattern = f"%{search}%" filters.append( - col(ApiKey.hashed_key).like(pattern) - | col(ApiKey.refund_address).like(pattern) + col(ApiKey.hashed_key).ilike(pattern) + | col(ApiKey.refund_address).ilike(pattern) ) async with create_session() as session: @@ -165,12 +165,15 @@ async def get_temporary_balances_api( for key in api_keys ], "total": total, + # PostgreSQL SUM() over BIGINT returns NUMERIC, which asyncpg hands back + # as Decimal and JSON-serializes as a string. Coerce so both backends + # return ints. "totals": { - "total_balance": total_balance, - "total_reserved_balance": total_reserved_balance, - "total_available_balance": total_available_balance, - "total_spent": total_spent, - "total_requests": total_requests, + "total_balance": int(total_balance), + "total_reserved_balance": int(total_reserved_balance), + "total_available_balance": int(total_available_balance), + "total_spent": int(total_spent), + "total_requests": int(total_requests), }, } @@ -2016,10 +2019,10 @@ async def get_transactions_api( if search: search_pattern = f"%{search}%" base = base.where( - (col(CashuTransaction.id).like(search_pattern)) - | (col(CashuTransaction.token).like(search_pattern)) - | (col(CashuTransaction.request_id).like(search_pattern)) - | (col(CashuTransaction.api_key_hashed_key).like(search_pattern)) + (col(CashuTransaction.id).ilike(search_pattern)) + | (col(CashuTransaction.token).ilike(search_pattern)) + | (col(CashuTransaction.request_id).ilike(search_pattern)) + | (col(CashuTransaction.api_key_hashed_key).ilike(search_pattern)) ) count_result = await session.exec( @@ -2069,10 +2072,10 @@ async def get_lightning_invoices_api( if search: pattern = f"%{search}%" base = base.where( - (col(LightningInvoice.id).like(pattern)) - | (col(LightningInvoice.bolt11).like(pattern)) - | (col(LightningInvoice.payment_hash).like(pattern)) - | (col(LightningInvoice.api_key_hash).like(pattern)) + (col(LightningInvoice.id).ilike(pattern)) + | (col(LightningInvoice.bolt11).ilike(pattern)) + | (col(LightningInvoice.payment_hash).ilike(pattern)) + | (col(LightningInvoice.api_key_hash).ilike(pattern)) ) count_result = await session.exec( diff --git a/routstr/core/db.py b/routstr/core/db.py index 725ef25f..428fabd8 100644 --- a/routstr/core/db.py +++ b/routstr/core/db.py @@ -1,15 +1,13 @@ import asyncio -import concurrent.futures import hashlib import os import pathlib import sqlite3 import time import uuid -from collections.abc import Callable, Coroutine from contextlib import asynccontextmanager from enum import Enum -from typing import Any, AsyncGenerator, TypeVar +from typing import AsyncGenerator from alembic import command from alembic.config import Config @@ -38,8 +36,6 @@ from .settings import settings logger = get_logger(__name__) -T = TypeVar("T") - DATABASE_URL = os.environ.get("DATABASE_URL", "sqlite+aiosqlite:///keys.db") # Money (millisatoshis), lifetime counters and unix timestamps are all 64-bit @@ -53,23 +49,6 @@ UnixTimestamp = BigInteger Counter = BigInteger -def _run_async_from_sync(factory: "Callable[[], Coroutine[Any, Any, T]]") -> T: - """Run one coroutine from sync code, in or out of a running event loop. - - Migrations are driven synchronously by Alembic but may be triggered from - inside FastAPI's loop, where ``asyncio.run`` would raise. Same shape as - ``migrations/env.py``: borrow a worker thread when a loop is already - running. The coroutine is built inside the target loop — an engine created - on one loop cannot be awaited on another. - """ - try: - asyncio.get_running_loop() - except RuntimeError: - return asyncio.run(factory()) - with concurrent.futures.ThreadPoolExecutor() as executor: - return executor.submit(lambda: asyncio.run(factory())).result() - - def create_db_engine(database_url: str = DATABASE_URL) -> AsyncEngine: """Build and instrument an async engine from environment-only settings.""" url = make_url(database_url) @@ -602,7 +581,7 @@ class CashuTransaction(SQLModel, table=True): # type: ignore description="Unique transaction identifier", ) token: str = Field(description="Serialized Cashu token") - amount: int = Field(sa_type=Msats, description="Amount in the token's unit") + amount: int = Field(sa_type=BigInteger, description="Amount in the token's unit") unit: str = Field(description="Token unit (sat or msat)") mint_url: str | None = Field(default=None, description="Mint URL for the token") type: str = Field(default="out", description="Transaction type: in or out") @@ -889,11 +868,6 @@ class NsecState(str, Enum): cleared = "cleared" -def _enum_values(enum_cls: type[Enum]) -> list[str]: - """Persist an enum by ``value``, matching rows written before it was typed.""" - return [str(member.value) for member in enum_cls] - - class Secret(SQLModel, table=True): # type: ignore """Node-level secrets, stored encrypted/hashed at rest (singleton, id=1). @@ -906,17 +880,20 @@ class Secret(SQLModel, table=True): # type: ignore id: int = Field(default=1, primary_key=True) admin_password_hash: str | None = Field(default=None) encrypted_nsec: str | None = Field(default=None) - # ``native_enum=False`` keeps this a VARCHAR on every backend. - # A bare Python Enum makes SQLAlchemy reach for a native PostgreSQL ENUM - # type named ``nsecstate``, which the migration never creates — so reading - # or writing the secrets singleton died with `type "nsecstate" does not - # exist`, taking node bootstrap with it. SQLite renders Enum as VARCHAR - # either way, which is why this stayed invisible until PostgreSQL. + # ``native_enum=False`` keeps this a VARCHAR on every backend, matching the + # column the migration actually creates. A bare Python Enum makes SQLAlchemy + # reach for a native PostgreSQL ENUM type named ``nsecstate``, which no + # migration creates: on a schema built by Alembic alone, reading or writing + # the secrets singleton fails with `type "nsecstate" does not exist`. A + # fresh node survived it only because ``init_db`` runs ``create_all`` right + # after the migrations and creates the type as a side effect. SQLite renders + # Enum as VARCHAR either way, which is why this stayed invisible until + # PostgreSQL. nsec_state: NsecState = Field( default=NsecState.legacy, sa_column=Column( "nsec_state", - SAEnum(NsecState, native_enum=False, values_callable=_enum_values), + SAEnum(NsecState, native_enum=False), nullable=False, ), ) @@ -1320,22 +1297,16 @@ def fix_cashu_migrations() -> None: def _clear_alembic_version() -> None: """Clear the alembic_version table so stamp/upgrade can proceed. - Driven through the configured async driver rather than a second sync engine. - The old code built one by stripping ``+aiosqlite`` from the URL, which left - ``postgresql+asyncpg`` intact and raised ``MissingGreenlet``; naming the - sync backend instead only moves the problem, since it then demands a - separate sync driver (psycopg2) that a PostgreSQL deployment need not have. + Callers are gated to SQLite, so stripping ``+aiosqlite`` is enough to get a + sync engine here. """ + sync_url = DATABASE_URL.replace("+aiosqlite", "") + from sqlalchemy import create_engine - async def clear() -> None: - eng = create_async_engine(DATABASE_URL) - try: - async with eng.begin() as conn: - await conn.execute(text("DELETE FROM alembic_version")) - finally: - await eng.dispose() - - _run_async_from_sync(clear) + eng = create_engine(sync_url) + with eng.begin() as conn: + conn.execute(text("DELETE FROM alembic_version")) + eng.dispose() def run_migrations() -> None: @@ -1362,10 +1333,9 @@ def run_migrations() -> None: try: command.upgrade(alembic_cfg, "head") except CommandError as e: - # Preserve legacy SQLite recovery only. PostgreSQL rolls back DDL - # transactionally: stamping head after an error can hide migrations - # that never committed. Unknown PostgreSQL revisions need operator - # reconciliation, not an automatic stamp. + # Preserve legacy SQLite recovery only. Blindly stamping head hides + # whatever the real failure was, so an unknown revision on any other + # backend needs operator reconciliation, not an automatic stamp. if make_url( DATABASE_URL ).get_backend_name() == "sqlite" and "Can't locate revision" in str(e): diff --git a/tests/integration/test_postgres_compatibility.py b/tests/integration/test_postgres_compatibility.py index d9f6b5be..171d38a1 100644 --- a/tests/integration/test_postgres_compatibility.py +++ b/tests/integration/test_postgres_compatibility.py @@ -1,4 +1,4 @@ -"""End-to-end PostgreSQL coverage for the main application DB (issue #45). +"""End-to-end PostgreSQL coverage for the main application DB. Runs against a real server so the things SQLite cannot express are actually exercised: native enum types, INT4 column widths, per-dialect ``ON CONFLICT``, @@ -186,8 +186,7 @@ def test_migrated_schema_matches_the_orm(migrated_postgres: str) -> None: _touches(diff, "settings") or _touches(diff, "cashu_transactions", "api_key_hashed_key") or _touches(diff, "cli_tokens", "token") - or "modify_type" in str(diff) - and "TEXT()" in str(diff) + or ("modify_type" in str(diff) and "TEXT()" in str(diff)) ) ] assert unexplained == [], unexplained @@ -849,24 +848,6 @@ async def test_dead_keys_are_pruned(pg_session: AsyncSession) -> None: assert await pg_session.get(ApiKey, "alive") is not None -@pytest.mark.asyncio -async def test_version_clear_uses_async_driver(db_bound_to_postgres: Any) -> None: - db_module = db_bound_to_postgres - # Direct invocation really exercises the running-loop branch. - db_module._clear_alembic_version() - async with db_module.engine.connect() as conn: - assert ( - await conn.execute(text("SELECT count(*) FROM alembic_version")) - ).scalar_one() == 0 - # Explicit stamp is justified only here: this fixture is known to be at head. - await asyncio.to_thread(_alembic, "stamp", "head") - await asyncio.to_thread(db_module._clear_alembic_version) - async with db_module.engine.connect() as conn: - assert ( - await conn.execute(text("SELECT count(*) FROM alembic_version")) - ).scalar_one() == 0 - - @pytest.mark.asyncio @pytest.mark.parametrize("unknown_revision", [False, True]) async def test_postgres_migration_errors_never_stamp_head( diff --git a/tests/unit/test_postgres_schema_compat.py b/tests/unit/test_postgres_schema_compat.py index f0229f1a..62b74be2 100644 --- a/tests/unit/test_postgres_schema_compat.py +++ b/tests/unit/test_postgres_schema_compat.py @@ -3,7 +3,7 @@ These run without a PostgreSQL server: they compile the real SQLModel metadata against the PostgreSQL dialect and assert the properties that SQLite silently papered over. Each test here corresponds to a defect that was live on -PostgreSQL while every SQLite test stayed green (issue #45). +PostgreSQL while every SQLite test stayed green. The end-to-end proof against a real server lives in ``tests/integration/test_postgres_compatibility.py``. @@ -225,19 +225,6 @@ def test_model_path_upsert_is_built_for_the_bound_dialect() -> None: ) -def test_clear_alembic_version_uses_the_configured_async_driver() -> None: - """The recovery path must not need a second, sync-only driver. - - It used to build a sync engine from ``DATABASE_URL.replace("+aiosqlite", "")``, - which left ``postgresql+asyncpg`` intact and raised ``MissingGreenlet``. - """ - import inspect - - source = inspect.getsource(db_module._clear_alembic_version) - assert "create_engine(" not in source.replace("create_async_engine(", "") - assert 'replace("+aiosqlite"' not in source - - def test_sqlite_only_maintenance_stays_scoped_to_the_wallet_directory() -> None: """PRAGMA/sqlite3 handling in db.py may only touch ``.wallet/*.sqlite3``.""" import inspect