Delete fork_revert feature

beacon_node/beacon_chain/src/builder.rs

@@ -7,7 +7,6 @@ use crate::beacon_proposer_cache::BeaconProposerCache;
 use crate::custody_context::NodeCustodyType;
 use crate::data_availability_checker::DataAvailabilityChecker;
 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::kzg_utils::{build_data_column_sidecars_fulu, build_data_column_sidecars_gloas};
 use crate::light_client_server_cache::LightClientServerCache;
@@ -778,49 +777,17 @@ where
             .get_head(current_slot, &self.spec)
             .map_err(|e| format!("Unable to get fork choice head: {:?}", e))?;
 
-        // Try to decode the head block according to the current fork, if that fails, try
+        let head_block_root = initial_head_block_root;
-        // to backtrack to before the most recent fork.
+        let head_block = store
-        let (head_block_root, head_block, head_reverted) =
+            .get_full_block(&initial_head_block_root)
-            match store.get_full_block(&initial_head_block_root) {
+            .map_err(|e| descriptive_db_error("head block", &e))?
-                Ok(Some(block)) => (initial_head_block_root, block, false),
+            .ok_or("Head block not found in store")?;
-                Ok(None) => return Err("Head block not found in store".into()),
-                Err(StoreError::SszDecodeError(_)) => {
-                    error!(
-                        message = "This node has likely missed a hard fork. \
-                        It will try to revert the invalid blocks and keep running, \
-                        but any stray blocks and states will not be deleted. \
-                        Long-term you should consider re-syncing this node.",
-                        "Error decoding head block"
-                    );
-                    let (block_root, block) = revert_to_fork_boundary(
-                        current_slot,
-                        initial_head_block_root,
-                        store.clone(),
-                        &self.spec,
-                    )?;
-
-                    (block_root, block, true)
-                }
-                Err(e) => return Err(descriptive_db_error("head block", &e)),
-            };
 
         let (_head_state_root, head_state) = store
             .get_advanced_hot_state(head_block_root, current_slot, head_block.state_root())
             .map_err(|e| descriptive_db_error("head state", &e))?
             .ok_or("Head state not found in store")?;
 
