From 5a74bf1f0d93eb7dedc686020fb42e2eff086137 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 26 Sep 2026 15:07:05 +0700 Subject: [PATCH] update initial sync --- crates/signed_state/src/backend.rs | 34 ++-- crates/signed_state/src/repo.rs | 3 +- crates/signed_state/src/repos.rs | 13 +- docs/backend-audit.md | 217 +++++++++++++++++++++ docs/event-fetching-strategy.md | 296 +++++++++++++++++++++++++++++ 5 files changed, 541 insertions(+), 22 deletions(-) create mode 100644 docs/backend-audit.md create mode 100644 docs/event-fetching-strategy.md diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 1c461b4..156606c 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -23,12 +23,7 @@ pub const USER_KEYRING: &str = "Signed Safe Storage"; pub const NOSTR_CONNECT_TIMEOUT: u64 = 60; /// Relays connected at startup, before any user-specific relay config is known. -pub const BOOTSTRAP_RELAYS: [&str; 4] = [ - "wss://relay.primal.net", - "wss://relay.ditto.pub", - "wss://index.ngit.dev", - "wss://profiles.nostr1.com", -]; +pub const BOOTSTRAP_RELAYS: [&str; 2] = ["wss://relay.ditto.pub", "wss://index.ngit.dev"]; /// Relays used to index the user's NIP-65 relay list. pub const INDEXER_RELAYS: [&str; 2] = ["wss://indexer.coracle.social", "wss://user.kindpag.es"]; @@ -43,11 +38,6 @@ pub enum BackendEvent { /// The signer changed on login, logout or account switch. SignerChanged, /// New events were received from a relay and stored in the database. - /// - /// Batched: [`Backend`]'s notification pump coalesces everything a - /// relay delivers within one debounce window into a single event, - /// instead of emitting per-event and making every subscriber debounce - /// the same burst independently. NostrUpdate(Vec), Synced, SyncProgress { @@ -446,9 +436,6 @@ impl Backend { let client = self.client.clone(); // Initialize directly at the user's chosen destination. - // No mirror is pre-populated: `GitCache::ensure_clone` lazily clones - // from the grasp server the first time the repo detail view needs it, - // exactly like every other repository. let destination = { let dir_name = signed_git::sanitize_path_component(&name); let dir_name = if dir_name.is_empty() { @@ -1222,7 +1209,8 @@ impl Backend { .detach(); } - pub fn sync_bootstrap(&mut self, filter: Filter, cx: &mut Context) { + /// Sync several bootstrap filters in order, within a single task. + pub fn sync_bootstraps(&mut self, filters: Vec, cx: &mut Context) { let client = self.client.clone(); let (tx, mut rx) = SyncProgress::channel(); @@ -1259,8 +1247,19 @@ impl Backend { .detach(); let sync = cx.background_spawn(async move { - let opts = SyncOptions::default().progress(tx); - sync_bootstrap_only(&client, filter, opts).await + let mut first_error = None; + + for filter in filters { + let opts = SyncOptions::default().progress(tx.clone()); + if let Err(error) = sync_bootstrap_only(&client, filter, opts).await { + first_error.get_or_insert(error); + } + } + + match first_error { + Some(error) => Err(error), + None => Ok(()), + } }); cx.spawn(async move |this, cx| { @@ -1419,6 +1418,7 @@ pub(crate) async fn sync_bootstrap_only( .with(BOOTSTRAP_RELAYS) .opts(opts) .await?; + Ok(output.value) } diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 25cdf78..9ba4a81 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -288,9 +288,10 @@ impl RepoStore { self.repo_relays.extend(new.iter().cloned()); let backend = Backend::global(cx); + let filters = Self::repo_filters(&addr); backend.update(cx, |backend, cx| { - backend.connect_repo_relays(new, Self::repo_filters(&addr), cx); + backend.connect_repo_relays(new, filters, cx); }); } diff --git a/crates/signed_state/src/repos.rs b/crates/signed_state/src/repos.rs index f13ee91..c6309c8 100644 --- a/crates/signed_state/src/repos.rs +++ b/crates/signed_state/src/repos.rs @@ -134,10 +134,15 @@ impl RepoListStore { let backend = Backend::global(cx); backend.update(cx, |backend, cx| { - backend.sync_bootstrap(filters::all_announcements(), cx); - backend.sync_bootstrap(filters::all_states(), cx); - // Deletion requests, NIP-09/62, must be known before any announcement is shown. - backend.sync_bootstrap(filters::deletions(), cx); + backend.sync_bootstraps( + vec![ + filters::all_announcements(), + filters::all_states(), + // Deletion requests, NIP-09/62, must be known before any announcement is shown. + filters::deletions(), + ], + cx, + ); }); } diff --git a/docs/backend-audit.md b/docs/backend-audit.md new file mode 100644 index 0000000..0d0969a --- /dev/null +++ b/docs/backend-audit.md @@ -0,0 +1,217 @@ +# Backend audit: relays, sync and NIP-65 + +Status: audit of the current code, done before implementing +`docs/event-fetching-strategy.md`. Where this document and the first draft of +that proposal disagree, this document is authoritative. + +## Scope and method + +Crates read for relay and network behaviour: + +- `signed_state`: `backend`, `repo`, `repos`, `inbox`, `profile`, `checkouts`, + `local_repos`, `refresh`, `git_store`. +- `signed_nostr`: `backend`, `signer`, `update`. +- `signed_core`: `filters`, `deletions`, `model`, `state`, `status`, `inbox`. +- Relay touchpoints of `signed_git`, `settings`, `paths`, `utils`. + +Cross-checked against: + +- rust-nostr at the revision from `Cargo.lock`, + `b230cecf9dbb38e0228e6fff4544ed9d261326fc`. **All** `nostr*` crates + (`nostr`, `nostr-database`, `nostr-gossip`, `nostr-gossip-memory`, + `nostr-lmdb`, `nostr-sdk`) resolve to that single revision. The local + checkout is `~/.cargo/git/checkouts/nostr-619b808bb247a9ed/b230cec`. +- GitWorkshop, cloned with `ngit` from + `nostr://npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/gitworkshop` + at revision `420c0c3`. + +Paths below are relative to those checkouts unless prefixed with +`crates/`, which means this repository. + +## 1. Verified SDK behaviour + +### Target selection + +- `nostr-sdk/src/client/api/req_target.rs`: `ReqTarget::auto` wraps a bare + filter list; `ReqTarget::manual` wraps a relay map. `From>>` + (L91) makes an explicit map a **manual** request. +- `nostr-sdk/src/client/api/util.rs::build_targets`: + - Auto + gossip configured -> `gossip_break_down_filters`. + - Manual -> the map is used as-is, gossip is skipped. +- `nostr-sdk/src/client/api/subscribe.rs` and `api/sync.rs` both go through + these rules. `client.sync(filter)` with no `.with(...)` uses gossip; + `client.sync(filter).with(urls)` does not. +- `pool.sync` errors with `relay not found` when a targeted relay has not been + added to the pool. Targets are not implicitly added. + +### Gossip breakdown + +- `nostr-sdk/src/client/gossip/updater.rs::gossip_break_down_filter` (L412) + extracts pubkeys via `Filter::extract_public_keys` + (`nostr/src/filter/mod.rs` L591): **only `authors` and the lowercase `#p` + tag**. It first calls `ensure_gossip_public_keys_fresh`, then breaks the + filter down, then adds every resolved relay to the pool with + `RelayCapabilities::GOSSIP`. +- Freshness (`updater.rs` L381-L410, L164-L185) negentropy-syncs kind `10002` + for the extracted pubkeys over the pool's `DISCOVERY | READ` relays. This is + the only automatic way the gossip store learns kind `10002`. +- `nostr-sdk/src/client/gossip/resolver.rs::break_down_filter` (L93): + - `authors` only -> each author's **write** relays, plus hint and + most-received relays. + - `#p` only -> each pubkey's **read** relays, plus hint and most-received. + - both -> the union of read and write relays, keeping the filter unchanged. + - neither (`Other`), or no relay found (`Orphan`) -> the pool's **read** + relays (`read_relay_urls`). +- `relay/capabilities.rs`: `GOSSIP` is its own bit. `pool.read_relay_urls` / + `write_relay_urls` / `relay_urls_with_any_cap` use a raw bit test + (`filter_relays_with_any_cap`), so GOSSIP-only relays are excluded from + broadcast and from the `Other`/`Orphan` fallback. Per-relay send checks use + `can_read`/`can_write`, which do include GOSSIP, so gossip-resolved targets + work when named explicitly. +- `GossipConfig::default()` (`builder.rs` L30-L120): limits read 3, write 3, + hints 1, most-received 1, NIP-17 3 per user; `background_refresh` on by + default, disabled by `.no_background_refresh()`. +- `NostrGossipMemory` reads only the first 7 entries of a kind `10002` + (`MAX_NIP65_SIZE`) and stops tracking new hint/most-received relays once a + pubkey is near `MAX_RELAYS_PER_PK` (7). `get_best_relays(pk, selection, allowed)` + (public trait, `gossip/nostr-gossip/src/lib.rs`) reads whatever is in the + store, sorted by received-event count then recency. It does **not** fetch + kind `10002` itself. The store is primed by the freshness step of an Auto + request (or by any kind `10002` that arrives for another reason), and marks + a key outdated 24 h after its last fetch attempt. + +### Publishing + +- `client.send_event(event)` with **no** policy and gossip configured targets + NIP-65: it triggers the same freshness, then sends to the author's outbox + and every `p`-tagged pubkey's inbox (`send_event.rs::gossip_prepare_urls`). +- `client.send_event(event).broadcast()` overrides this to the pool's write + relays only (`send_event.rs` L428-L430). `signed` uses `broadcast()` + everywhere. + +### Notifications + +- Every message from a relay passes `relay/inner.rs::handle_event_msg`, which + saves new events and emits `ClientNotification::Event`. This includes events + received by a negentropy **sync** (the sync's down subscription is + registered as an auto-closing subscription, and the events travel the normal + message path). Events already in the database do not re-notify. +- The comment in `crates/signed_state/src/profile.rs` claiming synced events + produce no `NostrUpdate` is inaccurate for this revision. The re-read after + a sync is harmless, but the comment should not be relied on. + +## 2. What `signed` actually does today + +Every fetch path is manual. There is no Auto request anywhere in the +workspace, so `gossip_break_down_filter` and `ensure_gossip_public_keys_fresh` +never run. The gossip store is configured and passively fed +(`handle_event_msg` calls `gossip.process` for every received event), but it +is never consulted for targeting. + +| Call site | Request | Target relays | Target kind | +| --- | --- | --- | --- | +| `Backend::bootstrap` | `add_relay` + connect | `BOOTSTRAP_RELAYS` (READ\|WRITE), `INDEXER_RELAYS` (`DISCOVERY`) | n/a | +| `Backend::subscribe_bootstrap` -> `subscribe_bootstrap_only` | `client.subscribe(HashMap<&str, Vec>)`, `ExitOnEOSE`, 10 s timeout | `BOOTSTRAP_RELAYS` | manual | +| `Backend::sync_bootstraps` -> `sync_bootstrap_only` | `client.sync(filter).with(BOOTSTRAP_RELAYS)` | `BOOTSTRAP_RELAYS` | manual | +| `Backend::connect_repo_relays` | `add_relay().and_connect()` then `client.sync(filter).with(relays)` | announcement `relays` tag | manual | +| `RepoListStore::subscribe_remote` | `sync_bootstraps(all_announcements, all_states, deletions)` | `BOOTSTRAP_RELAYS` | manual | +| `RepoListStore::sync_own_repo_states` | `connect_repo_relays(state(addr))` | own repos' announced relays | manual | +| `RepoStore::subscribe_remote` | `subscribe_bootstrap(repo_filters)` | `BOOTSTRAP_RELAYS` | manual | +| `RepoStore::connect_announced_relays` | `connect_repo_relays(repo_filters)` | announced relays | manual | +| `RepoStore::run_refresh` root follow-ups | `subscribe_bootstrap(root_filters)` + `connect_repo_relays(root_filters)` | bootstrap + announced relays | manual | +| `Backend::sync_inbox` | `subscribe_bootstrap(notifications, authored_activity)` + `connect_repo_relays` over own announcements | bootstrap + own repos' relays | manual | +| `Backend::bootstrap_user` | `sync_bootstrap_only(grasp_list)` | `BOOTSTRAP_RELAYS` | manual | +| `ProfileStore::handle_requests` | `sync_bootstrap_only(metadata)` | `BOOTSTRAP_RELAYS` | manual | +| All publishes | `client.send_event(event).broadcast()` | pool write relays | no gossip | + +Consequences: + +- `INDEXER_RELAYS` are connected but receive no request: being `DISCOVERY`-only + they are excluded from every manual target, and no Auto request exists to + use them. They become useful only once an Auto request runs, or if a manual + request names them. +- The NIP-34 `p` tags on events do not influence fetching. `extract_public_keys` + reads the **filter**, not the events, and the repo filters carry no pubkeys + outside the announcement/state and author-deletion filters. +- The pool has no `max_relays` (`builder.rs` default `None`) and relays are + never removed. Any future Auto traffic accumulates GOSSIP relays for the + lifetime of the session. + +## 3. The NIP-34 `p` tag, per event kind + +The claim "activity events already carry a `p` tag pointing at the repository +owner" is true for the root kinds and false for kind-1111 comments: + +| Kind | Lowercase `p` | Source | +| --- | --- | --- | +| Issue 1621 | repository owner | `nostr/src/nips/nip34.rs::GitIssue` (L476-L484) | +| Patch 1617 | repository owner | `nip34.rs::GitPatch` (L575-L583); `RepoStore::publish_patch_series` builds patches by hand and adds `Tag::public_key(owner)` explicitly | +| PR 1618 | repository owner | `nip34.rs::GitPullRequest` (L654-L664) | +| PR update 1619 | repository owner | `nip34.rs::GitPullRequestUpdate` (L721-L730) | +| Status 1630-1633 | owner and root author | `RepoStore::set_status` / `publish_applied_status` push both explicitly | +| Comment 1111 | **parent author, not necessarily the owner** | `CommentBuilder` emits the root as uppercase `E`/`K`/`P` and the parent as lowercase `e`/`k`/`p` (`nostr/src/nips/nip22.rs::as_vec`). `RepoStore::comment_builder` passes the root as the parent for top-level comments, so `p` is the issue/PR author there. | + +So a single `Filter` with both `.coordinate(addr)` (`#a`) and +`.pubkey(maintainers)` (`#p`) would drop comments on roots authored by +non-maintainers. Keeping the existing `#a`-only filter and adding a second +`#p`-scoped filter is safe. The `#p`-scoped filter buys read-relay ("inbox") +targeting through gossip, not root coverage. + +## 4. Findings + +1. **Startup ordering hazard (medium).** `Backend::new` defers `bootstrap`, + which adds relays inside a background task; `RepoListStore::new` defers + `subscribe_remote`, which immediately background-spawns + `sync_bootstraps`. If the sync reaches `pool.sync` before the relays are in + the pool, every filter fails with `relay not found`. The global sync runs + only once per session and is not retried, so a lost race means no fresh + announcements, states or deletions are fetched until the next launch (the + persistent LMDB still supplies what previous sessions stored). Fix + options: add the bootstrap relays before spawning, or make the sync helpers + ensure their relays. +2. **Profile coverage vs. the indexers (medium).** Moving + `wss://profiles.nostr1.com` from `BOOTSTRAP_RELAYS` to `INDEXER_RELAYS` + (working tree) means `ProfileStore`, which syncs metadata over + `BOOTSTRAP_RELAYS`, no longer reaches it. `INDEXER_RELAYS` themselves are + currently inert (see above). Decide whether profile metadata should target + a profile indexer explicitly, and whether `profiles.nostr1.com` indexes + kind `10002` at all. +3. **State is owner-only (low, verify against NIP-34).** `filters::state` + and `repo_filters` fetch kind `30618` with `.author(addr.public_key)`, the + announcement author. GitWorkshop reads state from every confirmed + maintainer (`useResolvedRepository.ts`, `stateCandidates`). If co-maintainers + publish state events, `signed` ignores them. +4. **`profile.rs` comment wrong (cosmetic).** Synced events do emit + `NostrUpdate` on this SDK revision; the post-sync re-read in + `handle_requests` is defensive, not load-bearing. +5. **Progress fields are global (cosmetic).** Overlapping `sync_bootstraps` + calls share `sync_progress`; the last writer wins and completion of either + clears it. Progress counters from sequential filters accumulate correctly + because all filters share one watch channel. +6. **Background fetch failures are log-only (policy).** `connect_repo_relays` + and the per-filter sync errors are logged, not surfaced. Given the new + strategy adds more fetch paths, decide whether relay reachability should + feed the existing `last_error` / `last_warning` surfaces. + +## 5. Consequences for the fetching strategy + +- **Auto and Manual can coexist**, as asked. They are independent requests on + the same pool; events deduplicate in the database and only new events + notify. +- **The gossip store must be primed by an Auto request before it can resolve + anything.** `get_best_relays` is a pure store read, and + `ensure_gossip_public_keys_fresh` is private to the client. Sequence + Uncensored as: Auto request (freshness runs as a side effect) -> resolve + maintainers via `get_best_relays(pk, All { .. })` -> manual sync through + `connect_repo_relays`. +- **The manual leg already covers GitWorkshop's "outbox and inbox".** + `BestRelaySelection::All { read, write, hints, most_received }` returns both + directions, so the activity filter does not need `.pubkey(..)` for reach, + and the comment caveat in section 3 becomes optional rather than blocking. + Adding `.pubkey(maintainers)` still helps the Auto leg reach maintainer + inboxes and is worth doing as a second filter. +- **Curated can stay exactly what the code does today** (announced relays, + manual). The only decision is whether the per-repository REQ against + `BOOTSTRAP_RELAYS` remains in Curated or not. +- Publishing is unaffected; it already broadcasts to every pooled write relay + and does not consult the strategy. diff --git a/docs/event-fetching-strategy.md b/docs/event-fetching-strategy.md new file mode 100644 index 0000000..b94edc5 --- /dev/null +++ b/docs/event-fetching-strategy.md @@ -0,0 +1,296 @@ +# Proposal: Event Fetching Strategy (`Curated` / `Uncensored`) + +Status: proposal, not implemented. Verified against `docs/backend-audit.md`; +read that document first. + +References studied: + +- GitWorkshop `main` @ `420c0c3` + (`git clone nostr://npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr//gitworkshop`). +- rust-nostr @ `b230cecf9dbb38e0228e6fff4544ed9d261326fc` from `Cargo.lock`. + Every `nostr*` crate, including `nostr-gossip` and `nostr-gossip-memory`, + resolves to that single revision. + +## Goal + +Add a global setting controlling which relays `RepoStore` queries for a +repository's activity, mirroring GitWorkshop's "Event Fetching Strategy": + +- **Curated** (`repo`): only the relays declared in the repository + announcement. +- **Uncensored** (`outbox`): the repository's declared relays plus every + maintainer's relays, resolved through the NIP-65 outbox model. + +## GitWorkshop reference + +Source paths are relative to the cloned repository. + +- `src/services/settings.ts`: `RelayCurationMode = "repo" | "outbox"`, + persisted to `localStorage`, default `"outbox"` (the adjacent doc comment + wrongly says `repo`; the constant is authoritative). +- `src/pages/Settings.tsx` (`RelayCurationSection`, `CURATION_OPTIONS`): two + selectable cards, "Curated" and "Uncensored". +- `src/hooks/useResolvedRepository.ts` (layers 3-4): + - `repoRelayGroup` (`RepositoryRelayGroup`): the repo announcement's + `relays` tag. The **Curated** frontier. + - `extraRelaysForMaintainerMailboxCoverage`: a delta relay group built from + every maintainer's NIP-65 **outbox + inbox** relays, excluding relays + already in `repoRelayGroup`. The **Uncensored** addition. +- `src/services/nostr.ts`: + - `resolveMailboxes(pubkey)` reads NIP-65 kind `10002` with a 3 s timeout and + caps to `MAX_RESOLVED_RELAYS = 5` per direction. + - `nip34RepoLoader` / `nip34SupplementalRelayLoader` subscribe to the base + **and** extra groups in `outbox` mode, and additionally query each + discovered item's **author** inbox relays (`MAX_AUTHOR_INBOX_RELAYS = 3`). + - Deletions come from base + extra relays in `outbox` mode. + +## How the pinned Nostr SDK handles NIP-65 + +The SDK ships a full NIP-65 outbox model. `signed` configures it but, as of +the audit, no code path exercises it: every fetch uses an explicit relay list. + +- `crates/signed_nostr/src/backend.rs` builds the client with + `.gossip(NostrGossipMemory::unbounded())` and + `.gossip_config(GossipConfig::default().no_background_refresh())`. +- `nostr-sdk/src/client/builder.rs`: + - `GossipConfig { limits, allowed, sync/fetch timeouts, fetch_chunks, background_refresh }`. + `GossipRelayLimits` defaults: read 3, write 3, hint 1, most-used 1, + NIP-17 3, per user. `GossipAllowedRelays` gates onion/local/plain-TLS, not + read/write. + - `no_background_refresh()` disables the periodic refresher that re-fetches + NIP-65 lists for tracked and DB-seen keys. +- `nostr-sdk/src/client/api/req_target.rs`: `ReqTarget::auto` (from a `Filter`, + `Vec`, or `[Filter; N]`) vs `ReqTarget::manual` (from a relay map or + `(relay, filters)` pairs). +- `nostr-sdk/src/client/api/util.rs::build_targets` and + `nostr-sdk/src/client/api/sync.rs`: + - **Auto** target + gossip configured -> filters are broken down by NIP-65. + - **Manual** target (e.g. `client.sync(f).with(urls)`, or an explicit + `HashMap>`), or gossip unset -> no breakdown. +- `nostr-sdk/src/client/gossip/updater.rs::gossip_break_down_filter`: + - Extracts pubkeys via `Filter::extract_public_keys` + (`nostr/src/filter/mod.rs`), which reads **only `authors` and `#p`**. + - Ensures those pubkeys' NIP-65 lists are fresh (`ensure_gossip_public_keys_fresh` + syncs kind `10002` over `DISCOVERY | READ` relays), then breaks the filter + down (`gossip/resolver.rs::break_down_filter`): + - `authors` only -> each author's **write** relays (+ hints, + most-received). + - `#p` only -> each pubkey's **read** relays (+ hints, + most-received). + - both -> union of read + write relays. + - neither (`Other`) or no relay found (`Orphan`) -> the pool's **read** + relays (`pool/mod.rs::read_relay_urls`). + - Resolved gossip relays are added with `RelayCapabilities::GOSSIP` + (`relay/capabilities.rs`), which does **not** include `READ`. +- `gossip/nostr-gossip/src/lib.rs` exposes the public `NostrGossip` trait: + `get_best_relays(pubkey, BestRelaySelection, GossipAllowedRelays)`, plus + `process`. `NostrGossipMemory` (re-exported via + `nostr_gossip_memory::prelude`) implements it, so a retained handle can + resolve a pubkey's relays directly. It is a pure store read: it does not + fetch the kind `10002` itself. The store is primed by the freshness step of + an Auto request (or by any kind `10002` that arrives for another reason), + and marks a key outdated 24 h after its last fetch attempt. +- `client.send_event(event)` without `.broadcast()` is itself NIP-65 aware: + with gossip configured it resolves the author's outbox and the `p` tags' + inboxes. `signed` calls `.broadcast()` everywhere, so publishing goes to the + pool's write relays and is unaffected by this setting. + +### What this means for repository filters + +`RepoStore::repo_filters` mixes filter shapes. It is the **filter** (not the +event) that `extract_public_keys` reads, so filter tags decide what the SDK can +resolve: + +- announcement + state: `.author(owner).identifier(id)` -> has `authors`, so an + Auto request resolves the owner's **write** relays. +- `activity(addr)`: `.coordinate(addr)` only (`#a`). The events do carry the repo + owner in a lowercase `p` tag, per NIP-34 - issues `1621`, patches `1617`, PRs + `1618`, PR updates `1619`, statuses `1630..=1633` (see the SDK's + `nostr/src/nips/nip34.rs` builders and `RepoStore::set_status` / + `publish_patch_series`) - but the filter never asks for `p`, so + `extract_public_keys` is empty and the filter is classified `Other`. +- `deletions_for_repo(addr)`: one `.author(owner)` filter (owner write relays) + and one `.coordinate(addr)` filter (`Other`). + +Adding `.pubkey(owner)` (or all `effective_maintainers()`) to the activity +filter makes an Auto request resolve those pubkeys' **read** relays (the +`#p`-only branch, plus hints and most-received). Two caveats: + +- `Filter::pubkey` ANDs with `.coordinate`. Root kinds and statuses carry the + owner's `p`, per the SDK's `nostr/src/nips/nip34.rs` builders and + `RepoStore::set_status` / `publish_patch_series`, but kind-1111 comments set + `p` to the parent author (`nostr/src/nips/nip22.rs::as_vec` emits the root + as uppercase `E`/`K`/`P` and the parent as lowercase `e`/`k`/`p`), which for + a top-level comment is the issue/PR author, not the repository owner. A + single `#a` + `#p` filter can drop comments. Keep them as two filters (a `#a` + filter plus a `#p` filter, unioned in the database) or accept the loss. +- `#p` yields the **read** (inbox) relays only. GitWorkshop's extra group is + outbox **and** inbox, so covering the write side still needs an explicit + target. + +## Current behaviour in `signed` + +Global discovery is unchanged: `RepoListStore::subscribe_remote` +(`crates/signed_state/src/repos.rs`) syncs `all_announcements`, `all_states`, +and `deletions` from `BOOTSTRAP_RELAYS`. + +Per-repository fetching in `crates/signed_state/src/repo.rs`: + +- `RepoStore::subscribe_remote` -> `Backend::subscribe_bootstrap`, a one-shot + REQ of `repo_filters` on `BOOTSTRAP_RELAYS`. It passes an explicit + `HashMap<&str, Vec>`, i.e. a **Manual** target, so gossip is skipped. +- `RepoStore::connect_announced_relays` -> `Backend::connect_repo_relays`, + which connects the announcement's `relays` tag and negentropy-`sync`s + `repo_filters` with `.with(relays.iter())`, again a **Manual** target. +- `RepoStore::run_refresh` reads results back from the local database. + +Net effect: a repository's activity and per-repo deletions are fetched from the +global bootstrap relays and its announced relays. Although `signed` configures a +gossip store, **every current fetch path uses manual targets and bypasses the +SDK's NIP-65 handling**. + +## Proposed behaviour + +```mermaid +flowchart TD + A[RepoStore opens repo] --> B{Event fetching strategy} + B -->|Curated| C[Repo-declared relays\nmanual targets] + B -->|Uncensored| D[Resolve maintainers' relays\nvia NostrGossip] + D --> E[Repo-declared + maintainer relays] + C --> F[fetch repo_filters] + E --> F + F --> G[Local database] + G --> H[run_refresh] +``` + +- **Curated**: fetch `repo_filters` from the announcement's `relays` tag only. +- **Uncensored**: additionally fetch the same filters from every + `Announcement::effective_maintainers()`'s NIP-65 write + read relays. + +Announcements, state events, and global deletions keep arriving from the global +`RepoListStore` bootstrap sync in both modes. + +### Recommended: an Auto request plus a Manual request + +Both can run on the same `Client`. They are independent - the pool supports +concurrent subscriptions and syncs, and events deduplicate in the database. Use +each for what it is good at: + +1. **Auto**: pass filters straight to `client.subscribe(filters)` / + `client.sync(filter)` (no `.with(...)`). Broadening the author-scoped + filters (announcement, state, author-scoped deletions) to + `effective_maintainers()` resolves each maintainer's NIP-65 **write** relays; + adding `.pubkey(..)` to the activity filter (see above) resolves their + **read** relays. This request also drives `ensure_gossip_public_keys_fresh`, + populating the gossip store. That side effect is what makes step 2 possible + at all, so the Auto request must run before resolution; resolution is a + pure store read. +2. **Manual**: the activity filter cannot reach maintainers' **write** relays + through the Auto path (that branch needs `authors`), so cover the outbox side + explicitly: resolve each maintainer's relays from the retained gossip handle + via `get_best_relays(pk, BestRelaySelection::All { .. }, ...)`, then + negentropy-`sync` through the existing `Backend::connect_repo_relays`. `All` + returns the union of read, write, hint and most-received relays, i.e. + GitWorkshop's outbox **and** inbox. Because this leg targets the whole + filter, it also covers comments that an `#a` + `#p` filter would drop. + +Retain the gossip store handle so step 2 can resolve relays: + +```rust +let gossip = Arc::new(NostrGossipMemory::unbounded()); +let client = ClientBuilder::default().gossip(gossip.clone()) /* ... */ .build(); +``` + +Store `gossip` (as `Arc`) on `Backend`, then call +`gossip.get_best_relays(..)` per maintainer, union the results, drop relays +already in `repo_relays`, and track the rest in a new +`mailbox_relays: HashSet`. + +Prefer splitting by filter shape (author-scoped -> Auto, coordinate-scoped -> +Manual) to avoid duplicate REQs. Sending the full `repo_filters` through both is +also valid; the database dedups events, at the cost of re-querying overlapping +relays. + +### Variant: Auto only + +Drop the manual maintainer request and pass `repo_filters` directly to +`client.subscribe(filters)` / `client.sync(filter)`, adding `.pubkey(..)` to the +activity filter. Simpler, but activity then reaches only maintainers' **read** +relays (plus hints/most-received); their write relays never receive it. + +Recommendation: the Auto + Manual combination for `Uncensored`, matching +GitWorkshop's base + extra relay groups. + +## Setting and UI + +Add to `crates/settings/src/settings.rs`, following the bare-enum pattern of +`AppearanceMode`: + +```rust +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum EventFetchingStrategy { + /// Only the relays declared in the repository announcement. + #[default] + Curated, + /// Repository relays plus every maintainer's NIP-65 relays. + Uncensored, +} +``` + +Add `pub event_fetching: EventFetchingStrategy` to `Settings`, and a section to +`crates/workspace/src/views/sidebar/settings_dialog.rs` rendered from +`settings_view`, reusing the `Select` used by `appearance_section` (or +`setting_block` from `signed_ui` for a two-card layout closer to GitWorkshop). + +### Default + +Recommend **Uncensored**, matching GitWorkshop. Curated remains the +spam-resistant choice. Note the default changes what existing users fetch. + +## Files to change + +- `crates/settings/src/settings.rs` - enum and `Settings` field. +- `crates/workspace/src/views/sidebar/settings_dialog.rs` - new section. +- `crates/signed_nostr/src/backend.rs` - retain the `NostrGossipMemory` handle; + optionally reconsider `no_background_refresh`. +- `crates/signed_state/src/backend.rs` - store the gossip handle; expose a + maintainer-relay resolver using `get_best_relays`; reuse `connect_repo_relays`. +- `crates/signed_state/src/repo.rs` - strategy-aware `subscribe_remote` / + `connect_announced_relays`, new `connect_maintainer_relays`, new + `mailbox_relays` field. +- `crates/signed_core/src/filters.rs` - `relay_list(public_keys)` filter for the + freshness trigger. Note `ensure_gossip_public_keys_fresh` is private to the + client; an Auto request naming the pubkeys is the public way to trigger it, so + this helper is only useful as part of an Auto filter list. + +## Open questions / decisions + +1. **Activity `#p` shaping.** Add a second `.pubkey(maintainers)` activity + filter so the Auto leg reaches maintainer inboxes, or keep the activity + filter coordinate-only? The manual leg already reaches read and write relays + for the whole filter, so comments are covered either way; this only decides + how much the Auto leg contributes. +2. **Bootstrap REQ in Curated mode.** Recommended: drop it for this repository + (global `RepoListStore` still covers announcements, state, deletions). + Alternative: keep it and make the setting purely additive. +3. **Gossip freshness.** Trigger kind `10002` on demand per repo, or re-enable + the SDK background refresher? Background refresh is currently disabled in + `signed_nostr/src/backend.rs`. +4. **Runtime changes.** Re-fetch on `SettingsStore` edit (subscribe + `RepoStore`), or apply on next open? +5. **Per-item author inbox relays.** GitWorkshop also queries each discovered + item author's inbox relays in `outbox` mode. Larger change; propose a phase 2. + +## Testing + +- Unit-test maintainer-relay resolution: given maintainers and a gossip store + seeded with kind `10002`, assert the resolved, deduped relay set reaches + `connect_repo_relays`. +- Assert Curated uses only the announced relays. +- Keep the `settings` round-trip tests updated for the new field. + +## Phasing + +1. Setting, UI, and gossip-handle plumbing. +2. Curated wiring (bootstrap REQ gating) + Uncensored maintainer relay fetch. +3. Runtime re-fetch on setting change. +4. (Optional) per-item author inbox relays.