From 04c31c1295eb4ecd42afd54b8e353cbda93d83f0 Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Tue, 1 Mar 2022 16:14:12 -0500
Subject: [PATCH] Add ChainstateManager::m_adjusted_time_callback

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.
---
 src/Makefile.am                          | 3 ---
 src/bitcoin-chainstate.cpp               | 1 +
 src/init.cpp                             | 1 +
 src/kernel/chainstatemanager_opts.h      | 1 +
 src/node/miner.cpp                       | 2 +-
 src/rpc/mining.cpp                       | 5 +++--
 src/test/miner_tests.cpp                 | 1 +
 src/test/util/setup_common.cpp           | 2 ++
 src/test/validation_chainstate_tests.cpp | 8 +++++---
 src/validation.cpp                       | 8 ++++----
 src/validation.h                         | 6 +++++-
 11 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index d421e8f89..a0e793e42 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -877,7 +877,6 @@ libbitcoinkernel_la_SOURCES = \
   init/common.cpp \
   key.cpp \
   logging.cpp \
-  netaddress.cpp \
   node/blockstorage.cpp \
   node/chainstate.cpp \
   node/coinstats.cpp \
@@ -906,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 \
diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index b66764196..99aa23fb0 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -72,6 +72,7 @@ int main(int argc, char* argv[])
     // SETUP: Chainstate
     const ChainstateManager::Options chainman_opts{
         chainparams,
+        static_cast<int64_t(*)()>(GetTime),
     };
     ChainstateManager chainman{chainman_opts};
 
diff --git a/src/init.cpp b/src/init.cpp
index ec94060e9..b1fe91518 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1423,6 +1423,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
         const ChainstateManager::Options chainman_opts{
             chainparams,
+            GetAdjustedTime,
         };
         node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
         ChainstateManager& chainman = *node.chainman;
diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h
index f2646700a..575d94e2e 100644
--- a/src/kernel/chainstatemanager_opts.h
+++ b/src/kernel/chainstatemanager_opts.h
@@ -17,6 +17,7 @@ class CChainParams;
  */
 struct ChainstateManagerOpts {
     const CChainParams& chainparams;
+    const std::function<int64_t()> adjusted_time_callback{nullptr};
 };
 
 #endif // BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 48e50f371..2464579dd 100644
--- a/src/node/miner.cpp
+++ b/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();
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index b2c25b60e..8fb6daf0c 100644
--- a/src/rpc/mining.cpp
+++ b/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);
         }
 
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index 01f9d6862..439ad174b 100644
--- a/src/test/miner_tests.cpp
+++ b/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>
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index ba37a6113..b7566bd1f 100644
--- a/src/test/util/setup_common.cpp
+++ b/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>
@@ -166,6 +167,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
 
     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);
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
index 55c222876..98cb713a8 100644
--- a/src/test/validation_chainstate_tests.cpp
+++ b/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>
@@ -24,6 +25,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
 {
     const ChainstateManager::Options chainman_opts{
         Params(),
+        GetAdjustedTime,
     };
     ChainstateManager manager{chainman_opts};
 
diff --git a/src/validation.cpp b/src/validation.cpp
index a7cdf63a2..a54ec8269 100644
--- a/src/validation.cpp
+++ b/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());
diff --git a/src/validation.h b/src/validation.h
index cae4ed4f1..6f4404e7e 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -362,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);
 
@@ -837,6 +838,8 @@ private:
 
     const CChainParams& m_chainparams;
 
+    const std::function<int64_t()> m_adjusted_time_callback;
+
     //! Internal helper for ActivateSnapshot().
     [[nodiscard]] bool PopulateAndValidateSnapshot(
         CChainState& snapshot_chainstate,
@@ -857,7 +860,8 @@ public:
     using Options = ChainstateManagerOpts;
 
     explicit ChainstateManager(const Options& opts)
-        : m_chainparams(opts.chainparams) {};
+        : 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(); }