pr feedback: zerocopy from proof account to proof data struct

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<u8>,
-    #[serde(with = "serde_bytes")]
-    pub shred2: Vec<u8>,
+const LENGTH_SIZE: usize = std::mem::size_of::<PodU32>();
+
+/// 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<u8> {
+        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<Self, SlashingError>
+    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::<PodU32>(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::<PodU32>(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]

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,

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;

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 {

slashing/program/src/shred.rs

@@ -2,13 +2,15 @@
 use {
     crate::error::SlashingError,
     bitflags::bitflags,
+    bytemuck::Pod,
     generic_array::{typenum::U64, GenericArray},
     num_enum::{IntoPrimitive, TryFromPrimitive},
     serde_derive::Deserialize,
     solana_program::{
         clock::Slot,
         hash::{hashv, Hash},
     },
+    spl_pod::primitives::{PodU16, PodU32, PodU64},
 };
 
 pub(crate) const SIZE_OF_SIGNATURE: usize = 64;
@@ -88,23 +90,23 @@ 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<u8>,
+    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<u8>) -> Result<Self, SlashingError> {
-        match Self::get_shred_variant(&payload)? {
+    pub(crate) fn new_from_payload(payload: &'a [u8]) -> Result<Self, SlashingError> {
+        match Self::get_shred_variant(payload)? {
             ShredVariant::LegacyCode | ShredVariant::LegacyData => Err(SlashingError::LegacyShreds),
             ShredVariant::MerkleCode {
                 proof_size,
@@ -131,45 +133,45 @@ impl Shred {
         }
     }
 
-    fn get_bytes<const OFFSET: usize, const SIZE: usize>(
+    fn pod_from_bytes<const OFFSET: usize, const SIZE: usize, T: Pod>(
         &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)?,
         )
         .map_err(|_| SlashingError::ShredDeserializationError)
     }
 
-    fn get_shred_variant(payload: &[u8]) -> Result<ShredVariant, SlashingError> {
+    fn get_shred_variant(payload: &'a [u8]) -> Result<ShredVariant, SlashingError> {
         let Some(&shred_variant) = payload.get(OFFSET_OF_SHRED_VARIANT) else {
             return Err(SlashingError::ShredDeserializationError);
         };
         ShredVariant::try_from(shred_variant).map_err(|_| SlashingError::InvalidShredVariant)
     }
 
     pub(crate) fn slot(&self) -> Result<Slot, SlashingError> {
-        self.get_bytes::<OFFSET_OF_SLOT, SIZE_OF_SLOT>()
-            .map(Slot::from_le_bytes)
+        self.pod_from_bytes::<OFFSET_OF_SLOT, SIZE_OF_SLOT, PodU64>()
+            .map(|x| u64::from(*x))
     }
 
     pub(crate) fn index(&self) -> Result<u32, SlashingError> {
-        self.get_bytes::<OFFSET_OF_INDEX, SIZE_OF_INDEX>()
-            .map(u32::from_le_bytes)
+        self.pod_from_bytes::<OFFSET_OF_INDEX, SIZE_OF_INDEX, PodU32>()
+            .map(|x| u32::from(*x))
     }
 
     pub(crate) fn version(&self) -> Result<u16, SlashingError> {
-        self.get_bytes::<OFFSET_OF_VERSION, SIZE_OF_VERSION>()
-            .map(u16::from_le_bytes)
+        self.pod_from_bytes::<OFFSET_OF_VERSION, SIZE_OF_VERSION, PodU16>()
+            .map(|x| u16::from(*x))
     }
 
     pub(crate) fn fec_set_index(&self) -> Result<u32, SlashingError> {
-        self.get_bytes::<OFFSET_OF_FEC_SET_INDEX, SIZE_OF_FEC_SET_INDEX>()
-            .map(u32::from_le_bytes)
+        self.pod_from_bytes::<OFFSET_OF_FEC_SET_INDEX, SIZE_OF_FEC_SET_INDEX, PodU32>()
+            .map(|x| u32::from(*x))
     }
 
     pub(crate) fn shred_type(&self) -> ShredType {
@@ -189,23 +191,20 @@ impl Shred {
 
     fn num_data_shreds(&self) -> Result<usize, SlashingError> {
         debug_assert!(self.shred_type == ShredType::Code);
-        self.get_bytes::<OFFSET_OF_CODING_NUM_DATA_SHREDS, SIZE_OF_NUM_DATA_SHREDS>()
-            .map(u16::from_le_bytes)
-            .map(usize::from)
+        self.pod_from_bytes::<OFFSET_OF_CODING_NUM_DATA_SHREDS, SIZE_OF_NUM_DATA_SHREDS, PodU16>()
+            .map(|x| u16::from(*x) as usize)
     }
 
     fn num_coding_shreds(&self) -> Result<usize, SlashingError> {
         debug_assert!(self.shred_type == ShredType::Code);
-        self.get_bytes::<OFFSET_OF_CODING_NUM_CODING_SHREDS, SIZE_OF_NUM_CODING_SHREDS>()
-            .map(u16::from_le_bytes)
-            .map(usize::from)
+        self.pod_from_bytes::<OFFSET_OF_CODING_NUM_CODING_SHREDS, SIZE_OF_NUM_CODING_SHREDS, PodU16>()
+            .map(|x| u16::from(*x) as usize)
     }
 
     fn position(&self) -> Result<usize, SlashingError> {
         debug_assert!(self.shred_type == ShredType::Code);
-        self.get_bytes::<OFFSET_OF_CODING_POSITION, SIZE_OF_POSITION>()
-            .map(u16::from_le_bytes)
-            .map(usize::from)
+        self.pod_from_bytes::<OFFSET_OF_CODING_POSITION, SIZE_OF_POSITION, PodU16>()
+            .map(|x| u16::from(*x) as usize)
     }
 
     pub(crate) fn next_fec_set_index(&self) -> Result<u32, SlashingError> {
@@ -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<Hash, SlashingError>
+    fn get_merkle_root<'b, I>(index: usize, node: Hash, proof: I) -> Result<Hash, SlashingError>
     where
-        I: IntoIterator<Item = &'a MerkleProofEntry>,
+        I: IntoIterator<Item = &'b MerkleProofEntry>,
     {
         let (index, root) = proof
             .into_iter()
@@ -333,20 +332,17 @@ 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;
+                return shred.payload;
             };
             if !shred.resigned {
-                return &shred.payload;
+                return shred.payload;
             }
             let Some(offset) = proof_offset.checked_add(proof_size) else {
-                return &shred.payload;
+                return shred.payload;
             };
-            shred
-                .payload
-                .get(..offset)
-                .unwrap_or_else(|| &shred.payload)
+            shred.payload.get(..offset).unwrap_or(shred.payload)
         }
         get_payload(self) != get_payload(other)
     }
@@ -532,7 +528,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());