From 4f947a0db3ff9a1622fc50527134ea7617860da8 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Mon, 2 May 2022 15:02:06 +0000 Subject: [PATCH] (de-)serializes shred headers through a Cursor (#24876) Current serialize/de-serialize code for shreds manually tracks offsets into the payload; This can instead be done with std::io::Cursor. https://github.com/solana-labs/solana/blob/e812430e2/ledger/src/shred.rs#L232-L258 --- ledger/src/shred.rs | 177 +++++++++++++++++++------------------------- 1 file changed, 76 insertions(+), 101 deletions(-) diff --git a/ledger/src/shred.rs b/ledger/src/shred.rs index e26a89cd456905..65af685ac5fa52 100644 --- a/ledger/src/shred.rs +++ b/ledger/src/shred.rs @@ -55,7 +55,6 @@ pub use crate::{ }; use { crate::blockstore::MAX_DATA_SHREDS_PER_SLOT, - bincode::config::Options, bitflags::bitflags, num_enum::{IntoPrimitive, TryFromPrimitive}, serde::{Deserialize, Serialize}, @@ -68,7 +67,7 @@ use { pubkey::Pubkey, signature::{Keypair, Signature, Signer}, }, - std::{fmt::Debug, mem::size_of, ops::RangeInclusive}, + std::{fmt::Debug, io::Cursor, mem::size_of, ops::RangeInclusive}, thiserror::Error, }; @@ -229,34 +228,6 @@ impl ErasureSetId { } impl Shred { - fn deserialize_obj<'de, T>(index: &mut usize, size: usize, buf: &'de [u8]) -> bincode::Result - where - T: Deserialize<'de>, - { - let end = std::cmp::min(*index + size, buf.len()); - let ret = bincode::options() - .with_limit(PACKET_DATA_SIZE as u64) - .with_fixint_encoding() - .allow_trailing_bytes() - .deserialize(&buf[*index..end])?; - *index += size; - Ok(ret) - } - - fn serialize_obj_into<'de, T>( - index: &mut usize, - size: usize, - buf: &'de mut [u8], - obj: &T, - ) -> bincode::Result<()> - where - T: Serialize, - { - bincode::serialize_into(&mut buf[*index..*index + size], obj)?; - *index += size; - Ok(()) - } - pub fn copy_to_packet(&self, packet: &mut Packet) { let len = self.payload.len(); packet.data[..len].copy_from_slice(&self.payload[..]); @@ -306,24 +277,13 @@ impl Shred { data_header.flags |= ShredFlags::LAST_SHRED_IN_SLOT } - let mut start = 0; - Self::serialize_obj_into( - &mut start, - SIZE_OF_COMMON_SHRED_HEADER, - &mut payload, - &common_header, - ) - .expect("Failed to write common header into shred buffer"); - - Self::serialize_obj_into( - &mut start, - SIZE_OF_DATA_SHRED_HEADER, - &mut payload, - &data_header, - ) - .expect("Failed to write data header into shred buffer"); + let mut cursor = Cursor::new(&mut payload[..]); + bincode::serialize_into(&mut cursor, &common_header).unwrap(); + bincode::serialize_into(&mut cursor, &data_header).unwrap(); // TODO: Need to check if data is too large! - payload[start..start + data.len()].copy_from_slice(data); + let offset = cursor.position() as usize; + debug_assert_eq!(offset, SHRED_DATA_OFFSET); + payload[offset..offset + data.len()].copy_from_slice(data); Self { common_header, data_header, @@ -333,23 +293,19 @@ impl Shred { } pub fn new_from_serialized_shred(mut payload: Vec) -> Result { - let mut start = 0; - let common_header: ShredCommonHeader = - Self::deserialize_obj(&mut start, SIZE_OF_COMMON_SHRED_HEADER, &payload)?; - - // Shreds should be padded out to SHRED_PAYLOAD_SIZE - // so that erasure generation/recovery works correctly - // But only the data_header.size is stored in blockstore. - payload.resize(SHRED_PAYLOAD_SIZE, 0); + let mut cursor = Cursor::new(&payload[..]); + let common_header: ShredCommonHeader = bincode::deserialize_from(&mut cursor)?; let (data_header, coding_header) = match common_header.shred_type { ShredType::Code => { - let coding_header: CodingShredHeader = - Self::deserialize_obj(&mut start, SIZE_OF_CODING_SHRED_HEADER, &payload)?; + let coding_header = bincode::deserialize_from(&mut cursor)?; + // see: https://github.com/solana-labs/solana/pull/10109 + payload.truncate(SHRED_PAYLOAD_SIZE); (DataShredHeader::default(), coding_header) } ShredType::Data => { - let data_header: DataShredHeader = - Self::deserialize_obj(&mut start, SIZE_OF_DATA_SHRED_HEADER, &payload)?; + let data_header = bincode::deserialize_from(&mut cursor)?; + // see: https://github.com/solana-labs/solana/pull/16602 + payload.resize(SHRED_PAYLOAD_SIZE, 0u8); (data_header, CodingShredHeader::default()) } }; @@ -386,24 +342,14 @@ impl Shred { position, }; let mut payload = vec![0; SHRED_PAYLOAD_SIZE]; - let mut start = 0; - Self::serialize_obj_into( - &mut start, - SIZE_OF_COMMON_SHRED_HEADER, - &mut payload, - &common_header, - ) - .expect("Failed to write header into shred buffer"); - Self::serialize_obj_into( - &mut start, - SIZE_OF_CODING_SHRED_HEADER, - &mut payload, - &coding_header, - ) - .expect("Failed to write coding header into shred buffer"); + let mut cursor = Cursor::new(&mut payload[..]); + bincode::serialize_into(&mut cursor, &common_header).unwrap(); + bincode::serialize_into(&mut cursor, &coding_header).unwrap(); // Tests may have an empty parity_shard. if !parity_shard.is_empty() { - payload[SIZE_OF_CODING_SHRED_HEADERS..].copy_from_slice(parity_shard); + let offset = cursor.position() as usize; + debug_assert_eq!(offset, SIZE_OF_CODING_SHRED_HEADERS); + payload[offset..].copy_from_slice(parity_shard); } Shred { common_header, @@ -491,9 +437,14 @@ impl Shred { }, }; match shred_type { - ShredType::Code => Ok(shred), + ShredType::Code => { + if shred.len() != SHRED_PAYLOAD_SIZE { + return Err(Error::InvalidPayloadSize(shred.len())); + } + Ok(shred) + } ShredType::Data => { - if shred.len() > SHRED_PAYLOAD_SIZE { + if !(SHRED_DATA_OFFSET..SHRED_PAYLOAD_SIZE).contains(&shred.len()) { return Err(Error::InvalidPayloadSize(shred.len())); } shred.resize(SHRED_PAYLOAD_SIZE, 0u8); @@ -627,24 +578,12 @@ impl Shred { pub fn set_index(&mut self, index: u32) { self.common_header.index = index; - Self::serialize_obj_into( - &mut 0, - SIZE_OF_COMMON_SHRED_HEADER, - &mut self.payload, - &self.common_header, - ) - .unwrap(); + bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap(); } pub fn set_slot(&mut self, slot: Slot) { self.common_header.slot = slot; - Self::serialize_obj_into( - &mut 0, - SIZE_OF_COMMON_SHRED_HEADER, - &mut self.payload, - &self.common_header, - ) - .unwrap(); + bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap(); } pub fn signature(&self) -> Signature { @@ -695,6 +634,8 @@ impl Shred { if self.is_data() { self.data_header.flags |= ShredFlags::LAST_SHRED_IN_SLOT } + let buffer = &mut self.payload[SIZE_OF_COMMON_SHRED_HEADER..]; + bincode::serialize_into(buffer, &self.data_header).unwrap(); } #[cfg(test)] @@ -704,17 +645,9 @@ impl Shred { .flags .remove(ShredFlags::DATA_COMPLETE_SHRED); } - // Data header starts after the shred common header - let mut start = SIZE_OF_COMMON_SHRED_HEADER; - let size_of_data_shred_header = SIZE_OF_DATA_SHRED_HEADER; - Self::serialize_obj_into( - &mut start, - size_of_data_shred_header, - &mut self.payload, - &self.data_header, - ) - .expect("Failed to write data header into shred buffer"); + let buffer = &mut self.payload[SIZE_OF_COMMON_SHRED_HEADER..]; + bincode::serialize_into(buffer, &self.data_header).unwrap(); } pub fn data_complete(&self) -> bool { @@ -1276,6 +1209,48 @@ mod tests { ); } + #[test] + fn test_serde_compat_shred_data_empty() { + const SEED: &str = "E3M5hm8yAEB7iPhQxFypAkLqxNeZCTuGBDMa8Jdrghoo"; + const PAYLOAD: &str = "nRNFVBEsV9FEM5KfmsCXJsgELRSkCV55drTavdy5aZPnsp\ + B8WvsgY99ZuNHDnwkrqe6Lx7ARVmercwugR5HwDcLA9ivKMypk9PNucDPLs67TXWy6k9R\ + ozKmy"; + let mut rng = { + let seed = <[u8; 32]>::try_from(bs58_decode(SEED)).unwrap(); + ChaChaRng::from_seed(seed) + }; + let keypair = Keypair::generate(&mut rng); + let mut shred = Shred::new_from_data( + 142076266, // slot + 21443, // index + 51279, // parent_offset + &[], // data + true, // is_last_data + false, // is_last_in_slot + 49, // reference_tick + 59445, // version + 21414, // fec_set_index + ); + shred.sign(&keypair); + assert!(shred.verify(&keypair.pubkey())); + assert_matches!(shred.sanitize(), Ok(())); + let payload = bs58_decode(PAYLOAD); + let mut packet = Packet::default(); + packet.data[..payload.len()].copy_from_slice(&payload); + packet.meta.size = payload.len(); + assert_eq!(shred.bytes_to_store(), payload); + assert_eq!(shred, Shred::new_from_serialized_shred(payload).unwrap()); + assert_eq!( + shred.reference_tick(), + Shred::reference_tick_from_data(&packet.data) + ); + assert_eq!(Shred::get_slot_from_packet(&packet), Some(shred.slot())); + assert_eq!( + get_shred_slot_index_type(&packet, &mut ShredFetchStats::default()), + Some((shred.slot(), shred.index(), shred.shred_type())) + ); + } + #[test] fn test_serde_compat_shred_code() { const SEED: &str = "4jfjh3UZVyaEgvyG9oQmNyFY9yHDmbeH9eUhnBKkrcrN";