add fetch strategy
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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<Filter>, cx: &mut Context<Self>) {
|
||||
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<Filter>, cx: &mut Context<Self>) {
|
||||
let client = self.client.clone();
|
||||
|
||||
|
||||
@@ -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<EventId>,
|
||||
/// Maintainers already synced through gossip in Uncensored mode.
|
||||
///
|
||||
/// Avoids re-running the maintainer Auto sync on every refresh.
|
||||
synced_maintainers: HashSet<PublicKey>,
|
||||
refresh: RefreshGate,
|
||||
/// Backend subscription of an announced repository. `None` while local-only.
|
||||
_subscription: Option<Subscription>,
|
||||
@@ -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<Filter> {
|
||||
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<Self>) {
|
||||
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<Self>) {
|
||||
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<PublicKey> {
|
||||
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::<HashSet<_>>(),
|
||||
expected
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<SelectOption>, Vec<SelectOption>) {
|
||||
/// Created once when the dialog opens, so control state survives re-renders.
|
||||
struct SettingsControls {
|
||||
appearance: Entity<SelectState<Vec<SelectOption>>>,
|
||||
event_fetching: Entity<SelectState<Vec<SelectOption>>>,
|
||||
light_theme: Entity<SelectState<Vec<SelectOption>>>,
|
||||
dark_theme: Entity<SelectState<Vec<SelectOption>>>,
|
||||
font_size: Entity<InputState>,
|
||||
@@ -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()
|
||||
|
||||
@@ -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<HashMap<T, Vec<Filter>>>`
|
||||
(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<Filter>>)`, `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.
|
||||
@@ -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<Filter> {
|
||||
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<SelectState<Vec<SelectOption>>>` 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<Filter>, cx: &mut Context<Self>) {
|
||||
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<PublicKey>,
|
||||
```
|
||||
|
||||
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<Self>) {
|
||||
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.
|
||||
Reference in New Issue
Block a user