Merge bitcoin#28191: refactor: Remove unused MessageStartChars parameters from BlockManager methods

fanquake · fanquake · commit be4433280371 · 2023-08-07T10:57:39.000+02:00
fa69e3a Remove unused MessageStartChars parameters from BlockManager methods (MarcoFalke) Pull request description: Seems odd to expose these for mocking, when it is not needed. Fix this by removing the the unused parameters and use the already existing member field instead. ACKs for top commit: Empact: utACK fa69e3a dergoegge: utACK fa69e3a Tree-SHA512: 7814e9560abba8d9c0926bcffc70f92e502d22f543af43671248f6fcd1433f35238553c0f05123fde6d8e0f80261af0ab0500927548115153bd68d57fe2da746
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -2212,7 +2212,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
         // 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<uint8_t> block_data;
-        if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos(), m_chainparams.MessageStart())) {
+        if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos())) {
             assert(!"cannot load block from disk");
         }
         m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, Span{block_data}));
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
@@ -476,7 +476,7 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n)
     return &m_blockfile_info.at(n);
 }
 
-bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock, const CMessageHeader::MessageStartChars& messageStart) const
+bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const
 {
     // Open history file to append
     AutoFile fileout{OpenUndoFile(pos)};
@@ -486,7 +486,7 @@ bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos
 
     // Write index header
     unsigned int nSize = GetSerializeSize(blockundo, CLIENT_VERSION);
-    fileout << messageStart << nSize;
+    fileout << GetParams().MessageStart() << nSize;
 
     // Write undo data
     long fileOutPos = ftell(fileout.Get());
@@ -708,7 +708,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP
     return true;
 }
 
-bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const CMessageHeader::MessageStartChars& messageStart) const
+bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const
 {
     // Open history file to append
     CAutoFile fileout(OpenBlockFile(pos), SER_DISK, CLIENT_VERSION);
@@ -718,7 +718,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const
 
     // Write index header
     unsigned int nSize = GetSerializeSize(block, fileout.GetVersion());
-    fileout << messageStart << nSize;
+    fileout << GetParams().MessageStart() << nSize;
 
     // Write block
     long fileOutPos = ftell(fileout.Get());
@@ -740,7 +740,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid
         if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo, CLIENT_VERSION) + 40)) {
             return error("ConnectBlock(): FindUndoPos failed");
         }
-        if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash(), GetParams().MessageStart())) {
+        if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) {
             return FatalError(m_opts.notifications, state, "Failed to write undo data");
         }
         // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order)
@@ -806,7 +806,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) co
     return true;
 }
 
-bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start) const
+bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos) const
 {
     FlatFilePos hpos = pos;
     hpos.nPos -= 8; // Seek back 8 bytes for meta header
@@ -821,10 +821,10 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF
 
         filein >> blk_start >> blk_size;
 
-        if (memcmp(blk_start, message_start, CMessageHeader::MESSAGE_START_SIZE)) {
+        if (memcmp(blk_start, GetParams().MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) {
             return error("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(),
                          HexStr(blk_start),
-                         HexStr(message_start));
+                         HexStr(GetParams().MessageStart()));
         }
 
         if (blk_size > MAX_SIZE) {
@@ -859,7 +859,7 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, cons
         return FlatFilePos();
     }
     if (!position_known) {
-        if (!WriteBlockToDisk(block, blockPos, GetParams().MessageStart())) {
+        if (!WriteBlockToDisk(block, blockPos)) {
             m_opts.notifications.fatalError("Failed to write block");
             return FlatFilePos();
         }
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
@@ -101,8 +101,8 @@ class BlockManager
 
     FILE* OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false) const;
 
-    bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const CMessageHeader::MessageStartChars& messageStart) const;
-    bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock, const CMessageHeader::MessageStartChars& messageStart) const;
+    bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const;
+    bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const;
 
     /* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */
     void FindFilesToPruneManual(std::set<int>& setFilesToPrune, int nManualPruneHeight, int chain_tip_height);
@@ -264,7 +264,7 @@ class BlockManager
     /** Functions for disk access for blocks */
     bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) const;
     bool ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) const;
-    bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start) const;
+    bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos) const;
 
     bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& index) const;
 

Original file line number	Diff line number	Diff line change
`@@ -2212,7 +2212,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&`
`2212`	`2212`	`// Fast-path: in this case it is possible to serve the block directly from disk,`
`2213`	`2213`	`// as the network format matches the format on disk`
`2214`	`2214`	`std::vector<uint8_t> block_data;`
`2215`		`- if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos(), m_chainparams.MessageStart())) {`
	`2215`	`+ if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos())) {`
