coop/docs/concord-simplification-plan.md

Concord backend audit and simplification plan

Audit of crates/concord, triggered by CommunityRegistry never reaching subscribe: sync::load found zero community state documents. Tracing that surfaced two separate things: the app only uses a fraction of the crate, and the crate's writers take a concrete nostr::Keys, which the app's signer can never produce.

Sizes: ~10,500 lines total — ~6,750 production, ~3,750 tests.

Decisions taken

• D1 — Keep the unwired protocol surface. cord05/cord06/pins/paging stay in the tree for future use. No mass deletion. (Findings are recorded in §4 for reference only.)
• D2 — Replace &Keys with a signer boundary for account-key operations. Verified feasible against the pinned SDK; design in §2.

---

1. &Keys cannot be replaced by a public key — but it can be replaced by a signer

The original question was whether functions like genesis only need signer.get_public_key_async(). They do not: they sign.

• cord02::genesis (cords/cord02/mod.rs:117) → seal_edition (:711) → build_seal (cord01.rs:217), which signs the seal (.finalize(author), cord01.rs:226), and wrap_seal_with (:247), which signs the wrap.
• Self-addressed documents use NIP-44 to self: seal_to_self (cord01.rs:201) derives a conversation key from keys.secret_key().

A public key can produce neither a Schnorr signature nor an ECDH key, so "public-key-only" is impossible. The real defect is the concrete type: the app holds state::UniversalSigner (async, possibly NIP-46), and a nostr::Keys can never be conjured from it. docs/concord-usage.md:535-536 already records this as a deliberate migration pass.

What the pinned SDK actually provides

Pinned rev b230cec (nostr 0.45.4 / nostr-sdk 0.45.2):

• There is no NostrSigner trait in this revision. The async signer surface is three traits, all in the nostr crate:
  • AsyncGetPublicKey — nostr/src/key/public_key.rs:39
  • AsyncSignEvent — nostr/src/event/mod.rs:366
  • AsyncNip44 — nostr/src/nips/nip44/traits.rs:30
• Keys implements all three (nostr/src/key/mod.rs:298,309,342), so tests and local key holders keep working.
• UniversalSigner already implements all three with Error = UniversalSignerError (crates/state/src/signer.rs:148-191).
• SDK helpers accept them:
  • EventBuilder::finalize_async — S: AsyncGetPublicKey + AsyncSignEvent + ?Sized (nostr/src/event/builder.rs:171-193)
  • GiftWrapBuilder::finalize_async — S: AsyncGetPublicKey + AsyncSignEvent + AsyncNip44 (nostr/src/nips/nip59.rs:334-355)
  • UnwrappedGift::from_gift_wrap_async — T: AsyncNip44 (nip59.rs:84-90)

So the answer is yes: pass a signer. UniversalSigner works as-is.

Per-function bounds, not a bundle

Each function should request only the capabilities it uses. The SDK itself is designed this way (UnsignedEvent::finalize_async takes only AsyncSignEvent, EventBuilder::finalize_async takes AsyncGetPublicKey + AsyncSignEvent, NIP-59 takes all three).

Operation	Bounds
Sign a seal/edition/rekey wrap, author already known	AsyncSignEvent
Build an event where the author comes from the signer	AsyncGetPublicKey + AsyncSignEvent
To-self documents (Community List, Invite List)	AsyncGetPublicKey + AsyncNip44, plus AsyncSignEvent when the document is itself an event
Decrypt-only (parse_list_event, unwrap_direct_invite)	AsyncNip44
Rekey blob encrypt (build_blob)	AsyncGetPublicKey + AsyncNip44 (no signing)
Rekey blob open (open_blob)	AsyncNip44
Direct invite build (GiftWrapBuilder)	all three

Use generics (S: AsyncSignEvent + ?Sized), never &dyn: the traits carry associated Error types, so dyn AsyncSignEvent would force the concrete error at every call site (dyn AsyncSignEvent<Error = UniversalSignerError>), defeating the abstraction. The SDK uses generics throughout for this reason.

Do not define a supertrait bundle trait Signer: AsyncGetPublicKey + AsyncSignEvent + AsyncNip44 {}: all three supertraits declare an associated Error, so Self::Error becomes ambiguous, and the bundle forces NIP-44 onto purely-signing callers (and vice versa).

Inside concord, replace builder.finalize(&keys) with builder.finalize_async(signer).await. finalize_async fetches the signer's public key and uses it as the event author, exactly as finalize did, so the bytes are unchanged for every caller that passes a matching signer.

