Skip to content

refactor(chain): extract hardfork application into database_hardfork.cpp - #149

Closed
chiliec wants to merge 2 commits into
masterfrom
refactor/split-database-hardfork
Closed

refactor(chain): extract hardfork application into database_hardfork.cpp#149
chiliec wants to merge 2 commits into
masterfrom
refactor/split-database-hardfork

Conversation

@chiliec

@chiliec chiliec commented Aug 10, 2026

Copy link
Copy Markdown
Member

First step of splitting the database.cpp god file (F001 in the tech-debt audit). This is a pure file-level extraction — no behavior change.

Why

libraries/chain/database.cpp is ~8,000 lines compiled into a single object file — the longest-to-compile TU in the project, and it can't be parallelized (the chain CMake even comments "database takes the longest to compile, start it first"; MSVC needs /bigobj for it). Moving a self-contained cluster into its own TU lets it compile in parallel and stops hardfork edits from forcing a recompile of the whole file.

I deliberately started with the smallest, most self-contained, provably-bounded cluster rather than attempting the whole split at once.

What moved

The contiguous hardfork block (originally lines 7012–7989) moved verbatim into libraries/chain/database_hardfork.cpp:

  • init_hardforks
  • reset_virtual_schedule_time
  • process_hardforks
  • has_hardfork
  • set_hardfork
  • apply_hardfork

database.cpp shrinks ~975 lines (7991 → 7016); the new file is ~1,020 lines.

Why it's safe

  • Method bodies are byte-for-byte identical — only their file location changed.
  • The cluster touches only database members declared in database.hpp (_hardfork_times, _hardfork_versions, _log_hardforks), so a separate TU can define these methods.
  • CHAIN_HARDFORK_* / HARDFORK_* constants come from the generated hardfork.hpp, which database.hpp includes; the new file includes database.hpp first, so it sees them.
  • The two VIRTUAL_SCHEDULE_LAP_LENGTH* macros are still used elsewhere in database.cpp (≈ lines 3348, 3817), so they stay defined there. The new file gets its own copy (needed by reset_virtual_schedule_time), with a sync note in both places.
  • to256() is a tiny inline helper already duplicated per-TU (database.cpp and chain_evaluator.cpp each define it); the new TU gets the same local copy.
  • Added database_hardfork.cpp to both the SHARED and STATIC source lists. Existing add_dependencies(graphene_chain ... build_hardfork_hpp) already covers the new TU's need for the generated header.

Verification

I don't have Boost dev libs or the submodules in my environment, so a full tree build wasn't possible. What I did verify:

  • Symbol accounting: 163 database:: definitions/references before == 163 after (database.cpp + database_hardfork.cpp) — none lost, none added.
  • Verbatim move: the original 7012–7989 block appears byte-for-byte in the new file.
  • Structure: both files brace-balanced; namespace nesting closed exactly once.
  • Real compiler check: the added scaffolding (the two macros, the to256 helper, u256 usage, namespace) passes g++ -std=c++14 -fsyntax-only against minimal stubs.

Ask for a maintainer: please confirm a full -DBUILD_TESTNET=OFF build links clean on a box with the real deps before merge. The extraction is mechanical and symbol-accounted, but I couldn't run the final compile+link here.

If this lands cleanly, the same pattern extends to the other natural seams the audit names (apply-block path, snapshot integration, undo-session lifecycle).

database.cpp is the single longest-to-compile translation unit in the project
(~8,000 lines, one object file, cannot be parallelized). This moves the
self-contained hardfork cluster into its own TU so it compiles in parallel with
the rest of database.cpp and edits to hardfork logic no longer force a recompile
of the entire file.

Moved verbatim (byte-for-byte) from database.cpp into database_hardfork.cpp:
  init_hardforks, reset_virtual_schedule_time, process_hardforks,
  has_hardfork, set_hardfork, apply_hardfork
(the contiguous block that was lines 7012-7989). database.cpp shrinks by ~975
lines (7991 -> 7016); the new file is ~1,020 lines.

No behavior change:
- Method bodies are identical; only their file location changed.
- The cluster touches only database members declared in database.hpp
  (_hardfork_times, _hardfork_versions, _log_hardforks), so a separate TU can
  define them. CHAIN_HARDFORK_*/HARDFORK_* constants come from the generated
  hardfork.hpp, which database.hpp includes, so the new file (which includes
  database.hpp first) sees them.
