removes manual trait impl for contact-info (#17332) (#17335)

The current implementations use only the id and disregard other fields,
in particular wallclock. This can lead to bugs where an outdated
contact-info shadows or overrides a current one because they compare
equal.

(cherry picked from commit 13b032b)

Co-authored-by: behzad nouri <behzadnouri@gmail.com>

core/src/contact_info.rs

@@ -6,11 +6,10 @@ use solana_sdk::sanitize::{Sanitize, SanitizeError};
 #[cfg(test)]
 use solana_sdk::signature::{Keypair, Signer};
 use solana_sdk::timing::timestamp;
-use std::cmp::{Ord, Ordering, PartialEq, PartialOrd};
 use std::net::{IpAddr, SocketAddr};
 
 /// Structure representing a node on the network
-#[derive(Serialize, Deserialize, Clone, Debug, AbiExample)]
+#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, AbiExample, Deserialize, Serialize)]
 pub struct ContactInfo {
     pub id: Pubkey,
     /// gossip address
@@ -48,34 +47,13 @@ impl Sanitize for ContactInfo {
     }
 }
 
-impl Ord for ContactInfo {
-    fn cmp(&self, other: &Self) -> Ordering {
-        self.id.cmp(&other.id)
-    }
-}
-
-impl PartialOrd for ContactInfo {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl PartialEq for ContactInfo {
-    fn eq(&self, other: &Self) -> bool {
-        self.id == other.id
-    }
-}
-
-impl Eq for ContactInfo {}
-
 #[macro_export]
 macro_rules! socketaddr {
     ($ip:expr, $port:expr) => {
         std::net::SocketAddr::from((std::net::Ipv4Addr::from($ip), $port))
     };
     ($str:expr) => {{
-        let a: std::net::SocketAddr = $str.parse().unwrap();
-        a
+        $str.parse::<std::net::SocketAddr>().unwrap()
     }};
 }
 #[macro_export]

core/src/crds.rs

@@ -19,7 +19,7 @@
 //! CrdsValue enums.
 //!
 //! Merge strategy is implemented in:
-//!     impl PartialOrd for VersionedCrdsValue
+//!     fn overrides(value: &CrdsValue, other: &VersionedCrdsValue) -> bool
 //!
 //! A value is updated to a new version if the labels match, and the value
 //! wallclock is later, or the value hash is greater.
@@ -69,8 +69,6 @@ pub enum CrdsError {
 }
 
 /// This structure stores some local metadata associated with the CrdsValue
-/// The implementation of PartialOrd ensures that the "highest" version is always picked to be
-/// stored in the Crds
 #[derive(PartialEq, Debug, Clone)]
 pub struct VersionedCrdsValue {
     /// Ordinal index indicating insert order.

core/src/crds_gossip_push.rs

@@ -759,36 +759,32 @@ mod test {
     #[test]
     fn test_personalized_push_messages() {
         let now = timestamp();
+        let mut rng = rand::thread_rng();
         let mut crds = Crds::default();
         let mut push = CrdsGossipPush::default();
-        let peer_1 = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
-            &solana_sdk::pubkey::new_rand(),
-            0,
-        )));
-        assert_eq!(crds.insert(peer_1.clone(), now), Ok(None));
-        let peer_2 = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
-            &solana_sdk::pubkey::new_rand(),
-            0,
-        )));
-        assert_eq!(crds.insert(peer_2.clone(), now), Ok(None));
-        let peer_3 = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
-            &solana_sdk::pubkey::new_rand(),
-            now,
-        )));
+        let peers: Vec<_> = vec![0, 0, now]
+            .into_iter()
+            .map(|wallclock| {
+                let mut peer = ContactInfo::new_rand(&mut rng, /*pubkey=*/ None);
+                peer.wallclock = wallclock;
+                CrdsValue::new_unsigned(CrdsData::ContactInfo(peer))
+            })
+            .collect();
+        assert_eq!(crds.insert(peers[0].clone(), now), Ok(None));
+        assert_eq!(crds.insert(peers[1].clone(), now), Ok(None));
         assert_eq!(
-            push.process_push_message(&mut crds, &Pubkey::default(), peer_3.clone(), now),
+            push.process_push_message(&mut crds, &Pubkey::default(), peers[2].clone(), now),
             Ok(None)
         );
         push.refresh_push_active_set(&crds, &HashMap::new(), None, &Pubkey::default(), 0, 1, 1);
 
         // push 3's contact info to 1 and 2 and 3
-        let new_msg = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
-            &peer_3.pubkey(),
-            0,
-        )));
-        let mut expected = HashMap::new();
-        expected.insert(peer_1.pubkey(), vec![new_msg.clone()]);
-        expected.insert(peer_2.pubkey(), vec![new_msg]);
+        let expected: HashMap<_, _> = vec![
+            (peers[0].pubkey(), vec![peers[2].clone()]),
+            (peers[1].pubkey(), vec![peers[2].clone()]),
+        ]
+        .into_iter()
+        .collect();
         assert_eq!(push.active_set.len(), 3);
         assert_eq!(push.new_push_messages(&crds, now), expected);
     }