From 4b808d3c72f01aebc348a11ab59f0a8db52f341c Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 26 Nov 2021 13:13:46 +1100 Subject: [PATCH] More variable-variable lists --- .../src/validator_pubkey_cache.rs | 5 ++- beacon_node/store/Cargo.toml | 3 ++ beacon_node/store/src/chunked_vector.rs | 30 +++++++++++++--- beacon_node/store/src/errors.rs | 12 +++++++ beacon_node/store/src/partial_beacon_state.rs | 11 +++--- .../per_block_processing/signature_sets.rs | 3 -- .../altair/participation_cache.rs | 3 -- .../altair/participation_flag_updates.rs | 3 -- .../altair/rewards_and_penalties.rs | 3 -- .../base/rewards_and_penalties.rs | 3 -- .../base/validator_statuses.rs | 3 -- .../src/per_epoch_processing/errors.rs | 12 +++++++ .../src/per_epoch_processing/resets.rs | 6 ++-- .../state_processing/src/upgrade/altair.rs | 7 ++-- consensus/types/src/beacon_state.rs | 35 +++++++++++++------ .../types/src/beacon_state/committee_cache.rs | 3 -- consensus/types/src/lib.rs | 2 +- 17 files changed, 91 insertions(+), 53 deletions(-) diff --git a/beacon_node/beacon_chain/src/validator_pubkey_cache.rs b/beacon_node/beacon_chain/src/validator_pubkey_cache.rs index 2dbe8ce7bf..af89339c0c 100644 --- a/beacon_node/beacon_chain/src/validator_pubkey_cache.rs +++ b/beacon_node/beacon_chain/src/validator_pubkey_cache.rs @@ -111,9 +111,12 @@ impl ValidatorPubkeyCache { state: &BeaconState, ) -> Result<(), BeaconChainError> { if state.validators().len() > self.pubkeys.len() { + // FIXME(sproul): iter_from would be more efficient than `skip` here self.import( - state.validators()[self.pubkeys.len()..] + state + .validators() .iter() + .skip(self.pubkeys.len()) .map(|v| v.pubkey), ) } else { diff --git a/beacon_node/store/Cargo.toml b/beacon_node/store/Cargo.toml index d641f87aaf..bfd865ba0b 100644 --- a/beacon_node/store/Cargo.toml +++ b/beacon_node/store/Cargo.toml @@ -25,3 +25,6 @@ lighthouse_metrics = { path = "../../common/lighthouse_metrics" } lru = "0.6.0" sloggers = "2.0.2" directory = { path = "../../common/directory" } + +[features] +milhouse = ["state_processing/milhouse"] diff --git a/beacon_node/store/src/chunked_vector.rs b/beacon_node/store/src/chunked_vector.rs index 25169b4790..fdad5cc87e 100644 --- a/beacon_node/store/src/chunked_vector.rs +++ b/beacon_node/store/src/chunked_vector.rs @@ -18,6 +18,10 @@ use self::UpdatePattern::*; use crate::*; use ssz::{Decode, Encode}; use typenum::Unsigned; +use types::VList; + +#[cfg(feature = "milhouse")] +use types::milhouse::ImmList; /// Description of how a `BeaconState` field is updated during state processing. /// @@ -318,7 +322,7 @@ field!( |_| OncePerNSlots { n: T::SlotsPerHistoricalRoot::to_u64() }, - |state: &BeaconState<_>, index, _| safe_modulo_index(state.historical_roots(), index) + |state: &BeaconState<_>, index, _| safe_modulo_index_tree(state.historical_roots(), index) ); field!( @@ -533,7 +537,7 @@ pub fn load_variable_list_from_db, E: EthSpec, S: KeyV store: &S, slot: Slot, spec: &ChainSpec, -) -> Result, Error> { +) -> Result, Error> { let chunk_size = F::chunk_size(); let (start_vindex, end_vindex) = F::start_and_end_vindex(slot, spec); let start_cindex = start_vindex / chunk_size; @@ -541,19 +545,19 @@ pub fn load_variable_list_from_db, E: EthSpec, S: KeyV let chunks: Vec> = range_query(store, F::column(), start_cindex, end_cindex)?; - let mut result = Vec::with_capacity(chunk_size * chunks.len()); + let mut result = VList::empty(); for (chunk_index, chunk) in chunks.into_iter().enumerate() { for (i, value) in chunk.values.into_iter().enumerate() { let vindex = chunk_index * chunk_size + i; if vindex >= start_vindex && vindex < end_vindex { - result.push(value); + result.push(value)?; } } } - Ok(result.into()) + Ok(result) } /// Index into a field of the state, avoiding out of bounds and division by 0. @@ -565,6 +569,21 @@ fn safe_modulo_index(values: &[T], index: u64) -> Result } } +#[cfg(not(feature = "milhouse"))] +use safe_modulo_index as safe_modulo_index_tree; + +#[cfg(feature = "milhouse")] +fn safe_modulo_index_tree, T: Copy>(values: &V, index: u64) -> Result { + if values.is_empty() { + Err(ChunkError::ZeroLengthVector) + } else { + values + .get(index as usize % values.len()) + .copied() + .ok_or(ChunkError::OutOfBounds) + } +} + /// A chunk of a fixed-size vector from the `BeaconState`, stored in the database. #[derive(Debug, Clone, PartialEq)] pub struct Chunk { @@ -679,6 +698,7 @@ pub enum ChunkError { end_vindex: usize, length: usize, }, + OutOfBounds, } #[cfg(test)] diff --git a/beacon_node/store/src/errors.rs b/beacon_node/store/src/errors.rs index 0be8b43d6d..34829f9e32 100644 --- a/beacon_node/store/src/errors.rs +++ b/beacon_node/store/src/errors.rs @@ -4,6 +4,9 @@ use crate::hot_cold_store::HotColdDBError; use ssz::DecodeError; use types::{BeaconStateError, Hash256, Slot}; +#[cfg(feature = "milhouse")] +use types::milhouse; + pub type Result = std::result::Result; #[derive(Debug)] @@ -39,6 +42,8 @@ pub enum Error { expected: Hash256, computed: Hash256, }, + #[cfg(feature = "milhouse")] + MilhouseError(milhouse::Error), } pub trait HandleUnavailable { @@ -91,6 +96,13 @@ impl From for Error { } } +#[cfg(feature = "milhouse")] +impl From for Error { + fn from(e: milhouse::Error) -> Self { + Self::MilhouseError(e) + } +} + #[derive(Debug)] pub struct DBError { pub message: String, diff --git a/beacon_node/store/src/partial_beacon_state.rs b/beacon_node/store/src/partial_beacon_state.rs index 9c8fcc4b76..127bfc665f 100644 --- a/beacon_node/store/src/partial_beacon_state.rs +++ b/beacon_node/store/src/partial_beacon_state.rs @@ -39,15 +39,15 @@ where pub state_roots: Option>, #[ssz(skip_serializing, skip_deserializing)] - pub historical_roots: Option>, + pub historical_roots: Option>, // Ethereum 1.0 chain data pub eth1_data: Eth1Data, - pub eth1_data_votes: VariableList, + pub eth1_data_votes: VList, pub eth1_deposit_index: u64, // Registry - pub validators: VariableList, + pub validators: VList, pub balances: VariableList, // Shuffling @@ -61,9 +61,9 @@ where // Attestations (genesis fork only) #[superstruct(only(Base))] - pub previous_epoch_attestations: VariableList, T::MaxPendingAttestations>, + pub previous_epoch_attestations: VList, T::MaxPendingAttestations>, #[superstruct(only(Base))] - pub current_epoch_attestations: VariableList, T::MaxPendingAttestations>, + pub current_epoch_attestations: VList, T::MaxPendingAttestations>, // Participation (Altair and later) #[superstruct(only(Altair))] @@ -297,6 +297,7 @@ macro_rules! impl_try_into_beacon_state { committee_caches: <_>::default(), pubkey_cache: <_>::default(), exit_cache: <_>::default(), + #[cfg(not(feature = "milhouse"))] tree_hash_cache: <_>::default(), // Variant-specific fields diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index d2e9286b49..a3b8dcaf21 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -14,9 +14,6 @@ use types::{ SignedVoluntaryExit, SigningData, Slot, SyncAggregate, SyncAggregatorSelectionData, Unsigned, }; -#[cfg(feature = "milhouse")] -use types::milhouse::prelude::*; - pub type Result = std::result::Result; #[derive(Debug, PartialEq, Clone)] diff --git a/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs b/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs index 46d0d928e8..503dadfc70 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair/participation_cache.rs @@ -21,9 +21,6 @@ use types::{ BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, ParticipationFlags, RelativeEpoch, }; -#[cfg(feature = "milhouse")] -use types::milhouse::prelude::*; - #[derive(Debug, PartialEq)] pub enum Error { InvalidFlagIndex(usize), diff --git a/consensus/state_processing/src/per_epoch_processing/altair/participation_flag_updates.rs b/consensus/state_processing/src/per_epoch_processing/altair/participation_flag_updates.rs index db1c030a18..7162fa7f4a 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair/participation_flag_updates.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair/participation_flag_updates.rs @@ -6,9 +6,6 @@ use types::eth_spec::EthSpec; use types::participation_flags::ParticipationFlags; use types::VariableList; -#[cfg(feature = "milhouse")] -use types::milhouse::prelude::*; - pub fn process_participation_flag_updates( state: &mut BeaconState, ) -> Result<(), EpochProcessingError> { diff --git a/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs b/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs index 7f8fd63341..5906e0f8d2 100644 --- a/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs +++ b/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs @@ -6,9 +6,6 @@ use types::consts::altair::{ }; use types::{BeaconState, ChainSpec, EthSpec}; -#[cfg(feature = "milhouse")] -use types::milhouse::prelude::*; - use crate::common::{altair::get_base_reward, decrease_balance, increase_balance}; use crate::per_epoch_processing::{Delta, Error}; diff --git a/consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs b/consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs index 1c2db2cbbb..2c1ef6178e 100644 --- a/consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs +++ b/consensus/state_processing/src/per_epoch_processing/base/rewards_and_penalties.rs @@ -7,9 +7,6 @@ use safe_arith::SafeArith; use std::array::IntoIter as ArrayIter; use types::{BeaconState, ChainSpec, EthSpec}; -#[cfg(feature = "milhouse")] -use types::milhouse::prelude::*; - /// Combination of several deltas for different components of an attestation reward. /// /// Exists only for compatibility with EF rewards tests. diff --git a/consensus/state_processing/src/per_epoch_processing/base/validator_statuses.rs b/consensus/state_processing/src/per_epoch_processing/base/validator_statuses.rs index bd7bd73256..b40f91ce5a 100644 --- a/consensus/state_processing/src/per_epoch_processing/base/validator_statuses.rs +++ b/consensus/state_processing/src/per_epoch_processing/base/validator_statuses.rs @@ -2,9 +2,6 @@ use crate::common::get_attesting_indices; use safe_arith::SafeArith; use types::{BeaconState, BeaconStateError, ChainSpec, Epoch, EthSpec, PendingAttestation}; -#[cfg(feature = "milhouse")] -use types::milhouse::prelude::*; - #[cfg(feature = "arbitrary-fuzz")] use arbitrary::Arbitrary; diff --git a/consensus/state_processing/src/per_epoch_processing/errors.rs b/consensus/state_processing/src/per_epoch_processing/errors.rs index 04797c5634..b9c14ac1be 100644 --- a/consensus/state_processing/src/per_epoch_processing/errors.rs +++ b/consensus/state_processing/src/per_epoch_processing/errors.rs @@ -1,6 +1,9 @@ use crate::per_epoch_processing::altair::participation_cache::Error as ParticipationCacheError; use types::{BeaconStateError, InconsistentFork}; +#[cfg(feature = "milhouse")] +use types::milhouse; + #[derive(Debug, PartialEq)] pub enum EpochProcessingError { UnableToDetermineProducer, @@ -25,6 +28,8 @@ pub enum EpochProcessingError { InvalidJustificationBit(ssz_types::Error), InvalidFlagIndex(usize), ParticipationCache(ParticipationCacheError), + #[cfg(feature = "milhouse")] + MilhouseError(milhouse::Error), } impl From for EpochProcessingError { @@ -57,6 +62,13 @@ impl From for EpochProcessingError { } } +#[cfg(feature = "milhouse")] +impl From for EpochProcessingError { + fn from(e: milhouse::Error) -> Self { + Self::MilhouseError(e) + } +} + #[derive(Debug, PartialEq)] pub enum InclusionError { /// The validator did not participate in an attestation in this period. diff --git a/consensus/state_processing/src/per_epoch_processing/resets.rs b/consensus/state_processing/src/per_epoch_processing/resets.rs index dc3c9f07c0..8664bd98aa 100644 --- a/consensus/state_processing/src/per_epoch_processing/resets.rs +++ b/consensus/state_processing/src/per_epoch_processing/resets.rs @@ -1,10 +1,8 @@ use super::errors::EpochProcessingError; -use core::result::Result; -use core::result::Result::Ok; use safe_arith::SafeArith; use types::beacon_state::BeaconState; use types::eth_spec::EthSpec; -use types::{Unsigned, VariableList}; +use types::{Unsigned, VList}; pub fn process_eth1_data_reset( state: &mut BeaconState, @@ -15,7 +13,7 @@ pub fn process_eth1_data_reset( .safe_rem(T::SlotsPerEth1VotingPeriod::to_u64())? == 0 { - *state.eth1_data_votes_mut() = VariableList::empty(); + *state.eth1_data_votes_mut() = VList::empty(); } Ok(()) } diff --git a/consensus/state_processing/src/upgrade/altair.rs b/consensus/state_processing/src/upgrade/altair.rs index 91ff41bf14..b32afc5ee3 100644 --- a/consensus/state_processing/src/upgrade/altair.rs +++ b/consensus/state_processing/src/upgrade/altair.rs @@ -3,16 +3,13 @@ use std::mem; use std::sync::Arc; use types::{ BeaconState, BeaconStateAltair, BeaconStateError as Error, ChainSpec, EthSpec, Fork, - ParticipationFlags, PendingAttestation, RelativeEpoch, SyncCommittee, VariableList, + ParticipationFlags, PendingAttestation, RelativeEpoch, SyncCommittee, VList, VariableList, }; -#[cfg(feature = "milhouse")] -use types::milhouse::prelude::*; - /// Translate the participation information from the epoch prior to the fork into Altair's format. pub fn translate_participation( state: &mut BeaconState, - pending_attestations: &VariableList, E::MaxPendingAttestations>, + pending_attestations: &VList, E::MaxPendingAttestations>, spec: &ChainSpec, ) -> Result<(), Error> { // Previous epoch committee cache is required for `get_attesting_indices`. diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 98cd4348ec..5859264212 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -30,10 +30,10 @@ pub use eth_spec::*; pub use iter::BlockRootsIter; #[cfg(feature = "milhouse")] -use milhouse::prelude::{List as VList, *}; +pub use milhouse::{interface::Interface, List as VList, List}; #[cfg(not(feature = "milhouse"))] -use {ssz_types::FixedVector, tree_hash_cache::BeaconTreeHashCache, VariableList as VList}; +pub use {ssz_types::FixedVector, tree_hash_cache::BeaconTreeHashCache, VariableList as VList}; #[macro_use] mod committee_cache; @@ -46,7 +46,7 @@ mod tests; mod tree_hash_cache; #[cfg(feature = "milhouse")] -pub type ListMut<'a, T, N> = Interface>; +pub type ListMut<'a, T, N> = Interface<'a, T, List>; #[cfg(feature = "milhouse")] pub type ValidatorsMut<'a, N> = ListMut<'a, Validator, N>; @@ -138,6 +138,8 @@ pub enum Error { current_epoch: Epoch, epoch: Epoch, }, + #[cfg(feature = "milhouse")] + MilhouseError(milhouse::Error), } /// Control whether an epoch-indexed field can be indexed at the next epoch or not. @@ -234,11 +236,13 @@ where pub block_roots: FixedVector, #[compare_fields(as_slice)] pub state_roots: FixedVector, - pub historical_roots: VariableList, + #[test_random(default)] + pub historical_roots: VList, // Ethereum 1.0 chain data pub eth1_data: Eth1Data, - pub eth1_data_votes: VariableList, + #[test_random(default)] + pub eth1_data_votes: VList, #[superstruct(getter(copy))] #[serde(with = "eth2_serde_utils::quoted_u64")] pub eth1_deposit_index: u64, @@ -260,9 +264,11 @@ where // Attestations (genesis fork only) #[superstruct(only(Base))] - pub previous_epoch_attestations: VariableList, T::MaxPendingAttestations>, + #[test_random(default)] + pub previous_epoch_attestations: VList, T::MaxPendingAttestations>, #[superstruct(only(Base))] - pub current_epoch_attestations: VariableList, T::MaxPendingAttestations>, + #[test_random(default)] + pub current_epoch_attestations: VList, T::MaxPendingAttestations>, // Participation (Altair and later) #[superstruct(only(Altair))] @@ -351,11 +357,11 @@ impl BeaconState { latest_block_header: BeaconBlock::::empty(spec).temporary_block_header(), block_roots: FixedVector::from_elem(Hash256::zero()), state_roots: FixedVector::from_elem(Hash256::zero()), - historical_roots: VariableList::empty(), + historical_roots: VList::empty(), // Eth1 eth1_data, - eth1_data_votes: VariableList::empty(), + eth1_data_votes: VList::empty(), eth1_deposit_index: 0, // Validator registry @@ -369,8 +375,8 @@ impl BeaconState { slashings: FixedVector::from_elem(0), // Attestations - previous_epoch_attestations: VariableList::empty(), - current_epoch_attestations: VariableList::empty(), + previous_epoch_attestations: VList::empty(), + current_epoch_attestations: VList::empty(), // Finality justification_bits: BitVector::new(), @@ -1721,6 +1727,13 @@ impl From for Error { } } +#[cfg(feature = "milhouse")] +impl From for Error { + fn from(e: milhouse::Error) -> Self { + Self::MilhouseError(e) + } +} + /// Helper function for "cloning" a field by using its default value. fn clone_default(_value: &T) -> T { T::default() diff --git a/consensus/types/src/beacon_state/committee_cache.rs b/consensus/types/src/beacon_state/committee_cache.rs index 3dce0103f0..c1effa2ac1 100644 --- a/consensus/types/src/beacon_state/committee_cache.rs +++ b/consensus/types/src/beacon_state/committee_cache.rs @@ -10,9 +10,6 @@ use ssz_derive::{Decode, Encode}; use std::ops::Range; use swap_or_not_shuffle::shuffle_list; -#[cfg(feature = "milhouse")] -use milhouse::prelude::*; - mod tests; // Define "legacy" implementations of `Option`, `Option` which use four bytes diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 044791b116..de41093be0 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -163,4 +163,4 @@ pub use ssz_types::{typenum, typenum::Unsigned, BitList, BitVector, FixedVector, pub use superstruct::superstruct; #[cfg(feature = "milhouse")] -pub use milhouse::{self, prelude::*}; +pub use milhouse;