laantungir/routstr-core
1
0
Fork 0
mirror of https://github.com/Routstr/routstr-core.git synced 2026-10-05 12:28:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: address PostgreSQL review findings and run the PG suite in CI

Browse Source
This commit is contained in:
9qeklajc
2026-09-30 22:49:47 +02:00
parent 250864b45d
commit cc28b86080
6 changed files with 91 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/test.yml
+40
View File
@@ -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
docs/provider/configuration.md
+7 -1
View File
@@ -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
routstr/core/admin.py
+18 -15
View File
@@ -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(
routstr/core/db.py
+23 -53
View File
@@ -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):
tests/integration/test_postgres_compatibility.py
+2 -21
View File
@@ -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(
tests/unit/test_postgres_schema_compat.py
+1 -14
View File
@@ -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