Merge pull request #6477 from nuttycom/hotfix-v5.4.2

Back-merge hotfixes v5.3.3 and v5.4.2 to master

README.md

@@ -1,4 +1,4 @@
-Zcash 5.4.1
+Zcash 5.4.2
 <img align="right" width="120" height="80" src="doc/imgs/logo.png">
 ===========
 

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)))

contrib/debian/changelog

@@ -1,3 +1,9 @@
+zcash (5.4.2) stable; urgency=high
+
+  * 5.4.2 release.
+
+ -- Electric Coin Company <team@electriccoin.co>  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 <team@electriccoin.co>  Thu, 19 Jan 2023 22:57:59 +0000
 
+zcash (5.3.3) stable; urgency=high
+
+  * 5.3.3 release.
+
+ -- Electric Coin Company <team@electriccoin.co>  Mon, 20 Feb 2023 19:47:26 -0700
+
 zcash (5.3.2) stable; urgency=medium
 
   * 5.3.2 release.

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:

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:

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)

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 <https://z.cash/support/security/>.

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] <hex\-tx> [commands]

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]

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 <https://z.cash/support/security/>.

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
+

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
+

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

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)));
+    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();
 }
 
 /* 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<unsigned char>& 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<unsigned char>& 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<uint64_t>().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<unsigned int>::max());
     nEntriesThisGeneration = 0;
     nGeneration = 1;

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;

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

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.");

src/main.cpp

@@ -38,6 +38,7 @@
 #include "wallet/asyncrpcoperation_shieldcoinbase.h"
 #include "warnings.h"
 
+#include <algorithm>
 #include <atomic>
 #include <sstream>
 #include <variant>
@@ -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<CAddress> 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<double>(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<CInv> 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<size_t>(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);
                 }
             }
         }

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))

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;

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<int> 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<CAddress> vAddrToSend;
-    CRollingBloomFilter addrKnown;
     bool fGetAddr;
     std::set<uint256> setKnown;
     int64_t nNextAddrSend;
     int64_t nNextLocalAddrSend;
 
-    // inventory based relay
+    /** Number of addr messages that can be processed from this peer. Start at 1 to
-    CRollingBloomFilter filterInventoryKnown;
+     *  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<uint64_t> m_addr_rate_limited{0};
+    /** Total number of addresses that were processed (excludes rate limited ones). */
+    std::atomic<uint64_t> 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<uint256> 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<uint256> vInventoryBlockToSend;
-    CCriticalSection cs_inventory;
+    mutable CCriticalSection cs_inventory;
     std::set<WTxId> setAskFor;
     std::multimap<int64_t, CInv> mapAskFor;
     int64_t nNextInvSend;
@@ -448,10 +470,25 @@ public:
     }
 
 
-
+    bool AddAddressIfNotAlreadyKnown(const CAddress& addr)
-    void AddAddressKnown(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);

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);

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<unsigned char> 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()

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();

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<MerkleFrontiers> 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<MinerAddress>&) {};
-    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<void (const uint256 &)> UpdatedTransaction;
     /** Notifies listeners of a change to the tip of the active block chain. */
     boost::signals2::signal<void (const CBlockIndex *, const CBlock *, std::optional<MerkleFrontiers>)> ChainTip;
-    /** Notifies listeners about an inventory item being seen on the network. */
-    boost::signals2::signal<void (const uint256 &)> Inventory;
     /** Tells listeners to broadcast their data. */
     boost::signals2::signal<void (int64_t nBestBlockTime)> Broadcast;
     /** Notifies listeners of a block validation result */
     boost::signals2::signal<void (const CBlock&, const CValidationState&)> BlockChecked;
     /** Notifies listeners that an address for mining is required (coinbase) */
     boost::signals2::signal<void (std::optional<MinerAddress>&)> AddressForMining;
-    /** Notifies listeners that a block has been successfully mined */
-    boost::signals2::signal<void (const uint256 &)> BlockFound;
 };
 
 CMainSignals& GetMainSignals();

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<uint256, int>::const_iterator mi = pwallet->mapRequestCount.find(hashBlock);
-                if (mi != pwallet->mapRequestCount.end())
-                    nRequests = (*mi).second;
-            }
-        }
-        else
-        {
-            // Did anyone request this transaction?
-            map<uint256, int>::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<uint256, int>::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<COutputEntry>& listReceived,
                            std::list<COutputEntry>& listSent, CAmount& nFee, const isminefilter& filter) const
@@ -5917,9 +5878,6 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, std::optional<std::reference_
                 delete pwalletdb;
         }
 
-        // Track how many getdata requests our transaction gets
-        mapRequestCount[wtxNew.GetHash()] = 0;
-
         if (fBroadcastTransactions)
         {
             // Broadcast

src/wallet/wallet.h

@@ -691,7 +691,6 @@ public:
     bool IsTrusted(const std::optional<int>& asOfHeight) const;
 
     int64_t GetTxTime() const;
-    int GetRequestCount() const;
 
     bool RelayWalletTransaction();
 
@@ -1476,7 +1475,6 @@ public:
     TxItems wtxOrdered;
 
     int64_t nOrderPosNext;
-    std::map<uint256, int> mapRequestCount;
 
     std::map<CTxDestination, CAddressBookData> mapAddressBook;
 
@@ -2031,22 +2029,7 @@ public:
 
     void UpdatedTransaction(const uint256 &hashTx);
 
-    void Inventory(const uint256 &hash)
-    {
-        {
-            LOCK(cs_wallet);
-            std::map<uint256, int>::iterator mi = mapRequestCount.find(hash);
-            if (mi != mapRequestCount.end())
-                (*mi).second++;
-        }
-    }
-
     void GetAddressForMining(std::optional<MinerAddress> &minerAddress);
-    void ResetRequestCount(const uint256 &hash)
-    {
-        LOCK(cs_wallet);
-        mapRequestCount[hash] = 0;
-    };
 
     unsigned int GetKeyPoolSize()
     {