changes Shred::parent return type to Option<Slot> (#21370)

Shred::parent can return garbage if the struct fields are invalid:
https://github.com/solana-labs/solana/blob/8a50b6302/ledger/src/shred.rs#L446-L453

The commit adds more sanity checks and changes the return type to Option<Slot>.

core/src/broadcast_stage/standard_broadcast_run.rs

@@ -596,7 +596,7 @@ mod test {
             .expect("Expected a shred that signals an interrupt");
 
         // Validate the shred
-        assert_eq!(shred.parent(), parent);
+        assert_eq!(shred.parent(), Some(parent));
         assert_eq!(shred.slot(), slot);
         assert_eq!(shred.index(), next_shred_index);
         assert!(shred.is_data());

core/src/window_service.rs

@@ -164,7 +164,10 @@ impl ReceiveWindowStats {
 fn verify_shred_slot(shred: &Shred, root: u64) -> bool {
     match shred.shred_type() {
         // Only data shreds have parent information
-        ShredType::Data => blockstore::verify_shred_slots(shred.slot(), shred.parent(), root),
+        ShredType::Data => match shred.parent() {
+            Some(parent) => blockstore::verify_shred_slots(shred.slot(), parent, root),
+            None => false,
+        },
         // Filter out outdated coding shreds
         ShredType::Code => shred.slot() >= root,
     }

ledger/src/blockstore.rs

@@ -1209,8 +1209,12 @@ impl Blockstore {
             get_index_meta_entry(&self.db, slot, index_working_set, index_meta_time);
 
         let index_meta = &mut index_meta_working_set_entry.index;
-        let slot_meta_entry =
+        let slot_meta_entry = get_slot_meta_entry(
-            get_slot_meta_entry(&self.db, slot_meta_working_set, slot, shred.parent());
+            &self.db,
+            slot_meta_working_set,
+            slot,
+            shred.parent().ok_or(InsertDataShredError::InvalidShred)?,
+        );
 
         let slot_meta = &mut slot_meta_entry.new_slot_meta.borrow_mut();
 

ledger/src/shred.rs

@@ -69,7 +69,11 @@ use {
         pubkey::Pubkey,
         signature::{Keypair, Signature, Signer},
     },
-    std::{cell::RefCell, convert::TryInto, mem::size_of},
+    std::{
+        cell::RefCell,
+        convert::{TryFrom, TryInto},
+        mem::size_of,
+    },
     thiserror::Error,
 };
 
@@ -443,12 +447,13 @@ impl Shred {
         self.common_header.slot
     }
 
-    // TODO: This should return Option<Slot>
+    pub fn parent(&self) -> Option<Slot> {
-    pub fn parent(&self) -> Slot {
+        match self.shred_type() {
-        if self.is_data() {
+            ShredType::Data => {
-            self.common_header.slot - u64::from(self.data_header.parent_offset)
+                let parent_offset = Slot::try_from(self.data_header.parent_offset);
-        } else {
+                self.slot().checked_sub(parent_offset.ok()?)
-            std::u64::MAX
+            }
+            ShredType::Code => None,
         }
     }
 
@@ -1098,7 +1103,7 @@ pub fn verify_test_data_shred(
     assert!(shred.is_data());
     assert_eq!(shred.index(), index);
     assert_eq!(shred.slot(), slot);
-    assert_eq!(shred.parent(), parent);
+    assert_eq!(shred.parent(), Some(parent));
     assert_eq!(verify, shred.verify(pk));
     if is_last_in_slot {
         assert!(shred.last_in_slot());