Fix on-disk consensus context format (#5598)

* Fix on-disk consensus context format

* Keep indexed attestations, thanks Sean

beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs

@@ -8,8 +8,9 @@ use crate::{
 use lru::LruCache;
 use parking_lot::RwLock;
 use ssz_derive::{Decode, Encode};
-use state_processing::{BlockReplayer, ConsensusContext, StateProcessingStrategy};
+use state_processing::{BlockReplayer, StateProcessingStrategy};
 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};
@@ -26,7 +27,7 @@ pub struct DietAvailabilityPendingExecutedBlock<E: EthSpec> {
     parent_block: SignedBeaconBlock<E, BlindedPayload<E>>,
     parent_eth1_finalization_data: Eth1FinalizationData,
     confirmed_state_roots: Vec<Hash256>,
-    consensus_context: ConsensusContext<E>,
+    consensus_context: OnDiskConsensusContext<E>,
     payload_verification_outcome: PayloadVerificationOutcome,
 }
 
@@ -94,7 +95,9 @@ impl<T: BeaconChainTypes> StateLRUCache<T> {
             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: executed_block.import_data.consensus_context,
+            consensus_context: OnDiskConsensusContext::from_consensus_context(
+                executed_block.import_data.consensus_context,
+            ),
             payload_verification_outcome: executed_block.payload_verification_outcome,
         }
     }
@@ -119,7 +122,9 @@ impl<T: BeaconChainTypes> StateLRUCache<T> {
                     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,
+                    consensus_context: diet_executed_block
+                        .consensus_context
+                        .into_consensus_context(),
                 },
                 payload_verification_outcome: diet_executed_block.payload_verification_outcome,
             })
@@ -145,7 +150,9 @@ impl<T: BeaconChainTypes> StateLRUCache<T> {
                 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,
+                consensus_context: diet_executed_block
+                    .consensus_context
+                    .into_consensus_context(),
             },
             payload_verification_outcome: diet_executed_block.payload_verification_outcome,
         })
@@ -232,7 +239,9 @@ impl<E: EthSpec> From<AvailabilityPendingExecutedBlock<E>>
             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: value.import_data.consensus_context,
+            consensus_context: OnDiskConsensusContext::from_consensus_context(
+                value.import_data.consensus_context,
+            ),
             payload_verification_outcome: value.payload_verification_outcome,
         }
     }

beacon_node/store/src/consensus_context.rs

@@ -0,0 +1,66 @@
+use ssz_derive::{Decode, Encode};
+use state_processing::ConsensusContext;
+use std::collections::HashMap;
+use types::{AttestationData, BitList, EthSpec, Hash256, IndexedAttestation, Slot};
+
+/// The consensus context is stored on disk as part of the data availability overflow cache.
+///
+/// We use this separate struct to keep the on-disk format stable in the presence of changes to the
+/// in-memory `ConsensusContext`. You MUST NOT change the fields of this struct without
+/// superstructing it and implementing a schema migration.
+#[derive(Debug, PartialEq, Clone, Encode, Decode)]
+pub struct OnDiskConsensusContext<E: EthSpec> {
+    /// Slot to act as an identifier/safeguard
+    slot: Slot,
+    /// Proposer index of the block at `slot`.
+    proposer_index: Option<u64>,
+    /// Block root of the block at `slot`.
+    current_block_root: Option<Hash256>,
+    /// We keep the indexed attestations in the *in-memory* version of this struct so that we don't
+    /// need to regenerate them if roundtripping via this type *without* going to disk.
+    ///
+    /// They are not part of the on-disk format.
+    #[ssz(skip_serializing, skip_deserializing)]
+    indexed_attestations:
+        HashMap<(AttestationData, BitList<E::MaxValidatorsPerCommittee>), IndexedAttestation<E>>,
+}
+
+impl<E: EthSpec> OnDiskConsensusContext<E> {
+    pub fn from_consensus_context(ctxt: ConsensusContext<E>) -> Self {
+        // Match exhaustively on fields here so we are forced to *consider* updating the on-disk
+        // format when the `ConsensusContext` fields change.
+        let ConsensusContext {
+            slot,
+            previous_epoch: _,
+            current_epoch: _,
+            proposer_index,
+            current_block_root,
+            indexed_attestations,
+        } = ctxt;
+        OnDiskConsensusContext {
+            slot,
+            proposer_index,
+            current_block_root,
+            indexed_attestations,
+        }
+    }
+
+    pub fn into_consensus_context(self) -> ConsensusContext<E> {
+        let OnDiskConsensusContext {
+            slot,
+            proposer_index,
+            current_block_root,
+            indexed_attestations,
+        } = self;
+
+        let mut ctxt = ConsensusContext::new(slot);
+
+        if let Some(proposer_index) = proposer_index {
+            ctxt = ctxt.set_proposer_index(proposer_index);
+        }
+        if let Some(block_root) = current_block_root {
+            ctxt = ctxt.set_current_block_root(block_root);
+        }
+        ctxt.set_indexed_attestations(indexed_attestations)
+    }
+}

