From 11a39bdb7215437a832a90c3147893a2dd9f20f6 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Thu, 22 Feb 2024 12:18:49 +0800 Subject: [PATCH] Fix op_pool tests --- beacon_node/operation_pool/src/lib.rs | 37 ++++++++++++++++--- consensus/state_processing/src/epoch_cache.rs | 26 +++++++++---- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 7e1ddb1fd2f..4651ea92988 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -18,6 +18,8 @@ pub use persistence::{ PersistedOperationPoolV15, PersistedOperationPoolV5, }; pub use reward_cache::RewardCache; +use state_processing::epoch_cache::is_epoch_cache_initialized; +use types::EpochCacheError; use crate::attestation_storage::{AttestationMap, CheckpointKey}; use crate::bls_to_execution_changes::BlsToExecutionChanges; @@ -75,6 +77,8 @@ pub enum OpPoolError { RewardCacheValidatorUnknown(BeaconStateError), RewardCacheOutOfBounds, IncorrectOpPoolVariant, + EpochCacheNotInitialized, + EpochCacheError(EpochCacheError), } #[derive(Default)] @@ -252,6 +256,17 @@ impl OperationPool { curr_epoch_validity_filter: impl for<'a> FnMut(&AttestationRef<'a, T>) -> bool + Send, spec: &ChainSpec, ) -> Result>, OpPoolError> { + if let BeaconState::Base(_) = state { + // Ok + } else { + // epoch cache must be initialized to fetch base_reward values in the max_cover score + // function. Currently max_cover ignores items on errors. If epoch cache is not + // initialized, this function returns Ok([]). + if !is_epoch_cache_initialized(state).map_err(OpPoolError::EpochCacheError)? { + return Err(OpPoolError::EpochCacheNotInitialized); + } + }; + // Attestations for the current fork, which may be from the current or previous epoch. let (prev_epoch_key, curr_epoch_key) = CheckpointKey::keys_for_state(state); let all_attestations = self.attestations.read(); @@ -773,6 +788,7 @@ mod release_tests { }; use lazy_static::lazy_static; use maplit::hashset; + use state_processing::epoch_cache::initialize_epoch_cache; use state_processing::{common::get_attesting_indices_from_state, VerifyOperation}; use std::collections::BTreeSet; use types::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT; @@ -814,6 +830,15 @@ mod release_tests { (harness, spec) } + fn get_current_state_initialize_epoch_cache( + harness: &BeaconChainHarness>, + spec: &ChainSpec, + ) -> BeaconState { + let mut state = harness.get_current_state(); + initialize_epoch_cache(&mut state, spec).unwrap(); + state + } + /// Test state for sync contribution-related tests. async fn sync_contribution_test_state( num_committees: usize, @@ -847,7 +872,7 @@ mod release_tests { return; } - let mut state = harness.get_current_state(); + let mut state = get_current_state_initialize_epoch_cache(&harness, &spec); let slot = state.slot(); let committees = state .get_beacon_committees_at_slot(slot) @@ -929,7 +954,7 @@ mod release_tests { let (harness, ref spec) = attestation_test_state::(1); let op_pool = OperationPool::::new(); - let mut state = harness.get_current_state(); + let mut state = get_current_state_initialize_epoch_cache(&harness, &spec); let slot = state.slot(); let committees = state @@ -1004,7 +1029,7 @@ mod release_tests { fn attestation_duplicate() { let (harness, ref spec) = attestation_test_state::(1); - let state = harness.get_current_state(); + let state = get_current_state_initialize_epoch_cache(&harness, &spec); let op_pool = OperationPool::::new(); @@ -1044,7 +1069,7 @@ mod release_tests { fn attestation_pairwise_overlapping() { let (harness, ref spec) = attestation_test_state::(1); - let state = harness.get_current_state(); + let state = get_current_state_initialize_epoch_cache(&harness, &spec); let op_pool = OperationPool::::new(); @@ -1142,7 +1167,7 @@ mod release_tests { let (harness, ref spec) = attestation_test_state::(num_committees); - let mut state = harness.get_current_state(); + let mut state = get_current_state_initialize_epoch_cache(&harness, &spec); let op_pool = OperationPool::::new(); @@ -1232,7 +1257,7 @@ mod release_tests { let (harness, ref spec) = attestation_test_state::(num_committees); - let mut state = harness.get_current_state(); + let mut state = get_current_state_initialize_epoch_cache(&harness, &spec); let op_pool = OperationPool::::new(); let slot = state.slot(); diff --git a/consensus/state_processing/src/epoch_cache.rs b/consensus/state_processing/src/epoch_cache.rs index de0782d762e..954837d1977 100644 --- a/consensus/state_processing/src/epoch_cache.rs +++ b/consensus/state_processing/src/epoch_cache.rs @@ -72,25 +72,35 @@ impl PreEpochCache { } } -pub fn initialize_epoch_cache( - state: &mut BeaconState, - spec: &ChainSpec, -) -> Result<(), EpochCacheError> { +pub fn is_epoch_cache_initialized( + state: &BeaconState, +) -> Result { let current_epoch = state.current_epoch(); - let next_epoch = state.next_epoch().map_err(EpochCacheError::BeaconState)?; let epoch_cache: &EpochCache = state.epoch_cache(); let decision_block_root = state .proposer_shuffling_decision_root(Hash256::zero()) .map_err(EpochCacheError::BeaconState)?; - if epoch_cache + Ok(epoch_cache .check_validity::(current_epoch, decision_block_root) - .is_ok() - { + .is_ok()) +} + +pub fn initialize_epoch_cache( + state: &mut BeaconState, + spec: &ChainSpec, +) -> Result<(), EpochCacheError> { + if is_epoch_cache_initialized(state)? { // `EpochCache` has already been initialized and is valid, no need to initialize. return Ok(()); } + let current_epoch = state.current_epoch(); + let next_epoch = state.next_epoch().map_err(EpochCacheError::BeaconState)?; + let decision_block_root = state + .proposer_shuffling_decision_root(Hash256::zero()) + .map_err(EpochCacheError::BeaconState)?; + state.build_total_active_balance_cache_at(current_epoch, spec)?; let total_active_balance = state.get_total_active_balance_at_epoch(current_epoch)?;