Skip to content

Commit

Permalink
removes unused legacy-snapshot-hashes api in gossip (solana-labs#33593)
Browse files Browse the repository at this point in the history
solana-labs#33576
stops broadcasting legacy snapshot hashes over gossip, and this commit
removes unused legacy snapshot hashed code in gossip.
  • Loading branch information
behzadnouri authored Oct 9, 2023
1 parent 72574da commit 1d91b60
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 85 deletions.
75 changes: 8 additions & 67 deletions gossip/src/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -1208,15 +1189,6 @@ impl ClusterInfo {
.map(map)
}

pub fn get_legacy_snapshot_hash_for_node<F, Y>(&self, pubkey: &Pubkey, map: F) -> Option<Y>
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<SnapshotHashes> {
self.gossip
.crds
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions gossip/src/crds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -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);
}
Expand Down
11 changes: 1 addition & 10 deletions gossip/src/crds_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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))
}
Expand Down
8 changes: 4 additions & 4 deletions gossip/src/crds_value.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -195,7 +195,7 @@ impl AccountsHashes {

/// New random AccountsHashes for tests and benchmarks.
pub(crate) fn new_rand<R: Rng>(rng: &mut R, pubkey: Option<Pubkey>) -> 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();
Expand All @@ -211,7 +211,7 @@ impl AccountsHashes {
}
}

pub type LegacySnapshotHashes = AccountsHashes;
type LegacySnapshotHashes = AccountsHashes;

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, AbiExample)]
pub struct SnapshotHashes {
Expand Down

0 comments on commit 1d91b60

Please sign in to comment.