beacon_node/store/src/lib.rs

@@ -14,6 +14,7 @@ mod chunk_writer;
 pub mod chunked_iter;
 pub mod chunked_vector;
 pub mod config;
+pub mod consensus_context;
 pub mod errors;
 mod forwards_iter;
 mod garbage_collection;
@@ -30,6 +31,7 @@ pub mod iter;
 
 pub use self::chunk_writer::ChunkWriter;
 pub use self::config::StoreConfig;
+pub use self::consensus_context::OnDiskConsensusContext;
 pub use self::hot_cold_store::{HotColdDB, HotStateSummary, Split};
 pub use self::leveldb_store::LevelDB;
 pub use self::memory_store::MemoryStore;

consensus/state_processing/src/consensus_context.rs

@@ -1,7 +1,6 @@
 use crate::common::get_indexed_attestation;
 use crate::per_block_processing::errors::{AttestationInvalid, BlockOperationError};
 use crate::EpochCacheError;
-use ssz_derive::{Decode, Encode};
 use std::collections::{hash_map::Entry, HashMap};
 use tree_hash::TreeHash;
 use types::{
@@ -9,22 +8,20 @@ use types::{
     ChainSpec, Epoch, EthSpec, Hash256, IndexedAttestation, SignedBeaconBlock, Slot,
 };
 
-#[derive(Debug, PartialEq, Clone, Encode, Decode)]
+#[derive(Debug, PartialEq, Clone)]
 pub struct ConsensusContext<E: EthSpec> {
     /// Slot to act as an identifier/safeguard
-    slot: Slot,
+    pub slot: Slot,
     /// Previous epoch of the `slot` precomputed for optimization purpose.
-    pub(crate) previous_epoch: Epoch,
+    pub previous_epoch: Epoch,
     /// Current epoch of the `slot` precomputed for optimization purpose.
-    pub(crate) current_epoch: Epoch,
+    pub current_epoch: Epoch,
     /// Proposer index of the block at `slot`.
-    proposer_index: Option<u64>,
+    pub proposer_index: Option<u64>,
     /// Block root of the block at `slot`.
-    current_block_root: Option<Hash256>,
+    pub current_block_root: Option<Hash256>,
     /// Cache of indexed attestations constructed during block processing.
-    /// We can skip serializing / deserializing this as the cache will just be rebuilt
+    pub indexed_attestations:
-    #[ssz(skip_serializing, skip_deserializing)]
-    indexed_attestations:
         HashMap<(AttestationData, BitList<E::MaxValidatorsPerCommittee>), IndexedAttestation<E>>,
 }
 
@@ -62,6 +59,7 @@ impl<E: EthSpec> ConsensusContext<E> {
         }
     }
 
+    #[must_use]
     pub fn set_proposer_index(mut self, proposer_index: u64) -> Self {
         self.proposer_index = Some(proposer_index);
         self
@@ -109,6 +107,7 @@ impl<E: EthSpec> ConsensusContext<E> {
         Ok(proposer_index)
     }
 
+    #[must_use]
     pub fn set_current_block_root(mut self, block_root: Hash256) -> Self {
         self.current_block_root = Some(block_root);
         self
@@ -174,4 +173,16 @@ impl<E: EthSpec> ConsensusContext<E> {
     pub fn num_cached_indexed_attestations(&self) -> usize {
         self.indexed_attestations.len()
     }
+
+    #[must_use]
+    pub fn set_indexed_attestations(
+        mut self,
+        attestations: HashMap<
+            (AttestationData, BitList<E::MaxValidatorsPerCommittee>),
+            IndexedAttestation<E>,
+        >,
+    ) -> Self {
+        self.indexed_attestations = attestations;
+        self
+    }
 }