[Tests] Improve coverage for merkleblock and bloom code

src/Makefile.bench.include

@@ -31,6 +31,7 @@ bench_bench_pivx_SOURCES = \
   bench/perf.cpp \
   bench/perf.h \
   bench/prevector.cpp \
+  bench/rollingbloom.cpp \
   bench/util_time.cpp \
   bench/walletprocessblock.cpp
 

src/Makefile.test.include

@@ -98,6 +98,7 @@ BITCOIN_TESTS =\
   test/base64_tests.cpp \
   test/bech32_tests.cpp \
   test/bip32_tests.cpp \
+  test/bloom_tests.cpp \
   test/bls_tests.cpp \
   test/budget_tests.cpp \
   test/checkblock_tests.cpp \
@@ -121,6 +122,7 @@ BITCOIN_TESTS =\
   test/mnpayments_tests.cpp \
   test/mempool_tests.cpp \
   test/merkle_tests.cpp \
+  test/merkleblock_tests.cpp \
   test/multisig_tests.cpp \
   test/miner_tests.cpp \
   test/net_tests.cpp \

src/bench/CMakeLists.txt

@@ -53,6 +53,7 @@ set(BITCOIN_BENCH_SUITE
         ${CMAKE_CURRENT_SOURCE_DIR}/perf.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/perf.h
         ${CMAKE_CURRENT_SOURCE_DIR}/prevector.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/rollingbloom.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/util_time.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/walletprocessblock.cpp
         )

src/bench/rollingbloom.cpp

@@ -0,0 +1,43 @@
+// Copyright (c) 2016 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <iostream>
+
+#include "bench.h"
+#include "bloom.h"
+#include "utiltime.h"
+
+static void RollingBloom(benchmark::State& state)
+{
+    CRollingBloomFilter filter(120000, 0.000001);
+    std::vector<unsigned char> data(32);
+    uint32_t count = 0;
+    uint32_t nEntriesPerGeneration = (120000 + 1) / 2;
+    uint32_t countnow = 0;
+    uint64_t match = 0;
+    while (state.KeepRunning()) {
+        count++;
+        data[0] = count;
+        data[1] = count >> 8;
+        data[2] = count >> 16;
+        data[3] = count >> 24;
+        if (countnow == nEntriesPerGeneration) {
+            int64_t b = GetTimeMicros();
+            filter.insert(data);
+            int64_t e = GetTimeMicros();
+            std::cout << "RollingBloom-refresh,1," << (e-b)*0.000001 << "," << (e-b)*0.000001 << "," << (e-b)*0.000001 << "\n";
+            countnow = 0;
+        } else {
+            filter.insert(data);
+        }
+        countnow++;
+        data[0] = count >> 24;
+        data[1] = count >> 16;
+        data[2] = count >> 8;
+        data[3] = count;
+        match += filter.contains(data);
+    }
+}
+
+BENCHMARK(RollingBloom, 1500 * 1000);

src/bloom.cpp

@@ -250,14 +250,26 @@ CRollingBloomFilter::CRollingBloomFilter(unsigned int nElements, double fpRate)
      */
     uint32_t nFilterBits = (uint32_t)ceil(-1.0 * nHashFuncs * nMaxElements / log(1.0 - exp(logFpRate / nHashFuncs)));
     data.clear();
-    /* We store up to 16 'bits' per data element. */
-    data.resize((nFilterBits + 15) / 16);
+    /* For each data element we need to store 2 bits. If both bits are 0, the
+     * bit is treated as unset. If the bits are (01), (10), or (11), the bit is
+     * treated as set in generation 1, 2, or 3 respectively.
+     * These bits are stored in separate integers: position P corresponds to bit
+     * (P & 63) of the integers data[(P >> 6) * 2] and data[(P >> 6) * 2 + 1]. */
+    data.resize(((nFilterBits + 63) / 64) << 1);
     reset();
 }
 
 /* Similar to CBloomFilter::Hash */
-inline unsigned int CRollingBloomFilter::Hash(unsigned int nHashNum, const std::vector<unsigned char>& vDataToHash) const {
-    return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash) % (data.size() * 16);
+static inline uint32_t RollingBloomHash(unsigned int nHashNum, uint32_t nTweak, const std::vector<unsigned char>& vDataToHash) {
+    return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash);
+}
+
+
+// A replacement for x % n. This assumes that x and n are 32bit integers, and x is a uniformly random distributed 32bit value
+// which should be the case for a good hash.
+// See https://lemire.me/blog/2016/06/27/a-fast-alternative-to-the-modulo-reduction/
+static inline uint32_t FastMod(uint32_t x, size_t n) {
+    return ((uint64_t)x * (uint64_t)n) >> 32;
 }
 
 void CRollingBloomFilter::insert(const std::vector<unsigned char>& vKey)
