From 3a16649023e2d6d40d704dbe958cec712544aba9 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 18 Apr 2024 14:26:09 +1000 Subject: [PATCH 1/2] Fix on-disk consensus context format --- .../state_lru_cache.rs | 21 +++++-- beacon_node/store/src/consensus_context.rs | 56 +++++++++++++++++++ beacon_node/store/src/lib.rs | 2 + .../state_processing/src/consensus_context.rs | 17 +++--- 4 files changed, 80 insertions(+), 16 deletions(-) create mode 100644 beacon_node/store/src/consensus_context.rs diff --git a/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs index c3492b53bda..75993f5ab54 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs +++ b/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 { parent_block: SignedBeaconBlock>, parent_eth1_finalization_data: Eth1FinalizationData, confirmed_state_roots: Vec, - consensus_context: ConsensusContext, + consensus_context: OnDiskConsensusContext, payload_verification_outcome: PayloadVerificationOutcome, } @@ -94,7 +95,9 @@ impl StateLRUCache { 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 StateLRUCache { 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 StateLRUCache { 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 From> 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, } } diff --git a/beacon_node/store/src/consensus_context.rs b/beacon_node/store/src/consensus_context.rs new file mode 100644 index 00000000000..0dd6635bd6e --- /dev/null +++ b/beacon_node/store/src/consensus_context.rs @@ -0,0 +1,56 @@ +use ssz_derive::{Decode, Encode}; +use state_processing::ConsensusContext; +use types::{EthSpec, Hash256, 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 { + /// Slot to act as an identifier/safeguard + slot: Slot, + /// Proposer index of the block at `slot`. + proposer_index: Option, + /// Block root of the block at `slot`. + current_block_root: Option, +} + +impl OnDiskConsensusContext { + pub fn from_consensus_context(ctxt: &ConsensusContext) -> 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, + } + } + + pub fn into_consensus_context(self) -> ConsensusContext { + let OnDiskConsensusContext { + slot, + proposer_index, + current_block_root, + } = 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 + } +} diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index e86689b0cf1..c3136a910db 100644 --- a/beacon_node/store/src/lib.rs +++ b/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; diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs index 263539fa429..68659e367f0 100644 --- a/consensus/state_processing/src/consensus_context.rs +++ b/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 { /// 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, + pub proposer_index: Option, /// Block root of the block at `slot`. - current_block_root: Option, + pub current_block_root: Option, /// Cache of indexed attestations constructed during block processing. - /// We can skip serializing / deserializing this as the cache will just be rebuilt - #[ssz(skip_serializing, skip_deserializing)] - indexed_attestations: + pub indexed_attestations: HashMap<(AttestationData, BitList), IndexedAttestation>, } From 62ebdacd0874a67a5451e567219866a96077c242 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 19 Apr 2024 10:13:49 +1000 Subject: [PATCH 2/2] Keep indexed attestations, thanks Sean --- .../state_lru_cache.rs | 6 ++--- beacon_node/store/src/consensus_context.rs | 26 +++++++++++++------ .../state_processing/src/consensus_context.rs | 14 ++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs b/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs index 75993f5ab54..b6dbf2b952f 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/state_lru_cache.rs @@ -27,7 +27,7 @@ pub struct DietAvailabilityPendingExecutedBlock { parent_block: SignedBeaconBlock>, parent_eth1_finalization_data: Eth1FinalizationData, confirmed_state_roots: Vec, - consensus_context: OnDiskConsensusContext, + consensus_context: OnDiskConsensusContext, payload_verification_outcome: PayloadVerificationOutcome, } @@ -96,7 +96,7 @@ impl StateLRUCache { 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, + executed_block.import_data.consensus_context, ), payload_verification_outcome: executed_block.payload_verification_outcome, } @@ -240,7 +240,7 @@ impl From> 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, + value.import_data.consensus_context, ), payload_verification_outcome: value.payload_verification_outcome, } diff --git a/beacon_node/store/src/consensus_context.rs b/beacon_node/store/src/consensus_context.rs index 0dd6635bd6e..08fad17b14b 100644 --- a/beacon_node/store/src/consensus_context.rs +++ b/beacon_node/store/src/consensus_context.rs @@ -1,6 +1,7 @@ use ssz_derive::{Decode, Encode}; use state_processing::ConsensusContext; -use types::{EthSpec, Hash256, Slot}; +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. /// @@ -8,39 +9,48 @@ use types::{EthSpec, Hash256, Slot}; /// 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 { +pub struct OnDiskConsensusContext { /// Slot to act as an identifier/safeguard slot: Slot, /// Proposer index of the block at `slot`. proposer_index: Option, /// Block root of the block at `slot`. current_block_root: Option, + /// 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), IndexedAttestation>, } -impl OnDiskConsensusContext { - pub fn from_consensus_context(ctxt: &ConsensusContext) -> Self { +impl OnDiskConsensusContext { + pub fn from_consensus_context(ctxt: ConsensusContext) -> Self { // Match exhaustively on fields here so we are forced to *consider* updating the on-disk // format when the `ConsensusContext` fields change. - let &ConsensusContext { + let ConsensusContext { slot, previous_epoch: _, current_epoch: _, proposer_index, current_block_root, - indexed_attestations: _, + indexed_attestations, } = ctxt; OnDiskConsensusContext { slot, proposer_index, current_block_root, + indexed_attestations, } } - pub fn into_consensus_context(self) -> ConsensusContext { + pub fn into_consensus_context(self) -> ConsensusContext { let OnDiskConsensusContext { slot, proposer_index, current_block_root, + indexed_attestations, } = self; let mut ctxt = ConsensusContext::new(slot); @@ -51,6 +61,6 @@ impl OnDiskConsensusContext { if let Some(block_root) = current_block_root { ctxt = ctxt.set_current_block_root(block_root); } - ctxt + ctxt.set_indexed_attestations(indexed_attestations) } } diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs index 68659e367f0..073d87be85b 100644 --- a/consensus/state_processing/src/consensus_context.rs +++ b/consensus/state_processing/src/consensus_context.rs @@ -59,6 +59,7 @@ impl ConsensusContext { } } + #[must_use] pub fn set_proposer_index(mut self, proposer_index: u64) -> Self { self.proposer_index = Some(proposer_index); self @@ -106,6 +107,7 @@ impl ConsensusContext { 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 @@ -171,4 +173,16 @@ impl ConsensusContext { 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), + IndexedAttestation, + >, + ) -> Self { + self.indexed_attestations = attestations; + self + } }