Skip to content

blockstore: Parent must be full to set previous_blockhash#12012

Open
steviez wants to merge 2 commits intoanza-xyz:masterfrom
steviez:bstore_get_block_err_if_parent_not_full
Open

blockstore: Parent must be full to set previous_blockhash#12012
steviez wants to merge 2 commits intoanza-xyz:masterfrom
steviez:bstore_get_block_err_if_parent_not_full

Conversation

@steviez
Copy link
Copy Markdown

@steviez steviez commented Apr 17, 2026

Problem

The Blockstore:: do_get_complete_block_with_entries() method is in the call stack for an RPC getBlock call. One of the fields in the result is the blockhash of the parent block. The current logic grabs the last entry from the parent block and uses this. However, the parent block could be non-full which could result in an invalid value being set for previous_blockhash.

In normal operation, this is quite unlikely as rooting the current block implies that we'll have rooted the parent block. This bug was discovered debugging a devnet bigtable issue that stems from weirdness around deactivating features (see #7287 for more on that).

Summary of Changes

  • The first commit reworks the existing unit test to more exhaustively test scenarios
    • Additionally, there is an assert that is commented out that will fail with the tip of master
  • The second commit adjusts do_get_complete_block_with_entries() to ensure the previous blockhash is only populated when the parent is full
    • The commented out assert is uncommented and the full unit test passes

I plan on squashing commits prior to merge to clean up commit history + add details from above message into commit

Comment thread ledger/src/blockstore.rs
Comment on lines -8528 to -8529
let parent_meta = SlotMeta::default();
blockstore.put_meta(slot - 1, &parent_meta).unwrap();
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple notes on this removal:

  • Directly modifying the meta muddies the water on what insertion logic does
  • Insertion logic inserts an empty slot meta for the empty slot - 1 as part of chaining so this is redundant / unnecessary
  • If this was modifying a meta for a non-empty slot, it'd be creating a column inconsistency that isn't reachable with normal logic

Comment thread ledger/src/blockstore.rs Outdated
Comment on lines +8561 to +8569
// A full root will return an error if the previous blockhash is
// required and the parent slot is partially full
/*
// Uncomment me in next commit - this assert will fail currently
assert_matches!(
blockstore.get_rooted_block(slot, true),
Err(BlockstoreError::ParentEntriesUnavailable)
);
*/
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If reviewers want to verify, they can:

  • Pull the branch down
  • Revert the second commit
  • Uncomment this assert out
  • Run cargo test test_get_rooted_block to see this test fail
  • Pull down the tip of the branch again (to get both commits)
  • Run cargo test test_get_rooted_block again to see the test succeed since the second commit adjusts logic in the actual prod code

@steviez steviez force-pushed the bstore_get_block_err_if_parent_not_full branch from 23db450 to 9600df3 Compare April 17, 2026 05:59
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.59259% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.2%. Comparing base (039baa1) to head (9600df3).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #12012   +/-   ##
=======================================
  Coverage    83.2%    83.2%           
=======================================
  Files         860      860           
  Lines      321570   321579    +9     
=======================================
+ Hits       267774   267784   +10     
+ Misses      53796    53795    -1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@steviez steviez marked this pull request as ready for review April 17, 2026 06:38
@steviez steviez requested review from AshwinSekar and cpubot April 17, 2026 06:38
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.

4 participants