- The two VIRTUAL_SCHEDULE_LAP_LENGTH* macros are still used elsewhere in
  database.cpp (lines ~3348, ~3817), so they remain defined there; the new file
  gets its own copy because reset_virtual_schedule_time needs them. Kept in sync
  by a comment in both places.
- to256() is a small inline helper already duplicated per-TU (database.cpp and
  chain_evaluator.cpp both define it); the new TU gets the same local copy.
- Added database_hardfork.cpp to both the SHARED and STATIC source lists in
  libraries/chain/CMakeLists.txt. The existing
  add_dependencies(graphene_chain ... build_hardfork_hpp) already covers the new
  TU's need for the generated header.

Verification (full tree build not possible in my env - no Boost/submodules):
- Symbol accounting: 163 database:: definitions/references before == 163 after
  (db + hardfork), none lost, none added.
- The original 7012-7989 block appears verbatim in the new file.
- Both files are brace-balanced; namespace nesting is closed once.
- The added scaffolding (the two macros, the to256 helper, u256 usage, namespace)
  passes g++ -std=c++14 -fsyntax-only against minimal stubs.
CI caught a real compile error I could not reproduce locally (no Boost/
submodules in my env): apply_hardfork references proposal_index and
required_approval_index (from proposal_object.hpp), which my hand-picked include
list omitted -> 'proposal_index was not declared in this scope'.

Replace the partial include list with the full graphene/chain/* set that
database.cpp itself uses for the hardfork path (proposal_object.hpp,
block_summary_object.hpp, transaction_object.hpp, index.hpp, db_with.hpp,
evaluator_registry.hpp, operation_notification.hpp, chain_evaluator.hpp,
compound.hpp, shared_db_merkle.hpp). Since these methods previously compiled in
database.cpp with exactly this include surface, matching it guarantees every
object-index type they touch is visible.

Verified the new file's graphene/chain include set is now a superset-equal of
database.cpp's; braces balanced; namespace closed once.
On1x added a commit that referenced this pull request Aug 11, 2026
…cpp (pm, HF14-preserving)

Native re-implementation of #149 on the pm branch. #149 as authored is based on
master and its extracted database_hardfork.cpp does NOT contain pm's HF14 wiring
(init_hardforks registers CHAIN_HARDFORK_14 time/version; apply_hardfork's
CHAIN_HARDFORK_14 case instantiates the pm_lazy_pool_object singleton). Merging
#149 verbatim would have silently dropped that consensus code.

Instead this moves pm's OWN hardfork functions (init_hardforks,
reset_virtual_schedule_time, process_hardforks, has_hardfork, set_hardfork,
apply_hardfork) verbatim from database.cpp into database_hardfork.cpp. Verified
pure move: the 987 moved lines are byte-identical (diff clean). Added
pm_objects.hpp to the mirrored include set (HF14 apply case needs
pm_lazy_pool_object). Both TUs pass single-TU -fsyntax-only.
@On1x

On1x commented Aug 11, 2026

Copy link
Copy Markdown
Member

Folded into the pm branch (PR #124) — but re-implemented natively rather than merged verbatim, because this PR is master-based and its extracted database_hardfork.cpp does NOT carry pm's HF14 wiring:

  • init_hardforks() on pm registers CHAIN_HARDFORK_14 (_hardfork_times/_hardfork_versions);
  • apply_hardfork() on pm has a case CHAIN_HARDFORK_14: that instantiates the pm_lazy_pool_object singleton.

Merging this PR as-is into pm would have silently dropped that consensus code (HF14 would not activate, the PM lazy-pool singleton would never be created).

So on pm I applied the same refactor to pm's OWN functions: moved init_hardforks / reset_virtual_schedule_time / process_hardforks / has_hardfork / set_hardfork / apply_hardfork verbatim from database.cpp into a new database_hardfork.cpp. Verified as a pure move (987 moved lines are byte-identical), added pm_objects.hpp to the mirrored include set (the HF14 apply-case needs pm_lazy_pool_object), both TUs pass single-TU -fsyntax-only, and the full CI Docker build on pm is green.

Same idea as your PR, HF14 preserved — thanks for it! When pm merges to master the split lands there too. Closing this one since the extraction now lives in pm.

@On1x On1x closed this Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants