diff --git a/src/bench/readwriteblock.cpp b/src/bench/readwriteblock.cpp index cdf86185ae9..10434b15d20 100644 --- a/src/bench/readwriteblock.cpp +++ b/src/bench/readwriteblock.cpp @@ -27,7 +27,7 @@ static CBlock CreateTestBlock() return block; } -static void SaveBlockBench(benchmark::Bench& bench) +static void WriteBlockBench(benchmark::Bench& bench) { const auto testing_setup{MakeNoLogFileContext(ChainType::MAIN)}; auto& blockman{testing_setup->m_node.chainman->m_blockman}; @@ -63,6 +63,6 @@ static void ReadRawBlockBench(benchmark::Bench& bench) }); } -BENCHMARK(SaveBlockBench, benchmark::PriorityLevel::HIGH); +BENCHMARK(WriteBlockBench, benchmark::PriorityLevel::HIGH); BENCHMARK(ReadBlockBench, benchmark::PriorityLevel::HIGH); BENCHMARK(ReadRawBlockBench, benchmark::PriorityLevel::HIGH); diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index fe44aa8a3f2..a061dbea11a 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -657,30 +657,45 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n) bool BlockManager::ReadBlockUndo(CBlockUndo& blockundo, const CBlockIndex& index) const { - const FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())}; + FlatFilePos pos{WITH_LOCK(::cs_main, return index.GetUndoPos())}; + if (pos.nPos < HEADER_BYTE_SIZE) { + LogError("%s: OpenUndoFile failed for %s while reading", __func__, pos.ToString()); + return false; + } + uint32_t undo_size; + pos.nPos -= sizeof(undo_size); // Open history file to read AutoFile filein{OpenUndoFile(pos, true)}; if (filein.IsNull()) { - LogError("OpenUndoFile failed for %s", pos.ToString()); + LogError("OpenUndoFile failed for %s while reading", pos.ToString()); return false; } - // Read block - uint256 hashChecksum; - HashVerifier verifier{filein}; // Use HashVerifier as reserializing may lose data, c.f. commit d342424301013ec47dc146a4beb49d5c9319d80a try { + // Read block + filein >> undo_size; + if (undo_size > MAX_SIZE) { + LogError("Refusing to read undo data of size: %d", undo_size); + return false; + } + + BufferedFileR buff(filein, undo_size); + HashVerifier verifier{buff}; // Use HashVerifier, as reserializing may lose data, c.f. commit d3424243 + verifier << index.pprev->GetBlockHash(); verifier >> blockundo; - filein >> hashChecksum; - } catch (const std::exception& e) { - 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 at %s\n", __func__, pos.ToString()); + uint256 hashChecksum; + filein >> hashChecksum; + + // Verify checksum + if (hashChecksum != verifier.GetHash()) { + LogError("%s: Checksum mismatch at %s", __func__, pos.ToString()); + return false; + } + } catch (const std::exception& e) { + LogError("%s: Deserialize or I/O error - %s at %s", __func__, e.what(), pos.ToString()); return false; } @@ -933,27 +948,26 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt FlatFilePos pos; const unsigned int blockundo_size{static_cast(GetSerializeSize(blockundo))}; if (!FindUndoPos(state, block.nFile, pos, blockundo_size + UNDO_DATA_DISK_OVERHEAD)) { - LogError("FindUndoPos failed"); + LogError("FindUndoPos failed for %s while writing", pos.ToString()); return false; } // Open history file to append AutoFile fileout{OpenUndoFile(pos)}; if (fileout.IsNull()) { - LogError("OpenUndoFile failed"); + LogError("OpenUndoFile failed for %s while writing", pos.ToString()); return FatalError(m_opts.notifications, state, _("Failed to write undo data.")); } // Write index header - fileout << GetParams().MessageStart() << blockundo_size; - // Write undo data - pos.nPos += BLOCK_SERIALIZATION_HEADER_SIZE; - fileout << blockundo; - - // Calculate & write checksum - HashWriter hasher{}; - hasher << block.pprev->GetBlockHash(); - hasher << blockundo; - fileout << hasher.GetHash(); + BufferedFileW(fileout, HEADER_BYTE_SIZE) << GetParams().MessageStart() << blockundo_size; + pos.nPos += HEADER_BYTE_SIZE; + { + // Calculate checksum + HashWriter hasher{}; + hasher << block.pprev->GetBlockHash() << blockundo; + // Write undo data & checksum + BufferedFileW(fileout, blockundo_size + sizeof(uint256)) << blockundo << hasher.GetHash(); + } // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order) // we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height @@ -981,34 +995,47 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt return true; } -bool BlockManager::ReadBlock(CBlock& block, const FlatFilePos& pos) const +bool BlockManager::ReadBlock(CBlock& block, FlatFilePos pos) const { block.SetNull(); + if (pos.nPos < HEADER_BYTE_SIZE) { + LogError("%s: OpenBlockFile failed for %s", __func__, pos.ToString()); + return false; + } + uint32_t blk_size; + pos.nPos -= sizeof(blk_size); + // Open history file to read AutoFile filein{OpenBlockFile(pos, true)}; if (filein.IsNull()) { - LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); + LogError("%s: OpenBlockFile failed for %s", __func__, pos.ToString()); return false; } - // Read block try { - filein >> TX_WITH_WITNESS(block); + // Read block + filein >> blk_size; + if (blk_size > MAX_SIZE) { + LogError("Refusing to read block of size: %d", blk_size); + return false; + } + + BufferedFileR(filein, blk_size) >> TX_WITH_WITNESS(block); } catch (const std::exception& e) { - LogError("%s: Deserialize or I/O error - %s at %s\n", __func__, e.what(), pos.ToString()); + LogError("%s: Deserialize or I/O error - %s at %s", __func__, e.what(), pos.ToString()); return false; } // Check the header if (!CheckProofOfWork(block.GetHash(), block.nBits, GetConsensus())) { - LogError("%s: Errors in block header at %s\n", __func__, pos.ToString()); + LogError("%s: Errors in block header at %s", __func__, pos.ToString()); return false; } // Signet only: check block solution if (GetConsensus().signet_blocks && !CheckSignetBlockSolution(block, GetConsensus())) { - LogError("%s: Errors in block solution at %s\n", __func__, pos.ToString()); + LogError("%s: Errors in block solution at %s", __func__, pos.ToString()); return false; } @@ -1023,7 +1050,7 @@ bool BlockManager::ReadBlock(CBlock& block, const CBlockIndex& index) const return false; } if (block.GetHash() != index.GetBlockHash()) { - LogError("%s: GetHash() doesn't match index for %s at %s\n", __func__, index.ToString(), block_pos.ToString()); + LogError("%s: GetHash() doesn't match index for %s at %s", __func__, index.ToString(), block_pos.ToString()); return false; } return true; @@ -1035,13 +1062,13 @@ bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& // 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("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); + LogError("%s: OpenBlockFile failed for %s", __func__, pos.ToString()); return false; } hpos.nPos -= 8; // Seek back 8 bytes for meta header AutoFile filein{OpenBlockFile(hpos, true)}; if (filein.IsNull()) { - LogError("%s: OpenBlockFile failed for %s\n", __func__, pos.ToString()); + LogError("%s: OpenBlockFile failed for %s", __func__, pos.ToString()); return false; } @@ -1052,14 +1079,14 @@ bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& filein >> blk_start >> blk_size; if (blk_start != GetParams().MessageStart()) { - LogError("%s: Block magic mismatch for %s: %s versus expected %s\n", __func__, pos.ToString(), + LogError("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(), HexStr(blk_start), HexStr(GetParams().MessageStart())); return false; } if (blk_size > MAX_SIZE) { - LogError("%s: Block data is larger than maximum deserialization size for %s: %s versus %s\n", __func__, pos.ToString(), + LogError("%s: Block data is larger than maximum deserialization size for %s: %s versus %s", __func__, pos.ToString(), blk_size, MAX_SIZE); return false; } @@ -1067,7 +1094,7 @@ bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& block.resize(blk_size); // Zeroing of memory is intentional here filein.read(MakeWritableByteSpan(block)); } catch (const std::exception& e) { - LogError("%s: Read from block file failed: %s for %s\n", __func__, e.what(), pos.ToString()); + LogError("%s: Read from block file failed: %s for %s", __func__, e.what(), pos.ToString()); return false; } @@ -1077,23 +1104,23 @@ bool BlockManager::ReadRawBlock(std::vector& block, const FlatFilePos& FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight) { const unsigned int block_size{static_cast(GetSerializeSize(TX_WITH_WITNESS(block)))}; - FlatFilePos pos{FindNextBlockPos(block_size + BLOCK_SERIALIZATION_HEADER_SIZE, nHeight, block.GetBlockTime())}; + FlatFilePos pos{FindNextBlockPos(block_size + HEADER_BYTE_SIZE, nHeight, block.GetBlockTime())}; if (pos.IsNull()) { - LogError("FindNextBlockPos failed"); + LogError("FindNextBlockPos failed for %s while writing", pos.ToString()); return FlatFilePos(); } AutoFile fileout{OpenBlockFile(pos)}; if (fileout.IsNull()) { - LogError("OpenBlockFile failed"); + LogError("OpenBlockFile failed for %s while writing", pos.ToString()); m_opts.notifications.fatalError(_("Failed to write block.")); return FlatFilePos(); } // Write index header - fileout << GetParams().MessageStart() << block_size; + BufferedFileW(fileout, HEADER_BYTE_SIZE) << GetParams().MessageStart() << block_size; + pos.nPos += HEADER_BYTE_SIZE; // Write block - pos.nPos += BLOCK_SERIALIZATION_HEADER_SIZE; - fileout << TX_WITH_WITNESS(block); + BufferedFileW(fileout, block_size) << TX_WITH_WITNESS(block); return pos; } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 665c2ccd834..a519abc23c2 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -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 BLOCK_SERIALIZATION_HEADER_SIZE{std::tuple_size_v + sizeof(unsigned int)}; +static constexpr size_t HEADER_BYTE_SIZE{std::tuple_size_v + sizeof(unsigned int)}; /** Total overhead when writing undo data: header (8 bytes) plus checksum (32 bytes) */ -static constexpr size_t UNDO_DATA_DISK_OVERHEAD{BLOCK_SERIALIZATION_HEADER_SIZE + uint256::size()}; +static constexpr size_t UNDO_DATA_DISK_OVERHEAD{HEADER_BYTE_SIZE + 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 @@ -164,7 +164,7 @@ private: * blockfile info, and checks if there is enough disk space to save the block. * * The nAddSize argument passed to this function should include not just the size of the serialized CBlock, but also the size of - * separator fields (BLOCK_SERIALIZATION_HEADER_SIZE). + * separator fields (HEADER_BYTE_SIZE). */ [[nodiscard]] FlatFilePos FindNextBlockPos(unsigned int nAddSize, unsigned int nHeight, uint64_t nTime); [[nodiscard]] bool FlushChainstateBlockFile(int tip_height); @@ -411,7 +411,7 @@ public: void UnlinkPrunedFiles(const std::set& setFilesToPrune) const; /** Functions for disk access for blocks */ - bool ReadBlock(CBlock& block, const FlatFilePos& pos) const; + bool ReadBlock(CBlock& block, FlatFilePos pos) const; bool ReadBlock(CBlock& block, const CBlockIndex& index) const; bool ReadRawBlock(std::vector& block, const FlatFilePos& pos) const; diff --git a/src/streams.cpp b/src/streams.cpp index cd496ee2be4..3fc58e260c9 100644 --- a/src/streams.cpp +++ b/src/streams.cpp @@ -102,6 +102,16 @@ void AutoFile::write(Span src) } } +void AutoFile::write_large(Span src) +{ + if (!m_file) throw std::ios_base::failure("AutoFile::write_large: file handle is nullptr"); + util::Xor(src, m_xor, *m_position); // obfuscate in-place + if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) { + throw std::ios_base::failure("AutoFile::write_large: write failed"); + } + if (m_position) *m_position += src.size(); +} + bool AutoFile::Commit() { return ::FileCommit(m_file); diff --git a/src/streams.h b/src/streams.h index e5316da7e70..9dbbb2ad0c9 100644 --- a/src/streams.h +++ b/src/streams.h @@ -23,6 +23,7 @@ #include #include #include +#include namespace util { inline void Xor(Span write, Span key, size_t key_offset = 0) @@ -162,6 +163,7 @@ public: typedef vector_type::reverse_iterator reverse_iterator; explicit DataStream() = default; + explicit DataStream(size_type n) { reserve(n); } explicit DataStream(Span sp) : DataStream{AsBytes(sp)} {} explicit DataStream(Span sp) : vch(sp.data(), sp.data() + sp.size()) {} @@ -451,6 +453,7 @@ public: void read(Span dst); void ignore(size_t nSize); void write(Span src); + void write_large(Span src); // Note that src will be mutated template AutoFile& operator<<(const T& obj) @@ -467,6 +470,54 @@ public: } }; +class BufferedFileW +{ + AutoFile& m_file; + uint32_t m_buffer_size; + DataStream m_buf; + +public: + explicit BufferedFileW(AutoFile& file, const uint32_t buffer_size) + : m_file(file), m_buffer_size{buffer_size}, m_buf{buffer_size} {} + + ~BufferedFileW() + { + Assert(m_buf.size() <= m_buffer_size); + m_file.write_large(m_buf); + } + + void write(Span src) { m_buf.write(src); } + + template + BufferedFileW& operator<<(const T& obj) + { + Serialize(m_buf, obj); + return *this; + } +}; + +class BufferedFileR +{ + DataStream m_buf; + +public: + explicit BufferedFileR(AutoFile& file, const uint32_t buffer_size) + { + m_buf.resize(buffer_size); + file.read(m_buf); + Assert(m_buf.size() == buffer_size); + } + + void read(Span dst) { m_buf.read(dst); } + + template + BufferedFileR& operator>>(T&& obj) + { + Unserialize(m_buf, obj); + return *this; + } +}; + /** Wrapper around an AutoFile& that implements a ring buffer to * deserialize from. It guarantees the ability to rewind a given number of bytes. * diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 8f8cce687f5..7ec5c4073aa 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -17,7 +17,7 @@ #include #include -using node::BLOCK_SERIALIZATION_HEADER_SIZE; +using node::HEADER_BYTE_SIZE; using node::BlockManager; using node::KernelNotifications; using node::MAX_BLOCKFILE_SIZE; @@ -40,12 +40,12 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) }; BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; // simulate adding a genesis block normally - BOOST_CHECK_EQUAL(blockman.WriteBlock(params->GenesisBlock(), 0).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); + BOOST_CHECK_EQUAL(blockman.WriteBlock(params->GenesisBlock(), 0).nPos, HEADER_BYTE_SIZE); // simulate what happens during reindex // simulate a well-formed genesis block being found at offset 8 in the blk00000.dat file // the block is found at offset 8 because there is an 8 byte serialization header // consisting of 4 magic bytes + 4 length bytes before each block in a well-formed blk file. - const FlatFilePos pos{0, BLOCK_SERIALIZATION_HEADER_SIZE}; + const FlatFilePos pos{0, HEADER_BYTE_SIZE}; blockman.UpdateBlockInfo(params->GenesisBlock(), 0, pos); // now simulate what happens after reindex for the first new block processed // the actual block contents don't matter, just that it's a block. @@ -54,7 +54,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) // 8 bytes (for serialization header) + 285 (for serialized genesis block) = 293 // add another 8 bytes for the second block's serialization header and we get 293 + 8 = 301 FlatFilePos actual{blockman.WriteBlock(params->GenesisBlock(), 1)}; - BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(TX_WITH_WITNESS(params->GenesisBlock())) + BLOCK_SERIALIZATION_HEADER_SIZE); + BOOST_CHECK_EQUAL(actual.nPos, HEADER_BYTE_SIZE + ::GetSerializeSize(TX_WITH_WITNESS(params->GenesisBlock())) + HEADER_BYTE_SIZE); } BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain100Setup) @@ -172,7 +172,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) FlatFilePos pos2{blockman.WriteBlock(block2, /*nHeight=*/2)}; // Two blocks in the file - BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); + BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + HEADER_BYTE_SIZE) * 2); // First two blocks are written as expected // Errors are expected because block data is junk, thrown AFTER successful read @@ -199,7 +199,7 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) // Metadata is updated... BOOST_CHECK_EQUAL(block_data->nBlocks, 3); // ...but there are still only two blocks in the file - BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + BLOCK_SERIALIZATION_HEADER_SIZE) * 2); + BOOST_CHECK_EQUAL(blockman.CalculateCurrentUsage(), (TEST_BLOCK_SIZE + HEADER_BYTE_SIZE) * 2); // Block 2 was not overwritten: blockman.ReadBlock(read_block, pos2); diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 777122df6d0..c72f77325f7 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -567,4 +567,31 @@ BOOST_AUTO_TEST_CASE(streams_hashed) BOOST_CHECK_EQUAL(hash_writer.GetHash(), hash_verifier.GetHash()); } +BOOST_AUTO_TEST_CASE(streams_datastream_write_large) +{ + const uint32_t v1{m_rng.rand32()}, v2{m_rng.rand32()}, v3{m_rng.rand32()}; + const fs::path tmp_path{m_args.GetDataDirBase() / "test_datastream_write_large.bin"}; + + // Write out the values through a precisely sized BufferedFileW + { + AutoFile file{fsbridge::fopen(tmp_path, "w+b")}; + BufferedFileW f(file, sizeof(v1) + sizeof(v2) + sizeof(v3)); + f << v1 << v2; + f.write(AsBytes(Span{&v3, 1})); + } + // Read back and verify using BufferedFileR + { + AutoFile file{fsbridge::fopen(tmp_path, "rb")}; + uint32_t _v1{0}, _v2{0}, _v3{0}; + BufferedFileR f(file, sizeof(v1) + sizeof(v2) + sizeof(v3)); + f >> _v1 >> _v2; + f.read(AsWritableBytes(Span{&_v3, 1})); + BOOST_CHECK_EQUAL(_v1, v1); + BOOST_CHECK_EQUAL(_v2, v2); + BOOST_CHECK_EQUAL(_v3, v3); + } + + fs::remove(tmp_path); +} + BOOST_AUTO_TEST_SUITE_END()