blockstorage: return an error code from `ReadRawBlock()`
What changed, and why it matters
This commit refactors how Bitcoin Core reads raw block files from disk. Previously, the ReadRawBlock() function returned true/false and filled a caller-provided buffer. Now it returns a structured result that distinguishes I/O errors from success. The main observable security-relevant change is in the REST API: when a block cannot be read from disk due to an I/O problem, it now returns HTTP 500 (Internal Server Error) instead of HTTP 404 (Not Found). This is a correctness improvement that reduces the chance an attacker can trick the node into revealing whether a pruned block existed, but it is not a fix for an active vulnerability.
No urgent action required. Treat as a normal code-quality/refactor commit. Reviewers may want to confirm that all ReadRawBlock() callers handle the new util::Expected return correctly and that the REST 500 response is acceptable for existing API clients.
Security signals we found
REST endpoint error-code distinction between 'not found' and 'I/O error'
Potential minor information-disclosure reduction (404 vs 500) for pruned/disk-failure cases
Refactoring only; no change to block validation, deserialization, or network consensus rules
Evidence from the diff
ReadRawBlock() in src/node/blockstorage.cpp is changed to return util::Expected
Changed components
src/node/blockstorage.cpp/h ReadRawBlock() APIsrc/rest.cpp rest_block() HTTP response handlingsrc/net_processing.cpp ProcessGetBlockData() raw-block servingsrc/init.cpp ZMQ raw-block callbacksrc/rpc/blockchain.cpp GetRawBlockChecked()src/bench/readwriteblock.cpp ReadRawBlockBenchInspect captured patch +41 / −36
diff --git a/src/bench/readwriteblock.cpp b/src/bench/readwriteblock.cpp
index 4ca5b8eb..a742a1e8 100644
--- a/src/bench/readwriteblock.cpp
+++ b/src/bench/readwriteblock.cpp
@@ -57,11 +57,9 @@ static void ReadRawBlockBench(benchmark::Bench& bench)
const auto testing_setup{MakeNoLogFileContext<const TestingSetup>(ChainType::MAIN)};
auto& blockman{testing_setup->m_node.chainman->m_blockman};
const auto pos{blockman.WriteBlock(CreateTestBlock(), 413'567)};
- std::vector<std::byte> block_data;
- blockman.ReadRawBlock(block_data, pos); // warmup
bench.run([&] {
- const auto success{blockman.ReadRawBlock(block_data, pos)};
- assert(success);
+ const auto res{blockman.ReadRawBlock(pos)};
+ assert(res);
});
}
diff --git a/src/init.cpp b/src/init.cpp
index b35c6b72..1a0cd79f 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1759,7 +1759,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
g_zmq_notification_interface = CZMQNotificationInterface::Create(
[&chainman = node.chainman](std::vector<std::byte>& block, const CBlockIndex& index) {
assert(chainman);
- return chainman->m_blockman.ReadRawBlock(block, WITH_LOCK(cs_main, return index.GetBlockPos()));
+ if (auto ret{chainman->m_blockman.ReadRawBlock(WITH_LOCK(cs_main, return index.GetBlockPos()))}) {
+ block = std::move(*ret);
+ return true;
+ }
+ return false;
});
if (g_zmq_notification_interface) {
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index c8bc4a84..00acebd1 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2276,8 +2276,9 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
} else if (inv.IsMsgWitnessBlk()) {
// 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<std::byte> block_data;
- if (!m_chainman.m_blockman.ReadRawBlock(block_data, block_pos)) {
+ if (const auto block_data{m_chainman.m_blockman.ReadRawBlock(block_pos)}) {
+ MakeAndPushMessage(pfrom, NetMsgType::BLOCK, std::span{*block_data});
+ } else {
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 {
@@ -2286,7 +2287,6 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
pfrom.fDisconnect = true;
return;
}
- MakeAndPushMessage(pfrom, NetMsgType::BLOCK, std::span{block_data});
// Don't set pblock as we've sent the block
} else {
// Send block from disk
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index d2a4c620..0927eb85 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -1006,14 +1006,14 @@ bool BlockManager::ReadBlock(CBlock& block, const FlatFilePos& pos, const std::o
block.SetNull();
// Open history file to read
- std::vector<std::byte> block_data;
- if (!ReadRawBlock(block_data, pos)) {
+ const auto block_data{ReadRawBlock(pos)};
+ if (!block_data) {
return false;
}
try {
// Read block
- SpanReader{block_data} >> TX_WITH_WITNESS(block);
+ SpanReader{*block_data} >> TX_WITH_WITNESS(block);
} catch (const std::exception& e) {
LogError("Deserialize or I/O error - %s at %s while reading block", e.what(), pos.ToString());
return false;
@@ -1048,19 +1048,19 @@ bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const
return ReadBlock(block, block_pos, index.GetBlockHash());
}
-bool BlockManager::ReadRawBlock(std::vector<std::byte>& block, const FlatFilePos& pos) const
+BlockManager::ReadRawBlockResult BlockManager::ReadRawBlock(const FlatFilePos& pos) 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;
+ 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 false;
+ return util::Unexpected{ReadRawError::IO};
}
try {
@@ -1072,23 +1072,22 @@ bool BlockManager::ReadRawBlock(std::vector<std::byte>& block, const FlatFilePos
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;
+ return util::Unexpected{ReadRawError::IO};
}
if (blk_size > MAX_SIZE) {
LogError("Block data is larger than maximum deserialization size for %s: %s versus %s while reading raw block",
pos.ToString(), blk_size, MAX_SIZE);
- return false;
+ return util::Unexpected{ReadRawError::IO};
}
- block.resize(blk_size); // Zeroing of memory is intentional here
- filein.read(block);
+ std::vector<std::byte> data(blk_size); // Zeroing of memory is intentional here
+ filein.read(data);
+ return data;
} catch (const std::exception& e) {
LogError("Read from block file failed: %s for %s while reading raw block", e.what(), pos.ToString());
- return false;
+ return util::Unexpected{ReadRawError::IO};
}
-
- return true;
}
FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight)
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index 9ba7873c..1ce7e8f6 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -17,6 +17,7 @@
#include <streams.h>
#include <sync.h>
#include <uint256.h>
+#include <util/expected.h>
#include <util/fs.h>
#include <util/hasher.h>
@@ -169,6 +170,9 @@ struct BlockfileCursor {
std::ostream& operator<<(std::ostream& os, const BlockfileCursor& cursor);
+enum class ReadRawError {
+ IO,
+};
/**
* Maintains a tree of blocks (stored in `m_block_index`) which is consulted
@@ -302,6 +306,7 @@ private:
public:
using Options = kernel::BlockManagerOpts;
+ using ReadRawBlockResult = util::Expected<std::vector<std::byte>, ReadRawError>;
explicit BlockManager(const util::SignalInterrupt& interrupt, Options opts);
@@ -455,7 +460,7 @@ public:
/** Functions for disk access for blocks */
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;
+ ReadRawBlockResult ReadRawBlock(const FlatFilePos& pos) const;
bool ReadBlockUndo(CBlockUndo& blockundo, const CBlockIndex& index) const;
diff --git a/src/rest.cpp b/src/rest.cpp
index 40a07ba1..4883e26d 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -416,20 +416,23 @@ static bool rest_block(const std::any& context,
pos = pblockindex->GetBlockPos();
}
- std::vector<std::byte> block_data{};
- if (!chainman.m_blockman.ReadRawBlock(block_data, pos)) {
- return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found");
+ const auto block_data{chainman.m_blockman.ReadRawBlock(pos)};
+ if (!block_data) {
+ switch (block_data.error()) {
+ case node::ReadRawError::IO: return RESTERR(req, HTTP_INTERNAL_SERVER_ERROR, "I/O error reading " + hashStr);
+ }
+ assert(false);
}
switch (rf) {
case RESTResponseFormat::BINARY: {
req->WriteHeader("Content-Type", "application/octet-stream");
- req->WriteReply(HTTP_OK, block_data);
+ req->WriteReply(HTTP_OK, *block_data);
return true;
}
case RESTResponseFormat::HEX: {
- const std::string strHex{HexStr(block_data) + "\n"};
+ const std::string strHex{HexStr(*block_data) + "\n"};
req->WriteHeader("Content-Type", "text/plain");
req->WriteReply(HTTP_OK, strHex);
return true;
@@ -437,7 +440,7 @@ static bool rest_block(const std::any& context,
case RESTResponseFormat::JSON: {
CBlock block{};
- DataStream block_stream{block_data};
+ DataStream block_stream{*block_data};
block_stream >> TX_WITH_WITNESS(block);
UniValue objBlock = blockToJSON(chainman.m_blockman, block, *tip, *pblockindex, tx_verbosity, chainman.GetConsensus().powLimit);
std::string strJSON = objBlock.write() + "\n";
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 8f283ff4..c68bfb8c 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -680,7 +680,6 @@ static CBlock GetBlockChecked(BlockManager& blockman, const CBlockIndex& blockin
static std::vector<std::byte> GetRawBlockChecked(BlockManager& blockman, const CBlockIndex& blockindex)
{
- std::vector<std::byte> data{};
FlatFilePos pos{};
{
LOCK(cs_main);
@@ -688,13 +687,10 @@ static std::vector<std::byte> GetRawBlockChecked(BlockManager& blockman, const C
pos = blockindex.GetBlockPos();
}
- 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");
- }
-
- return data;
+ if (auto data{blockman.ReadRawBlock(pos)}) return std::move(*data);
+ // 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");
}
static CBlockUndo GetUndoChecked(BlockManager& blockman, const CBlockIndex& blockindex)
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.