-        // If the head reverted then we need to reset fork choice using the new head's finalized
-        // checkpoint.
-        if head_reverted {
-            fork_choice = reset_fork_choice_to_finalization(
-                head_block_root,
-                &head_state,
-                store.clone(),
-                Some(current_slot),
-                &self.spec,
-            )?;
-        }
-
         let head_shuffling_ids = BlockShufflingIds::try_from_head(head_block_root, &head_state)?;
 
         let mut head_snapshot = BeaconSnapshot {

beacon_node/beacon_chain/src/fork_revert.rs

@@ -1,204 +0,0 @@
-use crate::{BeaconForkChoiceStore, BeaconSnapshot};
-use fork_choice::{ForkChoice, PayloadVerificationStatus};
-use itertools::process_results;
-use state_processing::state_advance::complete_state_advance;
-use state_processing::{
-    ConsensusContext, VerifyBlockRoot, per_block_processing,
-    per_block_processing::BlockSignatureStrategy,
-};
-use std::sync::Arc;
-use std::time::Duration;
-use store::{HotColdDB, ItemStore, iter::ParentRootBlockIterator};
-use tracing::{info, warn};
-use types::{BeaconState, ChainSpec, EthSpec, ForkName, Hash256, SignedBeaconBlock, Slot};
-
-const CORRUPT_DB_MESSAGE: &str = "The database could be corrupt. Check its file permissions or \
-                                  consider deleting it by running with the --purge-db flag.";
-
-/// Revert the head to the last block before the most recent hard fork.
-///
-/// This function is destructive and should only be used if there is no viable alternative. It will
-/// cause the reverted blocks and states to be completely forgotten, lying dormant in the database
-/// forever.
-///
-/// Return the `(head_block_root, head_block)` that should be used post-reversion.
-pub fn revert_to_fork_boundary<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>>(
-    current_slot: Slot,
-    head_block_root: Hash256,
-    store: Arc<HotColdDB<E, Hot, Cold>>,
-    spec: &ChainSpec,
-) -> Result<(Hash256, SignedBeaconBlock<E>), String> {
-    let current_fork = spec.fork_name_at_slot::<E>(current_slot);
-    let fork_epoch = spec
-        .fork_epoch(current_fork)
-        .ok_or_else(|| format!("Current fork '{}' never activates", current_fork))?;
-
-    if current_fork == ForkName::Base {
-        return Err(format!(
-            "Cannot revert to before phase0 hard fork. {}",
-            CORRUPT_DB_MESSAGE
-        ));
-    }
-
-    warn!(
-        target_fork = %current_fork,
-        %fork_epoch,
-        "Reverting invalid head block"
-    );
-    let block_iter = ParentRootBlockIterator::fork_tolerant(&store, head_block_root);
-
-    let (block_root, blinded_block) = process_results(block_iter, |mut iter| {
-        iter.find_map(|(block_root, block)| {
-            if block.slot() < fork_epoch.start_slot(E::slots_per_epoch()) {
-                Some((block_root, block))
-            } else {
-                info!(
-                    ?block_root,
-                    slot = %block.slot(),
-                    "Reverting block"
-                );
-                None
-            }
-        })
-    })
-    .map_err(|e| {
-        format!(
-            "Error fetching blocks to revert: {:?}. {}",
-            e, CORRUPT_DB_MESSAGE
-        )
-    })?
-    .ok_or_else(|| format!("No pre-fork blocks found. {}", CORRUPT_DB_MESSAGE))?;
-
-    let block = store
-        .make_full_block(&block_root, blinded_block)
-        .map_err(|e| format!("Unable to add payload to new head block: {:?}", e))?;
-
-    Ok((block_root, block))
-}
-
-/// Reset fork choice to the finalized checkpoint of the supplied head state.
-///
-/// The supplied `head_block_root` should correspond to the most recently applied block on
-/// `head_state`.
-///
-/// This function avoids quirks of fork choice initialization by replaying all of the blocks from
-/// the checkpoint to the head.
-///
-/// See this issue for details: https://github.com/ethereum/consensus-specs/issues/2566
-///
-/// It will fail if the finalized state or any of the blocks to replay are unavailable.
-///
-/// WARNING: this function is destructive and causes fork choice to permanently forget all
-/// chains other than the chain leading to `head_block_root`. It should only be used in extreme
-/// circumstances when there is no better alternative.
-pub fn reset_fork_choice_to_finalization<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>>(
-    head_block_root: Hash256,
-    head_state: &BeaconState<E>,
-    store: Arc<HotColdDB<E, Hot, Cold>>,
-    current_slot: Option<Slot>,
-    spec: &ChainSpec,
-) -> Result<ForkChoice<BeaconForkChoiceStore<E, Hot, Cold>, E>, String> {
-    // Fetch finalized block.
-    let finalized_checkpoint = head_state.finalized_checkpoint();
-    let finalized_block_root = finalized_checkpoint.root;
-    let finalized_block = store
-        .get_full_block(&finalized_block_root)
-        .map_err(|e| format!("Error loading finalized block: {:?}", e))?
-        .ok_or_else(|| {
-            format!(
-                "Finalized block missing for revert: {:?}",
-                finalized_block_root
-            )
-        })?;
-
-    // Advance finalized state to finalized epoch (to handle skipped slots).
-    let finalized_state_root = finalized_block.state_root();
-    // The enshrined finalized state should be in the state cache.
-    let mut finalized_state = store
-        .get_state(&finalized_state_root, Some(finalized_block.slot()), true)
-        .map_err(|e| format!("Error loading finalized state: {:?}", e))?
-        .ok_or_else(|| {
-            format!(
-                "Finalized block state missing from database: {:?}",
-                finalized_state_root
-            )
-        })?;
-    let finalized_slot = finalized_checkpoint.epoch.start_slot(E::slots_per_epoch());
-    complete_state_advance(
-        &mut finalized_state,
-        Some(finalized_state_root),
-        finalized_slot,
-        spec,
-    )
-    .map_err(|e| {
-        format!(
-            "Error advancing finalized state to finalized epoch: {:?}",
-            e
-        )
-    })?;
-    let finalized_snapshot = BeaconSnapshot {
-        beacon_block_root: finalized_block_root,
-        beacon_block: Arc::new(finalized_block),
-        beacon_state: finalized_state,
-    };
-
-    let fc_store =
-        BeaconForkChoiceStore::get_forkchoice_store(store.clone(), finalized_snapshot.clone())
-            .map_err(|e| format!("Unable to reset fork choice store for revert: {e:?}"))?;
-
-    let mut fork_choice = ForkChoice::from_anchor(
-        fc_store,
-        finalized_block_root,
-        &finalized_snapshot.beacon_block,
-        &finalized_snapshot.beacon_state,
-        current_slot,
-        spec,
-    )
-    .map_err(|e| format!("Unable to reset fork choice for revert: {:?}", e))?;
-
-    // Replay blocks from finalized checkpoint back to head.
-    // We do not replay attestations presently, relying on the absence of other blocks
-    // to guarantee `head_block_root` as the head.
-    let blocks = store
-        .load_blocks_to_replay(finalized_slot + 1, head_state.slot(), head_block_root)
-        .map_err(|e| format!("Error loading blocks to replay for fork choice: {:?}", e))?;
-
-    let mut state = finalized_snapshot.beacon_state;
-    for block in blocks {
-        complete_state_advance(&mut state, None, block.slot(), spec)
-            .map_err(|e| format!("State advance failed: {:?}", e))?;
-
-        let mut ctxt = ConsensusContext::new(block.slot())
-            .set_proposer_index(block.message().proposer_index());
-        per_block_processing(
-            &mut state,
-            &block,
-            BlockSignatureStrategy::NoVerification,
-            VerifyBlockRoot::True,
-            &mut ctxt,
-            spec,
-        )
-        .map_err(|e| format!("Error replaying block: {:?}", e))?;
-
-        // Setting this to unverified is the safest solution, since we don't have a way to
-        // retro-actively determine if they were valid or not.
-        //
-        // This scenario is so rare that it seems OK to double-verify some blocks.
-        let payload_verification_status = PayloadVerificationStatus::Optimistic;
-
-        fork_choice
-            .on_block(
-                block.slot(),
-                block.message(),
-                block.canonical_root(),
-                // Reward proposer boost. We are reinforcing the canonical chain.
-                Duration::from_secs(0),
-                &state,
-                payload_verification_status,
-                spec,
-            )
-            .map_err(|e| format!("Error applying replayed block to fork choice: {:?}", e))?;
-    }
-
-    Ok(fork_choice)
-}

beacon_node/beacon_chain/src/lib.rs

@@ -26,7 +26,6 @@ pub mod events;
 pub mod execution_payload;
 pub mod fetch_blobs;
 pub mod fork_choice_signal;
-pub mod fork_revert;
 pub mod graffiti_calculator;
 pub mod historical_blocks;
 pub mod historical_data_columns;

beacon_node/beacon_chain/tests/store_tests.rs

@@ -3924,188 +3924,6 @@ async fn finalizes_after_resuming_from_db() {
     );
 }
 
