diff options
author | fanquake <fanquake@gmail.com> | 2022-07-19 20:56:49 +0100 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-07-19 21:42:48 +0100 |
commit | 92c8e1849dceea6d4f7de2b58f4c50e5e50762ea (patch) | |
tree | 6153cc4d3ca4ef2aebe3f824fc6cfb5ef0dd7ab2 | |
parent | 6900162aeaf8cde98815df790971b4521000f82a (diff) | |
parent | 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd (diff) | |
download | bitcoin-92c8e1849dceea6d4f7de2b58f4c50e5e50762ea.tar.xz |
Merge bitcoin/bitcoin#25494: indexes: Stop using node internal types
7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd indexes, refactor: Remove CChainState use in index CommitInternal method (Ryan Ofsky)
ee3a079fab2c33b4186b62ab822753954a4e545f indexes, refactor: Remove CBlockIndex* uses in index Rewind methods (Ryan Ofsky)
dc971be0831959e7ee6a6df9e1aa46091351a8fb indexes, refactor: Remove CBlockIndex* uses in index WriteBlock methods (Ryan Ofsky)
bef4e405f3de2718dfee279a9abff4daf016da26 indexes, refactor: Remove CBlockIndex* uses in index Init methods (Ryan Ofsky)
addb4f2af183a25ce4a6b6485b5b49575a2ba31b indexes, refactor: Remove CBlockIndex* uses in coinstatsindex LookUpOne function (Ryan Ofsky)
33b4d48cfcdf145f49cb2283ac3e2936a4e23fff indexes, refactor: Pass Chain interface instead of CChainState class to indexes (Ryan Ofsky)
a0b5b4ae5a24536d333cbce2ea584f2d935c651f interfaces, refactor: Add more block information to block connected notifications (Ryan Ofsky)
Pull request description:
Start transitioning index code away from using internal node types like `CBlockIndex` and `CChain` so index code is less coupled to node code and index code will later be able to stop locking cs_main and sync without having to deal with validationinterface race conditions, and so new indexes are easier to write and can run as plugins or separate processes.
This PR contains the first 7 commits from https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-1165625977 which have been split off for easier review. Previous review comments can be found in #24230
ACKs for top commit:
MarcoFalke:
ACK 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd though did not review the last commit 🤼
mzumsande:
Code Review ACK 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd
Tree-SHA512: f84ac2eb6dca2c305566ddeb35ea14d0b71c00860c0fd752bbcf1a0188be833d8c2a6ac9d3ef6ab5b46fbd02d7a24cbb8f60cf12464cb8ba208e22287f709989
-rw-r--r-- | src/Makefile.am | 3 | ||||
-rw-r--r-- | src/index/base.cpp | 63 | ||||
-rw-r--r-- | src/index/base.h | 24 | ||||
-rw-r--r-- | src/index/blockfilterindex.cpp | 42 | ||||
-rw-r--r-- | src/index/blockfilterindex.h | 12 | ||||
-rw-r--r-- | src/index/coinstatsindex.cpp | 68 | ||||
-rw-r--r-- | src/index/coinstatsindex.h | 10 | ||||
-rw-r--r-- | src/index/txindex.cpp | 17 | ||||
-rw-r--r-- | src/index/txindex.h | 4 | ||||
-rw-r--r-- | src/init.cpp | 12 | ||||
-rw-r--r-- | src/interfaces/chain.h | 31 | ||||
-rw-r--r-- | src/kernel/chain.cpp | 26 | ||||
-rw-r--r-- | src/kernel/chain.h | 19 | ||||
-rw-r--r-- | src/node/interfaces.cpp | 7 | ||||
-rw-r--r-- | src/test/blockfilter_index_tests.cpp | 11 | ||||
-rw-r--r-- | src/test/coinstatsindex_tests.cpp | 13 | ||||
-rw-r--r-- | src/test/txindex_tests.cpp | 5 | ||||
-rw-r--r-- | src/wallet/test/fuzz/notifications.cpp | 18 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 23 | ||||
-rw-r--r-- | src/wallet/wallet.h | 4 | ||||
-rwxr-xr-x | test/lint/lint-circular-dependencies.py | 3 |
21 files changed, 273 insertions, 142 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 02ec5c098e..7745f4d7c8 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/chain.h \ kernel/chainstatemanager_opts.h \ kernel/checks.h \ kernel/coinstats.h \ @@ -365,6 +366,7 @@ libbitcoin_node_a_SOURCES = \ index/coinstatsindex.cpp \ index/txindex.cpp \ init.cpp \ + kernel/chain.cpp \ kernel/checks.cpp \ kernel/coinstats.cpp \ kernel/context.cpp \ @@ -882,6 +884,7 @@ libbitcoinkernel_la_SOURCES = \ flatfile.cpp \ fs.cpp \ hash.cpp \ + kernel/chain.cpp \ kernel/checks.cpp \ kernel/coinstats.cpp \ kernel/context.cpp \ diff --git a/src/index/base.cpp b/src/index/base.cpp index 323547900d..4f399620e4 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -4,7 +4,10 @@ #include <chainparams.h> #include <index/base.h> +#include <interfaces/chain.h> +#include <kernel/chain.h> #include <node/blockstorage.h> +#include <node/context.h> #include <node/interface_ui.h> #include <shutdown.h> #include <tinyformat.h> @@ -31,6 +34,15 @@ static void FatalError(const char* fmt, const Args&... args) StartShutdown(); } +CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash) +{ + CBlockLocator locator; + bool found = chain.findBlock(block_hash, interfaces::FoundBlock().locator(locator)); + assert(found); + assert(!locator.IsNull()); + return locator; +} + BaseIndex::DB::DB(const fs::path& path, size_t n_cache_size, bool f_memory, bool f_wipe, bool f_obfuscate) : CDBWrapper(path, n_cache_size, f_memory, f_wipe, f_obfuscate) {} @@ -49,6 +61,9 @@ void BaseIndex::DB::WriteBestBlock(CDBBatch& batch, const CBlockLocator& locator batch.Write(DB_BEST_BLOCK, locator); } +BaseIndex::BaseIndex(std::unique_ptr<interfaces::Chain> chain) + : m_chain{std::move(chain)} {} + BaseIndex::~BaseIndex() { Interrupt(); @@ -175,12 +190,15 @@ void BaseIndex::ThreadSync() } CBlock block; + interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex); if (!ReadBlockFromDisk(block, pindex, consensus_params)) { FatalError("%s: Failed to read block %s from disk", __func__, pindex->GetBlockHash().ToString()); return; + } else { + block_info.data = █ } - if (!WriteBlock(block, pindex)) { + if (!CustomAppend(block_info)) { FatalError("%s: Failed to write block %s to index database", __func__, pindex->GetBlockHash().ToString()); return; @@ -197,22 +215,20 @@ void BaseIndex::ThreadSync() bool BaseIndex::Commit() { - CDBBatch batch(GetDB()); - if (!CommitInternal(batch) || !GetDB().WriteBatch(batch)) { - return error("%s: Failed to commit latest %s state", __func__, GetName()); - } - return true; -} - -bool BaseIndex::CommitInternal(CDBBatch& batch) -{ - LOCK(cs_main); // Don't commit anything if we haven't indexed any block yet // (this could happen if init is interrupted). - if (m_best_block_index == nullptr) { - return false; + bool ok = m_best_block_index != nullptr; + if (ok) { + CDBBatch batch(GetDB()); + ok = CustomCommit(batch); + if (ok) { + GetDB().WriteBestBlock(batch, GetLocator(*m_chain, m_best_block_index.load()->GetBlockHash())); + ok = GetDB().WriteBatch(batch); + } + } + if (!ok) { + return error("%s: Failed to commit latest %s state", __func__, GetName()); } - GetDB().WriteBestBlock(batch, m_chainstate->m_chain.GetLocator(m_best_block_index)); return true; } @@ -221,6 +237,10 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti assert(current_tip == m_best_block_index); assert(current_tip->GetAncestor(new_tip->nHeight) == new_tip); + if (!CustomRewind({current_tip->GetBlockHash(), current_tip->nHeight}, {new_tip->GetBlockHash(), new_tip->nHeight})) { + return false; + } + // In the case of a reorg, ensure persisted block locator is not stale. // Pruning has a minimum of 288 blocks-to-keep and getting the index // out of sync may be possible but a users fault. @@ -268,8 +288,8 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const return; } } - - if (WriteBlock(*block, pindex)) { + interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex, block.get()); + if (CustomAppend(block_info)) { SetBestBlockIndex(pindex); } else { FatalError("%s: Failed to write block %s to index", @@ -346,13 +366,18 @@ 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); - if (!Init()) { + if (!Init()) return false; + + const CBlockIndex* index = m_best_block_index.load(); + if (!CustomInit(index ? std::make_optional(interfaces::BlockKey{index->GetBlockHash(), index->nHeight}) : std::nullopt)) { return false; } diff --git a/src/index/base.h b/src/index/base.h index a8f6a18c8d..77c46dd428 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -6,12 +6,16 @@ #define BITCOIN_INDEX_BASE_H #include <dbwrapper.h> +#include <interfaces/chain.h> #include <threadinterrupt.h> #include <validationinterface.h> class CBlock; class CBlockIndex; class CChainState; +namespace interfaces { +class Chain; +} // namespace interfaces struct IndexSummary { std::string name; @@ -59,6 +63,9 @@ private: std::thread m_thread_sync; CThreadInterrupt m_interrupt; + /// Read best block locator and check that data needed to sync has not been pruned. + bool Init(); + /// Sync the index with the block index starting from the current best block. /// Intended to be run in its own thread, m_thread_sync, and can be /// interrupted with m_interrupt. Once the index gets in sync, the m_synced @@ -76,30 +83,32 @@ private: /// getting corrupted. bool Commit(); + /// Loop over disconnected blocks and call CustomRewind. + bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip); + virtual bool AllowPrune() const = 0; protected: + std::unique_ptr<interfaces::Chain> m_chain; CChainState* m_chainstate{nullptr}; void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override; void ChainStateFlushed(const CBlockLocator& locator) override; - const CBlockIndex* CurrentIndex() { return m_best_block_index.load(); }; - /// Initialize internal state from the database and block index. - [[nodiscard]] virtual bool Init(); + [[nodiscard]] virtual bool CustomInit(const std::optional<interfaces::BlockKey>& block) { return true; } /// Write update index entries for a newly connected block. - virtual bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) { return true; } + [[nodiscard]] virtual bool CustomAppend(const interfaces::BlockInfo& block) { return true; } /// Virtual method called internally by Commit that can be overridden to atomically /// commit more index state. - virtual bool CommitInternal(CDBBatch& batch); + virtual bool CustomCommit(CDBBatch& batch) { return true; } /// Rewind index to an earlier chain tip during a chain reorg. The tip must /// be an ancestor of the current best block. - virtual bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip); + [[nodiscard]] virtual bool CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) { return true; } virtual DB& GetDB() const = 0; @@ -110,6 +119,7 @@ protected: void SetBestBlockIndex(const CBlockIndex* block); public: + BaseIndex(std::unique_ptr<interfaces::Chain> chain); /// Destructor interrupts sync thread if running and blocks until it exits. virtual ~BaseIndex(); @@ -124,7 +134,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..8232567230 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -9,6 +9,7 @@ #include <index/blockfilterindex.h> #include <node/blockstorage.h> #include <util/system.h> +#include <validation.h> using node::UndoReadFromDisk; @@ -94,9 +95,9 @@ struct DBHashKey { static std::map<BlockFilterType, BlockFilterIndex> g_filter_indexes; -BlockFilterIndex::BlockFilterIndex(BlockFilterType filter_type, +BlockFilterIndex::BlockFilterIndex(std::unique_ptr<interfaces::Chain> 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"); @@ -109,7 +110,7 @@ BlockFilterIndex::BlockFilterIndex(BlockFilterType filter_type, m_filter_fileseq = std::make_unique<FlatFileSeq>(std::move(path), "fltr", FLTR_FILE_CHUNK_SIZE); } -bool BlockFilterIndex::Init() +bool BlockFilterIndex::CustomInit(const std::optional<interfaces::BlockKey>& block) { if (!m_db->Read(DB_FILTER_POS, m_next_filter_pos)) { // Check that the cause of the read failure is that the key does not exist. Any other errors @@ -124,10 +125,10 @@ bool BlockFilterIndex::Init() m_next_filter_pos.nFile = 0; m_next_filter_pos.nPos = 0; } - return BaseIndex::Init(); + return true; } -bool BlockFilterIndex::CommitInternal(CDBBatch& batch) +bool BlockFilterIndex::CustomCommit(CDBBatch& batch) { const FlatFilePos& pos = m_next_filter_pos; @@ -141,7 +142,7 @@ bool BlockFilterIndex::CommitInternal(CDBBatch& batch) } batch.Write(DB_FILTER_POS, pos); - return BaseIndex::CommitInternal(batch); + return true; } bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, const uint256& hash, BlockFilter& filter) const @@ -214,22 +215,25 @@ size_t BlockFilterIndex::WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& return data_size; } -bool BlockFilterIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) +bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block) { CBlockUndo block_undo; uint256 prev_header; - if (pindex->nHeight > 0) { + if (block.height > 0) { + // pindex variable gives indexing code access to node internals. It + // will be removed in upcoming commit + const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate->m_blockman.LookupBlockIndex(block.hash)); if (!UndoReadFromDisk(block_undo, pindex)) { return false; } std::pair<uint256, DBVal> read_out; - if (!m_db->Read(DBHeightKey(pindex->nHeight - 1), read_out)) { + if (!m_db->Read(DBHeightKey(block.height - 1), read_out)) { return false; } - uint256 expected_block_hash = pindex->pprev->GetBlockHash(); + uint256 expected_block_hash = *Assert(block.prev_hash); if (read_out.first != expected_block_hash) { return error("%s: previous block header belongs to unexpected block %s; expected %s", __func__, read_out.first.ToString(), expected_block_hash.ToString()); @@ -238,18 +242,18 @@ bool BlockFilterIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex prev_header = read_out.second.header; } - BlockFilter filter(m_filter_type, block, block_undo); + BlockFilter filter(m_filter_type, *Assert(block.data), block_undo); size_t bytes_written = WriteFilterToDisk(m_next_filter_pos, filter); if (bytes_written == 0) return false; std::pair<uint256, DBVal> value; - value.first = pindex->GetBlockHash(); + value.first = block.hash; value.second.hash = filter.GetHash(); value.second.header = filter.ComputeHeader(prev_header); value.second.pos = m_next_filter_pos; - if (!m_db->Write(DBHeightKey(pindex->nHeight), value)) { + if (!m_db->Write(DBHeightKey(block.height), value)) { return false; } @@ -283,17 +287,15 @@ static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, return true; } -bool BlockFilterIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) +bool BlockFilterIndex::CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) { - assert(current_tip->GetAncestor(new_tip->nHeight) == new_tip); - CDBBatch batch(*m_db); std::unique_ptr<CDBIterator> db_it(m_db->NewIterator()); // During a reorg, we need to copy all filters for blocks that are getting disconnected from the // height index to the hash index so we can still find them when the height index entries are // overwritten. - if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, new_tip->nHeight, current_tip->nHeight)) { + if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, new_tip.height, current_tip.height)) { return false; } @@ -303,7 +305,7 @@ bool BlockFilterIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* batch.Write(DB_FILTER_POS, m_next_filter_pos); if (!m_db->WriteBatch(batch)) return false; - return BaseIndex::Rewind(current_tip, new_tip); + return true; } static bool LookupOne(const CDBWrapper& db, const CBlockIndex* block_index, DBVal& result) @@ -467,12 +469,12 @@ void ForEachBlockFilterIndex(std::function<void (BlockFilterIndex&)> fn) for (auto& entry : g_filter_indexes) fn(entry.second); } -bool InitBlockFilterIndex(BlockFilterType filter_type, +bool InitBlockFilterIndex(std::function<std::unique_ptr<interfaces::Chain>()> 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..968eccb6b3 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -41,13 +41,13 @@ private: bool AllowPrune() const override { return true; } protected: - bool Init() override; + bool CustomInit(const std::optional<interfaces::BlockKey>& block) override; - bool CommitInternal(CDBBatch& batch) override; + bool CustomCommit(CDBBatch& batch) override; - bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override; + bool CustomAppend(const interfaces::BlockInfo& block) override; - bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) override; + bool CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) override; BaseIndex::DB& GetDB() const override { return *m_db; } @@ -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<interfaces::Chain> 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<void (BlockFilterIndex&)> 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<std::unique_ptr<interfaces::Chain>()> 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..b722118b2e 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -102,7 +102,8 @@ struct DBHashKey { std::unique_ptr<CoinStatsIndex> g_coin_stats_index; -CoinStatsIndex::CoinStatsIndex(size_t n_cache_size, bool f_memory, bool f_wipe) +CoinStatsIndex::CoinStatsIndex(std::unique_ptr<interfaces::Chain> 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); @@ -110,24 +111,27 @@ CoinStatsIndex::CoinStatsIndex(size_t n_cache_size, bool f_memory, bool f_wipe) m_db = std::make_unique<CoinStatsIndex::DB>(path / "db", n_cache_size, f_memory, f_wipe); } -bool CoinStatsIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) +bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) { CBlockUndo block_undo; - const CAmount block_subsidy{GetBlockSubsidy(pindex->nHeight, Params().GetConsensus())}; + const CAmount block_subsidy{GetBlockSubsidy(block.height, Params().GetConsensus())}; m_total_subsidy += block_subsidy; // Ignore genesis block - if (pindex->nHeight > 0) { + if (block.height > 0) { + // pindex variable gives indexing code access to node internals. It + // will be removed in upcoming commit + const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate->m_blockman.LookupBlockIndex(block.hash)); if (!UndoReadFromDisk(block_undo, pindex)) { return false; } std::pair<uint256, DBVal> read_out; - if (!m_db->Read(DBHeightKey(pindex->nHeight - 1), read_out)) { + if (!m_db->Read(DBHeightKey(block.height - 1), read_out)) { return false; } - uint256 expected_block_hash{pindex->pprev->GetBlockHash()}; + uint256 expected_block_hash{*Assert(block.prev_hash)}; if (read_out.first != expected_block_hash) { LogPrintf("WARNING: previous block header belongs to unexpected block %s; expected %s\n", read_out.first.ToString(), expected_block_hash.ToString()); @@ -139,12 +143,13 @@ bool CoinStatsIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) } // TODO: Deduplicate BIP30 related code - bool is_bip30_block{(pindex->nHeight == 91722 && pindex->GetBlockHash() == uint256S("0x00000000000271a2dc26e7667f8419f2e15416dc6955e5a6c6cdf3f2574dd08e")) || - (pindex->nHeight == 91812 && pindex->GetBlockHash() == uint256S("0x00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f"))}; + bool is_bip30_block{(block.height == 91722 && block.hash == uint256S("0x00000000000271a2dc26e7667f8419f2e15416dc6955e5a6c6cdf3f2574dd08e")) || + (block.height == 91812 && block.hash == uint256S("0x00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f"))}; // Add the new utxos created from the block - for (size_t i = 0; i < block.vtx.size(); ++i) { - const auto& tx{block.vtx.at(i)}; + assert(block.data); + for (size_t i = 0; i < block.data->vtx.size(); ++i) { + const auto& tx{block.data->vtx.at(i)}; // Skip duplicate txid coinbase transactions (BIP30). if (is_bip30_block && tx->IsCoinBase()) { @@ -155,7 +160,7 @@ bool CoinStatsIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) for (uint32_t j = 0; j < tx->vout.size(); ++j) { const CTxOut& out{tx->vout[j]}; - Coin coin{out, pindex->nHeight, tx->IsCoinBase()}; + Coin coin{out, block.height, tx->IsCoinBase()}; COutPoint outpoint{tx->GetHash(), j}; // Skip unspendable coins @@ -211,7 +216,7 @@ bool CoinStatsIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) m_total_unspendables_unclaimed_rewards += unclaimed_rewards; std::pair<uint256, DBVal> value; - value.first = pindex->GetBlockHash(); + value.first = block.hash; value.second.transaction_output_count = m_transaction_output_count; value.second.bogo_size = m_bogo_size; value.second.total_amount = m_total_amount; @@ -231,7 +236,7 @@ bool CoinStatsIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) // Intentionally do not update DB_MUHASH here so it stays in sync with // DB_BEST_BLOCK, and the index is not corrupted if there is an unclean shutdown. - return m_db->Write(DBHeightKey(pindex->nHeight), value); + return m_db->Write(DBHeightKey(block.height), value); } static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, @@ -260,17 +265,15 @@ static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, return true; } -bool CoinStatsIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) +bool CoinStatsIndex::CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) { - assert(current_tip->GetAncestor(new_tip->nHeight) == new_tip); - CDBBatch batch(*m_db); std::unique_ptr<CDBIterator> db_it(m_db->NewIterator()); // During a reorg, we need to copy all hash digests for blocks that are // getting disconnected from the height index to the hash index so we can // still find them when the height index entries are overwritten. - if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, new_tip->nHeight, current_tip->nHeight)) { + if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, new_tip.height, current_tip.height)) { return false; } @@ -278,7 +281,8 @@ bool CoinStatsIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* n { LOCK(cs_main); - const CBlockIndex* iter_tip{m_chainstate->m_blockman.LookupBlockIndex(current_tip->GetBlockHash())}; + const CBlockIndex* iter_tip{m_chainstate->m_blockman.LookupBlockIndex(current_tip.hash)}; + const CBlockIndex* new_tip_index{m_chainstate->m_blockman.LookupBlockIndex(new_tip.hash)}; const auto& consensus_params{Params().GetConsensus()}; do { @@ -292,29 +296,29 @@ bool CoinStatsIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* n ReverseBlock(block, iter_tip); iter_tip = iter_tip->GetAncestor(iter_tip->nHeight - 1); - } while (new_tip != iter_tip); + } while (new_tip_index != iter_tip); } - return BaseIndex::Rewind(current_tip, new_tip); + return true; } -static bool LookUpOne(const CDBWrapper& db, const CBlockIndex* block_index, DBVal& result) +static bool LookUpOne(const CDBWrapper& db, const interfaces::BlockKey& block, DBVal& result) { // First check if the result is stored under the height index and the value // there matches the block hash. This should be the case if the block is on // the active chain. std::pair<uint256, DBVal> read_out; - if (!db.Read(DBHeightKey(block_index->nHeight), read_out)) { + if (!db.Read(DBHeightKey(block.height), read_out)) { return false; } - if (read_out.first == block_index->GetBlockHash()) { + if (read_out.first == block.hash) { result = std::move(read_out.second); return true; } // If value at the height index corresponds to an different block, the // result will be stored in the hash index. - return db.Read(DBHashKey(block_index->GetBlockHash()), result); + return db.Read(DBHashKey(block.hash), result); } std::optional<CCoinsStats> CoinStatsIndex::LookUpStats(const CBlockIndex* block_index) const @@ -323,7 +327,7 @@ std::optional<CCoinsStats> CoinStatsIndex::LookUpStats(const CBlockIndex* block_ stats.index_used = true; DBVal entry; - if (!LookUpOne(*m_db, block_index, entry)) { + if (!LookUpOne(*m_db, {block_index->GetBlockHash(), block_index->nHeight}, entry)) { return std::nullopt; } @@ -344,7 +348,7 @@ std::optional<CCoinsStats> CoinStatsIndex::LookUpStats(const CBlockIndex* block_ return stats; } -bool CoinStatsIndex::Init() +bool CoinStatsIndex::CustomInit(const std::optional<interfaces::BlockKey>& block) { if (!m_db->Read(DB_MUHASH, m_muhash)) { // Check that the cause of the read failure is that the key does not @@ -356,13 +360,9 @@ bool CoinStatsIndex::Init() } } - if (!BaseIndex::Init()) return false; - - const CBlockIndex* pindex{CurrentIndex()}; - - if (pindex) { + if (block) { DBVal entry; - if (!LookUpOne(*m_db, pindex, entry)) { + if (!LookUpOne(*m_db, *block, entry)) { return error("%s: Cannot read current %s state; index may be corrupted", __func__, GetName()); } @@ -391,12 +391,12 @@ bool CoinStatsIndex::Init() return true; } -bool CoinStatsIndex::CommitInternal(CDBBatch& batch) +bool CoinStatsIndex::CustomCommit(CDBBatch& batch) { // DB_MUHASH should always be committed in a batch together with DB_BEST_BLOCK // to prevent an inconsistent state of the DB. batch.Write(DB_MUHASH, m_muhash); - return BaseIndex::CommitInternal(batch); + return true; } // Reverse a single block as part of a reorg diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h index cae052d913..c4af223388 100644 --- a/src/index/coinstatsindex.h +++ b/src/index/coinstatsindex.h @@ -39,13 +39,13 @@ private: bool AllowPrune() const override { return true; } protected: - bool Init() override; + bool CustomInit(const std::optional<interfaces::BlockKey>& block) override; - bool CommitInternal(CDBBatch& batch) override; + bool CustomCommit(CDBBatch& batch) override; - bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override; + bool CustomAppend(const interfaces::BlockInfo& block) override; - bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) override; + bool CustomRewind(const interfaces::BlockKey& current_tip, const interfaces::BlockKey& new_tip) override; BaseIndex::DB& GetDB() const override { return *m_db; } @@ -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<interfaces::Chain> 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<kernel::CCoinsStats> LookUpStats(const CBlockIndex* block_index) const; diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index 97c11c4383..b719aface8 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -48,23 +48,22 @@ bool TxIndex::DB::WriteTxs(const std::vector<std::pair<uint256, CDiskTxPos>>& v_ return WriteBatch(batch); } -TxIndex::TxIndex(size_t n_cache_size, bool f_memory, bool f_wipe) - : m_db(std::make_unique<TxIndex::DB>(n_cache_size, f_memory, f_wipe)) +TxIndex::TxIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe) + : BaseIndex(std::move(chain)), m_db(std::make_unique<TxIndex::DB>(n_cache_size, f_memory, f_wipe)) {} TxIndex::~TxIndex() = default; -bool TxIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) +bool TxIndex::CustomAppend(const interfaces::BlockInfo& block) { // Exclude genesis block transaction because outputs are not spendable. - if (pindex->nHeight == 0) return true; + if (block.height == 0) return true; - CDiskTxPos pos{ - WITH_LOCK(::cs_main, return pindex->GetBlockPos()), - GetSizeOfCompactSize(block.vtx.size())}; + assert(block.data); + CDiskTxPos pos({block.file_number, block.data_pos}, GetSizeOfCompactSize(block.data->vtx.size())); std::vector<std::pair<uint256, CDiskTxPos>> vPos; - vPos.reserve(block.vtx.size()); - for (const auto& tx : block.vtx) { + vPos.reserve(block.data->vtx.size()); + for (const auto& tx : block.data->vtx) { vPos.emplace_back(tx->GetHash(), pos); pos.nTxOffset += ::GetSerializeSize(*tx, CLIENT_VERSION); } diff --git a/src/index/txindex.h b/src/index/txindex.h index ec339abaa1..be240c4582 100644 --- a/src/index/txindex.h +++ b/src/index/txindex.h @@ -23,7 +23,7 @@ private: bool AllowPrune() const override { return false; } protected: - bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override; + bool CustomAppend(const interfaces::BlockInfo& block) override; BaseIndex::DB& GetDB() const override; @@ -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<interfaces::Chain> 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 5ed1def4db..5e9df42881 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1593,22 +1593,22 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return InitError(*error); } - g_txindex = std::make_unique<TxIndex>(cache_sizes.tx_index, false, fReindex); - if (!g_txindex->Start(chainman.ActiveChainstate())) { + g_txindex = std::make_unique<TxIndex>(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<CoinStatsIndex>(/* cache size */ 0, false, fReindex); - if (!g_coin_stats_index->Start(chainman.ActiveChainstate())) { + g_coin_stats_index = std::make_unique<CoinStatsIndex>(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 ff994b8edc..5fc0e540a9 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -18,6 +18,7 @@ class ArgsManager; class CBlock; +class CBlockUndo; class CFeeRate; class CRPCCommand; class CScheduler; @@ -37,6 +38,12 @@ namespace interfaces { class Handler; class Wallet; +//! Hash/height pair to help track and identify blocks. +struct BlockKey { + uint256 hash; + int height = -1; +}; + //! Helper for findBlock to selectively return pieces of block data. If block is //! found, data will be returned by setting specified output variables. If block //! is not found, output variables will keep their previous values. @@ -50,6 +57,8 @@ public: FoundBlock& mtpTime(int64_t& mtp_time) { m_mtp_time = &mtp_time; return *this; } //! Return whether block is in the active (most-work) chain. FoundBlock& inActiveChain(bool& in_active_chain) { m_in_active_chain = &in_active_chain; return *this; } + //! Return locator if block is in the active chain. + FoundBlock& locator(CBlockLocator& locator) { m_locator = &locator; return *this; } //! Return next block in the active chain if current block is in the active chain. FoundBlock& nextBlock(const FoundBlock& next_block) { m_next_block = &next_block; return *this; } //! Read block data from disk. If the block exists but doesn't have data @@ -62,11 +71,25 @@ public: int64_t* m_max_time = nullptr; int64_t* m_mtp_time = nullptr; bool* m_in_active_chain = nullptr; + CBlockLocator* m_locator = nullptr; const FoundBlock* m_next_block = nullptr; CBlock* m_data = nullptr; mutable bool found = false; }; +//! Block data sent with blockConnected, blockDisconnected notifications. +struct BlockInfo { + const uint256& hash; + const uint256* prev_hash = nullptr; + int height = -1; + int file_number = -1; + unsigned data_pos = 0; + const CBlock* data = nullptr; + const CBlockUndo* undo_data = nullptr; + + BlockInfo(const uint256& hash LIFETIMEBOUND) : hash(hash) {} +}; + //! Interface giving clients (wallet processes, maybe other analysis tools in //! the future) ability to access to the chain state, receive notifications, //! estimate fees, and submit transactions. @@ -239,8 +262,8 @@ public: virtual ~Notifications() {} virtual void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) {} virtual void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) {} - virtual void blockConnected(const CBlock& block, int height) {} - virtual void blockDisconnected(const CBlock& block, int height) {} + virtual void blockConnected(const BlockInfo& block) {} + virtual void blockDisconnected(const BlockInfo& block) {} virtual void updatedBlockTip() {} virtual void chainStateFlushed(const CBlockLocator& locator) {} }; @@ -290,6 +313,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/kernel/chain.cpp b/src/kernel/chain.cpp new file mode 100644 index 0000000000..82e77125d7 --- /dev/null +++ b/src/kernel/chain.cpp @@ -0,0 +1,26 @@ +// 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. + +#include <chain.h> +#include <interfaces/chain.h> +#include <sync.h> +#include <uint256.h> + +class CBlock; + +namespace kernel { +interfaces::BlockInfo MakeBlockInfo(const CBlockIndex* index, const CBlock* data) +{ + interfaces::BlockInfo info{index ? *index->phashBlock : uint256::ZERO}; + if (index) { + info.prev_hash = index->pprev ? index->pprev->phashBlock : nullptr; + info.height = index->nHeight; + LOCK(::cs_main); + info.file_number = index->nFile; + info.data_pos = index->nDataPos; + } + info.data = data; + return info; +} +} // namespace kernel diff --git a/src/kernel/chain.h b/src/kernel/chain.h new file mode 100644 index 0000000000..f0750f8266 --- /dev/null +++ b/src/kernel/chain.h @@ -0,0 +1,19 @@ +// 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_CHAIN_H +#define BITCOIN_KERNEL_CHAIN_H + +class CBlock; +class CBlockIndex; +namespace interfaces { +struct BlockInfo; +} // namespace interfaces + +namespace kernel { +//! Return data from block index. +interfaces::BlockInfo MakeBlockInfo(const CBlockIndex* block_index, const CBlock* data = nullptr); +} // namespace kernel + +#endif // BITCOIN_KERNEL_CHAIN_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 46e0efb9b6..56b5a928ad 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -19,6 +19,7 @@ #include <netaddress.h> #include <netbase.h> #include <node/blockstorage.h> +#include <kernel/chain.h> #include <node/coin.h> #include <node/context.h> #include <node/transaction.h> @@ -401,6 +402,7 @@ bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<Rec if (block.m_max_time) *block.m_max_time = index->GetBlockTimeMax(); if (block.m_mtp_time) *block.m_mtp_time = index->GetMedianTimePast(); if (block.m_in_active_chain) *block.m_in_active_chain = active[index->nHeight] == index; + if (block.m_locator) { *block.m_locator = active.GetLocator(index); } if (block.m_next_block) FillBlock(active[index->nHeight] == index ? active[index->nHeight + 1] : nullptr, *block.m_next_block, lock, active); if (block.m_data) { REVERSE_LOCK(lock); @@ -426,11 +428,11 @@ public: } void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override { - m_notifications->blockConnected(*block, index->nHeight); + m_notifications->blockConnected(kernel::MakeBlockInfo(index, block.get())); } void BlockDisconnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override { - m_notifications->blockDisconnected(*block, index->nHeight); + m_notifications->blockDisconnected(kernel::MakeBlockInfo(index, block.get())); } void UpdatedBlockTip(const CBlockIndex* index, const CBlockIndex* fork_index, bool is_ibd) override { @@ -780,6 +782,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 <consensus/merkle.h> #include <consensus/validation.h> #include <index/blockfilterindex.h> +#include <interfaces/chain.h> #include <node/miner.h> #include <pow.h> #include <script/standard.h> @@ -110,7 +111,7 @@ bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex, BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) { - BlockFilterIndex filter_index(BlockFilterType::BASIC, 1 << 20, true); + BlockFilterIndex filter_index(interfaces::MakeChain(m_node), BlockFilterType::BASIC, 1 << 20, true); uint256 last_header; @@ -137,7 +138,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) // BlockUntilSyncedToCurrentChain should return false before index is started. BOOST_CHECK(!filter_index.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(filter_index.Start(m_node.chainman->ActiveChainstate())); + BOOST_REQUIRE(filter_index.Start()); // Allow filter index to catch up with the block index. constexpr int64_t timeout_ms = 10 * 1000; @@ -279,14 +280,14 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_init_destroy, BasicTestingSetup) filter_index = GetBlockFilterIndex(BlockFilterType::BASIC); BOOST_CHECK(filter_index == nullptr); - BOOST_CHECK(InitBlockFilterIndex(BlockFilterType::BASIC, 1 << 20, true, false)); + BOOST_CHECK(InitBlockFilterIndex([&]{ return interfaces::MakeChain(m_node); }, BlockFilterType::BASIC, 1 << 20, true, false)); filter_index = GetBlockFilterIndex(BlockFilterType::BASIC); BOOST_CHECK(filter_index != nullptr); BOOST_CHECK(filter_index->GetFilterType() == BlockFilterType::BASIC); // Initialize returns false if index already exists. - BOOST_CHECK(!InitBlockFilterIndex(BlockFilterType::BASIC, 1 << 20, true, false)); + BOOST_CHECK(!InitBlockFilterIndex([&]{ return interfaces::MakeChain(m_node); }, BlockFilterType::BASIC, 1 << 20, true, false)); int iter_count = 0; ForEachBlockFilterIndex([&iter_count](BlockFilterIndex& _index) { iter_count++; }); @@ -301,7 +302,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_init_destroy, BasicTestingSetup) BOOST_CHECK(filter_index == nullptr); // Reinitialize index. - BOOST_CHECK(InitBlockFilterIndex(BlockFilterType::BASIC, 1 << 20, true, false)); + BOOST_CHECK(InitBlockFilterIndex([&]{ return interfaces::MakeChain(m_node); }, BlockFilterType::BASIC, 1 << 20, true, false)); DestroyAllBlockFilterIndexes(); diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 1d9a037a66..c93d05a93b 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -4,6 +4,7 @@ #include <chainparams.h> #include <index/coinstatsindex.h> +#include <interfaces/chain.h> #include <test/util/setup_common.h> #include <test/util/validation.h> #include <util/time.h> @@ -28,7 +29,7 @@ static void IndexWaitSynced(BaseIndex& index) BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) { - CoinStatsIndex coin_stats_index{1 << 20, true}; + CoinStatsIndex coin_stats_index{interfaces::MakeChain(m_node), 1 << 20, true}; const CBlockIndex* block_index; { @@ -43,7 +44,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) // is started. BOOST_CHECK(!coin_stats_index.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(coin_stats_index.Start(m_node.chainman->ActiveChainstate())); + BOOST_REQUIRE(coin_stats_index.Start()); IndexWaitSynced(coin_stats_index); @@ -87,8 +88,8 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) CChainState& chainstate = Assert(m_node.chainman)->ActiveChainstate(); const CChainParams& params = Params(); { - CoinStatsIndex index{1 << 20}; - BOOST_REQUIRE(index.Start(chainstate)); + CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20}; + BOOST_REQUIRE(index.Start()); IndexWaitSynced(index); std::shared_ptr<const CBlock> new_block; CBlockIndex* new_block_index = nullptr; @@ -113,9 +114,9 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) } { - CoinStatsIndex index{1 << 20}; + CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20}; // Make sure the index can be loaded. - BOOST_REQUIRE(index.Start(chainstate)); + BOOST_REQUIRE(index.Start()); index.Stop(); } } diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index 15213f826b..62c7ddb673 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -4,6 +4,7 @@ #include <chainparams.h> #include <index/txindex.h> +#include <interfaces/chain.h> #include <script/standard.h> #include <test/util/setup_common.h> #include <util/time.h> @@ -15,7 +16,7 @@ BOOST_AUTO_TEST_SUITE(txindex_tests) BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) { - TxIndex txindex(1 << 20, true); + TxIndex txindex(interfaces::MakeChain(m_node), 1 << 20, true); CTransactionRef tx_disk; uint256 block_hash; @@ -28,7 +29,7 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) // BlockUntilSyncedToCurrentChain should return false before txindex is started. BOOST_CHECK(!txindex.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(txindex.Start(m_node.chainman->ActiveChainstate())); + BOOST_REQUIRE(txindex.Start()); // Allow tx index to catch up with the block index. constexpr int64_t timeout_ms = 10 * 1000; diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index 816bef6148..5c173773e4 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -137,8 +137,13 @@ FUZZ_TARGET_INIT(wallet_notifications, initialize_setup) block.vtx.emplace_back(MakeTransactionRef(tx)); } // Mine block - a.wallet->blockConnected(block, chain.size()); - b.wallet->blockConnected(block, chain.size()); + const uint256& hash = block.GetHash(); + interfaces::BlockInfo info{hash}; + info.prev_hash = &block.hashPrevBlock; + info.height = chain.size(); + info.data = █ + a.wallet->blockConnected(info); + b.wallet->blockConnected(info); // Store the coins for the next block Coins coins_new; for (const auto& tx : block.vtx) { @@ -154,8 +159,13 @@ FUZZ_TARGET_INIT(wallet_notifications, initialize_setup) auto& [coins, block]{chain.back()}; if (block.vtx.empty()) return; // Can only disconnect if the block was submitted first // Disconnect block - a.wallet->blockDisconnected(block, chain.size() - 1); - b.wallet->blockDisconnected(block, chain.size() - 1); + const uint256& hash = block.GetHash(); + interfaces::BlockInfo info{hash}; + info.prev_hash = &block.hashPrevBlock; + info.height = chain.size() - 1; + info.data = █ + a.wallet->blockDisconnected(info); + b.wallet->blockDisconnected(info); chain.pop_back(); }); auto& [coins, first_block]{chain.front()}; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9af68000aa..54a3221e2d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1314,30 +1314,31 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRe } } -void CWallet::blockConnected(const CBlock& block, int height) +void CWallet::blockConnected(const interfaces::BlockInfo& block) { - const uint256& block_hash = block.GetHash(); + assert(block.data); LOCK(cs_wallet); - m_last_block_processed_height = height; - m_last_block_processed = block_hash; - for (size_t index = 0; index < block.vtx.size(); index++) { - SyncTransaction(block.vtx[index], TxStateConfirmed{block_hash, height, static_cast<int>(index)}); - transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::BLOCK, 0 /* mempool_sequence */); + m_last_block_processed_height = block.height; + m_last_block_processed = block.hash; + for (size_t index = 0; index < block.data->vtx.size(); index++) { + SyncTransaction(block.data->vtx[index], TxStateConfirmed{block.hash, block.height, static_cast<int>(index)}); + transactionRemovedFromMempool(block.data->vtx[index], MemPoolRemovalReason::BLOCK, 0 /* mempool_sequence */); } } -void CWallet::blockDisconnected(const CBlock& block, int height) +void CWallet::blockDisconnected(const interfaces::BlockInfo& block) { + assert(block.data); LOCK(cs_wallet); // At block disconnection, this will change an abandoned transaction to // be unconfirmed, whether or not the transaction is added back to the mempool. // User may have to call abandontransaction again. It may be addressed in the // future with a stickier abandoned state or even removing abandontransaction call. - m_last_block_processed_height = height - 1; - m_last_block_processed = block.hashPrevBlock; - for (const CTransactionRef& ptx : block.vtx) { + m_last_block_processed_height = block.height - 1; + m_last_block_processed = *Assert(block.prev_hash); + for (const CTransactionRef& ptx : Assert(block.data)->vtx) { SyncTransaction(ptx, TxStateInactive{}); } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e2c3d76438..15f5917040 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -508,8 +508,8 @@ public: CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false); bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override; - void blockConnected(const CBlock& block, int height) override; - void blockDisconnected(const CBlock& block, int height) override; + void blockConnected(const interfaces::BlockInfo& block) override; + void blockDisconnected(const interfaces::BlockInfo& block) override; void updatedBlockTip() override; int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update); diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index f44ed8f7c7..a0f17ac119 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -23,6 +23,9 @@ EXPECTED_CIRCULAR_DEPENDENCIES = ( "wallet/wallet -> wallet/walletdb -> wallet/wallet", "kernel/coinstats -> validation -> kernel/coinstats", "kernel/mempool_persist -> validation -> kernel/mempool_persist", + + # Temporary, removed in followup https://github.com/bitcoin/bitcoin/pull/24230 + "index/base -> node/context -> net_processing -> index/blockfilterindex -> index/base", ) CODE_DIR = "src" |