Merge bitcoin/bitcoin#28904: Drop CAutoFile

4eb2a9e streams: Drop unused CAutoFile (Anthony Towns)
cde9a4b refactor: switch from CAutoFile to AutoFile (Anthony Towns)
bbd4646 blockstorage: switch from CAutoFile to AutoFile (Anthony Towns)
c72ddf0 streams: Remove unused CAutoFile::GetVersion (Anthony Towns)
e63f643 streams: Base BufferedFile on AutoFile instead of CAutoFile (Anthony Towns)

Pull request description:

  Continuing the move away from `GetVersion()`, replace uses of `CAutoFile` with `AutoFile`.

ACKs for top commit:
  maflcko:
    re-ACK 4eb2a9e 🖼
  TheCharlatan:
    ACK 4eb2a9e
  stickies-v:
    ACK 4eb2a9e

Tree-SHA512: 1a68c42fdb725ca4bf573e22794fe7809fea764a5f97ecb33435add3c609d40f336038fb22ab1ea72567530efd39678278c9016f92ed04891afdb310631b4e82

Author: fanquake

src/bench/load_external.cpp

@@ -55,7 +55,7 @@ static void LoadExternalBlockFile(benchmark::Bench& bench)
     bench.run([&] {
         // "rb" is "binary, O_RDONLY", positioned to the start of the file.
         // The file will be closed by LoadExternalBlockFile().
-        CAutoFile file{fsbridge::fopen(blkfile, "rb"), CLIENT_VERSION};
+        AutoFile file{fsbridge::fopen(blkfile, "rb")};
         testing_setup->m_node.chainman->LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
     });
     fs::remove(blkfile);

src/bench/streams_findbyte.cpp

@@ -14,7 +14,7 @@
 static void FindByte(benchmark::Bench& bench)
 {
     // Setup
-    CAutoFile file{fsbridge::fopen("streams_tmp", "w+b"), 0};
+    AutoFile file{fsbridge::fopen("streams_tmp", "w+b")};
     const size_t file_size = 200;
     uint8_t data[file_size] = {0};
     data[file_size-1] = 1;

src/index/txindex.cpp

@@ -79,7 +79,7 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe
         return false;
     }
 
-    CAutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true)};
+    AutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true)};
     if (file.IsNull()) {
         return error("%s: OpenBlockFile failed", __func__);
     }

src/node/blockstorage.cpp

@@ -4,23 +4,32 @@
 
 #include <node/blockstorage.h>
 
+#include <arith_uint256.h>
 #include <chain.h>
-#include <clientversion.h>
+#include <consensus/params.h>
 #include <consensus/validation.h>
 #include <dbwrapper.h>
 #include <flatfile.h>
 #include <hash.h>
-#include <kernel/chain.h>
+#include <kernel/blockmanager_opts.h>
 #include <kernel/chainparams.h>
 #include <kernel/messagestartchars.h>
+#include <kernel/notifications_interface.h>
 #include <logging.h>
 #include <pow.h>
+#include <primitives/block.h>
+#include <primitives/transaction.h>
 #include <reverse_iterator.h>
+#include <serialize.h>
 #include <signet.h>
+#include <span.h>
 #include <streams.h>
 #include <sync.h>
+#include <tinyformat.h>
+#include <uint256.h>
 #include <undo.h>
 #include <util/batchpriority.h>
+#include <util/check.h>
 #include <util/fs.h>
 #include <util/signalinterrupt.h>
 #include <util/strencodings.h>
@@ -656,7 +665,7 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n)
 bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const
 {
     // Open history file to append
-    CAutoFile fileout{OpenUndoFile(pos)};
+    AutoFile fileout{OpenUndoFile(pos)};
     if (fileout.IsNull()) {
         return error("%s: OpenUndoFile failed", __func__);
     }
@@ -691,7 +700,7 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in
     }
 
     // Open history file to read
-    CAutoFile filein{OpenUndoFile(pos, true)};
+    AutoFile filein{OpenUndoFile(pos, true)};
     if (filein.IsNull()) {
         return error("%s: OpenUndoFile failed", __func__);
     }
@@ -810,15 +819,15 @@ FlatFileSeq BlockManager::UndoFileSeq() const
     return FlatFileSeq(m_opts.blocks_dir, "rev", UNDOFILE_CHUNK_SIZE);
 }
 
-CAutoFile BlockManager::OpenBlockFile(const FlatFilePos& pos, bool fReadOnly) const
+AutoFile BlockManager::OpenBlockFile(const FlatFilePos& pos, bool fReadOnly) const
 {
-    return CAutoFile{BlockFileSeq().Open(pos, fReadOnly), CLIENT_VERSION};
+    return AutoFile{BlockFileSeq().Open(pos, fReadOnly)};
 }
 
 /** Open an undo file (rev?????.dat) */
