gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

uses enum for shred type

Browse Source 
Current code is using u8 which does not have any type-safety and can
contain invalid values:
https://github.com/solana-labs/solana/blob/66fa062f1/ledger/src/shred.rs#L167

Checks for invalid shred-types are scattered through the code:
https://github.com/solana-labs/solana/blob/66fa062f1/ledger/src/blockstore.rs#L849-L851
https://github.com/solana-labs/solana/blob/66fa062f1/ledger/src/shred.rs#L346-L348

The commit uses enum for shred type with #[repr(u8)]. Backward
compatibility is maintained by implementing Serialize and Deserialize
compatible with u8, and adding a test to assert that.
This commit is contained in:
behzad nouri
2021-11-16 12:50:56 -05:00
parent bae5dae61d
commit 57057f8d39
8 changed files with 183 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
ledger/src/blockstore.rs
View File

@ -13,7 +13,7 @@ use {
erasure::ErasureConfig,
leader_schedule_cache::LeaderScheduleCache,
next_slots_iterator::NextSlotsIterator,
shred::{Result as ShredResult, Shred, Shredder, MAX_DATA_SHREDS_PER_FEC_BLOCK},
shred::{Result as ShredResult, Shred, ShredType, Shredder, MAX_DATA_SHREDS_PER_FEC_BLOCK},
},
bincode::deserialize,
log::*,
@ -803,52 +803,55 @@ impl Blockstore {
let mut newly_completed_data_sets: Vec<CompletedDataSetInfo> = vec![];
let mut inserted_indices = Vec::new();
for (i, (shred, is_repaired)) in shreds.into_iter().zip(is_repaired).enumerate() {
if shred.is_data() {
let shred_source = if is_repaired {
ShredSource::Repaired
} else {
ShredSource::Turbine
};
match self.check_insert_data_shred(
shred,
&mut erasure_metas,
&mut index_working_set,
&mut slot_meta_working_set,
&mut write_batch,
&mut just_inserted_data_shreds,
&mut index_meta_time,
is_trusted,
handle_duplicate,
leader_schedule,
shred_source,
) {
Err(InsertDataShredError::Exists) => metrics.num_data_shreds_exists += 1,
Err(InsertDataShredError::InvalidShred) => metrics.num_data_shreds_invalid += 1,
Err(InsertDataShredError::BlockstoreError(err)) => {
metrics.num_data_shreds_blockstore_error += 1;
error!("blockstore error: {}", err);
}
Ok(completed_data_sets) => {
newly_completed_data_sets.extend(completed_data_sets);
inserted_indices.push(i);
metrics.num_inserted += 1;
}
};
} else if shred.is_code() {
self.check_cache_coding_shred(
shred,
&mut erasure_metas,
&mut index_working_set,
&mut just_inserted_coding_shreds,
&mut index_meta_time,
handle_duplicate,
is_trusted,
is_repaired,
metrics,
);
} else {
panic!("There should be no other case");
}
match shred.shred_type() {
ShredType::Data => {
let shred_source = if is_repaired {
ShredSource::Repaired
} else {
ShredSource::Turbine
};
match self.check_insert_data_shred(
shred,
&mut erasure_metas,
&mut index_working_set,
&mut slot_meta_working_set,
&mut write_batch,
&mut just_inserted_data_shreds,
&mut index_meta_time,
is_trusted,
handle_duplicate,
leader_schedule,
shred_source,
) {
Err(InsertDataShredError::Exists) => metrics.num_data_shreds_exists += 1,
Err(InsertDataShredError::InvalidShred) => {
metrics.num_data_shreds_invalid += 1
}
Err(InsertDataShredError::BlockstoreError(err)) => {
metrics.num_data_shreds_blockstore_error += 1;
error!("blockstore error: {}", err);
}
Ok(completed_data_sets) => {
newly_completed_data_sets.extend(completed_data_sets);
inserted_indices.push(i);
metrics.num_inserted += 1;
}
};
}
ShredType::Code => {
self.check_cache_coding_shred(
shred,
&mut erasure_metas,
&mut index_working_set,
&mut just_inserted_coding_shreds,
&mut index_meta_time,
handle_duplicate,
is_trusted,
is_repaired,
metrics,
);
}
};
}
start.stop();
@ -3031,6 +3034,7 @@ impl Blockstore {
slot: u64,
index: u32,
new_shred_raw: &[u8],
// TODO: change arg type to ShredType.
is_data: bool,
) -> Option<Vec<u8>> {
let res = if is_data {

172
ledger/src/shred.rs
View File

@ -52,8 +52,10 @@
use {
crate::{blockstore::MAX_DATA_SHREDS_PER_SLOT, erasure::Session},
bincode::config::Options,
num_derive::FromPrimitive,
num_traits::FromPrimitive,
rayon::{prelude::*, ThreadPool},
serde::{Deserialize, Serialize},
serde::{Deserialize, Deserializer, Serialize, Serializer},
solana_entry::entry::{create_ticks, Entry},
solana_measure::measure::Measure,
solana_perf::packet::{limited_deserialize, Packet},
@ -131,10 +133,6 @@ thread_local!(static PAR_THREAD_POOL: RefCell<ThreadPool> = RefCell::new(rayon::
.build()
.unwrap()));
/// The constants that define if a shred is data or coding
pub const DATA_SHRED: u8 = 0b1010_0101;
pub const CODING_SHRED: u8 = 0b0101_1010;
pub const MAX_DATA_SHREDS_PER_FEC_BLOCK: u32 = 32;
pub const SHRED_TICK_REFERENCE_MASK: u8 = 0b0011_1111;
@ -163,11 +161,36 @@ pub enum ShredError {
pub type Result<T> = std::result::Result<T, ShredError>;
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, AbiExample, Deserialize, Serialize)]
pub struct ShredType(pub u8);
#[repr(u8)]
#[derive(Copy, Clone, Debug, Eq, FromPrimitive, Hash, PartialEq, AbiEnumVisitor, AbiExample)]
pub enum ShredType {
Data = 0b1010_0101,
Code = 0b0101_1010,
}
impl Default for ShredType {
fn default() -> Self {
ShredType(DATA_SHRED)
ShredType::Data
}
}
impl Serialize for ShredType {
fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
where
S: Serializer,
{
(*self as u8).serialize(serializer)
}
}
impl<'de> Deserialize<'de> for ShredType {
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let shred_type = u8::deserialize(deserializer)?;
Self::from_u8(shred_type)
.ok_or_else(|| serde::de::Error::custom(ShredError::InvalidShredType))
}
}
@ -319,32 +342,33 @@ impl Shred {
// so that erasure generation/recovery works correctly
// But only the data_header.size is stored in blockstore.
payload.resize(SHRED_PAYLOAD_SIZE, 0);
let shred = if common_header.shred_type == ShredType(CODING_SHRED) {
let coding_header: CodingShredHeader =
Self::deserialize_obj(&mut start, SIZE_OF_CODING_SHRED_HEADER, &payload)?;
Self {
common_header,
data_header: DataShredHeader::default(),
coding_header,
payload,
let shred = match common_header.shred_type {
ShredType::Code => {
let coding_header: CodingShredHeader =
Self::deserialize_obj(&mut start, SIZE_OF_CODING_SHRED_HEADER, &payload)?;
Self {
common_header,
data_header: DataShredHeader::default(),
coding_header,
payload,
}
}
} else if common_header.shred_type == ShredType(DATA_SHRED) {
let data_header: DataShredHeader =
Self::deserialize_obj(&mut start, SIZE_OF_DATA_SHRED_HEADER, &payload)?;
if u64::from(data_header.parent_offset) > common_header.slot {
return Err(ShredError::InvalidParentOffset {
slot,
parent_offset: data_header.parent_offset,
});
ShredType::Data => {
let data_header: DataShredHeader =
Self::deserialize_obj(&mut start, SIZE_OF_DATA_SHRED_HEADER, &payload)?;
if u64::from(data_header.parent_offset) > common_header.slot {
return Err(ShredError::InvalidParentOffset {
slot,
parent_offset: data_header.parent_offset,
});
}
Self {
common_header,
data_header,
coding_header: CodingShredHeader::default(),
payload,
}
}
Self {
common_header,
data_header,
coding_header: CodingShredHeader::default(),
payload,
}
} else {
return Err(ShredError::InvalidShredType);
};
Ok(shred)
@ -383,23 +407,22 @@ impl Shred {
&common_header,
)
.expect("Failed to write header into shred buffer");
if common_header.shred_type == ShredType(DATA_SHRED) {
Self::serialize_obj_into(
match common_header.shred_type {
ShredType::Data => Self::serialize_obj_into(
&mut start,
SIZE_OF_DATA_SHRED_HEADER,
&mut payload,
&data_header,
)
.expect("Failed to write data header into shred buffer");
} else if common_header.shred_type == ShredType(CODING_SHRED) {
Self::serialize_obj_into(
.expect("Failed to write data header into shred buffer"),
ShredType::Code => Self::serialize_obj_into(
&mut start,
SIZE_OF_CODING_SHRED_HEADER,
&mut payload,
&coding_header,
)
.expect("Failed to write data header into shred buffer");
}
.expect("Failed to write coding header into shred buffer"),
};
Shred {
common_header,
data_header,
@ -420,6 +443,7 @@ impl Shred {
self.common_header.slot
}
// TODO: This should return Option<Slot>
pub fn parent(&self) -> Slot {
if self.is_data() {
self.common_header.slot - u64::from(self.data_header.parent_offset)
@ -477,11 +501,16 @@ impl Shred {
}
}
#[inline]
pub fn shred_type(&self) -> ShredType {
self.common_header.shred_type
}
pub fn is_data(&self) -> bool {
self.common_header.shred_type == ShredType(DATA_SHRED)
self.shred_type() == ShredType::Data
}
pub fn is_code(&self) -> bool {
self.common_header.shred_type == ShredType(CODING_SHRED)
self.shred_type() == ShredType::Code
}
pub fn last_in_slot(&self) -> bool {
@ -745,7 +774,7 @@ impl Shredder {
version: u16,
) -> (ShredCommonHeader, CodingShredHeader) {
let header = ShredCommonHeader {
shred_type: ShredType(CODING_SHRED),
shred_type: ShredType::Code,
index,
slot,
version,
@ -965,7 +994,7 @@ pub struct ShredFetchStats {
pub fn get_shred_slot_index_type(
p: &Packet,
stats: &mut ShredFetchStats,
) -> Option<(Slot, u32, bool)> {
) -> Option<(Slot, u32, ShredType)> {
let index_start = OFFSET_OF_SHRED_INDEX;
let index_end = index_start + SIZE_OF_SHRED_INDEX;
let slot_start = OFFSET_OF_SHRED_SLOT;
@ -1004,14 +1033,14 @@ pub fn get_shred_slot_index_type(
}
}
let shred_type = p.data[OFFSET_OF_SHRED_TYPE];
if shred_type == DATA_SHRED || shred_type == CODING_SHRED {
return Some((slot, index, shred_type == DATA_SHRED));
} else {
stats.bad_shred_type += 1;
}
None
let shred_type = match ShredType::from_u8(p.data[OFFSET_OF_SHRED_TYPE]) {
None => {
stats.bad_shred_type += 1;
return None;
}
Some(shred_type) => shred_type,
};
Some((slot, index, shred_type))
}
pub fn max_ticks_per_n_shreds(num_shreds: u64, shred_data_size: Option<usize>) -> u64 {
@ -1166,7 +1195,7 @@ pub mod tests {
let mut data_shred_indexes = HashSet::new();
let mut coding_shred_indexes = HashSet::new();
for shred in data_shreds.iter() {
assert_eq!(shred.common_header.shred_type, ShredType(DATA_SHRED));
assert_eq!(shred.shred_type(), ShredType::Data);
let index = shred.common_header.index;
let is_last = index as u64 == num_expected_data_shreds - 1;
verify_test_data_shred(
@ -1185,7 +1214,7 @@ pub mod tests {
for shred in coding_shreds.iter() {
let index = shred.common_header.index;
assert_eq!(shred.common_header.shred_type, ShredType(CODING_SHRED));
assert_eq!(shred.shred_type(), ShredType::Code);
verify_test_code_shred(shred, index, slot, &keypair.pubkey(), true);
assert!(!coding_shred_indexes.contains(&index));
coding_shred_indexes.insert(index);
@ -1797,7 +1826,7 @@ pub mod tests {
shred.copy_to_packet(&mut packet);
let mut stats = ShredFetchStats::default();
let ret = get_shred_slot_index_type(&packet, &mut stats);
assert_eq!(Some((1, 3, true)), ret);
assert_eq!(Some((1, 3, ShredType::Data)), ret);
assert_eq!(stats, ShredFetchStats::default());
packet.meta.size = OFFSET_OF_SHRED_TYPE;
@ -1818,7 +1847,7 @@ pub mod tests {
packet.meta.size = OFFSET_OF_SHRED_INDEX + SIZE_OF_SHRED_INDEX;
assert_eq!(
Some((1, 3, true)),
Some((1, 3, ShredType::Data)),
get_shred_slot_index_type(&packet, &mut stats)
);
assert_eq!(stats.index_overrun, 4);
@ -1826,7 +1855,7 @@ pub mod tests {
let shred = Shred::new_empty_coding(8, 2, 10, 30, 4, 200);
shred.copy_to_packet(&mut packet);
assert_eq!(
Some((8, 2, false)),
Some((8, 2, ShredType::Code)),
get_shred_slot_index_type(&packet, &mut stats)
);
@ -1835,12 +1864,39 @@ pub mod tests {
assert_eq!(None, get_shred_slot_index_type(&packet, &mut stats));
assert_eq!(1, stats.index_out_of_bounds);
let (mut header, coding_header) = Shredder::new_coding_shred_header(8, 2, 10, 30, 4, 200);
header.shred_type = ShredType(u8::MAX);
let (header, coding_header) = Shredder::new_coding_shred_header(8, 2, 10, 30, 4, 200);
let shred = Shred::new_empty_from_header(header, DataShredHeader::default(), coding_header);
shred.copy_to_packet(&mut packet);
packet.data[OFFSET_OF_SHRED_TYPE] = u8::MAX;
assert_eq!(None, get_shred_slot_index_type(&packet, &mut stats));
assert_eq!(1, stats.bad_shred_type);
}
// Asserts that ShredType is backward compatible with u8.
#[test]
fn test_shred_type_compat() {
assert_eq!(std::mem::size_of::<ShredType>(), std::mem::size_of::<u8>());
assert_eq!(ShredType::from_u8(0), None);
assert_eq!(ShredType::from_u8(1), None);
assert_matches!(bincode::deserialize::<ShredType>(&[0u8]), Err(_));
// data shred
assert_eq!(ShredType::Data as u8, 0b1010_0101);
assert_eq!(ShredType::from_u8(0b1010_0101), Some(ShredType::Data));
let buf = bincode::serialize(&ShredType::Data).unwrap();
assert_eq!(buf, vec![0b1010_0101]);
assert_matches!(
bincode::deserialize::<ShredType>(&[0b1010_0101]),
Ok(ShredType::Data)
);
// coding shred
assert_eq!(ShredType::Code as u8, 0b0101_1010);
assert_eq!(ShredType::from_u8(0b0101_1010), Some(ShredType::Code));
let buf = bincode::serialize(&ShredType::Code).unwrap();
assert_eq!(buf, vec![0b0101_1010]);
assert_matches!(
bincode::deserialize::<ShredType>(&[0b0101_1010]),
Ok(ShredType::Code)
);
}
}