Skip to content

Commit

Permalink
adds feature gated code to drop legacy shreds (#34328)
Browse files Browse the repository at this point in the history
  • Loading branch information
behzadnouri authored Dec 6, 2023
1 parent d21e7ed commit d5eee01
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 5 deletions.
57 changes: 53 additions & 4 deletions core/src/shred_fetch_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use {
solana_perf::packet::{PacketBatch, PacketBatchRecycler, PacketFlags, PACKETS_PER_BATCH},
solana_runtime::bank_forks::BankForks,
solana_sdk::{
clock::DEFAULT_MS_PER_SLOT,
clock::{Slot, DEFAULT_MS_PER_SLOT},
epoch_schedule::EpochSchedule,
feature_set::{self, FeatureSet},
packet::{Meta, PACKET_DATA_SIZE},
pubkey::Pubkey,
},
Expand Down Expand Up @@ -50,12 +52,20 @@ impl ShredFetchStage {
.as_ref()
.map(|(_, cluster_info)| cluster_info.keypair().clone());

let (mut last_root, mut slots_per_epoch, mut last_slot) = {
let (
mut last_root,
mut slots_per_epoch,
mut feature_set,
mut epoch_schedule,
mut last_slot,
) = {
let bank_forks_r = bank_forks.read().unwrap();
let root_bank = bank_forks_r.root_bank();
(
root_bank.slot(),
root_bank.get_slots_in_epoch(root_bank.epoch()),
root_bank.feature_set.clone(),
root_bank.epoch_schedule().clone(),
bank_forks_r.highest_slot(),
)
};
Expand All @@ -69,6 +79,8 @@ impl ShredFetchStage {
last_slot = bank_forks_r.highest_slot();
bank_forks_r.root_bank()
};
feature_set = root_bank.feature_set.clone();
epoch_schedule = root_bank.epoch_schedule().clone();
last_root = root_bank.slot();
slots_per_epoch = root_bank.get_slots_in_epoch(root_bank.epoch());
keypair = repair_context
Expand All @@ -92,10 +104,19 @@ 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 turbine_disabled = turbine_disabled.load(Ordering::Relaxed);
for packet in packet_batch.iter_mut().filter(|p| !p.meta().discard()) {
if turbine_disabled
|| should_discard_shred(packet, last_root, max_slot, shred_version, &mut stats)
|| should_discard_shred(
packet,
last_root,
max_slot,
shred_version,
should_drop_legacy_shreds,
&mut stats,
)
{
packet.meta_mut().set_discard(true);
} else {
Expand Down Expand Up @@ -373,6 +394,22 @@ pub(crate) fn receive_repair_quic_packets(
}
}

#[must_use]
fn should_drop_legacy_shreds(
shred_slot: Slot,
feature_set: &FeatureSet,
epoch_schedule: &EpochSchedule,
) -> bool {
match feature_set.activated_slot(&feature_set::drop_legacy_shreds::id()) {
None => false,
Some(feature_slot) => {
let feature_epoch = epoch_schedule.get_epoch(feature_slot);
let shred_epoch = epoch_schedule.get_epoch(shred_slot);
feature_epoch < shred_epoch
}
}
}

#[cfg(test)]
mod tests {
use {
Expand Down Expand Up @@ -413,6 +450,7 @@ mod tests {
last_root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats,
));
let coding = solana_ledger::shred::Shredder::generate_coding_shreds(
Expand All @@ -426,6 +464,7 @@ mod tests {
last_root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats,
));
}
Expand All @@ -447,6 +486,7 @@ mod tests {
last_root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats,
));
assert_eq!(stats.index_overrun, 1);
Expand All @@ -468,12 +508,18 @@ mod tests {
3,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats,
));
assert_eq!(stats.slot_out_of_range, 1);

assert!(should_discard_shred(
&packet, last_root, max_slot, /*shred_version:*/ 345, &mut stats,
&packet,
last_root,
max_slot,
345, // shred_version
|_| false, // should_drop_legacy_shreds
&mut stats,
));
assert_eq!(stats.shred_version_mismatch, 1);

Expand All @@ -483,6 +529,7 @@ mod tests {
last_root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats,
));

Expand All @@ -504,6 +551,7 @@ mod tests {
last_root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats,
));

Expand All @@ -515,6 +563,7 @@ mod tests {
last_root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats,
));
}
Expand Down
18 changes: 17 additions & 1 deletion ledger/src/shred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,7 @@ pub fn should_discard_shred(
root: Slot,
max_slot: Slot,
shred_version: u16,
should_drop_legacy_shreds: impl Fn(Slot) -> bool,
stats: &mut ShredFetchStats,
) -> bool {
debug_assert!(root < max_slot);
Expand Down Expand Up @@ -969,7 +970,11 @@ pub fn should_discard_shred(
}
}
match shred_variant {
ShredVariant::LegacyCode | ShredVariant::LegacyData => (),
ShredVariant::LegacyCode | ShredVariant::LegacyData => {
if should_drop_legacy_shreds(slot) {
return true;
}
}
ShredVariant::MerkleCode(_) => {
stats.num_shreds_merkle_code = stats.num_shreds_merkle_code.saturating_add(1);
}
Expand Down Expand Up @@ -1173,6 +1178,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(stats, ShredFetchStats::default());
Expand All @@ -1183,6 +1189,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(stats.index_overrun, 1);
Expand All @@ -1193,6 +1200,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(stats.index_overrun, 2);
Expand All @@ -1203,6 +1211,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(stats.index_overrun, 3);
Expand All @@ -1213,6 +1222,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(stats.index_overrun, 4);
Expand All @@ -1223,6 +1233,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(stats.bad_parent_offset, 1);
Expand All @@ -1243,6 +1254,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));

Expand All @@ -1262,6 +1274,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(1, stats.index_out_of_bounds);
Expand All @@ -1282,6 +1295,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
packet.buffer_mut()[OFFSET_OF_SHRED_VARIANT] = u8::MAX;
Expand All @@ -1291,6 +1305,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(1, stats.bad_shred_type);
Expand All @@ -1302,6 +1317,7 @@ mod tests {
root,
max_slot,
shred_version,
|_| false, // should_drop_legacy_shreds
&mut stats
));
assert_eq!(1, stats.bad_shred_type);
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,10 @@ pub mod enable_zk_transfer_with_fee {
solana_sdk::declare_id!("zkNLP7EQALfC1TYeB3biDU7akDckj8iPkvh9y2Mt2K3");
}

pub mod drop_legacy_shreds {
solana_sdk::declare_id!("GV49KKQdBNaiv2pgqhS2Dy3GWYJGXMTVYbYkdk91orRy");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -910,6 +914,7 @@ lazy_static! {
(validate_fee_collector_account::id(), "validate fee collector account #33888"),
(disable_rent_fees_collection::id(), "Disable rent fees collection #33945"),
(enable_zk_transfer_with_fee::id(), "enable Zk Token proof program transfer with fee"),
(drop_legacy_shreds::id(), "drops legacy shreds #34328"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down

0 comments on commit d5eee01

Please sign in to comment.