Skip to content

Fix/discoverability publishing - #120

Open
ContextVM-org wants to merge 5 commits into
mainfrom
fix/discoverability-publishing
Open

ContextVM-org wants to merge 5 commits into
mainfrom
fix/discoverability-publishing

Conversation

@ContextVM-org

Copy link
Copy Markdown
Collaborator

Problem

A client handed only a coordinator's pubkey can learn where to reach it from
exactly one place: the coordinator's own NIP-65 relay list (kind 10002). The
SDK's discoverability publication never delivered it. Observed in the wild
(cordn-server, 2026-10-08):

WARN contextvm_sdk::transport::server::announcement: Failed to publish relay list
     error=Transport error: relay not found
INFO contextvm_sdk::transport::server::announcement: Discoverability event publishing complete

Three root causes, all in the SDK:

  1. RelayNotFound on every publish with default bootstrap relays.
    publish_to_discoverability_relays merges the advertised relays with
    DEFAULT_BOOTSTRAP_RELAY_URLS and hands the set to send_event_builder_to,
    but nostr-relay-pool rejects the whole batch when any URL is not a member
    of the main pool (send_event_to → Error::RelayNotFound) — and the
    bootstrap relays are never added to that pool. The publish failed 100% of
    the time, not just during a startup race.
  2. Total refusal was reported as success. send_event_to returns Ok with
    an empty success set when every relay refuses, so the old code logged
    "Published relay list" even when nothing landed anywhere.
  3. No socket wait, no retry, refusal reasons swallowed. The publish fired
    the instant start() called the non-blocking connect(), one-shot.

Change

  • RelayPool::publish_to now honors its documented contract: it delivers
    through a short-lived dedicated connection to exactly the target relays
    (they need not be pool members — same pattern as the TS SDK's
    publishEventToRelayUrls). It waits up to 10 s for the first socket before
    sending, logs each refusing relay's own reason (e.g. blocked: event kind 0 is not allowed), and errors when no relay accepted the event.
  • The announcement manager retries rounds with no acceptance every 30 s for up
    to 5 minutes, then gives up and says so. The two events publish
    concurrently with independent retry budgets, so an always-refusing kind 0
    can never delay the discovery-critical kind 10002.
  • The spawn/test duplication in spawn_publish_discoverability collapses into
    one shared publish path.
  • The rmcp integration example now asserts kind 10002 and kind 0 are served by
    the relay in both relay modes (its server config sets profile_metadata, so
    kind 0 is actually published), covering the real delivery path that unit
    tests can't reach through MockRelayPool.

Deliberately out of scope (app-level policy, and beyond TS parity): periodic
refresh, per-relay backoff ladders, and new bootstrap relays. Consumers that
need a refresh loop can re-publish on their own schedule; the SDK owns reliable
publication at startup.

Notes

  • Behavior change: publish_to no longer reuses the main pool's connections —
    it opens and tears down a dedicated connection per call. Failure logs gain
    the kind suffix (Failed to publish relay list (kind 10002)).
  • A timed-out retry round drops the temp pool without disconnect()
    (ponytail: comment in publish_with_retry); nostr-sdk relays tear down
    with their handles. Revisit only on leak reports.

Verification

  • New tests: retry-until-accepted (2 injected failures → exactly 3 rounds),
    give-up-after-deadline (bounded retries, loud error), and
    relay_list_publish_does_not_wait_for_a_stuck_profile — the last one
    fails on its own assertion when the publishes are serialized again
    (checked by breaking the fix on purpose).
  • Quality gate: fmt clean, clippy --all --all-features --all-targets -D warnings clean, cargo test --all --all-features 1127 passed / 0 failed,
    --no-default-features 729 passed, cargo doc 0 warnings.
  • Live smoke path: cargo run --example rmcp_integration_test --features rmcp -- relay-rmcp <relay> now verifies the 10002 + kind 0 land on the relay.

…nnections

Kind 10002 / kind 0 publication failed with 'relay not found' whenever the
default bootstrap relays were part of the publish set: send_event_to rejects
the whole batch if any target URL is not a member of the main pool, and the
bootstrap relays are never added to it. A total refusal was also reported as
published, since send_event_to returns Ok with an empty success set.

- RelayPool::publish_to now honors its contract and delivers through a
  short-lived dedicated connection to exactly the target relays, waiting up
  to 10 s for the first socket, logging each refusing relay's own reason,
  and erroring when no relay accepted the event.
- The announcement manager retries rounds with no acceptance every 30 s for
  up to 5 minutes before giving up with a log, and shares one publish path
  between the spawned task and the test-facing methods.
- New tests cover retry-until-accepted and give-up-after-deadline; the
  always-failing publish test runs on paused time so retry sleeps stay free.
The profile's retry budget ran to completion before the relay-list publish
started, so an always-refusing kind 0 (e.g. relay.contextvm.org with fallback
relays disabled) delayed the discovery-critical kind 10002 by up to 5 minutes
at every startup, and an all-down relay set took 10 minutes instead of 5.

- publish_discoverability_events now runs both publishes with tokio::join!,
  each with its own retry budget; the duplicate match/log blocks collapse
  into one labeled helper. Failure logs gain the kind suffix
  ('Failed to publish relay list (kind 10002)').
- docs: the 10 s socket wait is per round, not a one-time thing (CHANGELOG +
  docs/server-transport.md), and publish_to no longer doc-links a private
  constant (cargo doc warning).
- rmcp integration example asserts kind 10002 and kind 0 are served by the
  relay in both relay modes, covering the real temp-client delivery path;
  its server config now sets profile metadata so kind 0 is actually
  published. Correct the stale integration.yml reference in tests/integration.rs.
- New test: the relay list must land while a stuck profile publish is still
  waiting — checked to fail on its own assertion when the publishes are
  serialized again.
New toolchains deprecate Atomic*::fetch_update (renamed try_update), which
breaks -D warnings; try_update does not exist on the MSRV (1.88), so use a
plain compare_exchange loop instead. Covers both test helpers that use the
decrement-if-nonzero pattern.
…ead helper

The test-facing publish_relay_list/publish_profile_metadata still logged the
old unsuffixed failure strings, drifting from the labeled format the spawn
path uses. Both now go through publish_one, so every path emits the same
messages, and publish_to_discoverability_relays (its only two callers) is
deleted. Also reflows a run-on doc line in RelayPool::publish_to.
Review finding: the 30 s round backstop could cancel publish_to mid-send,
skipping the temp pool's disconnect(). nostr-relay-pool keeps each relay's
auto-reconnect task alive on a full strong clone and only stops it on an
explicit disconnect (no Drop teardown) — a cancelled round would leak tasks
and sockets. The removed comment claimed relays tear down with their handles,
which is not what the dependency does.

- publish_to now bounds the acknowledgement wait (15 s) and always
  disconnects its temporary pool before returning, keeping its total below
  the round backstop. (Upstream independently caps per-relay OK waits at 10 s
  and fans out in parallel, so the wrap is defense against drift and exotic
  stalls, e.g. a slow custom signer.)
- Two regression tests pin the bounds against a black-hole relay and a
  WebSocket relay that completes the handshake but never acknowledges.
- tokio-tungstenite/futures-util added as dev-deps (already transitive in the
  tree) for the minimal WebSocket test server.
- docs/CHANGELOG: rounds are self-bounded (10 s socket + 15 s acks) and the
  temp connection is always torn down.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant