diff --git a/pairing.py b/pairing.py index bd694c9..bc49eb2 100644 --- a/pairing.py +++ b/pairing.py @@ -52,6 +52,7 @@ from lnbits.core.services.nsec_bunker import ( NsecBunkerAdminClient, NsecBunkerError, NsecBunkerNotConfiguredError, + NsecBunkerTimeoutError, npub_to_hex, ) from lnbits.core.signers.remote_bunker import ensure_policy @@ -144,26 +145,27 @@ def _recover_token(tokens: list[dict], client_name: str) -> str: return token -async def _get_key_tokens_with_retry( - admin_client: NsecBunkerAdminClient, - key_name: str, - *, - attempts: int = 5, - delay: float = 0.4, -) -> list[dict]: - """`get_key_tokens` can race a just-issued token: when the bunker is briefly - slow (a `nip44_decrypt`/`create_new_token` under load), listing tokens - milliseconds later can return an empty set before the write lands, and - pairing fails with 'no tokens'. Retry a few times with a short backoff - before giving up (bitspire#70).""" - tokens: list[dict] = [] - for i in range(attempts): - tokens = await admin_client.get_key_tokens(key_name) - if tokens: - return tokens - if i < attempts - 1: +async def _bunker_retry(call, *, attempts: int = 5, delay: float = 0.4, retry_empty: bool = False): + """Retry a bunker admin RPC past a TRANSIENT failure — a `NsecBunkerTimeoutError` + (the nsecbunkerd briefly slow / unresponsive) and, when `retry_empty`, an empty + result (a just-issued token not yet listed). Both are seen intermittently on a + healthy bunker (bitspire#70 / spirekeeper#38). A real rejection + (`NsecBunkerRpcError`) or misconfig (`NsecBunkerNotConfiguredError`) is terminal + and propagates immediately. `call` is a zero-arg coroutine factory.""" + for attempt in range(attempts): + last = attempt == attempts - 1 + try: + result = await call() + except NsecBunkerTimeoutError: + if last: + raise await asyncio.sleep(delay) - return tokens + continue + if retry_empty and not result and not last: + await asyncio.sleep(delay) + continue + return result + raise AssertionError("unreachable: the final attempt returns or raises") def build_seed_url( @@ -243,6 +245,8 @@ def _validate_relay(url: str, field: str) -> None: if not isinstance(url, str) or not _WS_SCHEME_RE.match(url): raise PairingError(f"{field} must be a ws:// or wss:// URL (got {url!r})") host = (urlparse(url).hostname or "").lower() + if not host: + raise PairingError(f"{field} has no host ({url!r})") if host in _LOCAL_HOSTS or host.endswith(".localhost"): raise PairingError( f"{field} points at localhost ({url!r}). A pairing seed is redeemed by " @@ -327,18 +331,26 @@ async def pair_spire( client_name = f"spire-client-{machine.id}" try: - spire_npub = await admin_client.create_new_key(key_name, passphrase) + # Each admin RPC is retried past a transient bunker timeout (#38); + # create_new_key is replace-by-name so re-issuing on a timeout is safe, + # and ensure_policy reconciles idempotently (get_policies → reuse/create). + spire_npub = await _bunker_retry(lambda: admin_client.create_new_key(key_name, passphrase)) spire_pubkey_hex = npub_to_hex(spire_npub) - policy_id = await ensure_policy( - admin_client, - name=SPIRE_POLICY_NAME, - rules=SPIRE_POLICY_RULES, - methods_no_kind=SPIRE_POLICY_METHODS_NO_KIND, + policy_id = await _bunker_retry( + lambda: ensure_policy( + admin_client, + name=SPIRE_POLICY_NAME, + rules=SPIRE_POLICY_RULES, + methods_no_kind=SPIRE_POLICY_METHODS_NO_KIND, + ) ) - await admin_client.create_new_token( - key_name, client_name, policy_id, duration_hours=duration_hours + await _bunker_retry( + lambda: admin_client.create_new_token( + key_name, client_name, policy_id, duration_hours=duration_hours + ) ) - tokens = await _get_key_tokens_with_retry(admin_client, key_name) + # retry_empty: a just-issued token can list empty before the write lands. + tokens = await _bunker_retry(lambda: admin_client.get_key_tokens(key_name), retry_empty=True) except NsecBunkerNotConfiguredError as exc: raise PairingError(f"nsecbunkerd is not configured: {exc}") from exc except NsecBunkerError as exc: diff --git a/static/js/index.js b/static/js/index.js index a0659b5..09188ac 100644 --- a/static/js/index.js +++ b/static/js/index.js @@ -845,8 +845,8 @@ window.app = Vue.createApp({ this.pairDialog.durationHours = null this.pairDialog.result = null this.pairDialog.show = true - // Pre-fill with this lnbits' default relay (the nostrclient proxy - // endpoint derived from lnbits_baseurl). The operator can override or + // Pre-fill with this lnbits' default relay (the relay its nostr transport + // listens on, derived from lnbits_baseurl). The operator can override or // clear it; if left blank the backend fills in the same default. try { const {data} = await LNbits.api.request('GET', `${API}/default-relay`) @@ -861,7 +861,7 @@ window.app = Vue.createApp({ .split(/[\s,]+/) .map(s => s.trim()) .filter(Boolean) - // relays is optional — blank falls back to the nostrclient endpoint + // relays is optional — blank falls back to the transport relay // server-side (the pre-filled default), so no client-side requirement. const body = {} if (relays.length) { diff --git a/tests/test_pair_endpoint.py b/tests/test_pair_endpoint.py index 682cdd8..51529e5 100644 --- a/tests/test_pair_endpoint.py +++ b/tests/test_pair_endpoint.py @@ -115,7 +115,7 @@ def test_pair_persists_hex_npub_and_returns_seed(monkeypatch): def test_pair_empty_relays_accepted(monkeypatch): # Empty/omitted relays are no longer a 400 — pair_spire defaults them to the - # nostrclient endpoint (and validates), so the endpoint passes them through. + # transport relay (and validates), so the endpoint passes them through. _wire(monkeypatch) result = _call([]) assert result.seed_url == "spire-seed:v1:abc" diff --git a/tests/test_pairing.py b/tests/test_pairing.py index 967e7af..12c7dfc 100644 --- a/tests/test_pairing.py +++ b/tests/test_pairing.py @@ -297,6 +297,55 @@ def test_pair_raises_when_tokens_stay_empty(monkeypatch): assert calls["n"] == 5 # exhausted all attempts +def test_create_new_key_retries_past_transient_timeout(monkeypatch): + # A bunker timeout on any admin RPC (here create_new_key) is transient (#38). + import spirekeeper.pairing as pairing_mod + from lnbits.core.services.nsec_bunker import NsecBunkerTimeoutError + + async def _no_sleep(*_a, **_k): + pass + + monkeypatch.setattr(pairing_mod.asyncio, "sleep", _no_sleep) + + bunker = FakeBunker(token_secret="s") # pragma: allowlist secret + real = bunker.create_new_key + calls = {"n": 0} + + async def flaky(name, passphrase): + calls["n"] += 1 + if calls["n"] == 1: + raise NsecBunkerTimeoutError("no response for 'create_new_key' within 15.0s") + return await real(name, passphrase) + + bunker.create_new_key = flaky + result = _pair(bunker) + assert calls["n"] == 2 # timed out once, then succeeded + assert result.spire_npub == _SPIRE_NPUB + + +def test_bunker_rejection_fails_fast_without_retry(monkeypatch): + # A real rejection (NsecBunkerRpcError) is terminal — no retry. + import spirekeeper.pairing as pairing_mod + from lnbits.core.services.nsec_bunker import NsecBunkerRpcError + + async def _no_sleep(*_a, **_k): + pass + + monkeypatch.setattr(pairing_mod.asyncio, "sleep", _no_sleep) + + bunker = FakeBunker() + calls = {"n": 0} + + async def rejecting(name, passphrase): + calls["n"] += 1 + raise NsecBunkerRpcError("policy rejected") + + bunker.create_new_key = rejecting + with pytest.raises(PairingError, match="bunker admin RPC failed"): + _pair(bunker) + assert calls["n"] == 1 # terminal — not retried + + def test_bunker_relay_defaults_to_spire_event_relay(): """No explicit bunker_relay -> the relay baked into bunker_url is the spire's own public event relay (relays[0]), NOT lnbits's internal bunker URL. This