Merge bitcoin/bitcoin#25064: [kernel 2b/n] Add ChainstateManager::m_adjusted_time_callback

53494bc739 validation: Have ChainstateManager own m_chainparams (Carl Dong)
04c31c1295 Add ChainstateManager::m_adjusted_time_callback (Carl Dong)
dbe45c34f8 Add ChainstateManagerOpts, using as ::Options (Carl Dong)

Pull request description:

  ```
  This decouples validation.cpp from netaddress.cpp (transitively,
  timedata.cpp, and asmap.cpp).

  This is important for libbitcoinkernel as:

  - There is no reason for the consensus engine to be coupled with
    netaddress, timedata, and asmap
  - Users of libbitcoinkernel can now easily supply their own
    std::function that provides the adjusted time.

  See the src/Makefile.am changes for some satisfying removals.
  ```

Top commit has no ACKs.

Tree-SHA512: a093ec6ecacdc659d656574f05bd31ade6a6cdb64d5a97684f94ae7e55c0e360b78177553d4d1ef40280192674464d029a0d68e96caf8711d9274011172f1330

src/Makefile.am

@@ -171,6 +171,7 @@ BITCOIN_CORE_H = \
   interfaces/ipc.h \
   interfaces/node.h \
   interfaces/wallet.h \
+  kernel/chainstatemanager_opts.h \
   key.h \
   key_io.h \
   logging.h \
@@ -876,7 +877,6 @@ libbitcoinkernel_la_SOURCES = \
   init/common.cpp \
   key.cpp \
   logging.cpp \
-  netaddress.cpp \
   node/blockstorage.cpp \
   node/chainstate.cpp \
   node/coinstats.cpp \
@@ -905,11 +905,9 @@ libbitcoinkernel_la_SOURCES = \
   support/lockedpool.cpp \
   sync.cpp \
   threadinterrupt.cpp \
-  timedata.cpp \
   txdb.cpp \
   txmempool.cpp \
   uint256.cpp \
-  util/asmap.cpp \
   util/bytevectorhash.cpp \
   util/check.cpp \
   util/getuniquepath.cpp \

src/bitcoin-chainstate.cpp

@@ -70,7 +70,11 @@ int main(int argc, char* argv[])
 
 
     // SETUP: Chainstate
-    ChainstateManager chainman{chainparams};
+    const ChainstateManager::Options chainman_opts{
+        chainparams,
+        static_cast<int64_t(*)()>(GetTime),
+    };
+    ChainstateManager chainman{chainman_opts};
 
     auto rv = node::LoadChainstate(false,
                                    std::ref(chainman),

src/init.cpp

@@ -1421,7 +1421,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) {
         node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get(), mempool_check_ratio);
 
-        node.chainman = std::make_unique<ChainstateManager>(chainparams);
+        const ChainstateManager::Options chainman_opts{
+            chainparams,
+            GetAdjustedTime,
+        };
+        node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
         ChainstateManager& chainman = *node.chainman;
 
         const bool fReset = fReindex;

src/kernel/chainstatemanager_opts.h

@@ -0,0 +1,23 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
+#define BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
+
+#include <cstdint>
+#include <functional>
+
+class CChainParams;
+
+/**
+ * An options struct for `ChainstateManager`, more ergonomically referred to as
+ * `ChainstateManager::Options` due to the using-declaration in
+ * `ChainstateManager`.
+ */
+struct ChainstateManagerOpts {
+    const CChainParams& chainparams;
+    const std::function<int64_t()> adjusted_time_callback{nullptr};
+};
+
+#endif // BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H

src/node/miner.cpp

@@ -167,7 +167,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     pblocktemplate->vTxSigOpsCost[0] = WITNESS_SCALE_FACTOR * GetLegacySigOpCount(*pblock->vtx[0]);
 
     BlockValidationState state;
-    if (!TestBlockValidity(state, chainparams, m_chainstate, *pblock, pindexPrev, false, false)) {
+    if (!TestBlockValidity(state, chainparams, m_chainstate, *pblock, pindexPrev, GetAdjustedTime, false, false)) {
         throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, state.ToString()));
     }
     int64_t nTime2 = GetTimeMicros();

src/rpc/mining.cpp

@@ -27,6 +27,7 @@
 #include <script/script.h>
 #include <script/signingprovider.h>
 #include <shutdown.h>
+#include <timedata.h>
 #include <txmempool.h>
 #include <univalue.h>
 #include <util/strencodings.h>
@@ -371,7 +372,7 @@ static RPCHelpMan generateblock()
         LOCK(cs_main);
 
         BlockValidationState state;
-        if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) {
+        if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), GetAdjustedTime, false, false)) {
             throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString()));
         }
     }
@@ -640,7 +641,7 @@ static RPCHelpMan getblocktemplate()
             if (block.hashPrevBlock != pindexPrev->GetBlockHash())
                 return "inconclusive-not-best-prevblk";
             BlockValidationState state;
-            TestBlockValidity(state, chainman.GetParams(), active_chainstate, block, pindexPrev, false, true);
+            TestBlockValidity(state, chainman.GetParams(), active_chainstate, block, pindexPrev, GetAdjustedTime, false, true);
             return BIP22ValidationResult(state);
         }
 

src/test/miner_tests.cpp