-CAutoFile BlockManager::OpenUndoFile(const FlatFilePos& pos, bool fReadOnly) const
+AutoFile BlockManager::OpenUndoFile(const FlatFilePos& pos, bool fReadOnly) const
 {
-    return CAutoFile{UndoFileSeq().Open(pos, fReadOnly), CLIENT_VERSION};
+    return AutoFile{UndoFileSeq().Open(pos, fReadOnly)};
 }
 
 fs::path BlockManager::GetBlockPosFilename(const FlatFilePos& pos) const
@@ -950,7 +959,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP
 bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const
 {
     // Open history file to append
-    CAutoFile fileout{OpenBlockFile(pos)};
+    AutoFile fileout{OpenBlockFile(pos)};
     if (fileout.IsNull()) {
         return error("WriteBlockToDisk: OpenBlockFile failed");
     }
@@ -1016,7 +1025,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons
     block.SetNull();
 
     // Open history file to read
-    CAutoFile filein{OpenBlockFile(pos, true)};
+    AutoFile filein{OpenBlockFile(pos, true)};
     if (filein.IsNull()) {
         return error("ReadBlockFromDisk: OpenBlockFile failed for %s", pos.ToString());
     }
@@ -1059,7 +1068,7 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF
 {
     FlatFilePos hpos = pos;
     hpos.nPos -= 8; // Seek back 8 bytes for meta header
-    CAutoFile filein{OpenBlockFile(hpos, true)};
+    AutoFile filein{OpenBlockFile(hpos, true)};
     if (filein.IsNull()) {
         return error("%s: OpenBlockFile failed for %s", __func__, pos.ToString());
     }
@@ -1151,7 +1160,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
                 if (!fs::exists(chainman.m_blockman.GetBlockPosFilename(pos))) {
                     break; // No block files left to reindex
                 }
-                CAutoFile file{chainman.m_blockman.OpenBlockFile(pos, true)};
+                AutoFile file{chainman.m_blockman.OpenBlockFile(pos, true)};
                 if (file.IsNull()) {
                     break; // This error is logged in OpenBlockFile
                 }
@@ -1172,7 +1181,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
 
         // -loadblock=
         for (const fs::path& path : vImportFiles) {
-            CAutoFile file{fsbridge::fopen(path, "rb"), CLIENT_VERSION};
+            AutoFile file{fsbridge::fopen(path, "rb")};
             if (!file.IsNull()) {
                 LogPrintf("Importing blocks file %s...\n", fs::PathToString(path));
                 chainman.LoadExternalBlockFile(file);

src/node/blockstorage.h

@@ -8,36 +8,36 @@
 #include <attributes.h>
 #include <chain.h>
 #include <dbwrapper.h>
+#include <flatfile.h>
 #include <kernel/blockmanager_opts.h>
-#include <kernel/chain.h>
 #include <kernel/chainparams.h>
 #include <kernel/cs_main.h>
 #include <kernel/messagestartchars.h>
+#include <primitives/block.h>
+#include <streams.h>
 #include <sync.h>
+#include <uint256.h>
 #include <util/fs.h>
 #include <util/hasher.h>
 
+#include <array>
 #include <atomic>
 #include <cstdint>
 #include <functional>
 #include <limits>
 #include <map>
 #include <memory>
+#include <optional>
 #include <set>
 #include <string>
 #include <unordered_map>
 #include <utility>
 #include <vector>
 
 class BlockValidationState;
-class CAutoFile;
-class CBlock;
 class CBlockUndo;
-class CChainParams;
 class Chainstate;
 class ChainstateManager;
-struct CCheckpointData;
-struct FlatFilePos;
 namespace Consensus {
 struct Params;
 }
@@ -162,7 +162,7 @@ class BlockManager
     FlatFileSeq BlockFileSeq() const;
     FlatFileSeq UndoFileSeq() const;
 
-    CAutoFile OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false) const;
+    AutoFile OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false) const;
 
     bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const;
     bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const;
@@ -350,7 +350,7 @@ class BlockManager
     void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     /** Open a block file (blk?????.dat) */
-    CAutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false) const;
+    AutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false) const;
 
     /** Translation to a filesystem path */
     fs::path GetBlockPosFilename(const FlatFilePos& pos) const;

src/streams.h

@@ -505,31 +505,7 @@ class AutoFile
     }
 };
 
