Merge bitcoin/bitcoin#21992: p2p: Remove -feefilter option

a7a43e8fe85f6247c35d7ff99f36448574f3e34a Factor feefilter logic out (amadeuszpawlik)
c0385f10a133d5d8a4c296e7b7a6d75c9c4eec12 Remove -feefilter option (amadeuszpawlik)

Pull request description:

  net: Remove -feefilter option, as it is debug only and isn't used in any tests. Checking this option for every peer on every iteration of the message handler is unnecessary, as described in #21545.
  refactor: Move feefilter logic out into a separate `MaybeSendFeefilter(...)` function to improve readability of the already long `SendMessages(...)`. fixes  #21545

  The configuration option `-feefilter` has been added in 9e072a6e66efbda7d39bf61eded21d2b324323be: _"Implement "feefilter" P2P message"_
  According to the [BIP133](https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki), turning the fee filter off was ment for:
  > [...] a node [...] using prioritisetransaction to accept transactions whose actual fee rates might fall below the node's mempool min fee [in order to] disable the fee filter to make sure it is exposed to all possible txid's

  `-feefilter` was subsequently set as debug only in #8150, with the motivation that the help message was too difficult to translate.

ACKs for top commit:
  jnewbery:
    Code review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a
  promag:
    Code review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a.
  MarcoFalke:
    review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a 🦁

Tree-SHA512: 8ef9a2f255597c0279d3047dcc968fd30fb7402e981b69206d08eed452c705ed568c24e646e98d06eac118eddd09205b584f45611d1c874abf38f48b08b67630

3 files changed, 47 insertions, 43 deletions

diff --git a/src/init.cpp b/src/init.cpp
index 593128747e..a835df0572 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -390,7 +390,6 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (%d to %d, default: %d). In addition, unused mempool memory is shared for this cache (see -maxmempool).", nMinDbCache, nMaxDbCache, nDefaultDbCache), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    argsman.AddArg("-feefilter", strprintf("Tell other nodes to filter invs to us by our mempool min fee (default: %u)", DEFAULT_FEEFILTER), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     argsman.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-loadblock=<file>", "Imports blocks from external file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-maxmempool=<n>", strprintf("Keep the transaction memory pool below <n> megabytes (default: %u)", DEFAULT_MAX_MEMPOOL_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index a3f4b29cd3..0b83f756b3 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -364,6 +364,9 @@ private:
      */
     void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable);
 
+    /** Send `feefilter` message. */
+    void MaybeSendFeefilter(CNode& node, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+
     const CChainParams& m_chainparams;
     CConnman& m_connman;
     CAddrMan& m_addrman;
@@ -4276,6 +4279,49 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros
     }
 }
 
+void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, std::chrono::microseconds current_time)
+{
+    AssertLockHeld(cs_main);
+
+    if (m_ignore_incoming_txs) return;
+    if (!pto.m_tx_relay) return;
+    if (pto.GetCommonVersion() < FEEFILTER_VERSION) return;
+    // peers with the forcerelay permission should not filter txs to us
+    if (pto.HasPermission(NetPermissionFlags::ForceRelay)) return;
+
+    CAmount currentFilter = m_mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK();
+    static FeeFilterRounder g_filter_rounder{CFeeRate{DEFAULT_MIN_RELAY_TX_FEE}};
+
+    if (m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
+        // Received tx-inv messages are discarded when the active
+        // chainstate is in IBD, so tell the peer to not send them.
+        currentFilter = MAX_MONEY;
+    } else {
+        static const CAmount MAX_FILTER{g_filter_rounder.round(MAX_MONEY)};
+        if (pto.m_tx_relay->lastSentFeeFilter == MAX_FILTER) {
+            // Send the current filter if we sent MAX_FILTER previously
+            // and made it out of IBD.
+            pto.m_tx_relay->m_next_send_feefilter = 0us;
+        }
+    }
+    if (current_time > pto.m_tx_relay->m_next_send_feefilter) {
+        CAmount filterToSend = g_filter_rounder.round(currentFilter);
+        // We always have a fee filter of at least minRelayTxFee
+        filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK());
+        if (filterToSend != pto.m_tx_relay->lastSentFeeFilter) {
+            m_connman.PushMessage(&pto, CNetMsgMaker(pto.GetCommonVersion()).Make(NetMsgType::FEEFILTER, filterToSend));
+            pto.m_tx_relay->lastSentFeeFilter = filterToSend;
+        }
+        pto.m_tx_relay->m_next_send_feefilter = PoissonNextSend(current_time, AVG_FEEFILTER_BROADCAST_INTERVAL);
+    }
+    // If the fee filter has changed substantially and it's still more than MAX_FEEFILTER_CHANGE_DELAY
+    // until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY.
+    else if (current_time + MAX_FEEFILTER_CHANGE_DELAY < pto.m_tx_relay->m_next_send_feefilter &&
+                (currentFilter < 3 * pto.m_tx_relay->lastSentFeeFilter / 4 || currentFilter > 4 * pto.m_tx_relay->lastSentFeeFilter / 3)) {
+        pto.m_tx_relay->m_next_send_feefilter = current_time + GetRandomDuration<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY);
+    }
+}
+
 namespace {
 class CompareInvMempoolOrder
 {
@@ -4762,46 +4808,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
         if (!vGetData.empty())
             m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData));
 
