From cc88763a9dc2d5b37cb65777abb40e482f0282b8 Mon Sep 17 00:00:00 2001 From: Davide Grilli Date: Mon, 27 Jul 2026 00:30:48 +0200 Subject: [PATCH] Make "one active round" a DB invariant and record why a tx failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two schema changes the fixes in the following commits build on (BUGS.md B-09, B-04): ix_rounds_single_active is a unique index over the constant expression (1), restricted to the active statuses, so the table holds any number of closed rounds and only ever one live one. Rounds never overlapping was previously enforced only by a read-then-insert in open_new_round_if_needed, which two concurrent callers can both pass — and a second stuck "open" row blocks every future round forever, since get_active_round matches on status. The migration doesn't create that index blind: an instance that already has two active rounds (the very bug) would fail mid-migration with an opaque IntegrityError, so it first closes the stale duplicates and keeps the newest — which is what get_active_round was already doing silently. Verified against a DB seeded with an 'open' plus a 'closing' round. pending_transactions.failure_reason is for the reconciler added next: when it gives up on a transaction, an operator needs to see whether it was dropped or rejected. The PendingTransaction docstring now also documents the full status lifecycle (building -> pending -> confirmed | failed), since "building" is new and load-bearing. Co-Authored-By: Claude Opus 5 --- app/db/models.py | 39 ++++++++++++- ...re_reason_and_single_active_round_index.py | 57 +++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/8a1c4e7b2d90_add_failure_reason_and_single_active_round_index.py diff --git a/app/db/models.py b/app/db/models.py index e3e85ce..2ea24ce 100644 --- a/app/db/models.py +++ b/app/db/models.py @@ -1,6 +1,6 @@ from datetime import datetime, timezone -from sqlalchemy import BigInteger, ForeignKey, String, UniqueConstraint +from sqlalchemy import BigInteger, ForeignKey, Index, String, UniqueConstraint, text from sqlalchemy.orm import Mapped, mapped_column from app.db.base import Base @@ -39,9 +39,29 @@ class UtxoEvent(Base): spent_txid: Mapped[str | None] = mapped_column(String(64), default=None) +_ACTIVE_ROUND_STATUSES_SQL = "'open', 'closing', 'drawing', 'paying_out'" + + class Round(Base): __tablename__ = "rounds" + # At most one round may be active at a time. Rounds never overlap by design, + # but that was enforced only by a read-then-insert in + # rounds/service.open_new_round_if_needed, which two concurrent callers can + # both pass — and a second stuck "open" row blocks every future round forever + # (B-09). This is the database-level guarantee: a unique index over a constant + # expression, restricted to the active statuses, so the table can hold any + # number of closed rounds and only ever one live one. + __table_args__ = ( + Index( + "ix_rounds_single_active", + text("(1)"), + unique=True, + sqlite_where=text(f"status IN ({_ACTIVE_ROUND_STATUSES_SQL})"), + postgresql_where=text(f"status IN ({_ACTIVE_ROUND_STATUSES_SQL})"), + ), + ) + id: Mapped[int] = mapped_column(primary_key=True) status: Mapped[str] = mapped_column(String(16), default="open") opened_at: Mapped[datetime] = mapped_column(default=utcnow) @@ -102,7 +122,17 @@ class RoundConfig(Base): class PendingTransaction(Base): - """Single source of truth for the RBF timeout->bump->rebroadcast loop.""" + """Single source of truth for the RBF timeout->bump->rebroadcast loop, and the + row the reconciler (app/tx/reconcile.py) resolves against the chain. + + Status lifecycle: + building -> written before the tx is broadcast, so a crash between the two + leaves evidence instead of a silently-spent UTXO set (B-08). + pending -> broadcast, waiting for its 1st confirmation. + confirmed -> terminal, set by app/tx/confirmation.py. + failed -> terminal, set by the reconciler when the tx is gone from the + chain for good; its UTXOs have been released by then. + """ __tablename__ = "pending_transactions" @@ -116,8 +146,13 @@ class PendingTransaction(Base): raw_tx_hex: Mapped[str] = mapped_column(String) broadcast_at: Mapped[datetime] = mapped_column(default=utcnow) status: Mapped[str] = mapped_column(String(16), default="pending") + # The txid this row had *before* its most recent RBF bump (bump_fee rewrites + # current_txid in place). Despite the name reading forwards, it points + # backwards: current_txid is the replacement, this is what it replaced. replaced_by_txid: Mapped[str | None] = mapped_column(String(64), default=None) attempt_count: Mapped[int] = mapped_column(default=1) + # Why the reconciler gave up on this tx — operator-facing, only set on "failed". + failure_reason: Mapped[str | None] = mapped_column(String(128), default=None) updated_at: Mapped[datetime] = mapped_column(default=utcnow, onupdate=utcnow) diff --git a/migrations/versions/8a1c4e7b2d90_add_failure_reason_and_single_active_round_index.py b/migrations/versions/8a1c4e7b2d90_add_failure_reason_and_single_active_round_index.py new file mode 100644 index 0000000..796e930 --- /dev/null +++ b/migrations/versions/8a1c4e7b2d90_add_failure_reason_and_single_active_round_index.py @@ -0,0 +1,57 @@ +"""Add pending tx failure_reason and the single-active-round index + +Supports two fixes from BUGS.md: + +* B-04 — the reconciler (app/tx/reconcile.py) records *why* it abandoned a + transaction, so an operator can tell a dropped tx from a rejected one. +* B-09 — "at most one active round" becomes a database guarantee instead of a + read-then-insert that two concurrent callers could both pass. A unique index + over the constant expression (1), restricted to the active statuses: any number + of closed rounds, only ever one live one. + +The index creation is not blind: if an instance already has more than one active +round (the very bug this prevents), creating it would fail with an opaque +IntegrityError mid-migration. It closes the stale duplicates first, keeping the +newest — which is exactly what get_active_round was already doing silently. + +Revision ID: 8a1c4e7b2d90 +Revises: 6cb50b29f64c +Create Date: 2026-07-26 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "8a1c4e7b2d90" +down_revision: Union[str, Sequence[str], None] = "6cb50b29f64c" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + +_ACTIVE = "'open', 'closing', 'drawing', 'paying_out'" + + +def upgrade() -> None: + op.add_column( + "pending_transactions", sa.Column("failure_reason", sa.String(length=128), nullable=True) + ) + + connection = op.get_bind() + active_ids = [ + row[0] + for row in connection.execute( + sa.text(f"SELECT id FROM rounds WHERE status IN ({_ACTIVE}) ORDER BY id DESC") + ) + ] + for stale_id in active_ids[1:]: + connection.execute( + sa.text("UPDATE rounds SET status = 'closed' WHERE id = :id"), {"id": stale_id} + ) + + op.execute(f"CREATE UNIQUE INDEX ix_rounds_single_active ON rounds ((1)) WHERE status IN ({_ACTIVE})") + + +def downgrade() -> None: + op.execute("DROP INDEX IF EXISTS ix_rounds_single_active") + op.drop_column("pending_transactions", "failure_reason")