defines shred flags using bitflags crate (#24874)

Shred flags uses raw bit-masking ops which lacks type-safety:
https://github.com/solana-labs/solana/blob/a829ddc92/ledger/src/shred.rs#L112-L114

This commit instead uses bitflags crate to define shred flags.
This commit is contained in:
behzad nouri 2022-05-01 19:25:15 +00:00 committed by GitHub
parent a829ddc922
commit e812430e28
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 86 additions and 21 deletions

View File

@ -10,8 +10,7 @@ use {
},
solana_entry::entry::Entry,
solana_ledger::shred::{
ProcessShredsStats, Shred, Shredder, MAX_DATA_SHREDS_PER_FEC_BLOCK,
SHRED_TICK_REFERENCE_MASK,
ProcessShredsStats, Shred, ShredFlags, Shredder, MAX_DATA_SHREDS_PER_FEC_BLOCK,
},
solana_sdk::{
signature::Keypair,
@ -63,6 +62,7 @@ impl StandardBroadcastRun {
max_ticks_in_slot: u8,
stats: &mut ProcessShredsStats,
) -> Vec<Shred> {
const SHRED_TICK_REFERENCE_MASK: u8 = ShredFlags::SHRED_TICK_REFERENCE_MASK.bits();
let (current_slot, _) = self.current_slot_and_parent.unwrap();
match self.unfinished_slot {
None => Vec::default(),

View File

@ -56,6 +56,7 @@ pub use crate::{
use {
crate::blockstore::MAX_DATA_SHREDS_PER_SLOT,
bincode::config::Options,
bitflags::bitflags,
num_enum::{IntoPrimitive, TryFromPrimitive},
serde::{Deserialize, Serialize},
solana_entry::entry::{create_ticks, Entry},
@ -109,9 +110,14 @@ const ENCODED_PAYLOAD_SIZE: usize = SHRED_PAYLOAD_SIZE - SIZE_OF_CODING_SHRED_HE
pub const MAX_DATA_SHREDS_PER_FEC_BLOCK: u32 = 32;
pub const SHRED_TICK_REFERENCE_MASK: u8 = 0b0011_1111;
const LAST_SHRED_IN_SLOT: u8 = 0b1000_0000;
const DATA_COMPLETE_SHRED: u8 = 0b0100_0000;
bitflags! {
#[derive(Default, Serialize, Deserialize)]
pub struct ShredFlags:u8 {
const SHRED_TICK_REFERENCE_MASK = 0b0011_1111;
const DATA_COMPLETE_SHRED = 0b0100_0000;
const LAST_SHRED_IN_SLOT = 0b1000_0000;
}
}
#[derive(Debug, Error)]
pub enum Error {
@ -173,7 +179,7 @@ struct ShredCommonHeader {
#[derive(Clone, Copy, Debug, Default, PartialEq, Deserialize, Serialize)]
struct DataShredHeader {
parent_offset: u16,
flags: u8,
flags: ShredFlags,
size: u16, // common shred header + data shred header + data
}
@ -282,16 +288,22 @@ impl Shred {
let size = (data.len() + SIZE_OF_DATA_SHRED_HEADER + SIZE_OF_COMMON_SHRED_HEADER) as u16;
let mut data_header = DataShredHeader {
parent_offset,
flags: reference_tick.min(SHRED_TICK_REFERENCE_MASK),
flags: unsafe {
ShredFlags::from_bits_unchecked(
ShredFlags::SHRED_TICK_REFERENCE_MASK
.bits()
.min(reference_tick),
)
},
size,
};
if is_last_data {
data_header.flags |= DATA_COMPLETE_SHRED
data_header.flags |= ShredFlags::DATA_COMPLETE_SHRED
}
if is_last_in_slot {
data_header.flags |= LAST_SHRED_IN_SLOT
data_header.flags |= ShredFlags::LAST_SHRED_IN_SLOT
}
let mut start = 0;
@ -669,7 +681,9 @@ impl Shred {
pub fn last_in_slot(&self) -> bool {
if self.is_data() {
self.data_header.flags & LAST_SHRED_IN_SLOT == LAST_SHRED_IN_SLOT
self.data_header
.flags
.contains(ShredFlags::LAST_SHRED_IN_SLOT)
} else {
false
}
@ -679,14 +693,16 @@ impl Shred {
/// Use this only for test code which doesn't care about actual shred
pub fn set_last_in_slot(&mut self) {
if self.is_data() {
self.data_header.flags |= LAST_SHRED_IN_SLOT
self.data_header.flags |= ShredFlags::LAST_SHRED_IN_SLOT
}
}
#[cfg(test)]
pub(crate) fn unset_data_complete(&mut self) {
if self.is_data() {
self.data_header.flags &= !DATA_COMPLETE_SHRED;
self.data_header
.flags
.remove(ShredFlags::DATA_COMPLETE_SHRED);
}
// Data header starts after the shred common header
@ -703,7 +719,9 @@ impl Shred {
pub fn data_complete(&self) -> bool {
if self.is_data() {
self.data_header.flags & DATA_COMPLETE_SHRED == DATA_COMPLETE_SHRED
self.data_header
.flags
.contains(ShredFlags::DATA_COMPLETE_SHRED)
} else {
false
}
@ -711,10 +729,11 @@ impl Shred {
pub(crate) fn reference_tick(&self) -> u8 {
if self.is_data() {
self.data_header.flags & SHRED_TICK_REFERENCE_MASK
self.data_header.flags & ShredFlags::SHRED_TICK_REFERENCE_MASK
} else {
SHRED_TICK_REFERENCE_MASK
ShredFlags::SHRED_TICK_REFERENCE_MASK
}
.bits()
}
// Get slot from a shred packet with partial deserialize
@ -733,7 +752,7 @@ impl Shred {
let flags = data[SIZE_OF_COMMON_SHRED_HEADER + SIZE_OF_DATA_SHRED_HEADER
- size_of::<u8>()
- size_of::<u16>()];
flags & SHRED_TICK_REFERENCE_MASK
flags & ShredFlags::SHRED_TICK_REFERENCE_MASK.bits()
}
pub fn verify(&self, pubkey: &Pubkey) -> bool {
@ -1300,4 +1319,47 @@ mod tests {
Some((shred.slot(), shred.index(), shred.shred_type()))
);
}
#[test]
fn test_shred_flags() {
fn make_shred(is_last_data: bool, is_last_in_slot: bool, reference_tick: u8) -> Shred {
Shred::new_from_data(
0, // slot
0, // index
0, // parent_offset
&[], // data
is_last_data,
is_last_in_slot,
reference_tick,
0, // version
0, // fec_set_index
)
}
fn check_shred_flags(
shred: &Shred,
is_last_data: bool,
is_last_in_slot: bool,
reference_tick: u8,
) {
assert_eq!(shred.data_complete(), is_last_data);
assert_eq!(shred.last_in_slot(), is_last_in_slot);
assert_eq!(shred.reference_tick(), reference_tick.min(63u8));
assert_eq!(
Shred::reference_tick_from_data(&shred.payload),
reference_tick.min(63u8),
);
}
for is_last_data in [false, true] {
for is_last_in_slot in [false, true] {
for reference_tick in [0, 37, 63, 64, 80, 128, 255] {
let mut shred = make_shred(is_last_data, is_last_in_slot, reference_tick);
check_shred_flags(&shred, is_last_data, is_last_in_slot, reference_tick);
shred.set_last_in_slot();
check_shred_flags(&shred, is_last_data, true, reference_tick);
shred.unset_data_complete();
check_shred_flags(&shred, false, true, reference_tick);
}
}
}
}
}

View File

@ -351,8 +351,8 @@ mod tests {
use {
super::*,
crate::shred::{
max_entries_per_n_shred, max_ticks_per_n_shreds, verify_test_data_shred, ShredType,
SHRED_TICK_REFERENCE_MASK,
max_entries_per_n_shred, max_ticks_per_n_shreds, verify_test_data_shred, ShredFlags,
ShredType,
},
bincode::serialized_size,
matches::assert_matches,
@ -549,10 +549,13 @@ mod tests {
0, // next_code_index
);
data_shreds.iter().for_each(|s| {
assert_eq!(s.reference_tick(), SHRED_TICK_REFERENCE_MASK);
assert_eq!(
s.reference_tick(),
ShredFlags::SHRED_TICK_REFERENCE_MASK.bits()
);
assert_eq!(
Shred::reference_tick_from_data(s.payload()),
SHRED_TICK_REFERENCE_MASK
ShredFlags::SHRED_TICK_REFERENCE_MASK.bits()
);
});
@ -561,7 +564,7 @@ mod tests {
.unwrap();
assert_eq!(
deserialized_shred.reference_tick(),
SHRED_TICK_REFERENCE_MASK
ShredFlags::SHRED_TICK_REFERENCE_MASK.bits(),
);
}