@@ -10,6 +10,7 @@
 #include <node/miner.h>
 #include <policy/policy.h>
 #include <script/standard.h>
+#include <timedata.h>
 #include <txmempool.h>
 #include <uint256.h>
 #include <util/strencodings.h>

src/test/util/setup_common.cpp

@@ -29,6 +29,7 @@
 #include <shutdown.h>
 #include <streams.h>
 #include <test/util/net.h>
+#include <timedata.h>
 #include <txdb.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -164,7 +165,11 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
 
     m_cache_sizes = CalculateCacheSizes(m_args);
 
-    m_node.chainman = std::make_unique<ChainstateManager>(chainparams);
+    const ChainstateManager::Options chainman_opts{
+        chainparams,
+        GetAdjustedTime,
+    };
+    m_node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
     m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(m_cache_sizes.block_tree_db, true);
 
     // Start script-checking threads. Set g_parallel_script_checks to true so they are used.

src/test/validation_chainstate_tests.cpp

@@ -3,13 +3,14 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 //
 #include <chainparams.h>
-#include <random.h>
-#include <uint256.h>
 #include <consensus/validation.h>
-#include <sync.h>
+#include <random.h>
 #include <rpc/blockchain.h>
+#include <sync.h>
 #include <test/util/chainstate.h>
 #include <test/util/setup_common.h>
+#include <timedata.h>
+#include <uint256.h>
 #include <validation.h>
 
 #include <vector>
@@ -22,8 +23,12 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, TestingSetup)
 //!
 BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
 {
-    const CChainParams& chainparams = Params();
-    ChainstateManager manager(chainparams);
+    const ChainstateManager::Options chainman_opts{
+        Params(),
+        GetAdjustedTime,
+    };
+    ChainstateManager manager{chainman_opts};
+
     WITH_LOCK(::cs_main, manager.m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(1 << 20, true));
     CTxMemPool mempool;
 

src/validation.cpp

@@ -36,7 +36,6 @@
 #include <script/sigcache.h>
 #include <shutdown.h>
 #include <signet.h>
-#include <timedata.h>
 #include <tinyformat.h>
 #include <txdb.h>
 #include <txmempool.h>
@@ -2006,7 +2005,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
     // Also, currently the rule against blocks more than 2 hours in the future
     // is enforced in ContextualCheckBlockHeader(); we wouldn't want to
     // re-enforce that rule here (at least until we make it impossible for
-    // GetAdjustedTime() to go backward).
+    // m_adjusted_time_callback() to go backward).
     if (!CheckBlock(block, state, m_params.GetConsensus(), !fJustCheck, !fJustCheck)) {
         if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) {
             // We don't write down blocks to disk if they may have been
@@ -3613,7 +3612,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida
             LogPrint(BCLog::VALIDATION, "%s: %s prev block invalid\n", __func__, hash.ToString());
             return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk");
         }
-        if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev, GetAdjustedTime())) {
+        if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev, m_adjusted_time_callback())) {
             LogPrint(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString());
             return false;
         }
@@ -3837,6 +3836,7 @@ bool TestBlockValidity(BlockValidationState& state,
                        CChainState& chainstate,
                        const CBlock& block,
                        CBlockIndex* pindexPrev,
+                       const std::function<int64_t()>& adjusted_time_callback,
                        bool fCheckPOW,
                        bool fCheckMerkleRoot)
 {
@@ -3850,7 +3850,7 @@ bool TestBlockValidity(BlockValidationState& state,
     indexDummy.phashBlock = &block_hash;
 
     // NOTE: CheckBlockHeader is called by CheckBlock
-    if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev, GetAdjustedTime()))
+    if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev, adjusted_time_callback()))
         return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, state.ToString());
     if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot))
         return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString());

src/validation.h

@@ -14,6 +14,7 @@
 #include <attributes.h>
 #include <chain.h>
 #include <chainparams.h>
+#include <kernel/chainstatemanager_opts.h>
 #include <consensus/amount.h>
 #include <deploymentstatus.h>
 #include <fs.h>
@@ -361,6 +362,7 @@ bool TestBlockValidity(BlockValidationState& state,
                        CChainState& chainstate,
                        const CBlock& block,
                        CBlockIndex* pindexPrev,
+                       const std::function<int64_t()>& adjusted_time_callback,
                        bool fCheckPOW = true,
                        bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
@@ -834,7 +836,9 @@ private:
 
     CBlockIndex* m_best_invalid GUARDED_BY(::cs_main){nullptr};
 
-    const CChainParams& m_chainparams;
+    const CChainParams m_chainparams;
+
+    const std::function<int64_t()> m_adjusted_time_callback;
 
     //! Internal helper for ActivateSnapshot().
     [[nodiscard]] bool PopulateAndValidateSnapshot(
@@ -853,7 +857,11 @@ private:
     friend CChainState;
 
 public:
-    explicit ChainstateManager(const CChainParams& chainparams) : m_chainparams{chainparams} { }
+    using Options = ChainstateManagerOpts;
+
+    explicit ChainstateManager(const Options& opts)
+        : m_chainparams{opts.chainparams},
+          m_adjusted_time_callback{Assert(opts.adjusted_time_callback)} {};
 
     const CChainParams& GetParams() const { return m_chainparams; }
     const Consensus::Params& GetConsensus() const { return m_chainparams.GetConsensus(); }