Merge bitcoin/bitcoin#25168: refactor: Avoid passing params where not needed

fa1b76aeb064b315a3767a8f59836ca18aeb117e Do not call global Params() when chainman is in scope (MacroFake)
fa30234be81b6f49ae8150478a9255daa1611083 Do not pass CChainParams& to PeerManager::make (MacroFake)
fafe5c0ca2927642cbcec63ac73994737e1653d6 Do not pass CChainParams& to BlockAssembler constructor (MacroFake)
faf012b438b451dced785e7f031e07c0c55665e1 Do not pass Consensus::Params& to Chainstate helpers (MacroFake)
fa4ee53dca5ccf1b87f019f372ffc10528add943 Do not pass time getter to Chainstate helpers (MacroFake)

Pull request description:

  It seems confusing to pass chain params, consensus params, or a time function around when it is not needed.

  Fix this by:

  * Inlining the passed time getter function. I don't see a use case why this should be mockable.
  * Using `chainman.GetConsensus()` or `chainman.GetParams()`, where possible.

ACKs for top commit:
  promag:
    Code review ACK fa1b76aeb064b315a3767a8f59836ca18aeb117e.
  vincenzopalazzo:
    ACK https://github.com/bitcoin/bitcoin/pull/25168/commits/fa1b76aeb064b315a3767a8f59836ca18aeb117e

Tree-SHA512: 1abff5cba4b4871d97f17dbcdf67bc9255ff21fa4150a79a74e39b28f0610eab3e7dee24d56872dd6e111f003b55e288958cdd467e6218368d896f191e4ec9cd

5 files changed, 15 insertions, 21 deletions

diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 99615dea69..54ba5b7966 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -13,7 +13,6 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
                                                      ChainstateManager& chainman,
                                                      CTxMemPool* mempool,
                                                      bool fPruneMode,
-                                                     const Consensus::Params& consensus_params,
                                                      bool fReindexChainState,
                                                      int64_t nBlockTreeDBCache,
                                                      int64_t nCoinDBCache,
@@ -57,7 +56,7 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
     }
 
     if (!chainman.BlockIndex().empty() &&
-            !chainman.m_blockman.LookupBlockIndex(consensus_params.hashGenesisBlock)) {
+            !chainman.m_blockman.LookupBlockIndex(chainman.GetConsensus().hashGenesisBlock)) {
         return ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK;
     }
 
@@ -126,10 +125,8 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
 std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManager& chainman,
                                                                 bool fReset,
                                                                 bool fReindexChainState,
-                                                                const Consensus::Params& consensus_params,
                                                                 int check_blocks,
