From 453f2726981896983c9e41c00aae49ac125a7046 Mon Sep 17 00:00:00 2001 From: Brooks Date: Tue, 11 Apr 2023 10:30:29 -0400 Subject: [PATCH] Rename IncrementalSnapshotHashes to SnapshotHashes (#31136) --- core/src/snapshot_packager_service.rs | 6 +-- gossip/src/cluster_info.rs | 59 ++++++++++++--------------- gossip/src/crds.rs | 2 +- gossip/src/crds_entry.rs | 14 +++---- gossip/src/crds_value.rs | 34 ++++++++------- validator/src/bootstrap.rs | 26 ++++++------ 6 files changed, 66 insertions(+), 75 deletions(-) diff --git a/core/src/snapshot_packager_service.rs b/core/src/snapshot_packager_service.rs index 5a02463a64..c097c6a838 100644 --- a/core/src/snapshot_packager_service.rs +++ b/core/src/snapshot_packager_service.rs @@ -310,15 +310,15 @@ impl SnapshotGossipManager { // Pushing incremental snapshot hashes to the cluster should never fail. The only error // case is when the length of the hashes is too big, but we account for that with // `max_incremental_snapshot_hashes`. If this call ever does error, it's a programmer bug! - // Check to see what changed in `push_incremental_snapshot_hashes()` and handle the new + // Check to see what changed in `push_snapshot_hashes()` and handle the new // error condition here. self.cluster_info - .push_incremental_snapshot_hashes( + .push_snapshot_hashes( Self::clone_hash_for_crds(&self.incremental_snapshot_hashes.base), Self::clone_hashes_for_crds(&self.incremental_snapshot_hashes.hashes), ) .expect( - "Bug! The programmer contract has changed for push_incremental_snapshot_hashes() \ + "Bug! The programmer contract has changed for push_snapshot_hashes() \ and a new error case has been added, which has not been handled here.", ); } diff --git a/gossip/src/cluster_info.rs b/gossip/src/cluster_info.rs index a7534357fd..03f2ff5882 100644 --- a/gossip/src/cluster_info.rs +++ b/gossip/src/cluster_info.rs @@ -33,8 +33,8 @@ use { }, crds_value::{ self, AccountsHashes, CrdsData, CrdsValue, CrdsValueLabel, EpochSlotsIndex, - IncrementalSnapshotHashes, LegacySnapshotHashes, LowestSlot, NodeInstance, Version, - Vote, MAX_WALLCLOCK, + LegacySnapshotHashes, LowestSlot, NodeInstance, SnapshotHashes, Version, Vote, + MAX_WALLCLOCK, }, duplicate_shred::DuplicateShred, epoch_slots::EpochSlots, @@ -123,7 +123,7 @@ pub const MAX_ACCOUNTS_HASHES: usize = 16; /// 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 hashes in IncrementalSnapshotHashes a node publishes +/// 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. pub const MAX_INCREMENTAL_SNAPSHOT_HASHES: usize = 25; @@ -273,7 +273,7 @@ pub fn make_accounts_hashes_message( 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 = "9pQAWSpV411icPZjDnBcTJKjLyzYPVFWgYxx3bqWQbtg")] +#[frozen_abi(digest = "FsZnSeTYNH7F51AxTaKUixXxjT6if2ThmPN1mhDWtXZM")] #[derive(Serialize, Deserialize, Debug, AbiEnumVisitor, AbiExample)] #[allow(clippy::large_enum_variant)] pub(crate) enum Protocol { @@ -390,7 +390,7 @@ fn retain_staked(values: &mut Vec, stakes: &HashMap) { // Unstaked nodes can still help repair. CrdsData::EpochSlots(_, _) => true, // Unstaked nodes can still serve snapshots. - CrdsData::LegacySnapshotHashes(_) | CrdsData::IncrementalSnapshotHashes(_) => true, + CrdsData::LegacySnapshotHashes(_) | CrdsData::SnapshotHashes(_) => true, // Otherwise unstaked voting nodes will show up with no version in // the various dashboards. CrdsData::Version(_) => true, @@ -996,19 +996,19 @@ impl ClusterInfo { self.push_message(CrdsValue::new_signed(message, &self.keypair())); } - pub fn push_incremental_snapshot_hashes( + pub fn push_snapshot_hashes( &self, - base: (Slot, Hash), - hashes: Vec<(Slot, Hash)>, + full: (Slot, Hash), + incremental: Vec<(Slot, Hash)>, ) -> Result<(), ClusterInfoError> { - if hashes.len() > MAX_INCREMENTAL_SNAPSHOT_HASHES { + if incremental.len() > MAX_INCREMENTAL_SNAPSHOT_HASHES { return Err(ClusterInfoError::TooManyIncrementalSnapshotHashes); } - let message = CrdsData::IncrementalSnapshotHashes(IncrementalSnapshotHashes { + let message = CrdsData::SnapshotHashes(SnapshotHashes { from: self.id(), - base, - hashes, + full, + incremental, wallclock: timestamp(), }); self.push_message(CrdsValue::new_signed(message, &self.keypair())); @@ -1204,15 +1204,12 @@ impl ClusterInfo { Some(map(hashes)) } - pub fn get_incremental_snapshot_hashes_for_node( - &self, - pubkey: &Pubkey, - ) -> Option { + pub fn get_snapshot_hashes_for_node(&self, pubkey: &Pubkey) -> Option { self.gossip .crds .read() .unwrap() - .get::<&IncrementalSnapshotHashes>(*pubkey) + .get::<&SnapshotHashes>(*pubkey) .cloned() } @@ -3528,36 +3525,32 @@ RPC Enabled Nodes: 1"#; } #[test] - fn test_max_incremental_snapshot_hashes_with_push_messages() { + fn test_max_snapshot_hashes_with_push_messages() { let mut rng = rand::thread_rng(); - let incremental_snapshot_hashes = IncrementalSnapshotHashes { + let snapshot_hashes = SnapshotHashes { from: Pubkey::new_unique(), - base: (Slot::default(), Hash::default()), - hashes: vec![(Slot::default(), Hash::default()); MAX_INCREMENTAL_SNAPSHOT_HASHES], + full: (Slot::default(), Hash::default()), + incremental: vec![(Slot::default(), Hash::default()); MAX_INCREMENTAL_SNAPSHOT_HASHES], wallclock: timestamp(), }; - let crds_value = CrdsValue::new_signed( - CrdsData::IncrementalSnapshotHashes(incremental_snapshot_hashes), - &Keypair::new(), - ); + let crds_value = + CrdsValue::new_signed(CrdsData::SnapshotHashes(snapshot_hashes), &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_incremental_snapshot_hashes_with_pull_responses() { + fn test_max_snapshot_hashes_with_pull_responses() { let mut rng = rand::thread_rng(); - let incremental_snapshot_hashes = IncrementalSnapshotHashes { + let snapshot_hashes = SnapshotHashes { from: Pubkey::new_unique(), - base: (Slot::default(), Hash::default()), - hashes: vec![(Slot::default(), Hash::default()); MAX_INCREMENTAL_SNAPSHOT_HASHES], + full: (Slot::default(), Hash::default()), + incremental: vec![(Slot::default(), Hash::default()); MAX_INCREMENTAL_SNAPSHOT_HASHES], wallclock: timestamp(), }; - let crds_value = CrdsValue::new_signed( - CrdsData::IncrementalSnapshotHashes(incremental_snapshot_hashes), - &Keypair::new(), - ); + let crds_value = + CrdsValue::new_signed(CrdsData::SnapshotHashes(snapshot_hashes), &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()); diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 04b9b8504e..3c8c433f8a 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -688,7 +688,7 @@ impl CrdsDataStats { CrdsData::Version(_) => 7, CrdsData::NodeInstance(_) => 8, CrdsData::DuplicateShred(_, _) => 9, - CrdsData::IncrementalSnapshotHashes(_) => 10, + CrdsData::SnapshotHashes(_) => 10, CrdsData::ContactInfo(_) => 11, // Update CrdsCountsArray if new items are added here. } diff --git a/gossip/src/crds_entry.rs b/gossip/src/crds_entry.rs index 4bd3f7091d..ccb8ed310e 100644 --- a/gossip/src/crds_entry.rs +++ b/gossip/src/crds_entry.rs @@ -2,8 +2,8 @@ use { crate::{ crds::VersionedCrdsValue, crds_value::{ - CrdsData, CrdsValue, CrdsValueLabel, IncrementalSnapshotHashes, LegacySnapshotHashes, - LegacyVersion, LowestSlot, Version, + CrdsData, CrdsValue, CrdsValueLabel, LegacySnapshotHashes, LegacyVersion, LowestSlot, + SnapshotHashes, Version, }, legacy_contact_info::LegacyContactInfo, }, @@ -63,9 +63,9 @@ impl_crds_entry!( snapshot_hashes ); impl_crds_entry!( - IncrementalSnapshotHashes, - CrdsData::IncrementalSnapshotHashes(incremental_snapshot_hashes), - incremental_snapshot_hashes + SnapshotHashes, + CrdsData::SnapshotHashes(snapshot_hashes), + snapshot_hashes ); #[cfg(test)] @@ -121,8 +121,8 @@ mod tests { CrdsData::LegacySnapshotHashes(hash) => { assert_eq!(crds.get::<&LegacySnapshotHashes>(key), Some(hash)) } - CrdsData::IncrementalSnapshotHashes(hash) => { - assert_eq!(crds.get::<&IncrementalSnapshotHashes>(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 148fd4347f..e910fda68c 100644 --- a/gossip/src/crds_value.rs +++ b/gossip/src/crds_value.rs @@ -92,7 +92,7 @@ pub enum CrdsData { Version(Version), NodeInstance(NodeInstance), DuplicateShred(DuplicateShredIndex, DuplicateShred), - IncrementalSnapshotHashes(IncrementalSnapshotHashes), + SnapshotHashes(SnapshotHashes), ContactInfo(ContactInfo), } @@ -130,7 +130,7 @@ impl Sanitize for CrdsData { shred.sanitize() } } - CrdsData::IncrementalSnapshotHashes(val) => val.sanitize(), + CrdsData::SnapshotHashes(val) => val.sanitize(), CrdsData::ContactInfo(node) => node.sanitize(), } } @@ -214,24 +214,24 @@ impl AccountsHashes { pub type LegacySnapshotHashes = AccountsHashes; #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, AbiExample)] -pub struct IncrementalSnapshotHashes { +pub struct SnapshotHashes { pub from: Pubkey, - pub base: (Slot, Hash), - pub hashes: Vec<(Slot, Hash)>, + pub full: (Slot, Hash), + pub incremental: Vec<(Slot, Hash)>, pub wallclock: u64, } -impl Sanitize for IncrementalSnapshotHashes { +impl Sanitize for SnapshotHashes { fn sanitize(&self) -> Result<(), SanitizeError> { sanitize_wallclock(self.wallclock)?; - if self.base.0 >= MAX_SLOT { + if self.full.0 >= MAX_SLOT { return Err(SanitizeError::ValueOutOfBounds); } - for (slot, _) in &self.hashes { + for (slot, _) in &self.incremental { if *slot >= MAX_SLOT { return Err(SanitizeError::ValueOutOfBounds); } - if self.base.0 >= *slot { + if self.full.0 >= *slot { return Err(SanitizeError::InvalidValue); } } @@ -500,7 +500,7 @@ pub enum CrdsValueLabel { Version(Pubkey), NodeInstance(Pubkey), DuplicateShred(DuplicateShredIndex, Pubkey), - IncrementalSnapshotHashes(Pubkey), + SnapshotHashes(Pubkey), ContactInfo(Pubkey), } @@ -521,8 +521,8 @@ impl fmt::Display for CrdsValueLabel { CrdsValueLabel::Version(_) => write!(f, "Version({})", self.pubkey()), CrdsValueLabel::NodeInstance(pk) => write!(f, "NodeInstance({pk})"), CrdsValueLabel::DuplicateShred(ix, pk) => write!(f, "DuplicateShred({ix}, {pk})"), - CrdsValueLabel::IncrementalSnapshotHashes(_) => { - write!(f, "IncrementalSnapshotHashes({})", self.pubkey()) + CrdsValueLabel::SnapshotHashes(_) => { + write!(f, "SnapshotHashes({})", self.pubkey()) } CrdsValueLabel::ContactInfo(_) => write!(f, "ContactInfo({})", self.pubkey()), } @@ -542,7 +542,7 @@ impl CrdsValueLabel { CrdsValueLabel::Version(p) => *p, CrdsValueLabel::NodeInstance(p) => *p, CrdsValueLabel::DuplicateShred(_, p) => *p, - CrdsValueLabel::IncrementalSnapshotHashes(p) => *p, + CrdsValueLabel::SnapshotHashes(p) => *p, CrdsValueLabel::ContactInfo(pubkey) => *pubkey, } } @@ -592,7 +592,7 @@ impl CrdsValue { CrdsData::Version(version) => version.wallclock, CrdsData::NodeInstance(node) => node.wallclock, CrdsData::DuplicateShred(_, shred) => shred.wallclock, - CrdsData::IncrementalSnapshotHashes(hash) => hash.wallclock, + CrdsData::SnapshotHashes(hash) => hash.wallclock, CrdsData::ContactInfo(node) => node.wallclock(), } } @@ -608,7 +608,7 @@ impl CrdsValue { CrdsData::Version(version) => version.from, CrdsData::NodeInstance(node) => node.from, CrdsData::DuplicateShred(_, shred) => shred.from, - CrdsData::IncrementalSnapshotHashes(hash) => hash.from, + CrdsData::SnapshotHashes(hash) => hash.from, CrdsData::ContactInfo(node) => *node.pubkey(), } } @@ -626,9 +626,7 @@ impl CrdsValue { CrdsData::Version(_) => CrdsValueLabel::Version(self.pubkey()), CrdsData::NodeInstance(node) => CrdsValueLabel::NodeInstance(node.from), CrdsData::DuplicateShred(ix, shred) => CrdsValueLabel::DuplicateShred(*ix, shred.from), - CrdsData::IncrementalSnapshotHashes(_) => { - CrdsValueLabel::IncrementalSnapshotHashes(self.pubkey()) - } + CrdsData::SnapshotHashes(_) => CrdsValueLabel::SnapshotHashes(self.pubkey()), CrdsData::ContactInfo(node) => CrdsValueLabel::ContactInfo(*node.pubkey()), } } diff --git a/validator/src/bootstrap.rs b/validator/src/bootstrap.rs index 34c74bd2f2..adfcc71a43 100644 --- a/validator/src/bootstrap.rs +++ b/validator/src/bootstrap.rs @@ -864,8 +864,8 @@ fn get_snapshot_hashes_from_known_validators( // Get the incremental snapshot hashes for a node from CRDS let get_incremental_snapshot_hashes_for_node = |node| { cluster_info - .get_incremental_snapshot_hashes_for_node(node) - .map(|hashes| (hashes.base, hashes.hashes)) + .get_snapshot_hashes_for_node(node) + .map(|hashes| (hashes.full, hashes.incremental)) }; if !do_known_validators_have_all_snapshot_hashes( @@ -1446,17 +1446,17 @@ fn get_highest_incremental_snapshot_hash_for_peer( cluster_info: &ClusterInfo, peer: &Pubkey, ) -> Option { - cluster_info - .get_incremental_snapshot_hashes_for_node(peer) - .map( - |crds_value::IncrementalSnapshotHashes { base, hashes, .. }| { - let highest_incremental_snapshot_hash = hashes.into_iter().max(); - SnapshotHash { - full: base, - incr: highest_incremental_snapshot_hash, - } - }, - ) + cluster_info.get_snapshot_hashes_for_node(peer).map( + |crds_value::SnapshotHashes { + full, incremental, .. + }| { + let highest_incremental_snapshot_hash = incremental.into_iter().max(); + SnapshotHash { + full, + incr: highest_incremental_snapshot_hash, + } + }, + ) } #[cfg(test)]