Repository: bitcoin/bitcoin
File: src/node/blockstorage.cpp (C++)

Code before:
    if (GetConsensus().signet_blocks && !CheckSignetBlockSolution(block, GetConsensus())) {
        LogError("Errors in block solution at %s while reading block", pos.ToString());
        return false;
    }

    if (expected_hash && block_hash != *expected_hash) {
        LogError("GetHash() doesn't match index at %s while reading block (%s != %s)",
                 pos.ToString(), block_hash.ToString(), expected_hash->ToString());
        return false;
    }

    return true;
}

bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const
{
    const FlatFilePos block_pos{WITH_LOCK(cs_main, return index.GetBlockPos())};
    return ReadBlock(block, block_pos, index.GetBlockHash());
}

bool BlockManager::ReadRawBlock(std::vector<std::byte>& block, const FlatFilePos& pos) const
{
    return ReadRawBlockPart(block, pos, std::nullopt, std::nullopt);
}

bool BlockManager::ReadRawBlockPart(std::vector<std::byte>& data, const FlatFilePos& pos, std::optional<size_t> part_offset, std::optional<size_t> part_size) const
{
    if (pos.nPos < STORAGE_HEADER_BYTES) {
        // If nPos is less than STORAGE_HEADER_BYTES, we can't read the header that precedes the block data
        // This would cause an unsigned integer underflow when trying to position the file cursor
        // This can happen after pruning or default constructed positions
        LogError("Failed for %s while reading raw block storage header", pos.ToString());
        return false;
    }
    AutoFile filein{OpenBlockFile({pos.nFile, pos.nPos - STORAGE_HEADER_BYTES}, /*fReadOnly=*/true)};
    if (filein.IsNull()) {
        LogError("OpenBlockFile failed for %s while reading raw block", pos.ToString());
        return false;
    }

    try {
        MessageStartChars blk_start;
        unsigned int blk_size;

        filein >> blk_start >> blk_size;

        if (blk_start != GetParams().MessageStart()) {
            LogError("Block magic mismatch for %s: %s versus expected %s while reading raw block",
                pos.ToString(), HexStr(blk_start), HexStr(GetParams().MessageStart()));
            return false;
        }

Review comment (bug):
No need to have `std::optional` for the offset, could just be sent in as `0` when reading whole blocks. Same for parameter types in rest.cpp.
```suggestion
bool BlockManager::ReadRawBlockPart(std::vector<std::byte>& data, const FlatFilePos& pos, size_t part_offset, std::optional<size_t> part_size) const
```

<details><summary>Full compiling diff (includes suggestion to skip noop seeks, see other comment).</summary>

```diff
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 47beac674c..7062735067 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -1046,10 +1046,10 @@ bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const
 
 bool BlockManager::ReadRawBlock(std::vector<std::byte>& block, const FlatFilePos& pos) const
 {
-    return ReadRawBlockPart(block, pos, std::nullopt, std::nullopt);
+    return ReadRawBlockPart(block, pos, 0, std::nullopt);
 }
 
-bool BlockManager::ReadRawBlockPart(std::vector<std::byte>& data, const FlatFilePos& pos, std::optional<size_t> part_offset, std::optional<size_t> part_size) const
+bool BlockManager::ReadRawBlockPart(std::vector<std::byte>& data, const FlatFilePos& pos, size_t part_offset, std::optional<size_t> part_size) const
 {
     if (pos.nPos < STORAGE_HEADER_BYTES) {
         // If nPos is less than STORAGE_HEADER_BYTES, we can't read the header that precedes the block data
@@ -1082,16 +1082,12 @@ bool BlockManager::ReadRawBlockPart(std::vector<std::byte>& data, const FlatFile
             return false;
         }
 
-        size_t offset = 0;
-        if (part_offset.has_value()) {
-            if (*part_offset > blk_size) {
-                LogError("Read from block file failed: invalid offset: %u", *part_offset);
-                return false;
-            }
-            offset = *part_offset;
+        if (part_offset > blk_size) {
+            LogError("Read from block file failed: invalid offset: %u", part_offset);
+            return false;
         }
 
-        size_t size = blk_size - offset;
+        size_t size = blk_size - part_offset;
         if (part_size.has_value()) {
             if (*part_size > size || *part_size == 0) {
                 LogError("Read from block file failed: invalid size: %u", *part_size);
@@ -1102,7 +1098,7 @@ bool BlockManager::ReadRawBlockPart(std::vector<std::byte>& data, const FlatFile
 
 
         data.resize(size); // Zeroing of memory is intentional here
-        filein.seek(offset, SEEK_CUR);
+        if (part_offset > 0) filein.seek(part_offset, SEEK_CUR);
         filein.read(data);
     } catch (const std::exception& e) {
         LogError("Read from block file failed: %s for %s while reading raw block", e.what(), pos.ToString());
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index 61128942b3..9f33d826a1 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -414,7 +414,7 @@ public:
     bool ReadBlock(CBlock& block, const FlatFilePos& pos, const std::optional<uint256>& expected_hash) const;
     bool ReadBlock(CBlock& block, const CBlockIndex& index) const;
     bool ReadRawBlock(std::vector<std::byte>& block, const FlatFilePos& pos) const;
-    bool ReadRawBlockPart(std::vector<std::byte>& data, const FlatFilePos& pos, std::optional<size_t> part_offset, std::optional<size_t> part_size) const;
+    bool ReadRawBlockPart(std::vector<std::byte>& data, const FlatFilePos& pos, size_t part_offset, std::optional<size_t> part_size) const;
 
     bool ReadBlockUndo(CBlockUndo& blockundo, const CBlockIndex& index) const;
 
diff --git a/src/rest.cpp b/src/rest.cpp
index f3edc69d09..294337a24b 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -399,7 +399,7 @@ static bool rest_block(const std::any& context,
                        HTTPRequest* req,
                        const std::string& uri_part,
                        std::optional<TxVerbosity> tx_verbosity,
-                       std::optional<size_t> part_offset,
+                       size_t part_offset,
                        std::optional<size_t> part_size)
 {
     if (!CheckWarmup(req))
@@ -475,23 +475,25 @@ static bool rest_block(const std::any& context,
 
 static bool rest_block_extended(const std::any& context, HTTPRequest* req, const std::string& uri_part)
 {
-    return rest_block(context, req, uri_part, TxVerbosity::SHOW_DETAILS_AND_PREVOUT, /*part_offset=*/std::nullopt, /*part_size=*/std::nullopt);
+    return rest_block(context, req, uri_part, TxVerbosity::SHOW_DETAILS_AND_PREVOUT, /*part_offset=*/0, /*part_size=*/std::nullopt);
 }
 
 static bool rest_block_notxdetails(const std::any& context, HTTPRequest* req, const std::string& uri_part)
 {
-    return rest_block(context, req, uri_part, TxVerbosity::SHOW_TXID, /*part_offset=*/std::nullopt, /*part_size=*/std::nullopt);
+    return rest_block(context, req, uri_part, TxVerbosity::SHOW_TXID, /*part_offset=*/0, /*part_size=*/std::nullopt);
 }
 
 static bool rest_block_part(const std::any& context, HTTPRequest* req, const std::string& uri_part)
 {
-    std::optional<size_t> part_offset{};
+    size_t part_offset{0};
     std::optional<size_t> part_size{};
     try {
         const auto part_offset_str = req->GetQueryParameter("offset");
         if (part_offset_str.has_value()) {
-            part_offset = ToIntegral<size_t>(*part_offset_str);
-            if (!part_offset.has_value()) {
+            auto opt = ToIntegral<size_t>(*part_offset_str);
+            if (opt.has_value()) {
+                part_offset = opt.value();
+            } else {
                 return RESTERR(req, HTTP_BAD_REQUEST, strprintf("Block part offset is invalid: %s", *part_offset_str));
             }
         }
```

</details>

Code after:
        return false;
    }

    // Signet only: check block solution
    if (GetConsensus().signet_blocks && !CheckSignetBlockSolution(block, GetConsensus())) {
        LogError("Errors in block solution at %s while reading block", pos.ToString());
        return false;
    }

    if (expected_hash && block_hash != *expected_hash) {
        LogError("GetHash() doesn't match index at %s while reading block (%s != %s)",
                 pos.ToString(), block_hash.ToString(), expected_hash->ToString());
        return false;
    }

    return true;
}

bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const
{
    const FlatFilePos block_pos{WITH_LOCK(cs_main, return index.GetBlockPos())};
    return ReadBlock(block, block_pos, index.GetBlockHash());
}

BlockManager::ReadRawBlockResult BlockManager::ReadRawBlock(const FlatFilePos& pos, std::optional<std::pair<size_t, size_t>> block_part) const
{
    if (pos.nPos < STORAGE_HEADER_BYTES) {
        // If nPos is less than STORAGE_HEADER_BYTES, we can't read the header that precedes the block data
        // This would cause an unsigned integer underflow when trying to position the file cursor
        // This can happen after pruning or default constructed positions
        LogError("Failed for %s while reading raw block storage header", pos.ToString());
        return util::Unexpected{ReadRawError::IO};
    }
    AutoFile filein{OpenBlockFile({pos.nFile, pos.nPos - STORAGE_HEADER_BYTES}, /*fReadOnly=*/true)};
    if (filein.IsNull()) {
        LogError("OpenBlockFile failed for %s while reading raw block", pos.ToString());
        return util::Unexpected{ReadRawError::IO};
    }

    try {
        MessageStartChars blk_start;
        unsigned int blk_size;

        filein >> blk_start >> blk_size;

        if (blk_start != GetParams().MessageStart()) {
            LogError("Block magic mismatch for %s: %s versus expected %s while reading raw block",
                pos.ToString(), HexStr(blk_start), HexStr(GetParams().MessageStart()));
            return util::Unexpected{ReadRawError::IO};
        }