From 9494acdb60cdd4c63e80249fd9a5d9c60d0df230 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 27 Nov 2015 13:20:29 +0100 Subject: [PATCH 01/17] Switch to a more efficient rolling Bloom filter For each 'bit' in the filter we really maintain 2 bits, which store either: 0: not set 1-3: set in generation N After (nElements / 2) insertions, we switch to a new generation, and wipe entries which already had the new generation number, effectively switching from the last 1.5 * nElements set to the last 1.0 * nElements set. This is 25% more space efficient than the previous implementation, and can (at peak) store 1.5 times the requested amount of history (though only 1.0 times the requested history is guaranteed). The existing unit tests should be sufficient. (cherry picked from commit 086ee67d839b33bf475177f680fcc848a0625266) --- src/bloom.cpp | 77 +++++++++++++++++++++++++++++++++++---------------- src/bloom.h | 26 +++++++++++++---- src/main.cpp | 2 +- 3 files changed, 75 insertions(+), 30 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index 0406f8873..932420465 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -215,30 +215,54 @@ void CBloomFilter::UpdateEmptyFull() isEmpty = empty; } -CRollingBloomFilter::CRollingBloomFilter(unsigned int nElements, double fpRate) : - b1(nElements * 2, fpRate, 0), b2(nElements * 2, fpRate, 0) +CRollingBloomFilter::CRollingBloomFilter(unsigned int nElements, double fpRate) { - // Implemented using two bloom filters of 2 * nElements each. - // We fill them up, and clear them, staggered, every nElements - // inserted, so at least one always contains the last nElements - // inserted. - nInsertions = 0; - nBloomSize = nElements * 2; - + double logFpRate = log(fpRate); + /* The optimal number of hash functions is log(fpRate) / log(0.5), but + * restrict it to the range 1-50. */ + nHashFuncs = std::max(1, std::min((int)round(logFpRate / log(0.5)), 50)); + /* In this rolling bloom filter, we'll store between 2 and 3 generations of nElements / 2 entries. */ + nEntriesPerGeneration = (nElements + 1) / 2; + uint32_t nMaxElements = nEntriesPerGeneration * 3; + /* The maximum fpRate = pow(1.0 - exp(-nHashFuncs * nMaxElements / nFilterBits), nHashFuncs) + * => pow(fpRate, 1.0 / nHashFuncs) = 1.0 - exp(-nHashFuncs * nMaxElements / nFilterBits) + * => 1.0 - pow(fpRate, 1.0 / nHashFuncs) = exp(-nHashFuncs * nMaxElements / nFilterBits) + * => log(1.0 - pow(fpRate, 1.0 / nHashFuncs)) = -nHashFuncs * nMaxElements / nFilterBits + * => nFilterBits = -nHashFuncs * nMaxElements / log(1.0 - pow(fpRate, 1.0 / nHashFuncs)) + * => nFilterBits = -nHashFuncs * nMaxElements / log(1.0 - exp(logFpRate / nHashFuncs)) + */ + 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); reset(); } +/* Similar to CBloomFilter::Hash */ +inline unsigned int CRollingBloomFilter::Hash(unsigned int nHashNum, const std::vector& vDataToHash) const { + return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash) % (data.size() * 16); +} + void CRollingBloomFilter::insert(const std::vector& vKey) { - if (nInsertions == 0) { - b1.clear(); - } else if (nInsertions == nBloomSize / 2) { - b2.clear(); + if (nEntriesThisGeneration == nEntriesPerGeneration) { + nEntriesThisGeneration = 0; + nGeneration++; + if (nGeneration == 4) { + 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); + } + } } - b1.insert(vKey); - b2.insert(vKey); - if (++nInsertions == nBloomSize) { - nInsertions = 0; + nEntriesThisGeneration++; + + for (int n = 0; n < nHashFuncs; n++) { + uint32_t h = Hash(n, vKey); + put(h, nGeneration); } } @@ -250,10 +274,13 @@ void CRollingBloomFilter::insert(const uint256& hash) bool CRollingBloomFilter::contains(const std::vector& vKey) const { - if (nInsertions < nBloomSize / 2) { - return b2.contains(vKey); + for (int n = 0; n < nHashFuncs; n++) { + uint32_t h = Hash(n, vKey); + if (get(h) == 0) { + return false; + } } - return b1.contains(vKey); + return true; } bool CRollingBloomFilter::contains(const uint256& hash) const @@ -264,8 +291,10 @@ bool CRollingBloomFilter::contains(const uint256& hash) const void CRollingBloomFilter::reset() { - unsigned int nNewTweak = GetRand(std::numeric_limits::max()); - b1.reset(nNewTweak); - b2.reset(nNewTweak); - nInsertions = 0; + nTweak = GetRand(std::numeric_limits::max()); + nEntriesThisGeneration = 0; + nGeneration = 1; + for (std::vector::iterator it = data.begin(); it != data.end(); it++) { + *it = 0; + } } diff --git a/src/bloom.h b/src/bloom.h index 009436104..86d397dd6 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -110,8 +110,11 @@ public: * reset() is provided, which also changes nTweak to decrease the impact of * false-positives. * - * contains(item) will always return true if item was one of the last N things + * contains(item) will always return true if item was one of the last N to 1.5*N * insert()'ed ... but may also return true for items that were not inserted. + * + * It needs around 1.8 bytes per element per factor 0.1 of false positive rate. + * (More accurately: 3/(log(256)*log(2)) * log(1/fpRate) * nElements bytes) */ class CRollingBloomFilter { @@ -129,10 +132,23 @@ public: void reset(); private: - unsigned int nBloomSize; - unsigned int nInsertions; - CBloomFilter b1, b2; + int nEntriesPerGeneration; + int nEntriesThisGeneration; + int nGeneration; + std::vector data; + unsigned int nTweak; + int nHashFuncs; + + unsigned int Hash(unsigned int nHashNum, const std::vector& 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 diff --git a/src/main.cpp b/src/main.cpp index b5e28a3ee..2c9a60d59 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -235,7 +235,7 @@ namespace { * million to make it highly unlikely for users to have issues with this * filter. * - * Memory used: 1.7MB + * Memory used: 1.3 MB */ boost::scoped_ptr recentRejects; uint256 hashRecentRejectsChainTip; From 5064a933a06a0d5c4f6293fe63b678cf9f3e482a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 5 Apr 2016 14:26:01 +0200 Subject: [PATCH 02/17] Fix formatting of NOPs for generated script tests (cherry picked from commit d03e46625ac95954bb9ecbc2cf73ffd8de6b8a13) --- src/core_write.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core_write.cpp b/src/core_write.cpp index a80ad82a8..f9da3635c 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -34,7 +34,7 @@ string FormatScript(const CScript& script) } else if ((op >= OP_1 && op <= OP_16) || op == OP_1NEGATE) { ret += strprintf("%i ", op - OP_1NEGATE - 1); continue; - } else if (op >= OP_NOP && op <= OP_CHECKMULTISIGVERIFY) { + } else if (op >= OP_NOP && op <= OP_NOP10) { string str(GetOpName(op)); if (str.substr(0, 3) == string("OP_")) { ret += str.substr(3, string::npos) + " "; @@ -44,7 +44,7 @@ string FormatScript(const CScript& script) if (vch.size() > 0) { ret += strprintf("0x%x 0x%x ", HexStr(it2, it - vch.size()), HexStr(it - vch.size(), it)); } else { - ret += strprintf("0x%x", HexStr(it2, it)); + ret += strprintf("0x%x ", HexStr(it2, it)); } continue; } From 7d488feb127ca397126320ecb41b613b2e615546 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 24 Apr 2016 18:37:29 +0200 Subject: [PATCH 03/17] More efficient bitsliced rolling Bloom filter This patch changes the implementation from one that stores 16 2-bit integers in one uint32_t's, to one that stores the first bit of 64 2-bit integers in one uint64_t and the second bit in another. This allows for 450x faster refreshing and 2.2x faster average speed. (cherry picked from commit 1953c40aa9589a03035fd294f3ba3549374a4826) --- src/bloom.cpp | 40 +++++++++++++++++++++++++++------------- src/bloom.h | 13 +------------ src/test/bloom_tests.cpp | 5 ++++- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index 932420465..246d94e47 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -233,14 +233,18 @@ 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& 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& vDataToHash) { + return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash); } void CRollingBloomFilter::insert(const std::vector& vKey) @@ -251,18 +255,25 @@ void CRollingBloomFilter::insert(const std::vector& 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; + uint32_t pos = (h >> 6) % 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; } } @@ -275,8 +286,11 @@ void CRollingBloomFilter::insert(const uint256& hash) bool CRollingBloomFilter::contains(const std::vector& 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 = (h >> 6) % 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; } } @@ -294,7 +308,7 @@ void CRollingBloomFilter::reset() nTweak = GetRand(std::numeric_limits::max()); nEntriesThisGeneration = 0; nGeneration = 1; - for (std::vector::iterator it = data.begin(); it != data.end(); it++) { + for (std::vector::iterator it = data.begin(); it != data.end(); it++) { *it = 0; } } diff --git a/src/bloom.h b/src/bloom.h index 86d397dd6..5c31eda58 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -135,20 +135,9 @@ private: int nEntriesPerGeneration; int nEntriesThisGeneration; int nGeneration; - std::vector data; + std::vector data; unsigned int nTweak; int nHashFuncs; - - unsigned int Hash(unsigned int nHashNum, const std::vector& 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 diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index 377f5b614..ee936014d 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -507,11 +507,14 @@ BOOST_AUTO_TEST_CASE(rolling_bloom) if (i >= 100) BOOST_CHECK(rb1.contains(data[i-100])); rb1.insert(data[i]); + BOOST_CHECK(rb1.contains(data[i])); } // Insert 999 more random entries: for (int i = 0; i < 999; i++) { - rb1.insert(RandomData()); + std::vector d = RandomData(); + rb1.insert(d); + BOOST_CHECK(rb1.contains(d)); } // Sanity check to make sure the filter isn't just filling up: nHits = 0; From 48be374f68deee708ee069ade09dd6b886c95edf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Jan=C3=ADk?= Date: Fri, 2 Sep 2016 18:19:01 +0200 Subject: [PATCH 04/17] Do not shadow variables (cherry picked from commit 4731cab8fbff51a8178c85d572e2482040278616) Zcash: Excluding changes to code we haven't backported. --- src/bloom.cpp | 8 +++---- src/key.cpp | 6 ++--- src/main.cpp | 8 +++---- src/pubkey.cpp | 6 ++--- src/pubkey.h | 4 ++-- src/reverselock.h | 6 ++--- src/test/crypto_tests.cpp | 20 ++++++++--------- src/test/net_tests.cpp | 4 ++-- src/test/script_tests.cpp | 4 ++-- src/torcontrol.cpp | 46 +++++++++++++++++++-------------------- src/wallet/wallet.cpp | 16 +++++++------- 11 files changed, 64 insertions(+), 64 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index 246d94e47..e9c927b27 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -279,8 +279,8 @@ void CRollingBloomFilter::insert(const std::vector& vKey) void CRollingBloomFilter::insert(const uint256& hash) { - vector data(hash.begin(), hash.end()); - insert(data); + vector vData(hash.begin(), hash.end()); + insert(vData); } bool CRollingBloomFilter::contains(const std::vector& vKey) const @@ -299,8 +299,8 @@ bool CRollingBloomFilter::contains(const std::vector& vKey) const bool CRollingBloomFilter::contains(const uint256& hash) const { - vector data(hash.begin(), hash.end()); - return contains(data); + vector vData(hash.begin(), hash.end()); + return contains(vData); } void CRollingBloomFilter::reset() diff --git a/src/key.cpp b/src/key.cpp index 5ed4797e1..9f03cf0c2 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -274,12 +274,12 @@ bool CKey::Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const return ret; } -bool CExtKey::Derive(CExtKey &out, unsigned int nChild) const { +bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const { out.nDepth = nDepth + 1; CKeyID id = key.GetPubKey().GetID(); memcpy(&out.vchFingerprint[0], &id, 4); - out.nChild = nChild; - return key.Derive(out.key, out.chaincode, nChild, chaincode); + out.nChild = _nChild; + return key.Derive(out.key, out.chaincode, _nChild, chaincode); } void CExtKey::SetMaster(const unsigned char *seed, unsigned int nSeedLen) { diff --git a/src/main.cpp b/src/main.cpp index 2c9a60d59..ed0d0a5e7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3018,14 +3018,14 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin if (pindex->GetUndoPos().IsNull() || !pindex->IsValid(BLOCK_VALID_SCRIPTS)) { if (pindex->GetUndoPos().IsNull()) { - CDiskBlockPos pos; - if (!FindUndoPos(state, pindex->nFile, pos, ::GetSerializeSize(blockundo, SER_DISK, CLIENT_VERSION) + 40)) + CDiskBlockPos _pos; + if (!FindUndoPos(state, pindex->nFile, _pos, ::GetSerializeSize(blockundo, SER_DISK, CLIENT_VERSION) + 40)) return error("ConnectBlock(): FindUndoPos failed"); - if (!UndoWriteToDisk(blockundo, pos, pindex->pprev->GetBlockHash(), chainparams.MessageStart())) + if (!UndoWriteToDisk(blockundo, _pos, pindex->pprev->GetBlockHash(), chainparams.MessageStart())) return AbortNode(state, "Failed to write undo data"); // update nUndoPos in block index - pindex->nUndoPos = pos.nPos; + pindex->nUndoPos = _pos.nPos; pindex->nStatus |= BLOCK_HAVE_UNDO; } diff --git a/src/pubkey.cpp b/src/pubkey.cpp index adabd11d1..32808a999 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -116,12 +116,12 @@ void CExtPubKey::Decode(const unsigned char code[BIP32_EXTKEY_SIZE]) { pubkey.Set(code+41, code+BIP32_EXTKEY_SIZE); } -bool CExtPubKey::Derive(CExtPubKey &out, unsigned int nChild) const { +bool CExtPubKey::Derive(CExtPubKey &out, unsigned int _nChild) const { out.nDepth = nDepth + 1; CKeyID id = pubkey.GetID(); memcpy(&out.vchFingerprint[0], &id, 4); - out.nChild = nChild; - return pubkey.Derive(out.pubkey, out.chaincode, nChild, chaincode); + out.nChild = _nChild; + return pubkey.Derive(out.pubkey, out.chaincode, _nChild, chaincode); } /* static */ bool CPubKey::CheckLowS(const std::vector& vchSig) { diff --git a/src/pubkey.h b/src/pubkey.h index edca4083b..4cdfbef38 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -100,9 +100,9 @@ public: } //! Construct a public key from a byte vector. - CPubKey(const std::vector& vch) + CPubKey(const std::vector& _vch) { - Set(vch.begin(), vch.end()); + Set(_vch.begin(), _vch.end()); } //! Simple read-only vector-like interface to the pubkey data. diff --git a/src/reverselock.h b/src/reverselock.h index 85d2d964f..7c0131f35 100644 --- a/src/reverselock.h +++ b/src/reverselock.h @@ -13,9 +13,9 @@ class reverse_lock { public: - explicit reverse_lock(Lock& lock) : lock(lock) { - lock.unlock(); - lock.swap(templock); + explicit reverse_lock(Lock& _lock) : lock(_lock) { + _lock.unlock(); + _lock.swap(templock); } ~reverse_lock() { diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp index fb61bc978..821cf59dc 100644 --- a/src/test/crypto_tests.cpp +++ b/src/test/crypto_tests.cpp @@ -132,13 +132,13 @@ void TestAES128CBC(const std::string &hexkey, const std::string &hexiv, bool pad { std::vector sub(i, in.end()); std::vector subout(sub.size() + AES_BLOCKSIZE); - int size = enc.Encrypt(&sub[0], sub.size(), &subout[0]); - if (size != 0) + int _size = enc.Encrypt(&sub[0], sub.size(), &subout[0]); + if (_size != 0) { - subout.resize(size); + subout.resize(_size); std::vector subdecrypted(subout.size()); - size = dec.Decrypt(&subout[0], subout.size(), &subdecrypted[0]); - subdecrypted.resize(size); + _size = dec.Decrypt(&subout[0], subout.size(), &subdecrypted[0]); + subdecrypted.resize(_size); BOOST_CHECK(decrypted.size() == in.size()); BOOST_CHECK_MESSAGE(subdecrypted == sub, HexStr(subdecrypted) + std::string(" != ") + HexStr(sub)); } @@ -173,13 +173,13 @@ void TestAES256CBC(const std::string &hexkey, const std::string &hexiv, bool pad { std::vector sub(i, in.end()); std::vector subout(sub.size() + AES_BLOCKSIZE); - int size = enc.Encrypt(&sub[0], sub.size(), &subout[0]); - if (size != 0) + int _size = enc.Encrypt(&sub[0], sub.size(), &subout[0]); + if (_size != 0) { - subout.resize(size); + subout.resize(_size); std::vector subdecrypted(subout.size()); - size = dec.Decrypt(&subout[0], subout.size(), &subdecrypted[0]); - subdecrypted.resize(size); + _size = dec.Decrypt(&subout[0], subout.size(), &subdecrypted[0]); + subdecrypted.resize(_size); BOOST_CHECK(decrypted.size() == in.size()); BOOST_CHECK_MESSAGE(subdecrypted == sub, HexStr(subdecrypted) + std::string(" != ") + HexStr(sub)); } diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 2d368224b..304da55ce 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -50,11 +50,11 @@ public: } }; -CDataStream AddrmanToStream(CAddrManSerializationMock& addrman) +CDataStream AddrmanToStream(CAddrManSerializationMock& _addrman) { CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); ssPeersIn << FLATDATA(Params().MessageStart()); - ssPeersIn << addrman; + ssPeersIn << _addrman; std::string str = ssPeersIn.str(); vector vchData(str.begin(), str.end()); return CDataStream(vchData, SER_DISK, CLIENT_VERSION); diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 9f5a36b3a..6ed81ade6 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -208,10 +208,10 @@ public: spendTx = BuildSpendingTransaction(CScript(), creditTx); } - TestBuilder& Add(const CScript& script) + TestBuilder& Add(const CScript& _script) { DoPush(); - spendTx.vin[0].scriptSig += script; + spendTx.vin[0].scriptSig += _script; return *this; } diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 0f4df5911..acfbe45fa 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -122,8 +122,8 @@ private: static void eventcb(struct bufferevent *bev, short what, void *ctx); }; -TorControlConnection::TorControlConnection(struct event_base *base): - base(base), b_conn(0) +TorControlConnection::TorControlConnection(struct event_base *_base): + base(_base), b_conn(0) { } @@ -194,7 +194,7 @@ void TorControlConnection::eventcb(struct bufferevent *bev, short what, void *ct } } -bool TorControlConnection::Connect(const std::string &target, const ConnectionCB& connected, const ConnectionCB& disconnected) +bool TorControlConnection::Connect(const std::string &target, const ConnectionCB& _connected, const ConnectionCB& _disconnected) { if (b_conn) Disconnect(); @@ -213,8 +213,8 @@ bool TorControlConnection::Connect(const std::string &target, const ConnectionCB return false; bufferevent_setcb(b_conn, TorControlConnection::readcb, NULL, TorControlConnection::eventcb, this); bufferevent_enable(b_conn, EV_READ|EV_WRITE); - this->connected = connected; - this->disconnected = disconnected; + this->connected = _connected; + this->disconnected = _disconnected; // Finally, connect to target if (bufferevent_socket_connect(b_conn, (struct sockaddr*)&connect_to_addr, connect_to_addrlen) < 0) { @@ -452,18 +452,18 @@ private: static void reconnect_cb(evutil_socket_t fd, short what, void *arg); }; -TorController::TorController(struct event_base* baseIn, const std::string& target): - base(baseIn), - target(target), conn(base), reconnect(true), reconnect_ev(0), +TorController::TorController(struct event_base* _base, const std::string& _target): + base(_base), + target(_target), conn(base), reconnect(true), reconnect_ev(0), reconnect_timeout(RECONNECT_TIMEOUT_START) { reconnect_ev = event_new(base, -1, 0, reconnect_cb, this); if (!reconnect_ev) LogPrintf("tor: Failed to create event for reconnection: out of memory?\n"); // Start connection attempts immediately - if (!conn.Connect(target, boost::bind(&TorController::connected_cb, this, _1), + if (!conn.Connect(_target, boost::bind(&TorController::connected_cb, this, _1), boost::bind(&TorController::disconnected_cb, this, _1) )) { - LogPrintf("tor: Initiating connection to Tor control port %s failed\n", target); + LogPrintf("tor: Initiating connection to Tor control port %s failed\n", _target); } // Read service private key if cached std::pair pkf = ReadBinaryFile(GetPrivateKeyFile()); @@ -484,7 +484,7 @@ TorController::~TorController() } } -void TorController::add_onion_cb(TorControlConnection& conn, const TorControlReply& reply) +void TorController::add_onion_cb(TorControlConnection& _conn, const TorControlReply& reply) { if (reply.code == 250) { LogPrint("tor", "tor: ADD_ONION successful\n"); @@ -520,7 +520,7 @@ void TorController::add_onion_cb(TorControlConnection& conn, const TorControlRep } } -void TorController::auth_cb(TorControlConnection& conn, const TorControlReply& reply) +void TorController::auth_cb(TorControlConnection& _conn, const TorControlReply& reply) { if (reply.code == 250) { LogPrint("tor", "tor: Authentication successful\n"); @@ -539,7 +539,7 @@ void TorController::auth_cb(TorControlConnection& conn, const TorControlReply& r // Request hidden service, redirect port. // Note that the 'virtual' port doesn't have to be the same as our internal port, but this is just a convenient // choice. TODO; refactor the shutdown sequence some day. - conn.Command(strprintf("ADD_ONION %s Port=%i,127.0.0.1:%i", private_key, GetListenPort(), GetListenPort()), + _conn.Command(strprintf("ADD_ONION %s Port=%i,127.0.0.1:%i", private_key, GetListenPort(), GetListenPort()), boost::bind(&TorController::add_onion_cb, this, _1, _2)); } else { LogPrintf("tor: Authentication failed\n"); @@ -573,7 +573,7 @@ static std::vector ComputeResponse(const std::string &key, const std::v return computedHash; } -void TorController::authchallenge_cb(TorControlConnection& conn, const TorControlReply& reply) +void TorController::authchallenge_cb(TorControlConnection& _conn, const TorControlReply& reply) { if (reply.code == 250) { LogPrint("tor", "tor: SAFECOOKIE authentication challenge successful\n"); @@ -599,7 +599,7 @@ void TorController::authchallenge_cb(TorControlConnection& conn, const TorContro } std::vector computedClientHash = ComputeResponse(TOR_SAFE_CLIENTKEY, cookie, clientNonce, serverNonce); - conn.Command("AUTHENTICATE " + HexStr(computedClientHash), boost::bind(&TorController::auth_cb, this, _1, _2)); + _conn.Command("AUTHENTICATE " + HexStr(computedClientHash), boost::bind(&TorController::auth_cb, this, _1, _2)); } else { LogPrintf("tor: Invalid reply to AUTHCHALLENGE\n"); } @@ -608,7 +608,7 @@ void TorController::authchallenge_cb(TorControlConnection& conn, const TorContro } } -void TorController::protocolinfo_cb(TorControlConnection& conn, const TorControlReply& reply) +void TorController::protocolinfo_cb(TorControlConnection& _conn, const TorControlReply& reply) { if (reply.code == 250) { std::set methods; @@ -648,23 +648,23 @@ void TorController::protocolinfo_cb(TorControlConnection& conn, const TorControl if (methods.count("HASHEDPASSWORD")) { LogPrint("tor", "tor: Using HASHEDPASSWORD authentication\n"); boost::replace_all(torpassword, "\"", "\\\""); - conn.Command("AUTHENTICATE \"" + torpassword + "\"", boost::bind(&TorController::auth_cb, this, _1, _2)); + _conn.Command("AUTHENTICATE \"" + torpassword + "\"", boost::bind(&TorController::auth_cb, this, _1, _2)); } else { LogPrintf("tor: Password provided with -torpassword, but HASHEDPASSWORD authentication is not available\n"); } } else if (methods.count("NULL")) { LogPrint("tor", "tor: Using NULL authentication\n"); - conn.Command("AUTHENTICATE", boost::bind(&TorController::auth_cb, this, _1, _2)); + _conn.Command("AUTHENTICATE", boost::bind(&TorController::auth_cb, this, _1, _2)); } else if (methods.count("SAFECOOKIE")) { // Cookie: hexdump -e '32/1 "%02x""\n"' ~/.tor/control_auth_cookie LogPrint("tor", "tor: Using SAFECOOKIE authentication, reading cookie authentication from %s\n", cookiefile); std::pair status_cookie = ReadBinaryFile(cookiefile, TOR_COOKIE_SIZE); if (status_cookie.first && status_cookie.second.size() == TOR_COOKIE_SIZE) { - // conn.Command("AUTHENTICATE " + HexStr(status_cookie.second), boost::bind(&TorController::auth_cb, this, _1, _2)); + // _conn.Command("AUTHENTICATE " + HexStr(status_cookie.second), boost::bind(&TorController::auth_cb, this, _1, _2)); cookie = std::vector(status_cookie.second.begin(), status_cookie.second.end()); clientNonce = std::vector(TOR_NONCE_SIZE, 0); GetRandBytes(&clientNonce[0], TOR_NONCE_SIZE); - conn.Command("AUTHCHALLENGE SAFECOOKIE " + HexStr(clientNonce), boost::bind(&TorController::authchallenge_cb, this, _1, _2)); + _conn.Command("AUTHCHALLENGE SAFECOOKIE " + HexStr(clientNonce), boost::bind(&TorController::authchallenge_cb, this, _1, _2)); } else { if (status_cookie.first) { LogPrintf("tor: Authentication cookie %s is not exactly %i bytes, as is required by the spec\n", cookiefile, TOR_COOKIE_SIZE); @@ -682,15 +682,15 @@ void TorController::protocolinfo_cb(TorControlConnection& conn, const TorControl } } -void TorController::connected_cb(TorControlConnection& conn) +void TorController::connected_cb(TorControlConnection& _conn) { reconnect_timeout = RECONNECT_TIMEOUT_START; // First send a PROTOCOLINFO command to figure out what authentication is expected - if (!conn.Command("PROTOCOLINFO 1", boost::bind(&TorController::protocolinfo_cb, this, _1, _2))) + if (!_conn.Command("PROTOCOLINFO 1", boost::bind(&TorController::protocolinfo_cb, this, _1, _2))) LogPrintf("tor: Error sending initial protocolinfo command\n"); } -void TorController::disconnected_cb(TorControlConnection& conn) +void TorController::disconnected_cb(TorControlConnection& _conn) { // Stop advertizing service when disconnected if (service.IsValid()) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c8ce6117b..6a6dbb801 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -824,8 +824,8 @@ set CWallet::GetConflicts(const uint256& txid) const if (mapTxSpends.count(txin.prevout) <= 1) continue; // No conflict if zero or one spends range = mapTxSpends.equal_range(txin.prevout); - for (TxSpends::const_iterator it = range.first; it != range.second; ++it) - result.insert(it->second); + for (TxSpends::const_iterator _it = range.first; _it != range.second; ++_it) + result.insert(_it->second); } std::pair range_n; @@ -2521,9 +2521,9 @@ int CWalletTx::GetRequestCount() const // How about the block it's in? if (nRequests == 0 && !hashBlock.IsNull()) { - map::const_iterator mi = pwallet->mapRequestCount.find(hashBlock); - if (mi != pwallet->mapRequestCount.end()) - nRequests = (*mi).second; + map::const_iterator _mi = pwallet->mapRequestCount.find(hashBlock); + if (_mi != pwallet->mapRequestCount.end()) + nRequests = (*_mi).second; else nRequests = 1; // If it's in someone else's block it must have got out } @@ -4309,17 +4309,17 @@ set< set > CWallet::GetAddressGroupings() set< set* > uniqueGroupings; // a set of pointers to groups of addresses map< CTxDestination, set* > setmap; // map addresses to the unique group containing it - for (set grouping : groupings) + for (set _grouping : groupings) { // make a set of all the groups hit by this new group set< set* > hits; map< CTxDestination, set* >::iterator it; - for (CTxDestination address : grouping) + for (CTxDestination address : _grouping) if ((it = setmap.find(address)) != setmap.end()) hits.insert((*it).second); // merge all hit groups into a new single group and delete old groups - set* merged = new set(grouping); + set* merged = new set(_grouping); for (set* hit : hits) { merged->insert(hit->begin(), hit->end()); From 5e81239e3e157ca738f94f1583fa55ad8c388a6e Mon Sep 17 00:00:00 2001 From: Robert McLaughlin Date: Tue, 1 Nov 2016 21:04:26 -0400 Subject: [PATCH 05/17] trivial: fix bloom filter init to isEmpty = true Fixes newly initialized bloom filters being constructed with isEmpty(false), which still works but loses the possible speedup when checking for key membership in an empty filter. (cherry picked from commit cccf73db0483cc3945bf8389ce197df35e931e16) --- src/bloom.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index e9c927b27..1ffa3d9a7 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -33,7 +33,7 @@ CBloomFilter::CBloomFilter(unsigned int nElements, double nFPRate, unsigned int * See https://en.wikipedia.org/wiki/Bloom_filter for an explanation of these formulas */ isFull(false), - isEmpty(false), + isEmpty(true), nHashFuncs(min((unsigned int)(vData.size() * 8 / nElements * LN2), MAX_HASH_FUNCS)), nTweak(nTweakIn), nFlags(nFlagsIn) From c6591f2cd2c3f80e2fb272668f849ac3d89e7172 Mon Sep 17 00:00:00 2001 From: "S. Matthew English" Date: Sat, 26 Nov 2016 14:57:19 +0100 Subject: [PATCH 06/17] unification of Bloom filter representation Output instances of "BloomFilter" changed to "Bloom filter", in accordance with Wikipedia standard notation: https://en.wikipedia.org/wiki/Bloom_filter also to sync with the majority of cases in the self-same file (cherry picked from commit b7aa2902fd3c29487f585c264d8845899f4c1846) --- src/test/bloom_tests.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index ee936014d..7c1f8b1ae 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -30,15 +30,15 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize) CBloomFilter filter(3, 0.01, 0, BLOOM_UPDATE_ALL); filter.insert(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")); - BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "BloomFilter doesn't contain just-inserted object!"); + BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter doesn't contain just-inserted object!"); // One bit different in first byte - BOOST_CHECK_MESSAGE(!filter.contains(ParseHex("19108ad8ed9bb6274d3980bab5a85c048f0950c8")), "BloomFilter contains something it shouldn't!"); + BOOST_CHECK_MESSAGE(!filter.contains(ParseHex("19108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter contains something it shouldn't!"); filter.insert(ParseHex("b5a2c786d9ef4658287ced5914b37a1b4aa32eee")); - BOOST_CHECK_MESSAGE(filter.contains(ParseHex("b5a2c786d9ef4658287ced5914b37a1b4aa32eee")), "BloomFilter doesn't contain just-inserted object (2)!"); + BOOST_CHECK_MESSAGE(filter.contains(ParseHex("b5a2c786d9ef4658287ced5914b37a1b4aa32eee")), "Bloom filter doesn't contain just-inserted object (2)!"); filter.insert(ParseHex("b9300670b4c5366e95b2699e8b18bc75e5f729c5")); - BOOST_CHECK_MESSAGE(filter.contains(ParseHex("b9300670b4c5366e95b2699e8b18bc75e5f729c5")), "BloomFilter doesn't contain just-inserted object (3)!"); + BOOST_CHECK_MESSAGE(filter.contains(ParseHex("b9300670b4c5366e95b2699e8b18bc75e5f729c5")), "Bloom filter doesn't contain just-inserted object (3)!"); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << filter; @@ -51,9 +51,9 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize) BOOST_CHECK_EQUAL_COLLECTIONS(stream.begin(), stream.end(), expected.begin(), expected.end()); - BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "BloomFilter doesn't contain just-inserted object!"); + BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter doesn't contain just-inserted object!"); filter.clear(); - BOOST_CHECK_MESSAGE( !filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "BloomFilter should be empty!"); + BOOST_CHECK_MESSAGE( !filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter should be empty!"); } BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) @@ -62,15 +62,15 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) CBloomFilter filter(3, 0.01, 2147483649UL, BLOOM_UPDATE_ALL); filter.insert(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")); - BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "BloomFilter doesn't contain just-inserted object!"); + BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter doesn't contain just-inserted object!"); // One bit different in first byte - BOOST_CHECK_MESSAGE(!filter.contains(ParseHex("19108ad8ed9bb6274d3980bab5a85c048f0950c8")), "BloomFilter contains something it shouldn't!"); + BOOST_CHECK_MESSAGE(!filter.contains(ParseHex("19108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter contains something it shouldn't!"); filter.insert(ParseHex("b5a2c786d9ef4658287ced5914b37a1b4aa32eee")); - BOOST_CHECK_MESSAGE(filter.contains(ParseHex("b5a2c786d9ef4658287ced5914b37a1b4aa32eee")), "BloomFilter doesn't contain just-inserted object (2)!"); + BOOST_CHECK_MESSAGE(filter.contains(ParseHex("b5a2c786d9ef4658287ced5914b37a1b4aa32eee")), "Bloom filter doesn't contain just-inserted object (2)!"); filter.insert(ParseHex("b9300670b4c5366e95b2699e8b18bc75e5f729c5")); - BOOST_CHECK_MESSAGE(filter.contains(ParseHex("b9300670b4c5366e95b2699e8b18bc75e5f729c5")), "BloomFilter doesn't contain just-inserted object (3)!"); + BOOST_CHECK_MESSAGE(filter.contains(ParseHex("b9300670b4c5366e95b2699e8b18bc75e5f729c5")), "Bloom filter doesn't contain just-inserted object (3)!"); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << filter; From 5d6fa863b3e113b75e5ea9c7419cae892ad56b3f Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Fri, 27 Jan 2017 17:43:41 +0900 Subject: [PATCH 07/17] Refactor: Remove using namespace from src/*.cpp. (cherry picked from commit b7b48c8bbdf7a90861610b035d8b0a247ef78c45) Zcash: Excluding changes to code we haven't backported yet that cause too many conflicts. --- src/bloom.cpp | 28 +++++++++++++--------------- src/chain.cpp | 2 -- src/core_read.cpp | 30 ++++++++++++++---------------- src/core_write.cpp | 42 ++++++++++++++++++++---------------------- src/init.cpp | 27 +++++++++++++-------------- src/merkleblock.cpp | 12 +++++------- src/txmempool.cpp | 6 +++--- 7 files changed, 68 insertions(+), 79 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index 1ffa3d9a7..f58164050 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -18,15 +18,13 @@ #define LN2SQUARED 0.4804530139182014246671025263266649717305529515945455 #define LN2 0.6931471805599453094172321214581765680755001343602552 -using namespace std; - CBloomFilter::CBloomFilter(unsigned int nElements, double nFPRate, unsigned int nTweakIn, unsigned char nFlagsIn) : /** * The ideal size for a bloom filter with a given number of elements and false positive rate is: * - nElements * log(fp rate) / ln(2)^2 * We ignore filter parameters which will create a bloom filter larger than the protocol limits */ - vData(min((unsigned int)(-1 / LN2SQUARED * nElements * log(nFPRate)), MAX_BLOOM_FILTER_SIZE * 8) / 8), + vData(std::min((unsigned int)(-1 / LN2SQUARED * nElements * log(nFPRate)), MAX_BLOOM_FILTER_SIZE * 8) / 8), /** * The ideal number of hash functions is filter size * ln(2) / number of elements * Again, we ignore filter parameters which will create a bloom filter with more hash functions than the protocol limits @@ -34,7 +32,7 @@ CBloomFilter::CBloomFilter(unsigned int nElements, double nFPRate, unsigned int */ isFull(false), isEmpty(true), - nHashFuncs(min((unsigned int)(vData.size() * 8 / nElements * LN2), MAX_HASH_FUNCS)), + nHashFuncs(std::min((unsigned int)(vData.size() * 8 / nElements * LN2), MAX_HASH_FUNCS)), nTweak(nTweakIn), nFlags(nFlagsIn) { @@ -57,7 +55,7 @@ inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, const std::vector< return MurmurHash3(nHashNum * 0xFBA4C795 + nTweak, vDataToHash) % (vData.size() * 8); } -void CBloomFilter::insert(const vector& vKey) +void CBloomFilter::insert(const std::vector& vKey) { if (isFull) return; @@ -74,17 +72,17 @@ void CBloomFilter::insert(const COutPoint& outpoint) { CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << outpoint; - vector data(stream.begin(), stream.end()); + std::vector data(stream.begin(), stream.end()); insert(data); } void CBloomFilter::insert(const uint256& hash) { - vector data(hash.begin(), hash.end()); + std::vector data(hash.begin(), hash.end()); insert(data); } -bool CBloomFilter::contains(const vector& vKey) const +bool CBloomFilter::contains(const std::vector& vKey) const { if (isFull) return true; @@ -104,13 +102,13 @@ bool CBloomFilter::contains(const COutPoint& outpoint) const { CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << outpoint; - vector data(stream.begin(), stream.end()); + std::vector data(stream.begin(), stream.end()); return contains(data); } bool CBloomFilter::contains(const uint256& hash) const { - vector data(hash.begin(), hash.end()); + std::vector data(hash.begin(), hash.end()); return contains(data); } @@ -153,7 +151,7 @@ bool CBloomFilter::IsRelevantAndUpdate(const CTransaction& tx) // This means clients don't have to update the filter themselves when a new relevant tx // is discovered in order to find spending transactions, which avoids round-tripping and race conditions. CScript::const_iterator pc = txout.scriptPubKey.begin(); - vector data; + std::vector data; while (pc < txout.scriptPubKey.end()) { opcodetype opcode; @@ -167,7 +165,7 @@ bool CBloomFilter::IsRelevantAndUpdate(const CTransaction& tx) else if ((nFlags & BLOOM_UPDATE_MASK) == BLOOM_UPDATE_P2PUBKEY_ONLY) { txnouttype type; - vector > vSolutions; + std::vector > vSolutions; if (Solver(txout.scriptPubKey, type, vSolutions) && (type == TX_PUBKEY || type == TX_MULTISIG)) insert(COutPoint(hash, i)); @@ -188,7 +186,7 @@ bool CBloomFilter::IsRelevantAndUpdate(const CTransaction& tx) // Match if the filter contains any arbitrary script data element in any scriptSig in tx CScript::const_iterator pc = txin.scriptSig.begin(); - vector data; + std::vector data; while (pc < txin.scriptSig.end()) { opcodetype opcode; @@ -279,7 +277,7 @@ void CRollingBloomFilter::insert(const std::vector& vKey) void CRollingBloomFilter::insert(const uint256& hash) { - vector vData(hash.begin(), hash.end()); + std::vector vData(hash.begin(), hash.end()); insert(vData); } @@ -299,7 +297,7 @@ bool CRollingBloomFilter::contains(const std::vector& vKey) const bool CRollingBloomFilter::contains(const uint256& hash) const { - vector vData(hash.begin(), hash.end()); + std::vector vData(hash.begin(), hash.end()); return contains(vData); } diff --git a/src/chain.cpp b/src/chain.cpp index 35013969f..499a8880d 100644 --- a/src/chain.cpp +++ b/src/chain.cpp @@ -5,8 +5,6 @@ #include "chain.h" -using namespace std; - /** * CChain implementation */ diff --git a/src/core_read.cpp b/src/core_read.cpp index 23b4a610c..19a5b6952 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -20,13 +20,11 @@ #include #include -using namespace std; - CScript ParseScript(const std::string& s) { CScript result; - static map mapOpNames; + static std::map mapOpNames; if (mapOpNames.empty()) { @@ -39,7 +37,7 @@ CScript ParseScript(const std::string& s) const char* name = GetOpName((opcodetype)op); if (strcmp(name, "OP_UNKNOWN") == 0) continue; - string strName(name); + std::string strName(name); mapOpNames[strName] = (opcodetype)op; // Convenience: OP_ADD and just ADD are both recognized: boost::algorithm::replace_first(strName, "OP_", ""); @@ -47,7 +45,7 @@ CScript ParseScript(const std::string& s) } } - vector words; + std::vector words; boost::algorithm::split(words, s, boost::algorithm::is_any_of(" \t\n"), boost::algorithm::token_compress_on); for (std::vector::const_iterator w = words.begin(); w != words.end(); ++w) @@ -57,16 +55,16 @@ CScript ParseScript(const std::string& s) // Empty string, ignore. (boost::split given '' will return one word) } else if (all(*w, boost::algorithm::is_digit()) || - (boost::algorithm::starts_with(*w, "-") && all(string(w->begin()+1, w->end()), boost::algorithm::is_digit()))) + (boost::algorithm::starts_with(*w, "-") && all(std::string(w->begin()+1, w->end()), boost::algorithm::is_digit()))) { // Number int64_t n = atoi64(*w); result << n; } - else if (boost::algorithm::starts_with(*w, "0x") && (w->begin()+2 != w->end()) && IsHex(string(w->begin()+2, w->end()))) + else if (boost::algorithm::starts_with(*w, "0x") && (w->begin()+2 != w->end()) && IsHex(std::string(w->begin()+2, w->end()))) { // Raw hex data, inserted NOT pushed onto stack: - std::vector raw = ParseHex(string(w->begin()+2, w->end())); + std::vector raw = ParseHex(std::string(w->begin()+2, w->end())); result.insert(result.end(), raw.begin(), raw.end()); } else if (w->size() >= 2 && boost::algorithm::starts_with(*w, "'") && boost::algorithm::ends_with(*w, "'")) @@ -83,7 +81,7 @@ CScript ParseScript(const std::string& s) } else { - throw runtime_error("script parse error"); + throw std::runtime_error("script parse error"); } } @@ -95,7 +93,7 @@ bool DecodeHexTx(CTransaction& tx, const std::string& strHexTx) if (!IsHex(strHexTx)) return false; - vector txData(ParseHex(strHexTx)); + std::vector txData(ParseHex(strHexTx)); CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION); try { ssData >> tx; @@ -124,9 +122,9 @@ bool DecodeHexBlk(CBlock& block, const std::string& strHexBlk) return true; } -uint256 ParseHashUV(const UniValue& v, const string& strName) +uint256 ParseHashUV(const UniValue& v, const std::string& strName) { - string strHex; + std::string strHex; if (v.isStr()) strHex = v.getValStr(); return ParseHashStr(strHex, strName); // Note: ParseHashStr("") throws a runtime_error @@ -135,19 +133,19 @@ uint256 ParseHashUV(const UniValue& v, const string& strName) uint256 ParseHashStr(const std::string& strHex, const std::string& strName) { if (!IsHex(strHex)) // Note: IsHex("") is false - throw runtime_error(strName+" must be hexadecimal string (not '"+strHex+"')"); + throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')"); uint256 result; result.SetHex(strHex); return result; } -vector ParseHexUV(const UniValue& v, const string& strName) +std::vector ParseHexUV(const UniValue& v, const std::string& strName) { - string strHex; + std::string strHex; if (v.isStr()) strHex = v.getValStr(); if (!IsHex(strHex)) - throw runtime_error(strName+" must be hexadecimal string (not '"+strHex+"')"); + throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')"); return ParseHex(strHex); } diff --git a/src/core_write.cpp b/src/core_write.cpp index f9da3635c..a809d1f51 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -17,16 +17,14 @@ #include -using namespace std; - -string FormatScript(const CScript& script) +std::string FormatScript(const CScript& script) { - string ret; + std::string ret; CScript::const_iterator it = script.begin(); opcodetype op; while (it != script.end()) { CScript::const_iterator it2 = it; - vector vch; + std::vector vch; if (script.GetOp2(it, op, &vch)) { if (op == OP_0) { ret += "0 "; @@ -35,9 +33,9 @@ string FormatScript(const CScript& script) ret += strprintf("%i ", op - OP_1NEGATE - 1); continue; } else if (op >= OP_NOP && op <= OP_NOP10) { - string str(GetOpName(op)); - if (str.substr(0, 3) == string("OP_")) { - ret += str.substr(3, string::npos) + " "; + std::string str(GetOpName(op)); + if (str.substr(0, 3) == std::string("OP_")) { + ret += str.substr(3, std::string::npos) + " "; continue; } } @@ -54,14 +52,14 @@ string FormatScript(const CScript& script) return ret.substr(0, ret.size() - 1); } -const map mapSigHashTypes = +const std::map mapSigHashTypes = boost::assign::map_list_of - (static_cast(SIGHASH_ALL), string("ALL")) - (static_cast(SIGHASH_ALL|SIGHASH_ANYONECANPAY), string("ALL|ANYONECANPAY")) - (static_cast(SIGHASH_NONE), string("NONE")) - (static_cast(SIGHASH_NONE|SIGHASH_ANYONECANPAY), string("NONE|ANYONECANPAY")) - (static_cast(SIGHASH_SINGLE), string("SINGLE")) - (static_cast(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY), string("SINGLE|ANYONECANPAY")) + (static_cast(SIGHASH_ALL), std::string("ALL")) + (static_cast(SIGHASH_ALL|SIGHASH_ANYONECANPAY), std::string("ALL|ANYONECANPAY")) + (static_cast(SIGHASH_NONE), std::string("NONE")) + (static_cast(SIGHASH_NONE|SIGHASH_ANYONECANPAY), std::string("NONE|ANYONECANPAY")) + (static_cast(SIGHASH_SINGLE), std::string("SINGLE")) + (static_cast(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY), std::string("SINGLE|ANYONECANPAY")) ; /** @@ -71,11 +69,11 @@ const map mapSigHashTypes = * of a signature. Only pass true for scripts you believe could contain signatures. For example, * pass false, or omit the this argument (defaults to false), for scriptPubKeys. */ -string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode) +std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode) { - string str; + std::string str; opcodetype opcode; - vector vch; + std::vector vch; CScript::const_iterator pc = script.begin(); while (pc < script.end()) { if (!str.empty()) { @@ -86,12 +84,12 @@ string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode) return str; } if (0 <= opcode && opcode <= OP_PUSHDATA4) { - if (vch.size() <= static_cast::size_type>(4)) { + if (vch.size() <= static_cast::size_type>(4)) { str += strprintf("%d", CScriptNum(vch, false).getint()); } else { // the IsUnspendable check makes sure not to try to decode OP_RETURN data that may match the format of a signature if (fAttemptSighashDecode && !script.IsUnspendable()) { - string strSigHashDecode; + std::string strSigHashDecode; // goal: only attempt to decode a defined sighash type from data that looks like a signature within a scriptSig. // this won't decode correctly formatted public keys in Pubkey or Multisig scripts due to // the restrictions on the pubkey formats (see IsCompressedOrUncompressedPubKey) being incongruous with the @@ -115,7 +113,7 @@ string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode) return str; } -string EncodeHexTx(const CTransaction& tx) +std::string EncodeHexTx(const CTransaction& tx) { CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); ssTx << tx; @@ -126,7 +124,7 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex) { txnouttype type; - vector addresses; + std::vector addresses; int nRequired; out.pushKV("asm", ScriptToAsmStr(scriptPubKey)); diff --git a/src/init.cpp b/src/init.cpp index 9f12b6dea..ae4265917 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -68,7 +68,6 @@ #include "librustzcash.h" -using namespace std; using namespace boost::placeholders; extern void ThreadSendAlert(); @@ -313,10 +312,10 @@ void OnRPCStopped() void OnRPCPreCommand(const CRPCCommand& cmd) { // Observe safe mode - string strWarning = GetWarnings("rpc").first; + std::string strWarning = GetWarnings("rpc").first; if (strWarning != "" && !GetBoolArg("-disablesafemode", DEFAULT_DISABLE_SAFEMODE) && !cmd.okSafeMode) - throw JSONRPCError(RPC_FORBIDDEN_BY_SAFE_MODE, string("Safe mode: ") + strWarning); + throw JSONRPCError(RPC_FORBIDDEN_BY_SAFE_MODE, std::string("Safe mode: ") + strWarning); } std::string HelpMessage(HelpMessageMode mode) @@ -326,7 +325,7 @@ std::string HelpMessage(HelpMessageMode mode) // When adding new options to the categories, please keep and ensure alphabetical ordering. // Do not translate _(...) -help-debug options, many technical terms, and only a very small audience, so is unnecessary stress to translators - string strUsage = HelpMessageGroup(_("Options:")); + std::string strUsage = HelpMessageGroup(_("Options:")); strUsage += HelpMessageOpt("-?", _("This help message")); strUsage += HelpMessageOpt("-alerts", strprintf(_("Receive and display P2P network alerts (default: %u)"), DEFAULT_ALERTS)); strUsage += HelpMessageOpt("-alertnotify=", _("Execute command when a relevant alert is received or we see a really long fork (%s in cmd is replaced by message)")); @@ -440,7 +439,7 @@ std::string HelpMessage(HelpMessageMode mode) "-fundingstream=streamId:startHeight:endHeight:comma_delimited_addresses", "Use given addresses for block subsidy share paid to the funding stream with id (regtest-only)"); } - string debugCategories = "addrman, alert, bench, coindb, db, estimatefee, http, libevent, lock, mempool, net, partitioncheck, pow, proxy, prune, " + std::string debugCategories = "addrman, alert, bench, coindb, db, estimatefee, http, libevent, lock, mempool, net, partitioncheck, pow, proxy, prune, " "rand, receiveunsafe, reindex, rpc, selectcoins, tor, zmq, zrpc, zrpcunsafe (implies zrpc)"; // Don't translate these strUsage += HelpMessageOpt("-debug=", strprintf(_("Output debugging information (default: %u, supplying is optional)"), 0) + ". " + _("If is not supplied or if = 1, output all debugging information.") + " " + _(" can be:") + " " + debugCategories + ". " + @@ -566,7 +565,7 @@ struct CImportingNow void CleanupBlockRevFiles() { using namespace fs; - map mapBlockFiles; + std::map mapBlockFiles; // Glob all blk?????.dat and rev?????.dat files from the blocks directory. // Remove the rev files immediately and insert the blk file paths into an @@ -590,7 +589,7 @@ void CleanupBlockRevFiles() // keeping a separate counter. Once we hit a gap (or if 0 doesn't exist) // start removing block files. int nContigCounter = 0; - for (const std::pair& item : mapBlockFiles) { + for (const std::pair& item : mapBlockFiles) { if (atoi(item.first) == nContigCounter) { nContigCounter++; continue; @@ -988,15 +987,15 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) fDebug = !mapMultiArgs["-debug"].empty(); // Special-case: if -debug=0/-nodebug is set, turn off debugging messages - const vector& categories = mapMultiArgs["-debug"]; - if (GetBoolArg("-nodebug", false) || find(categories.begin(), categories.end(), string("0")) != categories.end()) + const std::vector& categories = mapMultiArgs["-debug"]; + if (GetBoolArg("-nodebug", false) || find(categories.begin(), categories.end(), std::string("0")) != categories.end()) fDebug = false; // Special case: if debug=zrpcunsafe, implies debug=zrpc, so add it to debug categories - if (find(categories.begin(), categories.end(), string("zrpcunsafe")) != categories.end()) { - if (find(categories.begin(), categories.end(), string("zrpc")) == categories.end()) { + if (find(categories.begin(), categories.end(), std::string("zrpcunsafe")) != categories.end()) { + if (find(categories.begin(), categories.end(), std::string("zrpc")) == categories.end()) { LogPrintf("%s: parameter interaction: setting -debug=zrpcunsafe -> -debug=zrpc\n", __func__); - vector& v = mapMultiArgs["-debug"]; + std::vector& v = mapMultiArgs["-debug"]; v.push_back("zrpc"); } } @@ -1121,7 +1120,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (chainparams.NetworkIDString() != "regtest") { return InitError("Network upgrade parameters may only be overridden on regtest."); } - const vector& deployments = mapMultiArgs["-nuparams"]; + const std::vector& deployments = mapMultiArgs["-nuparams"]; for (auto i : deployments) { std::vector vDeploymentParams; boost::split(vDeploymentParams, i, boost::is_any_of(":")); @@ -1320,7 +1319,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) // sanitize comments per BIP-0014, format user agent and check total size std::vector uacomments; - for (string cmt : mapMultiArgs["-uacomment"]) + for (std::string cmt : mapMultiArgs["-uacomment"]) { if (cmt != SanitizeString(cmt, SAFE_CHARS_UA_COMMENT)) return InitError(strprintf("User Agent comment (%s) contains unsafe characters.", cmt)); diff --git a/src/merkleblock.cpp b/src/merkleblock.cpp index 25d3dc697..383a4585f 100644 --- a/src/merkleblock.cpp +++ b/src/merkleblock.cpp @@ -9,14 +9,12 @@ #include "consensus/consensus.h" #include "utilstrencodings.h" -using namespace std; - CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter& filter) { header = block.GetBlockHeader(); - vector vMatch; - vector vHashes; + std::vector vMatch; + std::vector vHashes; vMatch.reserve(block.vtx.size()); vHashes.reserve(block.vtx.size()); @@ -27,7 +25,7 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter& filter) if (filter.IsRelevantAndUpdate(block.vtx[i])) { vMatch.push_back(true); - vMatchedTxn.push_back(make_pair(i, hash)); + vMatchedTxn.push_back(std::make_pair(i, hash)); } else vMatch.push_back(false); @@ -41,8 +39,8 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, const std::set& txids) { header = block.GetBlockHeader(); - vector vMatch; - vector vHashes; + std::vector vMatch; + std::vector vHashes; vMatch.reserve(block.vtx.size()); vHashes.reserve(block.vtx.size()); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ea28dff81..476fe99a9 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -506,7 +506,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const const int64_t nSpendHeight = GetSpendHeight(mempoolDuplicate); LOCK(cs); - list waitingOnDependants; + std::list waitingOnDependants; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { unsigned int i = 0; checkTotal += it->GetTxSize(); @@ -628,7 +628,7 @@ void CTxMemPool::checkNullifiers(ShieldedType type) const } } -void CTxMemPool::queryHashes(vector& vtxid) +void CTxMemPool::queryHashes(std::vector& vtxid) { vtxid.clear(); @@ -693,7 +693,7 @@ CTxMemPool::ReadFeeEstimates(CAutoFile& filein) return true; } -void CTxMemPool::PrioritiseTransaction(const uint256 hash, const string strHash, double dPriorityDelta, const CAmount& nFeeDelta) +void CTxMemPool::PrioritiseTransaction(const uint256 hash, const std::string strHash, double dPriorityDelta, const CAmount& nFeeDelta) { { LOCK(cs); From 012dd8e740a6ce2a7c7316d1641027fc86eedf7b Mon Sep 17 00:00:00 2001 From: kobake Date: Tue, 14 Feb 2017 15:39:51 +0900 Subject: [PATCH 08/17] Fix msvc compiler error C4146 (minus operator applied to unsigned type) On msvc14, the compiler error C4146 (unary minus operator applied to unsigned type, result still unsigned) had been occured. Use '0 - x' styled formula instead of '-x' so as to fix the error. (cherry picked from commit 292112f87ef1780fee6164063a60af9ee7bf3f86) --- src/bloom.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index f58164050..575e40509 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -253,8 +253,8 @@ void CRollingBloomFilter::insert(const std::vector& vKey) if (nGeneration == 4) { nGeneration = 1; } - uint64_t nGenerationMask1 = -(uint64_t)(nGeneration & 1); - uint64_t nGenerationMask2 = -(uint64_t)(nGeneration >> 1); + uint64_t nGenerationMask1 = 0 - (uint64_t)(nGeneration & 1); + uint64_t nGenerationMask2 = 0 - (uint64_t)(nGeneration >> 1); /* Wipe old entries that used this generation number. */ for (uint32_t p = 0; p < data.size(); p += 2) { uint64_t p1 = data[p], p2 = data[p + 1]; From 8b1fe83120d9bc06652d2bbe7c861a85162aa1f7 Mon Sep 17 00:00:00 2001 From: kobake Date: Wed, 8 Mar 2017 05:19:31 +0900 Subject: [PATCH 09/17] Fix msvc compiler error C4146 (unary minus operator applied to unsigned type) On msvc14, int literal '-2147483648' is invalid, because '2147483648' is unsigned type and cant't apply minus operator to unsigned type. To define the int literal correctly, use '-2147483647 - 1' formula that is also used to define INT_MIN in limits.h. (cherry picked from commit 8e0720bdb9141b00b4c00893b8139904c67ddacf) --- src/test/util_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index c7ccc54b6..0e75189db 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -312,7 +312,7 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32) BOOST_CHECK(ParseInt32("1234", &n) && n == 1234); BOOST_CHECK(ParseInt32("01234", &n) && n == 1234); // no octal BOOST_CHECK(ParseInt32("2147483647", &n) && n == 2147483647); - BOOST_CHECK(ParseInt32("-2147483648", &n) && n == -2147483648); + BOOST_CHECK(ParseInt32("-2147483648", &n) && n == (-2147483647 - 1)); // (-2147483647 - 1) equals INT_MIN BOOST_CHECK(ParseInt32("-1234", &n) && n == -1234); // Invalid values BOOST_CHECK(!ParseInt32("", &n)); From 36603f25a9e33468f848b5cb431b39cf2cf9051a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=AD=E3=83=8F=E3=83=B3=20=E3=83=80=E3=83=AB?= Date: Mon, 13 Feb 2017 14:39:48 +0900 Subject: [PATCH 10/17] param variables made const (cherry picked from commit 64aa36e20368fa16d4ff757d56dc2690ed0f48ba) --- src/bloom.cpp | 8 ++++---- src/bloom.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index 575e40509..4c1f97dd3 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -18,7 +18,7 @@ #define LN2SQUARED 0.4804530139182014246671025263266649717305529515945455 #define LN2 0.6931471805599453094172321214581765680755001343602552 -CBloomFilter::CBloomFilter(unsigned int nElements, double nFPRate, unsigned int nTweakIn, unsigned char nFlagsIn) : +CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweakIn, unsigned char nFlagsIn) : /** * The ideal size for a bloom filter with a given number of elements and false positive rate is: * - nElements * log(fp rate) / ln(2)^2 @@ -39,7 +39,7 @@ CBloomFilter::CBloomFilter(unsigned int nElements, double nFPRate, unsigned int } // Private constructor used by CRollingBloomFilter -CBloomFilter::CBloomFilter(unsigned int nElements, double nFPRate, unsigned int nTweakIn) : +CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweakIn) : vData((unsigned int)(-1 / LN2SQUARED * nElements * log(nFPRate)) / 8), isFull(false), isEmpty(true), @@ -119,7 +119,7 @@ void CBloomFilter::clear() isEmpty = true; } -void CBloomFilter::reset(unsigned int nNewTweak) +void CBloomFilter::reset(const unsigned int nNewTweak) { clear(); nTweak = nNewTweak; @@ -213,7 +213,7 @@ void CBloomFilter::UpdateEmptyFull() isEmpty = empty; } -CRollingBloomFilter::CRollingBloomFilter(unsigned int nElements, double fpRate) +CRollingBloomFilter::CRollingBloomFilter(const unsigned int nElements, const double fpRate) { double logFpRate = log(fpRate); /* The optimal number of hash functions is log(fpRate) / log(0.5), but diff --git a/src/bloom.h b/src/bloom.h index 5c31eda58..100452003 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -54,7 +54,7 @@ private: unsigned int Hash(unsigned int nHashNum, const std::vector& vDataToHash) const; // Private constructor for CRollingBloomFilter, no restrictions on size - CBloomFilter(unsigned int nElements, double nFPRate, unsigned int nTweak); + CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak); friend class CRollingBloomFilter; public: @@ -67,7 +67,7 @@ public: * It should generally always be a random value (and is largely only exposed for unit testing) * nFlags should be one of the BLOOM_UPDATE_* enums (not _MASK) */ - CBloomFilter(unsigned int nElements, double nFPRate, unsigned int nTweak, unsigned char nFlagsIn); + CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak, unsigned char nFlagsIn); CBloomFilter() : isFull(true), isEmpty(false), nHashFuncs(0), nTweak(0), nFlags(0) {} ADD_SERIALIZE_METHODS; @@ -89,7 +89,7 @@ public: bool contains(const uint256& hash) const; void clear(); - void reset(unsigned int nNewTweak); + void reset(const unsigned int nNewTweak); //! True if the size is <= MAX_BLOOM_FILTER_SIZE and the number of hash functions is <= MAX_HASH_FUNCS //! (catch a filter which was just deserialized which was too big) @@ -122,7 +122,7 @@ public: // A random bloom filter calls GetRand() at creation time. // Don't create global CRollingBloomFilter objects, as they may be // constructed before the randomizer is properly initialized. - CRollingBloomFilter(unsigned int nElements, double nFPRate); + CRollingBloomFilter(const unsigned int nElements, const double nFPRate); void insert(const std::vector& vKey); void insert(const uint256& hash); From 1571d654552d498a874db38522854d1311bf3116 Mon Sep 17 00:00:00 2001 From: Martin Ankerl Date: Sun, 6 May 2018 13:55:33 +0200 Subject: [PATCH 11/17] replace modulus with FastMod Replaces the slow modulo operation with a much faster 32bit multiplication & shift. This works because the hash should be uniformly distributed between 0 and 2^32-1. This speeds up the benchmark by a factor of about 1.3: RollingBloom, 5, 1500000, 3.73733, 4.97569e-07, 4.99002e-07, 4.98372e-07 # before RollingBloom, 5, 1500000, 2.86842, 3.81630e-07, 3.83730e-07, 3.82473e-07 # FastMod Be aware that this changes the position of the bits that are toggled, so this should probably not be used for CBloomFilter which is serialized. (cherry picked from commit 9aac9f90d5e56752cc6cbfac48063ad29a01143c) --- src/bloom.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index 4c1f97dd3..d31cbd8d5 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -245,6 +245,14 @@ static inline uint32_t RollingBloomHash(unsigned int nHashNum, uint32_t nTweak, 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& vKey) { if (nEntriesThisGeneration == nEntriesPerGeneration) { @@ -268,7 +276,8 @@ void CRollingBloomFilter::insert(const std::vector& vKey) for (int n = 0; n < nHashFuncs; n++) { uint32_t h = RollingBloomHash(n, nTweak, vKey); int bit = h & 0x3F; - uint32_t pos = (h >> 6) % data.size(); + /* 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; @@ -286,7 +295,7 @@ bool CRollingBloomFilter::contains(const std::vector& vKey) const for (int n = 0; n < nHashFuncs; n++) { uint32_t h = RollingBloomHash(n, nTweak, vKey); int bit = h & 0x3F; - uint32_t pos = (h >> 6) % data.size(); + 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; From 6eb04fefb784d29599980b65ebaf6903f45daf20 Mon Sep 17 00:00:00 2001 From: 251 <13120787+251Labs@users.noreply.github.com> Date: Mon, 13 Aug 2018 00:37:15 +0200 Subject: [PATCH 12/17] Removes unsed `CBloomFilter` constructor. This commit removes the `CBloomFilter::CBloomFilter(const unsigned int, const double, const unsigned int)` constructor, which became obsolete with 086ee67. (cherry picked from commit 265bd50884ac1984c08f0e5916256d5f12e655f5) --- src/bloom.cpp | 11 ----------- src/bloom.h | 4 ---- 2 files changed, 15 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index d31cbd8d5..cd56dc361 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -38,17 +38,6 @@ CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, c { } -// Private constructor used by CRollingBloomFilter -CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweakIn) : - vData((unsigned int)(-1 / LN2SQUARED * nElements * log(nFPRate)) / 8), - isFull(false), - isEmpty(true), - nHashFuncs((unsigned int)(vData.size() * 8 / nElements * LN2)), - nTweak(nTweakIn), - nFlags(BLOOM_UPDATE_NONE) -{ -} - inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, const std::vector& vDataToHash) const { // 0xFBA4C795 chosen as it guarantees a reasonable bit difference between nHashNum values. diff --git a/src/bloom.h b/src/bloom.h index 100452003..06d409228 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -53,10 +53,6 @@ private: unsigned int Hash(unsigned int nHashNum, const std::vector& vDataToHash) const; - // Private constructor for CRollingBloomFilter, no restrictions on size - CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak); - friend class CRollingBloomFilter; - public: /** * Creates a new bloom filter which will provide the given fp rate when filled with the given number of elements From 4dfc6384e9ea6d782da5296ae8ee5ba6488fc24d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Wed, 22 May 2019 14:48:51 +0100 Subject: [PATCH 13/17] bench: Add benchmark for CRollingBloomFilter::reset (cherry picked from commit d2dbc7da26e1ca40200521c05a0b1ca75578acd2) --- src/bench/rollingbloom.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/bench/rollingbloom.cpp b/src/bench/rollingbloom.cpp index e9fa2ebe5..038e36fd5 100644 --- a/src/bench/rollingbloom.cpp +++ b/src/bench/rollingbloom.cpp @@ -39,4 +39,13 @@ static void RollingBloom(benchmark::State& state) } } +static void RollingBloomReset(benchmark::State& state) +{ + CRollingBloomFilter filter(120000, 0.000001); + while (state.KeepRunning()) { + filter.reset(); + } +} + BENCHMARK(RollingBloom); +BENCHMARK(RollingBloomReset); // To-backport: 20000 From 07a57164589715e0820c3ced6447b5d9c0bbce27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 21 May 2019 23:47:44 +0100 Subject: [PATCH 14/17] refactor: Improve CRollingBloomFilter::reset by using std::fill (cherry picked from commit df9e15f092c18a8047f09307576c2b77b9c8d01c) --- src/bloom.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index cd56dc361..5867c8c1d 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -14,6 +14,7 @@ #include #include +#include #define LN2SQUARED 0.4804530139182014246671025263266649717305529515945455 #define LN2 0.6931471805599453094172321214581765680755001343602552 @@ -304,7 +305,5 @@ void CRollingBloomFilter::reset() nTweak = GetRand(std::numeric_limits::max()); nEntriesThisGeneration = 0; nGeneration = 1; - for (std::vector::iterator it = data.begin(); it != data.end(); it++) { - *it = 0; - } + std::fill(data.begin(), data.end(), 0); } From 48fe1baf005d833336d6e3a9eafd203db4c15ef3 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 16 Apr 2020 14:41:46 +0200 Subject: [PATCH 15/17] refactor: Remove unused methods CBloomFilter::reset()/clear() Co-authored-by: MarcoFalke (cherry picked from commit 69ffddc83e0f3e265bf6cf7ae31489ae629fe6be) Zcash: Excluding change to src/test/fuzz/bloom_filter.cpp which we don't have (we haven't backported upstream's fuzzing framework). --- src/bloom.cpp | 13 ------------- src/bloom.h | 3 --- src/test/bloom_tests.cpp | 3 +-- 3 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index 5867c8c1d..a524720b5 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -102,19 +102,6 @@ bool CBloomFilter::contains(const uint256& hash) const return contains(data); } -void CBloomFilter::clear() -{ - vData.assign(vData.size(),0); - isFull = false; - isEmpty = true; -} - -void CBloomFilter::reset(const unsigned int nNewTweak) -{ - clear(); - nTweak = nNewTweak; -} - bool CBloomFilter::IsWithinSizeConstraints() const { return vData.size() <= MAX_BLOOM_FILTER_SIZE && nHashFuncs <= MAX_HASH_FUNCS; diff --git a/src/bloom.h b/src/bloom.h index 06d409228..ce4021ed9 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -84,9 +84,6 @@ public: bool contains(const COutPoint& outpoint) const; bool contains(const uint256& hash) const; - void clear(); - void reset(const unsigned int nNewTweak); - //! True if the size is <= MAX_BLOOM_FILTER_SIZE and the number of hash functions is <= MAX_HASH_FUNCS //! (catch a filter which was just deserialized which was too big) bool IsWithinSizeConstraints() const; diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index 7c1f8b1ae..e8807e970 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -29,6 +29,7 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize) { CBloomFilter filter(3, 0.01, 0, BLOOM_UPDATE_ALL); + BOOST_CHECK_MESSAGE( !filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter should be empty!"); filter.insert(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")); BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter doesn't contain just-inserted object!"); // One bit different in first byte @@ -52,8 +53,6 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize) BOOST_CHECK_EQUAL_COLLECTIONS(stream.begin(), stream.end(), expected.begin(), expected.end()); BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter doesn't contain just-inserted object!"); - filter.clear(); - BOOST_CHECK_MESSAGE( !filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "Bloom filter should be empty!"); } BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) From 3e0ef7ffbb1bdc53c42083c04060ac17221596a5 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 28 Apr 2020 19:19:34 +0200 Subject: [PATCH 16/17] net: remove is{Empty,Full} flags from CBloomFilter, clarify CVE fix (cherry picked from commit 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8) Zcash: Excluding change to src/test/fuzz/bloom_filter.cpp which we don't have (we haven't backported upstream's fuzzing framework). --- src/bloom.cpp | 26 +++----------------------- src/bloom.h | 7 +------ src/main.cpp | 1 - 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/src/bloom.cpp b/src/bloom.cpp index a524720b5..b95420075 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -31,8 +31,6 @@ CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, c * Again, we ignore filter parameters which will create a bloom filter with more hash functions than the protocol limits * See https://en.wikipedia.org/wiki/Bloom_filter for an explanation of these formulas */ - isFull(false), - isEmpty(true), nHashFuncs(std::min((unsigned int)(vData.size() * 8 / nElements * LN2), MAX_HASH_FUNCS)), nTweak(nTweakIn), nFlags(nFlagsIn) @@ -47,7 +45,7 @@ inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, const std::vector< void CBloomFilter::insert(const std::vector& vKey) { - if (isFull) + if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700) return; for (unsigned int i = 0; i < nHashFuncs; i++) { @@ -55,7 +53,6 @@ void CBloomFilter::insert(const std::vector& vKey) // Sets bit nIndex of vData vData[nIndex >> 3] |= (1 << (7 & nIndex)); } - isEmpty = false; } void CBloomFilter::insert(const COutPoint& outpoint) @@ -74,10 +71,8 @@ void CBloomFilter::insert(const uint256& hash) bool CBloomFilter::contains(const std::vector& vKey) const { - if (isFull) + if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700) return true; - if (isEmpty) - return false; for (unsigned int i = 0; i < nHashFuncs; i++) { unsigned int nIndex = Hash(i, vKey); @@ -112,10 +107,8 @@ bool CBloomFilter::IsRelevantAndUpdate(const CTransaction& tx) bool fFound = false; // Match if the filter contains the hash of tx // for finding tx when they appear in a block - if (isFull) + if (vData.empty()) // zero-size = "match-all" filter return true; - if (isEmpty) - return false; const uint256& hash = tx.GetHash(); if (contains(hash)) fFound = true; @@ -177,19 +170,6 @@ bool CBloomFilter::IsRelevantAndUpdate(const CTransaction& tx) return false; } -void CBloomFilter::UpdateEmptyFull() -{ - bool full = true; - bool empty = true; - for (unsigned int i = 0; i < vData.size(); i++) - { - full &= vData[i] == 0xff; - empty &= vData[i] == 0; - } - isFull = full; - isEmpty = empty; -} - CRollingBloomFilter::CRollingBloomFilter(const unsigned int nElements, const double fpRate) { double logFpRate = log(fpRate); diff --git a/src/bloom.h b/src/bloom.h index ce4021ed9..ab2de22cd 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -45,8 +45,6 @@ class CBloomFilter { private: std::vector vData; - bool isFull; - bool isEmpty; unsigned int nHashFuncs; unsigned int nTweak; unsigned char nFlags; @@ -64,7 +62,7 @@ public: * nFlags should be one of the BLOOM_UPDATE_* enums (not _MASK) */ CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak, unsigned char nFlagsIn); - CBloomFilter() : isFull(true), isEmpty(false), nHashFuncs(0), nTweak(0), nFlags(0) {} + CBloomFilter() : nHashFuncs(0), nTweak(0), nFlags(0) {} ADD_SERIALIZE_METHODS; @@ -90,9 +88,6 @@ public: //! Also adds any outputs which match the filter to the filter (to match their spending txes) bool IsRelevantAndUpdate(const CTransaction& tx); - - //! Checks for empty and full filters to avoid wasting cpu - void UpdateEmptyFull(); }; /** diff --git a/src/main.cpp b/src/main.cpp index ed0d0a5e7..c3b648351 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6595,7 +6595,6 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string LOCK(pfrom->cs_filter); delete pfrom->pfilter; pfrom->pfilter = new CBloomFilter(filter); - pfrom->pfilter->UpdateEmptyFull(); pfrom->fRelayTxes = true; } } From 1dd379a6ebc880d6550345f480bb0de49ab7c4ca Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Mon, 1 Jun 2020 14:18:32 +1000 Subject: [PATCH 17/17] doc: clarify CRollingBloomFilter size estimate (cherry picked from commit d9141a0002bb508b2e94e206a1bd28ef8f97ffde) --- src/bloom.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/bloom.h b/src/bloom.h index ab2de22cd..e4c01fe49 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -102,7 +102,18 @@ public: * insert()'ed ... but may also return true for items that were not inserted. * * It needs around 1.8 bytes per element per factor 0.1 of false positive rate. - * (More accurately: 3/(log(256)*log(2)) * log(1/fpRate) * nElements bytes) + * For example, if we want 1000 elements, we'd need: + * - ~1800 bytes for a false positive rate of 0.1 + * - ~3600 bytes for a false positive rate of 0.01 + * - ~5400 bytes for a false positive rate of 0.001 + * + * If we make these simplifying assumptions: + * - logFpRate / log(0.5) doesn't get rounded or clamped in the nHashFuncs calculation + * - nElements is even, so that nEntriesPerGeneration == nElements / 2 + * + * Then we get a more accurate estimate for filter bytes: + * + * 3/(log(256)*log(2)) * log(1/fpRate) * nElements */ class CRollingBloomFilter {