-class CAutoFile : public AutoFile
-{
-private:
-    const int nVersion;
-
-public:
-    explicit CAutoFile(std::FILE* file, int version, std::vector<std::byte> data_xor = {}) : AutoFile{file, std::move(data_xor)}, nVersion{version} {}
-    int GetVersion() const       { return nVersion; }
-
-    template<typename T>
-    CAutoFile& operator<<(const T& obj)
-    {
-        ::Serialize(*this, obj);
-        return (*this);
-    }
-
-    template<typename T>
-    CAutoFile& operator>>(T&& obj)
-    {
-        ::Unserialize(*this, obj);
-        return (*this);
-    }
-};
-
-/** Wrapper around a CAutoFile& that implements a ring buffer to
+/** Wrapper around an AutoFile& that implements a ring buffer to
  *  deserialize from. It guarantees the ability to rewind a given number of bytes.
  *
  *  Will automatically close the file when it goes out of scope if not null.
@@ -538,7 +514,7 @@ class CAutoFile : public AutoFile
 class BufferedFile
 {
 private:
-    CAutoFile& m_src;
+    AutoFile& m_src;
     uint64_t nSrcPos{0};  //!< how many bytes have been read from source
     uint64_t m_read_pos{0}; //!< how many bytes have been read from this
     uint64_t nReadLimit;  //!< up to which position we're allowed to read
@@ -585,15 +561,13 @@ class BufferedFile
     }
 
 public:
-    BufferedFile(CAutoFile& file, uint64_t nBufSize, uint64_t nRewindIn)
+    BufferedFile(AutoFile& file, 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)
             throw std::ios_base::failure("Rewind limit must be less than buffer size");
     }
 
-    int GetVersion() const { return m_src.GetVersion(); }
-
     //! check whether we're at the end of the source file
     bool eof() const {
         return m_read_pos == nSrcPos && m_src.feof();

src/test/fuzz/buffered_file.cpp

@@ -20,9 +20,8 @@ FUZZ_TARGET(buffered_file)
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
     FuzzedFileProvider fuzzed_file_provider{fuzzed_data_provider};
     std::optional<BufferedFile> opt_buffered_file;
-    CAutoFile fuzzed_file{
+    AutoFile fuzzed_file{
         fuzzed_file_provider.open(),
-        0,
         ConsumeRandomLengthByteVector<std::byte>(fuzzed_data_provider),
     };
     try {
@@ -65,6 +64,5 @@ FUZZ_TARGET(buffered_file)
                 });
         }
         opt_buffered_file->GetPos();
-        opt_buffered_file->GetVersion();
     }
 }

src/test/fuzz/load_external_block_file.cpp

@@ -28,7 +28,7 @@ FUZZ_TARGET(load_external_block_file, .init = initialize_load_external_block_fil
 {
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
     FuzzedFileProvider fuzzed_file_provider{fuzzed_data_provider};
-    CAutoFile fuzzed_block_file{fuzzed_file_provider.open(), CLIENT_VERSION};
+    AutoFile fuzzed_block_file{fuzzed_file_provider.open()};
     if (fuzzed_block_file.IsNull()) {
         return;
     }

src/test/streams_tests.cpp

@@ -249,7 +249,7 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
 BOOST_AUTO_TEST_CASE(streams_buffered_file)
 {
     fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp";
-    CAutoFile file{fsbridge::fopen(streams_test_filename, "w+b"), 333};
+    AutoFile file{fsbridge::fopen(streams_test_filename, "w+b")};
 
     // The value at each offset is the offset.
     for (uint8_t j = 0; j < 40; ++j) {
@@ -271,9 +271,6 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
     BufferedFile bf{file, 25, 10};
     BOOST_CHECK(!bf.eof());
 
-    // This member has no functional effect.
-    BOOST_CHECK_EQUAL(bf.GetVersion(), 333);
-
     uint8_t i;
     bf >> i;
     BOOST_CHECK_EQUAL(i, 0);
@@ -383,7 +380,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
 BOOST_AUTO_TEST_CASE(streams_buffered_file_skip)
 {
     fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp";
-    CAutoFile file{fsbridge::fopen(streams_test_filename, "w+b"), 333};
+    AutoFile file{fsbridge::fopen(streams_test_filename, "w+b")};
     // The value at each offset is the byte offset (e.g. byte 1 in the file has the value 0x01).
     for (uint8_t j = 0; j < 40; ++j) {
         file << j;
@@ -436,7 +433,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
 
     fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp";
     for (int rep = 0; rep < 50; ++rep) {
-        CAutoFile file{fsbridge::fopen(streams_test_filename, "w+b"), 333};
+        AutoFile file{fsbridge::fopen(streams_test_filename, "w+b")};
         size_t fileSize = InsecureRandRange(256);
         for (uint8_t i = 0; i < fileSize; ++i) {
             file << i;

src/validation.cpp

@@ -4649,7 +4649,7 @@ bool Chainstate::LoadGenesisBlock()
 }
 
 void ChainstateManager::LoadExternalBlockFile(
-    CAutoFile& file_in,
+    AutoFile& file_in,
     FlatFilePos* dbp,
     std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent)
 {