refactor: centralize seed hash validation

datasketches/src/countmin/mod.rs

@@ -15,9 +15,9 @@
 // specific language governing permissions and limitations
 // under the License.
 
-//! Count-Min sketch implementation for frequency estimation.
+//! CountMin sketch implementation for frequency estimation.
 //!
-//! The Count-Min sketch provides approximate frequency counts for streaming data
+//! The CountMin sketch provides approximate frequency counts for streaming data
 //! with configurable relative error and confidence bounds.
 //!
 //! # Usage

datasketches/src/countmin/sketch.rs

@@ -31,13 +31,15 @@ use crate::countmin::serialization::LONG_SIZE_BYTES;
 use crate::countmin::serialization::PREAMBLE_LONGS_SHORT;
 use crate::countmin::serialization::SERIAL_VERSION;
 use crate::error::Error;
+use crate::error::ErrorKind;
 use crate::hash::DEFAULT_UPDATE_SEED;
 use crate::hash::MurmurHash3X64128;
+use crate::hash::check_seed_hash;
 use crate::hash::compute_seed_hash;
 
 const MAX_TABLE_ENTRIES: usize = 1 << 30;
 
-/// Count-Min sketch for estimating item frequencies.
+/// CountMin sketch for estimating item frequencies.
 ///
 /// The sketch provides upper and lower bounds on estimated item frequencies
 /// with configurable relative error and confidence.
@@ -53,7 +55,7 @@ pub struct CountMinSketch<T: CountMinValue> {
 }
 
 impl<T: CountMinValue> CountMinSketch<T> {
-    /// Creates a new Count-Min sketch with the default seed.
+    /// Creates a new CountMin sketch with the default seed.
     ///
     /// # Panics
     ///
@@ -72,7 +74,7 @@ impl<T: CountMinValue> CountMinSketch<T> {
         Self::with_seed(num_hashes, num_buckets, DEFAULT_UPDATE_SEED)
     }
 
-    /// Creates a new Count-Min sketch with the provided seed.
+    /// Creates a new CountMin sketch with the provided seed.
     ///
     /// # Panics
     ///
@@ -264,7 +266,7 @@ impl<T: CountMinValue> CountMinSketch<T> {
         self.total_weight = self.total_weight + other.total_weight;
     }
 
-    /// Serializes this sketch into the DataSketches Count-Min format.
+    /// Serializes this sketch into the DataSketches CountMin format.
     ///
     /// # Examples
     ///
@@ -376,12 +378,12 @@ impl<T: CountMinValue> CountMinSketch<T> {
             .map_err(insufficient_data("seed_hash"))?;
         cursor.read_u8().map_err(insufficient_data("unused8"))?;
 
-        let expected_seed_hash = compute_seed_hash(seed);
-        if seed_hash != expected_seed_hash {
-            return Err(Error::deserial(format!(
-                "incompatible seed hash: expected {expected_seed_hash}, got {seed_hash}",
-            )));
-        }
+        check_seed_hash(
+            compute_seed_hash(seed),
+            seed_hash,
+            "deserialized CountMinSketch",
+            ErrorKind::InvalidData,
+        )?;
 
         let entries = entries_for_config_checked(num_hashes, num_buckets)?;
         let mut sketch = Self::make(num_hashes, num_buckets, seed, entries);

datasketches/src/cpc/sketch.rs

@@ -48,6 +48,7 @@ use crate::error::Error;
 use crate::error::ErrorKind;
 use crate::hash::DEFAULT_UPDATE_SEED;
 use crate::hash::MurmurHash3X64128;
+use crate::hash::check_seed_hash;
 use crate::hash::compute_seed_hash;
 
 /// A Compressed Probabilistic Counting sketch.
@@ -625,16 +626,12 @@ impl CpcSketch {
         let expected_preamble_ints =
             make_preamble_ints(num_coupons, has_hip, has_table, has_window);
         ensure_preamble_longs_in(&[expected_preamble_ints], preamble_ints)?;
-        if seed_hash != compute_seed_hash(seed) {
-            return Err(Error::new(
-                ErrorKind::InvalidData,
-                format!(
-                    "incompatible seed hash: expected {}, got {}",
-                    compute_seed_hash(seed),
-                    seed_hash
-                ),
-            ));
-        }
+        check_seed_hash(
+            compute_seed_hash(seed),
+            seed_hash,
+            "deserialized CpcSketch",
+            ErrorKind::InvalidData,
+        )?;
         if !(MIN_LG_K..=MAX_LG_K).contains(&lg_k) {
             return Err(Error::invalid_argument(format!(
                 "lg_k out of range; got {}",

datasketches/src/error.rs

@@ -116,7 +116,7 @@ impl Error {
 
     pub(crate) fn invalid_preamble_longs(expected: &[u8], actual: u8) -> Self {
         Error::deserial(format!(
-            "invalid preamble longs: expected {expected:?}, got {actual}"
+            "invalid preamble longs: expected one of {expected:?}, got {actual}"
         ))
     }
 }

datasketches/src/hash/mod.rs

@@ -36,12 +36,27 @@ mod murmurhash;
     feature = "theta",
     feature = "tuple",
 ))]
-pub(crate) use self::murmurhash::MurmurHash3X64128;
+pub(crate) use self::murmurhash::*;
 
 #[cfg(feature = "bloom")]
 mod xxhash;
 #[cfg(feature = "bloom")]
-pub(crate) use self::xxhash::XxHash64;
+pub(crate) use self::xxhash::*;
+
+#[cfg(any(
+    feature = "countmin",
+    feature = "cpc",
+    feature = "theta",
+    feature = "tuple",
+))]
+mod seed;
+#[cfg(any(
+    feature = "countmin",
+    feature = "cpc",
+    feature = "theta",
+    feature = "tuple",
+))]
+pub(crate) use self::seed::*;
 
 /// The seed 9001 used in the sketch update methods is a prime number that was chosen very early
 /// on in experimental testing.
