Refactor: block tree and fork choice #51
Conversation
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
There was a problem hiding this comment.
Pull request overview
This pull request refactors the block tree and fork choice implementation, extracting block and state collections into dedicated storage, implementing state persistence, and improving the CLI logging configuration system.
Key Changes
- Refactored block tree architecture by removing FCBlockTree and enhancing BlockTreeImpl with integrated fork choice
- Introduced state storage alongside blocks, replacing justification-based finalization with state-based approach
- Added LRU caching for blockchain states to improve performance
- Enhanced CLI logging system with improved argument parsing and validation
Reviewed changes
Copilot reviewed 57 out of 58 changed files in this pull request and generated 25 comments.
Show a summary per file
| File | Description |
|---|---|
| src/blockchain/fork_choice.cpp | Major refactoring to use BlockTree/BlockStorage instead of internal maps; added state caching |
| src/blockchain/impl/block_tree_impl.cpp | Updated finalize method to remove justification parameter; renamed getNumberByHash to getSlotByHash |
| src/blockchain/impl/block_storage_impl.cpp | Replaced justification storage methods with state storage methods |
| src/blockchain/impl/anchor_block_impl.cpp | New implementation for anchor block based on anchor state |
| src/blockchain/impl/anchor_state_impl.cpp | New implementation for anchor state from genesis config |
| src/utils/lru_cache.hpp | New LRU cache implementation for performance optimization |
| src/log/logger.cpp | Enhanced logging configuration with better CLI argument parsing |
| tests/unit/blockchain/fork_choice_test.cpp | Tests need updates for new constructor signatures |
| src/types/justification.hpp | File deleted - justification concept replaced with state storage |
| src/blockchain/impl/fc_block_tree.cpp | File deleted - functionality merged into BlockTreeImpl |
| example/0-single/genesis/config.yaml | Updated genesis configuration with validator settings |
| CMakeLists.txt | Added Boost.DI configuration macros |
Comments suppressed due to low confidence (1)
tests/unit/blockchain/fork_choice_test.cpp:132
- The makeBlockMap function returns a ForkChoiceStore::Blocks type which has been commented out in the fork_choice.hpp (lines 64-65). This will cause a compilation error. The function and its usages need to be refactored to work with the new block tree-based storage.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
turuslan
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 50 changed files in this pull request and generated 23 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…specify base paths for nodes
| if (not hash_opt.has_value()) { | ||
| updateHash(); | ||
| } |
There was a problem hiding this comment.
I'd leave updateHash call explicit, e.g. caller must call updateHash once after building block, to avoid inconsistent hash
There was a problem hiding this comment.
I'd separate preparing block header and done as NewBlockHeader with mutable fields, without hash(). And method BlockHeader NewBlockHeader::seal()&& method (or func BlockHeader seal(NewBlockHeader&&).
Right now uninitialized hash_opt doesn't safe from lost updateHash(), and opposite - can be foggoten updateHash() call after header's internals update.
xDimon
force-pushed
the
refactor/block_tree_and_forck_choice
branch
from
e7122e5 to
2146991
Compare
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Base path set by CLI.
Text-banner with digest of current state in stderr.
Take to use block tree and block storage.
Extract block+state collection into block tree/storage
Implement storing of block states.
Fix logging system tuning over cli.
Some minor fixes and improvements.