From dbe45c34f8b4fd7d615f7e05ef1454798ef0c8ca Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 1 Mar 2022 16:14:12 -0500 Subject: [PATCH 1/3] Add ChainstateManagerOpts, using as ::Options [META] Although it seems like we don't need it for just one option, we're going to introduce another member to this struct *in the next commit*. In future patchsets for libbitcoinkernel decoupling it from ArgsManager, even more members will be added here. --- src/Makefile.am | 1 + src/bitcoin-chainstate.cpp | 5 ++++- src/init.cpp | 5 ++++- src/kernel/chainstatemanager_opts.h | 22 ++++++++++++++++++++++ src/test/util/setup_common.cpp | 5 ++++- src/test/validation_chainstate_tests.cpp | 7 +++++-- src/validation.h | 6 +++++- 7 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 src/kernel/chainstatemanager_opts.h diff --git a/src/Makefile.am b/src/Makefile.am index 357e562c69..d421e8f89d 100644 --- a/src/Makefile.am +++ b/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 \ diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 812585ba1e..b667641964 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -70,7 +70,10 @@ int main(int argc, char* argv[]) // SETUP: Chainstate - ChainstateManager chainman{chainparams}; + const ChainstateManager::Options chainman_opts{ + chainparams, + }; + ChainstateManager chainman{chainman_opts}; auto rv = node::LoadChainstate(false, std::ref(chainman), diff --git a/src/init.cpp b/src/init.cpp index 060541e975..ec94060e99 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1421,7 +1421,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) { node.mempool = std::make_unique(node.fee_estimator.get(), mempool_check_ratio); - node.chainman = std::make_unique(chainparams); + const ChainstateManager::Options chainman_opts{ + chainparams, + }; + node.chainman = std::make_unique(chainman_opts); ChainstateManager& chainman = *node.chainman; const bool fReset = fReindex; diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h new file mode 100644 index 0000000000..f2646700a4 --- /dev/null +++ b/src/kernel/chainstatemanager_opts.h @@ -0,0 +1,22 @@ +// 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 +#include + +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; +}; + +#endif // BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index ff78f87ec1..ba37a6113f 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -164,7 +164,10 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_cache_sizes = CalculateCacheSizes(m_args); - m_node.chainman = std::make_unique(chainparams); + const ChainstateManager::Options chainman_opts{ + chainparams, + }; + m_node.chainman = std::make_unique(chainman_opts); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(m_cache_sizes.block_tree_db, true); // Start script-checking threads. Set g_parallel_script_checks to true so they are used. diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index e7c7584f1c..55c2228767 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -22,8 +22,11 @@ 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(), + }; + ChainstateManager manager{chainman_opts}; + WITH_LOCK(::cs_main, manager.m_blockman.m_block_tree_db = std::make_unique(1 << 20, true)); CTxMemPool mempool; diff --git a/src/validation.h b/src/validation.h index 04745a6e36..cae4ed4f14 100644 --- a/src/validation.h +++ b/src/validation.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -853,7 +854,10 @@ private: friend CChainState; public: - explicit ChainstateManager(const CChainParams& chainparams) : m_chainparams{chainparams} { } + using Options = ChainstateManagerOpts; + + explicit ChainstateManager(const Options& opts) + : m_chainparams(opts.chainparams) {}; const CChainParams& GetParams() const { return m_chainparams; } const Consensus::Params& GetConsensus() const { return m_chainparams.GetConsensus(); } From 04c31c1295eb4ecd42afd54b8e353cbda93d83f0 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 1 Mar 2022 16:14:12 -0500 Subject: [PATCH 2/3] 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 d421e8f89d..a0e793e42a 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 b667641964..99aa23fb06 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(GetTime), }; ChainstateManager chainman{chainman_opts}; diff --git a/src/init.cpp b/src/init.cpp index ec94060e99..b1fe915189 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(chainman_opts); ChainstateManager& chainman = *node.chainman; diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index f2646700a4..575d94e2e9 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 adjusted_time_callback{nullptr}; }; #endif // BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 48e50f3714..2464579dd1 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -167,7 +167,7 @@ std::unique_ptr 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 b2c25b60ee..8fb6daf0cb 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -27,6 +27,7 @@ #include