@@ -67,31 +82,6 @@ pub(crate) use self::xxhash::XxHash64;
 ))]
 pub(crate) const DEFAULT_UPDATE_SEED: u64 = 9001;
 
-/// Computes and checks the 16-bit seed hash from the given long seed.
-///
-/// The computed seed hash must not be zero in order to maintain compatibility with older
-/// serialized versions that did not have this concept.
-///
-/// # Panics
-///
-/// Panics if the computed seed hash is zero.
-#[cfg(any(
-    feature = "countmin",
-    feature = "cpc",
-    feature = "theta",
-    feature = "tuple",
-))]
-pub(crate) fn compute_seed_hash(seed: u64) -> u16 {
-    use std::hash::Hasher;
-
-    let mut hasher = MurmurHash3X64128::with_seed(0);
-    hasher.write(&seed.to_le_bytes());
-    let (h1, _) = hasher.finish128();
-    let seed_hash = (h1 & 0xffff) as u16;
-    assert_ne!(seed_hash, 0);
-    seed_hash
-}
-
 /// Reads an u64 from a byte slice in little-endian order.
 ///
 /// # Panics

datasketches/src/hash/seed.rs

@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::error::Error;
+use crate::error::ErrorKind;
+use crate::hash::MurmurHash3X64128;
+
+/// Computes and checks the 16-bit seed hash from the given long seed.
+///
+/// The computed seed hash must not be zero in order to maintain compatibility with older
+/// serialized versions that did not have this concept.
+///
+/// # Panics
+///
+/// Panics if the computed seed hash is zero.
+pub(crate) fn compute_seed_hash(seed: u64) -> u16 {
+    use std::hash::Hasher;
+
+    let mut hasher = MurmurHash3X64128::with_seed(0);
+    hasher.write(&seed.to_le_bytes());
+    let (h1, _) = hasher.finish128();
+    let seed_hash = (h1 & 0xffff) as u16;
+    assert_ne!(seed_hash, 0);
+    seed_hash
+}
+
+/// Checks that an actual seed hash matches the expected seed hash.
+pub(crate) fn check_seed_hash(
+    expected: u16,
+    actual: u16,
+    name: &'static str,
+    kind: ErrorKind,
+) -> Result<(), Error> {
+    if actual != expected {
+        return Err(Error::new(
+            kind,
+            format!("incompatible seed hash of {name}: expected {expected}, got {actual}"),
+        ));
+    }
+    Ok(())
+}

datasketches/src/thetafamily/common/a_not_b.rs

@@ -18,6 +18,8 @@
 use std::collections::HashSet;
 
 use crate::error::Error;
+use crate::error::ErrorKind;
+use crate::hash::check_seed_hash;
 use crate::hash::compute_seed_hash;
 use crate::thetacommon::RetainedEntry;
 use crate::thetacommon::ThetaFamilySketchView;
@@ -70,13 +72,12 @@ impl ANotBOperator {
         }
 
         // A is non-empty, so its seed must be compatible.
