Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename CRDS SnapshotHash to SnapshotHashes #20421

Merged
merged 2 commits into from
Oct 5, 2021
Merged

Rename CRDS SnapshotHash to SnapshotHashes #20421

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ca2b74
Rename CRDS SnapshotHash to SnapshotHashes
brooksprumo Oct 4, 2021
804c9e8
fix: update frozen abi digest
brooksprumo Oct 4, 2021
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
18 changes: 9 additions & 9 deletions gossip/src/cluster_info.rs
View file
Open in desktop
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,7 +247,7 @@ 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))
}

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 •
edited
Loading

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 •
edited
Loading

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
View file
Open in desktop
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