Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Rename CRDS SnapshotHash to SnapshotHashes #20421

Merged
merged 2 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions gossip/src/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use {
crds_gossip_pull::{CrdsFilter, ProcessPullStats, CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS},
crds_value::{
self, CrdsData, CrdsValue, CrdsValueLabel, EpochSlotsIndex, LowestSlot, NodeInstance,
SnapshotHash, Version, Vote, MAX_WALLCLOCK,
SnapshotHashes, Version, Vote, MAX_WALLCLOCK,
},
epoch_slots::EpochSlots,
gossip_error::GossipError,
Expand Down Expand Up @@ -247,14 +247,14 @@ pub fn make_accounts_hashes_message(
keypair: &Keypair,
accounts_hashes: Vec<(Slot, Hash)>,
) -> Option<CrdsValue> {
let message = CrdsData::AccountsHashes(SnapshotHash::new(keypair.pubkey(), accounts_hashes));
let message = CrdsData::AccountsHashes(SnapshotHashes::new(keypair.pubkey(), accounts_hashes));
Some(CrdsValue::new_signed(message, keypair))
}

pub(crate) type Ping = ping_pong::Ping<[u8; GOSSIP_PING_TOKEN_SIZE]>;

// TODO These messages should go through the gpu pipeline for spam filtering
#[frozen_abi(digest = "AqKhoLDkFr85WPiZnXG4bcRwHU4qSSyDZ3MQZLk3cnJf")]
#[frozen_abi(digest = "F2XvJ1e5qdQdtUcRhMNYAJFbMBJyFi3NRbfdfghzBajW")]
#[derive(Serialize, Deserialize, Debug, AbiEnumVisitor, AbiExample)]
#[allow(clippy::large_enum_variant)]
pub(crate) enum Protocol {
Expand Down Expand Up @@ -932,7 +932,7 @@ impl ClusterInfo {
return;
}

let message = CrdsData::AccountsHashes(SnapshotHash::new(self.id(), accounts_hashes));
let message = CrdsData::AccountsHashes(SnapshotHashes::new(self.id(), accounts_hashes));
self.push_message(CrdsValue::new_signed(message, &self.keypair()));
}

Expand All @@ -945,7 +945,7 @@ impl ClusterInfo {
return;
}

let message = CrdsData::SnapshotHashes(SnapshotHash::new(self.id(), snapshot_hashes));
let message = CrdsData::SnapshotHashes(SnapshotHashes::new(self.id(), snapshot_hashes));
self.push_message(CrdsValue::new_signed(message, &self.keypair()));
}

