removes set_{slot,index,last_in_slot} implementations for merkle shre…

…ds (backport #27689) (#27691) removes set_{slot,index,last_in_slot} implementations for merkle shreds (#27689) These methods are only used in tests but invoked on a merkle shred they will always invalidate the shred because the merkle proof will no longer verify. As a result the shred will not sanitize and blockstore will avoid inserting them. Their use in tests will result in spurious test coverage because the shreds will not be ingested. The commit removes implementation of these methods for merkle shreds. Follow up commits will entirely remove these methods from shreds api. (cherry picked from commit 37296ca) Co-authored-by: behzad nouri <[email protected]>
solana-labs · Sep 9, 2022 · 515d7f2 · 515d7f2
1 parent 132ab5e
commit 515d7f2
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 25 deletions.
diff --git a/ledger/src/shred/common.rs b/ledger/src/shred/common.rs
@@ -51,14 +51,28 @@ macro_rules! impl_shred_common {
 
         // Only for tests.
         fn set_index(&mut self, index: u32) {
-            self.common_header.index = index;
-            bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap();
+            match self.common_header.shred_variant {
+                ShredVariant::LegacyCode | ShredVariant::LegacyData => {
+                    self.common_header.index = index;
+                    bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap();
+                }
+                ShredVariant::MerkleCode(_) | ShredVariant::MerkleData(_) => {
+                    panic!("Not Implemented!");
+                }
+            }
         }
 
         // Only for tests.
         fn set_slot(&mut self, slot: Slot) {
-            self.common_header.slot = slot;
-            bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap();
+            match self.common_header.shred_variant {
+                ShredVariant::LegacyCode | ShredVariant::LegacyData => {
+                    self.common_header.slot = slot;
+                    bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap();
+                }
+                ShredVariant::MerkleCode(_) | ShredVariant::MerkleData(_) => {
+                    panic!("Not Implemented!");
+                }
+            }
         }
     };
 }

diff --git a/ledger/src/shred/legacy.rs b/ledger/src/shred/legacy.rs
@@ -196,13 +196,6 @@ impl ShredDataTrait for ShredData {
         }
         Ok(&self.payload[Self::SIZE_OF_HEADERS..size])
     }
-
-    // Only for tests.
-    fn set_last_in_slot(&mut self) {
-        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();
-    }
 }
 
 impl ShredCodeTrait for ShredCode {
@@ -278,6 +271,13 @@ impl ShredData {
         shred.resize(Self::SIZE_OF_PAYLOAD, 0u8);
         Ok(shred)
     }
+
+    // Only for tests.
+    pub(crate) fn set_last_in_slot(&mut self) {
+        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();
+    }
 }
 
 impl ShredCode {

diff --git a/ledger/src/shred/merkle.rs b/ledger/src/shred/merkle.rs
@@ -8,9 +8,8 @@ use {
             traits::{
                 Shred as ShredTrait, ShredCode as ShredCodeTrait, ShredData as ShredDataTrait,
             },
-            CodingShredHeader, DataShredHeader, Error, ShredCommonHeader, ShredFlags, ShredVariant,
-            SIZE_OF_CODING_SHRED_HEADERS, SIZE_OF_COMMON_SHRED_HEADER, SIZE_OF_DATA_SHRED_HEADERS,
-            SIZE_OF_SIGNATURE,
+            CodingShredHeader, DataShredHeader, Error, ShredCommonHeader, ShredVariant,
+            SIZE_OF_CODING_SHRED_HEADERS, SIZE_OF_DATA_SHRED_HEADERS, SIZE_OF_SIGNATURE,
         },
         shredder::ReedSolomon,
     },
@@ -511,13 +510,6 @@ impl ShredDataTrait for ShredData {
         }
         Ok(&self.payload[Self::SIZE_OF_HEADERS..size])
     }
-
-    // Only for tests.
-    fn set_last_in_slot(&mut self) {
-        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();
-    }
 }
 
 impl ShredCodeTrait for ShredCode {
@@ -746,6 +738,7 @@ pub(super) fn recover(mut shreds: Vec<Shred>) -> Result<Vec<Shred>, Error> {
 mod test {
     use {
         super::*,
+        crate::shred::ShredFlags,
         itertools::Itertools,
         matches::assert_matches,
         rand::{seq::SliceRandom, CryptoRng, Rng},

diff --git a/ledger/src/shred/shred_data.rs b/ledger/src/shred/shred_data.rs
@@ -28,7 +28,6 @@ impl ShredData {
     dispatch!(pub(super) fn parent(&self) -> Result<Slot, Error>);
     dispatch!(pub(super) fn payload(&self) -> &Vec<u8>);
     dispatch!(pub(super) fn sanitize(&self) -> Result<(), Error>);
-    dispatch!(pub(super) fn set_last_in_slot(&mut self));
     dispatch!(pub(super) fn set_signature(&mut self, signature: Signature));
     dispatch!(pub(super) fn signed_message(&self) -> &[u8]);
 
@@ -105,6 +104,14 @@ impl ShredData {
             Some(proof_size) => merkle::ShredData::capacity(proof_size),
         }
     }
+
+    // Only for tests.
+    pub(super) fn set_last_in_slot(&mut self) {
+        match self {
+            Self::Legacy(shred) => shred.set_last_in_slot(),
+            Self::Merkle(_) => panic!("Not Implemented!"),
+        }
+    }
 }
 
 impl From<legacy::ShredData> for ShredData {

diff --git a/ledger/src/shred/traits.rs b/ledger/src/shred/traits.rs
@@ -54,9 +54,6 @@ pub(super) trait ShredData: Shred {
     }
 
     fn data(&self) -> Result<&[u8], Error>;
-
-    // Only for tests.
-    fn set_last_in_slot(&mut self);
 }
 
 pub(super) trait ShredCode: Shred {