4eb2a9ea4b streams: Drop unused CAutoFile (Anthony Towns)
cde9a4b137 refactor: switch from CAutoFile to AutoFile (Anthony Towns)
bbd4646a2e blockstorage: switch from CAutoFile to AutoFile (Anthony Towns)
c72ddf04db streams: Remove unused CAutoFile::GetVersion (Anthony Towns)
e63f643079 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 4eb2a9ea4b 🖼
  TheCharlatan:
    ACK 4eb2a9ea4b
  stickies-v:
    ACK 4eb2a9ea4b

Tree-SHA512: 1a68c42fdb725ca4bf573e22794fe7809fea764a5f97ecb33435add3c609d40f336038fb22ab1ea72567530efd39678278c9016f92ed04891afdb310631b4e82
This commit is contained in:
fanquake
2023-11-22 11:19:40 +00:00
11 changed files with 43 additions and 65 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -28,7 +28,7 @@ FUZZ_TARGET(load_external_block_file, .init = initialize_load_external_block_fil
{ {
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
FuzzedFileProvider fuzzed_file_provider{fuzzed_data_provider}; 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()) { if (fuzzed_block_file.IsNull()) {
return; return;
} }

View File

@ -249,7 +249,7 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
BOOST_AUTO_TEST_CASE(streams_buffered_file) BOOST_AUTO_TEST_CASE(streams_buffered_file)
{ {
fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp"; 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. // The value at each offset is the offset.
for (uint8_t j = 0; j < 40; ++j) { for (uint8_t j = 0; j < 40; ++j) {
@ -271,9 +271,6 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
BufferedFile bf{file, 25, 10}; BufferedFile bf{file, 25, 10};
BOOST_CHECK(!bf.eof()); BOOST_CHECK(!bf.eof());
// This member has no functional effect.
BOOST_CHECK_EQUAL(bf.GetVersion(), 333);
uint8_t i; uint8_t i;
bf >> i; bf >> i;
BOOST_CHECK_EQUAL(i, 0); BOOST_CHECK_EQUAL(i, 0);
@ -383,7 +380,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
BOOST_AUTO_TEST_CASE(streams_buffered_file_skip) BOOST_AUTO_TEST_CASE(streams_buffered_file_skip)
{ {
fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp"; 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). // 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) { for (uint8_t j = 0; j < 40; ++j) {
file << 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"; fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp";
for (int rep = 0; rep < 50; ++rep) { 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); size_t fileSize = InsecureRandRange(256);
for (uint8_t i = 0; i < fileSize; ++i) { for (uint8_t i = 0; i < fileSize; ++i) {
file << i; file << i;

View File

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

View File

@ -1137,7 +1137,7 @@ public:
* (only used for reindex) * (only used for reindex)
* */ * */
void LoadExternalBlockFile( void LoadExternalBlockFile(
CAutoFile& file_in, AutoFile& file_in,
FlatFilePos* dbp = nullptr, FlatFilePos* dbp = nullptr,
std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent = nullptr); std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent = nullptr);