mirror of
https://github.com/block/buzz.git
synced 2026-10-30 14:01:37 +00:00
fix(mesh): make open-relay mode consistent across discovery, reconcile, and probing
Follow-up to the open-mesh change (d99cba5bb), closing three gaps found in
review:
1. A stray membership snapshot on an open relay no longer gates routing.
Admission runs TrustPolicy::Off there, but discovery still intersected
status notes with any kind:13534 roster that happened to exist (added out
of band), and the query layer applied the same authors filter — a hybrid
where admission admits everyone while discovery hides non-roster peers.
Both branches now check the relay mode: open relays ignore membership
entirely and rely on freshness plus owner-signed endpoint binding.
2. Reconcile recovers a node isolated by the startup probe fallback. A
transient NIP-11 probe failure at start falls back to the enforcing path
and a self-only allowlist; treating Ok(None) as an unconditional Keep then
pinned that node at "1 NODE" until an app restart — the original bug,
reintroduced by one HTTP blip. A self-only roster admits nobody else, so
restarting into open mesh relaxes nothing that protected anyone. An
allowlist admitting other owners is still never relaxed mid-flight.
3. Successful NIP-11 probes are memoized per relay URL. The deployment mode
does not flap at runtime, and discovery re-probed on every reconcile poll.
Failures are not cached, so the fallback in (2) re-probes and recovers.
Co-authored-by: Michael Neale <michael.neale@gmail.com>
Signed-off-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
@@ -219,9 +219,29 @@ async fn query_mesh_discovery_events_at(
|
||||
///
|
||||
/// A failed probe must not open the mesh up: an unreachable or malformed NIP-11
|
||||
/// document leaves admission exactly as strict as it is today.
|
||||
///
|
||||
/// Successful probes are memoized per relay URL: the deployment mode
|
||||
/// (`BUZZ_REQUIRE_RELAY_MEMBERSHIP`) does not flap at runtime, and the
|
||||
/// discovery/reconcile paths would otherwise re-issue this HTTP GET on every
|
||||
/// poll. Failures are NOT cached, so a node that fell back to enforcing on a
|
||||
/// transient blip re-probes on the next poll and can recover.
|
||||
async fn resolved_relay_mesh_mode(relay_url: &str) -> mesh_llm::MeshRelayMode {
|
||||
static CACHE: std::sync::OnceLock<
|
||||
std::sync::Mutex<std::collections::HashMap<String, mesh_llm::MeshRelayMode>>,
|
||||
> = std::sync::OnceLock::new();
|
||||
let cache = CACHE.get_or_init(Default::default);
|
||||
if let Ok(guard) = cache.lock() {
|
||||
if let Some(mode) = guard.get(relay_url).copied() {
|
||||
return mode;
|
||||
}
|
||||
}
|
||||
match probe_relay_mesh_mode(relay_url).await {
|
||||
Ok(mode) => mode,
|
||||
Ok(mode) => {
|
||||
if let Ok(mut guard) = cache.lock() {
|
||||
guard.insert(relay_url.to_string(), mode);
|
||||
}
|
||||
mode
|
||||
}
|
||||
Err(error) => {
|
||||
eprintln!("buzz-mesh: NIP-11 probe failed; assuming membership is enforced: {error}");
|
||||
mesh_llm::MeshRelayMode::ClosedMembershipEnforced
|
||||
@@ -260,9 +280,12 @@ async fn query_mesh_discovery_events_for_mode(
|
||||
}
|
||||
}
|
||||
let mut status_filter = mesh_llm::mesh_status_filter();
|
||||
// On an enforcing relay, scope status reads to current members. Without a
|
||||
// roster there is nobody to scope to, so read every reporter's note.
|
||||
if !member_pubkeys.is_empty() {
|
||||
// On an enforcing relay, scope status reads to current members. On an open
|
||||
// relay the snapshot is not a trust boundary even when one exists (added
|
||||
// out of band) — admission runs unenforced there, so scoping reads to a
|
||||
// stray roster would hide peers the mesh admits. Read every reporter's
|
||||
// note and let freshness + endpoint-binding checks gate routing.
|
||||
if !mode.is_open() && !member_pubkeys.is_empty() {
|
||||
status_filter["authors"] = serde_json::json!(member_pubkeys);
|
||||
}
|
||||
let mut previous_cursor: Option<(u64, String)> = None;
|
||||
|
||||
@@ -234,17 +234,24 @@ fn roster_shrinks(current: &[String], fresh: &[String]) -> bool {
|
||||
///
|
||||
/// Rules:
|
||||
/// - query failed (`Err`) → `Keep` (never de-admit on a relay blip)
|
||||
/// - relay enforces no membership (`Ok(None)`) → `Keep` (see below)
|
||||
/// - relay enforces no membership (`Ok(None)`), roster beyond self → `Keep` (see below)
|
||||
/// - relay enforces no membership (`Ok(None)`), self-only roster → `RestartProcess` (see below)
|
||||
/// - resolved roster == current → `Keep` (no-op)
|
||||
/// - grows (only additions) → `RestartProcess` immediately (fast admission)
|
||||
/// - shrinks/empties, first observation → `AwaitConfirm` (hold, re-check next poll)
|
||||
/// - shrinks/empties, confirmed → `RestartProcess` (same reduced roster twice)
|
||||
///
|
||||
/// `Ok(None)` means the relay does not enforce NIP-43 membership, so there is
|
||||
/// no roster to reconcile against. A node that is *already* enforcing an
|
||||
/// allowlist keeps it: relaxing a live node's admission on the strength of a
|
||||
/// NIP-11 read would let a relay downgrade (or a spoofed document) de-restrict
|
||||
/// a running mesh. The open-relay path is chosen at start, not mid-flight.
|
||||
/// no roster to reconcile against. A node enforcing an allowlist that admits
|
||||
/// *other* owners keeps it: relaxing a live multi-party allowlist on the
|
||||
/// strength of a NIP-11 read would let a relay downgrade (or a spoofed
|
||||
/// document) de-restrict a running mesh. A **self-only** allowlist is
|
||||
/// different: it admits nobody else, so it protects nothing — and it is
|
||||
/// exactly the fallback a transient NIP-11 probe failure at startup produces
|
||||
/// on an open relay. Without recovery here that node stays at "1 NODE" until
|
||||
/// an app restart (the original bug, reintroduced by a single HTTP blip).
|
||||
/// Restarting resolves the roster afresh, which on an open relay yields
|
||||
/// `None` → `TrustPolicy::Off`.
|
||||
fn roster_reconcile_action(
|
||||
current_owners: &[String],
|
||||
pending_shrink: Option<&[String]>,
|
||||
@@ -257,6 +264,13 @@ fn roster_reconcile_action(
|
||||
);
|
||||
return RosterReconcileAction::Keep;
|
||||
}
|
||||
Ok(None) if current_owners.is_empty() => {
|
||||
eprintln!(
|
||||
"buzz-mesh: relay does not enforce membership and this node is isolated by the \
|
||||
startup fallback; restarting to run the mesh unenforced"
|
||||
);
|
||||
return RosterReconcileAction::RestartProcess;
|
||||
}
|
||||
Ok(None) => {
|
||||
eprintln!(
|
||||
"buzz-mesh: relay does not enforce membership; keeping this node's current allowlist"
|
||||
@@ -571,6 +585,34 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
// Regression: a transient NIP-11 probe failure at startup on an OPEN relay
|
||||
// falls back to the enforcing path and starts the node with a self-only
|
||||
// allowlist. If reconcile then treated `Ok(None)` as an unconditional
|
||||
// `Keep`, that node would sit at "1 NODE" until an app restart — the
|
||||
// original open-relay bug, reintroduced by a single HTTP blip. A self-only
|
||||
// roster admits nobody else, so restarting to run unenforced relaxes
|
||||
// nothing that was protecting anyone.
|
||||
#[test]
|
||||
fn open_relay_recovers_a_fallback_isolated_node() {
|
||||
let self_only: Vec<String> = Vec::new();
|
||||
let action = roster_reconcile_action(&self_only, None, Ok(None));
|
||||
assert_eq!(
|
||||
action,
|
||||
RosterReconcileAction::RestartProcess,
|
||||
"a fallback-isolated node on an open relay must restart into open mesh"
|
||||
);
|
||||
}
|
||||
|
||||
// The other side of that rule: an allowlist that admits OTHER owners is
|
||||
// never relaxed on the strength of a NIP-11 read. A relay downgrade (or a
|
||||
// spoofed document) must not de-restrict a running mesh.
|
||||
#[test]
|
||||
fn open_relay_report_keeps_a_multi_party_allowlist() {
|
||||
let current = vec!["owner-a".to_string(), "owner-b".to_string()];
|
||||
let action = roster_reconcile_action(¤t, None, Ok(None));
|
||||
assert_eq!(action, RosterReconcileAction::Keep);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unchanged_roster_is_a_noop() {
|
||||
let current = vec!["owner-a".to_string()];
|
||||
|
||||
@@ -240,16 +240,25 @@ pub fn availability_from_events_for_mode(
|
||||
if events.is_empty() {
|
||||
return MeshAvailability::unavailable("Buzz shared compute status is not published yet");
|
||||
}
|
||||
let members = match latest_membership_list(&events) {
|
||||
Some(members) => Some(members),
|
||||
// An enforcing relay always publishes a snapshot (even a zero-member
|
||||
// one), so its absence there means an incomplete read: stay closed.
|
||||
None if !mode.is_open() => {
|
||||
return MeshAvailability::unavailable(
|
||||
"Buzz shared compute is waiting for the current member roster",
|
||||
);
|
||||
// On an open relay the snapshot is not a trust boundary even when one
|
||||
// happens to exist (added out of band): admission runs `TrustPolicy::Off`
|
||||
// there, so intersecting routing with a stray roster would hide peers the
|
||||
// mesh happily admits. Ignore membership entirely and accept status notes
|
||||
// on their own merits.
|
||||
let members = if mode.is_open() {
|
||||
None
|
||||
} else {
|
||||
match latest_membership_list(&events) {
|
||||
Some(members) => Some(members),
|
||||
// An enforcing relay always publishes a snapshot (even a
|
||||
// zero-member one), so its absence means an incomplete read:
|
||||
// stay closed.
|
||||
None => {
|
||||
return MeshAvailability::unavailable(
|
||||
"Buzz shared compute is waiting for the current member roster",
|
||||
);
|
||||
}
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
|
||||
// Status is replaceable per member pubkey, so a query returns multiple
|
||||
|
||||
@@ -725,6 +725,45 @@ fn open_relay_routes_without_a_membership_snapshot() {
|
||||
assert_eq!(open.serve_targets[0].endpoint_addr, endpoint);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn open_relay_ignores_a_stray_membership_snapshot() {
|
||||
use super::MeshRelayMode;
|
||||
|
||||
// On an open relay a kind:13534 snapshot can still exist (members added
|
||||
// out of band), but it is not a trust boundary there: admission runs
|
||||
// `TrustPolicy::Off` regardless. Intersecting routing with that stray
|
||||
// roster would hide peers the mesh happily admits — a hybrid state where
|
||||
// admission and discovery disagree. Routing must ignore it entirely.
|
||||
let reporter_secret = "a".repeat(64);
|
||||
let reporter_pubkey = nostr::Keys::parse(&reporter_secret)
|
||||
.unwrap()
|
||||
.public_key()
|
||||
.to_hex();
|
||||
let endpoint = test_endpoint_token();
|
||||
let events = vec![
|
||||
// Snapshot deliberately lists someone else, NOT the reporter.
|
||||
signed_membership_event(&["f".repeat(64)]),
|
||||
signed_reporter_target(&reporter_secret, "model-open", &endpoint),
|
||||
];
|
||||
|
||||
let closed = super::availability_from_events_for_mode(
|
||||
events.clone(),
|
||||
MeshRelayMode::ClosedMembershipEnforced,
|
||||
);
|
||||
assert!(
|
||||
closed.serve_targets.is_empty(),
|
||||
"an enforcing relay still intersects routing with the roster"
|
||||
);
|
||||
|
||||
let open = super::availability_from_events_for_mode(events, MeshRelayMode::OpenNoMembership);
|
||||
assert_eq!(
|
||||
open.serve_targets.len(),
|
||||
1,
|
||||
"an open relay accepts a non-roster reporter's status: {reporter_pubkey} was hidden"
|
||||
);
|
||||
assert_eq!(open.serve_targets[0].endpoint_addr, endpoint);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn closed_relay_without_snapshot_never_silently_opens_up() {
|
||||
use super::MeshRelayMode;
|
||||
|
||||
Reference in New Issue
Block a user