Prevent Overflow LRU Cache from Exploding (#4801)

* Initial Commit of State LRU Cache

* Build State Caches After Reconstruction

* Cleanup Duplicated Code in OverflowLRUCache Tests

* Added Test for State LRU Cache

* Prune Cache of Old States During Maintenance

* Address Michael's Comments

* Few More Comments

* Removed Unused impl

* Last touch up

* Fix Clippy

beacon_node/beacon_chain/src/block_verification_types.rs

@@ -5,15 +5,10 @@ pub use crate::data_availability_checker::{AvailableBlock, MaybeAvailableBlock};
 use crate::eth1_finalization_cache::Eth1FinalizationData;
 use crate::{get_block_root, GossipVerifiedBlock, PayloadVerificationOutcome};
 use derivative::Derivative;
-use ssz_derive::{Decode, Encode};
 use ssz_types::VariableList;
 use state_processing::ConsensusContext;
 use std::sync::Arc;
-use types::blob_sidecar::FixedBlobSidecarList;
-use types::{
-    blob_sidecar::BlobIdentifier, ssz_tagged_beacon_state, ssz_tagged_signed_beacon_block,
-    ssz_tagged_signed_beacon_block_arc,
-};
+use types::blob_sidecar::{BlobIdentifier, FixedBlobSidecarList};
 use types::{
     BeaconBlockRef, BeaconState, BlindedPayload, BlobSidecarList, Epoch, EthSpec, Hash256,
     SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
@@ -251,9 +246,7 @@ impl<E: EthSpec> AvailableExecutedBlock<E> {
 /// A block that has completed all pre-deneb block processing checks, verification
 /// by an EL client but does not have all requisite blob data to get imported into
 /// fork choice.
-#[derive(Encode, Decode, Clone)]
 pub struct AvailabilityPendingExecutedBlock<E: EthSpec> {
-    #[ssz(with = "ssz_tagged_signed_beacon_block_arc")]
     pub block: Arc<SignedBeaconBlock<E>>,
     pub import_data: BlockImportData<E>,
     pub payload_verification_outcome: PayloadVerificationOutcome,
@@ -285,14 +278,10 @@ impl<E: EthSpec> AvailabilityPendingExecutedBlock<E> {
     }
 }
 
-#[derive(Debug, PartialEq, Encode, Decode, Clone)]
-// TODO (mark): investigate using an Arc<state> / Arc<parent_block>
-//              here to make this cheaper to clone
+#[derive(Debug, PartialEq)]
 pub struct BlockImportData<E: EthSpec> {
     pub block_root: Hash256,
-    #[ssz(with = "ssz_tagged_beacon_state")]
     pub state: BeaconState<E>,
-    #[ssz(with = "ssz_tagged_signed_beacon_block")]
     pub parent_block: SignedBeaconBlock<E, BlindedPayload<E>>,
     pub parent_eth1_finalization_data: Eth1FinalizationData,
     pub confirmed_state_roots: Vec<Hash256>,

beacon_node/beacon_chain/src/data_availability_checker.rs

@@ -10,17 +10,14 @@ use crate::data_availability_checker::overflow_lru_cache::OverflowLRUCache;
 use crate::data_availability_checker::processing_cache::ProcessingCache;
 use crate::{BeaconChain, BeaconChainTypes, BeaconStore};
 use kzg::Kzg;
-use kzg::{Error as KzgError, KzgCommitment};
 use parking_lot::RwLock;
 pub use processing_cache::ProcessingComponents;
 use slasher::test_utils::E;
 use slog::{debug, error, Logger};
 use slot_clock::SlotClock;
-use ssz_types::Error;
 use std::fmt;
 use std::fmt::Debug;
 use std::sync::Arc;
-use strum::IntoStaticStr;
 use task_executor::TaskExecutor;
 use types::beacon_block_body::{KzgCommitmentOpts, KzgCommitments};
 use types::blob_sidecar::{BlobIdentifier, BlobSidecar, FixedBlobSidecarList};
@@ -29,54 +26,21 @@ use types::{BlobSidecarList, ChainSpec, Epoch, EthSpec, Hash256, SignedBeaconBlo
 
 mod availability_view;
 mod child_components;
+mod error;
 mod overflow_lru_cache;
 mod processing_cache;
+mod state_lru_cache;
+
+pub use error::{Error as AvailabilityCheckError, ErrorCategory as AvailabilityCheckErrorCategory};
 
 /// The LRU Cache stores `PendingComponents` which can store up to
 /// `MAX_BLOBS_PER_BLOCK = 6` blobs each. A `BlobSidecar` is 0.131256 MB. So
 /// the maximum size of a `PendingComponents` is ~ 0.787536 MB. Setting this
 /// to 1024 means the maximum size of the cache is ~ 0.8 GB. But the cache
 /// will target a size of less than 75% of capacity.
 pub const OVERFLOW_LRU_CAPACITY: usize = 1024;
-
-#[derive(Debug, IntoStaticStr)]
-pub enum AvailabilityCheckError {
-    Kzg(KzgError),
-    KzgNotInitialized,
-    KzgVerificationFailed,
-    KzgCommitmentMismatch {
-        blob_commitment: KzgCommitment,
-        block_commitment: KzgCommitment,
-    },
-    Unexpected,
-    SszTypes(ssz_types::Error),
-    MissingBlobs,
-    BlobIndexInvalid(u64),
-    StoreError(store::Error),
-    DecodeError(ssz::DecodeError),
-    InconsistentBlobBlockRoots {
-        block_root: Hash256,
-        blob_block_root: Hash256,
-    },
-}
-
-impl From<ssz_types::Error> for AvailabilityCheckError {
-    fn from(value: Error) -> Self {
-        Self::SszTypes(value)
-    }
-}
-
-impl From<store::Error> for AvailabilityCheckError {
-    fn from(value: store::Error) -> Self {
-        Self::StoreError(value)
-    }
-}
-
-impl From<ssz::DecodeError> for AvailabilityCheckError {
-    fn from(value: ssz::DecodeError) -> Self {
-        Self::DecodeError(value)
-    }
-}
+/// Until tree-states is implemented, we can't store very many states in memory :(
+pub const STATE_LRU_CAPACITY: usize = 2;
 
 /// This includes a cache for any blocks or blobs that have been received over gossip or RPC
 /// and are awaiting more components before they can be imported. Additionally the
@@ -120,7 +84,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
         log: &Logger,
         spec: ChainSpec,
     ) -> Result<Self, AvailabilityCheckError> {
-        let overflow_cache = OverflowLRUCache::new(OVERFLOW_LRU_CAPACITY, store)?;
+        let overflow_cache = OverflowLRUCache::new(OVERFLOW_LRU_CAPACITY, store, spec.clone())?;
         Ok(Self {
             processing_cache: <_>::default(),
             availability_cache: Arc::new(overflow_cache),

beacon_node/beacon_chain/src/data_availability_checker/availability_view.rs

@@ -1,9 +1,9 @@
 use super::child_components::ChildComponents;
+use super::state_lru_cache::DietAvailabilityPendingExecutedBlock;
 use crate::blob_verification::KzgVerifiedBlob;
 use crate::block_verification_types::AsBlock;
 use crate::data_availability_checker::overflow_lru_cache::PendingComponents;
 use crate::data_availability_checker::ProcessingComponents;
-use crate::AvailabilityPendingExecutedBlock;
 use kzg::KzgCommitment;
 use ssz_types::FixedVector;
 use std::sync::Arc;
@@ -190,7 +190,7 @@ impl_availability_view!(
 
 impl_availability_view!(
     PendingComponents,
-    AvailabilityPendingExecutedBlock<E>,
+    DietAvailabilityPendingExecutedBlock<E>,
     KzgVerifiedBlob<E>,
     executed_block,
     verified_blobs
@@ -225,7 +225,7 @@ impl<E: EthSpec> GetCommitment<E> for KzgCommitment {
 }
 
 // These implementations are required to implement `AvailabilityView` for `PendingComponents`.
-impl<E: EthSpec> GetCommitments<E> for AvailabilityPendingExecutedBlock<E> {
+impl<E: EthSpec> GetCommitments<E> for DietAvailabilityPendingExecutedBlock<E> {
     fn get_commitments(&self) -> KzgCommitments<E> {
         self.as_block()
             .message()
@@ -235,6 +235,7 @@ impl<E: EthSpec> GetCommitments<E> for AvailabilityPendingExecutedBlock<E> {
             .unwrap_or_default()
     }
 }
+
 impl<E: EthSpec> GetCommitment<E> for KzgVerifiedBlob<E> {
     fn get_commitment(&self) -> &KzgCommitment {
         &self.as_blob().kzg_commitment
@@ -264,6 +265,7 @@ pub mod tests {
     use crate::block_verification_types::BlockImportData;
     use crate::eth1_finalization_cache::Eth1FinalizationData;
     use crate::test_utils::{generate_rand_block_and_blobs, NumBlobs};
+    use crate::AvailabilityPendingExecutedBlock;
     use crate::PayloadVerificationOutcome;
     use eth2_network_config::get_trusted_setup;
     use fork_choice::PayloadVerificationStatus;
@@ -346,7 +348,7 @@ pub mod tests {
     }
 
     type PendingComponentsSetup<E> = (
-        AvailabilityPendingExecutedBlock<E>,
+        DietAvailabilityPendingExecutedBlock<E>,
         FixedVector<Option<KzgVerifiedBlob<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
         FixedVector<Option<KzgVerifiedBlob<E>>, <E as EthSpec>::MaxBlobsPerBlock>,
     );
@@ -395,7 +397,7 @@ pub mod tests {
                 is_valid_merge_transition_block: false,
             },
         };
-        (block, blobs, invalid_blobs)
+        (block.into(), blobs, invalid_blobs)
     }
 
     type ChildComponentsSetup<E> = (

beacon_node/beacon_chain/src/data_availability_checker/error.rs

@@ -0,0 +1,79 @@
+use kzg::{Error as KzgError, KzgCommitment};
+use strum::IntoStaticStr;
+use types::{BeaconStateError, Hash256};
+
+#[derive(Debug, IntoStaticStr)]
+pub enum Error {
+    Kzg(KzgError),
+    KzgNotInitialized,
+    KzgVerificationFailed,
+    KzgCommitmentMismatch {
+        blob_commitment: KzgCommitment,
+        block_commitment: KzgCommitment,
+    },
+    Unexpected,
+    SszTypes(ssz_types::Error),
+    MissingBlobs,
+    BlobIndexInvalid(u64),
+    StoreError(store::Error),
+    DecodeError(ssz::DecodeError),
+    InconsistentBlobBlockRoots {
+        block_root: Hash256,
+        blob_block_root: Hash256,
+    },
+    ParentStateMissing(Hash256),
+    BlockReplayError(state_processing::BlockReplayError),
+    RebuildingStateCaches(BeaconStateError),
+}
+
+pub enum ErrorCategory {
+    /// Internal Errors (not caused by peers)
+    Internal,
+    /// Errors caused by faulty / malicious peers
+    Malicious,
+}
+
+impl Error {
+    pub fn category(&self) -> ErrorCategory {
+        match self {
+            Error::KzgNotInitialized
+            | Error::SszTypes(_)
+            | Error::MissingBlobs
+            | Error::StoreError(_)
+            | Error::DecodeError(_)
+            | Error::Unexpected
+            | Error::ParentStateMissing(_)
+            | Error::BlockReplayError(_)
+            | Error::RebuildingStateCaches(_) => ErrorCategory::Internal,
+            Error::Kzg(_)
+            | Error::BlobIndexInvalid(_)
+            | Error::KzgCommitmentMismatch { .. }
+            | Error::KzgVerificationFailed
+            | Error::InconsistentBlobBlockRoots { .. } => ErrorCategory::Malicious,
+        }
+    }
+}
+
+impl From<ssz_types::Error> for Error {
+    fn from(value: ssz_types::Error) -> Self {
+        Self::SszTypes(value)
+    }
+}
+
+impl From<store::Error> for Error {
+    fn from(value: store::Error) -> Self {
+        Self::StoreError(value)
+    }
+}
+
+impl From<ssz::DecodeError> for Error {
+    fn from(value: ssz::DecodeError) -> Self {
+        Self::DecodeError(value)
+    }
+}
+
+impl From<state_processing::BlockReplayError> for Error {
+    fn from(value: state_processing::BlockReplayError) -> Self {
+        Self::BlockReplayError(value)
+    }
+}