From 102c8eac9173449dc3b61d040e540f95b188ff59 Mon Sep 17 00:00:00 2001 From: Padreug Date: Mon, 22 Jun 2026 16:53:45 +0200 Subject: [PATCH] fix: complete the unpaired-machine sweep + regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the call-site guards: a full sweep of every machine_npub deref found one more reachable crash — _record_rejected (tasks.py) logs machine_npub[:12], and the assert_nostr_attribution guard now routes an unpaired machine there, so None[:12] -> TypeError. Fall back to machine.id. Every other deref is safe by the attribution-gate invariant: a settlement only flows past assert_nostr_attribution (now rejecting unpaired) for a paired machine, so the downstream distribution / parse-path / "landed" logs can't see None; the collision-loop display already uses `(m.machine_npub or m.id)`. - tests/test_unpaired_machine_guards.py: regression — assert_nostr_attribution rejects an unpaired machine (domain error, not AttributeError) and build_state_d_tags skips it. - tests/test_pair_endpoint.py: update the fake_pair mock for bunker_relay/ keystore_passphrase (pre-existing drift from #29; was failing before this PR). Full suite green (213 passed). --- tasks.py | 5 ++- tests/test_pair_endpoint.py | 10 ++++- tests/test_unpaired_machine_guards.py | 57 +++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 tests/test_unpaired_machine_guards.py diff --git a/tasks.py b/tasks.py index eb874de..e3865d2 100644 --- a/tasks.py +++ b/tasks.py @@ -235,7 +235,10 @@ async def _record_rejected(payment: Payment, machine: Machine, exc: Exception) - return logger.error( f"spirekeeper: rejected settlement {rejected.id} " - f"(machine={machine.machine_npub[:12]}..., " + # An unpaired machine (machine_npub None) reaches here now that + # assert_nostr_attribution rejects it — fall back to the id so the + # log line doesn't crash on None[:12]. + f"(machine={(machine.machine_npub or machine.id)[:12]}..., " f"payment_hash={payment.payment_hash[:12]}...): {exc}" ) diff --git a/tests/test_pair_endpoint.py b/tests/test_pair_endpoint.py index 0d50d95..c5d6973 100644 --- a/tests/test_pair_endpoint.py +++ b/tests/test_pair_endpoint.py @@ -66,7 +66,15 @@ def _wire(monkeypatch, *, pair="ok"): async def fake_owned(machine_id, user_id): return _machine() - async def fake_pair(machine, *, relays, admin_client, duration_hours=None): + async def fake_pair( + machine, + *, + relays, + admin_client, + bunker_relay=None, + keystore_passphrase=None, + duration_hours=None, + ): if pair == "error": raise PairingError("boom") return _result() diff --git a/tests/test_unpaired_machine_guards.py b/tests/test_unpaired_machine_guards.py new file mode 100644 index 0000000..78ae196 --- /dev/null +++ b/tests/test_unpaired_machine_guards.py @@ -0,0 +1,57 @@ +""" +Regression: `machine_npub` is nullable (#29/m011 register-unpaired flow), so +every consumer that derives a Nostr identity from it must handle `None` rather +than crash `normalize_public_key(None)` (AttributeError: 'NoneType' has no +'startswith') or `machine_npub[:12]` (TypeError). See PR #33 — an unpaired +machine on the demo broke the platform-fee update (500) and the cassette +consumer. + +These cover the pure-function guards; the DB-backed loops +(get_machine_by_atm_pubkey_hex, the super-config republish loop) are exercised +on the dev stack with an unpaired active machine. +""" + +from datetime import datetime, timezone + +import pytest + +from ..bitspire import SettlementAttributionError, assert_nostr_attribution +from ..cassette_transport import build_state_d_tags_for_machines +from ..models import Machine + +_PAIRED_HEX = "82341f882b6eabcbd6b1c2da5cd14df14b8e91dd0e6da41a72b78ad8f3a7d3b9" + + +def _machine(npub: str | None) -> Machine: + now = datetime.now(timezone.utc) + return Machine( + id="unpaired1", + operator_user_id="op1", + machine_npub=npub, + wallet_id="w1", + name="unpaired", + location=None, + fiat_code="EUR", + is_active=True, + created_at=now, + updated_at=now, + ) + + +def test_attribution_rejects_unpaired_machine_cleanly(): + """An unpaired machine must raise the domain SettlementAttributionError + (which the listener records as 'rejected'), not an uncaught AttributeError + from normalize_public_key(None).""" + with pytest.raises(SettlementAttributionError): + assert_nostr_attribution( + _machine(None), + {"source": "bitspire", "nostr_sender_pubkey": _PAIRED_HEX}, + ) + + +def test_cassette_d_tags_skip_unpaired_machine(): + """build_state_d_tags_for_machines must skip unpaired machines rather than + crash _atm_hex_pubkey on a None npub — the cassette-consumer loop crash.""" + tags = build_state_d_tags_for_machines([_machine(_PAIRED_HEX), _machine(None)]) + assert len(tags) == 1 # only the paired machine contributes a d-tag + assert all("None" not in t for t in tags)