removes check for LAST_SHRED_IN_SLOT in Shredder::deshred (#4580)

LAST_SHRED_IN_SLOT already implies DATA_COMPLETE_SHRED: https://github.com/anza-xyz/agave/blob/e86d961cf/ledger/src/shred.rs#L125-L126 and we sanity check this as well: https://github.com/anza-xyz/agave/blob/e86d961cf/ledger/src/shred/shred_data.rs#L187-L191 So checking only for DATA_COMPLETE_SHRED in Shredder::deshred is sufficient.
anza-xyz · Jan 22, 2025 · bb4ce5a · bb4ce5a
1 parent e86d961
commit bb4ce5a
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/ledger/src/shred.rs b/ledger/src/shred.rs
@@ -2153,30 +2153,59 @@ mod tests {
     #[test]
     fn test_shred_flags_serde() {
         let flags: ShredFlags = bincode::deserialize(&[0b0001_0101]).unwrap();
+        assert_eq!(flags, ShredFlags::from_bits(0b0001_0101).unwrap());
         assert!(!flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
         assert!(!flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
         assert_eq!((flags & ShredFlags::SHRED_TICK_REFERENCE_MASK).bits(), 21u8);
         assert_eq!(bincode::serialize(&flags).unwrap(), [0b0001_0101]);
 
         let flags: ShredFlags = bincode::deserialize(&[0b0111_0001]).unwrap();
+        assert_eq!(flags, ShredFlags::from_bits(0b0111_0001).unwrap());
         assert!(flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
         assert!(!flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
         assert_eq!((flags & ShredFlags::SHRED_TICK_REFERENCE_MASK).bits(), 49u8);
         assert_eq!(bincode::serialize(&flags).unwrap(), [0b0111_0001]);
 
         let flags: ShredFlags = bincode::deserialize(&[0b1110_0101]).unwrap();
+        assert_eq!(flags, ShredFlags::from_bits(0b1110_0101).unwrap());
         assert!(flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
         assert!(flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
         assert_eq!((flags & ShredFlags::SHRED_TICK_REFERENCE_MASK).bits(), 37u8);
         assert_eq!(bincode::serialize(&flags).unwrap(), [0b1110_0101]);
 
         let flags: ShredFlags = bincode::deserialize(&[0b1011_1101]).unwrap();
+        assert_eq!(flags, ShredFlags::from_bits(0b1011_1101).unwrap());
         assert!(!flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
         assert!(!flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
         assert_eq!((flags & ShredFlags::SHRED_TICK_REFERENCE_MASK).bits(), 61u8);
         assert_eq!(bincode::serialize(&flags).unwrap(), [0b1011_1101]);
     }
 
+    // Verifies that LAST_SHRED_IN_SLOT also implies DATA_COMPLETE_SHRED.
+    #[test]
+    fn test_shred_flags_data_complete() {
+        let mut flags = ShredFlags::empty();
+        assert!(!flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
+        assert!(!flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
+        flags.insert(ShredFlags::LAST_SHRED_IN_SLOT);
+        assert!(flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
+        assert!(flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
+
+        let mut flags = ShredFlags::from_bits(0b0011_1111).unwrap();
+        assert!(!flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
+        assert!(!flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
+        flags |= ShredFlags::LAST_SHRED_IN_SLOT;
+        assert!(flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
+        assert!(flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
+
+        let mut flags: ShredFlags = bincode::deserialize(&[0b1011_1111]).unwrap();
+        assert!(!flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
+        assert!(!flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
+        flags.insert(ShredFlags::LAST_SHRED_IN_SLOT);
+        assert!(flags.contains(ShredFlags::DATA_COMPLETE_SHRED));
+        assert!(flags.contains(ShredFlags::LAST_SHRED_IN_SLOT));
+    }
+
     #[test_case(false, false)]
     #[test_case(false, true)]
     #[test_case(true, false)]

diff --git a/ledger/src/shredder.rs b/ledger/src/shredder.rs
@@ -399,7 +399,7 @@ impl Shredder {
             <(Vec<u8>, Option<u32>, bool)>::default(),
             |(mut data, prev, data_complete), shred| {
                 // No trailing shreds if we have already observed
-                // DATA_COMPLETE_SHRED or LAST_SHRED_IN_SLOT.
+                // DATA_COMPLETE_SHRED.
                 if data_complete {
                     return Err(Error::InvalidDeshredSet);
                 }
@@ -412,11 +412,10 @@ impl Shredder {
                     }
                 }
                 data.extend_from_slice(shred.data()?);
-                let data_complete = shred.data_complete() || shred.last_in_slot();
-                Ok((data, index, data_complete))
+                Ok((data, index, shred.data_complete()))
             },
         )?;
-        // The last shred should be DATA_COMPLETE_SHRED or LAST_SHRED_IN_SLOT.
+        // The last shred should be DATA_COMPLETE_SHRED.
         if !data_complete {
             return Err(Error::from(TooFewDataShards));
         }