From 0bb9939822a91d90b9f176d96279b6308829453c Mon Sep 17 00:00:00 2001 From: Padreug Date: Thu, 2 Jul 2026 00:43:54 +0200 Subject: [PATCH] fix(pairing): default relay to the transport's nostrrelay, not nostrclient proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nostrclient endpoint is a subscription MULTIPLEXER, not a full relay: its router forwards a client's EVENT upstream but never returns an OK ack (see nostrclient/router.py). A transport client that awaits OK on publish therefore times out ("publish timed out"), so kind-21000 RPCs never complete — verified on the Sintra: connect succeeded but list_wallets hung, and switching to the nostrrelay endpoint made the whole flow work (wallet, balance, availability). default_relay_endpoint now derives from settings.nostr_transport_relays — the relay the transport actually listens on: use it as-is when already machine-reachable, or re-home its path on lnbits_baseurl when it's a co-located loopback relay (the bundled nostrrelay). Validation/localhost-reject and the pair-dialog pre-fill/hint carry over; wording updated to "transport relay". 227 tests pass. Co-Authored-By: Claude Opus 4.8 --- models.py | 4 +-- pairing.py | 62 ++++++++++++++++++++------------ templates/spirekeeper/index.html | 2 +- tests/test_pair_endpoint.py | 11 +++--- tests/test_pairing.py | 50 ++++++++++++++++++-------- views_api.py | 9 +++-- 6 files changed, 89 insertions(+), 49 deletions(-) diff --git a/models.py b/models.py index 34db4b6..5a4e529 100644 --- a/models.py +++ b/models.py @@ -94,8 +94,8 @@ class PairMachineData(BaseModel): the relay lnbits uses to reach the bunker differs from the one the spire must reach — e.g. an internal docker hostname (`ws://lnbits:5001/…`) vs a LAN/public URL (`ws://192.168.0.32:5001/…`), or any split-relay deploy. - `relays` is optional: when omitted it defaults to this lnbits' nostrclient - proxy endpoint (derived from `lnbits_baseurl`), so the operator needn't + `relays` is optional: when omitted it defaults to the relay the transport + listens on (derived from the transport config), so the operator needn't supply one (bitspire#70). `duration_hours` optionally time-bounds the spire's connect token (None = non-expiring).""" diff --git a/pairing.py b/pairing.py index c372117..bd694c9 100644 --- a/pairing.py +++ b/pairing.py @@ -199,25 +199,41 @@ def build_seed_url( _WS_SCHEME_RE = re.compile(r"^wss?://", re.IGNORECASE) _LOCAL_HOSTS = {"localhost", "127.0.0.1", "::1", "0.0.0.0"} -# The nostrclient extension serves a public relay-multiplexer websocket at this -# path (see nostrclient/views_api.py `ws_relay`, gated by config.public_ws). -NOSTRCLIENT_RELAY_PATH = "/nostrclient/api/v1/relay" + +def _baseurl_ws() -> str | None: + """`settings.lnbits_baseurl` as a ws(s):// scheme+host (no trailing slash).""" + base = (settings.lnbits_baseurl or "").strip().rstrip("/") + if base.startswith("https://"): + return "wss://" + base[len("https://") :] + if base.startswith("http://"): + return "ws://" + base[len("http://") :] + return None def default_relay_endpoint() -> str | None: - """Derive the machine-facing relay from THIS lnbits' own nostrclient proxy - endpoint, so operators configure upstream relays once in the nostrclient - extension and every seed points at one stable URL (bitspire#70). Built from - `settings.lnbits_baseurl` (http→ws, https→wss). Returns None when the base - URL is unset. Requires the nostrclient extension's `public_ws` to be on.""" - base = (settings.lnbits_baseurl or "").strip().rstrip("/") + """Derive the machine-facing relay from lnbits' nostr-transport config, so a + seed points at the relay the transport ACTUALLY listens on (bitspire#70). + + NOTE: this is the nostr *relay* (a full relay that acks EVENTs), NOT the + nostrclient proxy — the client multiplexer forwards subscriptions but never + sends `OK`, so a transport client that awaits `OK` on publish times out. + + If the transport relay is already machine-reachable, use it as-is; if it's a + co-located loopback relay (the bundled nostrrelay at + `ws://localhost:5001/nostrrelay/`), re-home its path on lnbits' public + base URL so a remote ATM can reach the same relay. Returns None if neither + a transport relay nor a base URL is available.""" + relays = settings.nostr_transport_relays or [] + if not relays: + return None + relay = relays[0] + host = (urlparse(relay).hostname or "").lower() + if host and host not in _LOCAL_HOSTS and not host.endswith(".localhost"): + return relay # already remote-reachable + base = _baseurl_ws() if not base: return None - if base.startswith("https://"): - base = "wss://" + base[len("https://") :] - elif base.startswith("http://"): - base = "ws://" + base[len("http://") :] - return base + NOSTRCLIENT_RELAY_PATH + return base + urlparse(relay).path def _validate_relay(url: str, field: str) -> None: @@ -262,11 +278,11 @@ async def pair_spire( with a fake client. `relays` are the relays the spire uses for its *own* events - (kind-21000/30078). When omitted, they default to this lnbits' nostrclient - proxy endpoint (`default_relay_endpoint`, derived from `lnbits_baseurl`) so - operators configure upstream relays once in the extension and every seed - points at one stable URL. Every relay (and `bunker_relay`) is validated as a - remote-reachable `ws(s)://` URL — a loopback host is rejected (bitspire#70). + (kind-21000/30078). When omitted, they default to the relay the transport + listens on (`default_relay_endpoint`, from `nostr_transport_relays` re-homed + on `lnbits_baseurl`) so a seed points at a relay that actually acks EVENTs. + Every relay (and `bunker_relay`) is validated as a remote-reachable + `ws(s)://` URL — a loopback host is rejected (bitspire#70). `bunker_relay` (the relay baked into `bunker_url`, where the spire reaches the bunker) defaults to `relays[0]`; `keystore_passphrase` defaults to the lnbits bunker setting. All injectable for tests. @@ -284,14 +300,14 @@ async def pair_spire( "LNBITS_NSEC_BUNKER_KEYSTORE_PASSPHRASE is not set — " "cannot mint a spire key" ) - # Default the spire's event relay(s) to this lnbits' nostrclient proxy - # endpoint when the caller gives none. + # Default the spire's event relay(s) to the transport's own relay (re-homed + # on the base URL) when the caller gives none. if not relays: endpoint = default_relay_endpoint() if not endpoint: raise PairingError( - "no relays given and lnbits_baseurl is unset — cannot derive the " - "nostrclient relay endpoint for the seed" + "no relays given and none could be derived — set " + "nostr_transport_relays (and lnbits_baseurl if it's a loopback relay)" ) relays = [endpoint] diff --git a/templates/spirekeeper/index.html b/templates/spirekeeper/index.html index 8399529..322f4f2 100644 --- a/templates/spirekeeper/index.html +++ b/templates/spirekeeper/index.html @@ -868,7 +868,7 @@ diff --git a/tests/test_pair_endpoint.py b/tests/test_pair_endpoint.py index c123a81..682cdd8 100644 --- a/tests/test_pair_endpoint.py +++ b/tests/test_pair_endpoint.py @@ -177,13 +177,16 @@ def test_revoke_failure_maps_to_bad_gateway(monkeypatch): assert state["unpaired"] is None # not persisted on failure -def test_default_relay_endpoint_returns_nostrclient_url(): +def test_default_relay_endpoint_returns_transport_relay(): from lnbits.settings import settings - prev = settings.lnbits_baseurl + prev_base = settings.lnbits_baseurl + prev_relays = settings.nostr_transport_relays try: settings.lnbits_baseurl = "https://lnbits.example.com/" + settings.nostr_transport_relays = ["ws://localhost:5001/nostrrelay/test"] result = asyncio.run(views_api.api_default_relay(SimpleNamespace(id="op1"))) - assert result == {"relay": "wss://lnbits.example.com/nostrclient/api/v1/relay"} + assert result == {"relay": "wss://lnbits.example.com/nostrrelay/test"} finally: - settings.lnbits_baseurl = prev + settings.lnbits_baseurl = prev_base + settings.nostr_transport_relays = prev_relays diff --git a/tests/test_pairing.py b/tests/test_pairing.py index 1f9207f..967e7af 100644 --- a/tests/test_pairing.py +++ b/tests/test_pairing.py @@ -35,7 +35,9 @@ from ..pairing import ( ) _BASEURL = "https://lnbits.example.com/" -_NOSTRCLIENT_ENDPOINT = "wss://lnbits.example.com/nostrclient/api/v1/relay" +_TRANSPORT_RELAY = "ws://localhost:5001/nostrrelay/test" +# baseurl host (wss://lnbits.example.com) + transport relay path (/nostrrelay/test) +_DEFAULT_RELAY = "wss://lnbits.example.com/nostrrelay/test" _NOW = datetime(2026, 6, 16, tzinfo=timezone.utc) _SPIRE_HEX = "522a4538f1df96508d9ee8b14072344dd4a566acfe03c25a92a39179c6fca891" @@ -53,11 +55,14 @@ def _set_transport_pubkey(): # lnbits_npub in the seed (bitspire#70). Set it for every test; restore after. prev = settings.nostr_transport_public_key prev_base = settings.lnbits_baseurl + prev_relays = settings.nostr_transport_relays settings.nostr_transport_public_key = _LNBITS_HEX settings.lnbits_baseurl = _BASEURL + settings.nostr_transport_relays = [_TRANSPORT_RELAY] yield settings.nostr_transport_public_key = prev settings.lnbits_baseurl = prev_base + settings.nostr_transport_relays = prev_relays @pytest.fixture(autouse=True) @@ -327,8 +332,8 @@ def test_missing_passphrase_raises(): ) -def test_relays_default_to_nostrclient_endpoint(): - # No relays given → derived from lnbits_baseurl → nostrclient proxy endpoint. +def test_relays_default_to_transport_relay(): + # No relays given → derived from the transport relay, re-homed on baseurl. result = asyncio.run( pair_spire( _machine(), @@ -336,34 +341,51 @@ def test_relays_default_to_nostrclient_endpoint(): keystore_passphrase=_PASSPHRASE, ) ) - assert _decode_seed(result.seed_url)["relays"] == [_NOSTRCLIENT_ENDPOINT] + assert _decode_seed(result.seed_url)["relays"] == [_DEFAULT_RELAY] -def test_default_relay_endpoint_scheme_map(): - prev = settings.lnbits_baseurl +def test_default_relay_endpoint_rehomes_loopback_transport_relay(): + prev_base = settings.lnbits_baseurl + prev_relays = settings.nostr_transport_relays try: + # loopback transport relay → re-homed on the base URL host + settings.nostr_transport_relays = ["ws://localhost:5001/nostrrelay/test"] settings.lnbits_baseurl = "http://192.168.0.32:5001" - assert default_relay_endpoint() == "ws://192.168.0.32:5001/nostrclient/api/v1/relay" + assert default_relay_endpoint() == "ws://192.168.0.32:5001/nostrrelay/test" settings.lnbits_baseurl = "https://lnbits.example.com/" - assert default_relay_endpoint() == _NOSTRCLIENT_ENDPOINT + assert default_relay_endpoint() == _DEFAULT_RELAY + # loopback relay but no base URL → can't re-home → None settings.lnbits_baseurl = "" assert default_relay_endpoint() is None finally: - settings.lnbits_baseurl = prev + settings.lnbits_baseurl = prev_base + settings.nostr_transport_relays = prev_relays -def test_no_relays_and_no_baseurl_raises(): - prev = settings.lnbits_baseurl +def test_default_relay_endpoint_passes_through_reachable_transport_relay(): + prev = settings.nostr_transport_relays try: - settings.lnbits_baseurl = "" - with pytest.raises(PairingError, match="cannot derive"): + # already remote-reachable → used as-is, base URL irrelevant + settings.nostr_transport_relays = ["wss://relay.aiolabs.dev"] + assert default_relay_endpoint() == "wss://relay.aiolabs.dev" + settings.nostr_transport_relays = [] + assert default_relay_endpoint() is None + finally: + settings.nostr_transport_relays = prev + + +def test_no_relays_and_no_transport_relay_raises(): + prev = settings.nostr_transport_relays + try: + settings.nostr_transport_relays = [] + with pytest.raises(PairingError, match="none could be derived"): asyncio.run( pair_spire( _machine(), admin_client=FakeBunker(), keystore_passphrase=_PASSPHRASE ) ) finally: - settings.lnbits_baseurl = prev + settings.nostr_transport_relays = prev @pytest.mark.parametrize( diff --git a/views_api.py b/views_api.py index 86748eb..73b2d0b 100644 --- a/views_api.py +++ b/views_api.py @@ -301,9 +301,8 @@ async def api_create_machine( @spirekeeper_api_router.get("/api/v1/dca/default-relay") async def api_default_relay(user: User = Depends(check_user_exists)) -> dict: """The relay a pairing seed defaults to when the operator leaves it blank — - this lnbits' nostrclient proxy endpoint, derived from lnbits_baseurl - (bitspire#70). The pair dialog pre-fills it. `None` if lnbits_baseurl is - unset.""" + the relay the transport listens on, derived from the transport config + (bitspire#70). The pair dialog pre-fills it. `None` if it can't be derived.""" _ = user return {"relay": default_relay_endpoint()} @@ -326,8 +325,8 @@ async def api_pair_machine( `duration_hours` (optional) time-bounds the token; revoke via the sibling `POST .../revoke` endpoint.""" machine = await _machine_owned_by(machine_id, user.id) - # relays may be omitted — pair_spire defaults to this lnbits' nostrclient - # proxy endpoint and validates reachability (raises PairingError → 502). + # relays may be omitted — pair_spire defaults to the transport relay and + # validates reachability (raises PairingError → 502). try: async with NsecBunkerAdminClient.from_settings() as client: