From f38d2b2ef81b254edb6f0a58429ddb1a63c0e418 Mon Sep 17 00:00:00 2001
From: cloudhead <cloudhead@radicle.xyz>
Date: Tue, 30 Jan 2024 15:58:05 +0100
Subject: [PATCH] Prevent multiple events per synced peer

We were not properly guarding against the fact that we could receive
multiple sync events from the same node. Normally though, this wouldn't
happen, as further announcements would be stale.
---
 radicle/src/node.rs | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/radicle/src/node.rs b/radicle/src/node.rs
index 796e43f4..dfc7d6d4 100644
--- a/radicle/src/node.rs
+++ b/radicle/src/node.rs
@@ -1,4 +1,5 @@
 #![allow(clippy::type_complexity)]
+#![allow(clippy::collapsible_if)]
 mod features;
 
 pub mod address;
@@ -897,13 +898,13 @@ impl Node {
         rid: RepoId,
         seeds: impl IntoIterator<Item = NodeId>,
         timeout: time::Duration,
-        mut callback: impl FnMut(AnnounceEvent, &[PublicKey]) -> ControlFlow<()>,
+        mut callback: impl FnMut(AnnounceEvent, &HashSet<PublicKey>) -> ControlFlow<()>,
     ) -> Result<AnnounceResult, Error> {
         let events = self.subscribe(timeout)?;
         let refs = self.announce_refs(rid)?;
 
         let mut unsynced = seeds.into_iter().collect::<BTreeSet<_>>();
-        let mut synced = Vec::new();
+        let mut synced = HashSet::new();
         let mut timeout: Vec<NodeId> = Vec::new();
 
         callback(AnnounceEvent::Announced, &synced);
@@ -915,10 +916,15 @@ impl Node {
                     rid: rid_,
                     at,
                 }) if rid == rid_ && refs.at == at => {
+                    log::debug!(target: "radicle", "Received {e:?}");
+
                     unsynced.remove(&remote);
-                    synced.push(remote);
-                    if callback(AnnounceEvent::RefsSynced { remote }, &synced).is_break() {
-                        break;
+                    // We can receive synced events from nodes we didn't directly announce to,
+                    // and it's possible to receive duplicates as well.
+                    if synced.insert(remote) {
+                        if callback(AnnounceEvent::RefsSynced { remote }, &synced).is_break() {
+                            break;
+                        }
                     }
                 }
                 Ok(_) => {}
@@ -933,7 +939,10 @@ impl Node {
                 break;
             }
         }
-        Ok(AnnounceResult { timeout, synced })
+        Ok(AnnounceResult {
+            timeout,
+            synced: synced.into_iter().collect(),
+        })
     }
 }