fix(cassettes): order state events by created_at, not by one remembered id
The gate on the ATM-state consumer compared the incoming event id against the id stored on a single arbitrary row (SELECT ... LIMIT 1, no ORDER BY). That is a one-event memory, not a watermark: a re-delivered A, B, A applied three times. Worse, created_at was parsed, written to state_at and then never compared, so an event arriving late overwrote newer state — nothing in the path ever looked at the clock. Events are now applied only when strictly newer than the OLDEST state stamp on file. Strict '>' subsumes replay dedup, since a replay carries the same stamp. Oldest rather than newest is deliberate: every execute in this data layer commits on its own, so a multi-row apply cannot be made atomic here, and gating on the oldest means a crash mid-apply is re-applied on the next event instead of being mistaken for a complete one. The ATM republishes on a heartbeat, so it converges. Stamps are compared as unix floats because SQLite returns integers, Postgres returns timestamps and the incoming value is tz-aware; comparing raw would either raise or quietly mislead. An unparseable incoming stamp fails closed. Also renames apply_bootstrap_state to apply_reported_state and corrects the module comments. There has never been a once-per-machine guard, so calling it a one-shot bootstrap consumer described something the code did not do. Refs #43 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
08eea1ba16
commit
27449e1d11
4 changed files with 144 additions and 85 deletions
|
|
@ -5,11 +5,11 @@ Covers the pure pieces that don't need a live DB:
|
|||
- Pydantic validator behaviour on PublishCassettesPayload + the row /
|
||||
upsert models (position key coercion, integer ranges, multiple-same-
|
||||
denomination payloads, wire-format round-trip)
|
||||
- _should_apply_bootstrap_state dedup helper (extracted from
|
||||
apply_bootstrap_state so the relay-re-delivery decision is testable
|
||||
without a database round-trip)
|
||||
- _should_apply_state_event ordering gate (extracted from
|
||||
apply_reported_state so the decision is testable without a database
|
||||
round-trip)
|
||||
|
||||
DB-touching tests (apply_bootstrap_state actually upserting, list-by-
|
||||
DB-touching tests (apply_reported_state actually reconciling, list-by-
|
||||
machine ordering, etc.) follow the project convention from
|
||||
test_deposit_currency.py: "Layer 2 is an endpoint-level behaviour better
|
||||
covered by an integration test against a running LNbits; tracked in #26
|
||||
|
|
@ -21,9 +21,11 @@ denomination + count are operator-editable per row, multiple same-denom
|
|||
cassettes are valid.
|
||||
"""
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
import pytest
|
||||
|
||||
from ..crud import _should_apply_bootstrap_state
|
||||
from ..crud import _as_unix, _should_apply_state_event
|
||||
from ..models import (
|
||||
CassettePayloadRow,
|
||||
PublishCassettesPayload,
|
||||
|
|
@ -186,35 +188,62 @@ class TestUpsertCassetteConfigData:
|
|||
|
||||
|
||||
# =============================================================================
|
||||
# _should_apply_bootstrap_state — relay re-delivery dedup
|
||||
# _should_apply_state_event — ordering gate
|
||||
# =============================================================================
|
||||
|
||||
NOW = datetime(2026, 9, 22, 12, 0, 0, tzinfo=timezone.utc)
|
||||
|
||||
class TestShouldApplyBootstrapState:
|
||||
"""Pure-function dedup gate extracted from apply_bootstrap_state so the
|
||||
decision is testable without a DB. Logic: apply if-and-only-if the
|
||||
existing row's state_event_id differs from the incoming event_id.
|
||||
|
||||
In v1.1 the ATM publishes the bootstrap event exactly once per machine,
|
||||
so this is sufficient for replay protection. v2 will need a
|
||||
`last_state_created_at` watermark in addition (per bitspire's
|
||||
`meta.lastKnownConfigCreatedAt` on the ATM side) — flagged in #29's
|
||||
v2 forward-look section.
|
||||
class TestShouldApplyStateEvent:
|
||||
"""Ordering gate extracted from apply_reported_state so the decision is
|
||||
testable without a DB.
|
||||
|
||||
It compares created_at against the OLDEST state stamp on file. The gate it
|
||||
replaced compared event ids against a single arbitrary row, which is a
|
||||
one-event memory: a re-delivered A, B, A applied three times, and a late
|
||||
event overwrote newer state because nothing looked at the clock.
|
||||
"""
|
||||
|
||||
def test_applies_when_no_existing_row(self):
|
||||
assert _should_apply_bootstrap_state(None, "new-event-id") is True
|
||||
def test_applies_when_machine_has_no_state_yet(self):
|
||||
assert _should_apply_state_event(None, NOW) is True
|
||||
|
||||
def test_applies_when_existing_event_id_differs(self):
|
||||
assert _should_apply_bootstrap_state("old-event-id", "new-event-id") is True
|
||||
def test_applies_when_strictly_newer(self):
|
||||
assert _should_apply_state_event(NOW, NOW + timedelta(seconds=1)) is True
|
||||
|
||||
def test_skips_when_existing_event_id_matches(self):
|
||||
"""The same bootstrap event re-delivered after a relay reconnect
|
||||
or spirekeeper restart should no-op, not re-upsert the same
|
||||
rows (which would clobber any operator edits since)."""
|
||||
assert _should_apply_bootstrap_state("same-event", "same-event") is False
|
||||
def test_skips_an_exact_replay(self):
|
||||
"""A re-delivered event carries the same stamp, so strict `>` covers
|
||||
replay without needing to remember ids."""
|
||||
assert _should_apply_state_event(NOW, NOW) is False
|
||||
|
||||
def test_applies_when_existing_is_empty_string_and_incoming_is_id(self):
|
||||
"""Defensive — a sentinel empty-string existing_state_event_id
|
||||
shouldn't block a real incoming event from applying."""
|
||||
assert _should_apply_bootstrap_state("", "real-event-id") is True
|
||||
def test_skips_an_event_that_arrives_late(self):
|
||||
"""The failure the id-only gate allowed: an older event landing after
|
||||
a newer one used to overwrite it."""
|
||||
assert _should_apply_state_event(NOW, NOW - timedelta(seconds=30)) is False
|
||||
|
||||
def test_reapplies_over_a_partial_write(self):
|
||||
"""Every execute in this data layer commits on its own, so a crash
|
||||
mid-apply can leave some rows advanced and some not. Gating on the
|
||||
oldest stamp means the next event re-applies rather than treating the
|
||||
partial write as complete."""
|
||||
partially_applied_oldest = NOW - timedelta(seconds=300)
|
||||
assert _should_apply_state_event(partially_applied_oldest, NOW) is True
|
||||
|
||||
def test_handles_naive_and_epoch_stamps(self):
|
||||
"""SQLite hands these back as integers and Postgres as timestamps, and
|
||||
the incoming stamp is tz-aware. Comparing raw would raise or silently
|
||||
mislead."""
|
||||
assert _as_unix(NOW) == NOW.timestamp()
|
||||
assert _as_unix(NOW.replace(tzinfo=None)) == NOW.timestamp()
|
||||
assert _as_unix(int(NOW.timestamp())) == float(int(NOW.timestamp()))
|
||||
assert _as_unix(None) is None
|
||||
assert _as_unix("not-a-time") is None
|
||||
assert _should_apply_state_event(int(NOW.timestamp()), NOW) is False
|
||||
assert (
|
||||
_should_apply_state_event(int(NOW.timestamp()), NOW + timedelta(seconds=5))
|
||||
is True
|
||||
)
|
||||
|
||||
def test_skips_when_incoming_stamp_is_unusable(self):
|
||||
"""Fail closed: an unparseable incoming stamp must not overwrite
|
||||
state that is known-good."""
|
||||
assert _should_apply_state_event(NOW, "nonsense") is False
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ demonstrates the project lacks an asyncio plugin in CI; using asyncio.run
|
|||
inside the test body sidesteps that without changing project config).
|
||||
|
||||
Full handler tests (the dispatch through verify_event →
|
||||
get_machine_by_atm_pubkey_hex → apply_bootstrap_state) need a live LNbits
|
||||
get_machine_by_atm_pubkey_hex → apply_reported_state) need a live LNbits
|
||||
DB; smoke-tested manually via the dev container per the project
|
||||
convention (see test_deposit_currency.py rationale).
|
||||
"""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue