Skip to content

Commit 93f04d3

Browse files
ksn6AshwinSekar
authored andcommitted
feat: perform all BlockComponent checks in blockstore processor (anza-xyz#594)
Right now, we need to complete replay to verify that `BlockComponent`s in a block satisfy a number of invariants (e.g., exactly one header, exactly one footer, etc.). @AshwinSekar found a clever way to avoid this: anza-xyz#575 (comment). Implement the above.
1 parent c5fb11d commit 93f04d3

File tree

3 files changed

+133
-31
lines changed

3 files changed

+133
-31
lines changed

ledger/src/blockstore_processor.rs

Lines changed: 83 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ use {
2121
},
2222
solana_clock::{MAX_PROCESSING_AGE, Slot},
2323
solana_cost_model::{cost_model::CostModel, transaction_cost::TransactionCost},
24-
solana_entry::entry::{self, Entry, EntrySlice, EntryType, create_ticks},
24+
solana_entry::{
25+
block_component::BlockComponent,
26+
entry::{self, Entry, EntrySlice, EntryType, create_ticks},
27+
},
2528
solana_genesis_config::GenesisConfig,
2629
solana_hash::Hash,
2730
solana_keypair::Keypair,
@@ -32,6 +35,7 @@ use {
3235
bank::{Bank, PreCommitResult, TransactionBalancesSet},
3336
bank_forks::BankForks,
3437
bank_utils,
38+
block_component_processor::BlockComponentProcessorError,
3539
commitment::VOTE_THRESHOLD_SIZE,
3640
dependency_tracker::DependencyTracker,
3741
installed_scheduler_pool::BankWithScheduler,
@@ -824,6 +828,9 @@ pub enum BlockstoreProcessorError {
824828
#[error("user transactions found in vote only mode bank at slot {0}")]
825829
UserTransactionsInVoteOnlyBank(Slot),
826830

831+
#[error("block component processor error: {0}")]
832+
BlockComponentProcessor(#[from] BlockComponentProcessorError),
833+
827834
#[error("invalid parent -> child chained merkle root at slot {0} parent {1}")]
828835
ChainedBlockIdFailure(Slot, Slot),
829836
}
@@ -1494,10 +1501,10 @@ pub fn confirm_slot(
14941501
) -> result::Result<(), BlockstoreProcessorError> {
14951502
let slot = bank.slot();
14961503

1497-
let slot_entries_load_result = {
1504+
let (slot_components, completed_ranges, slot_full) = {
14981505
let mut load_elapsed = Measure::start("load_elapsed");
14991506
let load_result = blockstore
1500-
.get_slot_entries_with_shred_info(slot, progress.num_shreds, allow_dead_slots)
1507+
.get_slot_components_with_shred_info(slot, progress.num_shreds, allow_dead_slots)
15011508
.map_err(BlockstoreProcessorError::FailedToLoadEntries);
15021509
load_elapsed.stop();
15031510
if load_result.is_err() {
@@ -1508,20 +1515,79 @@ pub fn confirm_slot(
15081515
load_result
15091516
}?;
15101517

1511-
confirm_slot_entries(
1512-
bank,
1513-
replay_tx_thread_pool,
1514-
slot_entries_load_result,
1515-
timing,
1516-
progress,
1517-
skip_verification,
1518-
transaction_status_sender,
1519-
entry_notification_sender,
1520-
replay_vote_sender,
1521-
log_messages_bytes_limit,
1522-
prioritization_fee_cache,
1523-
migration_status,
1524-
)
1518+
// Process block components for Alpenglow slots. Note that we don't need to run migration checks
1519+
// for BlockMarkers here, despite BlockMarkers only being active post-Alpenglow. Here's why:
1520+
//
1521+
// Post-Alpenglow migration - validators that have Alpenglow enabled can parse BlockComponents.
1522+
// Things just work.
1523+
//
1524+
// Pre-Alpenglow migration, suppose a validator receives a BlockMarker:
1525+
//
1526+
// (1) validators *incapable* of processing BlockMarkers will mark the slot as dead on shred
1527+
// ingest in blockstore.
1528+
//
1529+
// (2) validators *capable* of processing BlockMarkers will store the BlockMarkers in shred
1530+
// ingest, run through this verifying code here, and then error out when processing a
1531+
// BlockMarker, resulting in the slot being marked as dead.
1532+
let mut processor = bank.block_component_processor.write().unwrap();
1533+
1534+
// Find the index of the last EntryBatch in slot_components
1535+
let last_entry_batch_index = slot_components
1536+
.iter()
1537+
.rposition(|bc| matches!(bc, BlockComponent::EntryBatch(_)));
1538+
1539+
for (ix, (completed_range, component)) in completed_ranges
1540+
.iter()
1541+
.zip(slot_components.into_iter())
1542+
.enumerate()
1543+
{
1544+
let num_shreds = completed_range.end - completed_range.start;
1545+
let is_final = slot_full && ix == completed_ranges.len() - 1;
1546+
1547+
match component {
1548+
BlockComponent::EntryBatch(entries) => {
1549+
let slot_full = slot_full
1550+
&& last_entry_batch_index.is_some_and(|last_entry_batch_ix| {
1551+
ix == last_entry_batch_ix
1552+
});
1553+
1554+
// Skip block component validation for genesis block. Slot 0 is handled specially,
1555+
// since it won't have the required block markers (e.g., the header and the footer).
1556+
if bank.slot() != 0 {
1557+
processor
1558+
.on_entry_batch(migration_status, is_final)
1559+
.inspect_err(|err| {
1560+
warn!("Block component processing failed for slot {slot}: {err:?}",);
1561+
})?;
1562+
}
1563+
1564+
confirm_slot_entries(
1565+
bank,
1566+
replay_tx_thread_pool,
1567+
(entries, num_shreds as u64, slot_full),
1568+
timing,
1569+
progress,
1570+
skip_verification,
1571+
transaction_status_sender,
1572+
entry_notification_sender,
1573+
replay_vote_sender,
1574+
log_messages_bytes_limit,
1575+
prioritization_fee_cache,
1576+
migration_status,
1577+
)?;
1578+
}
1579+
BlockComponent::BlockMarker(marker) => {
1580+
processor
1581+
.on_marker(&marker, migration_status, is_final)
1582+
.inspect_err(|err| {
1583+
warn!("Block component processing failed for slot {slot}: {err:?}",);
1584+
})?;
1585+
progress.num_shreds += num_shreds as u64;
1586+
}
1587+
}
1588+
}
1589+
1590+
Ok(())
15251591
}
15261592

15271593
#[allow(clippy::too_many_arguments)]

runtime/src/bank.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ use {
4343
},
4444
},
4545
bank_forks::BankForks,
46+
block_component_processor::BlockComponentProcessor,
4647
epoch_stakes::{
4748
BLSPubkeyToRankMap, DeserializableVersionedEpochStakes, NodeVoteAccounts,
4849
VersionedEpochStakes,
@@ -600,6 +601,7 @@ impl PartialEq for Bank {
600601
stats_for_accounts_lt_hash: _,
601602
block_id,
602603
expected_bank_hash: _,
604+
block_component_processor: _,
603605
bank_hash_stats: _,
604606
epoch_rewards_calculation_cache: _,
605607
// Ignore new fields explicitly if they do not impact PartialEq.
@@ -946,6 +948,9 @@ pub struct Bank {
946948
/// later when the bank is frozen.
947949
expected_bank_hash: RwLock<Option<Hash>>,
948950

951+
/// Processor for validating the per-slot block component stream.
952+
pub block_component_processor: RwLock<BlockComponentProcessor>,
953+
949954
/// Accounts stats for computing the bank hash
950955
bank_hash_stats: AtomicBankHashStats,
951956

@@ -1160,6 +1165,7 @@ impl Bank {
11601165
stats_for_accounts_lt_hash: AccountsLtHashStats::default(),
11611166
block_id: RwLock::new(None),
11621167
expected_bank_hash: RwLock::new(None),
1168+
block_component_processor: RwLock::new(BlockComponentProcessor::default()),
11631169
bank_hash_stats: AtomicBankHashStats::default(),
11641170
epoch_rewards_calculation_cache: Arc::new(Mutex::new(HashMap::default())),
11651171
};
@@ -1408,6 +1414,7 @@ impl Bank {
14081414
stats_for_accounts_lt_hash: AccountsLtHashStats::default(),
14091415
block_id: RwLock::new(None),
14101416
expected_bank_hash: RwLock::new(None),
1417+
block_component_processor: RwLock::new(BlockComponentProcessor::default()),
14111418
bank_hash_stats: AtomicBankHashStats::default(),
14121419
epoch_rewards_calculation_cache: parent.epoch_rewards_calculation_cache.clone(),
14131420
};
@@ -1970,6 +1977,7 @@ impl Bank {
19701977
cache_for_accounts_lt_hash: DashMap::default(),
19711978
stats_for_accounts_lt_hash: AccountsLtHashStats::default(),
19721979
block_id: RwLock::new(None),
1980+
block_component_processor: RwLock::new(BlockComponentProcessor::default()),
19731981
bank_hash_stats: AtomicBankHashStats::new(&fields.bank_hash_stats),
19741982
epoch_rewards_calculation_cache: Arc::new(Mutex::new(HashMap::default())),
19751983
expected_bank_hash: RwLock::new(None),

runtime/src/block_component_processor.rs

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,8 @@ pub struct BlockComponentProcessor {
2727
}
2828

2929
impl BlockComponentProcessor {
30-
pub fn finish(
31-
&self,
32-
migration_status: &MigrationStatus,
33-
) -> Result<(), BlockComponentProcessorError> {
34-
// Pre-migration: blocks with block components should be marked as dead
35-
if !migration_status.is_alpenglow_enabled() {
36-
match self.has_footer || self.has_header {
37-
false => return Ok(()),
38-
true => return Err(BlockComponentProcessorError::BlockComponentPreMigration),
39-
}
40-
}
41-
42-
// Post-migration: both header and footer are required
30+
fn on_final(&self) -> Result<(), BlockComponentProcessorError> {
31+
// Post-migration: both header and footer are required.
4332
if !self.has_footer {
4433
return Err(BlockComponentProcessorError::MissingBlockFooter);
4534
}
@@ -51,25 +40,64 @@ impl BlockComponentProcessor {
5140
Ok(())
5241
}
5342

43+
pub fn on_entry_batch(
44+
&mut self,
45+
migration_status: &MigrationStatus,
46+
is_final: bool,
47+
) -> Result<(), BlockComponentProcessorError> {
48+
if !migration_status.is_alpenglow_enabled() {
49+
return Ok(());
50+
}
51+
52+
// The block header must be the first component of each block.
53+
if !self.has_header {
54+
return Err(BlockComponentProcessorError::MissingBlockHeader);
55+
}
56+
57+
if is_final {
58+
self.on_final()
59+
} else {
60+
Ok(())
61+
}
62+
}
63+
5464
pub fn on_marker(
5565
&mut self,
5666
marker: &VersionedBlockMarker,
67+
migration_status: &MigrationStatus,
68+
is_final: bool,
5769
) -> Result<(), BlockComponentProcessorError> {
70+
// Pre-migration: blocks with block components should be marked as dead.
71+
if !migration_status.is_alpenglow_enabled() {
72+
return Err(BlockComponentProcessorError::BlockComponentPreMigration);
73+
}
74+
5875
let VersionedBlockMarker::V1(marker) = marker;
5976

6077
match marker {
6178
BlockMarkerV1::BlockFooter(footer) => self.on_footer(footer.inner()),
6279
BlockMarkerV1::BlockHeader(header) => self.on_header(header.inner()),
63-
// We process UpdateParent messages on shred ingest, so no callback needed here
80+
// We process UpdateParent messages on shred ingest, so no callback needed here.
6481
BlockMarkerV1::UpdateParent(_) => Ok(()),
6582
BlockMarkerV1::GenesisCertificate(_) => Ok(()),
83+
}?;
84+
85+
if is_final {
86+
self.on_final()
87+
} else {
88+
Ok(())
6689
}
6790
}
6891

6992
fn on_footer(
7093
&mut self,
7194
_footer: &VersionedBlockFooter,
7295
) -> Result<(), BlockComponentProcessorError> {
96+
// The block header must be the first component of each block.
97+
if !self.has_header {
98+
return Err(BlockComponentProcessorError::MissingBlockHeader);
99+
}
100+
73101
if self.has_footer {
74102
return Err(BlockComponentProcessorError::MultipleBlockFooters);
75103
}

0 commit comments

Comments
 (0)