Change SlotMeta is_connected bool to bitflags (#29001)

We currently use the is_connected field to be able to signal to
ReplayStage that a slot has replayable updates. It was discovered that
this functionality is effectively broken, and that is_connected is never
true. In order to convey this information to ReplayStage more
effectively, we need extra state information so this PR changes the
existing bool to bitflags with two bits.

From a compatibility standpoint, the is_connected bool was already
occupying one byte in the serialized SlotMeta in blockstore. Thus, the
change from a bool to bitflags still "fits" in that one byte allotment.

In consideration of a case where a client may wish to downgrade software
and use the same ledger, deserializing the bitflags into a bool could
fail if the new bit is set. As such, this PR introduces the second bit
field, but does not set it anywhere. Once clusters have mass adopted a
software version with this PR, a subsequent change to actually set and
use the new field can be introduced.

ledger/src/blockstore.rs

@@ -3728,13 +3728,13 @@ fn handle_chaining_for_slot(
     // connected to trunk of the ledger
     let should_propagate_is_connected =
         is_newly_completed_slot(&RefCell::borrow(meta), meta_backup)
-            && RefCell::borrow(meta).is_connected;
+            && RefCell::borrow(meta).is_connected();
 
     if should_propagate_is_connected {
         // slot_function returns a boolean indicating whether to explore the children
         // of the input slot
         let slot_function = |slot: &mut SlotMeta| {
-            slot.is_connected = true;
+            slot.set_connected();
 
             // We don't want to set the is_connected flag on the children of non-full
             // slots
@@ -3815,7 +3815,9 @@ fn chain_new_slot_to_prev_slot(
     current_slot_meta: &mut SlotMeta,
 ) {
     prev_slot_meta.next_slots.push(current_slot);
-    current_slot_meta.is_connected = prev_slot_meta.is_connected && prev_slot_meta.is_full();
+    if prev_slot_meta.is_connected() && prev_slot_meta.is_full() {
+        current_slot_meta.set_connected();
+    }
 }
 
 fn is_newly_completed_slot(slot_meta: &SlotMeta, backup_slot_meta: &Option<SlotMeta>) -> bool {
@@ -3829,7 +3831,7 @@ fn slot_has_updates(slot_meta: &SlotMeta, slot_meta_backup: &Option<SlotMeta>) -
     // from block 0, which is true iff:
     // 1) The block with index prev_block_index is itself part of the trunk of consecutive blocks
     // starting from block 0,
-    slot_meta.is_connected &&
+    slot_meta.is_connected() &&
         // AND either:
         // 1) The slot didn't exist in the database before, and now we have a consecutive
         // block for that slot
@@ -4817,7 +4819,7 @@ pub mod tests {
         assert_eq!(meta.parent_slot, Some(0));
         assert_eq!(meta.last_index, Some(num_shreds - 1));
         assert!(meta.next_slots.is_empty());
-        assert!(meta.is_connected);
+        assert!(meta.is_connected());
     }
 
     #[test]
@@ -5337,7 +5339,7 @@ pub mod tests {
         let meta1 = blockstore.meta(1).unwrap().unwrap();
         assert!(meta1.next_slots.is_empty());
         // Slot 1 is not trunk because slot 0 hasn't been inserted yet
-        assert!(!meta1.is_connected);
+        assert!(!meta1.is_connected());
         assert_eq!(meta1.parent_slot, Some(0));
         assert_eq!(meta1.last_index, Some(shreds_per_slot as u64 - 1));
 
@@ -5349,7 +5351,7 @@ pub mod tests {
         let meta2 = blockstore.meta(2).unwrap().unwrap();
         assert!(meta2.next_slots.is_empty());
         // Slot 2 is not trunk because slot 0 hasn't been inserted yet
-        assert!(!meta2.is_connected);
+        assert!(!meta2.is_connected());
         assert_eq!(meta2.parent_slot, Some(1));
         assert_eq!(meta2.last_index, Some(shreds_per_slot as u64 - 1));
 
@@ -5357,7 +5359,7 @@ pub mod tests {
         // but still isn't part of the trunk
         let meta1 = blockstore.meta(1).unwrap().unwrap();
         assert_eq!(meta1.next_slots, vec![2]);
-        assert!(!meta1.is_connected);
+        assert!(!meta1.is_connected());
         assert_eq!(meta1.parent_slot, Some(0));
         assert_eq!(meta1.last_index, Some(shreds_per_slot as u64 - 1));
 
@@ -5376,7 +5378,7 @@ pub mod tests {
                 assert_eq!(meta.parent_slot, Some(slot - 1));
             }
             assert_eq!(meta.last_index, Some(shreds_per_slot as u64 - 1));
-            assert!(meta.is_connected);
+            assert!(meta.is_connected());
         }
     }
 
@@ -5435,9 +5437,9 @@ pub mod tests {
             }
 
             if slot == 0 {
-                assert!(meta.is_connected);
+                assert!(meta.is_connected());
             } else {
-                assert!(!meta.is_connected);
+                assert!(!meta.is_connected());
             }
         }
 
@@ -5462,7 +5464,7 @@ pub mod tests {
                 assert_eq!(meta.parent_slot, Some(slot - 1));
             }
             assert_eq!(meta.last_index, Some(shreds_per_slot as u64 - 1));
-            assert!(meta.is_connected);
+            assert!(meta.is_connected());
         }
     }
 
@@ -5508,10 +5510,10 @@ pub mod tests {
             // Additionally, slot 0 should be the only connected slot
             if slot == 0 {
                 assert_eq!(meta.parent_slot, Some(0));
-                assert!(meta.is_connected);
+                assert!(meta.is_connected());
             } else {
                 assert_eq!(meta.parent_slot, Some(slot - 1));
-                assert!(!meta.is_connected);
+                assert!(!meta.is_connected());
             }
 
             assert_eq!(meta.last_index, Some(shreds_per_slot as u64 - 1));
@@ -5532,9 +5534,9 @@ pub mod tests {
                         assert!(meta.next_slots.is_empty());
                     }
                     if slot <= slot_index + 3 {
-                        assert!(meta.is_connected);
+                        assert!(meta.is_connected());
                     } else {
-                        assert!(!meta.is_connected);
+                        assert!(!meta.is_connected());
                     }
 
                     if slot == 0 {
@@ -5614,7 +5616,7 @@ pub mod tests {
                 let slot_meta = blockstore.meta(slot).unwrap().unwrap();
                 assert_eq!(slot_meta.consumed, entries_per_slot);
                 assert_eq!(slot_meta.received, entries_per_slot);
-                assert!(slot_meta.is_connected);
+                assert!(slot_meta.is_connected());
                 let slot_parent = {
                     if slot == 0 {
                         0

ledger/src/blockstore_meta.rs

@@ -1,5 +1,6 @@
 use {
     crate::shred::{Shred, ShredType},
+    bitflags::bitflags,
     serde::{Deserialize, Deserializer, Serialize, Serializer},
     solana_sdk::{
         clock::{Slot, UnixTimestamp},
@@ -11,6 +12,43 @@ use {
     },
 };
 
+bitflags! {
+    #[derive(Deserialize, Serialize)]
+    /// Flags to indicate whether a slot is a descendant of a slot on the main fork
+    pub struct ConnectedFlags:u8 {
+        // A slot S should be considered to be connected if:
+        // 1) S is a rooted slot itself OR
+        // 2) S's parent is connected AND S is full (S's complete block present)
+        //
+        // 1) is a straightfoward case, roots are finalized blocks on the main fork
+        // so by definition, they are connected. All roots are connected, but not
+        // all connected slots are (or will become) roots.
+        //
+        // Based on the criteria stated in 2), S is connected iff it has a series
+        // of ancestors (that are each connected) that form a chain back to
+        // some root slot.
+        //
+        // A ledger that is updating with a cluster will have either begun at
+        // genesis or at at some snapshot slot.
+        // - Genesis is obviously a special case, and slot 0's parent is deemed
+        //   to be connected in order to kick off the induction
+        // - Snapshots are taken at rooted slots, and as such, the snapshot slot
+        //   should be marked as connected so that a connected chain can start
+        //
+        // CONNECTED is explicitly the first bit to ensure backwards compatibility
+        // with the boolean field that ConnectedFlags replaced in SlotMeta.
+        const CONNECTED        = 0b0000_0001;
+        // PARENT_CONNECTED IS INTENTIIONALLY UNUSED FOR NOW
+        const PARENT_CONNECTED = 0b1000_0000;
+    }
+}
+
+impl Default for ConnectedFlags {
+    fn default() -> Self {
+        ConnectedFlags::empty()
+    }
+}
+
 #[derive(Clone, Debug, Default, Deserialize, Serialize, Eq, PartialEq)]
 // The Meta column family
 pub struct SlotMeta {
@@ -37,9 +75,8 @@ pub struct SlotMeta {
     // The list of slots, each of which contains a block that derives
     // from this one.
     pub next_slots: Vec<Slot>,
-    // True if this slot is full (consumed == last_index + 1) and if every
-    // slot that is a parent of this slot is also connected.
-    pub is_connected: bool,
+    // Connected status flags of this slot
+    pub connected_flags: ConnectedFlags,
     // Shreds indices which are marked data complete.
     pub completed_data_indexes: BTreeSet<u32>,
 }
@@ -214,6 +251,14 @@ impl SlotMeta {
         Some(self.consumed) == self.last_index.map(|ix| ix + 1)
     }
 
+    pub fn is_connected(&self) -> bool {
+        self.connected_flags.contains(ConnectedFlags::CONNECTED)
+    }
+
+    pub fn set_connected(&mut self) {
+        self.connected_flags.set(ConnectedFlags::CONNECTED, true);
+    }
+
     /// Dangerous. Currently only needed for a local-cluster test
     pub fn unset_parent(&mut self) {
         self.parent_slot = None;
@@ -225,10 +270,17 @@ impl SlotMeta {
     }
 
     pub(crate) fn new(slot: Slot, parent_slot: Option<Slot>) -> Self {
+        let connected_flags = if slot == 0 {
+            // Slot 0 is the start, mark it as having its' parent connected
+            // such that slot 0 becoming full will be updated as connected
+            ConnectedFlags::CONNECTED
+        } else {
+            ConnectedFlags::default()
+        };
         SlotMeta {
             slot,
             parent_slot,
-            is_connected: slot == 0,
+            connected_flags,
             ..SlotMeta::default()
         }
     }
@@ -426,6 +478,79 @@ mod test {
         }
     }
 
+    #[test]
+    fn test_connected_flags_compatibility() {
+        // Define a couple structs with bool and ConnectedFlags to illustrate
+        // that that ConnectedFlags can be deserialized into a bool if the
+        // PARENT_CONNECTED bit is NOT set
+        #[derive(Debug, Deserialize, PartialEq, Serialize)]
+        struct WithBool {
+            slot: Slot,
+            connected: bool,
+        }
+        #[derive(Debug, Deserialize, PartialEq, Serialize)]
+        struct WithFlags {
+            slot: Slot,
+            connected: ConnectedFlags,
+        }
+
+        let slot = 3;
+        let mut with_bool = WithBool {
+            slot,
+            connected: false,
+        };
+        let mut with_flags = WithFlags {
+            slot,
+            connected: ConnectedFlags::default(),
+        };
+
+        // Confirm that serialized byte arrays are same length
+        assert_eq!(
+            bincode::serialized_size(&with_bool).unwrap(),
+            bincode::serialized_size(&with_flags).unwrap()
+        );
+
+        // Confirm that connected=false equivalent to ConnectedFlags::default()
+        assert_eq!(
+            bincode::serialize(&with_bool).unwrap(),
+            bincode::serialize(&with_flags).unwrap()
+        );
+
+        // Set connected in WithBool and confirm inequality
+        with_bool.connected = true;
+        assert_ne!(
+            bincode::serialize(&with_bool).unwrap(),
+            bincode::serialize(&with_flags).unwrap()
+        );
+
+        // Set connected in WithFlags and confirm equality regained
+        with_flags.connected.set(ConnectedFlags::CONNECTED, true);
+        assert_eq!(
+            bincode::serialize(&with_bool).unwrap(),
+            bincode::serialize(&with_flags).unwrap()
+        );
+
+        // Dserializing WithBool into WithFlags succeeds
+        assert_eq!(
+            with_flags,
+            bincode::deserialize::<WithFlags>(&bincode::serialize(&with_bool).unwrap()).unwrap()
+        );
+
+        // Deserializing WithFlags into WithBool succeeds
+        assert_eq!(
+            with_bool,
+            bincode::deserialize::<WithBool>(&bincode::serialize(&with_flags).unwrap()).unwrap()
+        );
+
+        // Deserializing WithFlags with extra bit set into WithBool fails
+        with_flags
+            .connected
+            .set(ConnectedFlags::PARENT_CONNECTED, true);
+        assert!(
+            bincode::deserialize::<WithBool>(&bincode::serialize(&with_flags).unwrap()).is_err()
+        );
+    }
+
     #[test]
     fn test_clear_unconfirmed_slot() {
         let mut slot_meta = SlotMeta::new_orphan(5);