From a67d3eb91d5ef840fc49167d7048a33872ecddf8 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 9 Jul 2025 17:18:28 -0400 Subject: [PATCH 1/3] index: deduplicate Hash / Height handling The code was largely duplicated between coinstatsindex and blockfilterindex. Deduplicate it by moving it to a shared file. slight change in behavior: the index name is no longer part of the error msg in case of (un)serialization errors. --- src/index/blockfilterindex.cpp | 72 ++------------------------ src/index/coinstatsindex.cpp | 70 +------------------------ src/index/db_key.h | 94 ++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 137 deletions(-) create mode 100644 src/index/db_key.h diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 4bf2f5e66d9..62a78016ea0 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -25,7 +26,6 @@ #include #include -#include #include #include #include @@ -46,12 +46,8 @@ * disk location of the next block filter to be written (represented as a FlatFilePos) is stored * under the DB_FILTER_POS key. * - * Keys for the height index have the type [DB_BLOCK_HEIGHT, uint32 (BE)]. The height is represented - * as big-endian so that sequential reads of filters by height are fast. - * Keys for the hash index have the type [DB_BLOCK_HASH, uint256]. + * The logic for keys is shared with other indexes, see index/db_key.h. */ -constexpr uint8_t DB_BLOCK_HASH{'s'}; -constexpr uint8_t DB_BLOCK_HEIGHT{'t'}; constexpr uint8_t DB_FILTER_POS{'P'}; constexpr unsigned int MAX_FLTR_FILE_SIZE = 0x1000000; // 16 MiB @@ -74,45 +70,6 @@ struct DBVal { SERIALIZE_METHODS(DBVal, obj) { READWRITE(obj.hash, obj.header, obj.pos); } }; -struct DBHeightKey { - int height; - - explicit DBHeightKey(int height_in) : height(height_in) {} - - template - void Serialize(Stream& s) const - { - ser_writedata8(s, DB_BLOCK_HEIGHT); - ser_writedata32be(s, height); - } - - template - void Unserialize(Stream& s) - { - const uint8_t prefix{ser_readdata8(s)}; - if (prefix != DB_BLOCK_HEIGHT) { - throw std::ios_base::failure("Invalid format for block filter index DB height key"); - } - height = ser_readdata32be(s); - } -}; - -struct DBHashKey { - uint256 hash; - - explicit DBHashKey(const uint256& hash_in) : hash(hash_in) {} - - SERIALIZE_METHODS(DBHashKey, obj) { - uint8_t prefix{DB_BLOCK_HASH}; - READWRITE(prefix); - if (prefix != DB_BLOCK_HASH) { - throw std::ios_base::failure("Invalid format for block filter index DB hash key"); - } - - READWRITE(obj.hash); - } -}; - }; // namespace static std::map g_filter_indexes; @@ -317,29 +274,6 @@ bool BlockFilterIndex::Write(const BlockFilter& filter, uint32_t block_height, c return true; } -[[nodiscard]] static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, - const std::string& index_name, int height) -{ - DBHeightKey key(height); - db_it.Seek(key); - - if (!db_it.GetKey(key) || key.height != height) { - LogError("unexpected key in %s: expected (%c, %d)", - index_name, DB_BLOCK_HEIGHT, height); - return false; - } - - std::pair value; - if (!db_it.GetValue(value)) { - LogError("unable to read value in %s at key (%c, %d)", - index_name, DB_BLOCK_HEIGHT, height); - return false; - } - - batch.Write(DBHashKey(value.first), std::move(value.second)); - return true; -} - bool BlockFilterIndex::CustomRemove(const interfaces::BlockInfo& block) { CDBBatch batch(*m_db); @@ -348,7 +282,7 @@ bool BlockFilterIndex::CustomRemove(const interfaces::BlockInfo& block) // During a reorg, we need to copy block filter that is getting disconnected from the // height index to the hash index so we can still find it when the height index entry // is overwritten. - if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) { + if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) { return false; } diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 72bc09ce45c..a12c52d0658 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -28,7 +29,6 @@ #include #include -#include #include #include #include @@ -40,8 +40,6 @@ using kernel::CCoinsStats; using kernel::GetBogoSize; using kernel::RemoveCoinHash; -static constexpr uint8_t DB_BLOCK_HASH{'s'}; -static constexpr uint8_t DB_BLOCK_HEIGHT{'t'}; static constexpr uint8_t DB_MUHASH{'M'}; namespace { @@ -85,47 +83,6 @@ struct DBVal { SER_READ(obj, obj.total_coinbase_amount = UintToArith256(coinbase)); } }; - -struct DBHeightKey { - int height; - - explicit DBHeightKey(int height_in) : height(height_in) {} - - template - void Serialize(Stream& s) const - { - ser_writedata8(s, DB_BLOCK_HEIGHT); - ser_writedata32be(s, height); - } - - template - void Unserialize(Stream& s) - { - const uint8_t prefix{ser_readdata8(s)}; - if (prefix != DB_BLOCK_HEIGHT) { - throw std::ios_base::failure("Invalid format for coinstatsindex DB height key"); - } - height = ser_readdata32be(s); - } -}; - -struct DBHashKey { - uint256 block_hash; - - explicit DBHashKey(const uint256& hash_in) : block_hash(hash_in) {} - - SERIALIZE_METHODS(DBHashKey, obj) - { - uint8_t prefix{DB_BLOCK_HASH}; - READWRITE(prefix); - if (prefix != DB_BLOCK_HASH) { - throw std::ios_base::failure("Invalid format for coinstatsindex DB hash key"); - } - - READWRITE(obj.block_hash); - } -}; - }; // namespace std::unique_ptr g_coin_stats_index; @@ -257,29 +214,6 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) return true; } -[[nodiscard]] static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, - const std::string& index_name, int height) -{ - DBHeightKey key{height}; - db_it.Seek(key); - - if (!db_it.GetKey(key) || key.height != height) { - LogError("unexpected key in %s: expected (%c, %d)", - index_name, DB_BLOCK_HEIGHT, height); - return false; - } - - std::pair value; - if (!db_it.GetValue(value)) { - LogError("unable to read value in %s at key (%c, %d)", - index_name, DB_BLOCK_HEIGHT, height); - return false; - } - - batch.Write(DBHashKey(value.first), value.second); - return true; -} - bool CoinStatsIndex::CustomRemove(const interfaces::BlockInfo& block) { CDBBatch batch(*m_db); @@ -287,7 +221,7 @@ bool CoinStatsIndex::CustomRemove(const interfaces::BlockInfo& block) // During a reorg, copy the block's hash digest from the height index to the hash index, // ensuring it's still accessible after the height index entry is overwritten. - if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) { + if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) { return false; } diff --git a/src/index/db_key.h b/src/index/db_key.h new file mode 100644 index 00000000000..c5369675e46 --- /dev/null +++ b/src/index/db_key.h @@ -0,0 +1,94 @@ +// Copyright (c) 2025-present 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_INDEX_DB_KEY_H +#define BITCOIN_INDEX_DB_KEY_H + +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +/* + * This file includes the logic for the db keys used by blockfilterindex and coinstatsindex. + * Index data is usually indexed by height, but in case of a reorg, entries of blocks no + * longer in the main chain will be copied to a hash index by which they can still be queried. + * Keys for the height index have the type [DB_BLOCK_HEIGHT, uint32 (BE)]. The height is represented + * as big-endian so that sequential reads of filters by height are fast. + * Keys for the hash index have the type [DB_BLOCK_HASH, uint256]. + */ + +static constexpr uint8_t DB_BLOCK_HASH{'s'}; +static constexpr uint8_t DB_BLOCK_HEIGHT{'t'}; + +struct DBHeightKey { + int height; + + explicit DBHeightKey(int height_in) : height(height_in) {} + + template + void Serialize(Stream& s) const + { + ser_writedata8(s, DB_BLOCK_HEIGHT); + ser_writedata32be(s, height); + } + + template + void Unserialize(Stream& s) + { + const uint8_t prefix{ser_readdata8(s)}; + if (prefix != DB_BLOCK_HEIGHT) { + throw std::ios_base::failure("Invalid format for index DB height key"); + } + height = ser_readdata32be(s); + } +}; + +struct DBHashKey { + uint256 hash; + + explicit DBHashKey(const uint256& hash_in) : hash(hash_in) {} + + SERIALIZE_METHODS(DBHashKey, obj) { + uint8_t prefix{DB_BLOCK_HASH}; + READWRITE(prefix); + if (prefix != DB_BLOCK_HASH) { + throw std::ios_base::failure("Invalid format for index DB hash key"); + } + + READWRITE(obj.hash); + } +}; + +template +[[nodiscard]] static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, + const std::string& index_name, int height) +{ + DBHeightKey key(height); + db_it.Seek(key); + + if (!db_it.GetKey(key) || key.height != height) { + LogError("unexpected key in %s: expected (%c, %d)", + index_name, DB_BLOCK_HEIGHT, height); + return false; + } + + std::pair value; + if (!db_it.GetValue(value)) { + LogError("unable to read value in %s at key (%c, %d)", + index_name, DB_BLOCK_HEIGHT, height); + return false; + } + + batch.Write(DBHashKey(value.first), value.second); + return true; +} + +#endif // BITCOIN_INDEX_DB_KEY_H From 032f3503e3fe8e144640904670867afc18a4bbbf Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 16 Jul 2025 15:19:54 -0400 Subject: [PATCH 2/3] index, refactor: deduplicate LookUpOne LookUpOne is used by both coinstatsindex and blockfilterindex, the two implementations had already started to deviate slightly for no apparent reason. --- src/index/blockfilterindex.cpp | 22 ++-------------------- src/index/coinstatsindex.cpp | 19 ------------------- src/index/db_key.h | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+), 39 deletions(-) diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 62a78016ea0..68c11e35ef7 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -297,24 +297,6 @@ bool BlockFilterIndex::CustomRemove(const interfaces::BlockInfo& block) return true; } -static bool LookupOne(const CDBWrapper& db, const CBlockIndex* block_index, 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 read_out; - if (!db.Read(DBHeightKey(block_index->nHeight), read_out)) { - return false; - } - if (read_out.first == block_index->GetBlockHash()) { - 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); -} - static bool LookupRange(CDBWrapper& db, const std::string& index_name, int start_height, const CBlockIndex* stop_index, std::vector& results) { @@ -377,7 +359,7 @@ static bool LookupRange(CDBWrapper& db, const std::string& index_name, int start bool BlockFilterIndex::LookupFilter(const CBlockIndex* block_index, BlockFilter& filter_out) const { DBVal entry; - if (!LookupOne(*m_db, block_index, entry)) { + if (!LookUpOne(*m_db, {block_index->GetBlockHash(), block_index->nHeight}, entry)) { return false; } @@ -400,7 +382,7 @@ bool BlockFilterIndex::LookupFilterHeader(const CBlockIndex* block_index, uint25 } DBVal entry; - if (!LookupOne(*m_db, block_index, entry)) { + if (!LookUpOne(*m_db, {block_index->GetBlockHash(), block_index->nHeight}, entry)) { return false; } diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index a12c52d0658..bb914c1ef13 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -234,25 +234,6 @@ bool CoinStatsIndex::CustomRemove(const interfaces::BlockInfo& block) return true; } -static bool LookUpOne(const CDBWrapper& db, const interfaces::BlockRef& 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 read_out; - if (!db.Read(DBHeightKey(block.height), read_out)) { - return false; - } - 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.hash), result); -} - std::optional CoinStatsIndex::LookUpStats(const CBlockIndex& block_index) const { CCoinsStats stats{block_index.nHeight, block_index.GetBlockHash()}; diff --git a/src/index/db_key.h b/src/index/db_key.h index c5369675e46..329f5643344 100644 --- a/src/index/db_key.h +++ b/src/index/db_key.h @@ -91,4 +91,24 @@ template return true; } +template +static bool LookUpOne(const CDBWrapper& db, const interfaces::BlockRef& 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 read_out; + if (!db.Read(DBHeightKey(block.height), read_out)) { + return false; + } + 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.hash), result); +} + #endif // BITCOIN_INDEX_DB_KEY_H From 5646e6c0d3581f12419913b88745f51c7a3161b9 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Wed, 3 Dec 2025 15:43:24 -0500 Subject: [PATCH 3/3] index: restrict index helper function to namespace --- src/index/blockfilterindex.cpp | 20 ++++++++++---------- src/index/coinstatsindex.cpp | 12 ++++++------ src/index/db_key.h | 2 ++ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 68c11e35ef7..8bf5e4a7e43 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -235,7 +235,7 @@ size_t BlockFilterIndex::WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& std::optional BlockFilterIndex::ReadFilterHeader(int height, const uint256& expected_block_hash) { std::pair read_out; - if (!m_db->Read(DBHeightKey(height), read_out)) { + if (!m_db->Read(index_util::DBHeightKey(height), read_out)) { return std::nullopt; } @@ -268,7 +268,7 @@ bool BlockFilterIndex::Write(const BlockFilter& filter, uint32_t block_height, c value.second.header = filter_header; value.second.pos = m_next_filter_pos; - m_db->Write(DBHeightKey(block_height), value); + m_db->Write(index_util::DBHeightKey(block_height), value); m_next_filter_pos.nPos += bytes_written; return true; @@ -282,7 +282,7 @@ bool BlockFilterIndex::CustomRemove(const interfaces::BlockInfo& block) // During a reorg, we need to copy block filter that is getting disconnected from the // height index to the hash index so we can still find it when the height index entry // is overwritten. - if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) { + if (!index_util::CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) { return false; } @@ -313,9 +313,9 @@ static bool LookupRange(CDBWrapper& db, const std::string& index_name, int start size_t results_size = static_cast(stop_index->nHeight - start_height + 1); std::vector> values(results_size); - DBHeightKey key(start_height); + index_util::DBHeightKey key(start_height); std::unique_ptr db_it(db.NewIterator()); - db_it->Seek(DBHeightKey(start_height)); + db_it->Seek(index_util::DBHeightKey(start_height)); for (int height = start_height; height <= stop_index->nHeight; ++height) { if (!db_it->Valid() || !db_it->GetKey(key) || key.height != height) { return false; @@ -324,7 +324,7 @@ static bool LookupRange(CDBWrapper& db, const std::string& index_name, int start size_t i = static_cast(height - start_height); if (!db_it->GetValue(values[i])) { LogError("unable to read value in %s at key (%c, %d)", - index_name, DB_BLOCK_HEIGHT, height); + index_name, index_util::DB_BLOCK_HEIGHT, height); return false; } @@ -346,9 +346,9 @@ static bool LookupRange(CDBWrapper& db, const std::string& index_name, int start continue; } - if (!db.Read(DBHashKey(block_hash), results[i])) { + if (!db.Read(index_util::DBHashKey(block_hash), results[i])) { LogError("unable to read value in %s at key (%c, %s)", - index_name, DB_BLOCK_HASH, block_hash.ToString()); + index_name, index_util::DB_BLOCK_HASH, block_hash.ToString()); return false; } } @@ -359,7 +359,7 @@ static bool LookupRange(CDBWrapper& db, const std::string& index_name, int start bool BlockFilterIndex::LookupFilter(const CBlockIndex* block_index, BlockFilter& filter_out) const { DBVal entry; - if (!LookUpOne(*m_db, {block_index->GetBlockHash(), block_index->nHeight}, entry)) { + if (!index_util::LookUpOne(*m_db, {block_index->GetBlockHash(), block_index->nHeight}, entry)) { return false; } @@ -382,7 +382,7 @@ bool BlockFilterIndex::LookupFilterHeader(const CBlockIndex* block_index, uint25 } DBVal entry; - if (!LookUpOne(*m_db, {block_index->GetBlockHash(), block_index->nHeight}, entry)) { + if (!index_util::LookUpOne(*m_db, {block_index->GetBlockHash(), block_index->nHeight}, entry)) { return false; } diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index bb914c1ef13..0917ac451c9 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -210,7 +210,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) // 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. - m_db->Write(DBHeightKey(block.height), value); + m_db->Write(index_util::DBHeightKey(block.height), value); return true; } @@ -221,7 +221,7 @@ bool CoinStatsIndex::CustomRemove(const interfaces::BlockInfo& block) // During a reorg, copy the block's hash digest from the height index to the hash index, // ensuring it's still accessible after the height index entry is overwritten. - if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) { + if (!index_util::CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) { return false; } @@ -240,7 +240,7 @@ std::optional CoinStatsIndex::LookUpStats(const CBlockIndex& block_ stats.index_used = true; DBVal entry; - if (!LookUpOne(*m_db, {block_index.GetBlockHash(), block_index.nHeight}, entry)) { + if (!index_util::LookUpOne(*m_db, {block_index.GetBlockHash(), block_index.nHeight}, entry)) { return std::nullopt; } @@ -275,7 +275,7 @@ bool CoinStatsIndex::CustomInit(const std::optional& block if (block) { DBVal entry; - if (!LookUpOne(*m_db, *block, entry)) { + if (!index_util::LookUpOne(*m_db, *block, entry)) { LogError("Cannot read current %s state; index may be corrupted", GetName()); return false; @@ -330,7 +330,7 @@ bool CoinStatsIndex::RevertBlock(const interfaces::BlockInfo& block) // Ignore genesis block if (block.height > 0) { - if (!m_db->Read(DBHeightKey(block.height - 1), read_out)) { + if (!m_db->Read(index_util::DBHeightKey(block.height - 1), read_out)) { return false; } @@ -339,7 +339,7 @@ bool CoinStatsIndex::RevertBlock(const interfaces::BlockInfo& block) LogWarning("previous block header belongs to unexpected block %s; expected %s", read_out.first.ToString(), expected_block_hash.ToString()); - if (!m_db->Read(DBHashKey(expected_block_hash), read_out)) { + if (!m_db->Read(index_util::DBHashKey(expected_block_hash), read_out)) { LogError("previous block header not found; expected %s", expected_block_hash.ToString()); return false; diff --git a/src/index/db_key.h b/src/index/db_key.h index 329f5643344..b7334c81e41 100644 --- a/src/index/db_key.h +++ b/src/index/db_key.h @@ -16,6 +16,7 @@ #include #include +namespace index_util { /* * This file includes the logic for the db keys used by blockfilterindex and coinstatsindex. * Index data is usually indexed by height, but in case of a reorg, entries of blocks no @@ -110,5 +111,6 @@ static bool LookUpOne(const CDBWrapper& db, const interfaces::BlockRef& block, D // result will be stored in the hash index. return db.Read(DBHashKey(block.hash), result); } +} // namespace index_util #endif // BITCOIN_INDEX_DB_KEY_H