diff --git a/README.md b/README.md index 7f7fbcc16..ff9120a25 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -Zcash 5.4.1 +Zcash 5.4.2 =========== diff --git a/configure.ac b/configure.ac index 8a043c318..4a88acac7 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ dnl require autoconf 2.60 (AS_ECHO/AS_ECHO_N) AC_PREREQ([2.60]) define(_CLIENT_VERSION_MAJOR, 5) define(_CLIENT_VERSION_MINOR, 4) -define(_CLIENT_VERSION_REVISION, 1) +define(_CLIENT_VERSION_REVISION, 2) define(_CLIENT_VERSION_BUILD, 50) define(_ZC_BUILD_VAL, m4_if(m4_eval(_CLIENT_VERSION_BUILD < 25), 1, m4_incr(_CLIENT_VERSION_BUILD), m4_eval(_CLIENT_VERSION_BUILD < 50), 1, m4_eval(_CLIENT_VERSION_BUILD - 24), m4_eval(_CLIENT_VERSION_BUILD == 50), 1, , m4_eval(_CLIENT_VERSION_BUILD - 50))) define(_CLIENT_VERSION_SUFFIX, m4_if(m4_eval(_CLIENT_VERSION_BUILD < 25), 1, _CLIENT_VERSION_REVISION-beta$1, m4_eval(_CLIENT_VERSION_BUILD < 50), 1, _CLIENT_VERSION_REVISION-rc$1, m4_eval(_CLIENT_VERSION_BUILD == 50), 1, _CLIENT_VERSION_REVISION, _CLIENT_VERSION_REVISION-$1))) diff --git a/contrib/debian/changelog b/contrib/debian/changelog index 1e9390894..30b830de1 100644 --- a/contrib/debian/changelog +++ b/contrib/debian/changelog @@ -1,3 +1,9 @@ +zcash (5.4.2) stable; urgency=high + + * 5.4.2 release. + + -- Electric Coin Company Mon, 20 Feb 2023 20:04:31 -0700 + zcash (5.4.1) stable; urgency=medium * 5.4.1 release. @@ -34,6 +40,12 @@ zcash (5.4.0~rc1) stable; urgency=medium -- Electric Coin Company Thu, 19 Jan 2023 22:57:59 +0000 +zcash (5.3.3) stable; urgency=high + + * 5.3.3 release. + + -- Electric Coin Company Mon, 20 Feb 2023 19:47:26 -0700 + zcash (5.3.2) stable; urgency=medium * 5.3.2 release. diff --git a/contrib/gitian-descriptors/gitian-linux-parallel.yml b/contrib/gitian-descriptors/gitian-linux-parallel.yml index 11eadc822..c868dab4c 100644 --- a/contrib/gitian-descriptors/gitian-linux-parallel.yml +++ b/contrib/gitian-descriptors/gitian-linux-parallel.yml @@ -1,5 +1,5 @@ --- -name: "zcash-5.4.1" +name: "zcash-5.4.2" enable_cache: true distro: "debian" suites: diff --git a/contrib/gitian-descriptors/gitian-linux.yml b/contrib/gitian-descriptors/gitian-linux.yml index cd40a4148..196a7644a 100644 --- a/contrib/gitian-descriptors/gitian-linux.yml +++ b/contrib/gitian-descriptors/gitian-linux.yml @@ -1,5 +1,5 @@ --- -name: "zcash-5.4.1" +name: "zcash-5.4.2" enable_cache: true distro: "debian" suites: diff --git a/doc/authors.md b/doc/authors.md index 78ffe3c31..80b750bae 100644 --- a/doc/authors.md +++ b/doc/authors.md @@ -2,13 +2,13 @@ Zcash Contributors ================== Jack Grigg (1297) -Kris Nuttycombe (618) +Kris Nuttycombe (622) Simon Liu (460) Sean Bowe (389) -Daira Hopwood (376) +Daira Hopwood (379) Eirik Ogilvie-Wigley (216) Wladimir J. van der Laan (159) -Pieter Wuille (143) +Pieter Wuille (146) Alfredo Garcia (120) Taylor Hornby (118) Marshall Gaucher (118) @@ -17,10 +17,10 @@ Marco Falke (90) Jonas Schnelli (90) Jay Graber (89) Larry Ruane (88) -Greg Pfeil (83) +Greg Pfeil (84) Cory Fields (78) sasha (62) -Matt Corallo (60) +Matt Corallo (61) Nathan Wilcox (57) practicalswift (42) Dimitris Apostolou (40) @@ -219,6 +219,7 @@ Josh Lehan (1) Josh Ellithorpe (1) Jonas Nick (1) Jon Layton (1) +Jon Atack (1) Jeffrey Walton (1) Janito Vaqueiro Ferreira Filho (1) James White (1) diff --git a/doc/man/zcash-cli.1 b/doc/man/zcash-cli.1 index f5a940622..25f8f4f51 100644 --- a/doc/man/zcash-cli.1 +++ b/doc/man/zcash-cli.1 @@ -1,9 +1,9 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.1. -.TH ZCASH-CLI "1" "February 2023" "zcash-cli v5.4.1" "User Commands" +.TH ZCASH-CLI "1" "February 2023" "zcash-cli v5.4.2" "User Commands" .SH NAME -zcash-cli \- manual page for zcash-cli v5.4.1 +zcash-cli \- manual page for zcash-cli v5.4.2 .SH DESCRIPTION -Zcash RPC client version v5.4.1 +Zcash RPC client version v5.4.2 .PP In order to ensure you are adequately protecting your privacy when using Zcash, please see . diff --git a/doc/man/zcash-tx.1 b/doc/man/zcash-tx.1 index 2d1b03cba..3727e1c58 100644 --- a/doc/man/zcash-tx.1 +++ b/doc/man/zcash-tx.1 @@ -1,9 +1,9 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.1. -.TH ZCASH-TX "1" "February 2023" "zcash-tx v5.4.1" "User Commands" +.TH ZCASH-TX "1" "February 2023" "zcash-tx v5.4.2" "User Commands" .SH NAME -zcash-tx \- manual page for zcash-tx v5.4.1 +zcash-tx \- manual page for zcash-tx v5.4.2 .SH DESCRIPTION -Zcash zcash\-tx utility version v5.4.1 +Zcash zcash\-tx utility version v5.4.2 .SS "Usage:" .TP zcash\-tx [options] [commands] diff --git a/doc/man/zcashd-wallet-tool.1 b/doc/man/zcashd-wallet-tool.1 index 43970617c..433c97c47 100644 --- a/doc/man/zcashd-wallet-tool.1 +++ b/doc/man/zcashd-wallet-tool.1 @@ -1,7 +1,7 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.1. -.TH ZCASHD-WALLET-TOOL "1" "February 2023" "zcashd-wallet-tool v5.4.1" "User Commands" +.TH ZCASHD-WALLET-TOOL "1" "February 2023" "zcashd-wallet-tool v5.4.2" "User Commands" .SH NAME -zcashd-wallet-tool \- manual page for zcashd-wallet-tool v5.4.1 +zcashd-wallet-tool \- manual page for zcashd-wallet-tool v5.4.2 .SH SYNOPSIS .B zcashd-wallet-tool [\fI\,OPTIONS\/\fR] diff --git a/doc/man/zcashd.1 b/doc/man/zcashd.1 index 4a9e7c8c5..7e8354c4b 100644 --- a/doc/man/zcashd.1 +++ b/doc/man/zcashd.1 @@ -1,9 +1,9 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man 1.49.1. -.TH ZCASHD "1" "February 2023" "zcashd v5.4.1" "User Commands" +.TH ZCASHD "1" "February 2023" "zcashd v5.4.2" "User Commands" .SH NAME -zcashd \- manual page for zcashd v5.4.1 +zcashd \- manual page for zcashd v5.4.2 .SH DESCRIPTION -Zcash Daemon version v5.4.1 +Zcash Daemon version v5.4.2 .PP In order to ensure you are adequately protecting your privacy when using Zcash, please see . diff --git a/doc/release-notes/release-notes-5.3.3.md b/doc/release-notes/release-notes-5.3.3.md new file mode 100644 index 000000000..51053c239 --- /dev/null +++ b/doc/release-notes/release-notes-5.3.3.md @@ -0,0 +1,36 @@ +Notable changes +=============== + +This hotfix remediates memory exhaustion vulnerabilities that zcashd inherited +as a fork of bitcoind. These bugs could allow an attacker to use peer-to-peer +messages to fill the memory of a node, resulting in a crash. + + +Changelog +========= + +Daira Hopwood (3): + Enable a CRollingBloomFilter to be reset to a state where it takes little memory. + Ensure that CNode::{addrKnown, filterInventoryKnown} immediately take little memory when we disconnect the node. + Improve the encapsulation of `CNode::filterInventoryKnown`. + +Greg Pfeil (1): + Remove `ResetRequestCount` + +Jon Atack (1): + p2p, rpc, test: address rate-limiting follow-ups + +Kris Nuttycombe (4): + Update release notes for v5.3.3 hotfix + Postpone dependency updates. + make-release.py: Versioning changes for 5.3.3. + make-release.py: Updated manpages for 5.3.3. + +Matt Corallo (1): + Remove useless mapRequest tracking that just effects Qt display. + +Pieter Wuille (3): + Rate limit the processing of incoming addr messages + Randomize the order of addr processing + Add logging and addr rate limiting statistics + diff --git a/doc/release-notes/release-notes-5.4.2.md b/doc/release-notes/release-notes-5.4.2.md new file mode 100644 index 000000000..0deb12c8a --- /dev/null +++ b/doc/release-notes/release-notes-5.4.2.md @@ -0,0 +1,36 @@ +Notable changes +=============== + +This hotfix remediates memory exhaustion vulnerabilities that zcashd inherited +as a fork of bitcoind. These bugs could allow an attacker to use peer-to-peer +messages to fill the memory of a node, resulting in a crash. + + +Changelog +========= + +Daira Hopwood (3): + Enable a CRollingBloomFilter to be reset to a state where it takes little memory. + Ensure that CNode::{addrKnown, filterInventoryKnown} immediately take little memory when we disconnect the node. + Improve the encapsulation of `CNode::filterInventoryKnown`. + +Greg Pfeil (1): + Remove `ResetRequestCount` + +Jon Atack (1): + p2p, rpc, test: address rate-limiting follow-ups + +Kris Nuttycombe (4): + Update release notes for v5.3.3 hotfix + Postpone dependency updates for v5.4.2 hotfix. + make-release.py: Versioning changes for 5.4.2. + make-release.py: Updated manpages for 5.4.2. + +Matt Corallo (1): + Remove useless mapRequest tracking that just effects Qt display. + +Pieter Wuille (3): + Rate limit the processing of incoming addr messages + Randomize the order of addr processing + Add logging and addr rate limiting statistics + diff --git a/qa/zcash/postponed-updates.txt b/qa/zcash/postponed-updates.txt index 81db7c23c..f97d6d888 100644 --- a/qa/zcash/postponed-updates.txt +++ b/qa/zcash/postponed-updates.txt @@ -19,4 +19,4 @@ native_clang 15.0.7 2023-04-30 leveldb 1.23 2023-06-01 # We're never updating to this version -bdb 18.1.40 2024-02-01 +bdb 18.1.40 2024-03-01 diff --git a/src/bloom.cpp b/src/bloom.cpp index c97bfd61b..86bb99d18 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -187,15 +187,7 @@ CRollingBloomFilter::CRollingBloomFilter(const unsigned int nElements, const dou * => 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(); - /* 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(); + nFilterBits = (uint32_t)ceil(-1.0 * nHashFuncs * nMaxElements / log(1.0 - exp(logFpRate / nHashFuncs))); } /* Similar to CBloomFilter::Hash */ @@ -213,6 +205,9 @@ static inline uint32_t FastMod(uint32_t x, size_t n) { void CRollingBloomFilter::insert(const std::vector& vKey) { + if (data.empty()) { + initialize(); + } if (nEntriesThisGeneration == nEntriesPerGeneration) { nEntriesThisGeneration = 0; nGeneration++; @@ -250,6 +245,9 @@ void CRollingBloomFilter::insert(const uint256& hash) bool CRollingBloomFilter::contains(const std::vector& vKey) const { + if (data.empty()) { + return false; + } for (int n = 0; n < nHashFuncs; n++) { uint32_t h = RollingBloomHash(n, nTweak, vKey); int bit = h & 0x3F; @@ -268,8 +266,19 @@ bool CRollingBloomFilter::contains(const uint256& hash) const return contains(vData); } -void CRollingBloomFilter::reset() +void CRollingBloomFilter::reset() { + std::vector().swap(data); +} + +void CRollingBloomFilter::initialize() { + /* 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); + nTweak = GetRand(std::numeric_limits::max()); nEntriesThisGeneration = 0; nGeneration = 1; diff --git a/src/bloom.h b/src/bloom.h index f1cecceb0..9c9687a8c 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -131,7 +131,13 @@ public: void reset(); +protected: + bool is_data_empty() const { return data.empty(); } + private: + void initialize(); + + uint32_t nFilterBits; int nEntriesPerGeneration; int nEntriesThisGeneration; int nGeneration; diff --git a/src/clientversion.h b/src/clientversion.h index 69498dbee..b898e3dbc 100644 --- a/src/clientversion.h +++ b/src/clientversion.h @@ -17,7 +17,7 @@ //! These need to be macros, as clientversion.cpp's and bitcoin*-res.rc's voodoo requires it #define CLIENT_VERSION_MAJOR 5 #define CLIENT_VERSION_MINOR 4 -#define CLIENT_VERSION_REVISION 1 +#define CLIENT_VERSION_REVISION 2 #define CLIENT_VERSION_BUILD 50 //! Set to true for release, false for prerelease or test build diff --git a/src/deprecation.h b/src/deprecation.h index 614d705c4..e98e935da 100644 --- a/src/deprecation.h +++ b/src/deprecation.h @@ -12,7 +12,7 @@ // Per https://zips.z.cash/zip-0200 // Shut down nodes running this version of code, 16 weeks' worth of blocks after the estimated // release block height. A warning is shown during the 14 days' worth of blocks prior to shut down. -static const int APPROX_RELEASE_HEIGHT = 1983000; +static const int APPROX_RELEASE_HEIGHT = 1992000; static const int RELEASE_TO_DEPRECATION_WEEKS = 16; static const int EXPECTED_BLOCKS_PER_HOUR = 3600 / Consensus::POST_BLOSSOM_POW_TARGET_SPACING; static_assert(EXPECTED_BLOCKS_PER_HOUR == 48, "The value of Consensus::POST_BLOSSOM_POW_TARGET_SPACING was chosen such that this assertion holds."); diff --git a/src/main.cpp b/src/main.cpp index db0cac599..5daaf90cb 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -38,6 +38,7 @@ #include "wallet/asyncrpcoperation_shieldcoinbase.h" #include "warnings.h" +#include #include #include #include @@ -6561,9 +6562,6 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam } } - // Track requests for our stuff. - GetMainSignals().Inventory(inv.hash); - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK) break; } @@ -6730,6 +6728,10 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string { pfrom->PushMessage("getaddr"); pfrom->fGetAddr = true; + + // When requesting a getaddr, accept an additional MAX_ADDR_TO_SEND addresses in response + // (bypassing the MAX_ADDR_PROCESSING_TOKEN_BUCKET limit). + pfrom->m_addr_token_bucket += MAX_ADDR_TO_SEND; } addrman.Good(pfrom->addr); } else { @@ -6822,13 +6824,38 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string vector vAddrOk; int64_t nNow = GetTime(); int64_t nSince = nNow - 10 * 60; + + // Update/increment addr rate limiting bucket. + const int64_t current_time = GetTimeMicros(); + if (pfrom->m_addr_token_bucket < MAX_ADDR_PROCESSING_TOKEN_BUCKET) { + // Don't increment bucket if it's already full + const auto time_diff = std::max(current_time - pfrom->m_addr_token_timestamp, (int64_t) 0); + const double increment = (time_diff / 1000000) * MAX_ADDR_RATE_PER_SECOND; + pfrom->m_addr_token_bucket = std::min(pfrom->m_addr_token_bucket + increment, MAX_ADDR_PROCESSING_TOKEN_BUCKET); + } + pfrom->m_addr_token_timestamp = current_time; + + uint64_t num_proc = 0; + uint64_t num_rate_limit = 0; + std::shuffle(vAddr.begin(), vAddr.end(), ZcashRandomEngine()); for (CAddress& addr : vAddr) { boost::this_thread::interruption_point(); + // Apply rate limiting if the address is not whitelisted + if (pfrom->m_addr_token_bucket < 1.0) { + if (!pfrom->IsWhitelistedRange(addr)) { + ++num_rate_limit; + continue; + } + } else { + pfrom->m_addr_token_bucket -= 1.0; + } + if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60) addr.nTime = nNow - 5 * 24 * 60 * 60; - pfrom->AddAddressKnown(addr); + pfrom->AddAddressIfNotAlreadyKnown(addr); + ++num_proc; bool fReachable = IsReachable(addr); if (addr.nTime > nSince && !pfrom->fGetAddr && vAddr.size() <= 10 && addr.IsRoutable()) { @@ -6859,6 +6886,15 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string if (fReachable) vAddrOk.push_back(addr); } + pfrom->m_addr_processed += num_proc; + pfrom->m_addr_rate_limited += num_rate_limit; + LogPrintf("ProcessMessage: Received addr: %u addresses (%u processed, %u rate-limited) from peer=%d%s\n", + vAddr.size(), + num_proc, + num_rate_limit, + pfrom->GetId(), + fLogIPs ? ", peeraddr=" + pfrom->addr.ToString() : ""); + addrman.Add(vAddrOk, pfrom->addr, 2 * 60 * 60); if (vAddr.size() < 1000) pfrom->fGetAddr = false; @@ -6923,16 +6959,13 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string } else { - pfrom->AddKnownTx(WTxId(inv.hash, inv.hashAux)); + pfrom->AddKnownWTxId(WTxId(inv.hash, inv.hashAux)); if (fBlocksOnly) LogPrint("net", "transaction (%s) inv sent in violation of protocol peer=%d\n", inv.hash.ToString(), pfrom->id); else if (!fAlreadyHave && !IsInitialBlockDownload(chainparams.GetConsensus())) pfrom->AskFor(inv); } - // Track requests for our stuff - GetMainSignals().Inventory(inv.hash); - if (pfrom->nSendSize > (SendBufferSize() * 2)) { Misbehaving(pfrom->GetId(), 50); return error("send buffer size() = %u", pfrom->nSendSize); @@ -7072,7 +7105,7 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string LOCK(cs_main); - pfrom->AddKnownTx(wtxid); + pfrom->AddKnownWTxId(wtxid); bool fMissingInputs = false; CValidationState state; @@ -7769,9 +7802,8 @@ bool SendMessages(const Consensus::Params& params, CNode* pto) vAddr.reserve(pto->vAddrToSend.size()); for (const CAddress& addr : pto->vAddrToSend) { - if (!pto->addrKnown.contains(addr.GetKey())) + if (pto->AddAddressIfNotAlreadyKnown(addr)) { - pto->addrKnown.insert(addr.GetKey()); vAddr.push_back(addr); // receiver rejects addr messages larger than 1000 if (vAddr.size() >= 1000) @@ -7844,6 +7876,12 @@ bool SendMessages(const Consensus::Params& params, CNode* pto) vector vInv; { LOCK(pto->cs_inventory); + // Avoid possibly adding to pto->filterInventoryKnown after it has been reset in CloseSocketDisconnect. + if (pto->fDisconnect) { + // We can safely return here because SendMessages would, in any case, do nothing after + // this block if pto->fDisconnect is set. + return true; + } vInv.reserve(std::max(pto->vInventoryBlockToSend.size(), INVENTORY_BROADCAST_MAX)); // Add blocks @@ -7891,7 +7929,7 @@ bool SendMessages(const Consensus::Params& params, CNode* pto) if (pto->pfilter) { if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; } - pto->filterInventoryKnown.insert(hash); + pto->AddKnownTxId(hash); vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { pto->PushMessage("inv", vInv); @@ -7926,7 +7964,7 @@ bool SendMessages(const Consensus::Params& params, CNode* pto) // Remove it from the to-be-sent set pto->setInventoryTxToSend.erase(it); // Check if not in the filter already - if (pto->filterInventoryKnown.contains(hash)) { + if (pto->HasKnownTxId(hash)) { continue; } // Not in the mempool anymore? don't bother sending it. @@ -7961,7 +7999,7 @@ bool SendMessages(const Consensus::Params& params, CNode* pto) pto->PushMessage("inv", vInv); vInv.clear(); } - pto->filterInventoryKnown.insert(hash); + pto->AddKnownTxId(hash); } } } diff --git a/src/miner.cpp b/src/miner.cpp index 6a2cb2a38..f8602cc38 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -890,9 +890,6 @@ static bool ProcessBlockFound(const CBlock* pblock, const CChainParams& chainpar return error("ZcashMiner: generated block is stale"); } - // Inform about the new block - GetMainSignals().BlockFound(pblock->GetHash()); - // Process this block the same as if we had received it from another node CValidationState state; if (!ProcessNewBlock(state, chainparams, NULL, pblock, true, NULL)) diff --git a/src/net.cpp b/src/net.cpp index 4086cc98b..9877b3a52 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -440,6 +440,14 @@ void CNode::CloseSocketDisconnect() CloseSocket(hSocket); } } + { + LOCK(cs_addrKnown); + addrKnown.reset(); + } + { + LOCK(cs_inventory); + filterInventoryKnown.reset(); + } // in case this fails, we'll empty the recv buffer when the CNode is deleted TRY_LOCK(cs_vRecvMsg, lockRecv); @@ -700,6 +708,9 @@ void CNode::copyStats(CNodeStats &stats) stats.dPingTime = (((double)nPingUsecTime) / 1e6); stats.dPingWait = (((double)nPingUsecWait) / 1e6); + stats.m_addr_processed = m_addr_processed.load(); + stats.m_addr_rate_limited = m_addr_rate_limited.load(); + // Leave string empty if addrLocal invalid (not filled in yet) CService addrLocalUnlocked = GetAddrLocal(); stats.addrLocal = addrLocalUnlocked.IsValid() ? addrLocalUnlocked.ToString() : ""; @@ -2221,7 +2232,6 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa nSendOffset = 0; hashContinue = uint256(); nStartingHeight = -1; - filterInventoryKnown.reset(); fSendMempool = false; fGetAddr = false; nNextLocalAddrSend = 0; diff --git a/src/net.h b/src/net.h index cc2b9f35e..51b902202 100644 --- a/src/net.h +++ b/src/net.h @@ -50,6 +50,13 @@ static const int TIMEOUT_INTERVAL = 20 * 60; static const unsigned int MAX_INV_SZ = 50000; /** The maximum number of new addresses to accumulate before announcing. */ static const unsigned int MAX_ADDR_TO_SEND = 1000; +/** The maximum rate of address records we're willing to process on average. Can be bypassed using + * the NetPermissionFlags::Addr permission. */ +static constexpr double MAX_ADDR_RATE_PER_SECOND{0.1}; +/** The soft limit of the address processing token bucket (the regular MAX_ADDR_RATE_PER_SECOND + * based increments won't go above this, but the MAX_ADDR_TO_SEND increment following GETADDR + * is exempt from this limit. */ +static constexpr size_t MAX_ADDR_PROCESSING_TOKEN_BUCKET{MAX_ADDR_TO_SEND}; /** Maximum length of incoming protocol messages (no message over 2 MiB is currently acceptable). */ static const unsigned int MAX_PROTOCOL_MESSAGE_LENGTH = 2 * 1024 * 1024; /** Maximum length of strSubVer in `version` message */ @@ -205,6 +212,8 @@ public: double dPingTime; double dPingWait; std::string addrLocal; + uint64_t m_addr_processed{0}; + uint64_t m_addr_rate_limited{0}; }; @@ -304,6 +313,12 @@ public: CBloomFilter* pfilter; NodeId id; std::atomic nRefCount; + CRollingBloomFilter addrKnown; + mutable CCriticalSection cs_addrKnown; + + // Inventory based relay + // This filter is protected by cs_inventory and contains both txids and wtxids. + CRollingBloomFilter filterInventoryKnown; const uint64_t nKeyedNetGroup; @@ -333,22 +348,29 @@ public: // flood relay std::vector vAddrToSend; - CRollingBloomFilter addrKnown; bool fGetAddr; std::set setKnown; int64_t nNextAddrSend; int64_t nNextLocalAddrSend; - // inventory based relay - CRollingBloomFilter filterInventoryKnown; + /** Number of addr messages that can be processed from this peer. Start at 1 to + * permit self-announcement. */ + double m_addr_token_bucket{1.0}; + /** When m_addr_token_bucket was last updated */ + int64_t m_addr_token_timestamp{GetTimeMicros()}; + /** Total number of addresses that were dropped due to rate limiting. */ + std::atomic m_addr_rate_limited{0}; + /** Total number of addresses that were processed (excludes rate limited ones). */ + std::atomic m_addr_processed{0}; + // Set of transaction ids we still have to announce. // They are sorted by the mempool before relay, so the order is not important. std::set setInventoryTxToSend; - // List of block ids we still have announce. + // List of block ids we still have to announce. // There is no final sorting before sending, as they are always sent immediately // and in the order requested. std::vector vInventoryBlockToSend; - CCriticalSection cs_inventory; + mutable CCriticalSection cs_inventory; std::set setAskFor; std::multimap mapAskFor; int64_t nNextInvSend; @@ -448,10 +470,25 @@ public: } - - void AddAddressKnown(const CAddress& addr) + bool AddAddressIfNotAlreadyKnown(const CAddress& addr) { - addrKnown.insert(addr.GetKey()); + LOCK(cs_addrKnown); + // Avoid adding to addrKnown after it has been reset in CloseSocketDisconnect. + if (fDisconnect) { + return false; + } + if (!addrKnown.contains(addr.GetKey())) { + addrKnown.insert(addr.GetKey()); + return true; + } else { + return false; + } + } + + bool IsAddressKnown(const CAddress& addr) const + { + LOCK(cs_addrKnown); + return addrKnown.contains(addr.GetKey()); } void PushAddress(const CAddress& addr, FastRandomContext &insecure_rand) @@ -459,7 +496,7 @@ public: // Known checking here is only to save space from duplicates. // SendMessages will filter it again for knowns that were added // after addresses were pushed. - if (addr.IsValid() && !addrKnown.contains(addr.GetKey())) { + if (addr.IsValid() && !IsAddressKnown(addr)) { if (vAddrToSend.size() >= MAX_ADDR_TO_SEND) { vAddrToSend[insecure_rand.randrange(vAddrToSend.size())] = addr; } else { @@ -469,18 +506,32 @@ public: } - void AddKnownTx(const WTxId& wtxid) + void AddKnownWTxId(const WTxId& wtxid) { - { - LOCK(cs_inventory); + LOCK(cs_inventory); + if (!fDisconnect) { filterInventoryKnown.insert(wtxid.ToBytes()); } } + void AddKnownTxId(const uint256& txid) + { + LOCK(cs_inventory); + if (!fDisconnect) { + filterInventoryKnown.insert(txid); + } + } + + bool HasKnownTxId(const uint256& txid) const + { + LOCK(cs_inventory); + return filterInventoryKnown.contains(txid); + } + void PushTxInventory(const WTxId& wtxid) { LOCK(cs_inventory); - if (!filterInventoryKnown.contains(wtxid.ToBytes())) { + if (!fDisconnect && !filterInventoryKnown.contains(wtxid.ToBytes())) { setInventoryTxToSend.insert(wtxid.hash); } } @@ -488,7 +539,9 @@ public: void PushBlockInventory(const uint256& hash) { LOCK(cs_inventory); - vInventoryBlockToSend.push_back(hash); + if (!fDisconnect) { + vInventoryBlockToSend.push_back(hash); + } } void AskFor(const CInv& inv); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index b8e5a217f..f9b4ff4b5 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -158,6 +158,8 @@ UniValue getpeerinfo(const UniValue& params, bool fHelp) } obj.pushKV("inflight", heights); } + obj.pushKV("addr_processed", stats.m_addr_processed); + obj.pushKV("addr_rate_limited", stats.m_addr_rate_limited); obj.pushKV("whitelisted", stats.fWhitelisted); ret.push_back(obj); diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index ae375a1e7..4e4f93dfa 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -538,4 +538,30 @@ BOOST_AUTO_TEST_CASE(rolling_bloom) } } +BOOST_AUTO_TEST_CASE(rolling_bloom_reset) +{ + struct TestRollingBloomFilter : CRollingBloomFilter + { + TestRollingBloomFilter() : CRollingBloomFilter(100, 0.01) {} + bool is_data_empty() const { return CRollingBloomFilter::is_data_empty(); } + }; + + TestRollingBloomFilter rb; + BOOST_CHECK(rb.is_data_empty()); + + std::vector d = RandomData(); + rb.insert(d); + BOOST_CHECK(!rb.is_data_empty()); + BOOST_CHECK(rb.contains(d)); + + // reset() should ensure minimal memory usage. + rb.reset(); + BOOST_CHECK(rb.is_data_empty()); + BOOST_CHECK(!rb.contains(d)); + + rb.insert(d); + BOOST_CHECK(!rb.is_data_empty()); + BOOST_CHECK(rb.contains(d)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 21de7514c..599f85ed0 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -37,19 +37,15 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.EraseTransaction.connect(boost::bind(&CValidationInterface::EraseFromWallet, pwalletIn, _1)); g_signals.UpdatedTransaction.connect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); g_signals.ChainTip.connect(boost::bind(&CValidationInterface::ChainTip, pwalletIn, _1, _2, _3)); - g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1)); g_signals.BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); g_signals.AddressForMining.connect(boost::bind(&CValidationInterface::GetAddressForMining, pwalletIn, _1)); - g_signals.BlockFound.connect(boost::bind(&CValidationInterface::ResetRequestCount, pwalletIn, _1)); } void UnregisterValidationInterface(CValidationInterface* pwalletIn) { - g_signals.BlockFound.disconnect(boost::bind(&CValidationInterface::ResetRequestCount, pwalletIn, _1)); g_signals.AddressForMining.disconnect(boost::bind(&CValidationInterface::GetAddressForMining, pwalletIn, _1)); g_signals.BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1)); - g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.ChainTip.disconnect(boost::bind(&CValidationInterface::ChainTip, pwalletIn, _1, _2, _3)); g_signals.UpdatedTransaction.disconnect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); g_signals.EraseTransaction.disconnect(boost::bind(&CValidationInterface::EraseFromWallet, pwalletIn, _1)); @@ -59,11 +55,9 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { } void UnregisterAllValidationInterfaces() { - g_signals.BlockFound.disconnect_all_slots(); g_signals.AddressForMining.disconnect_all_slots(); g_signals.BlockChecked.disconnect_all_slots(); g_signals.Broadcast.disconnect_all_slots(); - g_signals.Inventory.disconnect_all_slots(); g_signals.ChainTip.disconnect_all_slots(); g_signals.UpdatedTransaction.disconnect_all_slots(); g_signals.EraseTransaction.disconnect_all_slots(); diff --git a/src/validationinterface.h b/src/validationinterface.h index 0d6f395c0..5ab13eab2 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -90,11 +90,9 @@ protected: virtual void EraseFromWallet(const uint256 &hash) {} virtual void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, std::optional added) {} virtual void UpdatedTransaction(const uint256 &hash) {} - virtual void Inventory(const uint256 &hash) {} virtual void ResendWalletTransactions(int64_t nBestBlockTime) {} virtual void BlockChecked(const CBlock&, const CValidationState&) {} virtual void GetAddressForMining(std::optional&) {}; - virtual void ResetRequestCount(const uint256 &hash) {}; friend void ::RegisterValidationInterface(CValidationInterface*); friend void ::UnregisterValidationInterface(CValidationInterface*); friend void ::UnregisterAllValidationInterfaces(); @@ -158,16 +156,12 @@ struct CMainSignals { boost::signals2::signal UpdatedTransaction; /** Notifies listeners of a change to the tip of the active block chain. */ boost::signals2::signal)> ChainTip; - /** Notifies listeners about an inventory item being seen on the network. */ - boost::signals2::signal Inventory; /** Tells listeners to broadcast their data. */ boost::signals2::signal Broadcast; /** Notifies listeners of a block validation result */ boost::signals2::signal BlockChecked; /** Notifies listeners that an address for mining is required (coinbase) */ boost::signals2::signal&)> AddressForMining; - /** Notifies listeners that a block has been successfully mined */ - boost::signals2::signal BlockFound; }; CMainSignals& GetMainSignals(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ae3b129ed..91fde8586 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4455,45 +4455,6 @@ int64_t CWalletTx::GetTxTime() const return n ? n : nTimeReceived; } -int CWalletTx::GetRequestCount() const -{ - // Returns -1 if it wasn't being tracked - int nRequests = -1; - { - LOCK(pwallet->cs_wallet); - if (IsCoinBase()) - { - // Generated block - if (!hashBlock.IsNull()) - { - map::const_iterator mi = pwallet->mapRequestCount.find(hashBlock); - if (mi != pwallet->mapRequestCount.end()) - nRequests = (*mi).second; - } - } - else - { - // Did anyone request this transaction? - map::const_iterator mi = pwallet->mapRequestCount.find(GetHash()); - if (mi != pwallet->mapRequestCount.end()) - { - nRequests = (*mi).second; - - // 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; - else - nRequests = 1; // If it's in someone else's block it must have got out - } - } - } - } - return nRequests; -} - // GetAmounts will determine the transparent debits and credits for a given wallet tx. void CWalletTx::GetAmounts(std::list& listReceived, std::list& listSent, CAmount& nFee, const isminefilter& filter) const @@ -5917,9 +5878,6 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, std::optional& asOfHeight) const; int64_t GetTxTime() const; - int GetRequestCount() const; bool RelayWalletTransaction(); @@ -1476,7 +1475,6 @@ public: TxItems wtxOrdered; int64_t nOrderPosNext; - std::map mapRequestCount; std::map mapAddressBook; @@ -2031,22 +2029,7 @@ public: void UpdatedTransaction(const uint256 &hashTx); - void Inventory(const uint256 &hash) - { - { - LOCK(cs_wallet); - std::map::iterator mi = mapRequestCount.find(hash); - if (mi != mapRequestCount.end()) - (*mi).second++; - } - } - void GetAddressForMining(std::optional &minerAddress); - void ResetRequestCount(const uint256 &hash) - { - LOCK(cs_wallet); - mapRequestCount[hash] = 0; - }; unsigned int GetKeyPoolSize() {