diff --git a/desktop/src-tauri/src/commands/messages.rs b/desktop/src-tauri/src/commands/messages.rs index 479a13a63..ae4088997 100644 --- a/desktop/src-tauri/src/commands/messages.rs +++ b/desktop/src-tauri/src/commands/messages.rs @@ -264,10 +264,31 @@ pub async fn get_thread_replies( cap, thread_order.as_deref(), cursor.as_ref(), + true, ); let events = query_relay(&state, &[serde_json::Value::Object(filter)]).await?; - thread_page::parse(events, &root_event_id) + match thread_page::parse(events, &root_event_id) { + Ok(page) => Ok(page), + Err(thread_page::ParseError::MissingBounds) => { + // Relays predating the bounds extension ignore the opt-in fields. + // Retry their historical oldest-first contract so Desktop releases + // remain safe during partial rollout and relay rollback. + let legacy_filter = build_thread_replies_filter( + &root_event_id, + channel_id.as_deref(), + depth_limit.unwrap_or(64), + cap, + None, + cursor.as_ref(), + false, + ); + let legacy_events = + query_relay(&state, &[serde_json::Value::Object(legacy_filter)]).await?; + Ok(thread_page::parse_legacy(legacy_events, cap)) + } + Err(error) => Err(error.to_string()), + } } /// Build the relay `/query` filter for the server-side thread-subtree read. @@ -292,6 +313,7 @@ fn build_thread_replies_filter( cap: u32, thread_order: Option<&str>, cursor: Option<&crate::models::ThreadCursor>, + include_bounds: bool, ) -> serde_json::Map { let mut filter = serde_json::Map::new(); filter.insert("#e".to_string(), serde_json::json!([root_event_id])); @@ -300,11 +322,13 @@ fn build_thread_replies_filter( // defaults it to a deep-but-bounded value so nested replies aren't dropped. filter.insert("depth_limit".to_string(), serde_json::json!(depth_limit)); filter.insert("limit".to_string(), serde_json::json!(cap)); - // Opt into the relay's authoritative raw-scan bounds overlay. Older clients - // do not understand protocol events in this response and must not receive it. - filter.insert("thread_bounds".to_string(), serde_json::json!(true)); - if matches!(thread_order, Some("newest")) { - filter.insert("thread_order".to_string(), serde_json::json!("newest")); + // Opt into authoritative raw-scan bounds only on the modern attempt. + // Legacy retries omit both extensions to preserve the old relay contract. + if include_bounds { + filter.insert("thread_bounds".to_string(), serde_json::json!(true)); + if matches!(thread_order, Some("newest")) { + filter.insert("thread_order".to_string(), serde_json::json!("newest")); + } } if let Some(cid) = channel_id { filter.insert("#h".to_string(), serde_json::json!([cid])); diff --git a/desktop/src-tauri/src/commands/messages/thread_page.rs b/desktop/src-tauri/src/commands/messages/thread_page.rs index 7edd7e0cb..317494948 100644 --- a/desktop/src-tauri/src/commands/messages/thread_page.rs +++ b/desktop/src-tauri/src/commands/messages/thread_page.rs @@ -1,8 +1,26 @@ use nostr::Event; use serde::Deserialize; +use std::fmt; use crate::models::{ThreadCursor, ThreadRepliesResponse}; +#[derive(Debug, PartialEq, Eq)] +pub(super) enum ParseError { + MissingBounds, + InvalidBounds(String), +} + +impl fmt::Display for ParseError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::MissingBounds => { + formatter.write_str("Thread response omitted its bounds overlay") + } + Self::InvalidBounds(message) => formatter.write_str(message), + } + } +} + #[derive(Deserialize)] struct BoundsCursor { created_at: i64, @@ -18,13 +36,15 @@ struct BoundsPayload { pub(super) fn parse( events: Vec, root_event_id: &str, -) -> Result { +) -> Result { let mut replies = Vec::new(); let mut bounds = None; for event in events { if event.kind.as_u16() as u32 == buzz_core_pkg::kind::KIND_THREAD_BOUNDS { if bounds.is_some() { - return Err("Thread response contained multiple bounds overlays".to_string()); + return Err(ParseError::InvalidBounds( + "Thread response contained multiple bounds overlays".to_string(), + )); } let matches_root = event.tags.iter().any(|tag| { let values = tag.as_slice(); @@ -32,23 +52,28 @@ pub(super) fn parse( && values.get(1).map(String::as_str) == Some(root_event_id) }); if !matches_root { - return Err("Thread bounds overlay does not match the requested root".to_string()); + return Err(ParseError::InvalidBounds( + "Thread bounds overlay does not match the requested root".to_string(), + )); } bounds = Some( - serde_json::from_str::(&event.content) - .map_err(|error| format!("Invalid thread bounds overlay: {error}"))?, + serde_json::from_str::(&event.content).map_err(|error| { + ParseError::InvalidBounds(format!("Invalid thread bounds overlay: {error}")) + })?, ); } else if let Ok(value) = serde_json::to_value(&event) { replies.push(value); } } - let bounds = bounds.ok_or_else(|| "Thread response omitted its bounds overlay".to_string())?; + let bounds = bounds.ok_or(ParseError::MissingBounds)?; let next_cursor = bounds.next_cursor.map(|cursor| ThreadCursor { created_at: cursor.created_at, event_id: cursor.id, }); if bounds.has_more != next_cursor.is_some() { - return Err("Thread bounds has_more and next_cursor disagree".to_string()); + return Err(ParseError::InvalidBounds( + "Thread bounds has_more and next_cursor disagree".to_string(), + )); } Ok(ThreadRepliesResponse { events: replies, @@ -56,3 +81,25 @@ pub(super) fn parse( next_cursor, }) } + +pub(super) fn parse_legacy(events: Vec, cap: u32) -> ThreadRepliesResponse { + let has_more = events.len() as u32 >= cap; + let next_cursor = if has_more { + events.last().map(|event| ThreadCursor { + created_at: event.created_at.as_secs() as i64, + event_id: event.id.to_hex(), + }) + } else { + None + }; + let events = events + .iter() + .filter_map(|event| serde_json::to_value(event).ok()) + .collect(); + + ThreadRepliesResponse { + events, + has_more, + next_cursor, + } +} diff --git a/desktop/src-tauri/src/commands/messages_tests.rs b/desktop/src-tauri/src/commands/messages_tests.rs index ab2b37f2d..966cb39cb 100644 --- a/desktop/src-tauri/src/commands/messages_tests.rs +++ b/desktop/src-tauri/src/commands/messages_tests.rs @@ -1,16 +1,21 @@ use super::*; use nostr::{EventBuilder, Kind, Tag}; -fn thread_page_event(kind: u32, content: &str, root: Option<&str>) -> Event { +fn thread_page_event_at(kind: u32, content: &str, root: Option<&str>, created_at: u64) -> Event { let tags = root .map(|root| vec![Tag::parse(["e", root]).expect("valid root tag")]) .unwrap_or_default(); EventBuilder::new(Kind::Custom(kind as u16), content) .tags(tags) + .custom_created_at(nostr::Timestamp::from(created_at)) .sign_with_keys(&Keys::generate()) .expect("test event should sign") } +fn thread_page_event(kind: u32, content: &str, root: Option<&str>) -> Event { + thread_page_event_at(kind, content, root, 1_700_000_000) +} + fn bounds_event(root: &str, content: &str) -> Event { thread_page_event(buzz_core_pkg::kind::KIND_THREAD_BOUNDS, content, Some(root)) } @@ -161,7 +166,8 @@ fn thread_replies_filter_carries_non_p_gated_kinds_to_clear_the_gate() { // and every kind MUST be non-p-gated (else the gate still fires). The // Playwright mock does not model p-gating, so this unit test is the // only guard against the client/relay auth contract drifting. - let filter = build_thread_replies_filter("root-hex", Some("channel-1"), 64, 200, None, None); + let filter = + build_thread_replies_filter("root-hex", Some("channel-1"), 64, 200, None, None, true); let kinds = filter .get("kinds") @@ -191,8 +197,15 @@ fn thread_replies_filter_pages_with_composite_cursor() { created_at: 1_700_000_000, event_id: "abcd".to_string(), }; - let filter = - build_thread_replies_filter("root-hex", None, 64, 200, Some("newest"), Some(&cursor)); + let filter = build_thread_replies_filter( + "root-hex", + None, + 64, + 200, + Some("newest"), + Some(&cursor), + true, + ); assert_eq!(filter["thread_order"], serde_json::json!("newest")); assert_eq!(filter["thread_cursor"], serde_json::json!(1_700_000_000)); assert_eq!(filter["thread_cursor_id"], serde_json::json!("abcd")); @@ -202,6 +215,120 @@ fn thread_replies_filter_pages_with_composite_cursor() { ); } +#[test] +fn legacy_thread_filter_omits_modern_extensions_but_keeps_composite_cursor() { + let cursor = crate::models::ThreadCursor { + created_at: 1_700_000_000, + event_id: "abcd".to_string(), + }; + let filter = build_thread_replies_filter( + "root-hex", + Some("channel-1"), + 64, + 200, + None, + Some(&cursor), + false, + ); + + assert!(!filter.contains_key("thread_bounds")); + assert!(!filter.contains_key("thread_order")); + assert_eq!(filter["thread_cursor"], serde_json::json!(1_700_000_000)); + assert_eq!(filter["thread_cursor_id"], serde_json::json!("abcd")); +} + +#[test] +fn legacy_thread_pages_converge_past_two_same_second_boundaries() { + let mut all_events: Vec = (0..401) + .map(|_| thread_page_event_at(9, "reply", Some("root"), 1_700_000_000)) + .collect(); + // Old relays order and keyset on (created_at ASC, event_id ASC). + all_events.sort_by_key(|event| (event.created_at.as_secs(), event.id.to_hex())); + let expected_ids: Vec = all_events.iter().map(|event| event.id.to_hex()).collect(); + let mut received_ids = Vec::new(); + let mut cursor: Option = None; + + loop { + let modern_filter = build_thread_replies_filter( + "root", + Some("channel-1"), + 64, + 200, + Some("newest"), + cursor.as_ref(), + true, + ); + assert_eq!(modern_filter["thread_bounds"], serde_json::json!(true)); + assert_eq!(modern_filter["thread_order"], serde_json::json!("newest")); + + let select_old_relay_page = |request: &serde_json::Map| { + let cursor_created_at = request + .get("thread_cursor") + .and_then(|value| value.as_i64()); + let cursor_id = request + .get("thread_cursor_id") + .and_then(|value| value.as_str()); + all_events + .iter() + .filter(|event| match (cursor_created_at, cursor_id) { + (Some(created_at), Some(event_id)) => { + (event.created_at.as_secs() as i64, event.id.to_hex()) + > (created_at, event_id.to_string()) + } + _ => true, + }) + .take(200) + .cloned() + .collect::>() + }; + + // An old relay ignores the modern extensions and omits the overlay. + let modern_response = select_old_relay_page(&modern_filter); + assert!(matches!( + thread_page::parse(modern_response, "root"), + Err(thread_page::ParseError::MissingBounds) + )); + + let legacy_filter = build_thread_replies_filter( + "root", + Some("channel-1"), + 64, + 200, + None, + cursor.as_ref(), + false, + ); + assert!(!legacy_filter.contains_key("thread_bounds")); + assert!(!legacy_filter.contains_key("thread_order")); + if let Some(expected_cursor) = cursor.as_ref() { + assert_eq!( + legacy_filter["thread_cursor"], + serde_json::json!(expected_cursor.created_at) + ); + assert_eq!( + legacy_filter["thread_cursor_id"], + serde_json::json!(expected_cursor.event_id) + ); + } + + let page = thread_page::parse_legacy(select_old_relay_page(&legacy_filter), 200); + received_ids.extend( + page.events + .iter() + .map(|event| event["id"].as_str().expect("event id").to_string()), + ); + if !page.has_more { + assert!(page.next_cursor.is_none()); + break; + } + cursor = page.next_cursor; + } + + assert_eq!(received_ids, expected_ids); + let unique_ids: std::collections::HashSet<_> = received_ids.iter().collect(); + assert_eq!(unique_ids.len(), 401); +} + #[test] fn thread_page_parser_accepts_valid_empty_and_non_empty_pages() { let empty = thread_page::parse( @@ -243,6 +370,7 @@ fn thread_page_parser_rejects_missing_duplicate_and_wrong_root_bounds() { assert!(thread_page::parse(vec![reply], "root") .err() .expect("missing bounds must fail closed") + .to_string() .contains("omitted")); let terminal = r#"{"has_more":false,"next_cursor":null}"#; @@ -255,12 +383,14 @@ fn thread_page_parser_rejects_missing_duplicate_and_wrong_root_bounds() { ) .err() .expect("duplicate bounds must fail closed") + .to_string() .contains("multiple")); assert!( thread_page::parse(vec![bounds_event("other-root", terminal)], "root") .err() .expect("mismatched root must fail closed") + .to_string() .contains("does not match") ); } @@ -271,6 +401,7 @@ fn thread_page_parser_rejects_malformed_and_inconsistent_bounds() { thread_page::parse(vec![bounds_event("root", "not-json")], "root") .err() .expect("malformed bounds must fail closed") + .to_string() .contains("Invalid thread bounds") ); @@ -282,6 +413,7 @@ fn thread_page_parser_rejects_malformed_and_inconsistent_bounds() { thread_page::parse(vec![bounds_event("root", content)], "root") .err() .expect("cursor presence must agree with has_more") + .to_string() .contains("disagree") ); }