We deliberately do not pre-check the author against rumor.pubkey inside build_seal. The seal's author is the signer's own public key, matching the old finalize semantics; a signer that does not match the rumor is still caught by open_wrap_at as AuthorMismatch (cord01.rs:328). Pre-checking would also make it impossible to construct the hostile seals the cord suite relies on as test vectors (cord01.rs hostile_wraps_are_dropped_in_order).

If the repeated <S as ...>::Error: Error + Send + Sync + 'static bounds become too noisy, the only stable-Rust way to shorten them is an owned error-erased trait (as the app already does with crates/state/src/signer.rs:64-138). That trades precision for brevity; keep per-function bounds unless the noise proves unmanageable.

What must NOT go through the signer

• Group-key NIP-44. cord01::{seal_bytes, open_bytes, wrap_seal, wrap_seal_with, rewrap_seal} encrypt under a ConversationKey derived from HKDF group secrets. AsyncNip44 can only ECDH against a public key, so group encryption stays on ConversationKey / GroupKey::keys().
• Wrap signatures. Wraps are signed by the derived group signer key (GroupKey::keys()), not the account.
• Locally held raw secrets. cord05::{build_bundle_event, build_revocation} take a generated link_signer whose secret the app stores as signer_sk (docs/concord-usage.md:306-321). &Keys is correct there; the app has the secret itself.
• Local database artifacts. store::{cache_rumor, save_state} sign with the internal random LOCAL_KEYS (store.rs:18). No user signer involved.

Call-site inventory

Account-key sites to migrate:

Site	Today	After	Bounds
cord02::genesis (cord02/mod.rs:117)	owner: &Keys	owner: &S	AsyncGetPublicKey + AsyncSignEvent
ControlWriter::{publish, set_*} (cord02/mod.rs:214-425)	keys: &Keys	keys: &S	AsyncGetPublicKey + AsyncSignEvent
seal_edition (cord02/mod.rs:711, internal)	owner: &Keys	owner: &S	AsyncGetPublicKey + AsyncSignEvent
cord01::build_seal (cord01.rs:217)	author: &Keys	author: &S	AsyncGetPublicKey + AsyncSignEvent
cord01::{seal_to_self, open_to_self} (:201,209)	keys: &Keys	&S, async	AsyncGetPublicKey + AsyncNip44
guestbook::seal_rumor (guestbook.rs:186)	author: &Keys	author: &S	AsyncGetPublicKey + AsyncSignEvent
cord03::seal_rumor (cord03.rs:295)	author: &Keys	author: &S	AsyncGetPublicKey + AsyncSignEvent
list::build_list_event (list.rs:186)	keys: &Keys	keys: &S	all three
list::parse_list_event (list.rs:197)	keys: &Keys	keys: &S	AsyncGetPublicKey + AsyncNip44
cord05::{build_direct_invite, unwrap_direct_invite} (:451,478)	inviter/recipient: &Keys	done	build: all three (Sized); unwrap: AsyncNip44 (Sized)
cord05::{build_invite_list, parse_invite_list} (:593,604)	keys: &Keys	done	build: all three; parse: AsyncGetPublicKey + AsyncNip44
cord06::build_blob (:302)	rotator: &Keys	done	AsyncGetPublicKey + AsyncNip44
cord06::open_blob (:319)	recipient: &Keys	done	AsyncNip44
cord06::{build_rekey_chunks, seal_dissolved} (:602,737)	actor &Keys	done	AsyncGetPublicKey + AsyncSignEvent

Leave unchanged: cord05::{build_bundle_event, build_revocation}, all cord01 wrap functions, GroupKey::keys(), store::LOCAL_KEYS.

Async ripple and tests

Every migrated function becomes async. smol is already a dev-dependency of concord (crates/concord/Cargo.toml:21-23), so affected #[test]s become smol::block_on(...) wrappers. The app's call sites are already async background tasks.

Known constraints

• GiftWrapBuilder::finalize_async and UnwrappedGift::from_gift_wrap_async are generic over S: Sized (no ?Sized), so those functions must stay generic, never &dyn.
• Every converted S::Error must be Error + Send + Sync + 'static for the SDK helpers' Error::other (nostr/src/error.rs:100-105) and for anyhow; Keys::AsyncGetPublicKey::Error = Infallible, Keys::AsyncSignEvent::Error = nostr::Error, UniversalSignerError (crates/state/src/signer.rs:10-32) all qualify.
• AsyncGetPublicKey is worth requiring alongside AsyncSignEvent wherever the author is embedded in the payload: sign_event signs the id of the given unsigned event without rewriting its pubkey, so a mismatched signer is only caught later by signature verification.

---

2. Migration plan

Phase 1 — replace &Keys with per-function signer bounds (no behavior change) — DONE