Expand Down Expand Up @@ -1111,7 +1111,7 @@ impl ClusterInfo {
F: FnOnce(&Vec<(Slot, Hash)>) -> Y,
{
let gossip_crds = self.gossip.crds.read().unwrap();
let hashes = &gossip_crds.get::<&SnapshotHash>(*pubkey)?.hashes;
let hashes = &gossip_crds.get::<&SnapshotHashes>(*pubkey)?.hashes;
Some(map(hashes))
}

Expand Down Expand Up @@ -3171,7 +3171,7 @@ mod tests {
fn test_max_snapshot_hashes_with_push_messages() {
let mut rng = rand::thread_rng();
for _ in 0..256 {
let snapshot_hash = SnapshotHash::new_rand(&mut rng, None);
let snapshot_hash = SnapshotHashes::new_rand(&mut rng, None);
let crds_value =
CrdsValue::new_signed(CrdsData::SnapshotHashes(snapshot_hash), &Keypair::new());
let message = Protocol::PushMessage(Pubkey::new_unique(), vec![crds_value]);
Expand All @@ -3184,7 +3184,7 @@ mod tests {
fn test_max_snapshot_hashes_with_pull_responses() {
let mut rng = rand::thread_rng();
for _ in 0..256 {
let snapshot_hash = SnapshotHash::new_rand(&mut rng, None);
let snapshot_hash = SnapshotHashes::new_rand(&mut rng, None);
let crds_value =
CrdsValue::new_signed(CrdsData::AccountsHashes(snapshot_hash), &Keypair::new());
let response = Protocol::PullResponse(Pubkey::new_unique(), vec![crds_value]);
Expand Down Expand Up @@ -3827,15 +3827,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::SnapshotHashes(SnapshotHash {
let mut value = CrdsValue::new_unsigned(CrdsData::SnapshotHashes(SnapshotHashes {
from: Pubkey::default(),
hashes: vec![],
wallclock: 0,
}));

let mut i = 0;
while value.size() < PUSH_MESSAGE_MAX_PAYLOAD_SIZE as u64 {
value.data = CrdsData::SnapshotHashes(SnapshotHash {
value.data = CrdsData::SnapshotHashes(SnapshotHashes {
from: Pubkey::default(),
hashes: vec![(0, Hash::default()); i],
wallclock: 0,
Expand Down
4 changes: 2 additions & 2 deletions gossip/src/crds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ mod tests {
super::*,
crate::{
contact_info::ContactInfo,
crds_value::{new_rand_timestamp, NodeInstance, SnapshotHash},
crds_value::{new_rand_timestamp, NodeInstance, SnapshotHashes},
},
rand::{thread_rng, Rng, SeedableRng},
rand_chacha::ChaChaRng,
Expand Down Expand Up @@ -1061,7 +1061,7 @@ mod tests {
assert_eq!(crds.insert(node, timestamp()), Ok(()));
assert_eq!(crds.get_shred_version(&pubkey), Some(8));
// Add other crds values with the same pubkey.
let val = SnapshotHash::new_rand(&mut rng, Some(pubkey));
let val = SnapshotHashes::new_rand(&mut rng, Some(pubkey));
let val = CrdsData::SnapshotHashes(val);
let val = CrdsValue::new_unsigned(val);
assert_eq!(crds.insert(val, timestamp()), Ok(()));
Expand Down
8 changes: 4 additions & 4 deletions gossip/src/crds_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use {
contact_info::ContactInfo,
crds::VersionedCrdsValue,
crds_value::{
CrdsData, CrdsValue, CrdsValueLabel, LegacyVersion, LowestSlot, SnapshotHash, Version,
CrdsData, CrdsValue, CrdsValueLabel, LegacyVersion, LowestSlot, SnapshotHashes, Version,
},
},
indexmap::IndexMap,
Expand All @@ -14,7 +14,7 @@ type CrdsTable = IndexMap<CrdsValueLabel, VersionedCrdsValue>;

/// Represents types which can be looked up from crds table given a key. e.g.
/// CrdsValueLabel -> VersionedCrdsValue, CrdsValue, CrdsData
/// Pubkey -> ContactInfo, LowestSlot, SnapshotHash, ...
/// Pubkey -> ContactInfo, LowestSlot, SnapshotHashes, ...
pub trait CrdsEntry<'a, 'b>: Sized {
type Key; // Lookup key.
fn get_entry(table: &'a CrdsTable, key: Self::Key) -> Option<Self>;
Expand Down Expand Up @@ -57,7 +57,7 @@ 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<'a, 'b> CrdsEntry<'a, 'b> for &'a SnapshotHash {
impl<'a, 'b> CrdsEntry<'a, 'b> for &'a SnapshotHashes {
type Key = Pubkey;
fn get_entry(table: &'a CrdsTable, key: Self::Key) -> Option<Self> {
let key = CrdsValueLabel::SnapshotHashes(key);
Expand Down Expand Up @@ -112,7 +112,7 @@ mod tests {
assert_eq!(crds.get::<&LegacyVersion>(key), Some(version))
}
CrdsData::SnapshotHashes(hash) => {
assert_eq!(crds.get::<&SnapshotHash>(key), Some(hash))
assert_eq!(crds.get::<&SnapshotHashes>(key), Some(hash))
}
_ => (),
}
Expand Down
20 changes: 10 additions & 10 deletions gossip/src/crds_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ pub enum CrdsData {
ContactInfo(ContactInfo),
Vote(VoteIndex, Vote),
LowestSlot(/*DEPRECATED:*/ u8, LowestSlot),
SnapshotHashes(SnapshotHash),
AccountsHashes(SnapshotHash),
SnapshotHashes(SnapshotHashes),
AccountsHashes(SnapshotHashes),
Comment on lines +87 to +88
Copy link
Contributor Author

@brooksprumo brooksprumo Oct 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this PR comment (#20374 (comment)), @behzadnouri mentioned that following the naming scheme of (old) SnapshotHashes wasn't the right call. So here's the update to make this better.

EpochSlots(EpochSlotsIndex, EpochSlots),
LegacyVersion(LegacyVersion),
Version(Version),
Expand Down Expand Up @@ -147,8 +147,8 @@ impl CrdsData {
match kind {
0 => CrdsData::ContactInfo(ContactInfo::new_rand(rng, pubkey)),
1 => CrdsData::LowestSlot(rng.gen(), LowestSlot::new_rand(rng, pubkey)),
2 => CrdsData::SnapshotHashes(SnapshotHash::new_rand(rng, pubkey)),
3 => CrdsData::AccountsHashes(SnapshotHash::new_rand(rng, pubkey)),
2 => CrdsData::SnapshotHashes(SnapshotHashes::new_rand(rng, pubkey)),
3 => CrdsData::AccountsHashes(SnapshotHashes::new_rand(rng, pubkey)),
4 => CrdsData::Version(Version::new_rand(rng, pubkey)),
5 => CrdsData::Vote(rng.gen_range(0, MAX_VOTES), Vote::new_rand(rng, pubkey)),
_ => CrdsData::EpochSlots(
Expand All @@ -160,13 +160,13 @@ impl CrdsData {
}

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, AbiExample)]
pub struct SnapshotHash {
pub struct SnapshotHashes {
pub from: Pubkey,
pub hashes: Vec<(Slot, Hash)>,
pub wallclock: u64,
}
Comment on lines +163 to 167
Copy link
Contributor Author

@brooksprumo brooksprumo Oct 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here's the actual struct. The singular "SnapshotHash" was a little strange to me, since there's a vector of hashes inside...


impl Sanitize for SnapshotHash {
impl Sanitize for SnapshotHashes {
fn sanitize(&self) -> Result<(), SanitizeError> {
sanitize_wallclock(self.wallclock)?;
for (slot, _) in &self.hashes {
Expand All @@ -178,7 +178,7 @@ impl Sanitize for SnapshotHash {
}
}

impl SnapshotHash {
impl SnapshotHashes {
pub fn new(from: Pubkey, hashes: Vec<(Slot, Hash)>) -> Self {
Self {
from,
Expand All @@ -187,7 +187,7 @@ impl SnapshotHash {
}
}

/// New random SnapshotHash for tests and benchmarks.
/// New random SnapshotHashes 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_SNAPSHOT_HASHES) + 1;
let hashes = std::iter::repeat_with(|| {
Expand Down Expand Up @@ -475,7 +475,7 @@ impl fmt::Display for CrdsValueLabel {
CrdsValueLabel::ContactInfo(_) => write!(f, "ContactInfo({})", self.pubkey()),
CrdsValueLabel::Vote(ix, _) => write!(f, "Vote({}, {})", ix, self.pubkey()),
CrdsValueLabel::LowestSlot(_) => write!(f, "LowestSlot({})", self.pubkey()),
CrdsValueLabel::SnapshotHashes(_) => write!(f, "SnapshotHash({})", self.pubkey()),
CrdsValueLabel::SnapshotHashes(_) => write!(f, "SnapshotHashes({})", self.pubkey()),
CrdsValueLabel::EpochSlots(ix, _) => write!(f, "EpochSlots({}, {})", ix, self.pubkey()),
CrdsValueLabel::AccountsHashes(_) => write!(f, "AccountsHashes({})", self.pubkey()),
CrdsValueLabel::LegacyVersion(_) => write!(f, "LegacyVersion({})", self.pubkey()),
Expand Down Expand Up @@ -584,7 +584,7 @@ impl CrdsValue {
}
}

pub(crate) fn accounts_hash(&self) -> Option<&SnapshotHash> {
pub(crate) fn accounts_hash(&self) -> Option<&SnapshotHashes> {
match &self.data {
CrdsData::AccountsHashes(slots) => Some(slots),
_ => None,
Expand Down
8 changes: 4 additions & 4 deletions rpc/src/rpc_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ mod tests {
crate::rpc::create_validator_exit,
solana_gossip::{
contact_info::ContactInfo,
crds_value::{CrdsData, CrdsValue, SnapshotHash},
crds_value::{CrdsData, CrdsValue, SnapshotHashes},
},
solana_ledger::{
genesis_utils::{create_genesis_config, GenesisConfigInfo},
Expand Down Expand Up @@ -784,7 +784,7 @@ mod tests {
.write()
.unwrap()
.insert(
CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHash::new(
CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHashes::new(
trusted_validators[0],
vec![
(1, Hash::default()),
Expand All @@ -804,7 +804,7 @@ mod tests {
.write()
.unwrap()
.insert(
CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHash::new(
CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHashes::new(
trusted_validators[1],
vec![(1000 + health_check_slot_distance - 1, Hash::default())],
))),
Expand All @@ -820,7 +820,7 @@ mod tests {
.write()
.unwrap()
.insert(
CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHash::new(
CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHashes::new(
trusted_validators[2],
vec![(1000 + health_check_slot_distance, Hash::default())],
))),
Expand Down