-#[allow(clippy::large_stack_frames)]
-#[tokio::test]
-async fn revert_minority_fork_on_resume() {
-    let validator_count = 16;
-    let slots_per_epoch = MinimalEthSpec::slots_per_epoch();
-
-    let fork_epoch = Epoch::new(4);
-    let fork_slot = fork_epoch.start_slot(slots_per_epoch);
-    let initial_blocks = slots_per_epoch * fork_epoch.as_u64() - 1;
-    let post_fork_blocks = slots_per_epoch * 3;
-
-    let mut spec1 = MinimalEthSpec::default_spec();
-    spec1.altair_fork_epoch = None;
-    let mut spec2 = MinimalEthSpec::default_spec();
-    spec2.altair_fork_epoch = Some(fork_epoch);
-
-    let all_validators = (0..validator_count).collect::<Vec<usize>>();
-
-    // Chain with no fork epoch configured.
-    let db_path1 = tempdir().unwrap();
-    let store1 = get_store_generic(&db_path1, StoreConfig::default(), spec1.clone());
-    let harness1 = BeaconChainHarness::builder(MinimalEthSpec)
-        .spec(spec1.clone().into())
-        .keypairs(KEYPAIRS[0..validator_count].to_vec())
-        .fresh_disk_store(store1)
-        .mock_execution_layer()
-        .build();
-
-    // Chain with fork epoch configured.
-    let db_path2 = tempdir().unwrap();
-    let store2 = get_store_generic(&db_path2, StoreConfig::default(), spec2.clone());
-    let harness2 = BeaconChainHarness::builder(MinimalEthSpec)
-        .spec(spec2.clone().into())
-        .keypairs(KEYPAIRS[0..validator_count].to_vec())
-        .fresh_disk_store(store2)
-        .mock_execution_layer()
-        .build();
-
-    // Apply the same blocks to both chains initially.
-    let mut state = harness1.get_current_state();
-    let mut block_root = harness1.chain.genesis_block_root;
-    for slot in (1..=initial_blocks).map(Slot::new) {
-        let state_root = state.update_tree_hash_cache().unwrap();
-
-        let attestations = harness1.make_attestations(
-            &all_validators,
-            &state,
-            state_root,
-            block_root.into(),
-            slot,
-        );
-        harness1.set_current_slot(slot);
-        harness2.set_current_slot(slot);
-        harness1.process_attestations(attestations.clone(), &state);
-        harness2.process_attestations(attestations, &state);
-
-        let ((block, blobs), new_state) = harness1.make_block(state, slot).await;
-
-        harness1
-            .process_block(slot, block.canonical_root(), (block.clone(), blobs.clone()))
-            .await
-            .unwrap();
-        harness2
-            .process_block(slot, block.canonical_root(), (block.clone(), blobs.clone()))
-            .await
-            .unwrap();
-
-        state = new_state;
-        block_root = block.canonical_root();
-    }
-
-    assert_eq!(harness1.head_slot(), fork_slot - 1);
-    assert_eq!(harness2.head_slot(), fork_slot - 1);
-
-    // Fork the two chains.
-    let mut state1 = state.clone();
-    let mut state2 = state.clone();
-
-    let mut majority_blocks = vec![];
-
-    for i in 0..post_fork_blocks {
-        let slot = fork_slot + i;
-
-        // Attestations on majority chain.
-        let state_root = state.update_tree_hash_cache().unwrap();
-
-        let attestations = harness2.make_attestations(
-            &all_validators,
-            &state2,
-            state_root,
-            block_root.into(),
-            slot,
-        );
-        harness2.set_current_slot(slot);
-        harness2.process_attestations(attestations, &state2);
-
-        // Minority chain block (no attesters).
-        let ((block1, blobs1), new_state1) = harness1.make_block(state1, slot).await;
-        harness1
-            .process_block(slot, block1.canonical_root(), (block1, blobs1))
-            .await
-            .unwrap();
-        state1 = new_state1;
-
-        // Majority chain block (all attesters).
-        let ((block2, blobs2), new_state2) = harness2.make_block(state2, slot).await;
-        harness2
-            .process_block(slot, block2.canonical_root(), (block2.clone(), blobs2))
-            .await
-            .unwrap();
-
-        state2 = new_state2;
-        block_root = block2.canonical_root();
-
-        majority_blocks.push(block2);
-    }
-
-    let end_slot = fork_slot + post_fork_blocks - 1;
-    assert_eq!(harness1.head_slot(), end_slot);
-    assert_eq!(harness2.head_slot(), end_slot);
-
-    // Resume from disk with the hard-fork activated: this should revert the post-fork blocks.
-    // We have to do some hackery with the `slot_clock` so that the correct slot is set when
-    // the beacon chain builder loads the head block.
-    drop(harness1);
-    let resume_store = get_store_generic(&db_path1, StoreConfig::default(), spec2.clone());
-
-    let resumed_harness = TestHarness::builder(MinimalEthSpec)
-        .spec(spec2.clone().into())
-        .keypairs(KEYPAIRS[0..validator_count].to_vec())
-        .resumed_disk_store(resume_store)
-        .override_store_mutator(Box::new(move |mut builder| {
-            builder = builder
-                .resume_from_db()
-                .unwrap()
-                .testing_slot_clock(spec2.get_slot_duration())
-                .unwrap();
-            builder
-                .get_slot_clock()
-                .unwrap()
-                .set_slot(end_slot.as_u64());
-            builder
-        }))
-        .mock_execution_layer()
-        .build();
-
-    // Head should now be just before the fork.
-    resumed_harness.chain.recompute_head_at_current_slot().await;
-    assert_eq!(resumed_harness.head_slot(), fork_slot - 1);
-
-    // Fork choice should only know the canonical head. When we reverted the head we also should
-    // have called `reset_fork_choice_to_finalization` which rebuilds fork choice from scratch
-    // without the reverted block.
-    assert_eq!(
-        resumed_harness.chain.heads(),
-        vec![(resumed_harness.head_block_root(), fork_slot - 1)]
-    );
-
-    // Apply blocks from the majority chain and trigger finalization.
-    let initial_split_slot = resumed_harness.chain.store.get_split_slot();
-    for block in &majority_blocks {
-        resumed_harness
-            .process_block_result((block.clone(), None))
-            .await
-            .unwrap();
-
-        // The canonical head should be the block from the majority chain.
-        resumed_harness.chain.recompute_head_at_current_slot().await;
-        assert_eq!(resumed_harness.head_slot(), block.slot());
-        assert_eq!(resumed_harness.head_block_root(), block.canonical_root());
-    }
-    let advanced_split_slot = resumed_harness.chain.store.get_split_slot();
-
-    // Check that the migration ran successfully.
-    assert!(advanced_split_slot > initial_split_slot);
-
-    // Check that there is only a single head now matching harness2 (the minority chain is gone).
-    let heads = resumed_harness.chain.heads();
-    assert_eq!(heads, harness2.chain.heads());
-    assert_eq!(heads.len(), 1);
-}
-
 // This test checks whether the schema downgrade from the latest version to some minimum supported
 // version is correct. This is the easiest schema test to write without historic versions of
 // Lighthouse on-hand, but has the disadvantage that the min version needs to be adjusted manually