-        //
-        // Message: feefilter
-        //
-        if (pto->m_tx_relay != nullptr &&
-            !m_ignore_incoming_txs &&
-            pto->GetCommonVersion() >= FEEFILTER_VERSION &&
-            gArgs.GetBoolArg("-feefilter", DEFAULT_FEEFILTER) &&
-            !pto->HasPermission(NetPermissionFlags::ForceRelay) // peers with the forcerelay permission should not filter txs to us
-        ) {
-            CAmount currentFilter = m_mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK();
-            static FeeFilterRounder g_filter_rounder{CFeeRate{DEFAULT_MIN_RELAY_TX_FEE}};
-            if (m_chainman.ActiveChainstate().IsInitialBlockDownload()) {
-                // Received tx-inv messages are discarded when the active
-                // chainstate is in IBD, so tell the peer to not send them.
-                currentFilter = MAX_MONEY;
-            } else {
-                static const CAmount MAX_FILTER{g_filter_rounder.round(MAX_MONEY)};
-                if (pto->m_tx_relay->lastSentFeeFilter == MAX_FILTER) {
-                    // Send the current filter if we sent MAX_FILTER previously
-                    // and made it out of IBD.
-                    pto->m_tx_relay->m_next_send_feefilter = 0us;
-                }
-            }
-            if (current_time > pto->m_tx_relay->m_next_send_feefilter) {
-                CAmount filterToSend = g_filter_rounder.round(currentFilter);
-                // We always have a fee filter of at least minRelayTxFee
-                filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK());
-                if (filterToSend != pto->m_tx_relay->lastSentFeeFilter) {
-                    m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::FEEFILTER, filterToSend));
-                    pto->m_tx_relay->lastSentFeeFilter = filterToSend;
-                }
-                pto->m_tx_relay->m_next_send_feefilter = PoissonNextSend(current_time, AVG_FEEFILTER_BROADCAST_INTERVAL);
-            }
-            // If the fee filter has changed substantially and it's still more than MAX_FEEFILTER_CHANGE_DELAY
-            // until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY.
-            else if (current_time + MAX_FEEFILTER_CHANGE_DELAY < pto->m_tx_relay->m_next_send_feefilter &&
-                     (currentFilter < 3 * pto->m_tx_relay->lastSentFeeFilter / 4 || currentFilter > 4 * pto->m_tx_relay->lastSentFeeFilter / 3)) {
-                pto->m_tx_relay->m_next_send_feefilter = current_time + GetRandomDuration<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY);
-            }
-        }
+        MaybeSendFeefilter(*pto, current_time);
     } // release cs_main
     return true;
 }
diff --git a/src/validation.h b/src/validation.h
index 1b50644185..adc3d282b6 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -82,8 +82,6 @@ static constexpr bool DEFAULT_COINSTATSINDEX{false};
 static const char* const DEFAULT_BLOCKFILTERINDEX = "0";
 /** Default for -persistmempool */
 static const bool DEFAULT_PERSIST_MEMPOOL = true;
-/** Default for using fee filter */
-static const bool DEFAULT_FEEFILTER = true;
 /** Default for -stopatheight */
 static const int DEFAULT_STOPATHEIGHT = 0;
 /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of ::ChainActive().Tip() will not be pruned. */