v2.0: checks for duplicate instances using the new ContactInfo (backport of #2506) (#2510)

* checks for duplicate instances using the new ContactInfo (#2506)

Working towards deprecating NodeInstance CRDS value, the commit adds
check for duplicate instances using the new ContactInfo.

(cherry picked from commit 1d825df)

* removes unwarp

---------

Co-authored-by: behzad nouri <[email protected]>

Author: mergify[bot]

gossip/src/cluster_info.rs

@@ -2495,16 +2495,21 @@ impl ClusterInfo {
 
         // Check if there is a duplicate instance of
         // this node with more recent timestamp.
-        let instance = self.instance.read().unwrap();
-        let check_duplicate_instance = |values: &[CrdsValue]| {
-            if should_check_duplicate_instance {
-                for value in values {
-                    if instance.check_duplicate(value) {
-                        return Err(GossipError::DuplicateNodeInstance);
-                    }
+        let check_duplicate_instance = {
+            let instance = self.instance.read().unwrap();
+            let my_contact_info = self.my_contact_info();
+            move |values: &[CrdsValue]| {
+                if should_check_duplicate_instance
+                    && values.iter().any(|value| {
+                        instance.check_duplicate(value)
+                            || matches!(&value.data, CrdsData::ContactInfo(other)
+                                if my_contact_info.check_duplicate(other))
+                    })
+                {
+                    return Err(GossipError::DuplicateNodeInstance);
                 }
+                Ok(())
             }
-            Ok(())
         };
         let mut pings = Vec::new();
         let mut rng = rand::thread_rng();

gossip/src/contact_info.rs

@@ -434,6 +434,14 @@ impl ContactInfo {
         node.set_serve_repair_quic((addr, port + 4)).unwrap();
         node
     }
+
+    // Returns true if the other contact-info is a duplicate instance of this
+    // node, with a more recent `outset` timestamp.
+    #[inline]
+    #[must_use]
+    pub(crate) fn check_duplicate(&self, other: &ContactInfo) -> bool {
+        self.pubkey == other.pubkey && self.outset < other.outset
+    }
 }
 
 impl Default for ContactInfo {
@@ -1015,4 +1023,59 @@ mod tests {
             Err(Error::InvalidPort(0))
         );
     }
+
+    #[test]
+    fn test_check_duplicate() {
+        let mut rng = rand::thread_rng();
+        let mut node = ContactInfo::new(
+            Keypair::new().pubkey(),
+            rng.gen(), // wallclock
+            rng.gen(), // shred_version
+        );
+        // Same contact-info is not a duplicate instance.
+        {
+            let other = node.clone();
+            assert!(!node.check_duplicate(&other));
+            assert!(!other.check_duplicate(&node));
+        }
+        // Updated socket address is not a duplicate instance.
+        {
+            let mut other = node.clone();
+            while other.set_gossip(new_rand_socket(&mut rng)).is_err() {}
+            while other.set_serve_repair(new_rand_socket(&mut rng)).is_err() {}
+            assert!(!node.check_duplicate(&other));
+            assert!(!other.check_duplicate(&node));
+            other.remove_serve_repair();
+            assert!(!node.check_duplicate(&other));
+            assert!(!other.check_duplicate(&node));
+        }
+        // Updated wallclock is not a duplicate instance.
+        {
+            let other = node.clone();
+            node.set_wallclock(rng.gen());
+            assert!(!node.check_duplicate(&other));
+            assert!(!other.check_duplicate(&node));
+        }
+        // Different pubkey is not a duplicate instance.
+        {
+            let other = ContactInfo::new(
+                Keypair::new().pubkey(),
+                rng.gen(), // wallclock
+                rng.gen(), // shred_version
+            );
+            assert!(!node.check_duplicate(&other));
+            assert!(!other.check_duplicate(&node));
+        }
+        // Same pubkey, more recent outset timestamp is a duplicate instance.
+        {
+            let other = ContactInfo::new(
+                node.pubkey,
+                rng.gen(), // wallclock
+                rng.gen(), // shred_version
+            );
+            assert!(node.outset < other.outset);
+            assert!(node.check_duplicate(&other));
+            assert!(!other.check_duplicate(&node));
+        }
+    }
 }