diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index b0753b3aaa..08760786ed 100644 --- a/ledger/src/blockstore.rs +++ b/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) -> bool { @@ -3829,7 +3831,7 @@ fn slot_has_updates(slot_meta: &SlotMeta, slot_meta_backup: &Option) - // 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 diff --git a/ledger/src/blockstore_meta.rs b/ledger/src/blockstore_meta.rs index 5cacf78198..8394d1a8a6 100644 --- a/ledger/src/blockstore_meta.rs +++ b/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, - // 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, } @@ -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) -> 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::(&bincode::serialize(&with_bool).unwrap()).unwrap() + ); + + // Deserializing WithFlags into WithBool succeeds + assert_eq!( + with_bool, + bincode::deserialize::(&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::(&bincode::serialize(&with_flags).unwrap()).is_err() + ); + } + #[test] fn test_clear_unconfirmed_slot() { let mut slot_meta = SlotMeta::new_orphan(5);