Move ::hashAssumeValid into ChainstateManager

This changes the assumed valid block for the bitcoin-chainstate
executable. Previously it was uint256{}, now it is defaultAssumeValid.

Author: MacroFake

src/init.cpp

@@ -937,8 +937,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
     fCheckBlockIndex = args.GetBoolArg("-checkblockindex", chainparams.DefaultConsistencyChecks());
     fCheckpointsEnabled = args.GetBoolArg("-checkpoints", DEFAULT_CHECKPOINTS_ENABLED);
 
-    hashAssumeValid = uint256S(args.GetArg("-assumevalid", chainparams.GetConsensus().defaultAssumeValid.GetHex()));
-
     if (args.IsArgSet("-minimumchainwork")) {
         const std::string minChainWorkStr = args.GetArg("-minimumchainwork", "");
         if (!IsHexNumber(minChainWorkStr)) {

src/kernel/chainstatemanager_opts.h

@@ -5,10 +5,12 @@
 #ifndef BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
 #define BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
 
+#include <uint256.h>
 #include <util/time.h>
 
 #include <cstdint>
 #include <functional>
+#include <optional>
 
 class CChainParams;
 
@@ -24,6 +26,8 @@ namespace kernel {
 struct ChainstateManagerOpts {
     const CChainParams& chainparams;
     const std::function<NodeClock::time_point()> adjusted_time_callback{nullptr};
+    //! If set, it will override the block hash whose ancestors we will assume to have valid scripts without checking them.
+    std::optional<uint256> assumed_valid_block;
     //! If the tip is older than this, the node is considered to be in initial block download.
     std::chrono::seconds max_tip_age{DEFAULT_MAX_TIP_AGE};
 };

src/node/chainstate.cpp

@@ -32,8 +32,8 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
         return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
     };
 
-    if (!hashAssumeValid.IsNull()) {
-        LogPrintf("Assuming ancestors of block %s have valid signatures.\n", hashAssumeValid.GetHex());
+    if (!chainman.AssumedValidBlock().IsNull()) {
+        LogPrintf("Assuming ancestors of block %s have valid signatures.\n", chainman.AssumedValidBlock().GetHex());
     } else {
         LogPrintf("Validating signatures for all blocks.\n");
     }

src/node/chainstatemanager_args.cpp

@@ -12,6 +12,8 @@
 namespace node {
 void ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts)
 {
+    if (auto value{args.GetArg("-assumevalid")}) opts.assumed_valid_block = uint256S(*value);
+
     if (auto value{args.GetIntArg("-maxtipage")}) opts.max_tip_age = std::chrono::seconds{*value};
 }
 } // namespace node

src/validation.cpp

@@ -124,7 +124,6 @@ bool g_parallel_script_checks{false};
 bool fCheckBlockIndex = false;
 bool fCheckpointsEnabled = DEFAULT_CHECKPOINTS_ENABLED;
 
-uint256 hashAssumeValid;
 arith_uint256 nMinimumChainWork;
 
 const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locator) const
@@ -2036,13 +2035,13 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     }
 
     bool fScriptChecks = true;
-    if (!hashAssumeValid.IsNull()) {
+    if (!m_chainman.AssumedValidBlock().IsNull()) {
         // We've been configured with the hash of a block which has been externally verified to have a valid history.
         // A suitable default value is included with the software and updated from time to time.  Because validity
         //  relative to a piece of software is an objective fact these defaults can be easily reviewed.
         // This setting doesn't force the selection of any particular chain but makes validating some faster by
         //  effectively caching the result of part of the verification.
-        BlockMap::const_iterator  it = m_blockman.m_block_index.find(hashAssumeValid);
+        BlockMap::const_iterator it{m_blockman.m_block_index.find(m_chainman.AssumedValidBlock())};
         if (it != m_blockman.m_block_index.end()) {
             if (it->second.GetAncestor(pindex->nHeight) == pindex &&
                 m_chainman.m_best_header->GetAncestor(pindex->nHeight) == pindex &&
@@ -5244,6 +5243,20 @@ void ChainstateManager::ResetChainstates()
     m_active_chainstate = nullptr;
 }
 
+/**
+ * Apply default chain params to nullopt members.
+ * This helps to avoid coding errors around the accidental use of the compare
+ * operators that accept nullopt, thus ignoring the intended default value.
+ */
+static ChainstateManager::Options&& Flatten(ChainstateManager::Options&& opts)
+{
+    if (!opts.assumed_valid_block.has_value()) opts.assumed_valid_block = opts.chainparams.GetConsensus().defaultAssumeValid;
+    Assert(opts.adjusted_time_callback);
+    return std::move(opts);
+}
+
+ChainstateManager::ChainstateManager(Options options) : m_options{Flatten(std::move(options))} {}
+
 ChainstateManager::~ChainstateManager()
 {
     LOCK(::cs_main);

src/validation.h

@@ -99,9 +99,6 @@ extern bool g_parallel_script_checks;
 extern bool fCheckBlockIndex;
 extern bool fCheckpointsEnabled;
 
-/** Block hash whose ancestors we will assume to have valid scripts without checking them. */
-extern uint256 hashAssumeValid;
-
 /** Minimum work we will assume exists on some valid chain. */
 extern arith_uint256 nMinimumChainWork;
 
@@ -864,13 +861,11 @@ class ChainstateManager
 public:
     using Options = kernel::ChainstateManagerOpts;
 
-    explicit ChainstateManager(Options options) : m_options{std::move(options)}
-    {
-        Assert(m_options.adjusted_time_callback);
-    }
+    explicit ChainstateManager(Options options);
 
     const CChainParams& GetParams() const { return m_options.chainparams; }
     const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); }
+    const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); }
 
     /**
      * Alias for ::cs_main.