Skip to content

Commit

Permalink
adds feature to enable chained Merkle shreds (#34916)
Browse files Browse the repository at this point in the history 
During a cluster upgrade when only half of the cluster can ingest the new shred
variant, sending shreds of the new variant can cause nodes to diverge.
The commit adds a feature to enable chained Merkle shreds explicitly.
  • Loading branch information
@behzadnouri
behzadnouri authored Jan 27, 2024
1 parent 02062a6 commit d4fdcd9
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 4 deletions.
33 changes: 29 additions & 4 deletions core/src/shred_fetch_stage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,22 @@ impl ShredFetchStage {

// Limit shreds to 2 epochs away.
let max_slot = last_slot + 2 * slots_per_epoch;
let should_drop_legacy_shreds =
|shred_slot| should_drop_legacy_shreds(shred_slot, &feature_set, &epoch_schedule);
let should_drop_legacy_shreds = |shred_slot| {
check_feature_activation(
&feature_set::drop_legacy_shreds::id(),
shred_slot,
&feature_set,
&epoch_schedule,
)
};
let enable_chained_merkle_shreds = |shred_slot| {
check_feature_activation(
&feature_set::enable_chained_merkle_shreds::id(),
shred_slot,
&feature_set,
&epoch_schedule,
)
};
let turbine_disabled = turbine_disabled.load(Ordering::Relaxed);
for packet in packet_batch.iter_mut().filter(|p| !p.meta().discard()) {
if turbine_disabled
Expand All @@ -115,6 +129,7 @@ impl ShredFetchStage {
max_slot,
shred_version,
should_drop_legacy_shreds,
enable_chained_merkle_shreds,
&mut stats,
)
{
Expand Down Expand Up @@ -394,13 +409,15 @@ pub(crate) fn receive_repair_quic_packets(
}
}

// Returns true if the feature is effective for the shred slot.
#[must_use]
fn should_drop_legacy_shreds(
fn check_feature_activation(
feature: &Pubkey,
shred_slot: Slot,
feature_set: &FeatureSet,
epoch_schedule: &EpochSchedule,
) -> bool {
match feature_set.activated_slot(&feature_set::drop_legacy_shreds::id()) {
match feature_set.activated_slot(feature) {
None => false,
Some(feature_slot) => {
let feature_epoch = epoch_schedule.get_epoch(feature_slot);
Expand Down Expand Up @@ -451,6 +468,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats,
));
let coding = solana_ledger::shred::Shredder::generate_coding_shreds(
Expand All @@ -465,6 +483,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats,
));
}
Expand All @@ -487,6 +506,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats,
));
assert_eq!(stats.index_overrun, 1);
Expand All @@ -509,6 +529,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats,
));
assert_eq!(stats.slot_out_of_range, 1);
Expand All @@ -519,6 +540,7 @@ mod tests {
max_slot,
345, // shred_version
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats,
));
assert_eq!(stats.shred_version_mismatch, 1);
Expand All @@ -530,6 +552,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats,
));

Expand All @@ -552,6 +575,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats,
));

Expand All @@ -564,6 +588,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats,
));
}
Expand Down
18 changes: 18 additions & 0 deletions ledger/src/shred.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,7 @@ pub fn should_discard_shred(
max_slot: Slot,
shred_version: u16,
should_drop_legacy_shreds: impl Fn(Slot) -> bool,
enable_chained_merkle_shreds: impl Fn(Slot) -> bool,
stats: &mut ShredFetchStats,
) -> bool {
debug_assert!(root < max_slot);
Expand Down Expand Up @@ -999,13 +1000,19 @@ pub fn should_discard_shred(
stats.num_shreds_merkle_code = stats.num_shreds_merkle_code.saturating_add(1);
}
ShredVariant::MerkleCode(_, /*chained:*/ true) => {
if !enable_chained_merkle_shreds(slot) {
return true;
}
stats.num_shreds_merkle_code_chained =
stats.num_shreds_merkle_code_chained.saturating_add(1);
}
ShredVariant::MerkleData(_, /*chained:*/ false) => {
stats.num_shreds_merkle_data = stats.num_shreds_merkle_data.saturating_add(1);
}
ShredVariant::MerkleData(_, /*chained:*/ true) => {
if !enable_chained_merkle_shreds(slot) {
return true;
}
stats.num_shreds_merkle_data_chained =
stats.num_shreds_merkle_data_chained.saturating_add(1);
}
Expand Down Expand Up @@ -1209,6 +1216,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(stats, ShredFetchStats::default());
Expand All @@ -1220,6 +1228,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(stats.index_overrun, 1);
Expand All @@ -1231,6 +1240,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(stats.index_overrun, 2);
Expand All @@ -1242,6 +1252,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(stats.index_overrun, 3);
Expand All @@ -1253,6 +1264,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(stats.index_overrun, 4);
Expand All @@ -1264,6 +1276,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(stats.bad_parent_offset, 1);
Expand All @@ -1285,6 +1298,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));

Expand All @@ -1305,6 +1319,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(1, stats.index_out_of_bounds);
Expand All @@ -1326,6 +1341,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
packet.buffer_mut()[OFFSET_OF_SHRED_VARIANT] = u8::MAX;
Expand All @@ -1336,6 +1352,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(1, stats.bad_shred_type);
Expand All @@ -1348,6 +1365,7 @@ mod tests {
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
|_| true, // enable_chained_merkle_shreds
&mut stats
));
assert_eq!(1, stats.bad_shred_type);
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,10 @@ pub mod enable_gossip_duplicate_proof_ingestion {
solana_sdk::declare_id!("FNKCMBzYUdjhHyPdsKG2LSmdzH8TCHXn3ytj8RNBS4nG");
}

pub mod enable_chained_merkle_shreds {
solana_sdk::declare_id!("7uZBkJXJ1HkuP6R3MJfZs7mLwymBcDbKdqbF51ZWLier");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -965,6 +969,7 @@ lazy_static! {
(curve25519_restrict_msm_length::id(), "restrict curve25519 multiscalar multiplication vector lengths #34763"),
(cost_model_requested_write_lock_cost::id(), "cost model uses number of requested write locks #34819"),
(enable_gossip_duplicate_proof_ingestion::id(), "enable gossip duplicate proof ingestion #32963"),
(enable_chained_merkle_shreds::id(), "Enable chained Merkle shreds #34916"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down

0 comments on commit d4fdcd9

Please sign in to comment.