Skip to content

Commit 2ef1ea4

Browse files
alltheseasclaude
andcommitted
fix: relay confirmation before merge and welcome (MIP-02/MIP-03)
Rewrite add_member_device and remove_member_device to defer merge_pending_commit until after relay confirmation, matching MDK's documented ordering requirement. Before: merge locally → fire-and-forget publish → send welcomes After: create commit → return immediately → background: relay publish with retries → merge → send welcomes → UI update The Tauri command returns immediately (no UX blocking). A new publish_event_with_retries helper provides 5-attempt exponential backoff matching pika core's pattern. This prevents permanent group state divergence when all relays reject the commit, and ensures welcomes reference a commit that is actually available on relays (MIP-02). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 069bcbc commit 2ef1ea4

1 file changed

Lines changed: 163 additions & 82 deletions

File tree

src-tauri/src/mls/mod.rs

Lines changed: 163 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,51 @@ pub use tracking::{is_mls_event_processed, track_mls_event_processed, cleanup_ol
2727
use types::{has_encoding_tag, KeyPackageIndexEntry};
2828
use tracking::wipe_legacy_mls_database;
2929

30+
/// Publish a nostr event to TRUSTED_RELAYS with retries and exponential backoff.
31+
///
32+
/// Matches the retry pattern used in pika core's `publish_evolution_event`:
33+
/// 5 attempts, 250ms base backoff, retries on NIP-42 auth/protected errors.
34+
/// Returns `Ok(())` when at least one relay confirms, `Err` after exhausting retries.
35+
async fn publish_event_with_retries(
36+
client: &nostr_sdk::Client,
37+
event: &nostr_sdk::Event,
38+
) -> Result<(), String> {
39+
use std::time::Duration;
40+
41+
let mut last_err: Option<String> = None;
42+
for attempt in 0..5u8 {
43+
match client
44+
.send_event_to(TRUSTED_RELAYS.iter().copied(), event)
45+
.await
46+
{
47+
Ok(output) if !output.success.is_empty() => {
48+
return Ok(());
49+
}
50+
Ok(output) => {
51+
let errors: Vec<&str> = output.failed.values().map(|s| s.as_str()).collect();
52+
let summary = if errors.is_empty() {
53+
"no relay accepted event".to_string()
54+
} else {
55+
errors.join("; ")
56+
};
57+
let any_retryable = errors.iter().any(|e| {
58+
e.contains("protected") || e.contains("auth") || e.contains("AUTH")
59+
});
60+
last_err = Some(summary);
61+
if !any_retryable {
62+
break;
63+
}
64+
}
65+
Err(e) => {
66+
last_err = Some(e.to_string());
67+
}
68+
}
69+
let delay_ms = 250u64.saturating_mul(1u64 << attempt);
70+
tokio::time::sleep(Duration::from_millis(delay_ms)).await;
71+
}
72+
Err(last_err.unwrap_or_else(|| "unknown error".to_string()))
73+
}
74+
3075
/// Main MLS service facade
3176
///
3277
/// Responsibilities:
@@ -478,9 +523,11 @@ impl MlsService {
478523
///
479524
/// This will:
480525
/// 1. Fetch the device's keypackage from the network
481-
/// 2. Add the device to the group via nostr-mls
482-
/// 3. Send the welcome message
483-
/// 4. Update group metadata
526+
/// 2. Create the add-member commit via MDK (does not merge yet)
527+
/// 3. Return immediately — relay publish, merge, welcome, and metadata
528+
/// update happen in a background task (MIP-02 / MIP-03 ordering)
529+
///
530+
/// Background ordering: relay confirm → merge_pending_commit → send welcomes → UI update
484531
pub async fn add_member_device(
485532
&self,
486533
group_id: &str,
@@ -559,75 +606,89 @@ impl MlsService {
559606
// Convert engine_group_id hex to GroupId
560607
let mls_group_id = GroupId::from_slice(&hex_string_to_bytes(&group_meta.engine_group_id));
561608

562-
// Perform engine operations: add member and merge commit BEFORE publishing
563-
// This ensures our local state is correct before announcing to the network
609+
// Create the commit but do NOT merge yet — merge only after relay confirmation
610+
// (MIP-02: commit must be on relay before welcome; MIP-03: relay ack before local merge)
564611
let (evolution_event, welcome_rumors) = {
565612
let engine = self.engine()?;
566613

567-
// Add member to group - returns AddMembersResult with evolution_event and welcome_rumors
568614
let add_result = engine
569615
.add_members(&mls_group_id, std::slice::from_ref(&kp_event))
570616
.map_err(|e| {
571617
eprintln!("[MLS] Failed to add member: {}", e);
572618
MlsError::NostrMlsError(format!("Failed to add member: {}", e))
573619
})?;
574620

575-
// CRITICAL: Merge the pending commit immediately after creating it
576-
// This ensures our local state is correct BEFORE publishing to the network
577-
// If we publish first and merge fails, remote and local state will desync
578-
engine
579-
.merge_pending_commit(&mls_group_id)
580-
.map_err(|e| {
581-
eprintln!("[MLS] Failed to merge commit: {}", e);
582-
MlsError::NostrMlsError(format!("Failed to merge commit: {}", e))
583-
})?;
584-
585621
(add_result.evolution_event, add_result.welcome_rumors)
586622
};
587623

588-
// Publish evolution event (commit) in the background — local state is already updated
589-
let group_id_clone = group_id.to_string();
624+
// Spawn background task: relay publish → merge → welcomes → UI update.
625+
// The Tauri command returns immediately so the frontend isn't blocked.
626+
let db_path = self.db_path.clone();
627+
let group_id_owned = group_id.to_string();
628+
let engine_group_id = group_meta.engine_group_id.clone();
590629
tokio::spawn(async move {
591630
let client = NOSTR_CLIENT.get().unwrap();
592-
match client.send_event(&evolution_event).await {
593-
Ok(_) => {
594-
if let Some(handle) = TAURI_APP.get() {
595-
let _ = track_mls_event_processed(handle, &evolution_event.id.to_hex(), &group_id_clone, evolution_event.created_at.as_secs());
596-
}
597-
}
598-
Err(e) => eprintln!("[MLS] Failed to publish commit: {}", e),
631+
632+
// 1. Publish evolution event with retries
633+
if let Err(e) = publish_event_with_retries(client, &evolution_event).await {
634+
eprintln!("[MLS] Failed to publish commit after retries: {}", e);
635+
return;
599636
}
600-
});
601637

602-
// Send welcome messages to the new member (concurrently)
603-
if let Some(welcome_rumors) = welcome_rumors {
604-
let futs: Vec<_> = welcome_rumors
605-
.into_iter()
606-
.map(|welcome| async {
607-
if let Err(e) = client.gift_wrap_to(TRUSTED_RELAYS.iter().copied(), &member_pk, welcome, []).await {
608-
eprintln!("[MLS] Failed to send welcome: {}", e);
609-
}
610-
})
611-
.collect();
612-
futures_util::future::join_all(futs).await;
613-
}
638+
// Track the published event
639+
if let Some(handle) = TAURI_APP.get() {
640+
let _ = track_mls_event_processed(
641+
handle,
642+
&evolution_event.id.to_hex(),
643+
&group_id_owned,
644+
evolution_event.created_at.as_secs(),
645+
);
646+
}
614647

615-
// Update group metadata timestamp
616-
let mut groups = self.read_groups().await?;
617-
if let Some(group) = groups.iter_mut().find(|g| g.group_id == group_id) {
618-
group.updated_at = std::time::SystemTime::now()
619-
.duration_since(std::time::UNIX_EPOCH)
620-
.unwrap()
621-
.as_secs();
622-
self.write_groups(&groups).await?;
623-
}
648+
// 2. Merge pending commit now that relay confirmed
649+
let mls_group_id = GroupId::from_slice(&hex_string_to_bytes(&engine_group_id));
650+
let storage = match MdkSqliteStorage::new_unencrypted(&db_path) {
651+
Ok(s) => s,
652+
Err(e) => {
653+
eprintln!("[MLS] Failed to open storage for merge: {}", e);
654+
return;
655+
}
656+
};
657+
let engine = MDK::new(storage);
658+
if let Err(e) = engine.merge_pending_commit(&mls_group_id) {
659+
eprintln!("[MLS] Failed to merge commit after relay confirm: {}", e);
660+
return;
661+
}
624662

625-
// Emit event to refresh UI
626-
if let Some(handle) = TAURI_APP.get() {
627-
handle.emit("mls_group_updated", serde_json::json!({
628-
"group_id": group_id
629-
})).ok();
630-
}
663+
// 3. Send welcome messages (only after commit is on relay)
664+
if let Some(welcome_rumors) = welcome_rumors {
665+
let futs: Vec<_> = welcome_rumors
666+
.into_iter()
667+
.map(|welcome| async move {
668+
if let Err(e) = client.gift_wrap_to(TRUSTED_RELAYS.iter().copied(), &member_pk, welcome, []).await {
669+
eprintln!("[MLS] Failed to send welcome: {}", e);
670+
}
671+
})
672+
.collect();
673+
futures_util::future::join_all(futs).await;
674+
}
675+
676+
// 4. Update group metadata timestamp and emit UI refresh
677+
if let Some(handle) = TAURI_APP.get() {
678+
if let Ok(mut groups) = crate::db::load_mls_groups(handle).await {
679+
if let Some(group) = groups.iter_mut().find(|g| g.group_id == group_id_owned) {
680+
group.updated_at = std::time::SystemTime::now()
681+
.duration_since(std::time::UNIX_EPOCH)
682+
.unwrap()
683+
.as_secs();
684+
let _ = crate::db::save_mls_groups(handle.clone(), &groups).await;
685+
}
686+
}
687+
handle.emit("mls_group_updated", serde_json::json!({
688+
"group_id": group_id_owned
689+
})).ok();
690+
}
691+
});
631692

632693
Ok(())
633694
}
@@ -735,10 +796,11 @@ impl MlsService {
735796
/// Remove a member device from a group (admin only)
736797
///
737798
/// This will:
738-
/// 1. Remove the member using MDK's remove_members()
739-
/// 2. Publish the commit message to remaining group members
740-
/// 3. Merge the pending commit locally
741-
/// 4. Emit UI update event
799+
/// 1. Remove the member using MDK's remove_members() (does not merge yet)
800+
/// 2. Return immediately — relay publish, merge, and UI update happen in
801+
/// a background task (MIP-03 ordering)
802+
///
803+
/// Background ordering: relay confirm → merge_pending_commit → UI update
742804
pub async fn remove_member_device(
743805
&self,
744806
group_id: &str,
@@ -765,6 +827,9 @@ impl MlsService {
765827

766828
// Perform engine operation: remove member and merge commit BEFORE publishing
767829
// This ensures our local state is correct before announcing to the network
830+
// Create the commit but do NOT merge yet — merge only after relay confirmation
831+
// (MIP-03: relay ack before local merge)
832+
//
768833
// Note: We intentionally do NOT sync before removal. Syncing can re-process
769834
// our own commits from the relay, which may corrupt the tree state after
770835
// multiple kick/re-invite cycles. A fresh engine reads the latest SQLite state.
@@ -785,47 +850,63 @@ impl MlsService {
785850
));
786851
}
787852

788-
// Remove member from group - returns RemoveMembersResult with evolution_event
789853
let remove_result = engine
790854
.remove_members(&mls_group_id, &[member_pk])
791855
.map_err(|e| {
792856
eprintln!("[MLS] Failed to remove member: {}", e);
793857
MlsError::NostrMlsError(format!("Failed to remove member: {}", e))
794858
})?;
795859

796-
// CRITICAL: Merge the pending commit immediately after creating it
797-
// This ensures our local state is correct BEFORE publishing to the network
798-
// If we publish first and merge fails, remote and local state will desync
799-
engine
800-
.merge_pending_commit(&mls_group_id)
801-
.map_err(|e| {
802-
eprintln!("[MLS] Failed to merge commit: {}", e);
803-
MlsError::NostrMlsError(format!("Failed to merge commit: {}", e))
804-
})?;
805-
806860
remove_result.evolution_event
807861
};
808862

809-
// Publish evolution event (commit) in the background — local state is already updated
810-
let group_id_clone = group_id.to_string();
863+
// Spawn background task: relay publish → merge → UI update.
864+
// The Tauri command returns immediately so the frontend isn't blocked.
865+
let db_path = self.db_path.clone();
866+
let group_id_owned = group_id.to_string();
867+
let engine_group_id = group_meta.engine_group_id.clone();
811868
tokio::spawn(async move {
812869
let client = NOSTR_CLIENT.get().unwrap();
813-
match client.send_event(&evolution_event).await {
814-
Ok(_) => {
815-
if let Some(handle) = TAURI_APP.get() {
816-
let _ = track_mls_event_processed(handle, &evolution_event.id.to_hex(), &group_id_clone, evolution_event.created_at.as_secs());
817-
}
870+
871+
// 1. Publish evolution event with retries
872+
if let Err(e) = publish_event_with_retries(client, &evolution_event).await {
873+
eprintln!("[MLS] Failed to publish remove commit after retries: {}", e);
874+
return;
875+
}
876+
877+
// Track the published event
878+
if let Some(handle) = TAURI_APP.get() {
879+
let _ = track_mls_event_processed(
880+
handle,
881+
&evolution_event.id.to_hex(),
882+
&group_id_owned,
883+
evolution_event.created_at.as_secs(),
884+
);
885+
}
886+
887+
// 2. Merge pending commit now that relay confirmed
888+
let mls_group_id = GroupId::from_slice(&hex_string_to_bytes(&engine_group_id));
889+
let storage = match MdkSqliteStorage::new_unencrypted(&db_path) {
890+
Ok(s) => s,
891+
Err(e) => {
892+
eprintln!("[MLS] Failed to open storage for merge: {}", e);
893+
return;
818894
}
819-
Err(e) => eprintln!("[MLS] Failed to publish commit: {}", e),
895+
};
896+
let engine = MDK::new(storage);
897+
if let Err(e) = engine.merge_pending_commit(&mls_group_id) {
898+
eprintln!("[MLS] Failed to merge commit after relay confirm: {}", e);
899+
return;
900+
}
901+
902+
// 3. Emit event to refresh UI member list
903+
if let Some(handle) = TAURI_APP.get() {
904+
handle.emit("mls_group_updated", serde_json::json!({
905+
"group_id": group_id_owned
906+
})).ok();
820907
}
821908
});
822909

823-
// Emit event to refresh UI member list
824-
if let Some(handle) = TAURI_APP.get() {
825-
handle.emit("mls_group_updated", serde_json::json!({
826-
"group_id": group_id
827-
})).ok();
828-
}
829910
Ok(())
830911
}
831912

0 commit comments

Comments
 (0)