Skip to content

Commit

Permalink
Fix tree-states tests (#5277)
Browse files Browse the repository at this point in the history
* Fix beta compiler lints

* Fix fork choice tests

* Fix op_pool tests

---------

Co-authored-by: dapplion <35266934+dapplion@users.noreply.github.com>
  • Loading branch information
michaelsproul and dapplion authored Feb 22, 2024
1 parent 26117a5 commit f9c9c40
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 25 deletions.
4 changes: 2 additions & 2 deletions beacon_node/beacon_chain/src/state_advance_timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ const MAX_FORK_CHOICE_DISTANCE: u64 = 256;
#[derive(Debug)]
enum Error {
BeaconChain(BeaconChainError),
BeaconState(BeaconStateError),
Store(store::Error),
// We don't use the inner value directly, but it's used in the Debug impl.
BeaconState(#[allow(dead_code)] BeaconStateError),
Store(#[allow(dead_code)] store::Error),
HeadMissingFromSnapshotCache(#[allow(dead_code)] Hash256),
MaxDistanceExceeded {
current_slot: Slot,
Expand Down
37 changes: 31 additions & 6 deletions beacon_node/operation_pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -75,6 +77,8 @@ pub enum OpPoolError {
RewardCacheValidatorUnknown(BeaconStateError),
RewardCacheOutOfBounds,
IncorrectOpPoolVariant,
EpochCacheNotInitialized,
EpochCacheError(EpochCacheError),
}

#[derive(Default)]
Expand Down Expand Up @@ -252,6 +256,17 @@ impl<T: EthSpec> OperationPool<T> {
curr_epoch_validity_filter: impl for<'a> FnMut(&AttestationRef<'a, T>) -> bool + Send,
spec: &ChainSpec,
) -> Result<Vec<Attestation<T>>, 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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -814,6 +830,15 @@ mod release_tests {
(harness, spec)
}

fn get_current_state_initialize_epoch_cache<E: EthSpec>(
harness: &BeaconChainHarness<EphemeralHarnessType<E>>,
spec: &ChainSpec,
) -> BeaconState<E> {
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<E: EthSpec>(
num_committees: usize,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -929,7 +954,7 @@ mod release_tests {
let (harness, ref spec) = attestation_test_state::<MainnetEthSpec>(1);

let op_pool = OperationPool::<MainnetEthSpec>::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
Expand Down Expand Up @@ -1004,7 +1029,7 @@ mod release_tests {
fn attestation_duplicate() {
let (harness, ref spec) = attestation_test_state::<MainnetEthSpec>(1);

let state = harness.get_current_state();
let state = get_current_state_initialize_epoch_cache(&harness, &spec);

let op_pool = OperationPool::<MainnetEthSpec>::new();

Expand Down Expand Up @@ -1044,7 +1069,7 @@ mod release_tests {
fn attestation_pairwise_overlapping() {
let (harness, ref spec) = attestation_test_state::<MainnetEthSpec>(1);

let state = harness.get_current_state();
let state = get_current_state_initialize_epoch_cache(&harness, &spec);

let op_pool = OperationPool::<MainnetEthSpec>::new();

Expand Down Expand Up @@ -1142,7 +1167,7 @@ mod release_tests {

let (harness, ref spec) = attestation_test_state::<MainnetEthSpec>(num_committees);

let mut state = harness.get_current_state();
let mut state = get_current_state_initialize_epoch_cache(&harness, &spec);

let op_pool = OperationPool::<MainnetEthSpec>::new();

Expand Down Expand Up @@ -1232,7 +1257,7 @@ mod release_tests {

let (harness, ref spec) = attestation_test_state::<MainnetEthSpec>(num_committees);

let mut state = harness.get_current_state();
let mut state = get_current_state_initialize_epoch_cache(&harness, &spec);
let op_pool = OperationPool::<MainnetEthSpec>::new();

let slot = state.slot();
Expand Down
11 changes: 2 additions & 9 deletions consensus/fork_choice/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,7 @@ impl fmt::Debug for ForkChoiceTest {
impl ForkChoiceTest {
/// Creates a new tester.
pub fn new() -> Self {
// Run fork choice tests against the latest fork.
let spec = ForkName::latest().make_genesis_spec(ChainSpec::default());
let harness = BeaconChainHarness::builder(MainnetEthSpec)
.spec(spec)
.deterministic_keypairs(VALIDATOR_COUNT)
.fresh_ephemeral_store()
.build();

Self { harness }
Self::new_with_chain_config(ChainConfig::default())
}

/// Creates a new tester with a custom chain config.
Expand All @@ -67,6 +59,7 @@ impl ForkChoiceTest {
.chain_config(chain_config)
.deterministic_keypairs(VALIDATOR_COUNT)
.fresh_ephemeral_store()
.mock_execution_layer()
.build();

Self { harness }
Expand Down
26 changes: 18 additions & 8 deletions consensus/state_processing/src/epoch_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,35 @@ impl PreEpochCache {
}
}

pub fn initialize_epoch_cache<E: EthSpec>(
state: &mut BeaconState<E>,
spec: &ChainSpec,
) -> Result<(), EpochCacheError> {
pub fn is_epoch_cache_initialized<E: EthSpec>(
state: &BeaconState<E>,
) -> Result<bool, EpochCacheError> {
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::<E>(current_epoch, decision_block_root)
.is_ok()
{
.is_ok())
}

pub fn initialize_epoch_cache<E: EthSpec>(
state: &mut BeaconState<E>,
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)?;

Expand Down

0 comments on commit f9c9c40

Please sign in to comment.