diff --git a/Cargo.lock b/Cargo.lock index 8a282a60b79..3bc2fb5d135 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3580,6 +3580,7 @@ dependencies = [ "logging", "metrics", "proto_array", + "safe_arith", "state_processing", "store", "superstruct", diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 58532116e6c..17aba466ca0 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -71,8 +71,8 @@ use crate::validator_monitor::{ }; use crate::validator_pubkey_cache::ValidatorPubkeyCache; use crate::{ - AvailabilityPendingExecutedBlock, BeaconChainError, BeaconForkChoiceStore, BeaconSnapshot, - CachedHead, metrics, + AvailabilityPendingExecutedBlock, BalancesCache, BeaconChainError, BeaconSnapshot, CachedHead, + metrics, }; use eth2::types::{ EventKind, SseBlobSidecar, SseBlock, SseDataColumnSidecar, SseExtendedPayloadAttributes, @@ -134,7 +134,7 @@ use types::data_column_sidecar::ColumnIndex; use types::payload::BlockProductionVersion; use types::*; -pub type ForkChoiceError = fork_choice::Error; +pub type ForkChoiceError = fork_choice::Error; /// Alias to appease clippy. type HashBlockTuple = (Hash256, RpcBlock); @@ -344,7 +344,7 @@ pub enum BlockProcessStatus { pub type LightClientProducerEvent = (Hash256, Slot, SyncAggregate); pub type BeaconForkChoice = ForkChoice< - BeaconForkChoiceStore< + BalancesCache< ::EthSpec, ::HotStore, ::ColdStore, @@ -619,13 +619,11 @@ impl BeaconChain { let persisted_fork_choice = PersistedForkChoice::from_bytes(&persisted_fork_choice_bytes, store.get_config())?; - let fc_store = - BeaconForkChoiceStore::from_persisted(persisted_fork_choice.fork_choice_store, store)?; Ok(Some(ForkChoice::from_persisted( persisted_fork_choice.fork_choice, reset_payload_statuses, - fc_store, + BalancesCache::new(store.clone()), spec, )?)) } diff --git a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs index 440388661c2..c80c7d1d4a7 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -4,9 +4,9 @@ //! Additionally, the `BalancesCache` struct is defined; a cache designed to avoid database //! reads when fork choice requires the validator balances of the justified state. -use crate::{BeaconSnapshot, metrics}; +use crate::BeaconSnapshot; use derivative::Derivative; -use fork_choice::ForkChoiceStore; +use fork_choice::{BalancesGetter, Error as ForkChoiceError}; use proto_array::JustifiedBalances; use safe_arith::ArithError; use ssz_derive::{Decode, Encode}; @@ -16,8 +16,7 @@ use std::sync::Arc; use store::{Error as StoreError, HotColdDB, ItemStore}; use superstruct::superstruct; use types::{ - AbstractExecPayload, BeaconBlockRef, BeaconState, BeaconStateError, Checkpoint, Epoch, EthSpec, - FixedBytesExtended, Hash256, Slot, + BeaconState, BeaconStateError, Checkpoint, Epoch, EthSpec, FixedBytesExtended, Hash256, Slot, }; #[derive(Debug)] @@ -64,17 +63,22 @@ pub(crate) type CacheItem = CacheItemV8; variant_attributes(derive(PartialEq, Clone, Default, Debug, Encode, Decode)), no_enum )] -pub struct BalancesCache { +pub struct BalancesCacheInner { pub(crate) items: Vec, } -pub type BalancesCache = BalancesCacheV8; +pub type BalancesCacheInner = BalancesCacheInnerV8; -impl BalancesCache { +pub struct BalancesCache, Cold: ItemStore> { + pub(crate) cache: BalancesCacheInner, + pub(crate) store: Arc>, +} + +impl BalancesCacheInner { /// Inspect the given `state` and determine the root of the block at the first slot of /// `state.current_epoch`. If there is not already some entry for the given block root, then /// add the effective balances from the `state` to the cache. - pub fn process_state( + fn process_state( &mut self, block_root: Hash256, state: &BeaconState, @@ -119,12 +123,48 @@ impl BalancesCache { /// Get the balances for the given `block_root`, if any. /// /// If some balances are found, they are cloned from the cache. - pub fn get(&mut self, block_root: Hash256, epoch: Epoch) -> Option> { + fn get(&self, block_root: Hash256, epoch: Epoch) -> Option> { let i = self.position(block_root, epoch)?; Some(self.items[i].balances.clone()) } } +impl, Cold: ItemStore> BalancesCache { + pub fn new(store: Arc>) -> Self { + Self { + store, + cache: <_>::default(), + } + } +} + +impl, Cold: ItemStore> BalancesGetter + for BalancesCache +{ + fn get_balances( + &self, + state_root: Hash256, + checkpoint: Checkpoint, + ) -> Result { + if let Some(balances) = self.cache.get(checkpoint.root, checkpoint.epoch) { + return Ok(JustifiedBalances::from_effective_balances(balances) + .map_err(|e| format!("Invalid cached balances {e:?}"))?); + } + + let state_slot = checkpoint.epoch.start_slot(E::slots_per_epoch()); + let state = self + .store + .get_state(&state_root, Some(state_slot), false) + .map_err(|e| format!("Error getting state {state_root:?} {e:?}"))? + .ok_or_else(|| { + ForkChoiceError::BalancesGetterError(format!("Missing state {state_root:?}")) + })?; + let balances = JustifiedBalances::from_justified_state(&state) + .map_err(|e| format!("Invalid state balances {e:?}"))?; + return Ok(balances); + } +} + /// Implements `fork_choice::ForkChoiceStore` in order to provide a persistent backing to the /// `fork_choice::ForkChoice` struct. #[derive(Debug, Derivative)] @@ -132,7 +172,7 @@ impl BalancesCache { pub struct BeaconForkChoiceStore, Cold: ItemStore> { #[derivative(PartialEq = "ignore")] store: Arc>, - balances_cache: BalancesCache, + balances_cache: BalancesCacheInner, time: Slot, finalized_checkpoint: Checkpoint, justified_checkpoint: Checkpoint, @@ -292,121 +332,6 @@ where } } -impl ForkChoiceStore for BeaconForkChoiceStore -where - E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, -{ - type Error = Error; - - fn get_current_slot(&self) -> Slot { - self.time - } - - fn set_current_slot(&mut self, slot: Slot) { - self.time = slot - } - - fn on_verified_block>( - &mut self, - _block: BeaconBlockRef, - block_root: Hash256, - state: &BeaconState, - ) -> Result<(), Self::Error> { - self.balances_cache.process_state(block_root, state) - } - - fn justified_checkpoint(&self) -> &Checkpoint { - &self.justified_checkpoint - } - - fn justified_state_root(&self) -> Hash256 { - self.justified_state_root - } - - fn justified_balances(&self) -> &JustifiedBalances { - &self.justified_balances - } - - fn finalized_checkpoint(&self) -> &Checkpoint { - &self.finalized_checkpoint - } - - fn unrealized_justified_checkpoint(&self) -> &Checkpoint { - &self.unrealized_justified_checkpoint - } - - fn unrealized_justified_state_root(&self) -> Hash256 { - self.unrealized_justified_state_root - } - - fn unrealized_finalized_checkpoint(&self) -> &Checkpoint { - &self.unrealized_finalized_checkpoint - } - - fn proposer_boost_root(&self) -> Hash256 { - self.proposer_boost_root - } - - fn set_finalized_checkpoint(&mut self, checkpoint: Checkpoint) { - self.finalized_checkpoint = checkpoint - } - - fn set_justified_checkpoint( - &mut self, - checkpoint: Checkpoint, - justified_state_root: Hash256, - ) -> Result<(), Error> { - self.justified_checkpoint = checkpoint; - self.justified_state_root = justified_state_root; - - if let Some(balances) = self.balances_cache.get( - self.justified_checkpoint.root, - self.justified_checkpoint.epoch, - ) { - // NOTE: could avoid this re-calculation by introducing a `PersistedCacheItem`. - metrics::inc_counter(&metrics::BALANCES_CACHE_HITS); - self.justified_balances = JustifiedBalances::from_effective_balances(balances)?; - } else { - metrics::inc_counter(&metrics::BALANCES_CACHE_MISSES); - - // Justified state is reasonably useful to cache, it might be finalized soon. - let update_cache = true; - let state = self - .store - .get_hot_state(&self.justified_state_root, update_cache) - .map_err(Error::FailedToReadState)? - .ok_or(Error::MissingState(self.justified_state_root))?; - - self.justified_balances = JustifiedBalances::from_justified_state(&state)?; - } - - Ok(()) - } - - fn set_unrealized_justified_checkpoint(&mut self, checkpoint: Checkpoint, state_root: Hash256) { - self.unrealized_justified_checkpoint = checkpoint; - self.unrealized_justified_state_root = state_root; - } - - fn set_unrealized_finalized_checkpoint(&mut self, checkpoint: Checkpoint) { - self.unrealized_finalized_checkpoint = checkpoint; - } - - fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256) { - self.proposer_boost_root = proposer_boost_root; - } - - fn equivocating_indices(&self) -> &BTreeSet { - &self.equivocating_indices - } - - fn extend_equivocating_indices(&mut self, indices: impl IntoIterator) { - self.equivocating_indices.extend(indices); - } -} - pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV28; /// A container which allows persisting the `BeaconForkChoiceStore` to the on-disk database. @@ -418,7 +343,7 @@ pub type PersistedForkChoiceStore = PersistedForkChoiceStoreV28; pub struct PersistedForkChoiceStore { /// The balances cache was removed from disk storage in schema V28. #[superstruct(only(V17))] - pub balances_cache: BalancesCacheV8, + pub balances_cache: BalancesCacheInnerV8, pub time: Slot, pub finalized_checkpoint: Checkpoint, pub justified_checkpoint: Checkpoint, diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 719c24b9561..8712813945d 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -1,8 +1,9 @@ use crate::ChainConfig; use crate::CustodyContext; use crate::beacon_chain::{ - BEACON_CHAIN_DB_KEY, CanonicalHead, LightClientProducerEvent, OP_POOL_DB_KEY, + BEACON_CHAIN_DB_KEY, BeaconForkChoice, CanonicalHead, LightClientProducerEvent, OP_POOL_DB_KEY, }; +use crate::beacon_fork_choice_store::BalancesCache; use crate::beacon_proposer_cache::BeaconProposerCache; use crate::custody_context::NodeCustodyType; use crate::data_availability_checker::DataAvailabilityChecker; @@ -18,9 +19,7 @@ use crate::persisted_custody::load_custody_context; use crate::shuffling_cache::{BlockShufflingIds, ShufflingCache}; use crate::validator_monitor::{ValidatorMonitor, ValidatorMonitorConfig}; use crate::validator_pubkey_cache::ValidatorPubkeyCache; -use crate::{ - BeaconChain, BeaconChainTypes, BeaconForkChoiceStore, BeaconSnapshot, ServerSentEventHandler, -}; +use crate::{BeaconChain, BeaconChainTypes, BeaconSnapshot, ServerSentEventHandler}; use execution_layer::ExecutionLayer; use fork_choice::{ForkChoice, ResetPayloadStatuses}; use futures::channel::mpsc::Sender; @@ -81,9 +80,7 @@ pub struct BeaconChainBuilder { genesis_block_root: Option, genesis_state_root: Option, #[allow(clippy::type_complexity)] - fork_choice: Option< - ForkChoice, T::EthSpec>, - >, + fork_choice: Option>, op_pool: Option>, execution_layer: Option>, event_handler: Option>, @@ -380,7 +377,7 @@ where .map_err(|e| format!("Failed to initialize genesis anchor: {:?}", e))?, ); - let (genesis, updated_builder) = self.set_genesis_state(beacon_state)?; + let (mut genesis, updated_builder) = self.set_genesis_state(beacon_state)?; self = updated_builder; // Stage the database's metadata fields for atomic storage when `build` is called. @@ -395,16 +392,15 @@ where .map_err(|e| format!("Failed to initialize genesis data column info: {:?}", e))?, ); - let fc_store = BeaconForkChoiceStore::get_forkchoice_store(store, genesis.clone()) - .map_err(|e| format!("Unable to initialize fork choice store: {e:?}"))?; - let current_slot = None; + let balances_getter = BalancesCache::new(store); + let current_slot = Slot::new(0); let fork_choice = ForkChoice::from_anchor( - fc_store, genesis.beacon_block_root, &genesis.beacon_block, - &genesis.beacon_state, + &mut genesis.beacon_state, current_slot, + balances_getter, &self.spec, ) .map_err(|e| format!("Unable to initialize ForkChoice: {:?}", e))?; @@ -611,21 +607,14 @@ where .map_err(|e| format!("Failed to initialize data column info: {:?}", e))?, ); - let snapshot = BeaconSnapshot { - beacon_block_root: weak_subj_block_root, - beacon_block: Arc::new(weak_subj_block), - beacon_state: weak_subj_state, - }; - - let fc_store = BeaconForkChoiceStore::get_forkchoice_store(store, snapshot.clone()) - .map_err(|e| format!("Unable to initialize fork choice store: {e:?}"))?; + let balances_getter = BalancesCache::new(store); let fork_choice = ForkChoice::from_anchor( - fc_store, - snapshot.beacon_block_root, - &snapshot.beacon_block, - &snapshot.beacon_state, - Some(weak_subj_slot), + weak_subj_block_root, + &weak_subj_block, + &mut weak_subj_state, + weak_subj_slot, + balances_getter, &self.spec, ) .map_err(|e| format!("Unable to initialize ForkChoice: {:?}", e))?; @@ -797,7 +786,7 @@ where head_block_root, &head_state, store.clone(), - Some(current_slot), + current_slot, &self.spec, )?; } diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 7dd4c88c513..ac8bf5b7427 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -43,8 +43,7 @@ use crate::{ }; use eth2::types::{EventKind, SseChainReorg, SseFinalizedCheckpoint, SseHead, SseLateHead}; use fork_choice::{ - ExecutionStatus, ForkChoiceStore, ForkChoiceView, ForkchoiceUpdateParameters, ProtoBlock, - ResetPayloadStatuses, + ExecutionStatus, ForkChoiceView, ForkchoiceUpdateParameters, ProtoBlock, ResetPayloadStatuses, }; use itertools::process_results; use lighthouse_tracing::SPAN_RECOMPUTE_HEAD; @@ -304,7 +303,7 @@ impl CanonicalHead { let beacon_block = store .get_full_block(&beacon_block_root)? .ok_or(Error::MissingBeaconBlock(beacon_block_root))?; - let current_slot = fork_choice.fc_store().get_current_slot(); + let current_slot = fork_choice.get_current_slot(); let (_, beacon_state) = store .get_advanced_hot_state(beacon_block_root, current_slot, beacon_block.state_root())? .ok_or(Error::MissingBeaconState(beacon_block.state_root()))?; diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 9dc6e897fb1..62dc49b85ca 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -215,7 +215,7 @@ pub enum BeaconChainError { BlsToExecutionPriorToCapella, BlsToExecutionConflictsWithPool, InconsistentFork(InconsistentFork), - ProposerHeadForkChoiceError(fork_choice::Error), + ProposerHeadForkChoiceError(fork_choice::Error), UnableToPublish, UnableToBuildColumnSidecar(String), AvailabilityCheckError(AvailabilityCheckError), diff --git a/beacon_node/beacon_chain/src/fork_revert.rs b/beacon_node/beacon_chain/src/fork_revert.rs index 4db79790d38..9102207b765 100644 --- a/beacon_node/beacon_chain/src/fork_revert.rs +++ b/beacon_node/beacon_chain/src/fork_revert.rs @@ -1,4 +1,4 @@ -use crate::{BeaconForkChoiceStore, BeaconSnapshot}; +use crate::BalancesCache; use fork_choice::{ForkChoice, PayloadVerificationStatus}; use itertools::process_results; use state_processing::state_advance::complete_state_advance; @@ -95,9 +95,9 @@ pub fn reset_fork_choice_to_finalization, Cold: It head_block_root: Hash256, head_state: &BeaconState, store: Arc>, - current_slot: Option, + current_slot: Slot, spec: &ChainSpec, -) -> Result, E>, String> { +) -> Result, E>, String> { // Fetch finalized block. let finalized_checkpoint = head_state.finalized_checkpoint(); let finalized_block_root = finalized_checkpoint.root; @@ -136,22 +136,13 @@ pub fn reset_fork_choice_to_finalization, Cold: It e ) })?; - let finalized_snapshot = BeaconSnapshot { - beacon_block_root: finalized_block_root, - beacon_block: Arc::new(finalized_block), - beacon_state: finalized_state, - }; - - let fc_store = - BeaconForkChoiceStore::get_forkchoice_store(store.clone(), finalized_snapshot.clone()) - .map_err(|e| format!("Unable to reset fork choice store for revert: {e:?}"))?; let mut fork_choice = ForkChoice::from_anchor( - fc_store, finalized_block_root, - &finalized_snapshot.beacon_block, - &finalized_snapshot.beacon_state, + &finalized_block, + &mut finalized_state, current_slot, + BalancesCache::new(store.clone()), spec, ) .map_err(|e| format!("Unable to reset fork choice for revert: {:?}", e))?; @@ -163,7 +154,7 @@ pub fn reset_fork_choice_to_finalization, Cold: It .load_blocks_to_replay(finalized_slot + 1, head_state.slot(), head_block_root) .map_err(|e| format!("Error loading blocks to replay for fork choice: {:?}", e))?; - let mut state = finalized_snapshot.beacon_state; + let mut state = finalized_state; for block in blocks { complete_state_advance(&mut state, None, block.slot(), spec) .map_err(|e| format!("State advance failed: {:?}", e))?; diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 4ac3e54742d..98774951938 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -72,8 +72,8 @@ pub use self::errors::{BeaconChainError, BlockProductionError}; pub use self::historical_blocks::HistoricalBlockError; pub use attestation_verification::Error as AttestationError; pub use beacon_fork_choice_store::{ - BeaconForkChoiceStore, Error as ForkChoiceStoreError, PersistedForkChoiceStoreV17, - PersistedForkChoiceStoreV28, + BalancesCache, BeaconForkChoiceStore, Error as ForkChoiceStoreError, + PersistedForkChoiceStoreV17, PersistedForkChoiceStoreV28, }; pub use block_verification::{ BlockError, ExecutionPayloadError, ExecutionPendingBlock, GossipVerifiedBlock, diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs index e8bd526e19f..34923e1020f 100644 --- a/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v23.rs @@ -1,5 +1,5 @@ -use crate::BeaconForkChoiceStore; use crate::beacon_chain::BeaconChainTypes; +use crate::beacon_fork_choice_store::{BalancesCache, BeaconForkChoiceStore}; use crate::persisted_fork_choice::PersistedForkChoiceV17; use crate::schema_change::StoreError; use crate::test_utils::{BEACON_CHAIN_DB_KEY, FORK_CHOICE_DB_KEY, PersistedBeaconChain}; @@ -113,7 +113,7 @@ pub fn downgrade_from_v23( let fork_choice = ForkChoice::from_persisted( persisted_fork_choice.fork_choice_v17.try_into()?, reset_payload_statuses, - fc_store, + BalancesCache::new(db.clone()), &db.spec, ) .map_err(|e| { diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs index 5885eaabc00..927cc8818e0 100644 --- a/beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v28.rs @@ -1,10 +1,11 @@ use crate::{ - BeaconChain, BeaconChainTypes, BeaconForkChoiceStore, PersistedForkChoiceStoreV17, + BalancesCache, BeaconChain, BeaconChainTypes, BeaconForkChoiceStore, + PersistedForkChoiceStoreV17, beacon_chain::FORK_CHOICE_DB_KEY, persisted_fork_choice::{PersistedForkChoiceV17, PersistedForkChoiceV28}, summaries_dag::{DAGStateSummary, StateSummariesDAG}, }; -use fork_choice::{ForkChoice, ForkChoiceStore, ResetPayloadStatuses}; +use fork_choice::{ForkChoice, ResetPayloadStatuses}; use std::sync::Arc; use store::{Error, HotColdDB, KeyValueStoreOp, StoreItem}; use tracing::{info, warn}; @@ -91,7 +92,7 @@ pub fn upgrade_to_v28( let fork_choice = ForkChoice::from_persisted( persisted_fork_choice_v17.fork_choice_v17.try_into()?, reset_payload_statuses, - fc_store, + BalancesCache::new(db.clone()), db.get_chain_spec(), ) .map_err(|e| Error::MigrationError(format!("Unable to build ForkChoice: {e:?}")))?; diff --git a/consensus/fork_choice/Cargo.toml b/consensus/fork_choice/Cargo.toml index 0a244c2ba19..c1435735fff 100644 --- a/consensus/fork_choice/Cargo.toml +++ b/consensus/fork_choice/Cargo.toml @@ -11,6 +11,7 @@ ethereum_ssz_derive = { workspace = true } logging = { workspace = true } metrics = { workspace = true } proto_array = { workspace = true } +safe_arith = { workspace = true } state_processing = { workspace = true } superstruct = { workspace = true } tracing = { workspace = true } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index fe1f5fba9e4..9b3b2d08865 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -5,6 +5,7 @@ use proto_array::{ Block as ProtoBlock, DisallowedReOrgOffsets, ExecutionStatus, JustifiedBalances, ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, }; +use safe_arith::ArithError; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use state_processing::{ @@ -24,7 +25,7 @@ use types::{ }; #[derive(Debug)] -pub enum Error { +pub enum Error { InvalidAttestation(InvalidAttestation), InvalidAttesterSlashing(AttesterSlashingValidationError), InvalidBlock(InvalidBlock), @@ -48,10 +49,10 @@ pub enum Error { store: Slot, state: Slot, }, - ForkChoiceStoreError(T), - UnableToSetJustifiedCheckpoint(T), - AfterBlockFailed(T), - ProposerHeadError(T), + ForkChoiceStoreError, + UnableToSetJustifiedCheckpoint, + AfterBlockFailed, + ProposerHeadError, InvalidAnchor { block_slot: Slot, state_slot: Slot, @@ -76,27 +77,29 @@ pub enum Error { }, UnrealizedVoteProcessing(state_processing::EpochProcessingError), ValidatorStatuses(BeaconStateError), + InvalidBalances(ArithError), + BalancesGetterError(String), } -impl From for Error { +impl From for Error { fn from(e: InvalidAttestation) -> Self { Error::InvalidAttestation(e) } } -impl From for Error { +impl From for Error { fn from(e: AttesterSlashingValidationError) -> Self { Error::InvalidAttesterSlashing(e) } } -impl From for Error { +impl From for Error { fn from(e: state_processing::EpochProcessingError) -> Self { Error::UnrealizedVoteProcessing(e) } } -impl From for Error { +impl From for Error { fn from(e: BeaconStateError) -> Self { Error::BeaconStateError(e) } @@ -171,13 +174,13 @@ pub enum InvalidAttestation { AttestsToFutureBlock { block: Slot, attestation: Slot }, } -impl From for Error { +impl From for Error { fn from(e: String) -> Self { Error::ProtoArrayStringError(e) } } -impl From for Error { +impl From for Error { fn from(e: proto_array::Error) -> Self { Error::ProtoArrayError(e) } @@ -232,6 +235,14 @@ fn compute_start_slot_at_epoch(epoch: Epoch) -> Slot { epoch.start_slot(E::slots_per_epoch()) } +pub trait BalancesGetter { + fn get_balances( + &self, + state_root: Hash256, + checkpoint: Checkpoint, + ) -> Result; +} + /// Used for queuing attestations from the current slot. Only contains the minimum necessary /// information about the attestation. #[derive(Clone, PartialEq, Encode, Decode)] @@ -313,19 +324,21 @@ pub struct ForkChoiceView { /// - Queuing of attestations from the current slot. pub struct ForkChoice { /// Storage for `ForkChoice`, modelled off the spec `Store` object. - fc_store: T, + fc_store: ForkChoiceStore, /// The underlying representation of the block DAG. proto_array: ProtoArrayForkChoice, /// Attestations that arrived at the current slot and must be queued for later processing. queued_attestations: Vec, /// Stores a cache of the values required to be sent to the execution layer. forkchoice_update_parameters: ForkchoiceUpdateParameters, + /// Abstraction over the store to fetch justified balances + balances_getter: T, _phantom: PhantomData, } impl PartialEq for ForkChoice where - T: ForkChoiceStore + PartialEq, + T: BalancesGetter, E: EthSpec, { fn eq(&self, other: &Self) -> bool { @@ -337,18 +350,18 @@ where impl ForkChoice where - T: ForkChoiceStore, + T: BalancesGetter, E: EthSpec, { /// Instantiates `Self` from an anchor (genesis or another finalized checkpoint). pub fn from_anchor( - fc_store: T, anchor_block_root: Hash256, anchor_block: &SignedBeaconBlock, - anchor_state: &BeaconState, - current_slot: Option, + anchor_state: &mut BeaconState, + current_slot: Slot, + balances_getter: T, spec: &ChainSpec, - ) -> Result> { + ) -> Result { // Sanity check: the anchor must lie on an epoch boundary. if anchor_state.slot() % E::slots_per_epoch() != 0 { return Err(Error::InvalidAnchor { @@ -382,22 +395,39 @@ where }, ); - // If the current slot is not provided, use the value that was last provided to the store. - let current_slot = current_slot.unwrap_or_else(|| fc_store.get_current_slot()); + let justified_checkpoint = Checkpoint { + epoch: anchor_state.current_epoch(), + root: anchor_block_root, + }; + + let justified_balances = JustifiedBalances::from_justified_state(&anchor_state) + .map_err(Error::InvalidBalances)?; + let justified_state_root = anchor_state.canonical_root()?; let proto_array = ProtoArrayForkChoice::new::( current_slot, finalized_block_slot, finalized_block_state_root, - *fc_store.justified_checkpoint(), - *fc_store.finalized_checkpoint(), + justified_checkpoint, + justified_checkpoint, current_epoch_shuffling_id, next_epoch_shuffling_id, execution_status, )?; let mut fork_choice = Self { - fc_store, + fc_store: ForkChoiceStore { + time: anchor_state.slot(), + justified_checkpoint, + justified_balances, + justified_state_root, + finalized_checkpoint: justified_checkpoint, + unrealized_justified_checkpoint: justified_checkpoint, + unrealized_justified_state_root: justified_state_root, + unrealized_finalized_checkpoint: justified_checkpoint, + proposer_boost_root: Hash256::zero(), + equivocating_indices: BTreeSet::new(), + }, proto_array, queued_attestations: vec![], // This will be updated during the next call to `Self::get_head`. @@ -408,6 +438,7 @@ where // This will be updated during the next call to `Self::get_head`. head_root: Hash256::zero(), }, + balances_getter, _phantom: PhantomData, }; @@ -441,11 +472,7 @@ where &self, block_root: Hash256, ancestor_slot: Slot, - ) -> Result, Error> - where - T: ForkChoiceStore, - E: EthSpec, - { + ) -> Result, Error> { let block = self .proto_array .get_block(&block_root) @@ -479,7 +506,7 @@ where &mut self, system_time_current_slot: Slot, spec: &ChainSpec, - ) -> Result> { + ) -> Result { // Provide the slot (as per the system clock) to the `fc_store` and then return its view of // the current slot. The `fc_store` will ensure that the `current_slot` is never // decreasing, a property which we must maintain. @@ -532,7 +559,7 @@ where re_org_parent_threshold: ReOrgThreshold, disallowed_offsets: &DisallowedReOrgOffsets, max_epochs_since_finalization: Epoch, - ) -> Result>> { + ) -> Result> { // Ensure that fork choice has already been updated for the current slot. This prevents // us from having to take a write lock or do any dequeueing of attestations in this // function. @@ -576,7 +603,7 @@ where re_org_parent_threshold: ReOrgThreshold, disallowed_offsets: &DisallowedReOrgOffsets, max_epochs_since_finalization: Epoch, - ) -> Result>> { + ) -> Result> { let current_slot = self.fc_store.get_current_slot(); self.proto_array .get_proposer_head_info::( @@ -612,10 +639,7 @@ where } /// See `ProtoArrayForkChoice::process_execution_payload_validation` for documentation. - pub fn on_valid_execution_payload( - &mut self, - block_root: Hash256, - ) -> Result<(), Error> { + pub fn on_valid_execution_payload(&mut self, block_root: Hash256) -> Result<(), Error> { self.proto_array .process_execution_payload_validation(block_root) .map_err(Error::FailedToProcessValidExecutionPayload) @@ -625,7 +649,7 @@ where pub fn on_invalid_execution_payload( &mut self, op: &InvalidationOperation, - ) -> Result<(), Error> { + ) -> Result<(), Error> { self.proto_array .process_execution_payload_invalidation::(op) .map_err(Error::FailedToProcessInvalidExecutionPayload) @@ -665,7 +689,7 @@ where state: &BeaconState, payload_verification_status: PayloadVerificationStatus, spec: &ChainSpec, - ) -> Result<(), Error> { + ) -> Result<(), Error> { let _timer = metrics::start_timer(&metrics::FORK_CHOICE_ON_BLOCK_TIMES); // If this block has already been processed we do not need to reprocess it. @@ -822,7 +846,7 @@ where // If block is from past epochs, try to update store's justified & finalized checkpoints right away if block.slot().epoch(E::slots_per_epoch()) < current_slot.epoch(E::slots_per_epoch()) { - self.pull_up_store_checkpoints( + self.update_checkpoints( unrealized_justified_checkpoint, unrealized_finalized_checkpoint, || { @@ -847,10 +871,6 @@ where .map_err(Error::BeaconStateError)? }; - self.fc_store - .on_verified_block(block, block_root, state) - .map_err(Error::AfterBlockFailed)?; - let execution_status = if let Ok(execution_payload) = block.body().execution_payload() { let block_hash = execution_payload.block_hash(); @@ -918,14 +938,16 @@ where &mut self, justified_checkpoint: Checkpoint, finalized_checkpoint: Checkpoint, - justified_state_root_producer: impl FnOnce() -> Result>, - ) -> Result<(), Error> { + justified_state_root_producer: impl FnOnce() -> Result, + ) -> Result<(), Error> { // Update justified checkpoint. if justified_checkpoint.epoch > self.fc_store.justified_checkpoint().epoch { let justified_state_root = justified_state_root_producer()?; + let justified_balances = self + .balances_getter + .get_balances(justified_state_root, justified_checkpoint)?; self.fc_store - .set_justified_checkpoint(justified_checkpoint, justified_state_root) - .map_err(Error::UnableToSetJustifiedCheckpoint)?; + .set_justified_checkpoint(justified_checkpoint, justified_balances); } // Update finalized checkpoint. @@ -1073,7 +1095,7 @@ where system_time_current_slot: Slot, attestation: IndexedAttestationRef, is_from_block: AttestationFromBlock, - ) -> Result<(), Error> { + ) -> Result<(), Error> { let _timer = metrics::start_timer(&metrics::FORK_CHOICE_ON_ATTESTATION_TIMES); self.update_time(system_time_current_slot)?; @@ -1103,7 +1125,7 @@ where *validator_index as usize, attestation.data().beacon_block_root, attestation.data().target.epoch, - )?; + ); } } else { // The spec declares: @@ -1138,7 +1160,7 @@ where /// Call `on_tick` for all slots between `fc_store.get_current_slot()` and the provided /// `current_slot`. Returns the value of `self.fc_store.get_current_slot`. - pub fn update_time(&mut self, current_slot: Slot) -> Result> { + pub fn update_time(&mut self, current_slot: Slot) -> Result { while self.fc_store.get_current_slot() < current_slot { let previous_slot = self.fc_store.get_current_slot(); // Note: we are relying upon `on_tick` to update `fc_store.time` to ensure we don't @@ -1147,7 +1169,7 @@ where } // Process any attestations that might now be eligible. - self.process_attestation_queue()?; + self.process_attestation_queue(); Ok(self.fc_store.get_current_slot()) } @@ -1159,7 +1181,7 @@ where /// Equivalent to: /// /// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/fork-choice.md#on_tick - fn on_tick(&mut self, time: Slot) -> Result<(), Error> { + fn on_tick(&mut self, time: Slot) -> Result<(), Error> { let store = &mut self.fc_store; let previous_slot = store.get_current_slot(); @@ -1192,7 +1214,7 @@ where let unrealized_justified_checkpoint = *self.fc_store.unrealized_justified_checkpoint(); let unrealized_justified_state_root = self.fc_store.unrealized_justified_state_root(); let unrealized_finalized_checkpoint = *self.fc_store.unrealized_finalized_checkpoint(); - self.pull_up_store_checkpoints( + self.update_checkpoints( unrealized_justified_checkpoint, unrealized_finalized_checkpoint, || Ok(unrealized_justified_state_root), @@ -1201,22 +1223,9 @@ where Ok(()) } - fn pull_up_store_checkpoints( - &mut self, - unrealized_justified_checkpoint: Checkpoint, - unrealized_finalized_checkpoint: Checkpoint, - unrealized_justified_state_root_producer: impl FnOnce() -> Result>, - ) -> Result<(), Error> { - self.update_checkpoints( - unrealized_justified_checkpoint, - unrealized_finalized_checkpoint, - unrealized_justified_state_root_producer, - ) - } - /// Processes and removes from the queue any queued attestations which may now be eligible for /// processing due to the slot clock incrementing. - fn process_attestation_queue(&mut self) -> Result<(), Error> { + fn process_attestation_queue(&mut self) { for attestation in dequeue_attestations( self.fc_store.get_current_slot(), &mut self.queued_attestations, @@ -1226,11 +1235,9 @@ where *validator_index as usize, attestation.block_root, attestation.target_epoch, - )?; + ); } } - - Ok(()) } /// Returns `true` if the block is known **and** a descendant of the finalized root. @@ -1268,7 +1275,7 @@ where /// /// This does *not* return the "best justified checkpoint". It returns the justified checkpoint /// that is used for computing balances. - pub fn get_justified_block(&self) -> Result> { + pub fn get_justified_block(&self) -> Result { let justified_checkpoint = self.justified_checkpoint(); self.get_block(&justified_checkpoint.root) .ok_or(Error::MissingJustifiedBlock { @@ -1277,7 +1284,7 @@ where } /// Returns the `ProtoBlock` for the finalized checkpoint. - pub fn get_finalized_block(&self) -> Result> { + pub fn get_finalized_block(&self) -> Result { let finalized_checkpoint = self.finalized_checkpoint(); self.get_block(&finalized_checkpoint.root) .ok_or(Error::MissingFinalizedBlock { @@ -1305,10 +1312,7 @@ where /// `execution_status` of the current finalized block. /// /// This function assumes the `block_root` exists. - pub fn is_optimistic_or_invalid_block( - &self, - block_root: &Hash256, - ) -> Result> { + pub fn is_optimistic_or_invalid_block(&self, block_root: &Hash256) -> Result { if let Some(status) = self.get_block_execution_status(block_root) { Ok(status.is_optimistic_or_invalid()) } else { @@ -1327,7 +1331,7 @@ where pub fn is_optimistic_or_invalid_block_no_fallback( &self, block_root: &Hash256, - ) -> Result> { + ) -> Result { if let Some(status) = self.get_block_execution_status(block_root) { Ok(status.is_optimistic_or_invalid()) } else { @@ -1377,10 +1381,14 @@ where } /// Returns a reference to the underlying `fc_store`. - pub fn fc_store(&self) -> &T { + pub fn fc_store(&self) -> &ForkChoiceStore { &self.fc_store } + pub fn get_current_slot(&self) -> Slot { + self.fc_store.get_current_slot() + } + /// Returns a reference to the currently queued attestations. pub fn queued_attestations(&self) -> &[QueuedAttestation] { &self.queued_attestations @@ -1392,7 +1400,7 @@ where } /// Prunes the underlying fork choice DAG. - pub fn prune(&mut self) -> Result<(), Error> { + pub fn prune(&mut self) -> Result<(), Error> { let finalized_root = self.fc_store.finalized_checkpoint().root; self.proto_array @@ -1407,7 +1415,7 @@ where justified_balances: JustifiedBalances, reset_payload_statuses: ResetPayloadStatuses, spec: &ChainSpec, - ) -> Result> { + ) -> Result { let mut proto_array = ProtoArrayForkChoice::from_container( persisted_proto_array.clone(), justified_balances.clone(), @@ -1453,10 +1461,10 @@ where pub fn from_persisted( persisted: PersistedForkChoice, reset_payload_statuses: ResetPayloadStatuses, - fc_store: T, + balances_getter: T, spec: &ChainSpec, - ) -> Result> { - let justified_balances = fc_store.justified_balances().clone(); + ) -> Result { + let justified_balances = persisted.fc_store.justified_balances().clone(); let proto_array = Self::proto_array_from_persisted( persisted.proto_array, justified_balances, @@ -1464,12 +1472,11 @@ where spec, )?; - let current_slot = fc_store.get_current_slot(); - let mut fork_choice = Self { - fc_store, + fc_store: persisted.fc_store, proto_array, queued_attestations: persisted.queued_attestations, + balances_getter, // Will be updated in the following call to `Self::get_head`. forkchoice_update_parameters: ForkchoiceUpdateParameters { head_hash: None, @@ -1481,6 +1488,8 @@ where _phantom: PhantomData, }; + let current_slot = fork_choice.fc_store.get_current_slot(); + // If a call to `get_head` fails, the only known cause is because the only head with viable // FFG properties is has an invalid payload. In this scenario, set all the payloads back to // an optimistic status so that we can have a head to start from. @@ -1510,6 +1519,7 @@ where PersistedForkChoice { proto_array: self.proto_array().as_ssz_container(), queued_attestations: self.queued_attestations().to_vec(), + fc_store: self.fc_store.clone(), } } @@ -1533,6 +1543,7 @@ pub struct PersistedForkChoice { #[superstruct(only(V28))] pub proto_array: proto_array::core::SszContainerV28, pub queued_attestations: Vec, + pub fc_store: ForkChoiceStore, } pub type PersistedForkChoice = PersistedForkChoiceV28; @@ -1548,6 +1559,7 @@ impl TryFrom for PersistedForkChoiceV28 { Ok(Self { proto_array: container_v28, queued_attestations: v17.queued_attestations, + fc_store: v17.fc_store, }) } } @@ -1560,6 +1572,7 @@ impl From<(PersistedForkChoiceV28, JustifiedBalances)> for PersistedForkChoiceV1 Self { proto_array_bytes, queued_attestations: v28.queued_attestations, + fc_store: v28.fc_store, } } } diff --git a/consensus/fork_choice/src/fork_choice_store.rs b/consensus/fork_choice/src/fork_choice_store.rs index caa0ae9be24..ddc0bad587f 100644 --- a/consensus/fork_choice/src/fork_choice_store.rs +++ b/consensus/fork_choice/src/fork_choice_store.rs @@ -1,7 +1,9 @@ use proto_array::JustifiedBalances; +use ssz::{Decode, Encode}; +use ssz_derive::{Decode, Encode}; use std::collections::BTreeSet; use std::fmt::Debug; -use types::{AbstractExecPayload, BeaconBlockRef, BeaconState, Checkpoint, EthSpec, Hash256, Slot}; +use types::{Checkpoint, Hash256, Slot}; /// Approximates the `Store` in "Ethereum 2.0 Phase 0 -- Beacon Chain Fork Choice": /// @@ -19,74 +21,92 @@ use types::{AbstractExecPayload, BeaconBlockRef, BeaconState, Checkpoint, EthSpe /// The primary motivation for defining this as a trait to be implemented upstream rather than a /// concrete struct is to allow this crate to be free from "impure" on-disk database logic, /// hopefully making auditing easier. -pub trait ForkChoiceStore: Sized { - type Error: Debug; - - /// Returns the last value passed to `Self::set_current_slot`. - fn get_current_slot(&self) -> Slot; - - /// Set the value to be returned by `Self::get_current_slot`. - /// - /// ## Notes - /// - /// This should only ever be called from within `ForkChoice::on_tick`. - fn set_current_slot(&mut self, slot: Slot); - - /// Called whenever `ForkChoice::on_block` has verified a block, but not yet added it to fork - /// choice. Allows the implementer to performing caching or other housekeeping duties. - fn on_verified_block>( - &mut self, - block: BeaconBlockRef, - block_root: Hash256, - state: &BeaconState, - ) -> Result<(), Self::Error>; +#[derive(Debug, Clone, PartialEq, Encode, Decode)] +pub struct ForkChoiceStore { + pub(crate) time: Slot, + pub(crate) finalized_checkpoint: Checkpoint, + pub(crate) justified_checkpoint: Checkpoint, + pub(crate) justified_balances: JustifiedBalances, + pub(crate) justified_state_root: Hash256, + pub(crate) unrealized_justified_checkpoint: Checkpoint, + pub(crate) unrealized_justified_state_root: Hash256, + pub(crate) unrealized_finalized_checkpoint: Checkpoint, + pub(crate) proposer_boost_root: Hash256, + pub(crate) equivocating_indices: BTreeSet, +} + +impl ForkChoiceStore { + pub(crate) fn get_current_slot(&self) -> Slot { + self.time + } - /// Returns the `justified_checkpoint`. - fn justified_checkpoint(&self) -> &Checkpoint; + pub(crate) fn set_current_slot(&mut self, slot: Slot) { + self.time = slot + } - /// Returns the state root of the justified checkpoint. - fn justified_state_root(&self) -> Hash256; + pub(crate) fn justified_checkpoint(&self) -> &Checkpoint { + &self.justified_checkpoint + } - /// Returns balances from the `state` identified by `justified_checkpoint.root`. - fn justified_balances(&self) -> &JustifiedBalances; + pub(crate) fn justified_balances(&self) -> &JustifiedBalances { + &self.justified_balances + } - /// Returns the `finalized_checkpoint`. - fn finalized_checkpoint(&self) -> &Checkpoint; + pub(crate) fn finalized_checkpoint(&self) -> &Checkpoint { + &self.finalized_checkpoint + } - /// Returns the `unrealized_justified_checkpoint`. - fn unrealized_justified_checkpoint(&self) -> &Checkpoint; + pub(crate) fn unrealized_justified_checkpoint(&self) -> &Checkpoint { + &self.unrealized_justified_checkpoint + } - /// Returns the state root of the unrealized justified checkpoint. - fn unrealized_justified_state_root(&self) -> Hash256; + pub(crate) fn unrealized_justified_state_root(&self) -> Hash256 { + self.unrealized_justified_state_root + } - /// Returns the `unrealized_finalized_checkpoint`. - fn unrealized_finalized_checkpoint(&self) -> &Checkpoint; + pub(crate) fn unrealized_finalized_checkpoint(&self) -> &Checkpoint { + &self.unrealized_finalized_checkpoint + } - /// Returns the `proposer_boost_root`. - fn proposer_boost_root(&self) -> Hash256; + pub(crate) fn proposer_boost_root(&self) -> Hash256 { + self.proposer_boost_root + } - /// Sets `finalized_checkpoint`. - fn set_finalized_checkpoint(&mut self, checkpoint: Checkpoint); + pub(crate) fn set_finalized_checkpoint(&mut self, checkpoint: Checkpoint) { + self.finalized_checkpoint = checkpoint + } - /// Sets the `justified_checkpoint`. - fn set_justified_checkpoint( + pub(crate) fn set_justified_checkpoint( &mut self, checkpoint: Checkpoint, - state_root: Hash256, - ) -> Result<(), Self::Error>; - - /// Sets the `unrealized_justified_checkpoint`. - fn set_unrealized_justified_checkpoint(&mut self, checkpoint: Checkpoint, state_root: Hash256); - - /// Sets the `unrealized_finalized_checkpoint`. - fn set_unrealized_finalized_checkpoint(&mut self, checkpoint: Checkpoint); + justified_balances: JustifiedBalances, + ) { + self.justified_checkpoint = checkpoint; + self.justified_balances = justified_balances; + } - /// Sets the proposer boost root. - fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256); - - /// Gets the equivocating indices. - fn equivocating_indices(&self) -> &BTreeSet; - - /// Adds to the set of equivocating indices. - fn extend_equivocating_indices(&mut self, indices: impl IntoIterator); + pub(crate) fn set_unrealized_justified_checkpoint( + &mut self, + checkpoint: Checkpoint, + state_root: Hash256, + ) { + self.unrealized_justified_checkpoint = checkpoint; + self.unrealized_justified_state_root = state_root; + } + + pub(crate) fn set_unrealized_finalized_checkpoint(&mut self, checkpoint: Checkpoint) { + self.unrealized_finalized_checkpoint = checkpoint; + } + + pub(crate) fn set_proposer_boost_root(&mut self, proposer_boost_root: Hash256) { + self.proposer_boost_root = proposer_boost_root; + } + + pub(crate) fn equivocating_indices(&self) -> &BTreeSet { + &self.equivocating_indices + } + + pub(crate) fn extend_equivocating_indices(&mut self, indices: impl IntoIterator) { + self.equivocating_indices.extend(indices); + } } diff --git a/consensus/fork_choice/src/lib.rs b/consensus/fork_choice/src/lib.rs index afe06dee1bc..c5715720f33 100644 --- a/consensus/fork_choice/src/lib.rs +++ b/consensus/fork_choice/src/lib.rs @@ -3,9 +3,10 @@ mod fork_choice_store; mod metrics; pub use crate::fork_choice::{ - AttestationFromBlock, Error, ForkChoice, ForkChoiceView, ForkchoiceUpdateParameters, - InvalidAttestation, InvalidBlock, PayloadVerificationStatus, PersistedForkChoice, - PersistedForkChoiceV17, PersistedForkChoiceV28, QueuedAttestation, ResetPayloadStatuses, + AttestationFromBlock, BalancesGetter, Error, ForkChoice, ForkChoiceView, + ForkchoiceUpdateParameters, InvalidAttestation, InvalidBlock, PayloadVerificationStatus, + PersistedForkChoice, PersistedForkChoiceV17, PersistedForkChoiceV28, QueuedAttestation, + ResetPayloadStatuses, }; pub use fork_choice_store::ForkChoiceStore; pub use proto_array::{ diff --git a/consensus/fork_choice/src/metrics.rs b/consensus/fork_choice/src/metrics.rs index b5cda2f5871..9ac8d2f1bbc 100644 --- a/consensus/fork_choice/src/metrics.rs +++ b/consensus/fork_choice/src/metrics.rs @@ -1,8 +1,9 @@ +use crate::fork_choice::BalancesGetter; pub use metrics::*; use std::sync::LazyLock; use types::EthSpec; -use crate::{ForkChoice, ForkChoiceStore}; +use crate::ForkChoice; pub static FORK_CHOICE_QUEUED_ATTESTATIONS: LazyLock> = LazyLock::new(|| { try_create_int_gauge( @@ -46,7 +47,7 @@ pub static FORK_CHOICE_ON_ATTESTER_SLASHING_TIMES: LazyLock> = }); /// Update the global metrics `DEFAULT_REGISTRY` with info from the fork choice. -pub fn scrape_for_metrics, E: EthSpec>(fork_choice: &ForkChoice) { +pub fn scrape_for_metrics(fork_choice: &ForkChoice) { set_gauge( &FORK_CHOICE_QUEUED_ATTESTATIONS, fork_choice.queued_attestations().len() as i64, diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 20987dff26d..4f0fa9137f5 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -226,14 +226,7 @@ impl ForkChoiceTestDefinition { block_root, target_epoch, } => { - fork_choice - .process_attestation(validator_index, block_root, target_epoch) - .unwrap_or_else(|_| { - panic!( - "process_attestation op at index {} returned error", - op_index - ) - }); + fork_choice.process_attestation(validator_index, block_root, target_epoch); check_bytes_round_trip(&fork_choice); } Operation::Prune { diff --git a/consensus/proto_array/src/justified_balances.rs b/consensus/proto_array/src/justified_balances.rs index e08c8443eef..3924bdeb52d 100644 --- a/consensus/proto_array/src/justified_balances.rs +++ b/consensus/proto_array/src/justified_balances.rs @@ -1,7 +1,8 @@ use safe_arith::{ArithError, SafeArith}; +use ssz_derive::{Decode, Encode}; use types::{BeaconState, EthSpec}; -#[derive(Debug, PartialEq, Clone, Default)] +#[derive(Debug, PartialEq, Clone, Default, Encode, Decode)] pub struct JustifiedBalances { /// The effective balances for every validator in a given justified state. /// diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index dea853d245d..243949c3309 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -484,15 +484,13 @@ impl ProtoArrayForkChoice { validator_index: usize, block_root: Hash256, target_epoch: Epoch, - ) -> Result<(), String> { + ) { let vote = self.votes.get_mut(validator_index); if target_epoch > vote.next_epoch || *vote == VoteTracker::default() { vote.next_root = block_root; vote.next_epoch = target_epoch; } - - Ok(()) } pub fn process_block(