From 34f9a0157aad7c10ac364b7e4602c5f74c1f9e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Thu, 9 Jan 2025 12:41:44 +0100 Subject: [PATCH 1/7] refactor,bench: rename bench/readblock.cpp to bench/readwriteblock.cpp Done in separate commit to simplify review. Also renames benchmarks, since they're not strictly tests. Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> --- src/bench/CMakeLists.txt | 2 +- src/bench/{readblock.cpp => readwriteblock.cpp} | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) rename src/bench/{readblock.cpp => readwriteblock.cpp} (85%) diff --git a/src/bench/CMakeLists.txt b/src/bench/CMakeLists.txt index 4589ef177c3..c55bbb1e05f 100644 --- a/src/bench/CMakeLists.txt +++ b/src/bench/CMakeLists.txt @@ -44,7 +44,7 @@ add_executable(bench_bitcoin pool.cpp prevector.cpp random.cpp - readblock.cpp + readwriteblock.cpp rollingbloom.cpp rpc_blockchain.cpp rpc_mempool.cpp diff --git a/src/bench/readblock.cpp b/src/bench/readwriteblock.cpp similarity index 85% rename from src/bench/readblock.cpp rename to src/bench/readwriteblock.cpp index 058d953b4e9..7fd9adc2c13 100644 --- a/src/bench/readblock.cpp +++ b/src/bench/readwriteblock.cpp @@ -28,7 +28,7 @@ static FlatFilePos WriteBlockToDisk(ChainstateManager& chainman) return chainman.m_blockman.SaveBlockToDisk(block, 0); } -static void ReadBlockFromDiskTest(benchmark::Bench& bench) +static void ReadBlockFromDiskBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; ChainstateManager& chainman{*testing_setup->m_node.chainman}; @@ -42,7 +42,7 @@ static void ReadBlockFromDiskTest(benchmark::Bench& bench) }); } -static void ReadRawBlockFromDiskTest(benchmark::Bench& bench) +static void ReadRawBlockFromDiskBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; ChainstateManager& chainman{*testing_setup->m_node.chainman}; @@ -56,5 +56,5 @@ static void ReadRawBlockFromDiskTest(benchmark::Bench& bench) }); } -BENCHMARK(ReadBlockFromDiskTest, benchmark::PriorityLevel::HIGH); -BENCHMARK(ReadRawBlockFromDiskTest, benchmark::PriorityLevel::HIGH); +BENCHMARK(ReadBlockFromDiskBench, benchmark::PriorityLevel::HIGH); +BENCHMARK(ReadRawBlockFromDiskBench, benchmark::PriorityLevel::HIGH); From 86b85bb11f8999eb59e34bd026b0791dc866f2eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Thu, 9 Jan 2025 12:42:01 +0100 Subject: [PATCH 2/7] bench: add SaveBlockBench --- src/bench/readwriteblock.cpp | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/bench/readwriteblock.cpp b/src/bench/readwriteblock.cpp index 7fd9adc2c13..e6d87c6ef05 100644 --- a/src/bench/readwriteblock.cpp +++ b/src/bench/readwriteblock.cpp @@ -19,25 +19,33 @@ #include #include -static FlatFilePos WriteBlockToDisk(ChainstateManager& chainman) +static CBlock CreateTestBlock() { DataStream stream{benchmark::data::block413567}; CBlock block; stream >> TX_WITH_WITNESS(block); + return block; +} - return chainman.m_blockman.SaveBlockToDisk(block, 0); +static void SaveBlockBench(benchmark::Bench& bench) +{ + const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; + auto& blockman{testing_setup->m_node.chainman->m_blockman}; + const CBlock block{CreateTestBlock()}; + bench.run([&] { + const auto pos{blockman.SaveBlockToDisk(block, 413'567)}; + assert(!pos.IsNull()); + }); } static void ReadBlockFromDiskBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; - ChainstateManager& chainman{*testing_setup->m_node.chainman}; - + auto& blockman{testing_setup->m_node.chainman->m_blockman}; + const auto pos{blockman.SaveBlockToDisk(CreateTestBlock(), 413'567)}; CBlock block; - const auto pos{WriteBlockToDisk(chainman)}; - bench.run([&] { - const auto success{chainman.m_blockman.ReadBlockFromDisk(block, pos)}; + const auto success{blockman.ReadBlockFromDisk(block, pos)}; assert(success); }); } @@ -45,16 +53,16 @@ static void ReadBlockFromDiskBench(benchmark::Bench& bench) static void ReadRawBlockFromDiskBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; - ChainstateManager& chainman{*testing_setup->m_node.chainman}; - + auto& blockman{testing_setup->m_node.chainman->m_blockman}; + const auto pos{blockman.SaveBlockToDisk(CreateTestBlock(), 413'567)}; std::vector block_data; - const auto pos{WriteBlockToDisk(chainman)}; - + blockman.ReadRawBlockFromDisk(block_data, pos); // warmup bench.run([&] { - const auto success{chainman.m_blockman.ReadRawBlockFromDisk(block_data, pos)}; + const auto success{blockman.ReadRawBlockFromDisk(block_data, pos)}; assert(success); }); } +BENCHMARK(SaveBlockBench, benchmark::PriorityLevel::HIGH); BENCHMARK(ReadBlockFromDiskBench, benchmark::PriorityLevel::HIGH); BENCHMARK(ReadRawBlockFromDiskBench, benchmark::PriorityLevel::HIGH); From 42bc4914658d9834a653bd1763aa8f0d54355480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Wed, 18 Dec 2024 12:38:55 +0100 Subject: [PATCH 3/7] refactor,blocks: inline `UndoWriteToDisk` `UndoWriteToDisk` wasn't really extracting a meaningful subset of the `WriteUndoDataForBlock` functionality, it's tied closely to the only caller (needs the header size twice, recalculated undo serializes size, returns multiple branches, modifies parameter, needs documentation). The inlined code should only differ in these parts (modernization will be done in other commits): * renamed `_pos` to `pos` in `WriteUndoDataForBlock` to match the parameter name; * inlined `hashBlock` parameter usage into `hasher << block.pprev->GetBlockHash()`; * changed `return false` to `return FatalError`; * capitalize comment. Co-authored-by: Ryan Ofsky --- src/node/blockstorage.cpp | 62 +++++++++++++++++---------------------- src/node/blockstorage.h | 1 - 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 07878a56029..3ee6b5d2102 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -669,33 +669,6 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n) return &m_blockfile_info.at(n); } -bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const -{ - // Open history file to append - AutoFile fileout{OpenUndoFile(pos)}; - if (fileout.IsNull()) { - LogError("%s: OpenUndoFile failed\n", __func__); - return false; - } - - // Write index header - unsigned int nSize = GetSerializeSize(blockundo); - fileout << GetParams().MessageStart() << nSize; - - // Write undo data - long fileOutPos = fileout.tell(); - pos.nPos = (unsigned int)fileOutPos; - fileout << blockundo; - - // calculate & write checksum - HashWriter hasher{}; - hasher << hashBlock; - hasher << blockundo; - fileout << hasher.GetHash(); - - return true; -} - bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& index) const { const FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())}; @@ -992,33 +965,52 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid // Write undo information to disk if (block.GetUndoPos().IsNull()) { - FlatFilePos _pos; - if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo) + 40)) { + FlatFilePos pos; + if (!FindUndoPos(state, block.nFile, pos, ::GetSerializeSize(blockundo) + 40)) { LogError("%s: FindUndoPos failed\n", __func__); return false; } - if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) { + // Open history file to append + AutoFile fileout{OpenUndoFile(pos)}; + if (fileout.IsNull()) { + LogError("%s: OpenUndoFile failed\n", __func__); return FatalError(m_opts.notifications, state, _("Failed to write undo data.")); } + + // Write index header + unsigned int nSize = GetSerializeSize(blockundo); + fileout << GetParams().MessageStart() << nSize; + + // Write undo data + long fileOutPos = fileout.tell(); + pos.nPos = (unsigned int)fileOutPos; + fileout << blockundo; + + // Calculate & write checksum + HashWriter hasher{}; + hasher << block.pprev->GetBlockHash(); + hasher << blockundo; + fileout << hasher.GetHash(); + // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order) // we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height // in the block file info as below; note that this does not catch the case where the undo writes are keeping up // with the block writes (usually when a synced up node is getting newly mined blocks) -- this case is caught in // the FindNextBlockPos function - if (_pos.nFile < cursor.file_num && static_cast(block.nHeight) == m_blockfile_info[_pos.nFile].nHeightLast) { + if (pos.nFile < cursor.file_num && static_cast(block.nHeight) == m_blockfile_info[pos.nFile].nHeightLast) { // Do not propagate the return code, a failed flush here should not // be an indication for a failed write. If it were propagated here, // the caller would assume the undo data not to be written, when in // fact it is. Note though, that a failed flush might leave the data // file untrimmed. - if (!FlushUndoFile(_pos.nFile, true)) { - LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, "Failed to flush undo file %05i\n", _pos.nFile); + if (!FlushUndoFile(pos.nFile, true)) { + LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, "Failed to flush undo file %05i\n", pos.nFile); } - } else if (_pos.nFile == cursor.file_num && block.nHeight > cursor.undo_height) { + } else if (pos.nFile == cursor.file_num && block.nHeight > cursor.undo_height) { cursor.undo_height = block.nHeight; } // update nUndoPos in block index - block.nUndoPos = _pos.nPos; + block.nUndoPos = pos.nPos; block.nStatus |= BLOCK_HAVE_UNDO; m_dirty_blockindex.insert(&block); } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index ac6db0558df..339282227cb 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -176,7 +176,6 @@ private: * (BLOCK_SERIALIZATION_HEADER_SIZE) */ bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const; - bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const; /* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */ void FindFilesToPruneManual( From dfb2f9d004860c95fc6f0d4a016a9c038d53a475 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Wed, 18 Dec 2024 12:44:31 +0100 Subject: [PATCH 4/7] refactor,blocks: inline `WriteBlockToDisk` Similarly, `WriteBlockToDisk` wasn't really extracting a meaningful subset of the `SaveBlockToDisk` functionality, it's tied closely to the only caller (needs the header size twice, recalculated block serializes size, returns multiple branches, mutates parameter). The inlined code should only differ in these parts (modernization will be done in other commits): * renamed `blockPos` to `pos` in `SaveBlockToDisk` to match the parameter name; * changed `return false` to `return FlatFilePos()`. Also removed remaining references to `SaveBlockToDisk`. Co-authored-by: Ryan Ofsky --- src/node/blockstorage.cpp | 43 ++++++++++++++++----------------------- src/node/blockstorage.h | 15 +++----------- 2 files changed, 21 insertions(+), 37 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 3ee6b5d2102..93dd23a5c86 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -936,27 +936,6 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP return true; } -bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const -{ - // Open history file to append - AutoFile fileout{OpenBlockFile(pos)}; - if (fileout.IsNull()) { - LogError("%s: OpenBlockFile failed\n", __func__); - return false; - } - - // Write index header - unsigned int nSize = GetSerializeSize(TX_WITH_WITNESS(block)); - fileout << GetParams().MessageStart() << nSize; - - // Write block - long fileOutPos = fileout.tell(); - pos.nPos = (unsigned int)fileOutPos; - fileout << TX_WITH_WITNESS(block); - - return true; -} - bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex& block) { AssertLockHeld(::cs_main); @@ -1117,16 +1096,30 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight) // Account for the 4 magic message start bytes + the 4 length bytes (8 bytes total, // defined as BLOCK_SERIALIZATION_HEADER_SIZE) nBlockSize += static_cast(BLOCK_SERIALIZATION_HEADER_SIZE); - FlatFilePos blockPos{FindNextBlockPos(nBlockSize, nHeight, block.GetBlockTime())}; - if (blockPos.IsNull()) { + FlatFilePos pos{FindNextBlockPos(nBlockSize, nHeight, block.GetBlockTime())}; + if (pos.IsNull()) { LogError("%s: FindNextBlockPos failed\n", __func__); return FlatFilePos(); } - if (!WriteBlockToDisk(block, blockPos)) { + + // Open history file to append + AutoFile fileout{OpenBlockFile(pos)}; + if (fileout.IsNull()) { + LogError("%s: OpenBlockFile failed\n", __func__); m_opts.notifications.fatalError(_("Failed to write block.")); return FlatFilePos(); } - return blockPos; + + // Write index header + unsigned int nSize = GetSerializeSize(TX_WITH_WITNESS(block)); + fileout << GetParams().MessageStart() << nSize; + + // Write block + long fileOutPos = fileout.tell(); + pos.nPos = (unsigned int)fileOutPos; + fileout << TX_WITH_WITNESS(block); + + return pos; } static auto InitBlocksdirXorKey(const BlockManager::Options& opts) diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 339282227cb..545d65a8032 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -74,7 +74,7 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB /** The maximum size of a blk?????.dat file (since 0.8) */ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB -/** Size of header written by WriteBlockToDisk before a serialized CBlock */ +/** Size of header written by SaveBlockToDisk before a serialized CBlock */ static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = std::tuple_size_v + sizeof(unsigned int); // Because validation code takes pointers to the map's CBlockIndex objects, if @@ -161,7 +161,7 @@ private: * blockfile info, and checks if there is enough disk space to save the block. * * The nAddSize argument passed to this function should include not just the size of the serialized CBlock, but also the size of - * separator fields which are written before it by WriteBlockToDisk (BLOCK_SERIALIZATION_HEADER_SIZE). + * separator fields (BLOCK_SERIALIZATION_HEADER_SIZE). */ [[nodiscard]] FlatFilePos FindNextBlockPos(unsigned int nAddSize, unsigned int nHeight, uint64_t nTime); [[nodiscard]] bool FlushChainstateBlockFile(int tip_height); @@ -169,14 +169,6 @@ private: AutoFile OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false) const; - /** - * Write a block to disk. The pos argument passed to this function is modified by this call. Before this call, it should - * point to an unused file location where separator fields will be written, followed by the serialized CBlock data. - * After this call, it will point to the beginning of the serialized CBlock data, after the separator fields - * (BLOCK_SERIALIZATION_HEADER_SIZE) - */ - bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const; - /* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */ void FindFilesToPruneManual( std::set& setFilesToPrune, @@ -346,8 +338,7 @@ public: * * @param[in] block the block being processed * @param[in] nHeight the height of the block - * @param[in] pos the position of the serialized CBlock on disk. This is the position returned - * by WriteBlockToDisk pointing at the CBlock, not the separator fields before it + * @param[in] pos the position of the serialized CBlock on disk */ void UpdateBlockInfo(const CBlock& block, unsigned int nHeight, const FlatFilePos& pos); From fa39f27a0f8b8d14f6769d48f43999a3a1148e4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Wed, 18 Dec 2024 12:29:25 +0100 Subject: [PATCH 5/7] refactor,blocks: deduplicate block's serialized size calculations For consistency `UNDO_DATA_DISK_OVERHEAD` was also extracted to avoid the constant's ambiguity. Asserts were added to help with the review - they are removed in the next commit. Co-authored-by: Ryan Ofsky --- src/node/blockstorage.cpp | 32 +++++++++++++------------------- src/node/blockstorage.h | 7 +++++-- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 93dd23a5c86..b33b1c6258f 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -945,7 +945,9 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid // Write undo information to disk if (block.GetUndoPos().IsNull()) { FlatFilePos pos; - if (!FindUndoPos(state, block.nFile, pos, ::GetSerializeSize(blockundo) + 40)) { + const unsigned int blockundo_size{static_cast(GetSerializeSize(blockundo))}; + static_assert(UNDO_DATA_DISK_OVERHEAD == 40); // TODO remove + if (!FindUndoPos(state, block.nFile, pos, blockundo_size + UNDO_DATA_DISK_OVERHEAD)) { LogError("%s: FindUndoPos failed\n", __func__); return false; } @@ -957,12 +959,11 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid } // Write index header - unsigned int nSize = GetSerializeSize(blockundo); - fileout << GetParams().MessageStart() << nSize; - + assert(blockundo_size == GetSerializeSize(blockundo)); // TODO remove + fileout << GetParams().MessageStart() << blockundo_size; // Write undo data - long fileOutPos = fileout.tell(); - pos.nPos = (unsigned int)fileOutPos; + pos.nPos += BLOCK_SERIALIZATION_HEADER_SIZE; + assert(pos.nPos == fileout.tell()); // TODO remove fileout << blockundo; // Calculate & write checksum @@ -1092,17 +1093,12 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector& block, const FlatF FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight) { - unsigned int nBlockSize = ::GetSerializeSize(TX_WITH_WITNESS(block)); - // Account for the 4 magic message start bytes + the 4 length bytes (8 bytes total, - // defined as BLOCK_SERIALIZATION_HEADER_SIZE) - nBlockSize += static_cast(BLOCK_SERIALIZATION_HEADER_SIZE); - FlatFilePos pos{FindNextBlockPos(nBlockSize, nHeight, block.GetBlockTime())}; + const unsigned int block_size{static_cast(GetSerializeSize(TX_WITH_WITNESS(block)))}; + FlatFilePos pos{FindNextBlockPos(block_size + BLOCK_SERIALIZATION_HEADER_SIZE, nHeight, block.GetBlockTime())}; if (pos.IsNull()) { LogError("%s: FindNextBlockPos failed\n", __func__); return FlatFilePos(); } - - // Open history file to append AutoFile fileout{OpenBlockFile(pos)}; if (fileout.IsNull()) { LogError("%s: OpenBlockFile failed\n", __func__); @@ -1110,15 +1106,13 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight) return FlatFilePos(); } + assert(block_size == GetSerializeSize(TX_WITH_WITNESS(block))); // TODO remove // Write index header - unsigned int nSize = GetSerializeSize(TX_WITH_WITNESS(block)); - fileout << GetParams().MessageStart() << nSize; - + fileout << GetParams().MessageStart() << block_size; // Write block - long fileOutPos = fileout.tell(); - pos.nPos = (unsigned int)fileOutPos; + pos.nPos += BLOCK_SERIALIZATION_HEADER_SIZE; + assert(pos.nPos == fileout.tell()); // TODO remove fileout << TX_WITH_WITNESS(block); - return pos; } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 545d65a8032..e7fc7c26659 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -74,8 +74,11 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB /** The maximum size of a blk?????.dat file (since 0.8) */ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB -/** Size of header written by SaveBlockToDisk before a serialized CBlock */ -static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = std::tuple_size_v + sizeof(unsigned int); +/** Size of header written by SaveBlockToDisk before a serialized CBlock (8 bytes) */ +static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE{std::tuple_size_v + sizeof(unsigned int)}; + +/** Total overhead when writing undo data: header (8 bytes) plus checksum (32 bytes) */ +static constexpr size_t UNDO_DATA_DISK_OVERHEAD{BLOCK_SERIALIZATION_HEADER_SIZE + uint256::size()}; // Because validation code takes pointers to the map's CBlockIndex objects, if // we ever switch to another associative container, we need to either use a From baaa3b284671ba28dbbcbb43851ea46175fd2b13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Wed, 18 Dec 2024 12:57:25 +0100 Subject: [PATCH 6/7] refactor,blocks: remove costly asserts and modernize affected logs When the behavior was changes in a previous commit (caching `GetSerializeSize` and avoiding `AutoFile.tell`), (static)asserts were added to make sure the behavior was kept - to make sure reviewers and CI validates it. We can safely remove them now. Logs were also slightly modernized since they were trivial to do. Co-authored-by: Anthony Towns Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> --- src/node/blockstorage.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index b33b1c6258f..b31d77dad3d 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -676,7 +676,7 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in // Open history file to read AutoFile filein{OpenUndoFile(pos, true)}; if (filein.IsNull()) { - LogError("%s: OpenUndoFile failed for %s\n", __func__, pos.ToString()); + LogError("OpenUndoFile failed for %s", pos.ToString()); return false; } @@ -946,24 +946,21 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid if (block.GetUndoPos().IsNull()) { FlatFilePos pos; const unsigned int blockundo_size{static_cast(GetSerializeSize(blockundo))}; - static_assert(UNDO_DATA_DISK_OVERHEAD == 40); // TODO remove if (!FindUndoPos(state, block.nFile, pos, blockundo_size + UNDO_DATA_DISK_OVERHEAD)) { - LogError("%s: FindUndoPos failed\n", __func__); + LogError("FindUndoPos failed"); return false; } // Open history file to append AutoFile fileout{OpenUndoFile(pos)}; if (fileout.IsNull()) { - LogError("%s: OpenUndoFile failed\n", __func__); + LogError("OpenUndoFile failed"); return FatalError(m_opts.notifications, state, _("Failed to write undo data.")); } // Write index header - assert(blockundo_size == GetSerializeSize(blockundo)); // TODO remove fileout << GetParams().MessageStart() << blockundo_size; // Write undo data pos.nPos += BLOCK_SERIALIZATION_HEADER_SIZE; - assert(pos.nPos == fileout.tell()); // TODO remove fileout << blockundo; // Calculate & write checksum @@ -1096,22 +1093,20 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight) const unsigned int block_size{static_cast(GetSerializeSize(TX_WITH_WITNESS(block)))}; FlatFilePos pos{FindNextBlockPos(block_size + BLOCK_SERIALIZATION_HEADER_SIZE, nHeight, block.GetBlockTime())}; if (pos.IsNull()) { - LogError("%s: FindNextBlockPos failed\n", __func__); + LogError("FindNextBlockPos failed"); return FlatFilePos(); } AutoFile fileout{OpenBlockFile(pos)}; if (fileout.IsNull()) { - LogError("%s: OpenBlockFile failed\n", __func__); + LogError("OpenBlockFile failed"); m_opts.notifications.fatalError(_("Failed to write block.")); return FlatFilePos(); } - assert(block_size == GetSerializeSize(TX_WITH_WITNESS(block))); // TODO remove // Write index header fileout << GetParams().MessageStart() << block_size; // Write block pos.nPos += BLOCK_SERIALIZATION_HEADER_SIZE; - assert(pos.nPos == fileout.tell()); // TODO remove fileout << TX_WITH_WITNESS(block); return pos; } From 223081ece651dc616ff63d9ac447eedc5c2a28fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Thu, 9 Jan 2025 13:31:19 +0100 Subject: [PATCH 7/7] scripted-diff: rename block and undo functions for consistency Co-authored-by: Ryan Ofsky Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> -BEGIN VERIFY SCRIPT- grep -r -wE 'WriteBlock|ReadRawBlock|ReadBlock|WriteBlockUndo|ReadBlockUndo' $(git ls-files src/ ':!src/leveldb') && \ echo "Error: One or more target names already exist!" && exit 1 sed -i \ -e 's/\bSaveBlockToDisk/WriteBlock/g' \ -e 's/\bReadRawBlockFromDisk/ReadRawBlock/g' \ -e 's/\bReadBlockFromDisk/ReadBlock/g' \ -e 's/\bWriteUndoDataForBlock/WriteBlockUndo/g' \ -e 's/\bUndoReadFromDisk/ReadBlockUndo/g' \ $(git ls-files src/ ':!src/leveldb') -END VERIFY SCRIPT- --- src/bench/readwriteblock.cpp | 20 +++++++-------- src/index/base.cpp | 4 +-- src/index/blockfilterindex.cpp | 2 +- src/index/coinstatsindex.cpp | 6 ++--- src/init.cpp | 2 +- src/net_processing.cpp | 8 +++--- src/node/blockstorage.cpp | 14 +++++------ src/node/blockstorage.h | 14 +++++------ src/node/interfaces.cpp | 2 +- src/node/transaction.cpp | 2 +- src/rest.cpp | 2 +- src/rpc/blockchain.cpp | 8 +++--- src/rpc/rawtransaction.cpp | 4 +-- src/rpc/txoutproof.cpp | 2 +- src/test/blockmanager_tests.cpp | 18 ++++++------- src/test/util/blockfilter.cpp | 4 +-- src/test/validation_chainstate_tests.cpp | 2 +- src/validation.cpp | 32 ++++++++++++------------ 18 files changed, 73 insertions(+), 73 deletions(-) diff --git a/src/bench/readwriteblock.cpp b/src/bench/readwriteblock.cpp index e6d87c6ef05..cdf86185ae9 100644 --- a/src/bench/readwriteblock.cpp +++ b/src/bench/readwriteblock.cpp @@ -33,36 +33,36 @@ static void SaveBlockBench(benchmark::Bench& bench) auto& blockman{testing_setup->m_node.chainman->m_blockman}; const CBlock block{CreateTestBlock()}; bench.run([&] { - const auto pos{blockman.SaveBlockToDisk(block, 413'567)}; + const auto pos{blockman.WriteBlock(block, 413'567)}; assert(!pos.IsNull()); }); } -static void ReadBlockFromDiskBench(benchmark::Bench& bench) +static void ReadBlockBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; auto& blockman{testing_setup->m_node.chainman->m_blockman}; - const auto pos{blockman.SaveBlockToDisk(CreateTestBlock(), 413'567)}; + const auto pos{blockman.WriteBlock(CreateTestBlock(), 413'567)}; CBlock block; bench.run([&] { - const auto success{blockman.ReadBlockFromDisk(block, pos)}; + const auto success{blockman.ReadBlock(block, pos)}; assert(success); }); } -static void ReadRawBlockFromDiskBench(benchmark::Bench& bench) +static void ReadRawBlockBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; auto& blockman{testing_setup->m_node.chainman->m_blockman}; - const auto pos{blockman.SaveBlockToDisk(CreateTestBlock(), 413'567)}; + const auto pos{blockman.WriteBlock(CreateTestBlock(), 413'567)}; std::vector block_data; - blockman.ReadRawBlockFromDisk(block_data, pos); // warmup + blockman.ReadRawBlock(block_data, pos); // warmup bench.run([&] { - const auto success{blockman.ReadRawBlockFromDisk(block_data, pos)}; + const auto success{blockman.ReadRawBlock(block_data, pos)}; assert(success); }); } BENCHMARK(SaveBlockBench, benchmark::PriorityLevel::HIGH); -BENCHMARK(ReadBlockFromDiskBench, benchmark::PriorityLevel::HIGH); -BENCHMARK(ReadRawBlockFromDiskBench, benchmark::PriorityLevel::HIGH); +BENCHMARK(ReadBlockBench, benchmark::PriorityLevel::HIGH); +BENCHMARK(ReadRawBlockBench, benchmark::PriorityLevel::HIGH); diff --git a/src/index/base.cpp b/src/index/base.cpp index a8f9073d9f4..1169a1c86b7 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -188,7 +188,7 @@ void BaseIndex::Sync() CBlock block; interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex); - if (!m_chainstate->m_blockman.ReadBlockFromDisk(block, *pindex)) { + if (!m_chainstate->m_blockman.ReadBlock(block, *pindex)) { FatalErrorf("%s: Failed to read block %s from disk", __func__, pindex->GetBlockHash().ToString()); return; @@ -256,7 +256,7 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti // 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. - // In case we reorg beyond the pruned depth, ReadBlockFromDisk would + // In case we reorg beyond the pruned depth, ReadBlock would // throw and lead to a graceful shutdown SetBestBlockIndex(new_tip); if (!Commit()) { diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index a808cc90850..5ce85e1f844 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -256,7 +256,7 @@ bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block) // 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 (!m_chainstate->m_blockman.UndoReadFromDisk(block_undo, *pindex)) { + if (!m_chainstate->m_blockman.ReadBlockUndo(block_undo, *pindex)) { return false; } } diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index c950a18f3f8..b5869416b93 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -123,7 +123,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) // 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 (!m_chainstate->m_blockman.UndoReadFromDisk(block_undo, *pindex)) { + if (!m_chainstate->m_blockman.ReadBlockUndo(block_undo, *pindex)) { return false; } @@ -287,7 +287,7 @@ bool CoinStatsIndex::CustomRewind(const interfaces::BlockRef& current_tip, const do { CBlock block; - if (!m_chainstate->m_blockman.ReadBlockFromDisk(block, *iter_tip)) { + if (!m_chainstate->m_blockman.ReadBlock(block, *iter_tip)) { LogError("%s: Failed to read block %s from disk\n", __func__, iter_tip->GetBlockHash().ToString()); return false; @@ -415,7 +415,7 @@ bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex // Ignore genesis block if (pindex->nHeight > 0) { - if (!m_chainstate->m_blockman.UndoReadFromDisk(block_undo, *pindex)) { + if (!m_chainstate->m_blockman.ReadBlockUndo(block_undo, *pindex)) { return false; } diff --git a/src/init.cpp b/src/init.cpp index b5adcf64760..190e4f15bf5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1588,7 +1588,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) g_zmq_notification_interface = CZMQNotificationInterface::Create( [&chainman = node.chainman](std::vector& block, const CBlockIndex& index) { assert(chainman); - return chainman->m_blockman.ReadRawBlockFromDisk(block, WITH_LOCK(cs_main, return index.GetBlockPos())); + return chainman->m_blockman.ReadRawBlock(block, WITH_LOCK(cs_main, return index.GetBlockPos())); }); if (g_zmq_notification_interface) { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a19443c0f56..6a8d40dae2c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2268,7 +2268,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // Fast-path: in this case it is possible to serve the block directly from disk, // as the network format matches the format on disk std::vector block_data; - if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, block_pos)) { + if (!m_chainman.m_blockman.ReadRawBlock(block_data, block_pos)) { if (WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.IsBlockPruned(*pindex))) { LogDebug(BCLog::NET, "Block was pruned before it could be read, %s\n", pfrom.DisconnectMsg(fLogIPs)); } else { @@ -2282,7 +2282,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } else { // Send block from disk std::shared_ptr pblockRead = std::make_shared(); - if (!m_chainman.m_blockman.ReadBlockFromDisk(*pblockRead, block_pos)) { + if (!m_chainman.m_blockman.ReadBlock(*pblockRead, block_pos)) { if (WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.IsBlockPruned(*pindex))) { LogDebug(BCLog::NET, "Block was pruned before it could be read, %s\n", pfrom.DisconnectMsg(fLogIPs)); } else { @@ -4096,7 +4096,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (!block_pos.IsNull()) { CBlock block; - const bool ret{m_chainman.m_blockman.ReadBlockFromDisk(block, block_pos)}; + const bool ret{m_chainman.m_blockman.ReadBlock(block, block_pos)}; // If height is above MAX_BLOCKTXN_DEPTH then this block cannot get // pruned after we release cs_main above, so this read should never fail. assert(ret); @@ -5599,7 +5599,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) PushMessage(*pto, std::move(cached_cmpctblock_msg.value())); } else { CBlock block; - const bool ret{m_chainman.m_blockman.ReadBlockFromDisk(block, *pBestIndex)}; + const bool ret{m_chainman.m_blockman.ReadBlock(block, *pBestIndex)}; assert(ret); CBlockHeaderAndShortTxIDs cmpctblock{block, m_rng.rand64()}; MakeAndPushMessage(*pto, NetMsgType::CMPCTBLOCK, cmpctblock); diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index b31d77dad3d..57c82e5d5f7 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -669,7 +669,7 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n) return &m_blockfile_info.at(n); } -bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& index) const +bool BlockManager::ReadBlockUndo(CBlockUndo& blockundo, const CBlockIndex& index) const { const FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())}; @@ -936,7 +936,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP return true; } -bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex& block) +bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex& block) { AssertLockHeld(::cs_main); const BlockfileType type = BlockfileTypeForHeight(block.nHeight); @@ -995,7 +995,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid return true; } -bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) const +bool BlockManager::ReadBlock(CBlock& block, const FlatFilePos& pos) const { block.SetNull(); @@ -1029,11 +1029,11 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons return true; } -bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) const +bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const { const FlatFilePos block_pos{WITH_LOCK(cs_main, return index.GetBlockPos())}; - if (!ReadBlockFromDisk(block, block_pos)) { + if (!ReadBlock(block, block_pos)) { return false; } if (block.GetHash() != index.GetBlockHash()) { @@ -1043,7 +1043,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) co return true; } -bool BlockManager::ReadRawBlockFromDisk(std::vector& block, const FlatFilePos& pos) const +bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& pos) const { FlatFilePos hpos = pos; // If nPos is less than 8 the pos is null and we don't have the block data @@ -1088,7 +1088,7 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector& block, const FlatF return true; } -FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight) +FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight) { const unsigned int block_size{static_cast(GetSerializeSize(TX_WITH_WITNESS(block)))}; FlatFilePos pos{FindNextBlockPos(block_size + BLOCK_SERIALIZATION_HEADER_SIZE, nHeight, block.GetBlockTime())}; diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index e7fc7c26659..8a34efadfea 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -74,7 +74,7 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB /** The maximum size of a blk?????.dat file (since 0.8) */ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB -/** Size of header written by SaveBlockToDisk before a serialized CBlock (8 bytes) */ +/** Size of header written by WriteBlock before a serialized CBlock (8 bytes) */ static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE{std::tuple_size_v + sizeof(unsigned int)}; /** Total overhead when writing undo data: header (8 bytes) plus checksum (32 bytes) */ @@ -324,7 +324,7 @@ public: /** Get block file info entry for one block file */ CBlockFileInfo* GetBlockFileInfo(size_t n); - bool WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex& block) + bool WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex& block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Store block on disk and update block file statistics. @@ -335,7 +335,7 @@ public: * @returns in case of success, the position to which the block was written to * in case of an error, an empty FlatFilePos */ - FlatFilePos SaveBlockToDisk(const CBlock& block, int nHeight); + FlatFilePos WriteBlock(const CBlock& block, int nHeight); /** Update blockfile info while processing a block during reindex. The block must be available on disk. * @@ -414,11 +414,11 @@ public: void UnlinkPrunedFiles(const std::set& setFilesToPrune) const; /** Functions for disk access for blocks */ - bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) const; - bool ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) const; - bool ReadRawBlockFromDisk(std::vector& block, const FlatFilePos& pos) const; + bool ReadBlock(CBlock& block, const FlatFilePos& pos) const; + bool ReadBlock(CBlock& block, const CBlockIndex& index) const; + bool ReadRawBlock(std::vector& block, const FlatFilePos& pos) const; - bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& index) const; + bool ReadBlockUndo(CBlockUndo& blockundo, const CBlockIndex& index) const; void CleanupBlockRevFiles() const; }; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f3b8c6a072f..fe88a6dad11 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -442,7 +442,7 @@ bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLocknHeight] == index ? active[index->nHeight + 1] : nullptr, *block.m_next_block, lock, active, blockman); if (block.m_data) { REVERSE_LOCK(lock); - if (!blockman.ReadBlockFromDisk(*block.m_data, *index)) block.m_data->SetNull(); + if (!blockman.ReadBlock(*block.m_data, *index)) block.m_data->SetNull(); } block.found = true; return true; diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 0f45da45dbd..666597391e3 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -144,7 +144,7 @@ CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMe } if (block_index) { CBlock block; - if (blockman.ReadBlockFromDisk(block, *block_index)) { + if (blockman.ReadBlock(block, *block_index)) { for (const auto& tx : block.vtx) { if (tx->GetHash() == hash) { hashBlock = block_index->GetBlockHash(); diff --git a/src/rest.cpp b/src/rest.cpp index 3e4b8b37c6c..9214efcd388 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -319,7 +319,7 @@ static bool rest_block(const std::any& context, } std::vector block_data{}; - if (!chainman.m_blockman.ReadRawBlockFromDisk(block_data, pos)) { + if (!chainman.m_blockman.ReadRawBlock(block_data, pos)) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 823d2303c81..266113f7e30 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -196,7 +196,7 @@ UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIn CBlockUndo blockUndo; const bool is_not_pruned{WITH_LOCK(::cs_main, return !blockman.IsBlockPruned(blockindex))}; bool have_undo{is_not_pruned && WITH_LOCK(::cs_main, return blockindex.nStatus & BLOCK_HAVE_UNDO)}; - if (have_undo && !blockman.UndoReadFromDisk(blockUndo, blockindex)) { + if (have_undo && !blockman.ReadBlockUndo(blockUndo, blockindex)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Undo data expected but can't be read. This could be due to disk corruption or a conflict with a pruning event."); } for (size_t i = 0; i < block.vtx.size(); ++i) { @@ -624,7 +624,7 @@ static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex& blockin CheckBlockDataAvailability(blockman, blockindex, /*check_for_undo=*/false); } - if (!blockman.ReadBlockFromDisk(block, blockindex)) { + if (!blockman.ReadBlock(block, blockindex)) { // Block not found on disk. This shouldn't normally happen unless the block was // pruned right after we released the lock above. throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); @@ -643,7 +643,7 @@ static std::vector GetRawBlockChecked(BlockManager& blockman, const CBl pos = blockindex.GetBlockPos(); } - if (!blockman.ReadRawBlockFromDisk(data, pos)) { + if (!blockman.ReadRawBlock(data, pos)) { // Block not found on disk. This shouldn't normally happen unless the block was // pruned right after we released the lock above. throw JSONRPCError(RPC_MISC_ERROR, "Block not found on disk"); @@ -664,7 +664,7 @@ static CBlockUndo GetUndoChecked(BlockManager& blockman, const CBlockIndex& bloc CheckBlockDataAvailability(blockman, blockindex, /*check_for_undo=*/true); } - if (!blockman.UndoReadFromDisk(blockUndo, blockindex)) { + if (!blockman.ReadBlockUndo(blockUndo, blockindex)) { throw JSONRPCError(RPC_MISC_ERROR, "Can't read undo data from disk"); } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 184fff9386d..421656152cb 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -390,10 +390,10 @@ static RPCHelpMan getrawtransaction() TxToJSON(*tx, hash_block, result, chainman.ActiveChainstate()); return result; } - if (!chainman.m_blockman.UndoReadFromDisk(blockUndo, *blockindex)) { + if (!chainman.m_blockman.ReadBlockUndo(blockUndo, *blockindex)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Undo data expected but can't be read. This could be due to disk corruption or a conflict with a pruning event."); } - if (!chainman.m_blockman.ReadBlockFromDisk(block, *blockindex)) { + if (!chainman.m_blockman.ReadBlock(block, *blockindex)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Block data expected but can't be read. This could be due to disk corruption or a conflict with a pruning event."); } diff --git a/src/rpc/txoutproof.cpp b/src/rpc/txoutproof.cpp index 40294fda064..77fd22000c4 100644 --- a/src/rpc/txoutproof.cpp +++ b/src/rpc/txoutproof.cpp @@ -102,7 +102,7 @@ static RPCHelpMan gettxoutproof() CheckBlockDataAvailability(chainman.m_blockman, *pblockindex, /*check_for_undo=*/false); } CBlock block; - if (!chainman.m_blockman.ReadBlockFromDisk(block, *pblockindex)) { + if (!chainman.m_blockman.ReadBlock(block, *pblockindex)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk"); } diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index c2b95dd861e..5540e28fb77 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -36,7 +36,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) }; BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; // simulate adding a genesis block normally - BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); + BOOST_CHECK_EQUAL(blockman.WriteBlock(params->GenesisBlock(), 0).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); // simulate what happens during reindex // simulate a well-formed genesis block being found at offset 8 in the blk00000.dat file // the block is found at offset 8 because there is an 8 byte serialization header @@ -49,7 +49,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) // this is a check to make sure that https://github.com/bitcoin/bitcoin/issues/21379 does not recur // 8 bytes (for serialization header) + 285 (for serialized genesis block) = 293 // add another 8 bytes for the second block's serialization header and we get 293 + 8 = 301 - FlatFilePos actual{blockman.SaveBlockToDisk(params->GenesisBlock(), 1)}; + FlatFilePos actual{blockman.WriteBlock(params->GenesisBlock(), 1)}; BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(TX_WITH_WITNESS(params->GenesisBlock())) + BLOCK_SERIALIZATION_HEADER_SIZE); } @@ -158,10 +158,10 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), 0); // Write the first block to a new location. - FlatFilePos pos1{blockman.SaveBlockToDisk(block1, /*nHeight=*/1)}; + FlatFilePos pos1{blockman.WriteBlock(block1, /*nHeight=*/1)}; // Write second block - FlatFilePos pos2{blockman.SaveBlockToDisk(block2, /*nHeight=*/2)}; + FlatFilePos pos2{blockman.WriteBlock(block2, /*nHeight=*/2)}; // Two blocks in the file BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); @@ -171,13 +171,13 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) CBlock read_block; BOOST_CHECK_EQUAL(read_block.nVersion, 0); { - ASSERT_DEBUG_LOG("ReadBlockFromDisk: Errors in block header"); - BOOST_CHECK(!blockman.ReadBlockFromDisk(read_block, pos1)); + ASSERT_DEBUG_LOG("ReadBlock: Errors in block header"); + BOOST_CHECK(!blockman.ReadBlock(read_block, pos1)); BOOST_CHECK_EQUAL(read_block.nVersion, 1); } { - ASSERT_DEBUG_LOG("ReadBlockFromDisk: Errors in block header"); - BOOST_CHECK(!blockman.ReadBlockFromDisk(read_block, pos2)); + ASSERT_DEBUG_LOG("ReadBlock: Errors in block header"); + BOOST_CHECK(!blockman.ReadBlock(read_block, pos2)); BOOST_CHECK_EQUAL(read_block.nVersion, 2); } @@ -194,7 +194,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); // Block 2 was not overwritten: - blockman.ReadBlockFromDisk(read_block, pos2); + blockman.ReadBlock(read_block, pos2); BOOST_CHECK_EQUAL(read_block.nVersion, 2); } diff --git a/src/test/util/blockfilter.cpp b/src/test/util/blockfilter.cpp index 19f3d51d5e6..8a17f7f4f51 100644 --- a/src/test/util/blockfilter.cpp +++ b/src/test/util/blockfilter.cpp @@ -17,12 +17,12 @@ bool ComputeFilter(BlockFilterType filter_type, const CBlockIndex& block_index, LOCK(::cs_main); CBlock block; - if (!blockman.ReadBlockFromDisk(block, block_index.GetBlockPos())) { + if (!blockman.ReadBlock(block, block_index.GetBlockPos())) { return false; } CBlockUndo block_undo; - if (block_index.nHeight > 0 && !blockman.UndoReadFromDisk(block_undo, block_index)) { + if (block_index.nHeight > 0 && !blockman.ReadBlockUndo(block_undo, block_index)) { return false; } diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index bf8cd819f28..7204f1688dd 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -88,7 +88,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) std::shared_ptr pblockone = std::make_shared(); { LOCK(::cs_main); - chainman.m_blockman.ReadBlockFromDisk(*pblockone, *chainman.ActiveChain()[1]); + chainman.m_blockman.ReadBlock(*pblockone, *chainman.ActiveChain()[1]); } BOOST_REQUIRE(CreateAndActivateUTXOSnapshot( diff --git a/src/validation.cpp b/src/validation.cpp index 3e8a7cf520e..64588e802d7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2301,7 +2301,7 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn bool fClean = true; CBlockUndo blockUndo; - if (!m_blockman.UndoReadFromDisk(blockUndo, *pindex)) { + if (!m_blockman.ReadBlockUndo(blockUndo, *pindex)) { LogError("DisconnectBlock(): failure reading undo data\n"); return DISCONNECT_FAILED; } @@ -2747,7 +2747,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, return true; } - if (!m_blockman.WriteUndoDataForBlock(blockundo, state, *pindex)) { + if (!m_blockman.WriteBlockUndo(blockundo, state, *pindex)) { return false; } @@ -3068,7 +3068,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra // Read block from disk. std::shared_ptr pblock = std::make_shared(); CBlock& block = *pblock; - if (!m_blockman.ReadBlockFromDisk(block, *pindexDelete)) { + if (!m_blockman.ReadBlock(block, *pindexDelete)) { LogError("DisconnectTip(): Failed to read block\n"); return false; } @@ -3179,7 +3179,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, std::shared_ptr pthisBlock; if (!pblock) { std::shared_ptr pblockNew = std::make_shared(); - if (!m_blockman.ReadBlockFromDisk(*pblockNew, *pindexNew)) { + if (!m_blockman.ReadBlock(*pblockNew, *pindexNew)) { return FatalError(m_chainman.GetNotifications(), state, _("Failed to read block.")); } pthisBlock = pblockNew; @@ -4564,7 +4564,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr& pblock, blockPos = *dbp; m_blockman.UpdateBlockInfo(block, pindex->nHeight, blockPos); } else { - blockPos = m_blockman.SaveBlockToDisk(block, pindex->nHeight); + blockPos = m_blockman.WriteBlock(block, pindex->nHeight); if (blockPos.IsNull()) { state.Error(strprintf("%s: Failed to find position to write new block to disk", __func__)); return false; @@ -4797,8 +4797,8 @@ VerifyDBResult CVerifyDB::VerifyDB( } CBlock block; // check level 0: read from disk - if (!chainstate.m_blockman.ReadBlockFromDisk(block, *pindex)) { - LogPrintf("Verification error: ReadBlockFromDisk failed at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); + if (!chainstate.m_blockman.ReadBlock(block, *pindex)) { + LogPrintf("Verification error: ReadBlock failed at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); return VerifyDBResult::CORRUPTED_BLOCK_DB; } // check level 1: verify block validity @@ -4811,7 +4811,7 @@ VerifyDBResult CVerifyDB::VerifyDB( if (nCheckLevel >= 2 && pindex) { CBlockUndo undo; if (!pindex->GetUndoPos().IsNull()) { - if (!chainstate.m_blockman.UndoReadFromDisk(undo, *pindex)) { + if (!chainstate.m_blockman.ReadBlockUndo(undo, *pindex)) { LogPrintf("Verification error: found bad undo data at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); return VerifyDBResult::CORRUPTED_BLOCK_DB; } @@ -4863,8 +4863,8 @@ VerifyDBResult CVerifyDB::VerifyDB( m_notifications.progress(_("Verifying blocks…"), percentageDone, false); pindex = chainstate.m_chain.Next(pindex); CBlock block; - if (!chainstate.m_blockman.ReadBlockFromDisk(block, *pindex)) { - LogPrintf("Verification error: ReadBlockFromDisk failed at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); + if (!chainstate.m_blockman.ReadBlock(block, *pindex)) { + LogPrintf("Verification error: ReadBlock failed at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); return VerifyDBResult::CORRUPTED_BLOCK_DB; } if (!chainstate.ConnectBlock(block, state, pindex, coins)) { @@ -4892,8 +4892,8 @@ bool Chainstate::RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& in AssertLockHeld(cs_main); // TODO: merge with ConnectBlock CBlock block; - if (!m_blockman.ReadBlockFromDisk(block, *pindex)) { - LogError("ReplayBlock(): ReadBlockFromDisk failed at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); + if (!m_blockman.ReadBlock(block, *pindex)) { + LogError("ReplayBlock(): ReadBlock failed at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); return false; } @@ -4950,8 +4950,8 @@ bool Chainstate::ReplayBlocks() while (pindexOld != pindexFork) { if (pindexOld->nHeight > 0) { // Never disconnect the genesis block. CBlock block; - if (!m_blockman.ReadBlockFromDisk(block, *pindexOld)) { - LogError("RollbackBlock(): ReadBlockFromDisk() failed at %d, hash=%s\n", pindexOld->nHeight, pindexOld->GetBlockHash().ToString()); + if (!m_blockman.ReadBlock(block, *pindexOld)) { + LogError("RollbackBlock(): ReadBlock() failed at %d, hash=%s\n", pindexOld->nHeight, pindexOld->GetBlockHash().ToString()); return false; } LogPrintf("Rolling back %s (%i)\n", pindexOld->GetBlockHash().ToString(), pindexOld->nHeight); @@ -5062,7 +5062,7 @@ bool Chainstate::LoadGenesisBlock() try { const CBlock& block = params.GenesisBlock(); - FlatFilePos blockPos{m_blockman.SaveBlockToDisk(block, 0)}; + FlatFilePos blockPos{m_blockman.WriteBlock(block, 0)}; if (blockPos.IsNull()) { LogError("%s: writing genesis block to disk failed\n", __func__); return false; @@ -5219,7 +5219,7 @@ void ChainstateManager::LoadExternalBlockFile( while (range.first != range.second) { std::multimap::iterator it = range.first; std::shared_ptr pblockrecursive = std::make_shared(); - if (m_blockman.ReadBlockFromDisk(*pblockrecursive, it->second)) { + if (m_blockman.ReadBlock(*pblockrecursive, it->second)) { LogDebug(BCLog::REINDEX, "%s: Processing out of order child %s of %s\n", __func__, pblockrecursive->GetHash().ToString(), head.ToString()); LOCK(cs_main);