Skip to content

transport-nostr-adapter: relay sync-timing telemetry + initial-sync gate key the same relay under two different RelayIndex values (verbatim start endpoint vs RelayUrl-canonicalized inbound endpoint) — EOSE/first-event silently dropped, subscription_synced never reaches true #795

Description

@erskingardner

Summary

RelayIndexRegistry assigns a stable opaque RelayIndex keyed by the exact TransportEndpoint string, with no canonicalization. The subscription-start telemetry path records progress under index_for(verbatim_endpoint) (routing state carries the verbatim signed routing strings, which the adapter deliberately never rewrites — see the CanonicalEndpoint doc and mdk#482). But EOSE and first-event arrive with relay_url.to_string(), the nostr-sdk RelayUrl-canonicalized form (trailing slash, host case, default port, percent-encoding). These produce a different RelayIndex, so the per-relay guard misses and the record is dropped.

Result, whenever a configured relay's stored form isn't already RelayUrl-canonical (the common case, e.g. wss://relay.damus.io stored vs wss://relay.damus.io/ inbound):

  1. subscription_synced(id) returns Some(false) forever — the documented "initial-sync gate" never completes.
  2. The phase-2 first-event and EOSE latency histograms (RelaySyncSnapshot.first_event / .eose / .per_relay) stay empty — the feature is non-functional in production.

Location

  • crates/transport-nostr-adapter/src/telemetry.rs:77-85RelayIndexRegistry::index_for keys the raw TransportEndpoint string, no canonicalization.
  • crates/transport-nostr-adapter/src/lib.rs:1000-1010AdapterState::record_subscription_starts indexes subscription.endpoints() (verbatim signed routing strings).
  • crates/transport-nostr-adapter/src/lib.rs:1051-1069record_subscription_first_event / record_subscription_eose index the inbound endpoint from handle_relay_eose / observe_relay_event.
  • crates/transport-nostr-adapter/src/telemetry.rs:496-526record_first_event / record_eose: the .and_then(|sub| sub.relays.get_mut(&relay)) guard silently drops the record on the index miss.
  • Inbound endpoint source (canonical form): crates/marmot-app/src/relay_plane/mod.rs:697,718 and crates/transport-nostr-adapter/src/sdk_client.rs:157,176 — both pass TransportEndpoint(relay_url.to_string()).
// telemetry.rs record_eose (simplified)
if let Some(progress) = self.subscriptions
    .get_mut(subscription_id)                 // hits: subscription_id is our deterministic id
    .and_then(|sub| sub.relays.get_mut(&relay)) // MISS: sub.relays keyed by verbatim index; `relay` is canonical index
    && !progress.eose_seen { ... }             // never runs

Why it slips past tests

The existing tests use identical endpoint strings for setup and inbound (e.g. tests/inbound_routing.rs uses "wss://group.example" for both). There is no test where the verbatim stored endpoint differs from its RelayUrl-canonical form — exactly the difference mdk#482 documents.

Cross-relay arrival spread (record_delivery_timing) is unaffected because it uses the canonical inbound endpoint on both first and later sightings; only the sync path mixes the two id sources.

Severity

RelaySyncSnapshot is surfaced as diagnostic relay stats; I did not find a code path that hard-blocks on the gate, so impact is "documented feature silently dead + a wrong synced=false signal" rather than data-loss — MEDIUM (correctness + a dead perf/telemetry feature).

Suggested fix

Route both the start-time and event-time endpoints through the same canonicalization before index_for — e.g. key RelayIndexRegistry on the parsed RelayUrl with a verbatim fallback, mirroring the existing CanonicalEndpoint::matches used on the delivery path. Add a test where verbatim ≠ RelayUrl-canonical and assert subscription_synced reaches true and the histograms populate.

Dedup

Checked open + closed. Not a duplicate. Closest is mdk#482 (routing endpoint canonicalization, fixed on the delivery path via CanonicalEndpoint::matches) — this is the same normalization gap re-appearing on the untreated telemetry/sync path. Distinct from #748 (telemetry map size cap) and #763 (delivery channel HOL).


Found during a methodical multi-crate code review; verified against telemetry.rs:77-85 and the invariant that inbound RelayUrl-parsed endpoints differ from verbatim stored ones.

Metadata

Metadata

Assignees

No one assigned

    Labels

    MEDIUMbugSomething isn't workingperformancePerformance issue

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions