Merge of #6744

Author: mergify[bot]

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -33,7 +33,6 @@ use crate::events::ServerSentEventHandler;
 use crate::execution_payload::{get_execution_payload, NotifyExecutionLayer, PreparePayloadHandle};
 use crate::fork_choice_signal::{ForkChoiceSignalRx, ForkChoiceSignalTx, ForkChoiceWaitResult};
 use crate::graffiti_calculator::GraffitiCalculator;
-use crate::head_tracker::{HeadTracker, HeadTrackerReader, SszHeadTracker};
 use crate::kzg_utils::reconstruct_blobs;
 use crate::light_client_finality_update_verification::{
     Error as LightClientFinalityUpdateError, VerifiedLightClientFinalityUpdate,
@@ -57,7 +56,7 @@ use crate::observed_block_producers::ObservedBlockProducers;
 use crate::observed_data_sidecars::ObservedDataSidecars;
 use crate::observed_operations::{ObservationOutcome, ObservedOperations};
 use crate::observed_slashable::ObservedSlashable;
-use crate::persisted_beacon_chain::{PersistedBeaconChain, DUMMY_CANONICAL_HEAD_BLOCK_ROOT};
+use crate::persisted_beacon_chain::PersistedBeaconChain;
 use crate::persisted_fork_choice::PersistedForkChoice;
 use crate::pre_finalization_cache::PreFinalizationBlockCache;
 use crate::shuffling_cache::{BlockShufflingIds, ShufflingCache};
@@ -454,8 +453,6 @@ pub struct BeaconChain<T: BeaconChainTypes> {
     /// A handler for events generated by the beacon chain. This is only initialized when the
     /// HTTP server is enabled.
     pub event_handler: Option<ServerSentEventHandler<T::EthSpec>>,
-    /// Used to track the heads of the beacon chain.
-    pub(crate) head_tracker: Arc<HeadTracker>,
     /// Caches the attester shuffling for a given epoch and shuffling key root.
     pub shuffling_cache: RwLock<ShufflingCache>,
     /// A cache of eth1 deposit data at epoch boundaries for deposit finalization
@@ -607,57 +604,13 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         })
     }
 
-    /// Persists the head tracker and fork choice.
+    /// Return a database operation for writing the `PersistedBeaconChain` to disk.
     ///
-    /// We do it atomically even though no guarantees need to be made about blocks from
-    /// the head tracker also being present in fork choice.
-    pub fn persist_head_and_fork_choice(&self) -> Result<(), Error> {
-        let mut batch = vec![];
-
-        let _head_timer = metrics::start_timer(&metrics::PERSIST_HEAD);
-
-        // Hold a lock to head_tracker until it has been persisted to disk. Otherwise there's a race
-        // condition with the pruning thread which can result in a block present in the head tracker
-        // but absent in the DB. This inconsistency halts pruning and dramastically increases disk
-        // size. Ref: https://github.com/sigp/lighthouse/issues/4773
-        let head_tracker = self.head_tracker.0.read();
-        batch.push(self.persist_head_in_batch(&head_tracker));
-
-        let _fork_choice_timer = metrics::start_timer(&metrics::PERSIST_FORK_CHOICE);
-        batch.push(self.persist_fork_choice_in_batch());
-
-        self.store.hot_db.do_atomically(batch)?;
-        drop(head_tracker);
-
-        Ok(())
-    }
-
-    /// Return a `PersistedBeaconChain` without reference to a `BeaconChain`.
-    pub fn make_persisted_head(
-        genesis_block_root: Hash256,
-        head_tracker_reader: &HeadTrackerReader,
-    ) -> PersistedBeaconChain {
-        PersistedBeaconChain {
-            _canonical_head_block_root: DUMMY_CANONICAL_HEAD_BLOCK_ROOT,
-            genesis_block_root,
-            ssz_head_tracker: SszHeadTracker::from_map(head_tracker_reader),
-        }
-    }
-
-    /// Return a database operation for writing the beacon chain head to disk.
-    pub fn persist_head_in_batch(
-        &self,
-        head_tracker_reader: &HeadTrackerReader,
-    ) -> KeyValueStoreOp {
-        Self::persist_head_in_batch_standalone(self.genesis_block_root, head_tracker_reader)
-    }
-
-    pub fn persist_head_in_batch_standalone(
-        genesis_block_root: Hash256,
-        head_tracker_reader: &HeadTrackerReader,
-    ) -> KeyValueStoreOp {
-        Self::make_persisted_head(genesis_block_root, head_tracker_reader)
-            .as_kv_store_op(BEACON_CHAIN_DB_KEY)
+    /// These days the `PersistedBeaconChain` is only used to store the genesis block root, so it
+    /// should only ever be written once at startup. It used to be written more frequently, but
+    /// this is no longer necessary.
+    pub fn persist_head_in_batch_standalone(genesis_block_root: Hash256) -> KeyValueStoreOp {
+        PersistedBeaconChain { genesis_block_root }.as_kv_store_op(BEACON_CHAIN_DB_KEY)
     }
 
     /// Load fork choice from disk, returning `None` if it isn't found.
@@ -1450,12 +1403,13 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ///
     /// Returns `(block_root, block_slot)`.
     pub fn heads(&self) -> Vec<(Hash256, Slot)> {
-        self.head_tracker.heads()
-    }
-
-    /// Only used in tests.
-    pub fn knows_head(&self, block_hash: &SignedBeaconBlockHash) -> bool {
-        self.head_tracker.contains_head((*block_hash).into())
+        self.canonical_head
+            .fork_choice_read_lock()
+            .proto_array()
+            .heads_descended_from_finalization::<T::EthSpec>()
+            .iter()
+            .map(|node| (node.root, node.slot))
+            .collect()
     }
 
     /// Returns the `BeaconState` at the given slot.
@@ -1735,8 +1689,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let notif = ManualFinalizationNotification {
             state_root: state_root.into(),
             checkpoint,
-            head_tracker: self.head_tracker.clone(),
-            genesis_block_root: self.genesis_block_root,
         };
 
         self.store_migrator.process_manual_finalization(notif);
@@ -3762,7 +3714,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             state,
             parent_block,
             parent_eth1_finalization_data,
-            confirmed_state_roots,
             consensus_context,
         } = import_data;
 
@@ -3786,7 +3737,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         block,
                         block_root,
                         state,
-                        confirmed_state_roots,
                         payload_verification_outcome.payload_verification_status,
                         parent_block,
                         parent_eth1_finalization_data,
@@ -3824,7 +3774,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         signed_block: AvailableBlock<T::EthSpec>,
         block_root: Hash256,
         mut state: BeaconState<T::EthSpec>,
-        confirmed_state_roots: Vec<Hash256>,
         payload_verification_status: PayloadVerificationStatus,
         parent_block: SignedBlindedBeaconBlock<T::EthSpec>,
         parent_eth1_finalization_data: Eth1FinalizationData,
@@ -4012,11 +3961,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
         let block = signed_block.message();
         let db_write_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_DB_WRITE);
-        ops.extend(
-            confirmed_state_roots
-                .into_iter()
-                .map(StoreOp::DeleteStateTemporaryFlag),
-        );
         ops.push(StoreOp::PutBlock(block_root, signed_block.clone()));
         ops.push(StoreOp::PutState(block.state_root(), &state));
 
@@ -4043,9 +3987,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         // about it.
         let block_time_imported = timestamp_now();
 
-        let parent_root = block.parent_root();
-        let slot = block.slot();
-
         let current_eth1_finalization_data = Eth1FinalizationData {
             eth1_data: state.eth1_data().clone(),
             eth1_deposit_index: state.eth1_deposit_index(),
@@ -4066,9 +4007,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 });
         }
 
-        self.head_tracker
-            .register_block(block_root, parent_root, slot);
-
         metrics::stop_timer(db_write_timer);
 
         metrics::inc_counter(&metrics::BLOCK_PROCESSING_SUCCESSES);
@@ -7208,7 +7146,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 impl<T: BeaconChainTypes> Drop for BeaconChain<T> {
     fn drop(&mut self) {
         let drop = || -> Result<(), Error> {
-            self.persist_head_and_fork_choice()?;
+            self.persist_fork_choice()?;
             self.persist_op_pool()?;
             self.persist_eth1_cache()
         };

beacon_node/beacon_chain/src/block_verification.rs

@@ -1453,22 +1453,8 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
 
         let catchup_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_CATCHUP_STATE);
 
-        // Stage a batch of operations to be completed atomically if this block is imported
-        // successfully. If there is a skipped slot, we include the state root of the pre-state,
-        // which may be an advanced state that was stored in the DB with a `temporary` flag.
         let mut state = parent.pre_state;
 
-        let mut confirmed_state_roots =
-            if block.slot() > state.slot() && state.slot() > parent.beacon_block.slot() {
-                // Advanced pre-state. Delete its temporary flag.
-                let pre_state_root = state.update_tree_hash_cache()?;
-                vec![pre_state_root]
-            } else {
-                // Pre state is either unadvanced, or should not be stored long-term because there
-                // is no skipped slot between `parent` and `block`.
-                vec![]
-            };
-
         // The block must have a higher slot than its parent.
         if block.slot() <= parent.beacon_block.slot() {
             return Err(BlockError::BlockIsNotLaterThanParent {
@@ -1515,38 +1501,29 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
                 // processing, but we get early access to it.
                 let state_root = state.update_tree_hash_cache()?;
 
-                // Store the state immediately, marking it as temporary, and staging the deletion
-                // of its temporary status as part of the larger atomic operation.
+                // Store the state immediately.
                 let txn_lock = chain.store.hot_db.begin_rw_transaction();
                 let state_already_exists =
                     chain.store.load_hot_state_summary(&state_root)?.is_some();
 
                 let state_batch = if state_already_exists {
-                    // If the state exists, it could be temporary or permanent, but in neither case
-                    // should we rewrite it or store a new temporary flag for it. We *will* stage
-                    // the temporary flag for deletion because it's OK to double-delete the flag,
-                    // and we don't mind if another thread gets there first.
+                    // If the state exists, we do not need to re-write it.
                     vec![]
                 } else {
-                    vec![
-                        if state.slot() % T::EthSpec::slots_per_epoch() == 0 {
-                            StoreOp::PutState(state_root, &state)
-                        } else {
-                            StoreOp::PutStateSummary(
-                                state_root,
-                                HotStateSummary::new(&state_root, &state)?,
-                            )
-                        },
-                        StoreOp::PutStateTemporaryFlag(state_root),
-                    ]
+                    vec![if state.slot() % T::EthSpec::slots_per_epoch() == 0 {
+                        StoreOp::PutState(state_root, &state)
+                    } else {
+                        StoreOp::PutStateSummary(
+                            state_root,
+                            HotStateSummary::new(&state_root, &state)?,
+                        )
+                    }]
                 };
                 chain
                     .store
                     .do_atomically_with_block_and_blobs_cache(state_batch)?;
                 drop(txn_lock);
 
-                confirmed_state_roots.push(state_root);
-
                 state_root
             };
 
@@ -1713,7 +1690,6 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
                 state,
                 parent_block: parent.beacon_block,
                 parent_eth1_finalization_data,
-                confirmed_state_roots,
                 consensus_context,
             },
             payload_verification_handle,

beacon_node/beacon_chain/src/block_verification_types.rs

@@ -358,7 +358,6 @@ pub struct BlockImportData<E: EthSpec> {
     pub state: BeaconState<E>,
     pub parent_block: SignedBeaconBlock<E, BlindedPayload<E>>,
     pub parent_eth1_finalization_data: Eth1FinalizationData,
-    pub confirmed_state_roots: Vec<Hash256>,
     pub consensus_context: ConsensusContext<E>,
 }
 
@@ -376,7 +375,6 @@ impl<E: EthSpec> BlockImportData<E> {
                 eth1_data: <_>::default(),
                 eth1_deposit_index: 0,
             },
-            confirmed_state_roots: vec![],
             consensus_context: ConsensusContext::new(Slot::new(0)),
         }
     }

beacon_node/beacon_chain/src/builder.rs

@@ -8,7 +8,6 @@ use crate::eth1_finalization_cache::Eth1FinalizationCache;
 use crate::fork_choice_signal::ForkChoiceSignalTx;
 use crate::fork_revert::{reset_fork_choice_to_finalization, revert_to_fork_boundary};
 use crate::graffiti_calculator::{GraffitiCalculator, GraffitiOrigin};
-use crate::head_tracker::HeadTracker;
 use crate::kzg_utils::blobs_to_data_column_sidecars;
 use crate::light_client_server_cache::LightClientServerCache;
 use crate::migrate::{BackgroundMigrator, MigratorConfig};
@@ -93,7 +92,6 @@ pub struct BeaconChainBuilder<T: BeaconChainTypes> {
     slot_clock: Option<T::SlotClock>,
     shutdown_sender: Option<Sender<ShutdownReason>>,
     light_client_server_tx: Option<Sender<LightClientProducerEvent<T::EthSpec>>>,
-    head_tracker: Option<HeadTracker>,
     validator_pubkey_cache: Option<ValidatorPubkeyCache<T>>,
     spec: Arc<ChainSpec>,
     chain_config: ChainConfig,
@@ -136,7 +134,6 @@ where
             slot_clock: None,
             shutdown_sender: None,
             light_client_server_tx: None,
-            head_tracker: None,
             validator_pubkey_cache: None,
             spec: Arc::new(E::default_spec()),
             chain_config: ChainConfig::default(),
@@ -314,10 +311,6 @@ where
 
         self.genesis_block_root = Some(chain.genesis_block_root);
         self.genesis_state_root = Some(genesis_block.state_root());
-        self.head_tracker = Some(
-            HeadTracker::from_ssz_container(&chain.ssz_head_tracker)
-                .map_err(|e| format!("Failed to decode head tracker for database: {:?}", e))?,
-        );
         self.validator_pubkey_cache = Some(pubkey_cache);
         self.fork_choice = Some(fork_choice);
 
@@ -729,7 +722,6 @@ where
             .genesis_state_root
             .ok_or("Cannot build without a genesis state root")?;
         let validator_monitor_config = self.validator_monitor_config.unwrap_or_default();
-        let head_tracker = Arc::new(self.head_tracker.unwrap_or_default());
         let beacon_proposer_cache: Arc<Mutex<BeaconProposerCache>> = <_>::default();
 
         let mut validator_monitor =
@@ -769,8 +761,6 @@ where
                         &self.spec,
                     )?;
 
-                    // Update head tracker.
-                    head_tracker.register_block(block_root, block.parent_root(), block.slot());
                     (block_root, block, true)
                 }
                 Err(e) => return Err(descriptive_db_error("head block", &e)),
@@ -846,8 +836,7 @@ where
             })?;
 
         let migrator_config = self.store_migrator_config.unwrap_or_default();
-        let store_migrator =
-            BackgroundMigrator::new(store.clone(), migrator_config, genesis_block_root);
+        let store_migrator = BackgroundMigrator::new(store.clone(), migrator_config);
 
         if let Some(slot) = slot_clock.now() {
             validator_monitor.process_valid_state(
@@ -872,11 +861,10 @@ where
         //
         // This *must* be stored before constructing the `BeaconChain`, so that its `Drop` instance
         // doesn't write a `PersistedBeaconChain` without the rest of the batch.
-        let head_tracker_reader = head_tracker.0.read();
         self.pending_io_batch.push(BeaconChain::<
             Witness<TSlotClock, TEth1Backend, E, THotStore, TColdStore>,
         >::persist_head_in_batch_standalone(
-            genesis_block_root, &head_tracker_reader
+            genesis_block_root
         ));
         self.pending_io_batch.push(BeaconChain::<
             Witness<TSlotClock, TEth1Backend, E, THotStore, TColdStore>,
@@ -887,7 +875,6 @@ where
             .hot_db
             .do_atomically(self.pending_io_batch)
             .map_err(|e| format!("Error writing chain & metadata to disk: {:?}", e))?;
-        drop(head_tracker_reader);
 
         let genesis_validators_root = head_snapshot.beacon_state.genesis_validators_root();
         let genesis_time = head_snapshot.beacon_state.genesis_time();
@@ -968,7 +955,6 @@ where
             fork_choice_signal_tx,
             fork_choice_signal_rx,
             event_handler: self.event_handler,
-            head_tracker,
             shuffling_cache: RwLock::new(ShufflingCache::new(
                 shuffling_cache_size,
                 head_shuffling_ids,