Merge remote-tracking branch 'origin/drop-headtracker' into tree-states-hot-rebase

Author: michaelsproul

beacon_node/beacon_chain/src/state_advance_timer.rs

@@ -23,7 +23,6 @@ use std::sync::{
     atomic::{AtomicBool, Ordering},
     Arc,
 };
-use store::KeyValueStore;
 use task_executor::TaskExecutor;
 use tokio::time::{sleep, sleep_until, Instant};
 use tracing::{debug, error, warn};
@@ -297,7 +296,7 @@ fn advance_head<T: BeaconChainTypes>(beacon_chain: &Arc<BeaconChain<T>>) -> Resu
     // Protect against advancing a state more than a single slot.
     //
     // Advancing more than one slot without storing the intermediate state would corrupt the
-    // database. Future works might store temporary, intermediate states inside this function.
+    // database. Future works might store intermediate states inside this function.
     match state.slot().cmp(&state.latest_block_header().slot) {
         std::cmp::Ordering::Equal => (),
         std::cmp::Ordering::Greater => {
@@ -432,20 +431,13 @@ fn advance_head<T: BeaconChainTypes>(beacon_chain: &Arc<BeaconChain<T>>) -> Resu
         );
     }
 
-    // Write the advanced state to the database with a temporary flag that will be deleted when
-    // a block is imported on top of this state. We should delete this once we bring in the DB
-    // changes from tree-states that allow us to prune states without temporary flags.
+    // Write the advanced state to the database.
+    // We no longer use a transaction lock here when checking whether the state exists, because
+    // even if we race with the deletion of this state by the finalization pruning code, the worst
+    // case is we end up with a finalized state stored, that will get pruned the next time pruning
+    // runs.
     let advanced_state_root = state.update_tree_hash_cache()?;
