From 1d91b60a57b7d000fe0a72761c320d6f855b7c54 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Mon, 9 Oct 2023 15:22:34 +0000 Subject: [PATCH] removes unused legacy-snapshot-hashes api in gossip (#33593) https://github.com/solana-labs/solana/pull/33576 stops broadcasting legacy snapshot hashes over gossip, and this commit removes unused legacy snapshot hashed code in gossip. --- gossip/src/cluster_info.rs | 75 ++++---------------------------------- gossip/src/crds.rs | 8 ++-- gossip/src/crds_entry.rs | 11 +----- gossip/src/crds_value.rs | 8 ++-- 4 files changed, 17 insertions(+), 85 deletions(-) diff --git a/gossip/src/cluster_info.rs b/gossip/src/cluster_info.rs index b0b99b1c02dca4..113b387512608d 100644 --- a/gossip/src/cluster_info.rs +++ b/gossip/src/cluster_info.rs @@ -32,9 +32,8 @@ use { CrdsFilter, CrdsTimeouts, ProcessPullStats, CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS, }, crds_value::{ - self, AccountsHashes, CrdsData, CrdsValue, CrdsValueLabel, EpochSlotsIndex, - LegacySnapshotHashes, LowestSlot, NodeInstance, SnapshotHashes, Version, Vote, - MAX_WALLCLOCK, + self, AccountsHashes, CrdsData, CrdsValue, CrdsValueLabel, EpochSlotsIndex, LowestSlot, + NodeInstance, SnapshotHashes, Version, Vote, MAX_WALLCLOCK, }, duplicate_shred::DuplicateShred, epoch_slots::EpochSlots, @@ -118,10 +117,6 @@ pub(crate) const DUPLICATE_SHRED_MAX_PAYLOAD_SIZE: usize = PACKET_DATA_SIZE - 11 /// such that the serialized size of the push/pull message stays below /// PACKET_DATA_SIZE. pub const MAX_ACCOUNTS_HASHES: usize = 16; -/// Maximum number of hashes in LegacySnapshotHashes a node publishes -/// such that the serialized size of the push/pull message stays below -/// PACKET_DATA_SIZE. -pub const MAX_LEGACY_SNAPSHOT_HASHES: usize = 16; /// Maximum number of incremental hashes in SnapshotHashes a node publishes /// such that the serialized size of the push/pull message stays below /// PACKET_DATA_SIZE. @@ -997,20 +992,6 @@ impl ClusterInfo { self.push_message(CrdsValue::new_signed(message, &self.keypair())); } - pub fn push_legacy_snapshot_hashes(&self, snapshot_hashes: Vec<(Slot, Hash)>) { - if snapshot_hashes.len() > MAX_LEGACY_SNAPSHOT_HASHES { - warn!( - "snapshot hashes too large, ignored: {}", - snapshot_hashes.len(), - ); - return; - } - - let message = - CrdsData::LegacySnapshotHashes(LegacySnapshotHashes::new(self.id(), snapshot_hashes)); - self.push_message(CrdsValue::new_signed(message, &self.keypair())); - } - pub fn push_snapshot_hashes( &self, full: (Slot, Hash), @@ -1208,15 +1189,6 @@ impl ClusterInfo { .map(map) } - pub fn get_legacy_snapshot_hash_for_node(&self, pubkey: &Pubkey, map: F) -> Option - where - F: FnOnce(&Vec<(Slot, Hash)>) -> Y, - { - let gossip_crds = self.gossip.crds.read().unwrap(); - let hashes = &gossip_crds.get::<&LegacySnapshotHashes>(*pubkey)?.hashes; - Some(map(hashes)) - } - pub fn get_snapshot_hashes_for_node(&self, pubkey: &Pubkey) -> Option { self.gossip .crds @@ -3413,36 +3385,6 @@ mod tests { } } - #[test] - fn test_max_legecy_snapshot_hashes_with_push_messages() { - let mut rng = rand::thread_rng(); - for _ in 0..256 { - let snapshot_hash = LegacySnapshotHashes::new_rand(&mut rng, None); - let crds_value = CrdsValue::new_signed( - CrdsData::LegacySnapshotHashes(snapshot_hash), - &Keypair::new(), - ); - let message = Protocol::PushMessage(Pubkey::new_unique(), vec![crds_value]); - let socket = new_rand_socket_addr(&mut rng); - assert!(Packet::from_data(Some(&socket), message).is_ok()); - } - } - - #[test] - fn test_max_legacy_snapshot_hashes_with_pull_responses() { - let mut rng = rand::thread_rng(); - for _ in 0..256 { - let snapshot_hash = LegacySnapshotHashes::new_rand(&mut rng, None); - let crds_value = CrdsValue::new_signed( - CrdsData::LegacySnapshotHashes(snapshot_hash), - &Keypair::new(), - ); - let response = Protocol::PullResponse(Pubkey::new_unique(), vec![crds_value]); - let socket = new_rand_socket_addr(&mut rng); - assert!(Packet::from_data(Some(&socket), response).is_ok()); - } - } - #[test] fn test_max_snapshot_hashes_with_push_messages() { let mut rng = rand::thread_rng(); @@ -4110,16 +4052,15 @@ mod tests { fn test_split_messages_packet_size() { // Test that if a value is smaller than payload size but too large to be wrapped in a vec // that it is still dropped - let mut value = - CrdsValue::new_unsigned(CrdsData::LegacySnapshotHashes(LegacySnapshotHashes { - from: Pubkey::default(), - hashes: vec![], - wallclock: 0, - })); + let mut value = CrdsValue::new_unsigned(CrdsData::AccountsHashes(AccountsHashes { + from: Pubkey::default(), + hashes: vec![], + wallclock: 0, + })); let mut i = 0; while value.size() < PUSH_MESSAGE_MAX_PAYLOAD_SIZE as u64 { - value.data = CrdsData::LegacySnapshotHashes(LegacySnapshotHashes { + value.data = CrdsData::AccountsHashes(AccountsHashes { from: Pubkey::default(), hashes: vec![(0, Hash::default()); i], wallclock: 0, diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index d8ab6e45b3d593..b20ba9dfb15647 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -759,7 +759,7 @@ fn should_report_message_signature(signature: &Signature) -> bool { mod tests { use { super::*, - crate::crds_value::{new_rand_timestamp, LegacySnapshotHashes, NodeInstance}, + crate::crds_value::{new_rand_timestamp, AccountsHashes, NodeInstance}, rand::{thread_rng, Rng, SeedableRng}, rand_chacha::ChaChaRng, rayon::ThreadPoolBuilder, @@ -1341,8 +1341,8 @@ mod tests { ); assert_eq!(crds.get_shred_version(&pubkey), Some(8)); // Add other crds values with the same pubkey. - let val = LegacySnapshotHashes::new_rand(&mut rng, Some(pubkey)); - let val = CrdsData::LegacySnapshotHashes(val); + let val = AccountsHashes::new_rand(&mut rng, Some(pubkey)); + let val = CrdsData::AccountsHashes(val); let val = CrdsValue::new_unsigned(val); assert_eq!( crds.insert(val, timestamp(), GossipRoute::LocalMessage), @@ -1355,7 +1355,7 @@ mod tests { assert_eq!(crds.get::<&ContactInfo>(pubkey), None); assert_eq!(crds.get_shred_version(&pubkey), Some(8)); // Remove the remaining entry with the same pubkey. - crds.remove(&CrdsValueLabel::LegacySnapshotHashes(pubkey), timestamp()); + crds.remove(&CrdsValueLabel::AccountsHashes(pubkey), timestamp()); assert_eq!(crds.get_records(&pubkey).count(), 0); assert_eq!(crds.get_shred_version(&pubkey), None); } diff --git a/gossip/src/crds_entry.rs b/gossip/src/crds_entry.rs index ccb8ed310eea8a..526f04eb56aab8 100644 --- a/gossip/src/crds_entry.rs +++ b/gossip/src/crds_entry.rs @@ -2,8 +2,7 @@ use { crate::{ crds::VersionedCrdsValue, crds_value::{ - CrdsData, CrdsValue, CrdsValueLabel, LegacySnapshotHashes, LegacyVersion, LowestSlot, - SnapshotHashes, Version, + CrdsData, CrdsValue, CrdsValueLabel, LegacyVersion, LowestSlot, SnapshotHashes, Version, }, legacy_contact_info::LegacyContactInfo, }, @@ -57,11 +56,6 @@ impl_crds_entry!(LegacyContactInfo, CrdsData::LegacyContactInfo(node), node); impl_crds_entry!(LegacyVersion, CrdsData::LegacyVersion(version), version); impl_crds_entry!(LowestSlot, CrdsData::LowestSlot(_, slot), slot); impl_crds_entry!(Version, CrdsData::Version(version), version); -impl_crds_entry!( - LegacySnapshotHashes, - CrdsData::LegacySnapshotHashes(snapshot_hashes), - snapshot_hashes -); impl_crds_entry!( SnapshotHashes, CrdsData::SnapshotHashes(snapshot_hashes), @@ -118,9 +112,6 @@ mod tests { CrdsData::LegacyVersion(version) => { assert_eq!(crds.get::<&LegacyVersion>(key), Some(version)) } - CrdsData::LegacySnapshotHashes(hash) => { - assert_eq!(crds.get::<&LegacySnapshotHashes>(key), Some(hash)) - } CrdsData::SnapshotHashes(hash) => { assert_eq!(crds.get::<&SnapshotHashes>(key), Some(hash)) } diff --git a/gossip/src/crds_value.rs b/gossip/src/crds_value.rs index 87ba34604e61d2..125555ea51eeb4 100644 --- a/gossip/src/crds_value.rs +++ b/gossip/src/crds_value.rs @@ -1,6 +1,6 @@ use { crate::{ - cluster_info::MAX_LEGACY_SNAPSHOT_HASHES, + cluster_info::MAX_ACCOUNTS_HASHES, contact_info::ContactInfo, deprecated, duplicate_shred::{DuplicateShred, DuplicateShredIndex, MAX_DUPLICATE_SHREDS}, @@ -85,7 +85,7 @@ pub enum CrdsData { LegacyContactInfo(LegacyContactInfo), Vote(VoteIndex, Vote), LowestSlot(/*DEPRECATED:*/ u8, LowestSlot), - LegacySnapshotHashes(LegacySnapshotHashes), + LegacySnapshotHashes(LegacySnapshotHashes), // Deprecated AccountsHashes(AccountsHashes), EpochSlots(EpochSlotsIndex, EpochSlots), LegacyVersion(LegacyVersion), @@ -195,7 +195,7 @@ impl AccountsHashes { /// New random AccountsHashes for tests and benchmarks. pub(crate) fn new_rand(rng: &mut R, pubkey: Option) -> Self { - let num_hashes = rng.gen_range(0..MAX_LEGACY_SNAPSHOT_HASHES) + 1; + let num_hashes = rng.gen_range(0..MAX_ACCOUNTS_HASHES) + 1; let hashes = std::iter::repeat_with(|| { let slot = 47825632 + rng.gen_range(0..512); let hash = Hash::new_unique(); @@ -211,7 +211,7 @@ impl AccountsHashes { } } -pub type LegacySnapshotHashes = AccountsHashes; +type LegacySnapshotHashes = AccountsHashes; #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, AbiExample)] pub struct SnapshotHashes {