Merge #19914: refactor: Do not pass chain params to CheckForStaleTipAndEvictPeers twice

fa7e407b504bc60c77341f02636ed9d6a4b53d79 Do not pass chain params to CheckForStaleTipAndEvictPeers twice (MarcoFalke)

Pull request description:

  `PeerManager` already keeps a reference to the chain params as a member variable. No need to pass it in once again as a function parameter.

ACKs for top commit:
  naumenkogs:
    utACK fa7e407b504bc60c77341f02636ed9d6a4b53d79
  jnewbery:
    code review ACK fa7e407b504bc60c77341f02636ed9d6a4b53d79
  epson121:
    Code review ACK fa7e407b504bc60c77341f02636ed9d6a4b53d79

Tree-SHA512: 640c2d8adf9f1d54d0bfbdf81989064be2f5ba4b534d07d42258b372dc130f7b9c3fd087c7d28f0439678d124127f5d6f82f3139b1766f59f5ed661e7ac2a923

3 files changed, 15 insertions, 17 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 05f5188266..1d386b1ad8 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1247,13 +1247,12 @@ PeerManager::PeerManager(const CChainParams& chainparams, CConnman& connman, Ban
     // same probability that we have in the reject filter).
     g_recent_confirmed_transactions.reset(new CRollingBloomFilter(48000, 0.000001));
 
-    const Consensus::Params& consensusParams = Params().GetConsensus();
     // Stale tip checking and peer eviction are on two different timers, but we
     // don't want them to get out of sync due to drift in the scheduler, so we
     // combine them in one function and schedule at the quicker (peer-eviction)
     // timer.
     static_assert(EXTRA_PEER_CHECK_INTERVAL < STALE_CHECK_INTERVAL, "peer eviction timer should be less than stale tip check timer");
-    scheduler.scheduleEvery([this, consensusParams] { this->CheckForStaleTipAndEvictPeers(consensusParams); }, std::chrono::seconds{EXTRA_PEER_CHECK_INTERVAL});
+    scheduler.scheduleEvery([this] { this->CheckForStaleTipAndEvictPeers(); }, std::chrono::seconds{EXTRA_PEER_CHECK_INTERVAL});
 
     // schedule next run for 10-15 minutes in the future
     const std::chrono::milliseconds delta = std::chrono::minutes{10} + GetRandMillis(std::chrono::minutes{5});
@@ -1344,7 +1343,7 @@ void PeerManager::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_
         return;
     nHighestFastAnnounce = pindex->nHeight;
 
-    bool fWitnessEnabled = IsWitnessEnabled(pindex->pprev, Params().GetConsensus());
+    bool fWitnessEnabled = IsWitnessEnabled(pindex->pprev, m_chainparams.GetConsensus());
     uint256 hashBlock(pblock->GetHash());
 
     {
@@ -1572,7 +1571,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c
     } // release cs_main before calling ActivateBestChain
     if (need_activate_chain) {
         BlockValidationState state;
-        if (!ActivateBestChain(state, Params(), a_recent_block)) {
+        if (!ActivateBestChain(state, chainparams, a_recent_block)) {
             LogPrint(BCLog::NET, "failed to activate chain (%s)\n", state.ToString());
         }
     }
@@ -2786,7 +2785,7 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
                 a_recent_block = most_recent_block;
             }
             BlockValidationState state;
-            if (!ActivateBestChain(state, Params(), a_recent_block)) {
+            if (!ActivateBestChain(state, m_chainparams, a_recent_block)) {
                 LogPrint(BCLog::NET, "failed to activate chain (%s)\n", state.ToString());
             }
         }
@@ -4027,7 +4026,7 @@ void PeerManager::EvictExtraOutboundPeers(int64_t time_in_seconds)
     }
 }
 
-void PeerManager::CheckForStaleTipAndEvictPeers(const Consensus::Params &consensusParams)
+void PeerManager::CheckForStaleTipAndEvictPeers()
 {
     LOCK(cs_main);
 
@@ -4038,7 +4037,7 @@ void PeerManager::CheckForStaleTipAndEvictPeers(const Consensus::Params &consens
     if (time_in_seconds > m_stale_tip_check_time) {
         // Check whether our tip is stale, and if so, allow using an extra
         // outbound peer
-        if (!fImporting && !fReindex && m_connman.GetNetworkActive() && m_connman.GetUseAddrmanOutgoing() && TipMayBeStale(consensusParams)) {
+        if (!fImporting && !fReindex && m_connman.GetNetworkActive() && m_connman.GetUseAddrmanOutgoing() && TipMayBeStale(m_chainparams.GetConsensus())) {
             LogPrintf("Potential stale tip detected, will try using extra outbound peer (last tip update: %d seconds ago)\n", time_in_seconds - g_last_tip_update);
             m_connman.SetTryNewOutboundPeer(true);
         } else if (m_connman.GetTryNewOutboundPeer()) {
@@ -4071,7 +4070,7 @@ public:
 
 bool PeerManager::SendMessages(CNode* pto)
 {
-    const Consensus::Params& consensusParams = Params().GetConsensus();
+    const Consensus::Params& consensusParams = m_chainparams.GetConsensus();
 
     // We must call MaybeDiscourageAndDisconnect first, to ensure that we'll
     // disconnect misbehaving peers even before the version handshake is complete.
diff --git a/src/net_processing.h b/src/net_processing.h
index 520f7489e8..3e748c0c5b 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -76,7 +76,7 @@ public:
     /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */
     void ConsiderEviction(CNode& pto, int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     /** Evict extra outbound peers. If we think our tip may be stale, connect to an extra outbound */
-    void CheckForStaleTipAndEvictPeers(const Consensus::Params &consensusParams);
+    void CheckForStaleTipAndEvictPeers();
     /** If we have extra outbound peers, try to disconnect the one with the oldest block announcement */
     void EvictExtraOutboundPeers(int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     /** Retrieve unbroadcast transactions from the mempool and reattempt sending to peers */
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index f4d2204e1c..e4ee08db61 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -153,7 +153,6 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
     auto connman = MakeUnique<CConnmanTest>(0x1337, 0x1337);
     auto peerLogic = MakeUnique<PeerManager>(chainparams, *connman, nullptr, *m_node.scheduler, *m_node.chainman, *m_node.mempool);
 
-    const Consensus::Params& consensusParams = Params().GetConsensus();
     constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS;
     CConnman::Options options;
     options.nMaxConnections = DEFAULT_MAX_PEER_CONNECTIONS;
@@ -168,18 +167,18 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
         AddRandomOutboundPeer(vNodes, *peerLogic, connman.get());
     }
 
-    peerLogic->CheckForStaleTipAndEvictPeers(consensusParams);
+    peerLogic->CheckForStaleTipAndEvictPeers();
 
     // No nodes should be marked for disconnection while we have no extra peers
     for (const CNode *node : vNodes) {
         BOOST_CHECK(node->fDisconnect == false);
     }
 
-    SetMockTime(GetTime() + 3*consensusParams.nPowTargetSpacing + 1);
+    SetMockTime(GetTime() + 3 * chainparams.GetConsensus().nPowTargetSpacing + 1);
 
     // Now tip should definitely be stale, and we should look for an extra
     // outbound peer
-    peerLogic->CheckForStaleTipAndEvictPeers(consensusParams);
+    peerLogic->CheckForStaleTipAndEvictPeers();
     BOOST_CHECK(connman->GetTryNewOutboundPeer());
 
     // Still no peers should be marked for disconnection
@@ -192,8 +191,8 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
     // required time connected check should be satisfied).
     AddRandomOutboundPeer(vNodes, *peerLogic, connman.get());
 
-    peerLogic->CheckForStaleTipAndEvictPeers(consensusParams);
-    for (int i=0; i<max_outbound_full_relay; ++i) {
+    peerLogic->CheckForStaleTipAndEvictPeers();
+    for (int i = 0; i < max_outbound_full_relay; ++i) {
         BOOST_CHECK(vNodes[i]->fDisconnect == false);
     }
     // Last added node should get marked for eviction
@@ -205,8 +204,8 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
     // peer, and check that the next newest node gets evicted.
     UpdateLastBlockAnnounceTime(vNodes.back()->GetId(), GetTime());
 
-    peerLogic->CheckForStaleTipAndEvictPeers(consensusParams);
-    for (int i=0; i<max_outbound_full_relay-1; ++i) {
+    peerLogic->CheckForStaleTipAndEvictPeers();
+    for (int i = 0; i < max_outbound_full_relay - 1; ++i) {
         BOOST_CHECK(vNodes[i]->fDisconnect == false);
     }
     BOOST_CHECK(vNodes[max_outbound_full_relay-1]->fDisconnect == true);