From ccfe6b30daba6a285e4f2ba705a2fc6d8862efe4 Mon Sep 17 00:00:00 2001 From: greg Date: Thu, 3 Aug 2023 16:50:39 -0700 Subject: [PATCH 1/8] we only want to report received message signatures on PUSH requests, not PULL requests --- gossip/src/crds.rs | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index c19eb9b4d5c335..42200447873694 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -669,7 +669,7 @@ impl Default for CrdsDataStats { } impl CrdsDataStats { - fn record_insert(&mut self, entry: &VersionedCrdsValue) { + fn record_insert(&mut self, entry: &VersionedCrdsValue, route: GossipRoute) { self.counts[Self::ordinal(entry)] += 1; if let CrdsData::Vote(_, vote) = &entry.value.data { if let Some(slot) = vote.slot() { @@ -678,20 +678,25 @@ impl CrdsDataStats { } } - if should_report_message_signature(&entry.value.signature) { - datapoint_info!( - "gossip_crds_sample", - ( - "origin", - entry.value.pubkey().to_string().get(..8), - Option - ), - ( - "signature", - entry.value.signature.to_string().get(..8), - Option - ) - ); + match route { + GossipRoute::LocalMessage => { + if should_report_message_signature(&entry.value.signature) { + datapoint_info!( + "gossip_crds_sample", + ( + "origin", + entry.value.pubkey().to_string().get(..8), + Option + ), + ( + "signature", + entry.value.signature.to_string().get(..8), + Option + ) + ); + } + } + _ => {} } } @@ -723,8 +728,8 @@ impl CrdsStats { match route { GossipRoute::LocalMessage => (), GossipRoute::PullRequest => (), - GossipRoute::PushMessage => self.push.record_insert(entry), - GossipRoute::PullResponse => self.pull.record_insert(entry), + GossipRoute::PushMessage => self.push.record_insert(entry, route), + GossipRoute::PullResponse => self.pull.record_insert(entry, route), } } From 5035edf45bb9a7e46d33fafda30ceaa2025795bb Mon Sep 17 00:00:00 2001 From: greg Date: Thu, 3 Aug 2023 16:55:00 -0700 Subject: [PATCH 2/8] woops accidently had it has LocalMessage not PushMessage --- 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 42200447873694..cab5cf4ce95075 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -679,7 +679,7 @@ impl CrdsDataStats { } match route { - GossipRoute::LocalMessage => { + GossipRoute::PushMessage => { if should_report_message_signature(&entry.value.signature) { datapoint_info!( "gossip_crds_sample", From a8c305c1f6b33e00b9ef28a69e9a204c7980e6fd Mon Sep 17 00:00:00 2001 From: greg Date: Thu, 3 Aug 2023 17:19:46 -0700 Subject: [PATCH 3/8] switch from match to if let statement --- gossip/src/crds.rs | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index cab5cf4ce95075..08a205e5aa8de5 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -678,25 +678,22 @@ impl CrdsDataStats { } } - match route { - GossipRoute::PushMessage => { - if should_report_message_signature(&entry.value.signature) { - datapoint_info!( - "gossip_crds_sample", - ( - "origin", - entry.value.pubkey().to_string().get(..8), - Option - ), - ( - "signature", - entry.value.signature.to_string().get(..8), - Option - ) - ); - } + if let GossipRoute::PushMessage = route { + if should_report_message_signature(&entry.value.signature) { + datapoint_info!( + "gossip_crds_sample", + ( + "origin", + entry.value.pubkey().to_string().get(..8), + Option + ), + ( + "signature", + entry.value.signature.to_string().get(..8), + Option + ) + ); } - _ => {} } } From 9c784bc5d1897eac853b6bf6015bd351068ea15d Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 4 Aug 2023 14:32:30 -0700 Subject: [PATCH 4/8] convert if let to matches macro --- gossip/src/crds.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 08a205e5aa8de5..1dc7f9509df941 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -678,22 +678,22 @@ impl CrdsDataStats { } } - if let GossipRoute::PushMessage = route { - if should_report_message_signature(&entry.value.signature) { - datapoint_info!( - "gossip_crds_sample", - ( - "origin", - entry.value.pubkey().to_string().get(..8), - Option - ), - ( - "signature", - entry.value.signature.to_string().get(..8), - Option - ) - ); - } + if matches!(route, GossipRoute::PushMessage) + && should_report_message_signature(&entry.value.signature) + { + datapoint_info!( + "gossip_crds_sample", + ( + "origin", + entry.value.pubkey().to_string().get(..8), + Option + ), + ( + "signature", + entry.value.signature.to_string().get(..8), + Option + ) + ); } } From 49dbacf5fd482013c1490f0940bedf5831f6dc97 Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 4 Aug 2023 16:46:56 -0700 Subject: [PATCH 5/8] add in from field in PushMessage for message tracking --- gossip/src/crds.rs | 24 +++++++++++++++++++----- gossip/src/crds_gossip_push.rs | 2 +- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 1dc7f9509df941..a8711c58fc68e8 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -100,7 +100,16 @@ pub enum GossipRoute { LocalMessage, PullRequest, PullResponse, - PushMessage, + PushMessage(/*from:*/ Pubkey), +} + +impl std::fmt::Display for GossipRoute { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + GossipRoute::PushMessage(pubkey) => write!(f, "{:?}", pubkey), + _ => Ok(()) + } + } } type CrdsCountsArray = [usize; 12]; @@ -301,7 +310,7 @@ impl Crds { if entry.get().value_hash != value.value_hash { self.purged.push_back((value.value_hash, now)); Err(CrdsError::InsertFailed) - } else if matches!(route, GossipRoute::PushMessage) { + } else if matches!(route, GossipRoute::PushMessage(_)) { let entry = entry.get_mut(); entry.num_push_dups = entry.num_push_dups.saturating_add(1); Err(CrdsError::DuplicatePush(entry.num_push_dups)) @@ -678,7 +687,7 @@ impl CrdsDataStats { } } - if matches!(route, GossipRoute::PushMessage) + if matches!(route, GossipRoute::PushMessage(_)) && should_report_message_signature(&entry.value.signature) { datapoint_info!( @@ -692,6 +701,11 @@ impl CrdsDataStats { "signature", entry.value.signature.to_string().get(..8), Option + ), + ( + "from", + route.to_string().get(..8), + Option ) ); } @@ -725,7 +739,7 @@ impl CrdsStats { match route { GossipRoute::LocalMessage => (), GossipRoute::PullRequest => (), - GossipRoute::PushMessage => self.push.record_insert(entry, route), + GossipRoute::PushMessage(_) => self.push.record_insert(entry, route), GossipRoute::PullResponse => self.pull.record_insert(entry, route), } } @@ -734,7 +748,7 @@ impl CrdsStats { match route { GossipRoute::LocalMessage => (), GossipRoute::PullRequest => (), - GossipRoute::PushMessage => self.push.record_fail(entry), + GossipRoute::PushMessage(_) => self.push.record_fail(entry), GossipRoute::PullResponse => self.pull.record_fail(entry), } } diff --git a/gossip/src/crds_gossip_push.rs b/gossip/src/crds_gossip_push.rs index 4472576031cb9d..b97aa37807ca5f 100644 --- a/gossip/src/crds_gossip_push.rs +++ b/gossip/src/crds_gossip_push.rs @@ -144,7 +144,7 @@ impl CrdsGossipPush { continue; } let origin = value.pubkey(); - match crds.insert(value, now, GossipRoute::PushMessage) { + match crds.insert(value, now, GossipRoute::PushMessage(from)) { Ok(()) => { received_cache.record(origin, from, /*num_dups:*/ 0); origins.insert(origin); From bf665562fb4280eb27317ee4bc2d6b44d45031b3 Mon Sep 17 00:00:00 2001 From: greg Date: Fri, 4 Aug 2023 17:05:54 -0700 Subject: [PATCH 6/8] update with cargo fmt --- 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 a8711c58fc68e8..5054e8fca0db49 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -107,7 +107,7 @@ impl std::fmt::Display for GossipRoute { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { GossipRoute::PushMessage(pubkey) => write!(f, "{:?}", pubkey), - _ => Ok(()) + _ => Ok(()), } } } From f4e42dbb30060ec9c591054d5484d6d101576ce3 Mon Sep 17 00:00:00 2001 From: greg Date: Mon, 7 Aug 2023 10:05:31 -0700 Subject: [PATCH 7/8] remove display for gossip route and add lifetime param to pubkey reference in gossiproute enum --- gossip/src/crds.rs | 23 ++++++++--------------- gossip/src/crds_gossip_push.rs | 2 +- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/gossip/src/crds.rs b/gossip/src/crds.rs index 5054e8fca0db49..3b7a1d4413313c 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -96,20 +96,11 @@ pub enum CrdsError { } #[derive(Clone, Copy)] -pub enum GossipRoute { +pub enum GossipRoute<'a> { LocalMessage, PullRequest, PullResponse, - PushMessage(/*from:*/ Pubkey), -} - -impl std::fmt::Display for GossipRoute { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - GossipRoute::PushMessage(pubkey) => write!(f, "{:?}", pubkey), - _ => Ok(()), - } - } + PushMessage(/*from:*/&'a Pubkey), } type CrdsCountsArray = [usize; 12]; @@ -687,9 +678,11 @@ impl CrdsDataStats { } } - if matches!(route, GossipRoute::PushMessage(_)) - && should_report_message_signature(&entry.value.signature) - { + let GossipRoute::PushMessage(from) = route else { + return; + }; + + if should_report_message_signature(&entry.value.signature) { datapoint_info!( "gossip_crds_sample", ( @@ -704,7 +697,7 @@ impl CrdsDataStats { ), ( "from", - route.to_string().get(..8), + from.to_string().get(..8), Option ) ); diff --git a/gossip/src/crds_gossip_push.rs b/gossip/src/crds_gossip_push.rs index b97aa37807ca5f..345c9eaf17287f 100644 --- a/gossip/src/crds_gossip_push.rs +++ b/gossip/src/crds_gossip_push.rs @@ -144,7 +144,7 @@ impl CrdsGossipPush { continue; } let origin = value.pubkey(); - match crds.insert(value, now, GossipRoute::PushMessage(from)) { + match crds.insert(value, now, GossipRoute::PushMessage(&from)) { Ok(()) => { received_cache.record(origin, from, /*num_dups:*/ 0); origins.insert(origin); From 9813294ba4e86fc6172d20db5bc2be989f1a2858 Mon Sep 17 00:00:00 2001 From: greg Date: Mon, 7 Aug 2023 10:21:15 -0700 Subject: [PATCH 8/8] forgot to run fmt --- 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 3b7a1d4413313c..b98c025351e2dc 100644 --- a/gossip/src/crds.rs +++ b/gossip/src/crds.rs @@ -100,7 +100,7 @@ pub enum GossipRoute<'a> { LocalMessage, PullRequest, PullResponse, - PushMessage(/*from:*/&'a Pubkey), + PushMessage(/*from:*/ &'a Pubkey), } type CrdsCountsArray = [usize; 12];