Conversation
check_account() unconditionally called .value() on the std::optional returned by block_balance(transaction, block->previous()) when processing a state block, guarded only by "!node->ledger.pruning || prev_balance". When pruning is disabled (the common case), that guard is true regardless of whether prev_balance actually has a value, so the .value() calls below ran unconditionally. For any account open block, previous is the zero hash by definition, so block_balance() correctly returns an empty optional -- there is no block at hash zero. This meant --validate_blocks crashed with std::bad_optional_access (SIGABRT) on the very first state-type open block it encountered, on any ledger, whenever pruning was disabled. Fix: default prev_balance to zero for an open block (previous == zero), mirroring the pattern already used a few lines above for the epoch-link signature check. Only fall back to the pruned-block-exists check when pruning is actually enabled; report a clear error instead of crashing for the (otherwise unreachable) case of a missing previous balance with pruning disabled.
Test Results for Commit b916834Pull Request 5099: Results Test Case Results
Last updated: 2026-07-02 12:32:49 UTC |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a deterministic crash in nano_node --validate_blocks when validating state-type open blocks with pruning disabled by avoiding std::optional::value() on an empty previous-balance lookup for the zero previous hash.
Changes:
- Treat state open blocks (
previous == 0) as having an implicit previous balance of zero during sideband detail validation. - Avoid unconditional
.value()access on the previous-balance optional; emit an error message instead when the previous balance is unexpectedly unavailable with pruning disabled. - Keep the existing “check pruned previous exists” behavior gated to pruning-enabled mode.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1566
to
+1567
| nano::amount prev_balance{ 0 }; | ||
| bool prev_balance_known = true; |
| else | ||
| { | ||
| print_error_message (boost::str (boost::format ("Previous pruned block does not exist %1%\n") % block->previous ().to_string ())); | ||
| print_error_message (boost::str (boost::format ("Missing previous block balance for %1%\n") % hash.to_string ())); |
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
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.
Summary
nano_node --validate_blockscrashes withterminate called after throwing an instance of 'std::bad_optional_access'(SIGABRT) the first time it encounters a state-type open block, on any ledger, whenever pruning is disabled (the common case — pruning is opt-in).Root cause
In
check_account()'s "Validate block details set in the sideband" branch:prev_balanceis astd::optional<nano::amount>. The guard!node->ledger.pruning || prev_balanceis only a real check when pruning is enabled — when it's disabled,!node->ledger.pruningistrue, so the guard passes regardless of whetherprev_balanceactually has a value, and the.value()calls below run unconditionally.For any account's open block,
previousis the zero hash by definition.block_balance(transaction, zero_hash)correctly returns an empty optional — there is no block at hash zero to look up. So this crashes deterministically on the first state-type open block processed, on essentially every real-world ledger (pruning is opt-in and uncommon).Confirmed via
gdb+ aRelWithDebInfobuild with a temporary diagnostic print at the crash site, against a real synced ledger:(Not a special/reserved account — its public key is just numerically close to zero, hence the address encodes as mostly
1s.)Fix
Default
prev_balanceto zero whenblock->previous()is the zero hash (an open block), mirroring the pattern already used a few lines above for the epoch-link signature check in the same function, which already handles this correctly vianano::amount prev_balance (0)+value_or (0). Only fall back to the pruned-block-exists check when pruning is actually enabled; report a clear error via the existingprint_error_messagemechanism instead of crashing for the (otherwise unreachable, and likely indicative of a real problem) case of a missing previous balance with pruning disabled.Testing
--threads=1reproduces identically).V27.0,V28.2(current stable), anddevelopprior to this fix.--validate_blocksagainst the exact same ledger data that previously crashed: completes successfully now (45,684 accounts / 74,818 pending blocks validated, exits cleanly instead of SIGABRT).Note
That test run reported some unrelated pre-existing validation findings (
Incorrect source epoch for block ...) on this still-syncing ledger. Those are a separate concern, unaffected by and unrelated to this fix — this PR is scoped only to eliminating the crash so--validate_blockscan actually run to completion and report whatever it finds, rather than aborting immediately.