From d6ec03f13c7a7e4adc59041e13700a4034cb07e8 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Sun, 13 Sep 2020 13:08:25 +0000 Subject: [PATCH] patches default impl for crds filter (#12199) In CrdsFilter.mask all bits after mask_bits are set to 1: https://github.com/solana-labs/solana/blob/555252f4/core/src/crds_gossip_pull.rs#L65 However the default implementation, sets both mask and mask_bits to zero which is inconsistent with CrdsFilter::compute_mask for a mask_bits of zero. This commit changes the default implementation by setting mask to `!0u64` (i.e all bits set to one). As a result, for the default crds filter, `test_mask` will always return true, whereas previously it was always returning false. https://github.com/solana-labs/solana/blob/555252f4/core/src/crds_gossip_pull.rs#L85 This is only used in tests and benchmarks, but causes some benchmarks to be misleading by short circuiting in this line: https://github.com/solana-labs/solana/blob/555252f4/core/src/crds_gossip_pull.rs#L429 --- core/src/crds_gossip_pull.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/core/src/crds_gossip_pull.rs b/core/src/crds_gossip_pull.rs index 946fee1a1a..9dea319a1f 100644 --- a/core/src/crds_gossip_pull.rs +++ b/core/src/crds_gossip_pull.rs @@ -30,13 +30,23 @@ pub const CRDS_GOSSIP_PULL_MSG_TIMEOUT_MS: u64 = 60000; pub const FALSE_RATE: f64 = 0.1f64; pub const KEYS: f64 = 8f64; -#[derive(Serialize, Deserialize, Default, Clone, Debug, PartialEq, AbiExample)] +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, AbiExample)] pub struct CrdsFilter { pub filter: Bloom, mask: u64, mask_bits: u32, } +impl Default for CrdsFilter { + fn default() -> Self { + CrdsFilter { + filter: Bloom::default(), + mask: !0u64, + mask_bits: 0u32, + } + } +} + impl solana_sdk::sanitize::Sanitize for CrdsFilter { fn sanitize(&self) -> std::result::Result<(), solana_sdk::sanitize::SanitizeError> { self.filter.sanitize()?; @@ -565,6 +575,20 @@ mod test { } } + #[test] + fn test_crds_filter_default() { + let filter = CrdsFilter::default(); + let mask = CrdsFilter::compute_mask(0, filter.mask_bits); + assert_eq!(filter.mask, mask); + let mut rng = thread_rng(); + for _ in 0..10 { + let mut buf = [0u8; HASH_BYTES]; + rng.fill(&mut buf); + let hash = Hash::new(&buf); + assert!(filter.test_mask(&hash)); + } + } + #[test] fn test_new_pull_with_stakes() { let mut crds = Crds::default();