-        if a.seed_hash() != self.seed_hash {
-            return Err(Error::invalid_argument(format!(
-                "incompatible seed hash for A: expected {}, got {}",
-                self.seed_hash,
-                a.seed_hash()
-            )));
-        }
+        check_seed_hash(
+            self.seed_hash,
+            a.seed_hash(),
+            "A",
+            ErrorKind::InvalidArgument,
+        )?;
 
         // An empty B subtracts nothing, so the result is simply a copy of A. This also covers the
         // "A is non-empty but has no retained keys" state: B's seed and theta must not influence
@@ -86,13 +87,12 @@ impl ANotBOperator {
         }
 
         // B is non-empty, so its seed must be compatible.
-        if b.seed_hash() != self.seed_hash {
-            return Err(Error::invalid_argument(format!(
-                "incompatible seed hash for B: expected {}, got {}",
-                self.seed_hash,
-                b.seed_hash()
-            )));
-        }
+        check_seed_hash(
+            self.seed_hash,
+            b.seed_hash(),
+            "B",
+            ErrorKind::InvalidArgument,
+        )?;
 
         let theta = a.theta64().min(b.theta64());
         // A is non-empty here; the result only becomes empty if everything is subtracted in exact

datasketches/src/thetafamily/common/intersection.rs

@@ -17,6 +17,8 @@
 
 use crate::common::ResizeFactor;
 use crate::error::Error;
+use crate::error::ErrorKind;
+use crate::hash::check_seed_hash;
 use crate::thetacommon::RetainedEntry;
 use crate::thetacommon::ThetaFamilySketchView;
 use crate::thetacommon::constants::HASH_TABLE_REBUILD_THRESHOLD;
@@ -90,16 +92,15 @@ where
             return Ok(());
         }
 
-        if !sketch.is_empty() && sketch.seed_hash() != self.table.seed_hash() {
-            return Err(Error::invalid_argument(format!(
-                "incompatible seed hash: expected {}, got {}",
-                self.table.seed_hash(),
-                sketch.seed_hash()
-            )));
-        }
-
         if sketch.is_empty() {
             self.table.set_empty(true);
+        } else {
+            check_seed_hash(
+                self.table.seed_hash(),
+                sketch.seed_hash(),
+                "intersection update",
+                ErrorKind::InvalidArgument,
+            )?;
         }
 
         self.table.set_theta(if self.table.is_empty() {
@@ -257,6 +258,7 @@ where
 mod tests {
     use super::*;
     use crate::hash::DEFAULT_UPDATE_SEED;
+    use crate::hash::compute_seed_hash;
     use crate::thetacommon::ThetaKeySketchView;
 
     #[derive(Clone, Debug, Eq, PartialEq)]
@@ -288,7 +290,7 @@ mod tests {
 
     impl ThetaKeySketchView for TestSketch {
         fn seed_hash(&self) -> u16 {
-            crate::hash::compute_seed_hash(DEFAULT_UPDATE_SEED)
+            compute_seed_hash(DEFAULT_UPDATE_SEED)
         }
 
         fn theta64(&self) -> u64 {

datasketches/src/thetafamily/common/jaccard_similarity.rs

@@ -17,6 +17,8 @@
 
 use crate::common::ResizeFactor;
 use crate::error::Error;
+use crate::error::ErrorKind;
+use crate::hash::check_seed_hash;
 use crate::hash::compute_seed_hash;
 use crate::thetacommon::RetainedEntry;
 use crate::thetacommon::ThetaFamilySketchView;
@@ -295,8 +297,9 @@ impl JaccardSimilarityOperator {
         A: ThetaKeySketchView,
         B: ThetaKeySketchView,
     {
-        self.validate_seed_hash(sketch_a)?;
-        self.validate_seed_hash(sketch_b)?;
+        let seed_hash = compute_seed_hash(self.seed);
+        check_seed_hash(seed_hash, sketch_a.seed_hash(), "A", ErrorKind::InvalidData)?;
+        check_seed_hash(seed_hash, sketch_b.seed_hash(), "B", ErrorKind::InvalidData)?;
 
         let sketch_a = KeySketchView::new(sketch_a);
         let sketch_b = KeySketchView::new(sketch_b);
@@ -311,18 +314,6 @@ impl JaccardSimilarityOperator {
         union.update(&sketch_b)?;
         Ok(union.to_compact_parts(false))
     }
-
-    fn validate_seed_hash<S: ThetaKeySketchView>(&self, sketch: &S) -> Result<(), Error> {
-        let expected = compute_seed_hash(self.seed);
-        if expected != sketch.seed_hash() {
-            return Err(Error::invalid_argument(format!(
-                "incompatible seed hash: expected {}, got {}",
-                expected,
-                sketch.seed_hash(),
-            )));
-        }
-        Ok(())
-    }
 }
 
 /// Returns whether both sketches have the same retained keys and theta.