blockfilter: Refactor GCS params into struct.

4 files changed, 66 insertions, 41 deletions

diff --git a/src/bench/gcs_filter.cpp b/src/bench/gcs_filter.cpp
index 6f4e384e3b..535ad35571 100644
--- a/src/bench/gcs_filter.cpp
+++ b/src/bench/gcs_filter.cpp
@@ -17,7 +17,7 @@ static void ConstructGCSFilter(benchmark::State& state)
 
     uint64_t siphash_k0 = 0;
     while (state.KeepRunning()) {
-        GCSFilter filter(siphash_k0, 0, 20, 1 << 20, elements);
+        GCSFilter filter({siphash_k0, 0, 20, 1 << 20}, elements);
 
         siphash_k0++;
     }
@@ -32,7 +32,7 @@ static void MatchGCSFilter(benchmark::State& state)
         element[1] = static_cast<unsigned char>(i >> 8);
         elements.insert(std::move(element));
     }
-    GCSFilter filter(0, 0, 20, 1 << 20, elements);
+    GCSFilter filter({0, 0, 20, 1 << 20}, elements);
 
     while (state.KeepRunning()) {
         filter.Match(GCSFilter::Element());
diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp
index 163e2a52ef..6bbf887b24 100644
--- a/src/blockfilter.cpp
+++ b/src/blockfilter.cpp
@@ -79,7 +79,7 @@ static uint64_t MapIntoRange(uint64_t x, uint64_t n)
 
 uint64_t GCSFilter::HashToRange(const Element& element) const
 {
-    uint64_t hash = CSipHasher(m_siphash_k0, m_siphash_k1)
+    uint64_t hash = CSipHasher(m_params.m_siphash_k0, m_params.m_siphash_k1)
         .Write(element.data(), element.size())
         .Finalize();
     return MapIntoRange(hash, m_F);
@@ -96,16 +96,13 @@ std::vector<uint64_t> GCSFilter::BuildHashedSet(const ElementSet& elements) cons
     return hashed_elements;
 }
 
-GCSFilter::GCSFilter(uint64_t siphash_k0, uint64_t siphash_k1, uint8_t P, uint32_t M)
-    : m_siphash_k0(siphash_k0), m_siphash_k1(siphash_k1), m_P(P), m_M(M), m_N(0), m_F(0)
+GCSFilter::GCSFilter(const Params& params)
+    : m_params(params), m_N(0), m_F(0), m_encoded{0}
 {}
 
-GCSFilter::GCSFilter(uint64_t siphash_k0, uint64_t siphash_k1, uint8_t P, uint32_t M,
-                     std::vector<unsigned char> encoded_filter)
-    : GCSFilter(siphash_k0, siphash_k1, P, M)
+GCSFilter::GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter)
+    : m_params(params), m_encoded(std::move(encoded_filter))
 {
-    m_encoded = std::move(encoded_filter);
-
     VectorReader stream(GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0);
 
     uint64_t N = ReadCompactSize(stream);
@@ -113,29 +110,28 @@ GCSFilter::GCSFilter(uint64_t siphash_k0, uint64_t siphash_k1, uint8_t P, uint32
     if (m_N != N) {
         throw std::ios_base::failure("N must be <2^32");
     }
-    m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_M);
+    m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M);
 
     // Verify that the encoded filter contains exactly N elements. If it has too much or too little
     // data, a std::ios_base::failure exception will be raised.
     BitStreamReader<VectorReader> bitreader(stream);
     for (uint64_t i = 0; i < m_N; ++i) {
-        GolombRiceDecode(bitreader, m_P);
+        GolombRiceDecode(bitreader, m_params.m_P);
     }
     if (!stream.empty()) {
         throw std::ios_base::failure("encoded_filter contains excess data");
     }
 }
 
