Move block_dependencies() from ledger to block class#5028
Move block_dependencies() from ledger to block class#5028pwojcikdev merged 2 commits intonanocurrency:developfrom
Conversation
Test Results for Commit a966a0ePull Request 5028: Results Test Case Results
Last updated: 2026-02-12 16:32:57 UTC |
There was a problem hiding this comment.
Pull request overview
This PR relocates dependency discovery from nano::ledger into nano::block, so call sites can query a block directly for its prerequisite hashes, and refactors sideband serialization logic for clarity.
Changes:
- Removed
ledger::block_dependencies()and switched ledger/scheduler call sites toblock::dependencies(). - Added
nano::block::dependencies()and a comprehensive unit test covering dependency outputs across block types/subtypes. - Refactored
block_sideband(de)serialization logic by introducingincludes_*helpers and clarifying sideband layout expectations.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| nano/secure/ledger.hpp | Removes block_dependencies() declaration from the ledger interface. |
| nano/secure/ledger.cpp | Replaces ledger dependency computation with block::dependencies() and deletes the old visitor implementation. |
| nano/node/scheduler/hinted.cpp | Uses block::dependencies() when traversing dependency graph. |
| nano/lib/blocks.hpp | Adds block::dependencies() API and required <array> include. |
| nano/lib/blocks.cpp | Implements block::dependencies(). |
| nano/lib/block_sideband.hpp | Documents sideband layout caveats; declares includes_* helpers. |
| nano/lib/block_sideband.cpp | Implements includes_* helpers and uses them in size/serialize/deserialize. |
| nano/core_test/ledger.cpp | Adds ledger.block_dependencies test validating dependency outputs across block types. |
| nano/core_test/active_elections.cpp | Removes an assertion that relied on dependency checks for blocks without sideband. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bool is_epoch () const noexcept; | ||
|
|
||
| // Returns block hashes that this block depends on (must be confirmed before this block) | ||
| // Non-zero entries are: [0] = previous block, [1] = source/send block (for receives) |
There was a problem hiding this comment.
block::dependencies() calls release_assert(has_sideband()) (and relies on sideband-derived subtype via is_receive()), but the API comment doesn’t mention this requirement. Since block instances can exist without sideband (e.g. forks/blocks not deserialized from the DB), this method can currently hard-abort if called in those cases. Consider documenting that dependencies() is only valid when sideband is present, or alternatively returning an empty/optional result (or providing a separate helper that can compute dependencies without sideband when needed).
| // Non-zero entries are: [0] = previous block, [1] = source/send block (for receives) | |
| // Non-zero entries are: [0] = previous block, [1] = source/send block (for receives) | |
| // Requires: has_sideband() is true (dependencies() relies on sideband and may assert otherwise). |
No description provided.