-                                                                int check_level,
-                                                                std::function<int64_t()> get_unix_time_seconds)
+                                                                int check_level)
 {
     auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
         return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull();
@@ -140,12 +137,12 @@ std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManage
     for (CChainState* chainstate : chainman.GetAll()) {
         if (!is_coinsview_empty(chainstate)) {
             const CBlockIndex* tip = chainstate->m_chain.Tip();
-            if (tip && tip->nTime > get_unix_time_seconds() + MAX_FUTURE_BLOCK_TIME) {
+            if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) {
                 return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE;
             }
 
             if (!CVerifyDB().VerifyDB(
-                    *chainstate, consensus_params, chainstate->CoinsDB(),
+                    *chainstate, chainman.GetConsensus(), chainstate->CoinsDB(),
                     check_level,
                     check_blocks)) {
                 return ChainstateLoadVerifyError::ERROR_CORRUPTED_BLOCK_DB;
diff --git a/src/node/chainstate.h b/src/node/chainstate.h
index 8ba04f1436..ff7935e8e0 100644
--- a/src/node/chainstate.h
+++ b/src/node/chainstate.h
@@ -59,7 +59,6 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
                                                      ChainstateManager& chainman,
                                                      CTxMemPool* mempool,
                                                      bool fPruneMode,
-                                                     const Consensus::Params& consensus_params,
                                                      bool fReindexChainState,
                                                      int64_t nBlockTreeDBCache,
                                                      int64_t nCoinDBCache,
@@ -78,10 +77,8 @@ enum class ChainstateLoadVerifyError {
 std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManager& chainman,
                                                                 bool fReset,
                                                                 bool fReindexChainState,
-                                                                const Consensus::Params& consensus_params,
                                                                 int check_blocks,
-                                                                int check_level,
-                                                                std::function<int64_t()> get_unix_time_seconds);
+                                                                int check_level);
 } // namespace node
 
 #endif // BITCOIN_NODE_CHAINSTATE_H
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index f2debc790c..decc347954 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -228,7 +228,7 @@ public:
     uint256 getBestBlockHash() override
     {
         const CBlockIndex* tip = WITH_LOCK(::cs_main, return chainman().ActiveChain().Tip());
-        return tip ? tip->GetBlockHash() : Params().GenesisBlock().GetHash();
+        return tip ? tip->GetBlockHash() : chainman().GetParams().GenesisBlock().GetHash();
     }
     int64_t getLastBlockTime() override
     {
@@ -236,7 +236,7 @@ public:
         if (chainman().ActiveChain().Tip()) {
             return chainman().ActiveChain().Tip()->GetBlockTime();
         }
-        return Params().GenesisBlock().GetBlockTime(); // Genesis block's time of current network
+        return chainman().GetParams().GenesisBlock().GetBlockTime(); // Genesis block's time of current network
     }
     double getVerificationProgress() override
     {
@@ -245,7 +245,7 @@ public:
             LOCK(::cs_main);
             tip = chainman().ActiveChain().Tip();
         }
-        return GuessVerificationProgress(Params().TxData(), tip);
+        return GuessVerificationProgress(chainman().GetParams().TxData(), tip);
     }
     bool isInitialBlockDownload() override {
         return chainman().ActiveChainstate().IsInitialBlockDownload();
@@ -546,7 +546,7 @@ public:
     double guessVerificationProgress(const uint256& block_hash) override
     {
         LOCK(cs_main);
-        return GuessVerificationProgress(Params().TxData(), chainman().m_blockman.LookupBlockIndex(block_hash));
+        return GuessVerificationProgress(chainman().GetParams().TxData(), chainman().m_blockman.LookupBlockIndex(block_hash));
     }
     bool hasBlocks(const uint256& block_hash, int min_height, std::optional<int> max_height) override
     {
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 770ccdbe1a..48e50f3714 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -62,8 +62,8 @@ BlockAssembler::Options::Options()
     nBlockMaxWeight = DEFAULT_BLOCK_MAX_WEIGHT;
 }
 
-BlockAssembler::BlockAssembler(CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params, const Options& options)
-    : chainparams(params),
+BlockAssembler::BlockAssembler(CChainState& chainstate, const CTxMemPool& mempool, const Options& options)
+    : chainparams{chainstate.m_chainman.GetParams()},
       m_mempool(mempool),
       m_chainstate(chainstate)
 {
@@ -87,8 +87,8 @@ static BlockAssembler::Options DefaultOptions()
     return options;
 }
 
-BlockAssembler::BlockAssembler(CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params)
-    : BlockAssembler(chainstate, mempool, params, DefaultOptions()) {}
+BlockAssembler::BlockAssembler(CChainState& chainstate, const CTxMemPool& mempool)
+    : BlockAssembler(chainstate, mempool, DefaultOptions()) {}
 
 void BlockAssembler::resetBlock()
 {
diff --git a/src/node/miner.h b/src/node/miner.h
index 678df815c0..7cf8e3fb9e 100644
--- a/src/node/miner.h
+++ b/src/node/miner.h
@@ -157,8 +157,8 @@ public:
         CFeeRate blockMinFeeRate;
     };
 
-    explicit BlockAssembler(CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params);
-    explicit BlockAssembler(CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params, const Options& options);
+    explicit BlockAssembler(CChainState& chainstate, const CTxMemPool& mempool);
+    explicit BlockAssembler(CChainState& chainstate, const CTxMemPool& mempool, const Options& options);
 
     /** Construct a new block template with coinbase to scriptPubKeyIn */
     std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn);