From 33b4d48cfcdf145f49cb2283ac3e2936a4e23fff Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 13 Jan 2022 07:57:54 -0500 Subject: indexes, refactor: Pass Chain interface instead of CChainState class to indexes Passing abstract Chain interface will let indexes run in separate processes. This commit does not change behavior in any way. --- src/index/base.cpp | 11 +++++++++-- src/index/base.h | 8 +++++++- src/index/blockfilterindex.cpp | 8 ++++---- src/index/blockfilterindex.h | 4 ++-- src/index/coinstatsindex.cpp | 3 ++- src/index/coinstatsindex.h | 2 +- src/index/txindex.cpp | 4 ++-- src/index/txindex.h | 2 +- src/init.cpp | 12 ++++++------ src/interfaces/chain.h | 4 ++++ src/node/interfaces.cpp | 1 + src/test/blockfilter_index_tests.cpp | 11 ++++++----- src/test/coinstatsindex_tests.cpp | 13 +++++++------ src/test/txindex_tests.cpp | 5 +++-- test/lint/lint-circular-dependencies.py | 3 +++ 15 files changed, 58 insertions(+), 33 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 323547900d..26b3653f7b 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -4,7 +4,9 @@ #include #include +#include #include +#include #include #include #include @@ -49,6 +51,9 @@ void BaseIndex::DB::WriteBestBlock(CDBBatch& batch, const CBlockLocator& locator batch.Write(DB_BEST_BLOCK, locator); } +BaseIndex::BaseIndex(std::unique_ptr chain) + : m_chain{std::move(chain)} {} + BaseIndex::~BaseIndex() { Interrupt(); @@ -346,9 +351,11 @@ void BaseIndex::Interrupt() m_interrupt(); } -bool BaseIndex::Start(CChainState& active_chainstate) +bool BaseIndex::Start() { - m_chainstate = &active_chainstate; + // m_chainstate member gives indexing code access to node internals. It is + // removed in followup https://github.com/bitcoin/bitcoin/pull/24230 + m_chainstate = &m_chain->context()->chainman->ActiveChainstate(); // Need to register this ValidationInterface before running Init(), so that // callbacks are not missed if Init sets m_synced to true. RegisterValidationInterface(this); diff --git a/src/index/base.h b/src/index/base.h index a8f6a18c8d..3586376459 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -6,12 +6,16 @@ #define BITCOIN_INDEX_BASE_H #include +#include #include #include class CBlock; class CBlockIndex; class CChainState; +namespace interfaces { +class Chain; +} // namespace interfaces struct IndexSummary { std::string name; @@ -79,6 +83,7 @@ private: virtual bool AllowPrune() const = 0; protected: + std::unique_ptr m_chain; CChainState* m_chainstate{nullptr}; void BlockConnected(const std::shared_ptr& block, const CBlockIndex* pindex) override; @@ -110,6 +115,7 @@ protected: void SetBestBlockIndex(const CBlockIndex* block); public: + BaseIndex(std::unique_ptr chain); /// Destructor interrupts sync thread if running and blocks until it exits. virtual ~BaseIndex(); @@ -124,7 +130,7 @@ public: /// Start initializes the sync state and registers the instance as a /// ValidationInterface so that it stays in sync with blockchain updates. - [[nodiscard]] bool Start(CChainState& active_chainstate); + [[nodiscard]] bool Start(); /// Stops the instance from staying in sync with blockchain updates. void Stop(); diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index e7fad8eb64..15a7cfeaea 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -94,9 +94,9 @@ struct DBHashKey { static std::map g_filter_indexes; -BlockFilterIndex::BlockFilterIndex(BlockFilterType filter_type, +BlockFilterIndex::BlockFilterIndex(std::unique_ptr chain, BlockFilterType filter_type, size_t n_cache_size, bool f_memory, bool f_wipe) - : m_filter_type(filter_type) + : BaseIndex(std::move(chain)), m_filter_type(filter_type) { const std::string& filter_name = BlockFilterTypeName(filter_type); if (filter_name.empty()) throw std::invalid_argument("unknown filter_type"); @@ -467,12 +467,12 @@ void ForEachBlockFilterIndex(std::function fn) for (auto& entry : g_filter_indexes) fn(entry.second); } -bool InitBlockFilterIndex(BlockFilterType filter_type, +bool InitBlockFilterIndex(std::function()> make_chain, BlockFilterType filter_type, size_t n_cache_size, bool f_memory, bool f_wipe) { auto result = g_filter_indexes.emplace(std::piecewise_construct, std::forward_as_tuple(filter_type), - std::forward_as_tuple(filter_type, + std::forward_as_tuple(make_chain(), filter_type, n_cache_size, f_memory, f_wipe)); return result.second; } diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index fef8b573e8..71e150ba75 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -55,7 +55,7 @@ protected: public: /** Constructs the index, which becomes available to be queried. */ - explicit BlockFilterIndex(BlockFilterType filter_type, + explicit BlockFilterIndex(std::unique_ptr chain, BlockFilterType filter_type, size_t n_cache_size, bool f_memory = false, bool f_wipe = false); BlockFilterType GetFilterType() const { return m_filter_type; } @@ -88,7 +88,7 @@ void ForEachBlockFilterIndex(std::function fn); * Initialize a block filter index for the given type if one does not already exist. Returns true if * a new index is created and false if one has already been initialized. */ -bool InitBlockFilterIndex(BlockFilterType filter_type, +bool InitBlockFilterIndex(std::function()> make_chain, BlockFilterType filter_type, size_t n_cache_size, bool f_memory = false, bool f_wipe = false); /** diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 687e330fe0..7373ed249d 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -102,7 +102,8 @@ struct DBHashKey { std::unique_ptr g_coin_stats_index; -CoinStatsIndex::CoinStatsIndex(size_t n_cache_size, bool f_memory, bool f_wipe) +CoinStatsIndex::CoinStatsIndex(std::unique_ptr chain, size_t n_cache_size, bool f_memory, bool f_wipe) + : BaseIndex(std::move(chain)) { fs::path path{gArgs.GetDataDirNet() / "indexes" / "coinstats"}; fs::create_directories(path); diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h index cae052d913..06846d07ab 100644 --- a/src/index/coinstatsindex.h +++ b/src/index/coinstatsindex.h @@ -53,7 +53,7 @@ protected: public: // Constructs the index, which becomes available to be queried. - explicit CoinStatsIndex(size_t n_cache_size, bool f_memory = false, bool f_wipe = false); + explicit CoinStatsIndex(std::unique_ptr chain, size_t n_cache_size, bool f_memory = false, bool f_wipe = false); // Look up stats for a specific block using CBlockIndex std::optional LookUpStats(const CBlockIndex* block_index) const; diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index 97c11c4383..4039588586 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -48,8 +48,8 @@ bool TxIndex::DB::WriteTxs(const std::vector>& v_ return WriteBatch(batch); } -TxIndex::TxIndex(size_t n_cache_size, bool f_memory, bool f_wipe) - : m_db(std::make_unique(n_cache_size, f_memory, f_wipe)) +TxIndex::TxIndex(std::unique_ptr chain, size_t n_cache_size, bool f_memory, bool f_wipe) + : BaseIndex(std::move(chain)), m_db(std::make_unique(n_cache_size, f_memory, f_wipe)) {} TxIndex::~TxIndex() = default; diff --git a/src/index/txindex.h b/src/index/txindex.h index ec339abaa1..f74a7511dc 100644 --- a/src/index/txindex.h +++ b/src/index/txindex.h @@ -31,7 +31,7 @@ protected: public: /// Constructs the index, which becomes available to be queried. - explicit TxIndex(size_t n_cache_size, bool f_memory = false, bool f_wipe = false); + explicit TxIndex(std::unique_ptr chain, size_t n_cache_size, bool f_memory = false, bool f_wipe = false); // Destructor is declared because this class contains a unique_ptr to an incomplete type. virtual ~TxIndex() override; diff --git a/src/init.cpp b/src/init.cpp index 97c823fe0c..046857d60c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1594,22 +1594,22 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return InitError(*error); } - g_txindex = std::make_unique(cache_sizes.tx_index, false, fReindex); - if (!g_txindex->Start(chainman.ActiveChainstate())) { + g_txindex = std::make_unique(interfaces::MakeChain(node), cache_sizes.tx_index, false, fReindex); + if (!g_txindex->Start()) { return false; } } for (const auto& filter_type : g_enabled_filter_types) { - InitBlockFilterIndex(filter_type, cache_sizes.filter_index, false, fReindex); - if (!GetBlockFilterIndex(filter_type)->Start(chainman.ActiveChainstate())) { + InitBlockFilterIndex([&]{ return interfaces::MakeChain(node); }, filter_type, cache_sizes.filter_index, false, fReindex); + if (!GetBlockFilterIndex(filter_type)->Start()) { return false; } } if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) { - g_coin_stats_index = std::make_unique(/* cache size */ 0, false, fReindex); - if (!g_coin_stats_index->Start(chainman.ActiveChainstate())) { + g_coin_stats_index = std::make_unique(interfaces::MakeChain(node), /* cache size */ 0, false, fReindex); + if (!g_coin_stats_index->Start()) { return false; } } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 6396d0d359..4b192c29cc 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -304,6 +304,10 @@ public: //! Return true if an assumed-valid chain is in use. virtual bool hasAssumedValidChain() = 0; + + //! Get internal node context. Useful for testing, but not + //! accessible across processes. + virtual node::NodeContext* context() { return nullptr; } }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 2562d81e48..414eff7125 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -781,6 +781,7 @@ public: return Assert(m_node.chainman)->IsSnapshotActive(); } + NodeContext* context() override { return &m_node; } NodeContext& m_node; }; } // namespace diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index c31e4e51f7..1a182209b8 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include