highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-08-23 16:42:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

refactor: clear up blockstorage/streams in preparation for optimization

Browse Source 
Made every OpenBlockFile#fReadOnly value explicit.

Replaced hard-coded values in ReadRawBlock with STORAGE_HEADER_BYTES.
Changed `STORAGE_HEADER_BYTES` and `UNDO_DATA_DISK_OVERHEAD` to `uint32_t` to avoid casts.

Also added `LIFETIMEBOUND` to the `AutoFile` parameter of `BufferedFile`, which stores a reference to the underlying `AutoFile`, allowing Clang to emit warnings if the referenced `AutoFile` might be destroyed while `BufferedFile` still exists.
Without this attribute, code with lifetime violations wouldn't trigger compiler warnings.

Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
This commit is contained in:
Lőrinc
2025-03-26 12:40:20 +01:00
parent 67fcc64802
commit 056cb3c0d2
3 changed files with 15 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/node/blockstorage.cpp
View File

@@ -529,7 +529,7 @@ bool BlockManager::LoadBlockIndexDB(const std::optional<uint256>& snapshot_block
}
for (std::set<int>::iterator it = setBlkDataFiles.begin(); it != setBlkDataFiles.end(); it++) {
FlatFilePos pos(*it, 0);
if (OpenBlockFile(pos, true).IsNull()) {
if (OpenBlockFile(pos, /*fReadOnly=*/true).IsNull()) {
return false;
}
}
@@ -933,7 +933,7 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt
// Write undo information to disk
if (block.GetUndoPos().IsNull()) {
FlatFilePos pos;
const unsigned int blockundo_size{static_cast<unsigned int>(GetSerializeSize(blockundo))};
const auto blockundo_size{static_cast<uint32_t>(GetSerializeSize(blockundo))};
if (!FindUndoPos(state, block.nFile, pos, blockundo_size + UNDO_DATA_DISK_OVERHEAD)) {
LogError("FindUndoPos failed for %s while writing block undo", pos.ToString());
return false;
@@ -996,7 +996,7 @@ bool BlockManager::ReadBlock(CBlock& block, const FlatFilePos& pos) const
block.SetNull();
// Open history file to read
AutoFile filein{OpenBlockFile(pos, true)};
AutoFile filein{OpenBlockFile(pos, /*fReadOnly=*/true)};
if (filein.IsNull()) {
LogError("OpenBlockFile failed for %s while reading block", pos.ToString());
return false;
@@ -1041,15 +1041,14 @@ bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const
bool BlockManager::ReadRawBlock(std::vector<uint8_t>& 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
// Return early to prevent undefined behavior of unsigned int underflow
if (hpos.nPos < 8) {
LogError("Failed for %s while reading raw block", pos.ToString());
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;
}
hpos.nPos -= 8; // Seek back 8 bytes for meta header
AutoFile filein{OpenBlockFile(hpos, true)};
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;
@@ -1091,7 +1090,7 @@ FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight)
LogError("FindNextBlockPos failed for %s while writing block", pos.ToString());
return FlatFilePos();
}
AutoFile fileout{OpenBlockFile(pos)};
AutoFile fileout{OpenBlockFile(pos, /*fReadOnly=*/false)};
if (fileout.IsNull()) {
LogError("OpenBlockFile failed for %s while writing block", pos.ToString());
m_opts.notifications.fatalError(_("Failed to write block."));
@@ -1210,7 +1209,7 @@ void ImportBlocks(ChainstateManager& chainman, std::span<const fs::path> import_
if (!fs::exists(chainman.m_blockman.GetBlockPosFilename(pos))) {
break; // No block files left to reindex
}
AutoFile file{chainman.m_blockman.OpenBlockFile(pos, true)};
AutoFile file{chainman.m_blockman.OpenBlockFile(pos, /*fReadOnly=*/true)};
if (file.IsNull()) {
break; // This error is logged in OpenBlockFile
}

6
src/node/blockstorage.h
View File

@@ -75,10 +75,10 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB
static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB
/** Size of header written by WriteBlock before a serialized CBlock (8 bytes) */
static constexpr size_t STORAGE_HEADER_BYTES{std::tuple_size_v<MessageStartChars> + sizeof(unsigned int)};
static constexpr uint32_t STORAGE_HEADER_BYTES{std::tuple_size_v<MessageStartChars> + sizeof(unsigned int)};
/** Total overhead when writing undo data: header (8 bytes) plus checksum (32 bytes) */
static constexpr size_t UNDO_DATA_DISK_OVERHEAD{STORAGE_HEADER_BYTES + uint256::size()};
static constexpr uint32_t UNDO_DATA_DISK_OVERHEAD{STORAGE_HEADER_BYTES + 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
@@ -400,7 +400,7 @@ public:
void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
/** Open a block file (blk?????.dat) */
AutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false) const;
AutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly) const;
/** Translation to a filesystem path */
fs::path GetBlockPosFilename(const FlatFilePos& pos) const;

2
src/streams.h
View File

@@ -523,7 +523,7 @@ private:
}
public:
BufferedFile(AutoFile& file, uint64_t nBufSize, uint64_t nRewindIn)
BufferedFile(AutoFile& file LIFETIMEBOUND, uint64_t nBufSize, uint64_t nRewindIn)
: m_src{file}, nReadLimit{std::numeric_limits<uint64_t>::max()}, nRewind{nRewindIn}, vchBuf(nBufSize, std::byte{0})
{
if (nRewindIn >= nBufSize)