@@ -268,18 +280,26 @@ void CRollingBloomFilter::insert(const std::vector<unsigned char>& vKey)
         if (nGeneration == 4) {
             nGeneration = 1;
         }
+        uint64_t nGenerationMask1 = -(uint64_t)(nGeneration & 1);
+        uint64_t nGenerationMask2 = -(uint64_t)(nGeneration >> 1);
         /* Wipe old entries that used this generation number. */
-        for (uint32_t p = 0; p < data.size() * 16; p++) {
-            if (get(p) == nGeneration) {
-                put(p, 0);
-            }
+        for (uint32_t p = 0; p < data.size(); p += 2) {
+            uint64_t p1 = data[p], p2 = data[p + 1];
+            uint64_t mask = (p1 ^ nGenerationMask1) | (p2 ^ nGenerationMask2);
+            data[p] = p1 & mask;
+            data[p + 1] = p2 & mask;
         }
     }
     nEntriesThisGeneration++;
 
     for (int n = 0; n < nHashFuncs; n++) {
-        uint32_t h = Hash(n, vKey);
-        put(h, nGeneration);
+        uint32_t h = RollingBloomHash(n, nTweak, vKey);
+        int bit = h & 0x3F;
+        /* FastMod works with the upper bits of h, so it is safe to ignore that the lower bits of h are already used for bit. */
+        uint32_t pos = FastMod(h, data.size());
+        /* The lowest bit of pos is ignored, and set to zero for the first bit, and to one for the second. */
+        data[pos & ~1] = (data[pos & ~1] & ~(((uint64_t)1) << bit)) | ((uint64_t)(nGeneration & 1)) << bit;
+        data[pos | 1] = (data[pos | 1] & ~(((uint64_t)1) << bit)) | ((uint64_t)(nGeneration >> 1)) << bit;
     }
 }
 
@@ -292,8 +312,11 @@ void CRollingBloomFilter::insert(const uint256& hash)
 bool CRollingBloomFilter::contains(const std::vector<unsigned char>& vKey) const
 {
     for (int n = 0; n < nHashFuncs; n++) {
-        uint32_t h = Hash(n, vKey);
-        if (get(h) == 0) {
+        uint32_t h = RollingBloomHash(n, nTweak, vKey);
+        int bit = h & 0x3F;
+        uint32_t pos = FastMod(h, data.size());
+        /* If the relevant bit is not set in either data[pos & ~1] or data[pos | 1], the filter does not contain vKey */
+        if (!(((data[pos & ~1] | data[pos | 1]) >> bit) & 1)) {
             return false;
         }
     }
@@ -311,7 +334,7 @@ void CRollingBloomFilter::reset()
     nTweak = GetRand(std::numeric_limits<unsigned int>::max());
     nEntriesThisGeneration = 0;
     nGeneration = 1;
-    for (std::vector<uint32_t>::iterator it = data.begin(); it != data.end(); it++) {
+    for (std::vector<uint64_t>::iterator it = data.begin(); it != data.end(); it++) {
         *it = 0;
     }
 }

src/bloom.h

@@ -126,20 +126,9 @@ class CRollingBloomFilter
     int nEntriesPerGeneration;
     int nEntriesThisGeneration;
     int nGeneration;
-    std::vector<uint32_t> data;
+    std::vector<uint64_t> data;
     unsigned int nTweak;
     int nHashFuncs;
-
-    unsigned int Hash(unsigned int nHashNum, const std::vector<unsigned char>& vDataToHash) const;
-
-    inline int get(uint32_t position) const {
-        return (data[(position >> 4) % data.size()] >> (2 * (position & 0xF))) & 0x3;
-    }
-
-    inline void put(uint32_t position, uint32_t val) {
-        uint32_t& cell = data[(position >> 4) % data.size()];
-        cell = (cell & ~(((uint32_t)3) << (2 * (position & 0xF)))) | (val << (2 * (position & 0xF)));
-    }
 };
 
 #endif // BITCOIN_BLOOM_H

src/merkleblock.cpp

@@ -1,5 +1,5 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
-// Copyright (c) 2009-2014 The Bitcoin developers
+// Copyright (c) 2009-2016 The Bitcoin developers
 // Copyright (c) 2015-2020 The PIVX developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -30,7 +30,7 @@ std::vector<bool> BytesToBits(const std::vector<unsigned char>& bytes)
     return ret;
 }
 
-CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter& filter)
+CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std::set<uint256>* txids)
 {
     header = block.GetBlockHeader();
 
@@ -42,11 +42,14 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter& filter)
 
     for (unsigned int i = 0; i < block.vtx.size(); i++) {
         const uint256& hash = block.vtx[i]->GetHash();
-        if (filter.IsRelevantAndUpdate(*block.vtx[i])) {
+        if (txids && txids->count(hash)) {
+            vMatch.push_back(true);
+        } else if (filter && filter->IsRelevantAndUpdate(*block.vtx[i])) {
             vMatch.push_back(true);
             vMatchedTxn.emplace_back(i, hash);
-        } else
+        } else {
             vMatch.push_back(false);
+        }
         vHashes.push_back(hash);
     }
 

src/merkleblock.h

@@ -1,5 +1,5 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
-// Copyright (c) 2009-2014 The Bitcoin developers
+// Copyright (c) 2009-2016 The Bitcoin developers
 // Copyright (c) 2017-2019 The PIVX developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -121,21 +121,32 @@ class CMerkleBlock
     CBlockHeader header;
     CPartialMerkleTree txn;
 
-public:
-    /** Public only for unit testing and relay testing (not relayed) */
+    /**
+     * Public only for unit testing and relay testing (not relayed).
+     *
+     * Used only when a bloom filter is specified to allow
+     * testing the transactions which matched the bloom filter.
+     */
     std::vector<std::pair<unsigned int, uint256> > vMatchedTxn;
 
     /**
      * Create from a CBlock, filtering transactions according to filter
      * Note that this will call IsRelevantAndUpdate on the filter for each transaction,
      * thus the filter will likely be modified.
      */
-    CMerkleBlock(const CBlock& block, CBloomFilter& filter);
+    CMerkleBlock(const CBlock& block, CBloomFilter& filter) : CMerkleBlock(block, &filter, nullptr) { }
+
+    // Create from a CBlock, matching the txids in the set
+    CMerkleBlock(const CBlock& block, const std::set<uint256>& txids) : CMerkleBlock(block, nullptr, &txids) { }
 
     CMerkleBlock() {}
 
     SERIALIZE_METHODS(CMerkleBlock, obj) { READWRITE(obj.header, obj.txn); }
 
+private:
+    // Combined constructor to consolidate code
+    CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std::set<uint256>* txids);
+
 };
 
 #endif // BITCOIN_MERKLEBLOCK_H

src/test/CMakeLists.txt

@@ -117,6 +117,7 @@ set(BITCOIN_TESTS
         ${CMAKE_CURRENT_SOURCE_DIR}/bech32_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/budget_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/bip32_tests.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/bloom_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/bls_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/checkblock_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/Checkpoints_tests.cpp
@@ -138,6 +139,7 @@ set(BITCOIN_TESTS
         ${CMAKE_CURRENT_SOURCE_DIR}/mnpayments_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/mempool_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/merkle_tests.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/merkleblock_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/miner_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/multisig_tests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/net_tests.cpp

src/test/DoS_tests.cpp

@@ -167,7 +167,7 @@ CTransactionRef RandomOrphan()
 static void MakeNewKeyWithFastRandomContext(CKey& key)
 {
     std::vector<unsigned char> keydata;
-    keydata = insecure_rand_ctx.randbytes(32);
+    keydata = g_insecure_rand_ctx.randbytes(32);
     key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn*/ true);
     assert(key.IsValid());
 }
@@ -180,7 +180,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
     // ecdsa_signature_parse_der_lax are executed during this test.
     // Specifically branches that run only when an ECDSA
     // signature's R and S values have leading zeros.
-    insecure_rand_ctx = FastRandomContext(ArithToUint256(arith_uint256(33)));
+    g_insecure_rand_ctx = FastRandomContext(ArithToUint256(arith_uint256(33)));
 
     CKey key;
     MakeNewKeyWithFastRandomContext(key);

src/test/base58_tests.cpp

@@ -272,7 +272,7 @@ BOOST_AUTO_TEST_CASE(base58_random_encode_decode)
     for (int n = 0; n < 1000; ++n) {
         unsigned int len = 1 + InsecureRandBits(8);
         unsigned int zeroes = InsecureRandBool() ? InsecureRandRange(len + 1) : 0;
-        auto data = Cat(std::vector<unsigned char>(zeroes, '\000'), insecure_rand_ctx.randbytes(len - zeroes));
+        auto data = Cat(std::vector<unsigned char>(zeroes, '\000'), g_insecure_rand_ctx.randbytes(len - zeroes));
         auto encoded = EncodeBase58Check(data);
         std::vector<unsigned char> decoded;
         auto ok_too_small = DecodeBase58Check(encoded, decoded, InsecureRandRange(len));