mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): treat channel creator as member before 39002 provisioning (#1830)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
co-authored by
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
parent
f3599f2cd4
commit
7e62a25af0
@@ -81,6 +81,21 @@ pub struct AppState {
|
||||
/// listener is up before any restore/create can request a connection.
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
pub mesh_coordinator: AsyncMutex<Option<crate::mesh_llm::MeshCoordinator>>,
|
||||
/// `(creator_pubkey_hex, channel_id)` pairs for channels the *named*
|
||||
/// identity created via `create_channel` and has not yet observed its own
|
||||
/// kind:39002 membership entry for. The relay provisions that entry
|
||||
/// asynchronously (#1761), so without this overlay a freshly created
|
||||
/// channel's owner reads back as `is_member=false` until the snapshot
|
||||
/// propagates, disabling their own composer. Entries are bound to the
|
||||
/// creating identity so an in-process identity swap (`import_identity`,
|
||||
/// workspace apply) can never inherit another identity's stale
|
||||
/// membership. Populated only by this process's own `create_channel`
|
||||
/// calls — a relay can never write into it — so it carries no
|
||||
/// trust-boundary risk. `get_channels` clears an entry once the real
|
||||
/// kind:39002 is observed for the current identity, keeping the set
|
||||
/// bounded and letting a later leave correctly flip the channel back to
|
||||
/// `is_member=false`.
|
||||
pub pending_owned_channels: Mutex<std::collections::HashSet<(String, String)>>,
|
||||
}
|
||||
|
||||
/// Parse the `BUZZ_PRIVATE_KEY` env var into identity keys. `Some` means the
|
||||
@@ -146,6 +161,7 @@ pub fn build_app_state() -> AppState {
|
||||
mesh_llm_runtime: AsyncMutex::new(None),
|
||||
#[cfg(feature = "mesh-llm")]
|
||||
mesh_coordinator: AsyncMutex::new(None),
|
||||
pending_owned_channels: Mutex::new(std::collections::HashSet::new()),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -175,6 +191,33 @@ impl AppState {
|
||||
}
|
||||
}
|
||||
|
||||
/// Record that `channel_id` was just created by `creator_pubkey` and its
|
||||
/// kind:39002 owner membership has not yet been observed.
|
||||
pub fn mark_pending_owned_channel(&self, creator_pubkey: &str, channel_id: &str) {
|
||||
if let Ok(mut set) = self.pending_owned_channels.lock() {
|
||||
set.insert((creator_pubkey.to_string(), channel_id.to_string()));
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether `channel_id` is still awaiting `my_pubkey`'s kind:39002 entry.
|
||||
/// Bound to `my_pubkey` so an in-process identity swap never inherits
|
||||
/// another identity's pending-owner entry for the same channel id.
|
||||
pub fn is_pending_owned_channel(&self, my_pubkey: &str, channel_id: &str) -> bool {
|
||||
self.pending_owned_channels
|
||||
.lock()
|
||||
.map(|set| set.contains(&(my_pubkey.to_string(), channel_id.to_string())))
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Drop the `(my_pubkey, channel_id)` entry from the pending-owner
|
||||
/// overlay once that identity's real kind:39002 membership has been
|
||||
/// observed.
|
||||
pub fn clear_pending_owned_channel(&self, my_pubkey: &str, channel_id: &str) {
|
||||
if let Ok(mut set) = self.pending_owned_channels.lock() {
|
||||
set.remove(&(my_pubkey.to_string(), channel_id.to_string()));
|
||||
}
|
||||
}
|
||||
|
||||
/// Return the active identity keys if they are in a signable state.
|
||||
///
|
||||
/// Returns `Err` when the identity is in a lost state (`identity_lost`
|
||||
|
||||
@@ -5,7 +5,7 @@ use crate::{
|
||||
events,
|
||||
models::{ChannelDetailInfo, ChannelInfo, ChannelMembersResponse},
|
||||
nostr_convert,
|
||||
relay::{query_relay, submit_event},
|
||||
relay::{query_relay, submit_event, submit_event_with_keys},
|
||||
};
|
||||
|
||||
// ── Reads (pure-nostr via /query) ────────────────────────────────────────────
|
||||
@@ -45,6 +45,16 @@ async fn query_relay_all(
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether an open channel not yet in the real member set should still be
|
||||
/// classified `is_member=true` via the pending-owner overlay. Pulled out of
|
||||
/// `get_channels`'s open-channel branch so the exact `(d_tag, my_pubkey,
|
||||
/// overlay) -> is_member` decision — including the identity binding that
|
||||
/// keeps one identity's pending entry from covering another's — is directly
|
||||
/// unit-testable without going through the async relay-backed command.
|
||||
fn classify_pending_owner(state: &AppState, my_pubkey: &str, d_tag: Option<&str>) -> bool {
|
||||
d_tag.is_some_and(|d| state.is_pending_owned_channel(my_pubkey, d))
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
pub async fn get_channels(state: State<'_, AppState>) -> Result<Vec<ChannelInfo>, String> {
|
||||
let _profile_start = std::time::Instant::now();
|
||||
@@ -80,6 +90,15 @@ pub async fn get_channels(state: State<'_, AppState>) -> Result<Vec<ChannelInfo>
|
||||
channel_ids.sort();
|
||||
channel_ids.dedup();
|
||||
|
||||
// The real kind:39002 membership has now resolved for these channels —
|
||||
// drop them from the pending-owner overlay (see `AppState::pending_owned_channels`)
|
||||
// so a channel this identity created no longer speaks through the overlay
|
||||
// once genuine membership is observable, and a later leave correctly
|
||||
// flips it back to `is_member=false`.
|
||||
for id in &channel_ids {
|
||||
state.clear_pending_owned_channel(&my_pubkey, id);
|
||||
}
|
||||
|
||||
// Step 2: fetch channel metadata events (kind:39000) for member channels.
|
||||
// kind:39000 is addressable: exactly one event per `d` tag, so a limit
|
||||
// equal to the number of ids is both necessary and sufficient. Without
|
||||
@@ -146,7 +165,18 @@ pub async fn get_channels(state: State<'_, AppState>) -> Result<Vec<ChannelInfo>
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if let Ok(info) = nostr_convert::channel_info_from_event(ev, None, Some(false)) {
|
||||
// The overlay (`AppState::pending_owned_channels`) marks channels this
|
||||
// identity just created via `create_channel` whose kind:39002 owner
|
||||
// membership hasn't propagated yet (#1761) — a fresh channel has no
|
||||
// member event and would otherwise fall through to `is_member=false`
|
||||
// here, disabling the owner's own composer until that snapshot lands.
|
||||
// The overlay can only be populated by this process's own
|
||||
// `create_channel` call (never by relay data) and is keyed by
|
||||
// `(my_pubkey, d_tag)`, so it adds no trust-boundary risk and can
|
||||
// never speak for a channel a different identity created; `channel_ids`
|
||||
// above clears it once real membership is observed for `my_pubkey`.
|
||||
let is_pending_owner = classify_pending_owner(&state, &my_pubkey, d_tag.as_deref());
|
||||
if let Ok(info) = nostr_convert::channel_info_from_event(ev, None, Some(is_pending_owner)) {
|
||||
channels.push(info);
|
||||
}
|
||||
}
|
||||
@@ -443,10 +473,26 @@ pub async fn create_channel(
|
||||
description.as_deref(),
|
||||
ttl_seconds,
|
||||
)?;
|
||||
submit_event(builder, &state).await?;
|
||||
|
||||
// Capture the signing identity before submission so the pending-owner
|
||||
// mark below is bound to whoever actually signed this create — not
|
||||
// whoever `state.keys` holds once the network round-trip completes. An
|
||||
// in-process identity swap while the request is in flight must not be
|
||||
// able to retarget the mark onto the new identity.
|
||||
let creator_keys = state.signing_keys()?;
|
||||
let creator_pubkey = creator_keys.public_key().to_hex();
|
||||
submit_event_with_keys(builder, &state, &creator_keys, None).await?;
|
||||
|
||||
// Mark this channel pending-owner: we just created it, so we know we're
|
||||
// the owner, but the relay's kind:39002 membership entry (#1761) is
|
||||
// provisioned asynchronously. `get_channels` consults this overlay to
|
||||
// classify us as `is_member=true` until that entry is observable. Bound
|
||||
// to the identity that signed the create above, so an in-process
|
||||
// identity swap can neither inherit nor retarget this entry.
|
||||
let channel_uuid_string = channel_uuid.to_string();
|
||||
state.mark_pending_owned_channel(&creator_pubkey, &channel_uuid_string);
|
||||
|
||||
// Re-fetch the canonical metadata event to return ChannelInfo.
|
||||
let channel_uuid_string = channel_uuid.to_string();
|
||||
let events = query_relay(
|
||||
&state,
|
||||
&[serde_json::json!({
|
||||
|
||||
@@ -109,3 +109,134 @@ fn empty_input_yields_empty_map() {
|
||||
let membership = collect_members_by_channel(&[]);
|
||||
assert!(membership.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pending_overlay_marks_relay_signed_channel_as_member() {
|
||||
// The real production shape: kind:39000 is relay-signed (#1761), so the
|
||||
// event's author is never the creator. A fresh channel's owner is
|
||||
// classified via the pending-owner overlay (populated by `create_channel`
|
||||
// in this same process), not via the event's pubkey.
|
||||
let relay_keys = Keys::generate();
|
||||
let e = EventBuilder::new(Kind::from_u16(39000), "")
|
||||
.tags(vec![
|
||||
Tag::parse(["d", "chan-1"]).expect("parse tag"),
|
||||
Tag::parse(["name", "n"]).expect("parse tag"),
|
||||
])
|
||||
.sign_with_keys(&relay_keys)
|
||||
.expect("sign");
|
||||
|
||||
let state = crate::app_state::build_app_state();
|
||||
state.mark_pending_owned_channel(PK_A, "chan-1");
|
||||
|
||||
let info = crate::nostr_convert::channel_info_from_event(
|
||||
&e,
|
||||
None,
|
||||
Some(classify_pending_owner(&state, PK_A, Some("chan-1"))),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(info.is_member);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pending_overlay_leaves_unrelated_channel_as_non_member() {
|
||||
// A relay-signed channel this identity never created (not in the
|
||||
// overlay) must stay `is_member=false` — no over-broad match.
|
||||
let relay_keys = Keys::generate();
|
||||
let e = EventBuilder::new(Kind::from_u16(39000), "")
|
||||
.tags(vec![
|
||||
Tag::parse(["d", "chan-1"]).expect("parse tag"),
|
||||
Tag::parse(["name", "n"]).expect("parse tag"),
|
||||
])
|
||||
.sign_with_keys(&relay_keys)
|
||||
.expect("sign");
|
||||
|
||||
let state = crate::app_state::build_app_state();
|
||||
// Overlay has a different channel pending for the same identity, not
|
||||
// this one.
|
||||
state.mark_pending_owned_channel(PK_A, "chan-other");
|
||||
|
||||
let info = crate::nostr_convert::channel_info_from_event(
|
||||
&e,
|
||||
None,
|
||||
Some(classify_pending_owner(&state, PK_A, Some("chan-1"))),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(!info.is_member);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pending_overlay_cleared_once_real_membership_observed() {
|
||||
// Once the real kind:39002 lands (modeled here as `get_channels`'s
|
||||
// cleanup step: clearing every channel id it just found real membership
|
||||
// for), the overlay must stop speaking for that channel — otherwise a
|
||||
// later leave would never flip `is_member` back to false.
|
||||
let state = crate::app_state::build_app_state();
|
||||
state.mark_pending_owned_channel(PK_A, "chan-1");
|
||||
assert!(state.is_pending_owned_channel(PK_A, "chan-1"));
|
||||
|
||||
// Mirrors the `for id in &channel_ids { state.clear_pending_owned_channel(&my_pubkey, id) }`
|
||||
// step in `get_channels` once "chan-1" appears in PK_A's real member set.
|
||||
state.clear_pending_owned_channel(PK_A, "chan-1");
|
||||
assert!(!state.is_pending_owned_channel(PK_A, "chan-1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pending_overlay_does_not_leak_across_identity_swap() {
|
||||
// Regression for the IMPORTANT Thufir flagged on the bare-channel-id
|
||||
// overlay: `import_identity`/workspace-apply can replace `state.keys` in
|
||||
// process without clearing the overlay. Identity A creates a channel and
|
||||
// is recorded pending-owner; if the process then switches to identity B
|
||||
// (same `AppState`, same channel id), B must NOT inherit A's entry.
|
||||
let state = crate::app_state::build_app_state();
|
||||
state.mark_pending_owned_channel(PK_A, "chan-1");
|
||||
|
||||
assert!(state.is_pending_owned_channel(PK_A, "chan-1"));
|
||||
assert!(!state.is_pending_owned_channel(PK_B, "chan-1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_pending_owner_matches_only_the_owning_identity() {
|
||||
// Exercises the exact branch-level decision `get_channels`'s open-channel
|
||||
// fallthrough makes, not just the underlying `AppState` helpers in
|
||||
// isolation.
|
||||
let state = crate::app_state::build_app_state();
|
||||
state.mark_pending_owned_channel(PK_A, "chan-1");
|
||||
|
||||
assert!(classify_pending_owner(&state, PK_A, Some("chan-1")));
|
||||
// Different identity, same channel id: must not match.
|
||||
assert!(!classify_pending_owner(&state, PK_B, Some("chan-1")));
|
||||
// Same identity, different channel id: must not match.
|
||||
assert!(!classify_pending_owner(&state, PK_A, Some("chan-other")));
|
||||
// No `d` tag on the event at all: must not match.
|
||||
assert!(!classify_pending_owner(&state, PK_A, None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pending_owner_mark_uses_signer_captured_before_identity_swap() {
|
||||
// Regression for the write-side IMPORTANT Thufir flagged in pass 3:
|
||||
// `create_channel` used to re-read `state.keys` *after* the submit
|
||||
// await, so an identity swap that lands during the in-flight request
|
||||
// could mark the overlay under the new identity instead of the one that
|
||||
// actually signed the create. The fix captures the signer up front and
|
||||
// marks with that captured identity, so a swap that happens afterward
|
||||
// (i.e. during what would be the submit await) can't retarget the mark.
|
||||
let state = crate::app_state::build_app_state();
|
||||
|
||||
// Mirrors `create_channel`'s new capture-before-submit step: read the
|
||||
// signer identity once, before anything that could race with a swap.
|
||||
let creator_keys = state.signing_keys().expect("signable");
|
||||
let creator_pubkey = creator_keys.public_key().to_hex();
|
||||
|
||||
// Simulate an in-process identity swap landing during the (here,
|
||||
// implicit) submit await — e.g. `import_identity` replacing
|
||||
// `state.keys` while the create request is in flight.
|
||||
*state.keys.lock().expect("lock keys") = Keys::generate();
|
||||
|
||||
// The mark must use the captured signer, not whatever `state.keys`
|
||||
// holds now.
|
||||
state.mark_pending_owned_channel(&creator_pubkey, "chan-1");
|
||||
|
||||
assert!(state.is_pending_owned_channel(&creator_pubkey, "chan-1"));
|
||||
let post_swap_pubkey = state.keys.lock().expect("lock keys").public_key().to_hex();
|
||||
assert!(!state.is_pending_owned_channel(&post_swap_pubkey, "chan-1"));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user