From 2a077738489d2aa414bd088e7f02e6969755253a Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 10 Oct 2026 08:40:10 +0700 Subject: [PATCH] adopt two-step NIP-29 group creation with confirmation --- crates/nip29/src/lib.rs | 95 ++++++++++++-- crates/nip29/src/protocol.rs | 34 ++++- crates/nip29/src/utils.rs | 41 ++++++ crates/workspace/src/dialogs/new_group.rs | 46 ++----- crates/workspace/src/sidebar/mod.rs | 6 + docs/group-creation-plan.md | 144 ++++++++++++++++++++++ 6 files changed, 317 insertions(+), 49 deletions(-) create mode 100644 docs/group-creation-plan.md diff --git a/crates/nip29/src/lib.rs b/crates/nip29/src/lib.rs index 10420191..38f22f1c 100644 --- a/crates/nip29/src/lib.rs +++ b/crates/nip29/src/lib.rs @@ -23,6 +23,10 @@ pub(crate) static LOCAL_KEYS: LazyLock = LazyLock::new(Keys::generate); const BROWSE_LIMIT: usize = 500; const BROWSE_TIMEOUT: Duration = Duration::from_secs(10); const FORK_INTERVAL: Duration = Duration::from_secs(15 * 60); +const CONFIRM_POLLS: usize = 20; +const CONFIRM_INTERVAL: Duration = Duration::from_millis(250); +const METADATA_WARNING: &str = + "Group created, but the relay did not accept its name. Edit the group to set it."; pub fn init(cx: &mut App) { GroupsRegistry::set_global(cx.new(GroupsRegistry::new), cx); @@ -46,6 +50,11 @@ pub enum GroupsEvent { }, /// The discovery synced new groups into the local database. Synced, + /// A relay accepted a new group; the warning carries a non-fatal issue. + Created { + key: GroupKey, + warning: Option, + }, } #[derive(Debug)] @@ -191,31 +200,93 @@ impl GroupsRegistry { })); } - pub fn create(&mut self, key: GroupKey, metadata: GroupMetadata, cx: &mut Context) { + pub fn create(&mut self, relay: RelayUrl, options: GroupCreateOptions, cx: &mut Context) { + let name = options.name.trim().to_owned(); + + if name.is_empty() { + cx.emit(GroupsEvent::Error("group name is empty".to_owned())); + return; + } + + let raw = options.custom_id.as_deref().unwrap_or_default(); + let id = if raw.is_empty() { + readable_id() + } else { + raw.trim().to_lowercase() + }; + + let id = match GroupId::new(id) { + Ok(id) => id, + Err(error) => { + cx.emit(GroupsEvent::Error(error.to_string())); + return; + } + }; + + let key = GroupKey::new(relay, id); self.ensure(key.clone(), cx); let Some(group) = self.group(&key, cx) else { return; }; - self.upsert_entry(&key, metadata.name().map(str::to_owned)); - self.publish_list(cx); - - cx.emit(GroupsEvent::Updated); - cx.notify(); - - let builder = key.id().create_group(&metadata, None); + let metadata = options.metadata(name.clone()); + let builder = key.id().create_group(None); let published = group.read(cx).publish(builder, cx); - let logged = key.clone(); + let created = key; self.tasks.push(cx.spawn(async move |this, cx| { - if let Err(e) = published.await { + if let Err(error) = published.await { this.update(cx, |this, cx| { - this.forget(logged, cx); - cx.emit(GroupsEvent::Error(e.to_string())); + this.forget(created, cx); + cx.emit(GroupsEvent::Error(error.to_string())); }) .ok(); + return; } + + // Give the relay time to publish the group's metadata event, + // which settles the group id (the relay may override the suggested one). + for _ in 0..CONFIRM_POLLS { + if group.read_with(cx, |group, _cx| group.metadata().is_some()) { + break; + } + cx.background_executor().timer(CONFIRM_INTERVAL).await; + } + + // The relay can reject the metadata while the admin grant from the + // create is still settling, one delayed retry covers that race. + let mut warning = None; + + for attempt in 0..2 { + if attempt > 0 { + cx.background_executor().timer(Duration::from_secs(1)).await; + } + + let published = group.update(cx, |group, cx| group.edit_metadata(&metadata, cx)); + let accepted = published.await.is_ok(); + + if accepted { + break; + } + + if attempt == 1 { + warning = Some(METADATA_WARNING.to_owned()); + } + } + + this.update(cx, |this, cx| { + this.upsert_entry(&created, Some(name)); + this.publish_list(cx); + + cx.emit(GroupsEvent::Updated); + cx.emit(GroupsEvent::Created { + key: created, + warning, + }); + cx.notify(); + }) + .ok(); })); } diff --git a/crates/nip29/src/protocol.rs b/crates/nip29/src/protocol.rs index 596ec1f1..257d8caa 100644 --- a/crates/nip29/src/protocol.rs +++ b/crates/nip29/src/protocol.rs @@ -155,9 +155,9 @@ impl GroupId { .tag_maybe(previous) } - pub fn create_group(&self, metadata: &GroupMetadata, previous: Option) -> EventBuilder { + pub fn create_group(&self, previous: Option) -> EventBuilder { EventBuilder::new(Kind::GroupCreateGroup, "") - .tags(self.metadata_tags(metadata)) + .tag(self.h_tag()) .tag_maybe(previous) } @@ -435,6 +435,36 @@ impl GroupMetadata { } } +/// Input for creating a group: access flags are presence-only per NIP-29. +#[derive(Debug, Clone, Default)] +pub struct GroupCreateOptions { + pub name: String, + pub about: Option, + pub picture: Option, + pub private: bool, + pub closed: bool, + pub restricted: bool, + pub hidden: bool, + pub custom_id: Option, + pub parent: Option, +} + +impl GroupCreateOptions { + pub(crate) fn metadata(&self, name: String) -> GroupMetadata { + GroupMetadata { + name: Some(name), + picture: self.picture.clone(), + about: self.about.clone(), + private: self.private, + restricted: self.restricted, + hidden: self.hidden, + closed: self.closed, + parent: self.parent.clone(), + ..Default::default() + } + } +} + #[derive(Debug, Clone)] pub struct GroupCandidate { pub key: GroupKey, diff --git a/crates/nip29/src/utils.rs b/crates/nip29/src/utils.rs index f66d4bc5..153540ed 100644 --- a/crates/nip29/src/utils.rs +++ b/crates/nip29/src/utils.rs @@ -58,3 +58,44 @@ pub(crate) fn parse_members(event: &Event) -> Option<(GroupId, Timestamp, usize) Some((id, event.created_at, members)) } + +const ONSETS: [&str; 34] = [ + "b", "c", "d", "f", "g", "h", "j", "k", "l", "m", "n", "p", "r", "s", "t", "v", "br", "cr", + "dr", "fr", "gr", "pr", "tr", "bl", "cl", "fl", "gl", "pl", "sl", "sh", "ch", "th", "st", "sk", +]; +const NUCLEI: [&str; 9] = ["a", "e", "i", "o", "u", "ae", "ei", "io", "ou"]; +const CODAS: [&str; 11] = ["", "", "", "n", "r", "s", "t", "d", "l", "m", "k"]; + +/// A short pronounceable id for new groups, seeded from a fresh key. +pub(crate) fn readable_id() -> String { + let mut picks = SecretKey::generate().secret_bytes().into_iter(); + + let mut id = String::with_capacity(18); + + for _ in 0..3 { + let onset = ONSETS[usize::from(picks.next().unwrap_or(0)) % ONSETS.len()]; + let nucleus = NUCLEI[usize::from(picks.next().unwrap_or(0)) % NUCLEI.len()]; + let coda = CODAS[usize::from(picks.next().unwrap_or(0)) % CODAS.len()]; + + id.push_str(onset); + id.push_str(nucleus); + id.push_str(coda); + } + + id +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_readable_id() { + for _ in 0..32 { + let id = readable_id(); + + assert!((6..=15).contains(&id.len())); + assert!(id.chars().all(|character| character.is_ascii_lowercase())); + } + } +} diff --git a/crates/workspace/src/dialogs/new_group.rs b/crates/workspace/src/dialogs/new_group.rs index 42f5646c..e63ef083 100644 --- a/crates/workspace/src/dialogs/new_group.rs +++ b/crates/workspace/src/dialogs/new_group.rs @@ -1,7 +1,7 @@ use std::rc::Rc; use gpui::{App, AppContext, ClickEvent, Div, Entity, ParentElement, Styled, Window, px}; -use nip29::{GroupId, GroupKey, GroupMetadata, GroupsRegistry}; +use nip29::{GroupCreateOptions, GroupsRegistry}; use nostr_sdk::prelude::*; use theme::{ActiveTheme, TextStyle, Typeset as _}; use ui::button::{Button, ButtonVariants, IconPosition}; @@ -90,20 +90,6 @@ fn create( return false; } - let id = if raw_id.is_empty() { - slug(&name_value) - } else { - raw_id - }; - - let id = match GroupId::new(id) { - Ok(id) => id, - Err(error) => { - window.push_notification(Notification::error(error.to_string()), cx); - return false; - } - }; - let relay = match RelayUrl::parse(&raw_relay) { Ok(relay) => relay, Err(error) => { @@ -112,30 +98,20 @@ fn create( } }; - let mut metadata = GroupMetadata::default(); - metadata.name = Some(name_value); + let options = GroupCreateOptions { + name: name_value, + custom_id: if raw_id.is_empty() { + None + } else { + Some(raw_id) + }, + ..Default::default() + }; let registry = GroupsRegistry::global(cx); registry.update(cx, |registry, cx| { - registry.create(GroupKey::new(relay, id), metadata, cx); + registry.create(relay, options, cx); }); true } - -fn slug(name: &str) -> String { - let mut slug = String::with_capacity(name.len()); - let mut separator = false; - - for character in name.chars() { - if character.is_ascii_alphanumeric() { - slug.push(character.to_ascii_lowercase()); - separator = false; - } else if !separator && !slug.is_empty() { - slug.push('-'); - separator = true; - } - } - - slug.trim_end_matches('-').to_owned() -} diff --git a/crates/workspace/src/sidebar/mod.rs b/crates/workspace/src/sidebar/mod.rs index 5fa0f8e2..45d7d471 100644 --- a/crates/workspace/src/sidebar/mod.rs +++ b/crates/workspace/src/sidebar/mod.rs @@ -127,6 +127,12 @@ impl Sidebar { GroupsEvent::Error(error) => { window.push_notification(Notification::error(error.clone()), cx); } + GroupsEvent::Created { + warning: Some(warning), + .. + } => { + window.push_notification(Notification::warning(warning.clone()), cx); + } _ => {} }; }), diff --git a/docs/group-creation-plan.md b/docs/group-creation-plan.md new file mode 100644 index 00000000..749369cb --- /dev/null +++ b/docs/group-creation-plan.md @@ -0,0 +1,144 @@ +# Group creation flow (nostrord parity) + +Plan to bring the group creation flow in line with nostrord +(https://github.com/nostrord/nostrord), a NIP-29 client. Analysis of both +implementations, then a phased plan. + +## Current state (coop) + +- **Dialog** (`crates/workspace/src/dialogs/new_group.rs`): three plain text + inputs (Name, Group id, Relay). Slug is generated from the name when the id + is blank. Metadata is `GroupMetadata::default()` with only the name set. + Errors surface as toast notifications only. +- **Protocol** (`crates/nip29/src/protocol.rs`): + `GroupId::create_group` sends kind 9007 with the full metadata in one event. + No confirmed-group-id handling and no OK-retry on metadata. +- **Registry** (`GroupsRegistry::create` in `crates/nip29/src/lib.rs`): ensures + and subscribes the group, upserts the list entry, publishes kind 10009, and + on publish failure forgets the group and emits `GroupsEvent::Error`. + +## Reference (nostrord) + +### UI (`CreateGroupModal.kt`) + +- Title "Create a Group" ("Create Channel" for subgroups). +- Avatar upload row -> `picture`. +- Name (required, placeholder `#example`). +- Group ID optional, hidden behind a "Set a custom ID" disclosure; filtered to + lowercase `a-z0-9-_`; hint "Leave empty for a random ID. The relay may + override your choice." +- Relay dropdown: user's relays + current relay + suggested NIP-29 relays + + "Custom relay..." revealing a URL field. Locked to the parent's relay for + subgroups. +- Description, multi-line (3-5 rows) -> `about`. +- ACCESS SETTINGS: switch rows with NIP-29 copy — Private (only members can + read), Closed (join requests ignored), Restricted (only members can post), + Hidden (not discoverable). +- YOUR LIST: "Private on my list" — keeps the membership in the NIP-44 + encrypted section of kind 10009. +- Errors: friendly mapping (`blocked:` -> creation must happen on the relay's + website, `auth-required` -> authorization required, timeouts, ...) plus an + "Open relay website ->" link (wss -> https). +- Submit: spinner while in flight, cancelable; validation (name required, + valid relay URL). + +### Protocol (`GroupManager.createGroup`) + +Two-step flow: + +1. Group ID: custom (trimmed, lowercased) or a random readable id + (`ReadableId.generate()`). +2. Kind 9007 (create-group) with only the `h` tag -> await relay OK -> await + the relay's kind 39000 to learn the confirmed group id (relays may override + the suggested id). +3. Kind 9002 (edit-metadata) with `h` + name + access flags (presence-only + tags) + about + picture + parent -> await OK (7s timeout, one retry after + 1s to cover the admin-grant race). If still rejected, keep the group and + warn that the relay did not accept its name. +4. Bookkeeping: persist membership, discard the self-invite race, set the + private-list flag before publishing kind 10009, publish the list, request + group messages and metadata. + +### Existing coop infrastructure to reuse + +- `Group::publish` already awaits the relay OK and surfaces the rejection + reason (`crates/nip29/src/group.rs`). +- `GroupId::edit_metadata` / `Group::edit_metadata` for kind 9002 + (`crates/nip29/src/protocol.rs`, `group.rs`). +- `ui::Switch` (label + description), `PopupMenu` dropdown, `TextareaState`, + `Avatar` (`crates/ui`). +- Blossom upload via `state::upload` with `AppSettings::get_file_server`. +- NIP-65 relay-list extraction, as in `dialogs/relays.rs` + (`GossipRelays::load`). +- Group panel open flow used by the Browse panel + (`crates/workspace/src/panels/browse.rs` `open_panel`). + +## Plan + +### Phase 1 — Protocol layer (`crates/nip29`) + +- Adopt the two-step creation in `protocol.rs`: `GroupId::create_group` emits + kind 9007 with only the `h` tag; metadata moves to the existing + `edit_metadata` (kind 9002) builder. +- Add `GroupCreateOptions` (name, about, picture, private/closed/restricted/ + hidden, optional custom id, optional parent) passed to + `GroupsRegistry::create`. +- Rework `GroupsRegistry::create`: + - ID: custom (trim/lowercase/validate) or a random readable id (small port + of nostrord's syllable-based `ReadableId`). + - `ensure()` -> publish 9007 via `Group::publish` (relay OK) -> wait briefly + (with timeout) for the relay's kind 39000 to confirm the group id (the + group is already subscribed via `ensure`). + - Publish 9002 with the full metadata; retry once after 1s; on persistent + failure keep the group and emit a warning event (nostrord parity). + - On 9007 failure: `forget()` + emit `GroupsEvent::Error` with the relay's + reason. + - On success: upsert entry + `publish_list` + emit a new + `GroupsEvent::Created(GroupKey)` so the UI can navigate. + +### Phase 2 — New dialog (`crates/workspace/src/dialogs/new_group.rs`) + +Rebuild the modal to nostrord's layout and copy: + +- Header + subtitle; name field; "Set a custom ID" disclosure with filtered + input. +- Relay dropdown: user's NIP-65 relays (loaded like `GossipRelays::load`) + + suggested group relay + "Custom relay..." revealing a URL field with + validation. +- Description via `TextareaState` (3-5 lines). +- ACCESS SETTINGS: four `ui::Switch` rows with nostrord's NIP-29 + label/description copy. +- In-flight state on the Create button (spinner, disabled), inline error row + with the friendly error mapping + "Open relay website ->" link (wss -> + https). +- Subscribe to `GroupsEvent::Created` / `GroupsEvent::Error` while open -> + close the dialog and open the new group panel (same flow as + `browse.rs::open_panel`). + +### Phase 3 — Avatar upload + +- Picture row using `ui::Avatar` preview + file picker -> `state::upload` + (existing blossom upload), wiring upload errors into the dialog's error row. + +### Phase 4 — Optional extensions + +- **"Private on my list"**: requires NIP-44 encrypted-section support in + `GroupList` (kind 10009 parse/build/load) — meaningful extra scope; candidate + for a follow-up. +- **"Create Channel" subgroup mode**: protocol support (parent tag) already + exists; needs a trigger point in the group details/subgroups UI and + relay-locking in the dialog. + +### Validation + +- `cargo check` / `cargo clippy` after each phase. +- Manual run of the dialog against a NIP-29 relay. +- Unit tests for the readable-id and input-filter helpers. + +## Open questions + +1. Include "Private on my list" now, or defer the encrypted kind 10009 work? +2. Blank id behavior: nostrord generates a random readable id, coop currently + slugs the name. Match nostrord? +3. Avatar upload in scope for the first pass? +4. Subgroup / "Create Channel" mode now, or group creation only?