refactor: return std::optional<uint256> from ReadBlockFromDisk(FlatFilePos)

Replace the uint256* hash_out output parameter with a std::optional
return value. The optional is empty on failure, contains the block hash
on success. This makes the hash lifetime explicit and eliminates the
risk of dangling pointers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: UdjinM6

src/node/blockstorage.cpp

@@ -745,42 +745,44 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid
     return true;
 }
 
-bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos, const Consensus::Params& consensusParams, uint256* hash_out)
+std::optional<uint256> ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos, const Consensus::Params& consensusParams)
 {
     block.SetNull();
 
     // Open history file to read
     CAutoFile filein(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION);
     if (filein.IsNull()) {
-        return error("ReadBlockFromDisk: OpenBlockFile failed for %s", pos.ToString());
+        error("ReadBlockFromDisk: OpenBlockFile failed for %s", pos.ToString());
+        return std::nullopt;
     }
 
     // Read block
     try {
         filein >> block;
     } catch (const std::exception& e) {
-        return error("%s: Deserialize or I/O error - %s at %s", __func__, e.what(), pos.ToString());
+        error("%s: Deserialize or I/O error - %s at %s", __func__, e.what(), pos.ToString());
+        return std::nullopt;
     }
 
     // Check the header
     const uint256 hash{block.GetHash()};
     if (!CheckProofOfWork(hash, block.nBits, consensusParams)) {
-        return error("ReadBlockFromDisk: Errors in block header at %s", pos.ToString());
+        error("ReadBlockFromDisk: Errors in block header at %s", pos.ToString());
+        return std::nullopt;
     }
-    if (hash_out) *hash_out = hash;
 
-    return true;
+    return hash;
 }
 
 bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams)
 {
     const FlatFilePos block_pos{WITH_LOCK(cs_main, return pindex->GetBlockPos())};
 
-    uint256 hash;
-    if (!ReadBlockFromDisk(block, block_pos, consensusParams, &hash)) {
+    const auto hash{ReadBlockFromDisk(block, block_pos, consensusParams)};
+    if (!hash) {
         return false;
     }
-    if (hash != pindex->GetBlockHash()) {
+    if (*hash != pindex->GetBlockHash()) {
         return error("ReadBlockFromDisk(CBlock&, CBlockIndex*): GetHash() doesn't match index for %s at %s",
                      pindex->ToString(), block_pos.ToString());
     }

src/node/blockstorage.h

@@ -13,6 +13,7 @@
 #include <txdb.h>
 
 #include <cstdint>
+#include <optional>
 #include <unordered_map>
 #include <vector>
 
@@ -221,7 +222,7 @@ fs::path GetBlockPosFilename(const FlatFilePos& pos);
 void UnlinkPrunedFiles(const std::set<int>& setFilesToPrune);
 
 /** Functions for disk access for blocks */
-bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos, const Consensus::Params& consensusParams, uint256* hash_out = nullptr);
+std::optional<uint256> ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos, const Consensus::Params& consensusParams);
 bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams);
 
 bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex);

src/validation.cpp

@@ -5185,8 +5185,8 @@ void CChainState::LoadExternalBlockFile(
                     while (range.first != range.second) {
                         std::multimap<uint256, FlatFilePos>::iterator it = range.first;
                         std::shared_ptr<CBlock> pblockrecursive = std::make_shared<CBlock>();
-                        uint256 blockhash;
-                        if (ReadBlockFromDisk(*pblockrecursive, it->second, m_params.GetConsensus(), &blockhash)) {
+                        if (auto opt_hash{ReadBlockFromDisk(*pblockrecursive, it->second, m_params.GetConsensus())}) {
+                            const uint256& blockhash = *opt_hash;
                             LogPrint(BCLog::REINDEX, "%s: Processing out of order child %s of %s\n", __func__, blockhash.ToString(),
                                     head.ToString());
                             LOCK(cs_main);