1. No new module: change the signatures listed in the inventory table to generics over the SDK traits (S: AsyncGetPublicKey + AsyncSignEvent, S: AsyncSignEvent, S: AsyncGetPublicKey + AsyncNip44, or S: AsyncNip44).
2. Migrate the live path only: cord01::build_seal, cord01::{seal_to_self, open_to_self}, seal_edition, genesis, ControlWriter, guestbook:: seal_rumor, cord03::seal_rumor, list::{build,parse}_list_event.
3. Update docs/concord-usage.md examples to take a signer.
4. Update concord tests to smol::block_on; &Keys keeps working because it implements all three traits.

Validation: cargo test -p concord — 46 passed, 0 failed. The one behavior change from the plan sketch is the dropped up-front author check in §1. cord01::{seal_to_self, open_to_self} now take &str and return String (NIP-44 is UTF-8 text), so the list and invite-list callers read the plaintext with serde_json::from_str.

Unplanned but forced: cord01::{build_seal, seal_to_self, open_to_self} are shared helpers, so the unwired callers had to be migrated in the same pass to keep the crate compiling: cord05::{build_invite_list, parse_invite_list} and cord06::{build_rekey_chunks, seal_dissolved} (Phase 3's mechanical part). cord05::{build_direct_invite, unwrap_direct_invite} and cord06::{build_blob, open_blob} were untouched by Phase 1 — they use the NIP-59 and group-key paths, not the migrated helpers — and were migrated in Phase 3.

Phase 2 — app uses the signer — DONE

1. sync::create(client, signer, metadata) (crates/community/src/sync.rs) runs cord02::genesis, opens the genesis editions, persists the state with store::save_state, and also stores the genesis wraps so the control plane folds locally. It is generic over S: AsyncGetPublicKey + AsyncSignEvent + ?Sized (the bounds genesis needs and no more, per D2); the app passes its UniversalSigner, so no secret material is exposed and NIP-46 accounts work too. CommunityRegistry::create(metadata, cx) (crates/community/src/lib.rs) is the GPUI wrapper: it refuses when no account is signed in, otherwise runs the task off-thread and refreshes tracking, so sync::load now returns one state and subscribe finally fires.
2. The app-side reimplementation of list::parse_list_event (crates/community/src/sync.rs:154-156) is deleted; load_list calls the real cord02::list::parse_list_event.
3. Relays in the metadata are persisted but the genesis is not published yet; create is local-only. Wiring genesis/broadcast through the relay pool is the next app step, not part of this phase.

Validation: cargo test -p community (1 passed), cargo test -p concord (46 passed), cargo clippy -p community --all-targets, cargo fmt -p community --check, and cargo check --workspace --all-targets are all clean.

Deviation from the plan sketch: the planned "drive CommunityRegistry" test is instead a sync-layer test, sync::tests:: creating_a_community_persists_a_state_that_subscribes_and_folds. A GPUI-level test cannot construct a NostrRegistry — it opens LMDB at config_dir() and connects bootstrap relays in NostrRegistry::new, which is private and not injectable — so the test drives a Client on an in-memory database (nostr-memory, already a dev-dependency) directly. It asserts the whole contract the registry depends on: create persists a state load returns, the subscription filter addresses the genesis wraps, fold yields the created community, and an inbound control edit folds over it.

Phase 3 — migrate the remaining unwired writers — DONE

cord05::{build_direct_invite, unwrap_direct_invite} and cord06::{build_blob, open_blob} now take a signer. The NIP-59 pair keeps a Sized S (AsyncGetPublicKey + AsyncSignEvent + AsyncNip44 to build, AsyncNip44 to unwrap) because the SDK's GiftWrapBuilder::finalize_async and UnwrappedGift::from_gift_wrap_async are Sized-bounded. The blob pair is AsyncGetPublicKey + AsyncNip44 to build and AsyncNip44 to open, with ?Sized.

The blobs forced one behavior change, because a signer's NIP-44 is text-only (nip44_encrypt_async(public_key, &str)) while the blob plaintext is a fixed-width binary record. build_blob now carries that record base64-encoded inside the NIP-44 envelope and open_blob decodes it again. The record layout, the locator, and the envelope are unchanged; only the bytes inside the envelope differ. There are no golden vectors for blobs and no producer or consumer other than these two functions, so the round-trip stays self-consistent; cord06 remains unwired and persists nothing.

Validation: cargo test -p concord — 46 passed, 0 failed (the 80-blob a_full_send_chunk_stays_within_a_relay_event size assertion still holds under the base64 record). cargo clippy -p concord --all-targets and cargo fmt -p concord --check are clean.

Phase 4 — duplication and hygiene (independent, low risk) — DONE

1. DONE — store::load_states(client) added (with a direct store test), the app-side state-document scan in sync::load is gone.
2. DONE — store::STATE_PREFIX is public; the app-side concord/ literals are gone, and subscription ids reuse the exported prefix.
3. DONE — the shared rumor tag readers and error live in a new cords::rumor module (RumorError, tag, required, value, pubkey, optional_citation), re-exported as cord03::ChatError and cord02::guestbook::GuestbookError. cord06 keeps its own narrower RekeyError, which the plan scoped out.
4. RETAINED — none of the "never-varied parameters" were removed. Each is load-bearing for a flow the fold or a writer already implements (D1):
   • complete_memberlist's banned_at is read by the fold and is exercised with a non-empty map by join_leave_kick_and_snapshot_converge_to_one_memberlist; docs/concord-usage.md already promises to fill it once the banlist head's timestamp is plumbed through.
   • cache_rumor -> Result<bool> is read by backfill to drop expired rumors.
   • coalesce's snapshot_authority gates which snapshots apply; passing None today is a policy, not a dead parameter.
   • seal_rumor(ephemeral) and the until cursors on backfill/query_rumors select protocol modes and paging.
5. DONE — tightened cord04 visibility: edition_hash, fold, FoldResult, bootstrap_head, parse_banlist, Role::parse and Grant::parse are no longer pub. HeadSelection stays pub because the public fold_head returns it.
6. DONE — doc drift fixed: the store takes &Client throughout (including load_state/load_states/query_rumors, not just the writers), backfill arity, set_pin_list's missing .await, the GPUI init signature and registry names, and the "Not wired up yet" registry bullet.

Phase 5 — sidebar calls create — DONE

The last blocker was that nothing invoked CommunityRegistry::create; the running app logged community load: 0 state document(s) found and subscribe never ran. The sidebar now:

1. Renders CommunityRegistry::communities() instead of the hardcoded dummy_communities(). SidebarRow::Community carries an Entity<Community>, labelled with Community::name() (control-fold metadata, falling back to the community id until the first fold).
2. Adds a "New community" row to the Community section that opens a name prompt and calls CommunityRegistry::create with default metadata. Relays stay empty, so the subscription resolves through ReqTarget::auto against the pool's relays rather than a manual target that add_relay might not have connected.
3. Observes the registry, so a track or fold re-render reaches the list, and subscribes to CommunityEvent::Error, which is now logged (log::error!("community: {error}")) instead of vanishing. A cx.notify() in the registry's per-community observer propagates the fold that fills in the name.

Validation: cargo check -p workspace -p community --all-targets, cargo test -p community (1 passed), cargo clippy -p workspace -p community --all-targets, and cargo fmt -p workspace -p community --check are clean.

Still local-only: the genesis is persisted but not published to relays, so a second account cannot discover the community yet.

---

3. Retained-by-decision surface (reference only)

Per D1 these stay, but they should be understood as unwired, not live:

Module	Approx. prod LOC	App use
cord06 rotation/refounding/dissolution	~850	none
cord05 invites/links/direct/list	~650	none (types only, via unused list::join_material)
cord04::pins	~550	none
cord03 write path + fold + plane_keys	~340	only open / expiration_of
guestbook / list write paths	~240	open, coalesce, complete_memberlist, is_live
store paging / purge / query / load_state(s)	~180	cache_rumor, save_state, load_states

Truly unreferenced even by tests (safe candidates, but kept per D1): CommunityInvite::expired, GroupKey::pk_hex, From<[u8; 32]> impls, CommunityRoles::{roles, is_empty}.

---

4. Non-goals

• No mass deletion of unwired modules (D1).
• No changes to frozen HKDF derivations, locators, golden vectors, or cord01 envelope semantics. The one exception Phase 3 forced is the blob plaintext encoding (base64 inside the envelope, see Phase 3); the blob record layout and locator are untouched.
• No group-key encryption through the signer.
• Tests move only alongside the code they cover.

5. Validation

• cargo test -p concord after each phase; cargo test --workspace before landing.
• Phase 1 is behavior-preserving: the existing cord test suite is the oracle.
• Phase 2 adds the app-level test: seed a CommunityState via store::save_state, drive CommunityRegistry, assert a subscription is made and an inbound wrap folds into the community.
• Phase 4: cargo test -p concord -p community (47 + 1 passed), cargo clippy -p concord -p community --all-targets, and cargo fmt -p concord -p community --check are all clean.

6. Immediate unblock

Option 2 (the clean path, using UniversalSigner) landed in Phase 2. Option 1 (exposing the local Keys from crates/state/src/lib.rs:254) is obsolete.