`2216`	`2216`	`assert(!"cannot load block from disk");`
`2217`	`2217`	`}`
`2218`	`2218`	`m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, Span{block_data}));`
Original file line number	Diff line number	Diff line change
`@@ -476,7 +476,7 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n)`
`476`	`476`	`return &m_blockfile_info.at(n);`
`477`	`477`	`}`
`478`	`478`
`479`		`-bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock, const CMessageHeader::MessageStartChars& messageStart) const`
	`479`	`+bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const`
`480`	`480`	`{`
`481`	`481`	`// Open history file to append`
`482`	`482`	`AutoFile fileout{OpenUndoFile(pos)};`
`@@ -486,7 +486,7 @@ bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos`
`486`	`486`
`487`	`487`	`// Write index header`
`488`	`488`	`unsigned int nSize = GetSerializeSize(blockundo, CLIENT_VERSION);`
`489`		`- fileout << messageStart << nSize;`
	`489`	`+ fileout << GetParams().MessageStart() << nSize;`
`490`	`490`
`491`	`491`	`// Write undo data`
`492`	`492`	`long fileOutPos = ftell(fileout.Get());`
`@@ -708,7 +708,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP`
`708`	`708`	`return true;`
`709`	`709`	`}`
`710`	`710`
`711`		`-bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const CMessageHeader::MessageStartChars& messageStart) const`
	`711`	`+bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const`
`712`	`712`	`{`
`713`	`713`	`// Open history file to append`
`714`	`714`	`CAutoFile fileout(OpenBlockFile(pos), SER_DISK, CLIENT_VERSION);`
`@@ -718,7 +718,7 @@ bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const`
`718`	`718`
`719`	`719`	`// Write index header`
`720`	`720`	`unsigned int nSize = GetSerializeSize(block, fileout.GetVersion());`
`721`		`- fileout << messageStart << nSize;`
	`721`	`+ fileout << GetParams().MessageStart() << nSize;`
`722`	`722`
`723`	`723`	`// Write block`
`724`	`724`	`long fileOutPos = ftell(fileout.Get());`
`@@ -740,7 +740,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid`
`740`	`740`	`if (!FindUndoPos(state, block.nFile, _pos, ::GetSerializeSize(blockundo, CLIENT_VERSION) + 40)) {`
`741`	`741`	`return error("ConnectBlock(): FindUndoPos failed");`
`742`	`742`	`}`
`743`		`- if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash(), GetParams().MessageStart())) {`
	`743`	`+ if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash())) {`
`744`	`744`	`return FatalError(m_opts.notifications, state, "Failed to write undo data");`
`745`	`745`	`}`
`746`	`746`	`// rev files are written in block height order, whereas blk files are written as blocks come in (often out of order)`
`@@ -806,7 +806,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const CBlockIndex& index) co`
`806`	`806`	`return true;`
`807`	`807`	`}`
`808`	`808`
`809`		`-bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start) const`
	`809`	`+bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos) const`
`810`	`810`	`{`
`811`	`811`	`FlatFilePos hpos = pos;`
`812`	`812`	`hpos.nPos -= 8; // Seek back 8 bytes for meta header`
`@@ -821,10 +821,10 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF`
`821`	`821`
`822`	`822`	`filein >> blk_start >> blk_size;`
`823`	`823`
`824`		`- if (memcmp(blk_start, message_start, CMessageHeader::MESSAGE_START_SIZE)) {`
	`824`	`+ if (memcmp(blk_start, GetParams().MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) {`
`825`	`825`	`return error("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(),`
`826`	`826`	`HexStr(blk_start),`
`827`		`- HexStr(message_start));`
	`827`	`+ HexStr(GetParams().MessageStart()));`
`828`	`828`	`}`
`829`	`829`
`830`	`830`	`if (blk_size > MAX_SIZE) {`
`@@ -859,7 +859,7 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, cons`
`859`	`859`	`return FlatFilePos();`
`860`	`860`	`}`
`861`	`861`	`if (!position_known) {`
`862`		`- if (!WriteBlockToDisk(block, blockPos, GetParams().MessageStart())) {`
	`862`	`+ if (!WriteBlockToDisk(block, blockPos)) {`
`863`	`863`	`m_opts.notifications.fatalError("Failed to write block");`
`864`	`864`	`return FlatFilePos();`
`865`	`865`	`}`