Abolish temporary states concept

Author: michaelsproul

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -3679,7 +3679,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             state,
             parent_block,
             parent_eth1_finalization_data,
-            confirmed_state_roots,
             consensus_context,
             data_column_recv,
         } = import_data;
@@ -3704,7 +3703,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,
@@ -3743,7 +3741,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,
@@ -3945,11 +3942,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));
 

beacon_node/beacon_chain/src/block_verification.rs

@@ -1435,22 +1435,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 {
@@ -1497,38 +1483,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
             };
 
@@ -1705,7 +1682,6 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
                 state,
                 parent_block: parent.beacon_block,
                 parent_eth1_finalization_data,
-                confirmed_state_roots,
                 consensus_context,
                 data_column_recv: None,
             },

beacon_node/beacon_chain/src/block_verification_types.rs

@@ -345,7 +345,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>,
     #[derivative(PartialEq = "ignore")]
     /// An optional receiver for `DataColumnSidecarList`.
@@ -369,7 +368,6 @@ impl<E: EthSpec> BlockImportData<E> {
                 eth1_data: <_>::default(),
                 eth1_deposit_index: 0,
             },
-            confirmed_state_roots: vec![],
             consensus_context: ConsensusContext::new(Slot::new(0)),
             data_column_recv: None,
         }

beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs

@@ -881,7 +881,6 @@ mod test {
             state,
             parent_block,
             parent_eth1_finalization_data,
-            confirmed_state_roots: vec![],
             consensus_context,
             data_column_recv: None,
         };
@@ -1272,7 +1271,6 @@ mod pending_components_tests {
                     eth1_data: Default::default(),
                     eth1_deposit_index: 0,
                 },
-                confirmed_state_roots: vec![],
                 consensus_context: ConsensusContext::new(Slot::new(0)),
                 data_column_recv: None,
             },

beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs

@@ -7,26 +7,21 @@ use crate::{
 };
 use lru::LruCache;
 use parking_lot::RwLock;
-use ssz_derive::{Decode, Encode};
 use state_processing::BlockReplayer;
 use std::sync::Arc;
 use store::OnDiskConsensusContext;
 use types::beacon_block_body::KzgCommitments;
-use types::{ssz_tagged_signed_beacon_block, ssz_tagged_signed_beacon_block_arc};
 use types::{BeaconState, BlindedPayload, ChainSpec, Epoch, EthSpec, Hash256, SignedBeaconBlock};
 
 /// This mirrors everything in the `AvailabilityPendingExecutedBlock`, except
 /// that it is much smaller because it contains only a state root instead of
 /// a full `BeaconState`.
-#[derive(Encode, Decode, Clone)]
+#[derive(Clone)]
 pub struct DietAvailabilityPendingExecutedBlock<E: EthSpec> {
-    #[ssz(with = "ssz_tagged_signed_beacon_block_arc")]
     block: Arc<SignedBeaconBlock<E>>,
     state_root: Hash256,
-    #[ssz(with = "ssz_tagged_signed_beacon_block")]
     parent_block: SignedBeaconBlock<E, BlindedPayload<E>>,
     parent_eth1_finalization_data: Eth1FinalizationData,
-    confirmed_state_roots: Vec<Hash256>,
     consensus_context: OnDiskConsensusContext<E>,
     payload_verification_outcome: PayloadVerificationOutcome,
 }
@@ -103,7 +98,6 @@ impl<T: BeaconChainTypes> StateLRUCache<T> {
             state_root,
             parent_block: executed_block.import_data.parent_block,
             parent_eth1_finalization_data: executed_block.import_data.parent_eth1_finalization_data,
-            confirmed_state_roots: executed_block.import_data.confirmed_state_roots,
             consensus_context: OnDiskConsensusContext::from_consensus_context(
                 executed_block.import_data.consensus_context,
             ),
@@ -132,7 +126,6 @@ impl<T: BeaconChainTypes> StateLRUCache<T> {
                 state,
                 parent_block: diet_executed_block.parent_block,
                 parent_eth1_finalization_data: diet_executed_block.parent_eth1_finalization_data,
-                confirmed_state_roots: diet_executed_block.confirmed_state_roots,
                 consensus_context: diet_executed_block
                     .consensus_context
                     .into_consensus_context(),
@@ -221,7 +214,6 @@ impl<E: EthSpec> From<AvailabilityPendingExecutedBlock<E>>
             state_root: value.import_data.state.canonical_root().unwrap(),
             parent_block: value.import_data.parent_block,
             parent_eth1_finalization_data: value.import_data.parent_eth1_finalization_data,
-            confirmed_state_roots: value.import_data.confirmed_state_roots,
             consensus_context: OnDiskConsensusContext::from_consensus_context(
                 value.import_data.consensus_context,
             ),

beacon_node/store/src/garbage_collection.rs

@@ -395,9 +395,6 @@ impl<E: EthSpec> HotColdDB<E, BeaconNodeBackend<E>, BeaconNodeBackend<E>> {
         }
         db.store_config()?;
 
-        // Run a garbage collection pass.
-        db.remove_garbage()?;
-
         // If configured, run a foreground compaction pass.
         if db.config.compact_on_init {
             info!(db.log, "Running foreground compaction");

beacon_node/store/src/hot_cold_store.rs

@@ -14,7 +14,6 @@ pub mod config;
 pub mod consensus_context;
 pub mod errors;
 mod forwards_iter;
-mod garbage_collection;
 pub mod hdiff;
 pub mod historic_state_cache;
 pub mod hot_cold_store;
@@ -289,8 +288,10 @@ pub enum DBColumn {
     /// Mapping from state root to `ColdStateSummary` in the cold DB.
     #[strum(serialize = "bcs")]
     BeaconColdStateSummary,
-    /// For the list of temporary states stored during block import,
-    /// and then made non-temporary by the deletion of their state root from this column.
+    /// DEPRECATED.
+    ///
+    /// Previously used for the list of temporary states stored during block import, and then made
+    /// non-temporary by the deletion of their state root from this column.
     #[strum(serialize = "bst")]
     BeaconStateTemporary,
     /// Execution payloads for blocks more recent than the finalized checkpoint.