From 3a70bb7228ec7481aef0b2303be4ccb52c0e1a2b Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 26 Sep 2026 16:18:28 +0700 Subject: [PATCH] add fetch strategy --- CHANGELOG.md | 1 + crates/signed_state/src/backend.rs | 15 + crates/signed_state/src/repo.rs | 138 ++++++- .../src/views/sidebar/settings_dialog.rs | 46 ++- docs/backend-audit.md | 217 ----------- docs/event-fetching-strategy.md | 349 ------------------ 6 files changed, 197 insertions(+), 569 deletions(-) delete mode 100644 docs/backend-audit.md delete mode 100644 docs/event-fetching-strategy.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e8d330..1981338 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Show an avatar in each panel's tab, using the repository owner's profile picture when set and a pixel avatar otherwise - Add a Tab Bar setting to hide the previous/next tab buttons, hidden by default +- Add an Event Fetching Strategy setting, fetching a repository's activity from its announced relays only (Curated) or from every maintainer's relays as well (Uncensored, the default) ### Changed diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 156606c..0d6998b 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -1192,6 +1192,21 @@ impl Backend { .detach(); } + /// Sync filters through the SDK's NIP-65 gossip targeting. + pub fn sync_auto(&mut self, filters: Vec, cx: &mut Context) { + let client = self.client.clone(); + + cx.spawn(async move |_this, _cx| { + for filter in filters { + if let Err(e) = client.sync(filter).await { + log::warn!("gossip relay fetch failed: {e}"); + } + } + Ok::<(), Error>(()) + }) + .detach(); + } + pub fn subscribe_bootstrap(&mut self, filters: Vec, cx: &mut Context) { let client = self.client.clone(); diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 9ba4a81..c024078 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -7,6 +7,7 @@ use bitcoin_hashes::sha1::Hash as Sha1Hash; use gpui::{App, AppContext, AsyncApp, Context, SharedString, Subscription, Task, WeakEntity}; use nostr::event::IntoEventBuilder; use nostr_sdk::prelude::*; +use settings::{EventFetchingStrategy, SettingsStore}; use signed_core::{ Announcement, Deletions, RepoAddr, RepoStatus, filters, parse_state, pull_request_patch, pull_request_patches, @@ -84,6 +85,10 @@ pub struct RepoStore { /// /// The per-root fetches cover NIP-22 comments and statuses without an `a` tag. root_fetches: HashSet, + /// Maintainers already synced through gossip in Uncensored mode. + /// + /// Avoids re-running the maintainer Auto sync on every refresh. + synced_maintainers: HashSet, refresh: RefreshGate, /// Backend subscription of an announced repository. `None` while local-only. _subscription: Option, @@ -132,6 +137,7 @@ impl RepoStore { cloning: false, repo_relays: HashSet::new(), root_fetches: HashSet::new(), + synced_maintainers: HashSet::new(), refresh: RefreshGate::default(), _subscription: Some(subscription), } @@ -160,6 +166,7 @@ impl RepoStore { cloning: false, repo_relays: HashSet::new(), root_fetches: HashSet::new(), + synced_maintainers: HashSet::new(), refresh: RefreshGate::default(), _subscription: None, } @@ -295,6 +302,67 @@ impl RepoStore { }); } + /// Filters the SDK resolves through NIP-65 gossip in Uncensored mode. + fn maintainer_filters(addr: &RepoAddr, maintainers: &[PublicKey]) -> Vec { + let mut pubkeys = maintainers.to_vec(); + // NIP-34 events tag the announcement author, + // which may not be a maintainer for subordinate forks. + if !pubkeys.contains(&addr.public_key) { + pubkeys.push(addr.public_key); + } + + vec![ + // Announcement and state events, including co-maintainer states. + Filter::new() + .kinds([Kind::GitRepoAnnouncement, Kind::RepoState]) + .authors(pubkeys.clone()) + .identifier(addr.identifier.clone()), + // Activity tagging a maintainer, resolved to their read relays. + Filter::new() + .kinds(filters::ACTIVITY_KINDS) + .coordinate(addr) + .pubkeys(pubkeys.clone()), + // Activity authored by a maintainer, resolved to their write relays. + Filter::new() + .kinds(filters::ACTIVITY_KINDS) + .coordinate(addr) + .authors(pubkeys.clone()), + // Deletions authored by a maintainer. + Filter::new() + .kinds([Kind::EventDeletion, Kind::RequestToVanish]) + .authors(pubkeys), + ] + } + + /// In Uncensored mode, sync the maintainer-shaped filters through the SDK's NIP-65 gossip targeting + fn sync_maintainer_relays(&mut self, maintainers: &[PublicKey], cx: &mut Context) { + let strategy = SettingsStore::try_global(cx) + .map(|store| store.read(cx).settings().event_fetching) + .unwrap_or_default(); + if strategy != EventFetchingStrategy::Uncensored { + return; + } + + let Some(addr) = self.addr.clone() else { + return; + }; + + if !maintainers + .iter() + .any(|public_key| !self.synced_maintainers.contains(public_key)) + { + return; + } + self.synced_maintainers.extend(maintainers.iter().copied()); + + let filters = Self::maintainer_filters(&addr, maintainers); + let backend = Backend::global(cx); + + backend.update(cx, |backend, cx| { + backend.sync_auto(filters, cx); + }); + } + fn subscribe_remote(&mut self, cx: &mut Context) { let Some(addr) = self.addr.clone() else { return; @@ -489,7 +557,6 @@ impl RepoStore { } // The announcement may list relays for this repository's activity. - // Connect to any we have not fetched from yet. let relays = this .announcement .as_ref() @@ -498,6 +565,15 @@ impl RepoStore { this.connect_announced_relays(&relays, cx); + // Uncensored mode also covers the maintainers' NIP-65 relays. + let maintainers = this + .announcement + .as_ref() + .map(Announcement::effective_maintainers) + .unwrap_or_default(); + + this.sync_maintainer_relays(&maintainers, cx); + if let Some((_, head)) = state { this.head = head; } @@ -1734,9 +1810,11 @@ fn comment_builder( #[cfg(test)] mod tests { + use std::collections::HashSet; + use nostr_sdk::prelude::*; - use super::{comment_builder, patch_current_commit}; + use super::{RepoStore, comment_builder, patch_current_commit}; #[test] fn parses_format_patch_header() { @@ -1782,4 +1860,60 @@ mod tests { // Signed's own `references_root` must keep matching the comment. assert!(signed_core::references_root(&event, &root.id)); } + + #[test] + fn maintainer_filters_name_owner_and_maintainers() { + let owner = Keys::generate().public_key(); + let maintainer = Keys::generate().public_key(); + let addr = Coordinate::new(Kind::GitRepoAnnouncement, owner).identifier("my-repo"); + + // The owner is not among the maintainers, as on a subordinate fork. + let filters = RepoStore::maintainer_filters(&addr, &[maintainer]); + assert_eq!(filters.len(), 4); + + let expected = HashSet::from([owner, maintainer]); + + // Gossip only resolves pubkeys from `authors` and the lowercase `#p` tag. + let named = |filter: &Filter| -> HashSet { + let authors = filter.authors.iter().flatten().copied(); + let p_tag = filter + .generic_tags + .get(&SingleLetterTag::LOWERCASE_P) + .into_iter() + .flatten() + .filter_map(|value| PublicKey::from_hex(value).ok()); + authors.chain(p_tag).collect() + }; + + // Announcement and state events, scoped to the repository identifier. + let announcement = &filters[0]; + assert_eq!(named(announcement), expected); + assert!( + announcement + .generic_tags + .contains_key(&SingleLetterTag::LOWERCASE_D) + ); + + // Activity filters, scoped to the repository coordinate. + for filter in &filters[1..3] { + assert_eq!(named(filter), expected); + assert!( + filter + .generic_tags + .contains_key(&SingleLetterTag::LOWERCASE_A) + ); + } + + // Deletions, named by author only. + let deletions = &filters[3]; + assert_eq!( + deletions + .authors + .clone() + .unwrap_or_default() + .into_iter() + .collect::>(), + expected + ); + } } diff --git a/crates/workspace/src/views/sidebar/settings_dialog.rs b/crates/workspace/src/views/sidebar/settings_dialog.rs index 52a129b..e986a3c 100644 --- a/crates/workspace/src/views/sidebar/settings_dialog.rs +++ b/crates/workspace/src/views/sidebar/settings_dialog.rs @@ -18,7 +18,7 @@ use gpui_component::{ ActiveTheme, IconName, IndexPath, Sizable, Theme, ThemeMode, ThemeRegistry, WindowExt, h_flex, v_flex, }; -use settings::{AppearanceMode, Settings, SettingsStore}; +use settings::{AppearanceMode, EventFetchingStrategy, Settings, SettingsStore}; use signed_ui::{SelectOption, setting_block, setting_row}; use super::{normalize_server, server_host}; @@ -52,6 +52,7 @@ fn theme_options(cx: &App) -> (Vec, Vec) { /// Created once when the dialog opens, so control state survives re-renders. struct SettingsControls { appearance: Entity>>, + event_fetching: Entity>>, light_theme: Entity>>, dark_theme: Entity>>, font_size: Entity, @@ -89,6 +90,23 @@ impl SettingsControls { ) }); + let event_fetching_options = vec![ + SelectOption::new("curated", "Curated"), + SelectOption::new("uncensored", "Uncensored"), + ]; + let event_fetching_value = match settings.event_fetching { + EventFetchingStrategy::Curated => "curated", + EventFetchingStrategy::Uncensored => "uncensored", + }; + let event_fetching = cx.new(|cx| { + SelectState::new( + event_fetching_options.clone(), + selected_index(&event_fetching_options, event_fetching_value), + window, + cx, + ) + }); + let (light_options, dark_options) = theme_options(cx); let light_theme = cx.new(|cx| { SelectState::new( @@ -147,6 +165,19 @@ impl SettingsControls { } })); + subscriptions.push(cx.subscribe(&event_fetching, |_, event, cx| { + if let SelectEvent::Confirm(Some(value)) = event { + let strategy = match value.as_ref() { + "curated" => EventFetchingStrategy::Curated, + _ => EventFetchingStrategy::Uncensored, + }; + let store = SettingsStore::global(cx); + store.update(cx, |store, cx| { + store.edit(|settings| settings.event_fetching = strategy, cx); + }); + } + })); + subscriptions.push(cx.subscribe(&light_theme, |_, event, cx| { if let SelectEvent::Confirm(Some(value)) = event { let store = SettingsStore::global(cx); @@ -252,6 +283,7 @@ impl SettingsControls { Self { appearance, + event_fetching, light_theme, dark_theme, font_size, @@ -294,6 +326,8 @@ fn settings_view(controls: &SettingsControls, cx: &mut App) -> impl IntoElement .child(Separator::horizontal()) .child(grasp_servers_section(&settings, controls, cx)) .child(Separator::horizontal()) + .child(event_fetching_section(controls, cx)) + .child(Separator::horizontal()) .child(repositories_section(&settings, controls, cx)) } @@ -306,6 +340,16 @@ fn appearance_section(controls: &SettingsControls, cx: &App) -> impl IntoElement )) } +/// Which relays repository activity is fetched from. +fn event_fetching_section(controls: &SettingsControls, cx: &App) -> impl IntoElement { + v_flex().w_full().gap_3().child(setting_row( + cx, + "Event Fetching Strategy", + "Curated fetches events from relays in the repository's announcement. Uncensored also fetches from every maintainer's relays.", + Select::new(&controls.event_fetching).w_full(), + )) +} + fn theme_section(settings: &Settings, controls: &SettingsControls, cx: &App) -> impl IntoElement { v_flex() .gap_3() diff --git a/docs/backend-audit.md b/docs/backend-audit.md deleted file mode 100644 index 0d0969a..0000000 --- a/docs/backend-audit.md +++ /dev/null @@ -1,217 +0,0 @@ -# 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 deleted file mode 100644 index d267c9c..0000000 --- a/docs/event-fetching-strategy.md +++ /dev/null @@ -1,349 +0,0 @@ -# Plan: Event Fetching Strategy (`Curated` / `Uncensored`) - -Status: implementation plan, not implemented. This document replaces the -earlier proposal of the same name; it keeps the verified background from it -and `docs/backend-audit.md` (authoritative for current behaviour). - -Cross-checked against: - -- rust-nostr at the revision from `Cargo.lock`, - `b230cecf9dbb38e0228e6fff4544ed9d261326fc` (local checkout - `~/.cargo/git/checkouts/nostr-619b808bb247a9ed/b230cec`). -- GitWorkshop at `420c0c3`. - -## Goal - -Mirror GitWorkshop's "Event Fetching Strategy" for the per-repository fetches -in `RepoStore`: - -- **Curated**: only the relays declared in the repository announcement. -- **Uncensored**: the repository's declared relays plus every maintainer's - NIP-65 relays. - -Global discovery is unchanged in both modes: `RepoListStore` keeps syncing -announcements, states and deletions from `BOOTSTRAP_RELAYS`. - -## GitWorkshop reference - -- `src/services/settings.ts:197-210`: `RelayCurationMode = "repo" | "outbox"`, - persisted to `localStorage`, default `"outbox"` (Uncensored). -- `src/pages/Settings.tsx:85-100`: two selectable cards, "Curated" and - "Uncensored". -- `src/hooks/useResolvedRepository.ts`: `repoRelayGroup` is the announcement's - `relays` tag. `extraRelaysForMaintainerMailboxCoverage` is a delta group of - every maintainer's NIP-65 outbox + inbox relays, excluding relays already in - the repo group, capped at `MAX_MAILBOX_RELAYS_PER_USER = 3` per direction - (`addMailboxRelaysToGroup`). The pubkey set is the announcement chain's - `discoveryPubkeys` (maintainers, moderators, owner). -- Gating: `src/hooks/useNip34Loaders.ts:447` and - `src/pages/repo/RepoLayout.tsx:319-360` only subscribe the item loaders to - the maintainer group when the mode is `outbox`. Curated never touches - maintainer relays. - -## Current behaviour in `signed` - -`crates/signed_state/src/repo.rs`: - -- `subscribe_remote` -> `Backend::subscribe_bootstrap`: one-shot REQ of - `repo_filters` on `BOOTSTRAP_RELAYS` (manual target). -- `connect_announced_relays` -> `Backend::connect_repo_relays`: add and - connect the announcement's `relays` tag, then negentropy-`sync` - `repo_filters` (manual target). Deduped by the `repo_relays` set. Called - from `new`, `announce`, and on every announcement change in `run_refresh`. -- `run_refresh` also fetches comments (`filters::comments_for`) and statuses - (`filters::statuses_for`) for each newly seen root from bootstrap + - `repo_relays`. -- No fetch path uses NIP-65: the gossip store is configured but never - consulted (`docs/backend-audit.md`, section 2). - -## Design - -```mermaid -flowchart TD - A[RepoStore opens a repo] --> B{Event fetching strategy} - B -->|Curated| C[Manual sync to repo-declared relays] - B -->|Uncensored| D[Auto sync: SDK resolves maintainers' NIP-65 relays from authors and #p] --> E[Manual sync to repo-declared relays] - C --> F[(Local database)] - D --> F - E --> F -``` - -1. **Curated stays exactly what the code does today**: bootstrap REQ plus the - manual announced-relay sync. -2. **Uncensored adds one SDK Auto sync.** No relay URLs are resolved, stored, - or tracked, and no kind `10002` events are fetched or parsed by `signed`: - the SDK's NIP-65 gossip targeting resolves the maintainers' relays per - request (`client.sync(filter)` with no `.with(..)`, verified in - `nostr-sdk/src/client/api/sync.rs:151-166` and - `api/util.rs:12-29`). The existing manual announced-relay sync stays for - the repo's own relays in both modes. -3. **The filters must name the maintainers.** Gossip resolution is driven by - `Filter::extract_public_keys` (`nostr/src/filter/mod.rs:591`), which reads - only `authors` and the lowercase `#p` tag: - - | Filter shape | Auto target | - | --- | --- | - | `authors` only | each author's NIP-65 **write** relays | - | `#p` only | each pubkey's NIP-65 **read** relays | - | both | union of read and write relays | - | neither | the pool's read relays | - - Today's `repo_filters` resolve nothing for this purpose: the - announcement/state filters carry only the owner in `authors`, and the - activity filter is `#a`-only, so it is classified `Other` and falls back to - the pool's read relays. Uncensored therefore sends a separate - maintainer-shaped filter set to the Auto sync. - -4. **Maintainer filter set** (Uncensored only): - - ```rust - /// Filters the SDK resolves through NIP-65 gossip in Uncensored mode. - /// - /// Gossip reads pubkeys from `authors` and the lowercase `#p` tag only, - /// so every filter names the owner and the maintainers. - fn maintainer_filters(addr: &RepoAddr, maintainers: &[PublicKey]) -> Vec { - let mut pubkeys = maintainers.to_vec(); - // NIP-34 events tag the announcement author, which may not be a - // maintainer for subordinate forks. - if !pubkeys.contains(&addr.public_key) { - pubkeys.push(addr.public_key); - } - - vec![ - // Announcement and state events, including co-maintainer states, - // resolved to write relays. - Filter::new() - .kinds([Kind::GitRepoAnnouncement, Kind::RepoState]) - .authors(pubkeys.clone()) - .identifier(addr.identifier.clone()), - // Activity tagging a maintainer, resolved to their read relays. - Filter::new() - .kinds(filters::ACTIVITY_KINDS) - .coordinate(addr) - .pubkeys(pubkeys.clone()), - // Activity authored by a maintainer, resolved to their write relays. - Filter::new() - .kinds(filters::ACTIVITY_KINDS) - .coordinate(addr) - .authors(pubkeys.clone()), - // Deletions authored by a maintainer, resolved to write relays. - Filter::new() - .kinds([Kind::EventDeletion, Kind::RequestToVanish]) - .authors(pubkeys), - ] - } - ``` - - `maintainers` is `Announcement::effective_maintainers()`, already computed - by `run_refresh`. Because the announcement/state filter names every - maintainer, co-maintainer state events (kind `30618`) land in the local - database. They are not displayed yet: `run_refresh` reads state through - the owner-only `filters::state` (`docs/backend-audit.md`, finding 3). - -5. **The manual leg is unchanged**: `subscribe_bootstrap` plus - `connect_announced_relays` keep covering bootstrap and repo-declared - relays. Per-root follow-ups (`comments_for`, `statuses_for`) also stay - as-is. - -6. **Default: Uncensored**, matching GitWorkshop - (`DEFAULT_RELAY_CURATION_MODE = "outbox"`). Flagged under "Decisions" - because it changes what existing users fetch. - -### Coverage - -- The `#p` activity filter finds roots and statuses that tag a maintainer - (NIP-34 root events and statuses tag the owner) on the maintainers' read - relays. -- The `authors` activity filter finds maintainer-authored events (issues, - PRs, patches, comments, statuses) on their write relays. `signed`'s - comments and statuses carry the `a` tag - (`comment_builder`, `set_status`, `publish_applied_status`), so this filter - does not drop them. -- Comments whose parent author is not a maintainer are not matched by the - `#p` filter, but they are published to the parent author's inbox, not to a - maintainer's relays; maintainer-authored comments are still caught by the - `authors` filter. -- Not covered: status events without an `a` tag published to a maintainer's - outbox. Per-root `#e` filters carry no pubkeys, so they cannot resolve - through gossip; GitWorkshop covers these with per-item supplemental - queries (out of scope below). - -## Changes - -### 1. Setting, `crates/settings/src/settings.rs` - -Follow the bare-enum pattern of `AppearanceMode`: - -```rust -/// Which relays `RepoStore` queries for a repository's activity. -#[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. - Curated, - /// Repository relays plus every maintainer's NIP-65 relays. - #[default] - Uncensored, -} -``` - -Add `pub event_fetching: EventFetchingStrategy` to `Settings`. - -### 2. Settings UI, `crates/workspace/src/views/sidebar/settings_dialog.rs` - -- Options: `SelectOption::new("curated", "Curated")` and - `SelectOption::new("uncensored", "Uncensored")`. -- Add an `event_fetching: Entity>>` field to - `SettingsControls`, seeded from the persisted value, and a - `SelectEvent::Confirm` subscription that maps the value to the enum and - calls `store.edit(|settings| settings.event_fetching = strategy, cx)`. -- Add an `event_fetching_section` next to `appearance_section`, using - `setting_row` with the title "Event Fetching Strategy" and a description of - both modes, and insert it in `settings_view`. - -### 3. Backend, `crates/signed_state/src/backend.rs` - -New method next to `connect_repo_relays`: - -```rust -/// Sync filters through the SDK's NIP-65 gossip targeting. -/// -/// `client.sync(filter)` without `.with(..)` is an Auto request: the SDK -/// resolves each filter's `authors` and lowercase `#p` pubkeys to their -/// NIP-65 relays, connects them, and negentropy-syncs there. -pub fn sync_auto(&mut self, filters: Vec, cx: &mut Context) { - let client = self.client.clone(); - - cx.spawn(async move |_this, _cx| { - for filter in filters { - if let Err(e) = client.sync(filter).await { - log::warn!("gossip relay fetch failed: {e}"); - } - } - Ok::<(), Error>(()) - }) - .detach(); -} -``` - -Errors stay log-only, like `connect_repo_relays`; `BackendEvent::Synced` is -deliberately not emitted (it would refresh `RepoListStore` for repo-level -traffic). - -### 4. `RepoStore`, `crates/signed_state/src/repo.rs` - -New field, initialized empty in `new` and `new_local`: - -```rust -/// Maintainers already synced through gossip in Uncensored mode. -synced_maintainers: HashSet, -``` - -New methods after `connect_announced_relays`: `maintainer_filters` (listed -under Design) and - -```rust -/// In Uncensored mode, sync this repository's maintainer-shaped filters -/// through the SDK's NIP-65 gossip targeting. -fn sync_maintainer_relays(&mut self, maintainers: &[PublicKey], cx: &mut Context) { - let strategy = settings::SettingsStore::try_global(cx) - .map(|store| store.read(cx).settings().event_fetching) - .unwrap_or_default(); - if strategy != EventFetchingStrategy::Uncensored { - return; - } - - let Some(addr) = self.addr.clone() else { - return; - }; - - if !maintainers - .iter() - .any(|pk| !self.synced_maintainers.contains(pk)) - { - return; - } - self.synced_maintainers.extend(maintainers.iter().copied()); - - let filters = Self::maintainer_filters(&addr, maintainers); - let backend = Backend::global(cx); - backend.update(cx, |backend, cx| backend.sync_auto(filters, cx)); -} -``` - -Hook into `run_refresh`, in the foreground update after -`connect_announced_relays`: - -```rust -let maintainers = this - .announcement - .as_ref() - .map(Announcement::effective_maintainers) - .unwrap_or_default(); -this.sync_maintainer_relays(&maintainers, cx); -``` - -Notes: - -- `SettingsStore::try_global` keeps wasm safe: the settings store is only - installed by the desktop app (`desktop/src/main.rs:21`). -- No new crate dependencies: `signed_state` already depends on `settings` and - `nostr_sdk`. - -## Tests - -- `crates/settings`: extend `json_roundtrip_preserves_everything` and - `partial_json_merges_with_defaults` for `event_fetching` (snake_case - values, default). -- `crates/signed_state/src/repo.rs`, `mod tests`: unit-test - `maintainer_filters`: every filter names the owner and maintainers via - `authors` or `#p` (the announcement/state filter included), activity - filters carry the `#a` coordinate, and the owner is added for a - subordinate fork. -- `cargo test -p settings -p signed_state` and - `cargo check -p signed_workspace` for the UI. -- Manual smoke: open a repository whose activity exists only on a - maintainer's relays (not on the announced relays or bootstrap) in both - modes. - -## Decisions - -1. **Default.** Uncensored, to match GitWorkshop. Curated preserves today's - relay traffic; flipping the default is a one-line change. -2. **Bootstrap REQ stays in Curated.** It is the index path that keeps - repositories with unreachable announced relays usable. Strict GitWorkshop - parity (repo relays only in Curated) is possible later but is a behaviour - change unrelated to the option itself. -3. **Read and write coverage is approximated by two activity filters** (`#p` - and `authors`) rather than a resolved "all maintainer relays" set. The SDK - resolves the relay sets per filter shape; `signed` stores no relay URLs. -4. **The announcement/state Auto filter names every maintainer plus the - owner.** This also fetches co-maintainer state events into the local - database; showing them is a separate display-side change. -5. **Runtime switching.** `sync_maintainer_relays` reads the setting on every - `run_refresh`, so switching to Uncensored applies at the next refresh - without a restart. Switching back stops new Auto syncs but does not undo - relays already resolved by the gossip pool. -6. **Gossip pool growth.** Auto requests add resolved relays with - `RelayCapabilities::GOSSIP` and never remove them for the session - (`docs/backend-audit.md`); accepted, as the SDK is designed this way. -7. **Errors stay log-only**, consistent with existing background fetches. - -## Out of scope - -- Status events without an `a` tag published to maintainer outboxes - (GitWorkshop's per-item supplemental loader). -- Per-item author inbox relays - (`src/services/nostr.ts:1042,1070`, `MAX_AUTHOR_INBOX_RELAYS = 3`). -- Co-maintainer state display: co-maintainer states are fetched (decision 4) - but `run_refresh` still reads state through the owner-only - `filters::state` (`docs/backend-audit.md`, finding 3). -- Gating or replacing the bootstrap REQ in Curated mode. - -## Phasing - -1. Setting enum, field and settings tests. **Done.** -2. Settings UI control. -3. `Backend::sync_auto` and `RepoStore::sync_maintainer_relays` with the - maintainer filter set, plus the filter unit test. -4. `cargo test` / `cargo check`, then a manual smoke test.