-    let txn_lock = beacon_chain.store.hot_db.begin_rw_transaction();
-    let state_already_exists = beacon_chain
-        .store
-        .load_hot_state_summary(&advanced_state_root)?
-        .is_some();
-    let temporary = !state_already_exists;
-    beacon_chain
-        .store
-        .put_state_possibly_temporary(&advanced_state_root, &state, temporary)?;
-    drop(txn_lock);
+    beacon_chain.store.put_state(&advanced_state_root, &state)?;
 
     debug!(
         ?head_block_root,

beacon_node/store/src/database/leveldb_impl.rs

@@ -195,7 +195,6 @@ impl<E: EthSpec> LevelDB<E> {
         };
 
         for (start_key, end_key) in [
-            endpoints(DBColumn::BeaconStateTemporary),
             endpoints(DBColumn::BeaconState),
             endpoints(DBColumn::BeaconStateSummary),
         ] {

beacon_node/store/src/hot_cold_store.rs

@@ -944,26 +944,11 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
 
     /// Store a state in the store.
     pub fn put_state(&self, state_root: &Hash256, state: &BeaconState<E>) -> Result<(), Error> {
-        self.put_state_possibly_temporary(state_root, state, false)
-    }
-
-    /// Store a state in the store.
-    ///
-    /// The `temporary` flag indicates whether this state should be considered canonical.
-    pub fn put_state_possibly_temporary(
-        &self,
-        state_root: &Hash256,
-        state: &BeaconState<E>,
-        temporary: bool,
-    ) -> Result<(), Error> {
         let mut ops: Vec<KeyValueStoreOp> = Vec::new();
         if state.slot() < self.get_split_slot() {
             self.store_cold_state(state_root, state, &mut ops)?;
             self.cold_db.do_atomically(ops)
         } else {
-            if temporary {
-                ops.push(TemporaryFlag.as_kv_store_op(*state_root));
-            }
             self.store_hot_state(state_root, state, &mut ops)?;
             self.hot_db.do_atomically(ops)
         }
@@ -1208,17 +1193,6 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
                     key_value_batch.push(summary.as_kv_store_op(state_root));
                 }
 
-                StoreOp::PutStateTemporaryFlag(state_root) => {
-                    key_value_batch.push(TemporaryFlag.as_kv_store_op(state_root));
-                }
-
-                StoreOp::DeleteStateTemporaryFlag(state_root) => {
-                    key_value_batch.push(KeyValueStoreOp::DeleteKey(
-                        TemporaryFlag::db_column(),
-                        state_root.as_slice().to_vec(),
-                    ));
-                }
-
                 StoreOp::DeleteBlock(block_root) => {
                     key_value_batch.push(KeyValueStoreOp::DeleteKey(
                         DBColumn::BeaconBlock,
@@ -1248,13 +1222,6 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
                         state_root.as_slice().to_vec(),
                     ));
 
-                    // Delete the state temporary flag (if any). Temporary flags are commonly
-                    // created by the state advance routine.
-                    key_value_batch.push(KeyValueStoreOp::DeleteKey(
-                        DBColumn::BeaconStateTemporary,
-                        state_root.as_slice().to_vec(),
-                    ));
-
                     if let Some(slot) = slot {
                         match self.hot_storage_strategy(slot)? {
                             StorageStrategy::Snapshot => {
@@ -1440,10 +1407,6 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
 
                 StoreOp::PutStateSummary(_, _) => (),
 
-                StoreOp::PutStateTemporaryFlag(_) => (),
-
-                StoreOp::DeleteStateTemporaryFlag(_) => (),
-
                 StoreOp::DeleteBlock(block_root) => {
                     guard.delete_block(&block_root);
                     self.state_cache.lock().delete_block_states(&block_root);
@@ -1738,12 +1701,6 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
     ) -> Result<Option<(BeaconState<E>, Hash256)>, Error> {
         metrics::inc_counter(&metrics::BEACON_STATE_HOT_GET_COUNT);
 
-        // If the state is marked as temporary, do not return it. It will become visible
-        // only once its transaction commits and deletes its temporary flag.
-        if self.load_state_temporary_flag(state_root)?.is_some() {
-            return Ok(None);
-        }
-
         if let Some(HotStateSummary {
             slot,
             latest_block_root,
@@ -2804,17 +2761,6 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
             .collect()
     }
 
-    /// Load the temporary flag for a state root, if one exists.
-    ///
-    /// Returns `Some` if the state is temporary, or `None` if the state is permanent or does not
-    /// exist -- you should call `load_hot_state_summary` to find out which.
-    pub fn load_state_temporary_flag(
-        &self,
-        state_root: &Hash256,
-    ) -> Result<Option<TemporaryFlag>, Error> {
-        self.hot_db.get(state_root)
-    }
-
     /// Run a compaction pass to free up space used by deleted states.
     pub fn compact(&self) -> Result<(), Error> {
         self.hot_db.compact()?;
@@ -3549,23 +3495,6 @@ impl StoreItem for ColdStateSummary {
     }
 }
 
-#[derive(Debug, Clone, Copy, Default)]
-pub struct TemporaryFlag;
-
-impl StoreItem for TemporaryFlag {
-    fn db_column() -> DBColumn {
-        DBColumn::BeaconStateTemporary
-    }
-
-    fn as_store_bytes(&self) -> Vec<u8> {
-        vec![]
-    }
-
-    fn from_store_bytes(_: &[u8]) -> Result<Self, Error> {
-        Ok(TemporaryFlag)
-    }
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub struct BytesKey {
     pub key: Vec<u8>,

beacon_node/store/src/lib.rs

@@ -49,6 +49,12 @@ pub use types::*;
 
 const DATA_COLUMN_DB_KEY_SIZE: usize = 32 + 8;
 
+// TODO: check that these are not used by any variant of `DBColumn` using strum_macros
+const _DEPRECATED_COLUMN_PREFIXES: &[&str] = &[
+    // BeaconStateTemporary, deleted in Mar 2025.
+    "bst",
+];
+
 pub type ColumnIter<'a, K> = Box<dyn Iterator<Item = Result<(K, Vec<u8>), Error>> + 'a>;
 pub type ColumnKeyIter<'a, K> = Box<dyn Iterator<Item = Result<K, Error>> + 'a>;
 
@@ -249,8 +255,6 @@ pub enum StoreOp<'a, E: EthSpec> {
     PutBlobs(Hash256, BlobSidecarList<E>),
     PutDataColumns(Hash256, DataColumnSidecarList<E>),
     PutStateSummary(Hash256, HotStateSummary),
-    PutStateTemporaryFlag(Hash256),
-    DeleteStateTemporaryFlag(Hash256),
     DeleteBlock(Hash256),
     DeleteBlobs(Hash256),
     DeleteDataColumns(Hash256, Vec<ColumnIndex>),
@@ -315,12 +319,6 @@ pub enum DBColumn {
     /// Mapping from state root to `ColdStateSummary` in the cold DB.
     #[strum(serialize = "bcs")]
     BeaconColdStateSummary,
-    /// 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.
     #[strum(serialize = "exp")]
     ExecPayload,
@@ -422,7 +420,6 @@ impl DBColumn {
             | Self::BeaconStateHotSnapshot
             | Self::BeaconStateHotSummary
             | Self::BeaconColdStateSummary
-            | Self::BeaconStateTemporary
             | Self::ExecPayload
             | Self::BeaconChain
             | Self::OpPool