fix(pairing): retry all bunker admin RPCs past transient timeouts (#38)
Some checks failed
ci.yml / fix(pairing): retry all bunker admin RPCs past transient timeouts (#38) (pull_request) Failing after 0s

The get_key_tokens retry only covered an empty token list; a transient
NsecBunkerTimeoutError on any admin RPC still failed pairing with a 502 (seen
live on aio-demo: create_new_key and get_key_tokens both 15s-timed-out, then a
manual retry succeeded). Generalise to `_bunker_retry`, wrapping every admin
call in the pair_spire chain (create_new_key, ensure_policy, create_new_token,
get_key_tokens): a NsecBunkerTimeoutError (and, for get_key_tokens, an empty
list) is transient → retry with backoff; a NsecBunkerRpcError rejection or
misconfig is terminal → fail fast. create_new_key is replace-by-name and
ensure_policy reconciles idempotently, so retrying on timeout is safe.

Also: _validate_relay now rejects a host-less ws:// (mint was laxer than the
consumer's parseSpireSeed), and stale "nostrclient" comments in the pair UI +
a test are corrected to "transport relay".

229 tests pass (incl. timeout-retry + fail-fast-rejection coverage).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Padreug 2026-07-02 15:30:55 +02:00
commit 2b90590104
4 changed files with 93 additions and 32 deletions

View file

@ -52,6 +52,7 @@ from lnbits.core.services.nsec_bunker import (
NsecBunkerAdminClient, NsecBunkerAdminClient,
NsecBunkerError, NsecBunkerError,
NsecBunkerNotConfiguredError, NsecBunkerNotConfiguredError,
NsecBunkerTimeoutError,
npub_to_hex, npub_to_hex,
) )
from lnbits.core.signers.remote_bunker import ensure_policy 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 return token
async def _get_key_tokens_with_retry( async def _bunker_retry(call, *, attempts: int = 5, delay: float = 0.4, retry_empty: bool = False):
admin_client: NsecBunkerAdminClient, """Retry a bunker admin RPC past a TRANSIENT failure — a `NsecBunkerTimeoutError`
key_name: str, (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
attempts: int = 5, healthy bunker (bitspire#70 / spirekeeper#38). A real rejection
delay: float = 0.4, (`NsecBunkerRpcError`) or misconfig (`NsecBunkerNotConfiguredError`) is terminal
) -> list[dict]: and propagates immediately. `call` is a zero-arg coroutine factory."""
"""`get_key_tokens` can race a just-issued token: when the bunker is briefly for attempt in range(attempts):
slow (a `nip44_decrypt`/`create_new_token` under load), listing tokens last = attempt == attempts - 1
milliseconds later can return an empty set before the write lands, and try:
pairing fails with 'no tokens'. Retry a few times with a short backoff result = await call()
before giving up (bitspire#70).""" except NsecBunkerTimeoutError:
tokens: list[dict] = [] if last:
for i in range(attempts): raise
tokens = await admin_client.get_key_tokens(key_name)
if tokens:
return tokens
if i < attempts - 1:
await asyncio.sleep(delay) 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( 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): 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})") raise PairingError(f"{field} must be a ws:// or wss:// URL (got {url!r})")
host = (urlparse(url).hostname or "").lower() 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"): if host in _LOCAL_HOSTS or host.endswith(".localhost"):
raise PairingError( raise PairingError(
f"{field} points at localhost ({url!r}). A pairing seed is redeemed by " 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}" client_name = f"spire-client-{machine.id}"
try: 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) spire_pubkey_hex = npub_to_hex(spire_npub)
policy_id = await ensure_policy( policy_id = await _bunker_retry(
lambda: ensure_policy(
admin_client, admin_client,
name=SPIRE_POLICY_NAME, name=SPIRE_POLICY_NAME,
rules=SPIRE_POLICY_RULES, rules=SPIRE_POLICY_RULES,
methods_no_kind=SPIRE_POLICY_METHODS_NO_KIND, methods_no_kind=SPIRE_POLICY_METHODS_NO_KIND,
) )
await admin_client.create_new_token( )
await _bunker_retry(
lambda: admin_client.create_new_token(
key_name, client_name, policy_id, duration_hours=duration_hours 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: except NsecBunkerNotConfiguredError as exc:
raise PairingError(f"nsecbunkerd is not configured: {exc}") from exc raise PairingError(f"nsecbunkerd is not configured: {exc}") from exc
except NsecBunkerError as exc: except NsecBunkerError as exc:

View file

@ -845,8 +845,8 @@ window.app = Vue.createApp({
this.pairDialog.durationHours = null this.pairDialog.durationHours = null
this.pairDialog.result = null this.pairDialog.result = null
this.pairDialog.show = true this.pairDialog.show = true
// Pre-fill with this lnbits' default relay (the nostrclient proxy // Pre-fill with this lnbits' default relay (the relay its nostr transport
// endpoint derived from lnbits_baseurl). The operator can override or // listens on, derived from lnbits_baseurl). The operator can override or
// clear it; if left blank the backend fills in the same default. // clear it; if left blank the backend fills in the same default.
try { try {
const {data} = await LNbits.api.request('GET', `${API}/default-relay`) const {data} = await LNbits.api.request('GET', `${API}/default-relay`)
@ -861,7 +861,7 @@ window.app = Vue.createApp({
.split(/[\s,]+/) .split(/[\s,]+/)
.map(s => s.trim()) .map(s => s.trim())
.filter(Boolean) .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. // server-side (the pre-filled default), so no client-side requirement.
const body = {} const body = {}
if (relays.length) { if (relays.length) {

View file

@ -115,7 +115,7 @@ def test_pair_persists_hex_npub_and_returns_seed(monkeypatch):
def test_pair_empty_relays_accepted(monkeypatch): def test_pair_empty_relays_accepted(monkeypatch):
# Empty/omitted relays are no longer a 400 — pair_spire defaults them to the # 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) _wire(monkeypatch)
result = _call([]) result = _call([])
assert result.seed_url == "spire-seed:v1:abc" assert result.seed_url == "spire-seed:v1:abc"

View file

@ -297,6 +297,55 @@ def test_pair_raises_when_tokens_stay_empty(monkeypatch):
assert calls["n"] == 5 # exhausted all attempts 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(): def test_bunker_relay_defaults_to_spire_event_relay():
"""No explicit bunker_relay -> the relay baked into bunker_url is the spire's """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 own public event relay (relays[0]), NOT lnbits's internal bunker URL. This