From 84ac26926cb55d62b5f30aeb956618ac17f03419 Mon Sep 17 00:00:00 2001 From: greg Date: Tue, 11 Jul 2023 16:30:08 -0700 Subject: [PATCH 01/26] screwed up old branch and syncing with upstream branch --- gossip/src/cluster_info_metrics.rs | 33 +++++++++++++++++++++++++++--- gossip/src/crds.rs | 13 ++++++++++++ sdk/src/signature.rs | 13 ++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index 8765cd158cd469..a89682fa4f9c96 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -2,10 +2,14 @@ use { crate::crds_gossip::CrdsGossip, itertools::Itertools, solana_measure::measure::Measure, - solana_sdk::{clock::Slot, pubkey::Pubkey}, + solana_sdk::{ + clock::Slot, + pubkey::Pubkey, + signature::Signature + }, std::{ cmp::Reverse, - collections::HashMap, + collections::{HashMap, VecDeque}, ops::{Deref, DerefMut}, sync::atomic::{AtomicU64, Ordering}, time::Instant, @@ -189,7 +193,7 @@ pub(crate) fn submit_gossip_stats( gossip: &CrdsGossip, stakes: &HashMap, ) { - let (crds_stats, table_size, num_nodes, num_pubkeys, purged_values_size, failed_inserts_size) = { + let (mut crds_stats, table_size, num_nodes, num_pubkeys, purged_values_size, failed_inserts_size) = { let gossip_crds = gossip.crds.read().unwrap(); ( gossip_crds.take_stats(), @@ -675,6 +679,8 @@ pub(crate) fn submit_gossip_stats( ("all-push", crds_stats.push.fails.iter().sum::(), i64), ("all-pull", crds_stats.pull.fails.iter().sum::(), i64), ); + + submit_message_signature_stats("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.message_signatures); if !log::log_enabled!(log::Level::Trace) { return; } @@ -704,3 +710,24 @@ where datapoint_trace!(name, ("slot", slot, i64), ("num_votes", num_votes, i64)); } } + +// greg +fn submit_message_signature_stats<'a>( + name: &'static str, + message_signatures: &mut VecDeque +) { + // we want to submit all message signatures we have here. + // Need to pop them to remove them + // NOTE: we need to filter the signatures before we call this + // so only signatures with ending 0xFF or whatever end up here. + while !message_signatures.is_empty() { + match message_signatures.pop_front() { + Some(signature) => { + datapoint_info!(name, ("crds_signature", signature.to_string(), String)); + }, + None => { + error!("Error reporting submitting Crds signature. Invalid read from message signature queue"); + } + } + } +} diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index a4ab600e167401..642283cc222117 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -44,6 +44,7 @@ use { clock::Slot, hash::{hash, Hash}, pubkey::Pubkey, + signature::Signature, }, std::{ cmp::Ordering, @@ -101,6 +102,7 @@ pub(crate) struct CrdsDataStats { pub(crate) counts: CrdsCountsArray, pub(crate) fails: CrdsCountsArray, pub(crate) votes: LruCache, + pub(crate) message_signatures: VecDeque, //TODO: change to fixed size } #[derive(Default)] @@ -656,6 +658,7 @@ impl Default for CrdsDataStats { counts: CrdsCountsArray::default(), fails: CrdsCountsArray::default(), votes: LruCache::new(VOTE_SLOTS_METRICS_CAP), + message_signatures: VecDeque::with_capacity(1024), } } } @@ -669,6 +672,16 @@ impl CrdsDataStats { self.votes.put(slot, num_nodes + 1); } } + let specific_ending: [u8; 1] = [01]; + // let specific_ending: [u8; 2] = [57, 01]; + if entry.value.signature.check_ending_characters(&specific_ending) { + info!("gregg signature ends in a: {:?}", entry.value.signature); + self.message_signatures.push_back(entry.value.signature); + } + // else { + // info!("gregg signature does not end in a: {:?}", entry.value.signature); + // } + // self.message_signatures.push_back(entry.value.signature); } fn record_fail(&mut self, entry: &VersionedCrdsValue) { diff --git a/sdk/src/signature.rs b/sdk/src/signature.rs index a3f129ff9e620f..acc16b00c02a15 100644 --- a/sdk/src/signature.rs +++ b/sdk/src/signature.rs @@ -51,6 +51,19 @@ impl Signature { pub fn verify(&self, pubkey_bytes: &[u8], message_bytes: &[u8]) -> bool { self.verify_verbose(pubkey_bytes, message_bytes).is_ok() } + + pub fn check_ending_characters( + &self, + specific_ending: &[u8], + ) -> bool { + let signature_bytes: &[u8] = self.0.as_slice(); + if signature_bytes.len() < specific_ending.len() { + return false; + } + // info!("cmp bytes: {:?}, signature bytes: {:?}", specific_ending, signature_bytes); + let start_index = signature_bytes.len() - specific_ending.len(); + &signature_bytes[start_index..] == specific_ending + } } pub trait Signable { From c23e5c00e09a6e96fb4eb1587acb0e3a8fc60cc2 Mon Sep 17 00:00:00 2001 From: greg Date: Wed, 12 Jul 2023 12:42:18 -0700 Subject: [PATCH 02/26] add fixed size ring buff instead of variable sized vecdeque for reporting signatures --- Cargo.lock | 10 ++++++++++ Cargo.toml | 1 + gossip/Cargo.toml | 1 + gossip/src/cluster_info_metrics.rs | 17 ++++++++++++++++- gossip/src/crds.rs | 11 +++++++++++ 5 files changed, 39 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index afb3fdf76bdbb9..66e532d375713b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4435,6 +4435,15 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "ringbuf" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79abed428d1fd2a128201cec72c5f6938e2da607c6f3745f769fabea399d950a" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "rocksdb" version = "0.21.0" @@ -5994,6 +6003,7 @@ dependencies = [ "rand_chacha 0.2.2", "rayon", "regex", + "ringbuf", "rustc_version 0.4.0", "serde", "serde_bytes", diff --git a/Cargo.toml b/Cargo.toml index 0f83787bf4d05f..29e083c748aa35 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -405,6 +405,7 @@ winreg = "0.10" x509-parser = "0.14.0" zeroize = { version = "1.3", default-features = false } zstd = "0.11.2" +ringbuf = "0.3.3" [patch.crates-io] # for details, see https://github.com/solana-labs/crossbeam/commit/fd279d707025f0e60951e429bf778b4813d1b6bf diff --git a/gossip/Cargo.toml b/gossip/Cargo.toml index 60ecf66d4480f2..5c23ae3a460fd7 100644 --- a/gossip/Cargo.toml +++ b/gossip/Cargo.toml @@ -49,6 +49,7 @@ solana-version = { workspace = true } solana-vote-program = { workspace = true } static_assertions = { workspace = true } thiserror = { workspace = true } +ringbuf = { workspace = true } [dev-dependencies] num_cpus = { workspace = true } diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index a89682fa4f9c96..bb07613907f669 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -14,6 +14,7 @@ use { sync::atomic::{AtomicU64, Ordering}, time::Instant, }, + ringbuf::{HeapRb, Rb} }; #[derive(Default)] @@ -680,7 +681,8 @@ pub(crate) fn submit_gossip_stats( ("all-pull", crds_stats.pull.fails.iter().sum::(), i64), ); - submit_message_signature_stats("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.message_signatures); + // submit_message_signature_stats("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.message_signatures); + submit_message_signature_stats_2("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.message_signatures_2); if !log::log_enabled!(log::Level::Trace) { return; } @@ -731,3 +733,16 @@ fn submit_message_signature_stats<'a>( } } } + +fn submit_message_signature_stats_2<'a>( + name: &'static str, + message_signatures: &mut HeapRb, +) { + // we want to submit all message signatures we have here. + // Need to pop them to remove them + // NOTE: we need to filter the signatures before we call this + // so only signatures with ending 0xFF or whatever end up here. + while let Some(signature) = message_signatures.pop() { + datapoint_info!(name, ("crds_signature", signature.to_string(), String)); + } +} diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 642283cc222117..72ae25a3213b5f 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -24,6 +24,8 @@ //! A value is updated to a new version if the labels match, and the value //! wallclock is later, or the value hash is greater. +use ringbuf::Rb; + use { crate::{ crds_entry::CrdsEntry, @@ -52,6 +54,7 @@ use { ops::{Bound, Index, IndexMut}, sync::Mutex, }, + ringbuf::HeapRb, }; const CRDS_SHARDS_BITS: u32 = 12; @@ -103,6 +106,7 @@ pub(crate) struct CrdsDataStats { pub(crate) fails: CrdsCountsArray, pub(crate) votes: LruCache, pub(crate) message_signatures: VecDeque, //TODO: change to fixed size + pub(crate) message_signatures_2: HeapRb, } #[derive(Default)] @@ -659,6 +663,7 @@ impl Default for CrdsDataStats { fails: CrdsCountsArray::default(), votes: LruCache::new(VOTE_SLOTS_METRICS_CAP), message_signatures: VecDeque::with_capacity(1024), + message_signatures_2: HeapRb::::new(1024), } } } @@ -677,7 +682,13 @@ impl CrdsDataStats { if entry.value.signature.check_ending_characters(&specific_ending) { info!("gregg signature ends in a: {:?}", entry.value.signature); self.message_signatures.push_back(entry.value.signature); + + // fixed length ring buffer. overwrite latest element if buffer full + self.message_signatures_2.push_overwrite(entry.value.signature); + } + + // else { // info!("gregg signature does not end in a: {:?}", entry.value.signature); // } From 6166f591f6e1d7048148f1a762c31d1cf3ce1cf3 Mon Sep 17 00:00:00 2001 From: greg Date: Thu, 13 Jul 2023 17:36:57 -0700 Subject: [PATCH 03/26] modify difficulty to take in n 0 bits and check if signature ending ends in n 0 bits --- gossip/src/crds.rs | 10 ++++++---- sdk/src/signature.rs | 23 ++++++++++++++++++++++- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 72ae25a3213b5f..1d4a9244d6598f 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -677,10 +677,12 @@ impl CrdsDataStats { self.votes.put(slot, num_nodes + 1); } } - let specific_ending: [u8; 1] = [01]; + // let specific_ending: [u8; 1] = [01]; + let ending_zeros = 9; // let specific_ending: [u8; 2] = [57, 01]; - if entry.value.signature.check_ending_characters(&specific_ending) { - info!("gregg signature ends in a: {:?}", entry.value.signature); + // if entry.value.signature.check_ending_characters(&specific_ending) { + if entry.value.signature.last_n_bits_are_zero(ending_zeros) { + info!("greg signature ends in a: {:?}", entry.value.signature); self.message_signatures.push_back(entry.value.signature); // fixed length ring buffer. overwrite latest element if buffer full @@ -690,7 +692,7 @@ impl CrdsDataStats { // else { - // info!("gregg signature does not end in a: {:?}", entry.value.signature); + // info!("greg signature does not end in a: {:?}", entry.value.signature); // } // self.message_signatures.push_back(entry.value.signature); } diff --git a/sdk/src/signature.rs b/sdk/src/signature.rs index acc16b00c02a15..678299526f7d71 100644 --- a/sdk/src/signature.rs +++ b/sdk/src/signature.rs @@ -13,6 +13,7 @@ use { str::FromStr, }, thiserror::Error, + log::info, }; /// Number of bytes in a signature @@ -60,10 +61,30 @@ impl Signature { if signature_bytes.len() < specific_ending.len() { return false; } - // info!("cmp bytes: {:?}, signature bytes: {:?}", specific_ending, signature_bytes); + info!("sig bytes len: {}, cmp bytes: {:?}, signature bytes: {:?}", signature_bytes.len(), specific_ending, signature_bytes); let start_index = signature_bytes.len() - specific_ending.len(); &signature_bytes[start_index..] == specific_ending } + + pub fn last_n_bits_are_zero( + &self, + n: u8, + ) -> bool { + let signature_bytes: &[u8] = self.0.as_slice(); + let num_bytes = (n as usize + 7) / 8; // Number of bytes required to represent n bits. if n=8, need 1 byte + let last_signature_byte_index = signature_bytes.len() - 1; //with 64 byte signatures, this will be 56 + + // info!("sig bytes len: {}, signature bytes: {:?}", signature_bytes.len(), signature_bytes); + + let mut combined_value: u64 = 0; + for i in 0..num_bytes { + let shift = (num_bytes - i - 1) * 8; // if n = 8, shift = 0 + combined_value |= (signature_bytes[last_signature_byte_index - i] as u64) << shift; + } + + // check if last n bits of signature (aka combined_value) are all 0. + (combined_value & ((1 << n) - 1)) == 0 + } } pub trait Signable { From 5116f367bd7e2c9103a2269537653965cd689dcc Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 13:17:21 -0700 Subject: [PATCH 04/26] update to only push every 18 trailing zero bits. and clean up --- gossip/src/cluster_info_metrics.rs | 28 ++++----------------- gossip/src/crds.rs | 39 ++++++++++++------------------ sdk/src/signature.rs | 15 ------------ 3 files changed, 21 insertions(+), 61 deletions(-) diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index bb07613907f669..71c60e8e072676 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -681,8 +681,8 @@ pub(crate) fn submit_gossip_stats( ("all-pull", crds_stats.pull.fails.iter().sum::(), i64), ); - // submit_message_signature_stats("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.message_signatures); - submit_message_signature_stats_2("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.message_signatures_2); + submit_message_signature_stats("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.crds_signatures); + if !log::log_enabled!(log::Level::Trace) { return; } @@ -713,28 +713,10 @@ where } } -// greg +// ring buffer "CrdsDataStats.message_signatures" is RWLocked +// This won't spin since other threads cannot add to message_signature +// ring buffer while we're reading from it fn submit_message_signature_stats<'a>( - name: &'static str, - message_signatures: &mut VecDeque -) { - // we want to submit all message signatures we have here. - // Need to pop them to remove them - // NOTE: we need to filter the signatures before we call this - // so only signatures with ending 0xFF or whatever end up here. - while !message_signatures.is_empty() { - match message_signatures.pop_front() { - Some(signature) => { - datapoint_info!(name, ("crds_signature", signature.to_string(), String)); - }, - None => { - error!("Error reporting submitting Crds signature. Invalid read from message signature queue"); - } - } - } -} - -fn submit_message_signature_stats_2<'a>( name: &'static str, message_signatures: &mut HeapRb, ) { diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 1d4a9244d6598f..009f6e5f24f7ac 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -24,8 +24,6 @@ //! A value is updated to a new version if the labels match, and the value //! wallclock is later, or the value hash is greater. -use ringbuf::Rb; - use { crate::{ crds_entry::CrdsEntry, @@ -54,12 +52,22 @@ use { ops::{Bound, Index, IndexMut}, sync::Mutex, }, - ringbuf::HeapRb, + ringbuf::{HeapRb, Rb}, }; const CRDS_SHARDS_BITS: u32 = 12; // Number of vote slots to track in an lru-cache for metrics. const VOTE_SLOTS_METRICS_CAP: usize = 100; +// Number of ending zero bits for crds signature to get reported to influx +// mean new push messages received per minute per node +// testnet: ~500k, +// mainnet: ~280k +// target: 1-2 signatures reported per minute +// log2(250k) = ~17.9. +const TRAILING_ZEROS: u8 = 18; +// size of ring buffer to store signatures for metrics +const SIGNATURE_SLOTS_METRICS_CAP: usize = 64; + pub struct Crds { /// Stores the map of labels and values @@ -105,8 +113,7 @@ pub(crate) struct CrdsDataStats { pub(crate) counts: CrdsCountsArray, pub(crate) fails: CrdsCountsArray, pub(crate) votes: LruCache, - pub(crate) message_signatures: VecDeque, //TODO: change to fixed size - pub(crate) message_signatures_2: HeapRb, + pub(crate) crds_signatures: HeapRb, // fixed size ring buffer } #[derive(Default)] @@ -662,8 +669,7 @@ impl Default for CrdsDataStats { counts: CrdsCountsArray::default(), fails: CrdsCountsArray::default(), votes: LruCache::new(VOTE_SLOTS_METRICS_CAP), - message_signatures: VecDeque::with_capacity(1024), - message_signatures_2: HeapRb::::new(1024), + crds_signatures: HeapRb::::new(SIGNATURE_SLOTS_METRICS_CAP), } } } @@ -677,24 +683,11 @@ impl CrdsDataStats { self.votes.put(slot, num_nodes + 1); } } - // let specific_ending: [u8; 1] = [01]; - let ending_zeros = 9; - // let specific_ending: [u8; 2] = [57, 01]; - // if entry.value.signature.check_ending_characters(&specific_ending) { - if entry.value.signature.last_n_bits_are_zero(ending_zeros) { - info!("greg signature ends in a: {:?}", entry.value.signature); - self.message_signatures.push_back(entry.value.signature); - - // fixed length ring buffer. overwrite latest element if buffer full - self.message_signatures_2.push_overwrite(entry.value.signature); + // check trailing zero bits on signature for metrics + if entry.value.signature.last_n_bits_are_zero(TRAILING_ZEROS) { + self.crds_signatures.push_overwrite(entry.value.signature); } - - - // else { - // info!("greg signature does not end in a: {:?}", entry.value.signature); - // } - // self.message_signatures.push_back(entry.value.signature); } fn record_fail(&mut self, entry: &VersionedCrdsValue) { diff --git a/sdk/src/signature.rs b/sdk/src/signature.rs index 678299526f7d71..54630dd52c43ae 100644 --- a/sdk/src/signature.rs +++ b/sdk/src/signature.rs @@ -53,19 +53,6 @@ impl Signature { self.verify_verbose(pubkey_bytes, message_bytes).is_ok() } - pub fn check_ending_characters( - &self, - specific_ending: &[u8], - ) -> bool { - let signature_bytes: &[u8] = self.0.as_slice(); - if signature_bytes.len() < specific_ending.len() { - return false; - } - info!("sig bytes len: {}, cmp bytes: {:?}, signature bytes: {:?}", signature_bytes.len(), specific_ending, signature_bytes); - let start_index = signature_bytes.len() - specific_ending.len(); - &signature_bytes[start_index..] == specific_ending - } - pub fn last_n_bits_are_zero( &self, n: u8, @@ -74,8 +61,6 @@ impl Signature { let num_bytes = (n as usize + 7) / 8; // Number of bytes required to represent n bits. if n=8, need 1 byte let last_signature_byte_index = signature_bytes.len() - 1; //with 64 byte signatures, this will be 56 - // info!("sig bytes len: {}, signature bytes: {:?}", signature_bytes.len(), signature_bytes); - let mut combined_value: u64 = 0; for i in 0..num_bytes { let shift = (num_bytes - i - 1) * 8; // if n = 8, shift = 0 From b81f7c3f566926054035c0dc02ebf700547e40ef Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 13:45:41 -0700 Subject: [PATCH 05/26] report origin with signature. and set trailing 0s to 8 for local testing --- gossip/src/cluster_info_metrics.rs | 18 ++++++------------ gossip/src/crds.rs | 8 ++++---- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index 71c60e8e072676..9d88e49b9a75b3 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -681,7 +681,7 @@ pub(crate) fn submit_gossip_stats( ("all-pull", crds_stats.pull.fails.iter().sum::(), i64), ); - submit_message_signature_stats("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.crds_signatures); + submit_crds_signature_stats("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.crds_signatures); if !log::log_enabled!(log::Level::Trace) { return; @@ -713,18 +713,12 @@ where } } -// ring buffer "CrdsDataStats.message_signatures" is RWLocked -// This won't spin since other threads cannot add to message_signature -// ring buffer while we're reading from it -fn submit_message_signature_stats<'a>( +fn submit_crds_signature_stats<'a>( name: &'static str, - message_signatures: &mut HeapRb, + crds_signatures: &mut HeapRb<(Pubkey, Signature)>, ) { - // we want to submit all message signatures we have here. - // Need to pop them to remove them - // NOTE: we need to filter the signatures before we call this - // so only signatures with ending 0xFF or whatever end up here. - while let Some(signature) = message_signatures.pop() { - datapoint_info!(name, ("crds_signature", signature.to_string(), String)); + // submit all message signatures to metrics. + while let Some((origin, signature)) = crds_signatures.pop() { + datapoint_info!(name, ("crds_origin", origin.to_string(), String), ("crds_signature", signature.to_string(), String)); } } diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 009f6e5f24f7ac..8e074ac430e6ba 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -64,7 +64,7 @@ const VOTE_SLOTS_METRICS_CAP: usize = 100; // mainnet: ~280k // target: 1-2 signatures reported per minute // log2(250k) = ~17.9. -const TRAILING_ZEROS: u8 = 18; +const TRAILING_ZEROS: u8 = 8; // size of ring buffer to store signatures for metrics const SIGNATURE_SLOTS_METRICS_CAP: usize = 64; @@ -113,7 +113,7 @@ pub(crate) struct CrdsDataStats { pub(crate) counts: CrdsCountsArray, pub(crate) fails: CrdsCountsArray, pub(crate) votes: LruCache, - pub(crate) crds_signatures: HeapRb, // fixed size ring buffer + pub(crate) crds_signatures: HeapRb<(Pubkey, Signature)>, // crds origin and signature } #[derive(Default)] @@ -669,7 +669,7 @@ impl Default for CrdsDataStats { counts: CrdsCountsArray::default(), fails: CrdsCountsArray::default(), votes: LruCache::new(VOTE_SLOTS_METRICS_CAP), - crds_signatures: HeapRb::::new(SIGNATURE_SLOTS_METRICS_CAP), + crds_signatures: HeapRb::<(Pubkey, Signature)>::new(SIGNATURE_SLOTS_METRICS_CAP), } } } @@ -686,7 +686,7 @@ impl CrdsDataStats { // check trailing zero bits on signature for metrics if entry.value.signature.last_n_bits_are_zero(TRAILING_ZEROS) { - self.crds_signatures.push_overwrite(entry.value.signature); + self.crds_signatures.push_overwrite((entry.value.pubkey(), entry.value.signature)); } } From 27a899df4ff44f23c9950de69f25201c253ce547 Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 13:56:32 -0700 Subject: [PATCH 06/26] change back to 18 trailing zeros and rm unused imports --- gossip/src/cluster_info_metrics.rs | 2 +- gossip/src/crds.rs | 2 +- sdk/src/signature.rs | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index 9d88e49b9a75b3..b6cb6ab02ec2e1 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -9,7 +9,7 @@ use { }, std::{ cmp::Reverse, - collections::{HashMap, VecDeque}, + collections::HashMap, ops::{Deref, DerefMut}, sync::atomic::{AtomicU64, Ordering}, time::Instant, diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 8e074ac430e6ba..b2a85e1ea73552 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -64,7 +64,7 @@ const VOTE_SLOTS_METRICS_CAP: usize = 100; // mainnet: ~280k // target: 1-2 signatures reported per minute // log2(250k) = ~17.9. -const TRAILING_ZEROS: u8 = 8; +const TRAILING_ZEROS: u8 = 18; // size of ring buffer to store signatures for metrics const SIGNATURE_SLOTS_METRICS_CAP: usize = 64; diff --git a/sdk/src/signature.rs b/sdk/src/signature.rs index 54630dd52c43ae..63b26ee7ec5960 100644 --- a/sdk/src/signature.rs +++ b/sdk/src/signature.rs @@ -13,7 +13,6 @@ use { str::FromStr, }, thiserror::Error, - log::info, }; /// Number of bytes in a signature From 2d4d251b1b1280191bb1911fd756075ab3592068 Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 14:10:52 -0700 Subject: [PATCH 07/26] run cargo rmt --- gossip/src/cluster_info_metrics.rs | 34 +++++++++++++++++++----------- gossip/src/crds.rs | 12 +++++------ sdk/src/signature.rs | 23 +++++++++----------- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index b6cb6ab02ec2e1..a8cc1b0977fdb9 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -1,12 +1,9 @@ use { crate::crds_gossip::CrdsGossip, itertools::Itertools, + ringbuf::{HeapRb, Rb}, solana_measure::measure::Measure, - solana_sdk::{ - clock::Slot, - pubkey::Pubkey, - signature::Signature - }, + solana_sdk::{clock::Slot, pubkey::Pubkey, signature::Signature}, std::{ cmp::Reverse, collections::HashMap, @@ -14,7 +11,6 @@ use { sync::atomic::{AtomicU64, Ordering}, time::Instant, }, - ringbuf::{HeapRb, Rb} }; #[derive(Default)] @@ -194,7 +190,14 @@ pub(crate) fn submit_gossip_stats( gossip: &CrdsGossip, stakes: &HashMap, ) { - let (mut crds_stats, table_size, num_nodes, num_pubkeys, purged_values_size, failed_inserts_size) = { + let ( + mut crds_stats, + table_size, + num_nodes, + num_pubkeys, + purged_values_size, + failed_inserts_size, + ) = { let gossip_crds = gossip.crds.read().unwrap(); ( gossip_crds.take_stats(), @@ -681,8 +684,11 @@ pub(crate) fn submit_gossip_stats( ("all-pull", crds_stats.pull.fails.iter().sum::(), i64), ); - submit_crds_signature_stats("cluster_info_crds_stats_message_signatures_received", &mut crds_stats.push.crds_signatures); - + submit_crds_signature_stats( + "cluster_info_crds_stats_message_signatures_received", + &mut crds_stats.push.crds_signatures, + ); + if !log::log_enabled!(log::Level::Trace) { return; } @@ -714,11 +720,15 @@ where } fn submit_crds_signature_stats<'a>( - name: &'static str, + name: &'static str, crds_signatures: &mut HeapRb<(Pubkey, Signature)>, ) { - // submit all message signatures to metrics. + // submit all message signatures to metrics. while let Some((origin, signature)) = crds_signatures.pop() { - datapoint_info!(name, ("crds_origin", origin.to_string(), String), ("crds_signature", signature.to_string(), String)); + datapoint_info!( + name, + ("crds_origin", origin.to_string(), String), + ("crds_signature", signature.to_string(), String) + ); } } diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index b2a85e1ea73552..bf4766a34a2911 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -40,6 +40,7 @@ use { lru::LruCache, matches::debug_assert_matches, rayon::{prelude::*, ThreadPool}, + ringbuf::{HeapRb, Rb}, solana_sdk::{ clock::Slot, hash::{hash, Hash}, @@ -52,7 +53,6 @@ use { ops::{Bound, Index, IndexMut}, sync::Mutex, }, - ringbuf::{HeapRb, Rb}, }; const CRDS_SHARDS_BITS: u32 = 12; @@ -60,15 +60,14 @@ const CRDS_SHARDS_BITS: u32 = 12; const VOTE_SLOTS_METRICS_CAP: usize = 100; // Number of ending zero bits for crds signature to get reported to influx // mean new push messages received per minute per node -// testnet: ~500k, +// testnet: ~500k, // mainnet: ~280k // target: 1-2 signatures reported per minute -// log2(250k) = ~17.9. +// log2(250k) = ~17.9. const TRAILING_ZEROS: u8 = 18; // size of ring buffer to store signatures for metrics const SIGNATURE_SLOTS_METRICS_CAP: usize = 64; - pub struct Crds { /// Stores the map of labels and values table: IndexMap, @@ -686,8 +685,9 @@ impl CrdsDataStats { // check trailing zero bits on signature for metrics if entry.value.signature.last_n_bits_are_zero(TRAILING_ZEROS) { - self.crds_signatures.push_overwrite((entry.value.pubkey(), entry.value.signature)); - } + self.crds_signatures + .push_overwrite((entry.value.pubkey(), entry.value.signature)); + } } fn record_fail(&mut self, entry: &VersionedCrdsValue) { diff --git a/sdk/src/signature.rs b/sdk/src/signature.rs index 63b26ee7ec5960..7e62d5f0856bf1 100644 --- a/sdk/src/signature.rs +++ b/sdk/src/signature.rs @@ -52,22 +52,19 @@ impl Signature { self.verify_verbose(pubkey_bytes, message_bytes).is_ok() } - pub fn last_n_bits_are_zero( - &self, - n: u8, - ) -> bool { + pub fn last_n_bits_are_zero(&self, n: u8) -> bool { let signature_bytes: &[u8] = self.0.as_slice(); - let num_bytes = (n as usize + 7) / 8; // Number of bytes required to represent n bits. if n=8, need 1 byte - let last_signature_byte_index = signature_bytes.len() - 1; //with 64 byte signatures, this will be 56 - - let mut combined_value: u64 = 0; + let num_bytes = (n as usize + 7) / 8; // Number of bytes required to represent n bits + let last_signature_byte_index = signature_bytes.len() - 1; + + let mut signature_ending: u64 = 0; for i in 0..num_bytes { - let shift = (num_bytes - i - 1) * 8; // if n = 8, shift = 0 - combined_value |= (signature_bytes[last_signature_byte_index - i] as u64) << shift; + let shift = (num_bytes - i - 1) * 8; + signature_ending |= (signature_bytes[last_signature_byte_index - i] as u64) << shift; } - - // check if last n bits of signature (aka combined_value) are all 0. - (combined_value & ((1 << n) - 1)) == 0 + + // check if last n bits of signature (aka signature_ending) are all 0. + (signature_ending & ((1 << n) - 1)) == 0 } } From 13aad24bef98269b96799be8cd45c06412e69239 Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 15:25:36 -0700 Subject: [PATCH 08/26] run ./scripts/cargo-for-all-lock-files.sh tree --- programs/sbf/Cargo.lock | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index cfe36bc11f43c6..c603f1ab136e80 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -3972,6 +3972,15 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "ringbuf" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79abed428d1fd2a128201cec72c5f6938e2da607c6f3745f769fabea399d950a" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "rocksdb" version = "0.21.0" @@ -5064,6 +5073,7 @@ dependencies = [ "rand 0.7.3", "rand_chacha 0.2.2", "rayon", + "ringbuf", "rustc_version", "serde", "serde_bytes", From 3360748e6dbdef43c5d11157b19c046f2221af2c Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 15:34:19 -0700 Subject: [PATCH 09/26] allow integer arithmetic for bit comparison --- sdk/src/signature.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/src/signature.rs b/sdk/src/signature.rs index 7e62d5f0856bf1..a3ae9f4db18527 100644 --- a/sdk/src/signature.rs +++ b/sdk/src/signature.rs @@ -1,5 +1,6 @@ //! Functionality for public and private keys. #![cfg(feature = "full")] +#![allow(clippy::integer_arithmetic)] // legacy module paths pub use crate::signer::{keypair::*, null_signer::*, presigner::*, *}; From c09b643ccbbfac65bb7598e68078e7cd6edf0994 Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 15:41:47 -0700 Subject: [PATCH 10/26] rm unused lifetime --- gossip/src/cluster_info_metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index a8cc1b0977fdb9..6f767642795a50 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -719,7 +719,7 @@ where } } -fn submit_crds_signature_stats<'a>( +fn submit_crds_signature_stats( name: &'static str, crds_signatures: &mut HeapRb<(Pubkey, Signature)>, ) { From 66a73364b5f5b71237122b03bf718fd3c285bc62 Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 16:31:46 -0700 Subject: [PATCH 11/26] rm duplicate entry? --- programs/sbf/Cargo.lock | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index c603f1ab136e80..cfe36bc11f43c6 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -3972,15 +3972,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "ringbuf" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79abed428d1fd2a128201cec72c5f6938e2da607c6f3745f769fabea399d950a" -dependencies = [ - "crossbeam-utils", -] - [[package]] name = "rocksdb" version = "0.21.0" @@ -5073,7 +5064,6 @@ dependencies = [ "rand 0.7.3", "rand_chacha 0.2.2", "rayon", - "ringbuf", "rustc_version", "serde", "serde_bytes", From d2ef0d0e51d587b91b3e1fdd19a385bfcfd962ff Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 23:34:55 +0000 Subject: [PATCH 12/26] re implement ring buf --- programs/sbf/Cargo.lock | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index cfe36bc11f43c6..c603f1ab136e80 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -3972,6 +3972,15 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "ringbuf" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79abed428d1fd2a128201cec72c5f6938e2da607c6f3745f769fabea399d950a" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "rocksdb" version = "0.21.0" @@ -5064,6 +5073,7 @@ dependencies = [ "rand 0.7.3", "rand_chacha 0.2.2", "rayon", + "ringbuf", "rustc_version", "serde", "serde_bytes", From 15956f00c3f2b335aa02b4c2e951c3765b2810a9 Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 16:53:59 -0700 Subject: [PATCH 13/26] put ringbuf in sorted order --- gossip/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gossip/Cargo.toml b/gossip/Cargo.toml index 5c23ae3a460fd7..27a30f7428096f 100644 --- a/gossip/Cargo.toml +++ b/gossip/Cargo.toml @@ -24,6 +24,7 @@ num-traits = { workspace = true } rand = { workspace = true } rand_chacha = { workspace = true } rayon = { workspace = true } +ringbuf = { workspace = true } serde = { workspace = true } serde_bytes = { workspace = true } serde_derive = { workspace = true } @@ -49,7 +50,6 @@ solana-version = { workspace = true } solana-vote-program = { workspace = true } static_assertions = { workspace = true } thiserror = { workspace = true } -ringbuf = { workspace = true } [dev-dependencies] num_cpus = { workspace = true } From 0908dc3505c9bd4b9aad70c17c3657332272889b Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 14 Jul 2023 17:07:19 -0700 Subject: [PATCH 14/26] ringbuf in cargo.toml now in sorted order --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 29e083c748aa35..738d42360fcfd9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -270,6 +270,7 @@ rayon = "1.7.0" rcgen = "0.10.0" reed-solomon-erasure = "6.0.0" regex = "1.9.1" +ringbuf = "0.3.3" rolling-file = "0.2.0" reqwest = { version = "0.11.17", default-features = false } rpassword = "7.2" @@ -405,7 +406,6 @@ winreg = "0.10" x509-parser = "0.14.0" zeroize = { version = "1.3", default-features = false } zstd = "0.11.2" -ringbuf = "0.3.3" [patch.crates-io] # for details, see https://github.com/solana-labs/crossbeam/commit/fd279d707025f0e60951e429bf778b4813d1b6bf From 4e1590553e20205575be60b791ba0b75bf4b0a8a Mon Sep 17 00:00:00 2001 From: greg Date: Mon, 17 Jul 2023 11:29:21 -0700 Subject: [PATCH 15/26] rm ring buf, refactor, fix trailing zero bug --- Cargo.lock | 10 --------- Cargo.toml | 1 - gossip/Cargo.toml | 1 - gossip/src/cluster_info_metrics.rs | 24 ++------------------- gossip/src/crds.rs | 34 +++++++++++++++++++++--------- programs/sbf/Cargo.lock | 10 --------- sdk/src/signature.rs | 16 -------------- 7 files changed, 26 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 66e532d375713b..afb3fdf76bdbb9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4435,15 +4435,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "ringbuf" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79abed428d1fd2a128201cec72c5f6938e2da607c6f3745f769fabea399d950a" -dependencies = [ - "crossbeam-utils", -] - [[package]] name = "rocksdb" version = "0.21.0" @@ -6003,7 +5994,6 @@ dependencies = [ "rand_chacha 0.2.2", "rayon", "regex", - "ringbuf", "rustc_version 0.4.0", "serde", "serde_bytes", diff --git a/Cargo.toml b/Cargo.toml index 738d42360fcfd9..0f83787bf4d05f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -270,7 +270,6 @@ rayon = "1.7.0" rcgen = "0.10.0" reed-solomon-erasure = "6.0.0" regex = "1.9.1" -ringbuf = "0.3.3" rolling-file = "0.2.0" reqwest = { version = "0.11.17", default-features = false } rpassword = "7.2" diff --git a/gossip/Cargo.toml b/gossip/Cargo.toml index 27a30f7428096f..60ecf66d4480f2 100644 --- a/gossip/Cargo.toml +++ b/gossip/Cargo.toml @@ -24,7 +24,6 @@ num-traits = { workspace = true } rand = { workspace = true } rand_chacha = { workspace = true } rayon = { workspace = true } -ringbuf = { workspace = true } serde = { workspace = true } serde_bytes = { workspace = true } serde_derive = { workspace = true } diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index 6f767642795a50..e334b565adbe3e 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -1,9 +1,8 @@ use { crate::crds_gossip::CrdsGossip, itertools::Itertools, - ringbuf::{HeapRb, Rb}, solana_measure::measure::Measure, - solana_sdk::{clock::Slot, pubkey::Pubkey, signature::Signature}, + solana_sdk::{clock::Slot, pubkey::Pubkey}, std::{ cmp::Reverse, collections::HashMap, @@ -684,11 +683,6 @@ pub(crate) fn submit_gossip_stats( ("all-pull", crds_stats.pull.fails.iter().sum::(), i64), ); - submit_crds_signature_stats( - "cluster_info_crds_stats_message_signatures_received", - &mut crds_stats.push.crds_signatures, - ); - if !log::log_enabled!(log::Level::Trace) { return; } @@ -717,18 +711,4 @@ where for (slot, num_votes) in votes.into_iter().take(NUM_SLOTS) { datapoint_trace!(name, ("slot", slot, i64), ("num_votes", num_votes, i64)); } -} - -fn submit_crds_signature_stats( - name: &'static str, - crds_signatures: &mut HeapRb<(Pubkey, Signature)>, -) { - // submit all message signatures to metrics. - while let Some((origin, signature)) = crds_signatures.pop() { - datapoint_info!( - name, - ("crds_origin", origin.to_string(), String), - ("crds_signature", signature.to_string(), String) - ); - } -} +} \ No newline at end of file diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index bf4766a34a2911..ad8ec1217bb5b0 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -23,6 +23,7 @@ //! //! A value is updated to a new version if the labels match, and the value //! wallclock is later, or the value hash is greater. +#![allow(clippy::integer_arithmetic)] use { crate::{ @@ -40,12 +41,10 @@ use { lru::LruCache, matches::debug_assert_matches, rayon::{prelude::*, ThreadPool}, - ringbuf::{HeapRb, Rb}, solana_sdk::{ clock::Slot, hash::{hash, Hash}, pubkey::Pubkey, - signature::Signature, }, std::{ cmp::Ordering, @@ -64,9 +63,23 @@ const VOTE_SLOTS_METRICS_CAP: usize = 100; // mainnet: ~280k // target: 1-2 signatures reported per minute // log2(250k) = ~17.9. -const TRAILING_ZEROS: u8 = 18; -// size of ring buffer to store signatures for metrics -const SIGNATURE_SLOTS_METRICS_CAP: usize = 64; +const SIGNATURE_SAMPLE_TRAILING_ZEROS: u8 = 8; + +pub fn trailing_n_crds_signature_bytes_are_zero( + signature_bytes: &[u8], + n: u8, +) -> bool { + let num_bytes: usize = (n as usize + 7) / 8; // Number of bytes required to represent n bits + + let mut signature_ending: u64 = 0; + for i in 0..num_bytes { + let shift = (num_bytes - i - 1) * 8; + signature_ending |= (signature_bytes[i] as u64) << shift; + } + + // check if last n bits of signature (aka signature_ending) are all 0. + (signature_ending & ((1 << n) - 1)) == 0 +} pub struct Crds { /// Stores the map of labels and values @@ -112,7 +125,6 @@ pub(crate) struct CrdsDataStats { pub(crate) counts: CrdsCountsArray, pub(crate) fails: CrdsCountsArray, pub(crate) votes: LruCache, - pub(crate) crds_signatures: HeapRb<(Pubkey, Signature)>, // crds origin and signature } #[derive(Default)] @@ -668,7 +680,6 @@ impl Default for CrdsDataStats { counts: CrdsCountsArray::default(), fails: CrdsCountsArray::default(), votes: LruCache::new(VOTE_SLOTS_METRICS_CAP), - crds_signatures: HeapRb::<(Pubkey, Signature)>::new(SIGNATURE_SLOTS_METRICS_CAP), } } } @@ -684,9 +695,12 @@ impl CrdsDataStats { } // check trailing zero bits on signature for metrics - if entry.value.signature.last_n_bits_are_zero(TRAILING_ZEROS) { - self.crds_signatures - .push_overwrite((entry.value.pubkey(), entry.value.signature)); + if trailing_n_crds_signature_bytes_are_zero(entry.value.signature.as_ref(), SIGNATURE_SAMPLE_TRAILING_ZEROS) { + datapoint_info!( + "cluster_info_crds_message_signatures", + ("crds_origin", entry.value.pubkey().to_string(), String), + ("crds_signature", entry.value.signature.to_string(), String) + ); } } diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index c603f1ab136e80..cfe36bc11f43c6 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -3972,15 +3972,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "ringbuf" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79abed428d1fd2a128201cec72c5f6938e2da607c6f3745f769fabea399d950a" -dependencies = [ - "crossbeam-utils", -] - [[package]] name = "rocksdb" version = "0.21.0" @@ -5073,7 +5064,6 @@ dependencies = [ "rand 0.7.3", "rand_chacha 0.2.2", "rayon", - "ringbuf", "rustc_version", "serde", "serde_bytes", diff --git a/sdk/src/signature.rs b/sdk/src/signature.rs index 8e6264eede2e5a..e3cc900e49efc1 100644 --- a/sdk/src/signature.rs +++ b/sdk/src/signature.rs @@ -1,6 +1,5 @@ //! Functionality for public and private keys. #![cfg(feature = "full")] -#![allow(clippy::integer_arithmetic)] // legacy module paths pub use crate::signer::{keypair::*, null_signer::*, presigner::*, *}; @@ -55,21 +54,6 @@ impl Signature { pub fn verify(&self, pubkey_bytes: &[u8], message_bytes: &[u8]) -> bool { self.verify_verbose(pubkey_bytes, message_bytes).is_ok() } - - pub fn last_n_bits_are_zero(&self, n: u8) -> bool { - let signature_bytes: &[u8] = self.0.as_slice(); - let num_bytes = (n as usize + 7) / 8; // Number of bytes required to represent n bits - let last_signature_byte_index = signature_bytes.len() - 1; - - let mut signature_ending: u64 = 0; - for i in 0..num_bytes { - let shift = (num_bytes - i - 1) * 8; - signature_ending |= (signature_bytes[last_signature_byte_index - i] as u64) << shift; - } - - // check if last n bits of signature (aka signature_ending) are all 0. - (signature_ending & ((1 << n) - 1)) == 0 - } } pub trait Signable { From b5055373ddc7c7a2cac3f1266d981fd52ec088f6 Mon Sep 17 00:00:00 2001 From: greg Date: Mon, 17 Jul 2023 12:58:42 -0700 Subject: [PATCH 16/26] fix bug in trailing zeros. was comparing wrong ones --- gossip/src/cluster_info_metrics.rs | 11 ++--------- gossip/src/crds.rs | 16 ++++++++-------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index e334b565adbe3e..8a93c5e238cf8b 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -189,14 +189,7 @@ pub(crate) fn submit_gossip_stats( gossip: &CrdsGossip, stakes: &HashMap, ) { - let ( - mut crds_stats, - table_size, - num_nodes, - num_pubkeys, - purged_values_size, - failed_inserts_size, - ) = { + let (crds_stats, table_size, num_nodes, num_pubkeys, purged_values_size, failed_inserts_size) = { let gossip_crds = gossip.crds.read().unwrap(); ( gossip_crds.take_stats(), @@ -711,4 +704,4 @@ where for (slot, num_votes) in votes.into_iter().take(NUM_SLOTS) { datapoint_trace!(name, ("slot", slot, i64), ("num_votes", num_votes, i64)); } -} \ No newline at end of file +} diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index ad8ec1217bb5b0..ef6b6baa0ac5cd 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -63,17 +63,14 @@ const VOTE_SLOTS_METRICS_CAP: usize = 100; // mainnet: ~280k // target: 1-2 signatures reported per minute // log2(250k) = ~17.9. -const SIGNATURE_SAMPLE_TRAILING_ZEROS: u8 = 8; +const SIGNATURE_SAMPLE_TRAILING_ZEROS: u8 = 18; -pub fn trailing_n_crds_signature_bytes_are_zero( - signature_bytes: &[u8], - n: u8, -) -> bool { +pub fn trailing_n_crds_signature_bits_are_zero(signature_bytes: &[u8], n: u8) -> bool { let num_bytes: usize = (n as usize + 7) / 8; // Number of bytes required to represent n bits let mut signature_ending: u64 = 0; for i in 0..num_bytes { - let shift = (num_bytes - i - 1) * 8; + let shift = i * 8; signature_ending |= (signature_bytes[i] as u64) << shift; } @@ -695,12 +692,15 @@ impl CrdsDataStats { } // check trailing zero bits on signature for metrics - if trailing_n_crds_signature_bytes_are_zero(entry.value.signature.as_ref(), SIGNATURE_SAMPLE_TRAILING_ZEROS) { + if trailing_n_crds_signature_bits_are_zero( + entry.value.signature.as_ref(), + SIGNATURE_SAMPLE_TRAILING_ZEROS, + ) { datapoint_info!( "cluster_info_crds_message_signatures", ("crds_origin", entry.value.pubkey().to_string(), String), ("crds_signature", entry.value.signature.to_string(), String) - ); + ); } } From 0b0d90ffd328a47d73afdbc7ed432315873fadcc Mon Sep 17 00:00:00 2001 From: greg Date: Mon, 17 Jul 2023 13:19:49 -0700 Subject: [PATCH 17/26] fix needless range loop bug --- gossip/src/crds.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index ef6b6baa0ac5cd..c3e65571c025c1 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -69,9 +69,9 @@ pub fn trailing_n_crds_signature_bits_are_zero(signature_bytes: &[u8], n: u8) -> let num_bytes: usize = (n as usize + 7) / 8; // Number of bytes required to represent n bits let mut signature_ending: u64 = 0; - for i in 0..num_bytes { + for (i, signature_byte) in signature_bytes.iter().enumerate().take(num_bytes) { let shift = i * 8; - signature_ending |= (signature_bytes[i] as u64) << shift; + signature_ending |= (*signature_bytes as u64) << shift; } // check if last n bits of signature (aka signature_ending) are all 0. From 860356951682d4bf37b465bce1da972c6a7430ac Mon Sep 17 00:00:00 2001 From: greg Date: Mon, 17 Jul 2023 13:24:09 -0700 Subject: [PATCH 18/26] fix bug --- gossip/src/crds.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index c3e65571c025c1..4dbd700c4aa9df 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -71,7 +71,7 @@ pub fn trailing_n_crds_signature_bits_are_zero(signature_bytes: &[u8], n: u8) -> let mut signature_ending: u64 = 0; for (i, signature_byte) in signature_bytes.iter().enumerate().take(num_bytes) { let shift = i * 8; - signature_ending |= (*signature_bytes as u64) << shift; + signature_ending |= (*signature_byte as u64) << shift; } // check if last n bits of signature (aka signature_ending) are all 0. From 886f39e85de6b20a02cb4cf7833f479d614cf263 Mon Sep 17 00:00:00 2001 From: greg Date: Tue, 18 Jul 2023 10:30:14 -0700 Subject: [PATCH 19/26] change trailing zero checking to build in methods and only report first 8 bytes of signature and origin pubkey --- Cargo.lock | 1 + gossip/Cargo.toml | 1 + gossip/src/cluster_info_metrics.rs | 1 - gossip/src/crds.rs | 53 ++++++++++++++++++------------ 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index afb3fdf76bdbb9..2367166f2a89ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5979,6 +5979,7 @@ name = "solana-gossip" version = "1.17.0" dependencies = [ "bincode", + "bs58", "bv", "clap 2.33.3", "crossbeam-channel", diff --git a/gossip/Cargo.toml b/gossip/Cargo.toml index 60ecf66d4480f2..9d4d2dfdc9b3d4 100644 --- a/gossip/Cargo.toml +++ b/gossip/Cargo.toml @@ -11,6 +11,7 @@ edition = { workspace = true } [dependencies] bincode = { workspace = true } +bs58 = { workspace = true } bv = { workspace = true, features = ["serde"] } clap = { workspace = true } crossbeam-channel = { workspace = true } diff --git a/gossip/src/cluster_info_metrics.rs b/gossip/src/cluster_info_metrics.rs index 8a93c5e238cf8b..8765cd158cd469 100644 --- a/gossip/src/cluster_info_metrics.rs +++ b/gossip/src/cluster_info_metrics.rs @@ -675,7 +675,6 @@ pub(crate) fn submit_gossip_stats( ("all-push", crds_stats.push.fails.iter().sum::(), i64), ("all-pull", crds_stats.pull.fails.iter().sum::(), i64), ); - if !log::log_enabled!(log::Level::Trace) { return; } diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 4dbd700c4aa9df..1d9b4144a520ff 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -23,7 +23,6 @@ //! //! A value is updated to a new version if the labels match, and the value //! wallclock is later, or the value hash is greater. -#![allow(clippy::integer_arithmetic)] use { crate::{ @@ -45,6 +44,7 @@ use { clock::Slot, hash::{hash, Hash}, pubkey::Pubkey, + signature::Signature, }, std::{ cmp::Ordering, @@ -57,25 +57,30 @@ use { const CRDS_SHARDS_BITS: u32 = 12; // Number of vote slots to track in an lru-cache for metrics. const VOTE_SLOTS_METRICS_CAP: usize = 100; -// Number of ending zero bits for crds signature to get reported to influx +// Number of trailing zero bits for crds signature to get reported to influx // mean new push messages received per minute per node // testnet: ~500k, // mainnet: ~280k // target: 1-2 signatures reported per minute // log2(250k) = ~17.9. -const SIGNATURE_SAMPLE_TRAILING_ZEROS: u8 = 18; - -pub fn trailing_n_crds_signature_bits_are_zero(signature_bytes: &[u8], n: u8) -> bool { - let num_bytes: usize = (n as usize + 7) / 8; // Number of bytes required to represent n bits - - let mut signature_ending: u64 = 0; - for (i, signature_byte) in signature_bytes.iter().enumerate().take(num_bytes) { - let shift = i * 8; - signature_ending |= (*signature_byte as u64) << shift; - } - - // check if last n bits of signature (aka signature_ending) are all 0. - (signature_ending & ((1 << n) - 1)) == 0 +const SIGNATURE_SAMPLE_TRAILING_ZEROS: u32 = 8; + +fn should_report_message_signature(signature_bytes: Signature) -> bool { + // let num_bytes: usize = (SIGNATURE_SAMPLE_TRAILING_ZEROS as usize + 7) / 8; // Number of bytes required to represent n bits + + // let mut signature_ending: u64 = 0; + // for (i, signature_byte) in signature_bytes.as_ref().iter().enumerate().take(num_bytes) { + // let shift = i * 8; + // signature_ending |= (*signature_byte as u64) << shift; + // } + + // // check if last n bits of signature (aka signature_ending) are all 0. + // (signature_ending & ((1 << SIGNATURE_SAMPLE_TRAILING_ZEROS) - 1)) == 0 + let (trailing_signature_bytes, _) = signature_bytes + .as_ref() + .split_at(std::mem::size_of::()); + u64::from_le_bytes(trailing_signature_bytes.try_into().unwrap()).trailing_zeros() + == SIGNATURE_SAMPLE_TRAILING_ZEROS } pub struct Crds { @@ -692,15 +697,21 @@ impl CrdsDataStats { } // check trailing zero bits on signature for metrics - if trailing_n_crds_signature_bits_are_zero( - entry.value.signature.as_ref(), - SIGNATURE_SAMPLE_TRAILING_ZEROS, - ) { + if should_report_message_signature(entry.value.signature) { + // let sig_string = bs58::encode(entry.value.signature.as_ref()[..16]).into_string(); datapoint_info!( "cluster_info_crds_message_signatures", - ("crds_origin", entry.value.pubkey().to_string(), String), - ("crds_signature", entry.value.signature.to_string(), String) + ("crds_origin", bs58::encode(&entry.value.pubkey().as_ref()[..8]).into_string(), String), + ("crds_signature", bs58::encode(&entry.value.signature.as_ref()[..8]).into_string(), String) ); + + info!("crds_origin full string: {:?}", entry.value.pubkey().to_string()); + info!("crds_signature full string: {:?}", entry.value.signature.to_string()); + // datapoint_info!( + // "cluster_info_crds_message_signatures", + // ("crds_origin", entry.value.pubkey().to_string(), String), + // ("crds_signature", entry.value.signature.to_string(), String) + // ); } } From 5068f9d38b448fd5df95d9726eb9982044497fd9 Mon Sep 17 00:00:00 2001 From: greg Date: Tue, 18 Jul 2023 11:55:28 -0700 Subject: [PATCH 20/26] report full origin string and first 8 bytes of signature --- gossip/src/crds.rs | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 1d9b4144a520ff..c4794b8060fb09 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -57,7 +57,7 @@ use { const CRDS_SHARDS_BITS: u32 = 12; // Number of vote slots to track in an lru-cache for metrics. const VOTE_SLOTS_METRICS_CAP: usize = 100; -// Number of trailing zero bits for crds signature to get reported to influx +// Required number of trailing zero bits for crds signature to get reported to influx // mean new push messages received per minute per node // testnet: ~500k, // mainnet: ~280k @@ -65,22 +65,13 @@ const VOTE_SLOTS_METRICS_CAP: usize = 100; // log2(250k) = ~17.9. const SIGNATURE_SAMPLE_TRAILING_ZEROS: u32 = 8; +/// check if last SIGNATURE_SAMPLE_TRAILING_ZEROS bits of signature are 0 fn should_report_message_signature(signature_bytes: Signature) -> bool { - // let num_bytes: usize = (SIGNATURE_SAMPLE_TRAILING_ZEROS as usize + 7) / 8; // Number of bytes required to represent n bits - - // let mut signature_ending: u64 = 0; - // for (i, signature_byte) in signature_bytes.as_ref().iter().enumerate().take(num_bytes) { - // let shift = i * 8; - // signature_ending |= (*signature_byte as u64) << shift; - // } - - // // check if last n bits of signature (aka signature_ending) are all 0. - // (signature_ending & ((1 << SIGNATURE_SAMPLE_TRAILING_ZEROS) - 1)) == 0 let (trailing_signature_bytes, _) = signature_bytes .as_ref() .split_at(std::mem::size_of::()); u64::from_le_bytes(trailing_signature_bytes.try_into().unwrap()).trailing_zeros() - == SIGNATURE_SAMPLE_TRAILING_ZEROS + >= SIGNATURE_SAMPLE_TRAILING_ZEROS } pub struct Crds { @@ -696,22 +687,16 @@ impl CrdsDataStats { } } - // check trailing zero bits on signature for metrics if should_report_message_signature(entry.value.signature) { - // let sig_string = bs58::encode(entry.value.signature.as_ref()[..16]).into_string(); datapoint_info!( "cluster_info_crds_message_signatures", - ("crds_origin", bs58::encode(&entry.value.pubkey().as_ref()[..8]).into_string(), String), - ("crds_signature", bs58::encode(&entry.value.signature.as_ref()[..8]).into_string(), String) + ("crds_origin", entry.value.pubkey().to_string(), String), + ( + "crds_signature", + bs58::encode(&entry.value.signature.as_ref()[..8]).into_string(), + String + ) ); - - info!("crds_origin full string: {:?}", entry.value.pubkey().to_string()); - info!("crds_signature full string: {:?}", entry.value.signature.to_string()); - // datapoint_info!( - // "cluster_info_crds_message_signatures", - // ("crds_origin", entry.value.pubkey().to_string(), String), - // ("crds_signature", entry.value.signature.to_string(), String) - // ); } } From 1550d26aacabd957943af89e2f84890fb433c773 Mon Sep 17 00:00:00 2001 From: greg Date: Tue, 18 Jul 2023 11:55:55 -0700 Subject: [PATCH 21/26] set SIGNATURE_SAMPLE_TRAILING_ZEROS back to 18 --- gossip/src/crds.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index c4794b8060fb09..421e2537ceb964 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -63,7 +63,7 @@ const VOTE_SLOTS_METRICS_CAP: usize = 100; // mainnet: ~280k // target: 1-2 signatures reported per minute // log2(250k) = ~17.9. -const SIGNATURE_SAMPLE_TRAILING_ZEROS: u32 = 8; +const SIGNATURE_SAMPLE_TRAILING_ZEROS: u32 = 18; /// check if last SIGNATURE_SAMPLE_TRAILING_ZEROS bits of signature are 0 fn should_report_message_signature(signature_bytes: Signature) -> bool { From 4613833bb2f37b58c193b91ab3c3b663c44ec242 Mon Sep 17 00:00:00 2001 From: greg Date: Tue, 18 Jul 2023 12:06:34 -0700 Subject: [PATCH 22/26] forgot to run cargo tree --- programs/sbf/Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index cfe36bc11f43c6..c3435ceddc9f16 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5051,6 +5051,7 @@ name = "solana-gossip" version = "1.17.0" dependencies = [ "bincode", + "bs58", "bv", "clap 2.33.3", "crossbeam-channel", From 9e2e8437c7ab55812359d9a85320b1f4da2e3333 Mon Sep 17 00:00:00 2001 From: greg Date: Wed, 19 Jul 2023 13:31:37 -0700 Subject: [PATCH 23/26] avoid panic and change working --- gossip/src/crds.rs | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 421e2537ceb964..3fd06b84105370 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -57,22 +57,13 @@ use { const CRDS_SHARDS_BITS: u32 = 12; // Number of vote slots to track in an lru-cache for metrics. const VOTE_SLOTS_METRICS_CAP: usize = 100; -// Required number of trailing zero bits for crds signature to get reported to influx +// Required number of leading zero bits for crds signature to get reported to influx // mean new push messages received per minute per node // testnet: ~500k, // mainnet: ~280k -// target: 1-2 signatures reported per minute -// log2(250k) = ~17.9. -const SIGNATURE_SAMPLE_TRAILING_ZEROS: u32 = 18; - -/// check if last SIGNATURE_SAMPLE_TRAILING_ZEROS bits of signature are 0 -fn should_report_message_signature(signature_bytes: Signature) -> bool { - let (trailing_signature_bytes, _) = signature_bytes - .as_ref() - .split_at(std::mem::size_of::()); - u64::from_le_bytes(trailing_signature_bytes.try_into().unwrap()).trailing_zeros() - >= SIGNATURE_SAMPLE_TRAILING_ZEROS -} +// target: 1 signature reported per minute +// log2(500k) = ~18.9. +const SIGNATURE_SAMPLE_LEADING_ZEROS: u32 = 19; pub struct Crds { /// Stores the map of labels and values @@ -687,13 +678,17 @@ impl CrdsDataStats { } } - if should_report_message_signature(entry.value.signature) { + if should_report_message_signature(&entry.value.signature) { datapoint_info!( "cluster_info_crds_message_signatures", - ("crds_origin", entry.value.pubkey().to_string(), String), + ( + "crds_origin", + entry.value.pubkey().to_string().get(..8).unwrap(), + String + ), ( "crds_signature", - bs58::encode(&entry.value.signature.as_ref()[..8]).into_string(), + entry.value.signature.to_string().get(..8).unwrap(), String ) ); @@ -743,6 +738,15 @@ impl CrdsStats { } } +/// check if first SIGNATURE_SAMPLE_LEADING_ZEROS bits of signature are 0 +#[inline] +fn should_report_message_signature(signature: &Signature) -> bool { + let Some(Ok(bytes)) = signature.as_ref().get(..8).map(<[u8; 8]>::try_from) else { + return false; + }; + u64::from_le_bytes(bytes).trailing_zeros() >= SIGNATURE_SAMPLE_LEADING_ZEROS +} + #[cfg(test)] mod tests { use { From f94afc82fa5940917a4d4f6b0b04f5e5ec8a97de Mon Sep 17 00:00:00 2001 From: greg Date: Wed, 19 Jul 2023 13:38:21 -0700 Subject: [PATCH 24/26] rm bs58 --- Cargo.lock | 1 - gossip/Cargo.toml | 1 - programs/sbf/Cargo.lock | 1 - 3 files changed, 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2367166f2a89ad..afb3fdf76bdbb9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5979,7 +5979,6 @@ name = "solana-gossip" version = "1.17.0" dependencies = [ "bincode", - "bs58", "bv", "clap 2.33.3", "crossbeam-channel", diff --git a/gossip/Cargo.toml b/gossip/Cargo.toml index 9d4d2dfdc9b3d4..60ecf66d4480f2 100644 --- a/gossip/Cargo.toml +++ b/gossip/Cargo.toml @@ -11,7 +11,6 @@ edition = { workspace = true } [dependencies] bincode = { workspace = true } -bs58 = { workspace = true } bv = { workspace = true, features = ["serde"] } clap = { workspace = true } crossbeam-channel = { workspace = true } diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index c3435ceddc9f16..cfe36bc11f43c6 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5051,7 +5051,6 @@ name = "solana-gossip" version = "1.17.0" dependencies = [ "bincode", - "bs58", "bv", "clap 2.33.3", "crossbeam-channel", From 616326b2812e217a5aa1216b6bbddc30ac31d1c3 Mon Sep 17 00:00:00 2001 From: greg Date: Wed, 19 Jul 2023 14:51:05 -0700 Subject: [PATCH 25/26] pass in Option into datapoint_info --- gossip/src/crds.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 3fd06b84105370..582fdbfe2bb8eb 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -683,13 +683,13 @@ impl CrdsDataStats { "cluster_info_crds_message_signatures", ( "crds_origin", - entry.value.pubkey().to_string().get(..8).unwrap(), - String + entry.value.pubkey().to_string().get(..8), + Option ), ( "crds_signature", - entry.value.signature.to_string().get(..8).unwrap(), - String + entry.value.signature.to_string().get(..8), + Option ) ); } From 61151c10f6fff002dd0c6d675db69673a7724100 Mon Sep 17 00:00:00 2001 From: greg Date: Wed, 19 Jul 2023 15:21:58 -0700 Subject: [PATCH 26/26] shorten metric names --- gossip/src/crds.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 582fdbfe2bb8eb..c19eb9b4d5c335 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -680,14 +680,14 @@ impl CrdsDataStats { if should_report_message_signature(&entry.value.signature) { datapoint_info!( - "cluster_info_crds_message_signatures", + "gossip_crds_sample", ( - "crds_origin", + "origin", entry.value.pubkey().to_string().get(..8), Option ), ( - "crds_signature", + "signature", entry.value.signature.to_string().get(..8), Option )