-GCSFilter::GCSFilter(uint64_t siphash_k0, uint64_t siphash_k1, uint8_t P, uint32_t M,
-                     const ElementSet& elements)
-    : GCSFilter(siphash_k0, siphash_k1, P, M)
+GCSFilter::GCSFilter(const Params& params, const ElementSet& elements)
+    : m_params(params)
 {
     size_t N = elements.size();
     m_N = static_cast<uint32_t>(N);
     if (m_N != N) {
         throw std::invalid_argument("N must be <2^32");
     }
-    m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_M);
+    m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M);
 
     CVectorWriter stream(GCS_SER_TYPE, GCS_SER_VERSION, m_encoded, 0);
 
@@ -150,7 +146,7 @@ GCSFilter::GCSFilter(uint64_t siphash_k0, uint64_t siphash_k1, uint8_t P, uint32
     uint64_t last_value = 0;
     for (uint64_t value : BuildHashedSet(elements)) {
         uint64_t delta = value - last_value;
-        GolombRiceEncode(bitwriter, m_P, delta);
+        GolombRiceEncode(bitwriter, m_params.m_P, delta);
         last_value = value;
     }
 
@@ -170,7 +166,7 @@ bool GCSFilter::MatchInternal(const uint64_t* element_hashes, size_t size) const
     uint64_t value = 0;
     size_t hashes_index = 0;
     for (uint32_t i = 0; i < m_N; ++i) {
-        uint64_t delta = GolombRiceDecode(bitreader, m_P);
+        uint64_t delta = GolombRiceDecode(bitreader, m_params.m_P);
         value += delta;
 
         while (true) {
@@ -228,16 +224,28 @@ static GCSFilter::ElementSet BasicFilterElements(const CBlock& block,
 BlockFilter::BlockFilter(BlockFilterType filter_type, const CBlock& block, const CBlockUndo& block_undo)
     : m_filter_type(filter_type), m_block_hash(block.GetHash())
 {
+    GCSFilter::Params params;
+    if (!BuildParams(params)) {
+        throw std::invalid_argument("unknown filter_type");
+    }
+    m_filter = GCSFilter(params, BasicFilterElements(block, block_undo));
+}
+
+bool BlockFilter::BuildParams(GCSFilter::Params& params) const
+{
     switch (m_filter_type) {
     case BlockFilterType::BASIC:
-        m_filter = GCSFilter(m_block_hash.GetUint64(0), m_block_hash.GetUint64(1),
-                             BASIC_FILTER_P, BASIC_FILTER_M,
-                             BasicFilterElements(block, block_undo));
+        params.m_siphash_k0 = m_block_hash.GetUint64(0);
+        params.m_siphash_k1 = m_block_hash.GetUint64(1);
+        params.m_P = BASIC_FILTER_P;
+        params.m_M = BASIC_FILTER_M;
         break;
 
     default:
-        throw std::invalid_argument("unknown filter_type");
+        return false;
     }
+
+    return true;
 }
 
 uint256 BlockFilter::GetHash() const
diff --git a/src/blockfilter.h b/src/blockfilter.h
index 871be11769..e53f813ab3 100644
--- a/src/blockfilter.h
+++ b/src/blockfilter.h
@@ -25,11 +25,20 @@ public:
     typedef std::vector<unsigned char> Element;
     typedef std::unordered_set<Element, ByteVectorHash> ElementSet;
 
+    struct Params
+    {
+        uint64_t m_siphash_k0;
+        uint64_t m_siphash_k1;
+        uint8_t m_P;  //!< Golomb-Rice coding parameter
+        uint32_t m_M;  //!< Inverse false positive rate
+
+        Params(uint64_t siphash_k0 = 0, uint64_t siphash_k1 = 0, uint8_t P = 0, uint32_t M = 1)
+            : m_siphash_k0(siphash_k0), m_siphash_k1(siphash_k1), m_P(P), m_M(M)
+        {}
+    };
+
 private:
-    uint64_t m_siphash_k0;
-    uint64_t m_siphash_k1;
-    uint8_t m_P;  //!< Golomb-Rice coding parameter
-    uint32_t m_M;  //!< Inverse false positive rate
+    Params m_params;
     uint32_t m_N;  //!< Number of elements in the filter
     uint64_t m_F;  //!< Range of element hashes, F = N * M
     std::vector<unsigned char> m_encoded;
@@ -45,19 +54,16 @@ private:
 public:
 
     /** Constructs an empty filter. */
-    GCSFilter(uint64_t siphash_k0 = 0, uint64_t siphash_k1 = 0, uint8_t P = 0, uint32_t M = 0);
+    explicit GCSFilter(const Params& params = Params());
 
     /** Reconstructs an already-created filter from an encoding. */
-    GCSFilter(uint64_t siphash_k0, uint64_t siphash_k1, uint8_t P, uint32_t M,
-              std::vector<unsigned char> encoded_filter);
+    GCSFilter(const Params& params, std::vector<unsigned char> encoded_filter);
 
     /** Builds a new filter from the params and set of elements. */
-    GCSFilter(uint64_t siphash_k0, uint64_t siphash_k1, uint8_t P, uint32_t M,
-              const ElementSet& elements);
+    GCSFilter(const Params& params, const ElementSet& elements);
 
-    uint8_t GetP() const { return m_P; }
     uint32_t GetN() const { return m_N; }
-    uint32_t GetM() const { return m_M; }
+    const Params& GetParams() const { return m_params; }
     const std::vector<unsigned char>& GetEncoded() const { return m_encoded; }
 
     /**
@@ -93,6 +99,8 @@ private:
     uint256 m_block_hash;
     GCSFilter m_filter;
 
+    bool BuildParams(GCSFilter::Params& params) const;
+
 public:
 
     // Construct a new BlockFilter of the specified type from a block.
@@ -131,15 +139,11 @@ public:
 
         m_filter_type = static_cast<BlockFilterType>(filter_type);
 
-        switch (m_filter_type) {
-        case BlockFilterType::BASIC:
-            m_filter = GCSFilter(m_block_hash.GetUint64(0), m_block_hash.GetUint64(1),
-                                 BASIC_FILTER_P, BASIC_FILTER_M, std::move(encoded_filter));
-            break;
-
-        default:
+        GCSFilter::Params params;
+        if (!BuildParams(params)) {
             throw std::ios_base::failure("unknown filter_type");
         }
+        m_filter = GCSFilter(params, std::move(encoded_filter));
     }
 };
 
diff --git a/src/test/blockfilter_tests.cpp b/src/test/blockfilter_tests.cpp
index 2144202b8d..69149fad4f 100644
--- a/src/test/blockfilter_tests.cpp
+++ b/src/test/blockfilter_tests.cpp
@@ -29,7 +29,7 @@ BOOST_AUTO_TEST_CASE(gcsfilter_test)
         excluded_elements.insert(std::move(element2));
     }
 
-    GCSFilter filter(0, 0, 10, 1 << 10, included_elements);
+    GCSFilter filter({0, 0, 10, 1 << 10}, included_elements);
     for (const auto& element : included_elements) {
         BOOST_CHECK(filter.Match(element));
 
@@ -39,6 +39,19 @@ BOOST_AUTO_TEST_CASE(gcsfilter_test)
     }
 }
 
+BOOST_AUTO_TEST_CASE(gcsfilter_default_constructor)
+{
+    GCSFilter filter;
+    BOOST_CHECK_EQUAL(filter.GetN(), 0);
+    BOOST_CHECK_EQUAL(filter.GetEncoded().size(), 1);
+
+    const GCSFilter::Params& params = filter.GetParams();
+    BOOST_CHECK_EQUAL(params.m_siphash_k0, 0);
+    BOOST_CHECK_EQUAL(params.m_siphash_k1, 0);
+    BOOST_CHECK_EQUAL(params.m_P, 0);
+    BOOST_CHECK_EQUAL(params.m_M, 1);
+}
+
 BOOST_AUTO_TEST_CASE(blockfilter_basic_test)
 {
     CScript included_scripts[5], excluded_scripts[3];