From aaaa3323f37526862ebf2a2a4bf522c661e6976e Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 11 Jul 2024 10:50:28 +0200 Subject: [PATCH 1/3] refactor: Mark IsBlockPruned const Member fields are used read-only in this method. --- src/node/blockstorage.cpp | 2 +- src/node/blockstorage.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index fb62e781387..16d8d256150 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -588,7 +588,7 @@ const CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data) return nullptr; } -bool BlockManager::IsBlockPruned(const CBlockIndex& block) +bool BlockManager::IsBlockPruned(const CBlockIndex& block) const { AssertLockHeld(::cs_main); return m_have_pruned && !(block.nStatus & BLOCK_HAVE_DATA) && (block.nTx > 0); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 108a08a72b8..b4e199af507 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -381,7 +381,7 @@ public: bool m_have_pruned = false; //! Check whether the block associated with this index entry is pruned or not. - bool IsBlockPruned(const CBlockIndex& block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool IsBlockPruned(const CBlockIndex& block) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! Create or update a prune lock identified by its name void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); From fad59a2f0f37f5b7f6076fd91be43448e35f4b7e Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 11 Jul 2024 16:20:06 +0200 Subject: [PATCH 2/3] log: LogError with FlatFilePos in UndoReadFromDisk These errors should never happen in normal operation. If they do, knowing the FlatFilePos may be useful to determine if data corruption happened. Also, handle the error pos.IsNull() as part of OpenUndoFile, because it may as well have happened due to data corruption. This mirrors the LogError behavior from ReadBlockFromDisk. --- src/node/blockstorage.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 16d8d256150..fe5762e2c5f 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -703,15 +703,10 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in { const FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())}; - if (pos.IsNull()) { - LogError("%s: no undo data available\n", __func__); - return false; - } - // Open history file to read AutoFile filein{OpenUndoFile(pos, true)}; if (filein.IsNull()) { - LogError("%s: OpenUndoFile failed\n", __func__); + LogError("%s: OpenUndoFile failed for %s\n", __func__, pos.ToString()); return false; } @@ -723,13 +718,13 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in verifier >> blockundo; filein >> hashChecksum; } catch (const std::exception& e) { - LogError("%s: Deserialize or I/O error - %s\n", __func__, e.what()); + LogError("%s: Deserialize or I/O error - %s at %s\n", __func__, e.what(), pos.ToString()); return false; } // Verify checksum if (hashChecksum != verifier.GetHash()) { - LogError("%s: Checksum mismatch\n", __func__); + LogError("%s: Checksum mismatch at %s\n", __func__, pos.ToString()); return false; } From fa14e1d9d5c5dc44396a01583ae94480b7bc29ee Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 11 Jul 2024 16:41:42 +0200 Subject: [PATCH 3/3] log: Fix __func__ in LogError in blockstorage module These errors should never happen. However, when they do happen, it is useful to log the correct error location (function name). For example, this fixes an incorrect "ConnectBlock()" in "WriteUndoDataForBlock". --- src/node/blockstorage.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index fe5762e2c5f..43801eb1d4c 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -981,7 +981,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const // Open history file to append AutoFile fileout{OpenBlockFile(pos)}; if (fileout.IsNull()) { - LogError("WriteBlockToDisk: OpenBlockFile failed\n"); + LogError("%s: OpenBlockFile failed\n", __func__); return false; } @@ -992,7 +992,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const // Write block long fileOutPos = ftell(fileout.Get()); if (fileOutPos < 0) { - LogError("WriteBlockToDisk: ftell failed\n"); + LogError("%s: ftell failed\n", __func__); return false; } pos.nPos = (unsigned int)fileOutPos; @@ -1011,7 +1011,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid if (block.GetUndoPos().IsNull()) { FlatFilePos _pos; if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo) + 40)) { - LogError("ConnectBlock(): FindUndoPos failed\n"); + LogError("%s: FindUndoPos failed\n", __func__); return false; } if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) { @@ -1050,7 +1050,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons // Open history file to read AutoFile filein{OpenBlockFile(pos, true)}; if (filein.IsNull()) { - LogError("ReadBlockFromDisk: OpenBlockFile failed for %s\n", pos.ToString()); + LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); return false; } @@ -1064,13 +1064,13 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons // Check the header if (!CheckProofOfWork(block.GetHash(), block.nBits, GetConsensus())) { - LogError("ReadBlockFromDisk: Errors in block header at %s\n", pos.ToString()); + LogError("%s: Errors in block header at %s\n", __func__, pos.ToString()); return false; } // Signet only: check block solution if (GetConsensus().signet_blocks && !CheckSignetBlockSolution(block, GetConsensus())) { - LogError("ReadBlockFromDisk: Errors in block solution at %s\n", pos.ToString()); + LogError("%s: Errors in block solution at %s\n", __func__, pos.ToString()); return false; } @@ -1085,8 +1085,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) co return false; } if (block.GetHash() != index.GetBlockHash()) { - LogError("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s\n", - index.ToString(), block_pos.ToString()); + LogError("%s: GetHash() doesn't match index for %s at %s\n", __func__, index.ToString(), block_pos.ToString()); return false; } return true;