From e8ea7220618f498a719d2e02ffe0285c8261b513 Mon Sep 17 00:00:00 2001 From: Brooks Date: Mon, 3 Apr 2023 16:42:21 -0400 Subject: [PATCH] Uses AccountsHashes type for AccountsHashes CrdsData variant (#31003) --- gossip/src/cluster_info.rs | 48 +++++++++++++++++++++++++++++++------- gossip/src/crds_value.rs | 16 +++++++------ rpc/src/rpc_service.rs | 8 +++---- 3 files changed, 52 insertions(+), 20 deletions(-) diff --git a/gossip/src/cluster_info.rs b/gossip/src/cluster_info.rs index c024b635ac..b6cbf036fd 100644 --- a/gossip/src/cluster_info.rs +++ b/gossip/src/cluster_info.rs @@ -32,8 +32,9 @@ use { CrdsFilter, CrdsTimeouts, ProcessPullStats, CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS, }, crds_value::{ - self, CrdsData, CrdsValue, CrdsValueLabel, EpochSlotsIndex, IncrementalSnapshotHashes, - LowestSlot, NodeInstance, SnapshotHashes, Version, Vote, MAX_WALLCLOCK, + self, AccountsHashes, CrdsData, CrdsValue, CrdsValueLabel, EpochSlotsIndex, + IncrementalSnapshotHashes, LowestSlot, NodeInstance, SnapshotHashes, Version, Vote, + MAX_WALLCLOCK, }, duplicate_shred::DuplicateShred, epoch_slots::EpochSlots, @@ -114,10 +115,13 @@ const MAX_GOSSIP_TRAFFIC: usize = 128_000_000 / PACKET_DATA_SIZE; /// message: Protocol::PushMessage(Pubkey::default(), Vec::default()) const PUSH_MESSAGE_MAX_PAYLOAD_SIZE: usize = PACKET_DATA_SIZE - 44; pub(crate) const DUPLICATE_SHRED_MAX_PAYLOAD_SIZE: usize = PACKET_DATA_SIZE - 115; -/// Maximum number of hashes in SnapshotHashes/AccountsHashes a node publishes +/// Maximum number of hashes in AccountsHashes a node publishes +/// 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 SnapshotHashes a node publishes /// such that the serialized size of the push/pull message stays below /// PACKET_DATA_SIZE. -// TODO: Update this to 26 once payload sizes are upgraded across fleet. pub const MAX_SNAPSHOT_HASHES: usize = 16; /// Maximum number of hashes in IncrementalSnapshotHashes a node publishes /// such that the serialized size of the push/pull message stays below @@ -262,14 +266,14 @@ pub fn make_accounts_hashes_message( keypair: &Keypair, accounts_hashes: Vec<(Slot, Hash)>, ) -> Option { - let message = CrdsData::AccountsHashes(SnapshotHashes::new(keypair.pubkey(), accounts_hashes)); + let message = CrdsData::AccountsHashes(AccountsHashes::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 = "avtjwK4ZjdAVfR8ZqqJkxbbe7SXSvhX5Cv7hnNjbZ7g")] +#[frozen_abi(digest = "5rGt5M3ujgfduA2dtN3rYg1CmvrGpZdRBq7KW4U58C8H")] #[derive(Serialize, Deserialize, Debug, AbiEnumVisitor, AbiExample)] #[allow(clippy::large_enum_variant)] pub(crate) enum Protocol { @@ -966,7 +970,7 @@ impl ClusterInfo { } pub fn push_accounts_hashes(&self, accounts_hashes: Vec<(Slot, Hash)>) { - if accounts_hashes.len() > MAX_SNAPSHOT_HASHES { + if accounts_hashes.len() > MAX_ACCOUNTS_HASHES { warn!( "accounts hashes too large, ignored: {}", accounts_hashes.len(), @@ -974,7 +978,7 @@ impl ClusterInfo { return; } - let message = CrdsData::AccountsHashes(SnapshotHashes::new(self.id(), accounts_hashes)); + let message = CrdsData::AccountsHashes(AccountsHashes::new(self.id(), accounts_hashes)); self.push_message(CrdsValue::new_signed(message, &self.keypair())); } @@ -3466,6 +3470,32 @@ RPC Enabled Nodes: 1"#; vec![entrypoint_crdsvalue] } + #[test] + fn test_max_accounts_hashes_with_push_messages() { + let mut rng = rand::thread_rng(); + for _ in 0..256 { + let accounts_hash = AccountsHashes::new_rand(&mut rng, None); + let crds_value = + CrdsValue::new_signed(CrdsData::AccountsHashes(accounts_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_accounts_hashes_with_pull_responses() { + let mut rng = rand::thread_rng(); + for _ in 0..256 { + let accounts_hash = AccountsHashes::new_rand(&mut rng, None); + let crds_value = + CrdsValue::new_signed(CrdsData::AccountsHashes(accounts_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(); @@ -3485,7 +3515,7 @@ RPC Enabled Nodes: 1"#; for _ in 0..256 { let snapshot_hash = SnapshotHashes::new_rand(&mut rng, None); let crds_value = - CrdsValue::new_signed(CrdsData::AccountsHashes(snapshot_hash), &Keypair::new()); + CrdsValue::new_signed(CrdsData::SnapshotHashes(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()); diff --git a/gossip/src/crds_value.rs b/gossip/src/crds_value.rs index a33f49afd2..e60b7ce21d 100644 --- a/gossip/src/crds_value.rs +++ b/gossip/src/crds_value.rs @@ -86,7 +86,7 @@ pub enum CrdsData { Vote(VoteIndex, Vote), LowestSlot(/*DEPRECATED:*/ u8, LowestSlot), SnapshotHashes(SnapshotHashes), - AccountsHashes(SnapshotHashes), + AccountsHashes(AccountsHashes), EpochSlots(EpochSlotsIndex, EpochSlots), LegacyVersion(LegacyVersion), Version(Version), @@ -154,7 +154,7 @@ impl CrdsData { // Index for LowestSlot is deprecated and should be zero. 1 => CrdsData::LowestSlot(0, LowestSlot::new_rand(rng, pubkey)), 2 => CrdsData::SnapshotHashes(SnapshotHashes::new_rand(rng, pubkey)), - 3 => CrdsData::AccountsHashes(SnapshotHashes::new_rand(rng, pubkey)), + 3 => CrdsData::AccountsHashes(AccountsHashes::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( @@ -166,13 +166,13 @@ impl CrdsData { } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, AbiExample)] -pub struct SnapshotHashes { +pub struct AccountsHashes { pub from: Pubkey, pub hashes: Vec<(Slot, Hash)>, pub wallclock: u64, } -impl Sanitize for SnapshotHashes { +impl Sanitize for AccountsHashes { fn sanitize(&self) -> Result<(), SanitizeError> { sanitize_wallclock(self.wallclock)?; for (slot, _) in &self.hashes { @@ -184,7 +184,7 @@ impl Sanitize for SnapshotHashes { } } -impl SnapshotHashes { +impl AccountsHashes { pub fn new(from: Pubkey, hashes: Vec<(Slot, Hash)>) -> Self { Self { from, @@ -193,7 +193,7 @@ impl SnapshotHashes { } } - /// New random SnapshotHashes for tests and benchmarks. + /// 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_SNAPSHOT_HASHES) + 1; let hashes = std::iter::repeat_with(|| { @@ -211,6 +211,8 @@ impl SnapshotHashes { } } +pub type SnapshotHashes = AccountsHashes; + #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, AbiExample)] pub struct IncrementalSnapshotHashes { pub from: Pubkey, @@ -633,7 +635,7 @@ impl CrdsValue { } } - pub(crate) fn accounts_hash(&self) -> Option<&SnapshotHashes> { + pub(crate) fn accounts_hash(&self) -> Option<&AccountsHashes> { match &self.data { CrdsData::AccountsHashes(slots) => Some(slots), _ => None, diff --git a/rpc/src/rpc_service.rs b/rpc/src/rpc_service.rs index b4c3bd79c4..111fd6143d 100644 --- a/rpc/src/rpc_service.rs +++ b/rpc/src/rpc_service.rs @@ -583,7 +583,7 @@ mod tests { crate::rpc::{create_validator_exit, tests::new_test_cluster_info}, solana_gossip::{ crds::GossipRoute, - crds_value::{CrdsData, CrdsValue, SnapshotHashes}, + crds_value::{AccountsHashes, CrdsData, CrdsValue}, }, solana_ledger::{ genesis_utils::{create_genesis_config, GenesisConfigInfo}, @@ -931,7 +931,7 @@ mod tests { .write() .unwrap() .insert( - CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHashes::new( + CrdsValue::new_unsigned(CrdsData::AccountsHashes(AccountsHashes::new( known_validators[0], vec![ (1, Hash::default()), @@ -952,7 +952,7 @@ mod tests { .write() .unwrap() .insert( - CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHashes::new( + CrdsValue::new_unsigned(CrdsData::AccountsHashes(AccountsHashes::new( known_validators[1], vec![(1000 + health_check_slot_distance - 1, Hash::default())], ))), @@ -969,7 +969,7 @@ mod tests { .write() .unwrap() .insert( - CrdsValue::new_unsigned(CrdsData::AccountsHashes(SnapshotHashes::new( + CrdsValue::new_unsigned(CrdsData::AccountsHashes(AccountsHashes::new( known_validators[2], vec![(1000 + health_check_slot_distance, Hash::default())], ))),