Skip to content

replay: do not set block_id for leader_blocks#11613

Merged
AshwinSekar merged 1 commit intoanza-xyz:masterfrom
AshwinSekar:block-id-fix
Mar 27, 2026
Merged

replay: do not set block_id for leader_blocks#11613
AshwinSekar merged 1 commit intoanza-xyz:masterfrom
AshwinSekar:block-id-fix

Conversation

@AshwinSekar
Copy link
Copy Markdown

Problem

For leader banks, broadcast is responsible for setting the block_id once the block has finished shredding:

/// Set the block id on the bank and send it for consideration in voting
pub(super) fn set_block_id_and_send(
migration_status: &MigrationStatus,
votor_event_sender: &VotorEventSender,
bank: Arc<Bank>,
block_id: Hash,
) -> Result<()> {
bank.set_block_id(Some(block_id));
if bank.is_frozen() && migration_status.should_send_votor_event(bank.slot()) {
votor_event_sender.send(VotorEvent::Block(CompletedBlock {

In replay we accidentally set the None block id for leader blocks

agave/core/src/replay_stage.rs

Lines 3525 to 3558 in fb1208f

let block_id = if !is_leader_block {
// If the block does not have at least DATA_SHREDS_PER_FEC_BLOCK correctly retransmitted
// shreds in the last FEC set, mark it dead. No reason to perform this check on our leader block.
match process_active_banks_context
.blockstore
.check_last_fec_set_and_get_block_id(
bank.slot(),
bank.hash(),
&bank.feature_set,
) {
Ok(block_id) => block_id,
Err(result_err) => {
let root = bank_forks.read().unwrap().root();
Self::mark_dead_slot(
&process_active_banks_context.blockstore,
bank,
root,
&result_err,
process_active_banks_context.rpc_subscriptions.as_deref(),
process_active_banks_context.slot_status_notifier.as_ref(),
progress,
duplicate_slots_to_repair,
&process_active_banks_context.ancestor_hashes_replay_update_sender,
purge_repair_slot_counter,
&mut tbft_structs,
&process_active_banks_context.replay_vote_sender,
);
continue;
}
}
} else {
None
};
bank.set_block_id(block_id);

In rare cases, shredding can complete before bank freeze on a leader block. This will lead to:

  • broadcast setting Some(block_id)
  • replay setting block_id back to None

Summary of Changes

Do not set block_id in replay for leader blocks.

This was found and correctly addressed in the alpenglow repo - just didn't fix it here

https://github.com/anza-xyz/alpenglow/blob/8a139cd8c68b7ee9582b55f6f9c472cb9444917b/core/src/replay_stage.rs#L3689-L3690

@AshwinSekar AshwinSekar requested a review from bw-solana March 27, 2026 20:14
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.1%. Comparing base (981d3d3) to head (162aaad).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11613   +/-   ##
=======================================
  Coverage    83.1%    83.1%           
=======================================
  Files         850      850           
  Lines      320849   320900   +51     
=======================================
+ Hits       266759   266836   +77     
+ Misses      54090    54064   -26     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread core/src/replay_stage.rs
};
bank.set_block_id(block_id);

if block_id.is_some() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would probably be nice to have a comment explaining the race between broadcast/replay in setting block ID

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.

i'm planning to upstream the alpenglow code which splits this out in a separate fn soon, will add the comment then

@AshwinSekar AshwinSekar added this pull request to the merge queue Mar 27, 2026
Merged via the queue into anza-xyz:master with commit ab84ec6 Mar 27, 2026
48 checks passed
@AshwinSekar AshwinSekar deleted the block-id-fix branch March 27, 2026 21:23
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.

3 participants