refactor: move block-file I/O helpers into node/blockstorage#3073
Open
PrestackI wants to merge 6 commits into
Open
refactor: move block-file I/O helpers into node/blockstorage#3073PrestackI wants to merge 6 commits into
PrestackI wants to merge 6 commits into
Conversation
Lift the pure block/index value types (CBlockHeader, CBlock, CBlockIndex, CDiskBlockIndex, CBlockLocator, BlockHasher, the GRC::MRCFees/MintSummary reports, LOCKTIME_THRESHOLD, FutureDrift, GetTargetSpacing) out of the ~1200-line main.h god-header into a new, main.h-free primitives/block.h, mirroring Bitcoin Core's primitives/block.h. The eight inline methods that reference chain-state globals (pindexBest, mapBlockIndex, pindexGenesisBlock, fUseFastIndex, the genesis hashes) are relocated out-of-line into primitives/block.cpp so the header stays free of main.h. Their EXCLUSIVE_LOCKS_REQUIRED(cs_main) annotations remain on the in-class declarations. consensus/merkle.h and consensus/tx_verify.h now include primitives/block.h instead of main.h, removing the layering inversion where low-level consensus headers dragged in the entire top-of-stack include graph. tx_verify.cpp gains a direct main.h include for the chain-state globals it still uses. main.h keeps functional change.
A quote-include "validation.h" from src/consensus/tx_verify.h resolves to the sibling src/consensus/validation.h (which has no MapPrevTx), not src/validation.h. The original "main.h" include avoided this because there is no src/consensus/main.h. Use angle brackets so the include resolves against -I src to the top-level validation.h that defines MapPrevTx, matching the existing convention in test/script_p2sh_tests.cpp.
validation.h uses CTxIndex by value in the MapPrevTx typedef and the std::map<uint256, CTxIndex> parameters, but only ever compiled because main.h includes index/txindex.h (line 13) before validation.h (line 24). Now that consensus/tx_verify.h pulls validation.h directly, CTxIndex was undeclared, which also broke overload resolution for the CTxIndex ReadTxFromDisk overload at tx_verify.cpp:186. Include index/txindex.h directly (after index/disktxpos.h, since CTxIndex embeds CDiskTxPos), mirroring main.h's existing include order.
Move CTxMemPool, the MemPoolRemovalReason enum, and the global
`mempool` instance out of the main.h/main.cpp god-header into a
dedicated translation unit src/txmempool.{h,cpp}.
This is a pure code move with no behavior change. main.h now
includes txmempool.h, so all existing consumers (including
wallet code using MemPoolRemovalReason) resolve unchanged. It also
serves as Phase 0 of the mempool modernization work (gridcoin-community#3029).
Register txmempool.cpp in the gridcoin_util CMake target.
Continues the incremental main.h/main.cpp decomposition (gridcoin-community#3030). Moves the block-file I/O helpers out of the main.{h,cpp} monolith into the existing node/blockstorage.{h,cpp} translation unit, alongside WriteBlockToDisk / ReadBlockFromDisk which already live there and already call them: CheckDiskSpace, OpenBlockFile, AppendBlockFile, LoadExternalBlockFile (+ the file-local BlockFilePath helper) Their two file-local statics move with them (each has a single user): nMinDiskSpace -> CheckDiskSpace, nCurrentBlockFile -> AppendBlockFile. main.h keeps the consumers working by #include "node/blockstorage.h", so the ~79 includers that call these functions resolve transitively with no edits. The declarations are removed from main.h. Pure code move, no behavior change. Two incidental fixes that the move exposes: - blockstorage.cpp has no `using namespace std`, so the two unqualified `string` locals in the moved bodies are qualified to `std::string`. - blockstorage.cpp now sees its own header (via main.h), so the redundant default arguments on the two ReadBlockFromDisk *definitions* are dropped -- default args already live on the blockstorage.h declarations, and giving them in both places is ill-formed (-fpermissive). Behavior is unchanged.
PrestackI
force-pushed
the
blockstorage/extract-file-io
branch
from
b869d0a to
7c6a476
Compare
When a ctest case fails in the distro/native build path, ctest buffers the test's stdout into build/Testing/Temporary/LastTest.log and prints nothing to the console, so functional-test flakes (e.g. rpc_multisig.py) show only "1/3 Test #1: functional_tests ...***Failed" with no failing script name, traceback, or combined node logs. test_runner.py already runs with --ci --quiet --combinedlogslen=4000, so the detail exists — it's just swallowed. Add --output-on-failure to all four ctest invocations (native, depends, win64, macos), mirroring what the dedicated functional job already does (cmake_functional.yml). The flag is additive: no change on success, full captured output on failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Moves the block-file I/O helpers out of the ~1200-line
src/main.h/ ~3500-linesrc/main.cppmonolith into the existingsrc/node/blockstorage.{h,cpp}translation unit — the natural home, sinceWriteBlockToDisk/ReadBlockFromDiskalready live there and already call these functions.Moved from
main.cppintonode/blockstorage.cpp:CheckDiskSpaceOpenBlockFileAppendBlockFileLoadExternalBlockFileBlockFilePath(file-localstatichelper)Their two file-local statics move alongside their sole users:
nMinDiskSpace→CheckDiskSpace,nCurrentBlockFile→AppendBlockFile.The declarations are removed from
main.h; the four public functions are now declared innode/blockstorage.h.main.hgains#include "node/blockstorage.h", so the ~79 files that includemain.hand call these helpers keep resolving transitively with no consumer edits (validation.cpp,init.cpp,gridcoin/upgrade.cpp,gridcoin/staking/kernel.cpp,gridcoin/voting/result.cpp).No functional change — pure code move. The only textual change to the moved bodies:
blockstorage.cpphas nousing namespace std, so the two unqualifiedstringlocals are qualified tostd::string.Why
Continues the incremental
main.h/main.cppdecomposition tracked in #3030 (Workstream A), after A1 (block primitives, #3060) and A2 (txmempool, #3072). Block-file I/O is a self-contained, isolated slice that belongs with the rest of block storage.Diffstat
main.cpp−155,main.h−11;node/blockstorage.cpp+165,node/blockstorage.h+10. NoCMakeLists.txtchange (blockstorage.cppalready builds ingridcoin_util).Testing
Move-only diff (
git diff -Mshows pure relocation, no logic change). Local lint green:lint-whitespace,lint-include-guards,lint-circular-dependencies(the newmain.h → node/blockstorage.hedge introduces no cycle). Full CI matrix to confirm on the PR.