From 269f41db57a55a40f3f5d6ee3ee76e37a6bdd52b Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Fri, 22 Nov 2024 20:36:24 +0000 Subject: [PATCH] pr feedback: zerocopy from proof account to proof data struct --- slashing/program/src/duplicate_block_proof.rs | 80 ++++++++++++++----- slashing/program/src/error.rs | 8 ++ slashing/program/src/lib.rs | 2 +- slashing/program/src/processor.rs | 25 +++--- slashing/program/src/shred.rs | 57 +++++++------ slashing/program/src/state.rs | 7 +- .../program/tests/duplicate_block_proof.rs | 37 ++++----- 7 files changed, 135 insertions(+), 81 deletions(-) diff --git a/slashing/program/src/duplicate_block_proof.rs b/slashing/program/src/duplicate_block_proof.rs index b00c4f8b597..90215fd30d9 100644 --- a/slashing/program/src/duplicate_block_proof.rs +++ b/slashing/program/src/duplicate_block_proof.rs @@ -1,22 +1,40 @@ +//! Duplicate block proof data and verification use { crate::{ error::SlashingError, shred::{Shred, ShredType}, state::{ProofType, SlashingProofData}, }, - serde_derive::{Deserialize, Serialize}, + bytemuck::try_from_bytes, solana_program::{clock::Slot, msg, pubkey::Pubkey}, + spl_pod::primitives::PodU32, }; -#[derive(Deserialize, Serialize)] -pub struct DuplicateBlockProofData { - #[serde(with = "serde_bytes")] - pub shred1: Vec, - #[serde(with = "serde_bytes")] - pub shred2: Vec, +const LENGTH_SIZE: usize = std::mem::size_of::(); + +/// Proof of a duplicate block violation +pub struct DuplicateBlockProofData<'a> { + /// Shred signed by a leader + pub shred1: &'a [u8], + /// Conflicting shred signed by the same leader + pub shred2: &'a [u8], +} + +impl<'a> DuplicateBlockProofData<'a> { + #[allow(dead_code)] + /// Packs proof data to write in account for + /// `SlashingInstruction::DuplicateBlockProof` + pub fn pack(self) -> Vec { + let mut buf = vec![]; + buf.extend_from_slice(&(self.shred1.len() as u32).to_le_bytes()); + buf.extend_from_slice(self.shred1); + buf.extend_from_slice(&(self.shred2.len() as u32).to_le_bytes()); + buf.extend_from_slice(self.shred2); + buf + } } -impl SlashingProofData for DuplicateBlockProofData { +impl<'a> SlashingProofData<'a> for DuplicateBlockProofData<'a> { const PROOF_TYPE: ProofType = ProofType::DuplicateBlockProof; fn verify_proof(self, slot: Slot, _node_pubkey: &Pubkey) -> Result<(), SlashingError> { @@ -30,6 +48,32 @@ impl SlashingProofData for DuplicateBlockProofData { let shred2 = Shred::new_from_payload(self.shred2)?; check_shreds(slot, &shred1, &shred2) } + + fn unpack(data: &'a [u8]) -> Result + where + Self: Sized, + { + if data.len() < LENGTH_SIZE { + return Err(SlashingError::ProofBufferTooSmall); + } + let (length1, data) = data.split_at(LENGTH_SIZE); + let shred1_length = try_from_bytes::(length1) + .map_err(|_| SlashingError::ProofBufferDeserializationError)?; + let (shred1, data) = data.split_at(u32::from(*shred1_length) as usize); + + if data.len() < LENGTH_SIZE { + return Err(SlashingError::ProofBufferTooSmall); + } + let (length2, shred2) = data.split_at(LENGTH_SIZE); + let shred2_length = try_from_bytes::(length2) + .map_err(|_| SlashingError::ProofBufferDeserializationError)?; + + if shred2.len() < u32::from(*shred2_length) as usize { + return Err(SlashingError::ProofBufferTooSmall); + } + + Ok(Self { shred1, shred2 }) + } } /// Check that `shred1` and `shred2` indicate a valid duplicate proof @@ -194,21 +238,19 @@ mod tests { SIZE_OF_SIGNATURE, }, rand::Rng, - solana_ledger::{ - blockstore_meta::DuplicateSlotProof as SolanaDuplicateSlotProof, - shred::{Shred as SolanaShred, Shredder}, - }, + solana_ledger::shred::{Shred as SolanaShred, Shredder}, solana_sdk::signature::{Keypair, Signature, Signer}, std::sync::Arc, }; - fn generate_proof_data(shred1: &SolanaShred, shred2: &SolanaShred) -> DuplicateBlockProofData { - let duplicate_proof = SolanaDuplicateSlotProof { - shred1: shred1.payload().clone(), - shred2: shred2.payload().clone(), - }; - let data = bincode::serialize(&duplicate_proof).unwrap(); - bincode::deserialize(&data).unwrap() + fn generate_proof_data<'a>( + shred1: &'a SolanaShred, + shred2: &'a SolanaShred, + ) -> DuplicateBlockProofData<'a> { + DuplicateBlockProofData { + shred1: shred1.payload().as_slice(), + shred2: shred2.payload().as_slice(), + } } #[test] diff --git a/slashing/program/src/error.rs b/slashing/program/src/error.rs index 5d928feb808..0c6d2b4ec3b 100644 --- a/slashing/program/src/error.rs +++ b/slashing/program/src/error.rs @@ -49,6 +49,14 @@ pub enum SlashingError { #[error("Legacy shreds are not eligible for slashing")] LegacyShreds, + /// Unable to deserialize proof buffer + #[error("Proof buffer deserialization error")] + ProofBufferDeserializationError, + + /// Proof buffer is too small + #[error("Proof buffer too small")] + ProofBufferTooSmall, + /// Shred deserialization error #[error("Deserialization error")] ShredDeserializationError, diff --git a/slashing/program/src/lib.rs b/slashing/program/src/lib.rs index 4835032fdd8..24b7343ab09 100644 --- a/slashing/program/src/lib.rs +++ b/slashing/program/src/lib.rs @@ -1,7 +1,7 @@ //! Slashing program #![deny(missing_docs)] -mod duplicate_block_proof; +pub mod duplicate_block_proof; mod entrypoint; pub mod error; pub mod instruction; diff --git a/slashing/program/src/processor.rs b/slashing/program/src/processor.rs index 31c1c2d746b..bc4d8e2ee62 100644 --- a/slashing/program/src/processor.rs +++ b/slashing/program/src/processor.rs @@ -10,7 +10,6 @@ use { }, state::SlashingProofData, }, - serde::Deserialize, solana_program::{ account_info::{next_account_info, AccountInfo}, clock::{Slot, DEFAULT_SLOTS_PER_EPOCH}, @@ -24,7 +23,7 @@ use { fn verify_proof_data<'a, T>(slot: Slot, pubkey: &Pubkey, proof_data: &'a [u8]) -> ProgramResult where - T: SlashingProofData + Deserialize<'a>, + T: SlashingProofData<'a>, { // Statue of limitations is 1 epoch let clock = Clock::get()?; @@ -36,7 +35,7 @@ where } let proof_data: T = - bincode::deserialize(proof_data).map_err(|_| SlashingError::ShredDeserializationError)?; + T::unpack(proof_data).map_err(|_| SlashingError::ShredDeserializationError)?; SlashingProofData::verify_proof(proof_data, slot, pubkey)?; @@ -83,7 +82,7 @@ mod tests { shred::tests::new_rand_data_shred, }, rand::Rng, - solana_ledger::{blockstore_meta::DuplicateSlotProof, shred::Shredder}, + solana_ledger::shred::Shredder, solana_sdk::{ clock::{Clock, Slot, DEFAULT_SLOTS_PER_EPOCH}, program_error::ProgramError, @@ -105,34 +104,34 @@ mod tests { new_rand_data_shred(&mut rng, next_shred_index, &shredder, &leader, true, true); let shred2 = new_rand_data_shred(&mut rng, next_shred_index, &shredder, &leader, true, true); - let proof = DuplicateSlotProof { - shred1: shred1.payload().clone(), - shred2: shred2.payload().clone(), + let proof = DuplicateBlockProofData { + shred1: shred1.payload().as_slice(), + shred2: shred2.payload().as_slice(), }; - bincode::serialize(&proof).unwrap() + proof.pack() } #[test] - fn statue_of_limitations() { + fn test_statue_of_limitations() { unsafe { CLOCK_SLOT = SLOT + 5; - test_statue_of_limitations().unwrap(); + verify_with_clock().unwrap(); CLOCK_SLOT = SLOT - 1; assert_eq!( - test_statue_of_limitations().unwrap_err(), + verify_with_clock().unwrap_err(), ProgramError::ArithmeticOverflow ); CLOCK_SLOT = SLOT + DEFAULT_SLOTS_PER_EPOCH + 1; assert_eq!( - test_statue_of_limitations().unwrap_err(), + verify_with_clock().unwrap_err(), SlashingError::ExceedsStatueOfLimitations.into() ); } } - fn test_statue_of_limitations() -> Result<(), ProgramError> { + fn verify_with_clock() -> Result<(), ProgramError> { struct SyscallStubs {} impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { fn sol_get_clock_sysvar(&self, var_addr: *mut u8) -> u64 { diff --git a/slashing/program/src/shred.rs b/slashing/program/src/shred.rs index 03f50f10104..361e24c9def 100644 --- a/slashing/program/src/shred.rs +++ b/slashing/program/src/shred.rs @@ -2,6 +2,7 @@ use { crate::error::SlashingError, bitflags::bitflags, + bytemuck::Pod, generic_array::{typenum::U64, GenericArray}, num_enum::{IntoPrimitive, TryFromPrimitive}, serde_derive::Deserialize, @@ -9,6 +10,7 @@ use { clock::Slot, hash::{hashv, Hash}, }, + spl_pod::primitives::{PodU16, PodU32, PodU64}, }; pub(crate) const SIZE_OF_SIGNATURE: usize = 64; @@ -88,22 +90,22 @@ pub(crate) struct ErasureMeta { } #[derive(Clone, PartialEq, Eq)] -pub(crate) struct Shred { +pub(crate) struct Shred<'a> { shred_type: ShredType, proof_size: u8, chained: bool, resigned: bool, - payload: Vec, + payload: &'a [u8], } -impl Shred { +impl<'a> Shred<'a> { const SIZE_OF_CODING_PAYLOAD: usize = 1228; const SIZE_OF_DATA_PAYLOAD: usize = Self::SIZE_OF_CODING_PAYLOAD - Self::SIZE_OF_CODING_HEADERS + SIZE_OF_SIGNATURE; const SIZE_OF_CODING_HEADERS: usize = 89; const SIZE_OF_DATA_HEADERS: usize = 88; - pub(crate) fn new_from_payload(payload: Vec) -> Result { + pub(crate) fn new_from_payload(payload: &'a [u8]) -> Result { match Self::get_shred_variant(&payload)? { ShredVariant::LegacyCode | ShredVariant::LegacyData => Err(SlashingError::LegacyShreds), ShredVariant::MerkleCode { @@ -131,13 +133,13 @@ impl Shred { } } - fn get_bytes( + fn pod_from_bytes( &self, - ) -> Result<[u8; SIZE], SlashingError> { + ) -> Result<&T, SlashingError> { let end_index: usize = OFFSET .checked_add(SIZE) .ok_or(SlashingError::ShredDeserializationError)?; - <[u8; SIZE]>::try_from( + bytemuck::try_from_bytes( self.payload .get(OFFSET..end_index) .ok_or(SlashingError::ShredDeserializationError)?, @@ -145,7 +147,7 @@ impl Shred { .map_err(|_| SlashingError::ShredDeserializationError) } - fn get_shred_variant(payload: &[u8]) -> Result { + fn get_shred_variant(payload: &'a [u8]) -> Result { let Some(&shred_variant) = payload.get(OFFSET_OF_SHRED_VARIANT) else { return Err(SlashingError::ShredDeserializationError); }; @@ -153,23 +155,23 @@ impl Shred { } pub(crate) fn slot(&self) -> Result { - self.get_bytes::() - .map(Slot::from_le_bytes) + self.pod_from_bytes::() + .map(|x| u64::from(*x)) } pub(crate) fn index(&self) -> Result { - self.get_bytes::() - .map(u32::from_le_bytes) + self.pod_from_bytes::() + .map(|x| u32::from(*x)) } pub(crate) fn version(&self) -> Result { - self.get_bytes::() - .map(u16::from_le_bytes) + self.pod_from_bytes::() + .map(|x| u16::from(*x)) } pub(crate) fn fec_set_index(&self) -> Result { - self.get_bytes::() - .map(u32::from_le_bytes) + self.pod_from_bytes::() + .map(|x| u32::from(*x)) } pub(crate) fn shred_type(&self) -> ShredType { @@ -189,23 +191,20 @@ impl Shred { fn num_data_shreds(&self) -> Result { debug_assert!(self.shred_type == ShredType::Code); - self.get_bytes::() - .map(u16::from_le_bytes) - .map(usize::from) + self.pod_from_bytes::() + .map(|x| u16::from(*x) as usize) } fn num_coding_shreds(&self) -> Result { debug_assert!(self.shred_type == ShredType::Code); - self.get_bytes::() - .map(u16::from_le_bytes) - .map(usize::from) + self.pod_from_bytes::() + .map(|x| u16::from(*x) as usize) } fn position(&self) -> Result { debug_assert!(self.shred_type == ShredType::Code); - self.get_bytes::() - .map(u16::from_le_bytes) - .map(usize::from) + self.pod_from_bytes::() + .map(|x| u16::from(*x) as usize) } pub(crate) fn next_fec_set_index(&self) -> Result { @@ -305,9 +304,9 @@ impl Shred { // Recovers root of the merkle tree from a leaf node // at the given index and the respective proof. - fn get_merkle_root<'a, I>(index: usize, node: Hash, proof: I) -> Result + fn get_merkle_root<'b, I>(index: usize, node: Hash, proof: I) -> Result where - I: IntoIterator, + I: IntoIterator, { let (index, root) = proof .into_iter() @@ -333,7 +332,7 @@ impl Shred { { return false; } - fn get_payload(shred: &Shred) -> &[u8] { + fn get_payload<'a>(shred: &'a Shred<'a>) -> &'a [u8] { let Ok((proof_offset, proof_size)) = shred.get_proof_offset_and_size() else { return &shred.payload; }; @@ -532,7 +531,7 @@ pub(crate) mod tests { .into_iter() .chain(coding_solana_shreds.into_iter()) { - let payload = solana_shred.payload().clone(); + let payload = solana_shred.payload().as_slice(); let shred = Shred::new_from_payload(payload).unwrap(); assert_eq!(shred.slot().unwrap(), solana_shred.slot()); diff --git a/slashing/program/src/state.rs b/slashing/program/src/state.rs index 47c63a4e8cb..2d617312ed1 100644 --- a/slashing/program/src/state.rs +++ b/slashing/program/src/state.rs @@ -58,10 +58,15 @@ impl From for ProofType { /// Trait that proof accounts must satisfy in order to verify via the slashing /// program -pub trait SlashingProofData { +pub trait SlashingProofData<'a> { /// The type of proof this data represents const PROOF_TYPE: ProofType; + /// Zero copy from raw data buffer + fn unpack(data: &'a [u8]) -> Result + where + Self: Sized; + /// Verification logic for this type of proof data fn verify_proof(self, slot: Slot, pubkey: &Pubkey) -> Result<(), SlashingError>; } diff --git a/slashing/program/tests/duplicate_block_proof.rs b/slashing/program/tests/duplicate_block_proof.rs index 2038e4d4d52..bab00c51015 100644 --- a/slashing/program/tests/duplicate_block_proof.rs +++ b/slashing/program/tests/duplicate_block_proof.rs @@ -4,7 +4,7 @@ use { rand::Rng, solana_entry::entry::Entry, solana_ledger::{ - blockstore_meta::{DuplicateSlotProof, ErasureMeta}, + blockstore_meta::ErasureMeta, shred::{ProcessShredsStats, ReedSolomonCache, Shred, Shredder}, }, solana_program::pubkey::Pubkey, @@ -22,7 +22,8 @@ use { spl_pod::bytemuck::pod_get_packed_len, spl_record::{instruction as record, state::RecordData}, spl_slashing::{ - error::SlashingError, id, instruction, processor::process_instruction, state::ProofType, + duplicate_block_proof::DuplicateBlockProofData, error::SlashingError, id, instruction, + processor::process_instruction, state::ProofType, }, std::sync::Arc, }; @@ -208,11 +209,11 @@ async fn valid_proof_data() { "Expecting merkle root conflict", ); - let duplicate_proof = DuplicateSlotProof { - shred1: shred1.into_payload(), - shred2: shred2.into_payload(), + let duplicate_proof = DuplicateBlockProofData { + shred1: shred1.payload().as_slice(), + shred2: shred2.payload().as_slice(), }; - let data = bincode::serialize(&duplicate_proof).unwrap(); + let data = duplicate_proof.pack(); initialize_duplicate_proof_account(&mut context, &authority, &account).await; write_proof(&mut context, &authority, &account, &data).await; @@ -258,11 +259,11 @@ async fn valid_proof_coding() { "Expected erasure consistency failure", ); - let duplicate_proof = DuplicateSlotProof { - shred1: shred1.into_payload(), - shred2: shred2.into_payload(), + let duplicate_proof = DuplicateBlockProofData { + shred1: shred1.payload().as_slice(), + shred2: shred2.payload().as_slice(), }; - let data = bincode::serialize(&duplicate_proof).unwrap(); + let data = duplicate_proof.pack(); initialize_duplicate_proof_account(&mut context, &authority, &account).await; write_proof(&mut context, &authority, &account, &data).await; @@ -301,11 +302,11 @@ async fn invalid_proof_data() { let shred1 = new_rand_data_shred(&mut rng, next_shred_index, &shredder, &leader, true); let shred2 = shred1.clone(); - let duplicate_proof = DuplicateSlotProof { - shred1: shred1.into_payload(), - shred2: shred2.into_payload(), + let duplicate_proof = DuplicateBlockProofData { + shred1: shred1.payload().as_slice(), + shred2: shred2.payload().as_slice(), }; - let data = bincode::serialize(&duplicate_proof).unwrap(); + let data = duplicate_proof.pack(); initialize_duplicate_proof_account(&mut context, &authority, &account).await; write_proof(&mut context, &authority, &account, &data).await; @@ -355,11 +356,11 @@ async fn invalid_proof_coding() { ErasureMeta::check_erasure_consistency(&shred1, &shred2), "Expecting no erasure conflict" ); - let duplicate_proof = DuplicateSlotProof { - shred1: shred1.into_payload(), - shred2: shred2.into_payload(), + let duplicate_proof = DuplicateBlockProofData { + shred1: shred1.payload().as_slice(), + shred2: shred2.payload().as_slice(), }; - let data = bincode::serialize(&duplicate_proof).unwrap(); + let data = duplicate_proof.pack(); initialize_duplicate_proof_account(&mut context, &authority, &account).await; write_proof(&mut context, &authority, &account, &data).await;