From 9f68e5a6ff220b43167f641e5b9b6f958ac3347b Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 16 Apr 2021 18:09:20 +1000 Subject: [PATCH 1/4] [Altair] Remove global fork schedule --- beacon_node/store/src/hot_cold_store.rs | 39 +++++++--- beacon_node/store/src/impls.rs | 40 +++------- beacon_node/store/src/impls/beacon_state.rs | 32 ++++++-- .../store/src/impls/partial_beacon_state.rs | 16 ---- beacon_node/store/src/partial_beacon_state.rs | 70 +++++++++--------- common/eth2_network_config/src/lib.rs | 16 +++- consensus/ssz/src/decode.rs | 8 ++ consensus/state_processing/src/genesis.rs | 4 +- consensus/types/src/beacon_block.rs | 60 ++++++--------- consensus/types/src/beacon_state.rs | 61 ++++++---------- consensus/types/src/chain_spec.rs | 38 ++++------ consensus/types/src/fork_name.rs | 28 +++++++ consensus/types/src/fork_schedule.rs | 40 ---------- consensus/types/src/lib.rs | 6 +- consensus/types/src/signed_beacon_block.rs | 33 +++++---- testing/ef_tests/src/cases.rs | 16 +++- .../ef_tests/src/cases/bls_aggregate_sigs.rs | 2 +- .../src/cases/bls_aggregate_verify.rs | 2 +- .../src/cases/bls_fast_aggregate_verify.rs | 2 +- testing/ef_tests/src/cases/bls_sign_msg.rs | 2 +- testing/ef_tests/src/cases/bls_verify_msg.rs | 2 +- testing/ef_tests/src/cases/common.rs | 30 +++++++- .../ef_tests/src/cases/epoch_processing.rs | 39 ++++++---- .../src/cases/genesis_initialization.rs | 13 ++-- .../ef_tests/src/cases/genesis_validity.rs | 13 ++-- testing/ef_tests/src/cases/operations.rs | 59 ++++++++++++--- testing/ef_tests/src/cases/sanity_blocks.rs | 23 +++--- testing/ef_tests/src/cases/sanity_slots.rs | 15 ++-- testing/ef_tests/src/cases/shuffling.rs | 5 +- testing/ef_tests/src/cases/ssz_generic.rs | 10 +-- testing/ef_tests/src/cases/ssz_static.rs | 37 ++++++---- testing/ef_tests/src/decode.rs | 19 ++++- testing/ef_tests/src/handler.rs | 73 ++++++++++++------- testing/ef_tests/src/lib.rs | 23 +----- testing/ef_tests/tests/tests.rs | 53 ++++---------- 35 files changed, 491 insertions(+), 438 deletions(-) delete mode 100644 beacon_node/store/src/impls/partial_beacon_state.rs create mode 100644 consensus/types/src/fork_name.rs delete mode 100644 consensus/types/src/fork_schedule.rs diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 0e3745ce198..4f2ebb441a9 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -3,7 +3,10 @@ use crate::chunked_vector::{ }; use crate::config::{OnDiskStoreConfig, StoreConfig}; use crate::forwards_iter::HybridForwardsBlockRootsIterator; -use crate::impls::beacon_state::{get_full_state, store_full_state}; +use crate::impls::{ + beacon_block_as_kv_store_op, + beacon_state::{get_full_state, store_full_state}, +}; use crate::iter::{ParentRootBlockIterator, StateRootsIterator}; use crate::leveldb_store::BytesKey; use crate::leveldb_store::LevelDB; @@ -235,7 +238,8 @@ impl, Cold: ItemStore> HotColdDB block: SignedBeaconBlock, ) -> Result<(), Error> { // Store on disk. - self.hot_db.put(block_root, &block)?; + self.hot_db + .do_atomically(vec![beacon_block_as_kv_store_op(block_root, &block)])?; // Update cache. self.block_cache.lock().put(*block_root, block); @@ -254,10 +258,17 @@ impl, Cold: ItemStore> HotColdDB } // Fetch from database. - match self.hot_db.get::>(block_root)? { - Some(block) => { + match self + .hot_db + .get_bytes(DBColumn::BeaconBlock.into(), block_root.as_bytes())? + { + Some(block_bytes) => { + // Deserialize. + let block = SignedBeaconBlock::from_ssz_bytes(&block_bytes, &self.spec)?; + // Add to cache. self.block_cache.lock().put(*block_root, block.clone()); + Ok(Some(block)) } None => Ok(None), @@ -267,7 +278,8 @@ impl, Cold: ItemStore> HotColdDB /// Delete a block from the store and the block cache. pub fn delete_block(&self, block_root: &Hash256) -> Result<(), Error> { self.block_cache.lock().pop(block_root); - self.hot_db.delete::>(block_root) + self.hot_db + .key_delete(DBColumn::BeaconBlock.into(), block_root.as_bytes()) } pub fn put_state_summary( @@ -435,7 +447,7 @@ impl, Cold: ItemStore> HotColdDB for op in batch { match op { StoreOp::PutBlock(block_root, block) => { - key_value_batch.push(block.as_kv_store_op(*block_root)); + key_value_batch.push(beacon_block_as_kv_store_op(block_root, block)); } StoreOp::PutState(state_root, state) => { @@ -559,9 +571,10 @@ impl, Cold: ItemStore> HotColdDB epoch_boundary_state_root, }) = self.load_hot_state_summary(state_root)? { - let boundary_state = get_full_state(&self.hot_db, &epoch_boundary_state_root)?.ok_or( - HotColdDBError::MissingEpochBoundaryState(epoch_boundary_state_root), - )?; + let boundary_state = + get_full_state(&self.hot_db, &epoch_boundary_state_root, &self.spec)?.ok_or( + HotColdDBError::MissingEpochBoundaryState(epoch_boundary_state_root), + )?; // Optimization to avoid even *thinking* about replaying blocks if we're already // on an epoch boundary. @@ -649,10 +662,12 @@ impl, Cold: ItemStore> HotColdDB /// Load a restore point state by its `state_root`. fn load_restore_point(&self, state_root: &Hash256) -> Result, Error> { - let mut partial_state: PartialBeaconState = self + let partial_state_bytes = self .cold_db - .get(state_root)? + .get_bytes(DBColumn::BeaconState.into(), state_root.as_bytes())? .ok_or_else(|| HotColdDBError::MissingRestorePoint(*state_root))?; + let mut partial_state: PartialBeaconState = + PartialBeaconState::from_ssz_bytes(&partial_state_bytes, &self.spec)?; // Fill in the fields of the partial state. partial_state.load_block_roots(&self.cold_db, &self.spec)?; @@ -1042,7 +1057,7 @@ pub fn migrate_database, Cold: ItemStore>( let mut cold_db_ops: Vec = Vec::new(); if slot % store.config.slots_per_restore_point == 0 { - let state: BeaconState = get_full_state(&store.hot_db, &state_root)? + let state: BeaconState = get_full_state(&store.hot_db, &state_root, &store.spec)? .ok_or(HotColdDBError::MissingStateToFreeze(state_root))?; store.store_cold_state(&state_root, &state, &mut cold_db_ops)?; diff --git a/beacon_node/store/src/impls.rs b/beacon_node/store/src/impls.rs index b89bf1d4020..2321caf2b11 100644 --- a/beacon_node/store/src/impls.rs +++ b/beacon_node/store/src/impls.rs @@ -1,35 +1,15 @@ use crate::*; -use ssz::{Decode, Encode}; +use ssz::Encode; pub mod beacon_state; -pub mod partial_beacon_state; -impl StoreItem for SignedBeaconBlock { - fn db_column() -> DBColumn { - DBColumn::BeaconBlock - } - - fn as_store_bytes(&self) -> Vec { - let timer = metrics::start_timer(&metrics::BEACON_BLOCK_WRITE_TIMES); - let bytes = self.as_ssz_bytes(); - - metrics::stop_timer(timer); - metrics::inc_counter(&metrics::BEACON_BLOCK_WRITE_COUNT); - metrics::inc_counter_by(&metrics::BEACON_BLOCK_WRITE_BYTES, bytes.len() as u64); - - bytes - } - - fn from_store_bytes(bytes: &[u8]) -> Result { - let timer = metrics::start_timer(&metrics::BEACON_BLOCK_READ_TIMES); - - let len = bytes.len(); - let result = Self::from_ssz_bytes(bytes).map_err(Into::into); - - metrics::stop_timer(timer); - metrics::inc_counter(&metrics::BEACON_BLOCK_READ_COUNT); - metrics::inc_counter_by(&metrics::BEACON_BLOCK_READ_BYTES, len as u64); - - result - } +/// Prepare a signed beacon block for storage in the database. +#[must_use] +pub fn beacon_block_as_kv_store_op( + key: &Hash256, + block: &SignedBeaconBlock, +) -> KeyValueStoreOp { + // FIXME(altair): re-add block write/overhead metrics, or remove them + let db_key = get_key_for_col(DBColumn::BeaconBlock.into(), key.as_bytes()); + KeyValueStoreOp::PutKeyValue(db_key, block.as_ssz_bytes()) } diff --git a/beacon_node/store/src/impls/beacon_state.rs b/beacon_node/store/src/impls/beacon_state.rs index d6b74dc0b28..eb0acfb716f 100644 --- a/beacon_node/store/src/impls/beacon_state.rs +++ b/beacon_node/store/src/impls/beacon_state.rs @@ -1,8 +1,8 @@ use crate::*; -use ssz::{Decode, DecodeError, Encode}; -use ssz_derive::{Decode, Encode}; +use ssz::{DecodeError, Encode}; +use ssz_derive::Encode; use std::convert::TryInto; -use types::beacon_state::{CloneConfig, CommitteeCache, CACHED_EPOCHS}; +use types::beacon_state::{BeaconStateBase, CloneConfig, CommitteeCache, CACHED_EPOCHS}; pub fn store_full_state( state_root: &Hash256, @@ -23,13 +23,14 @@ pub fn store_full_state( pub fn get_full_state, E: EthSpec>( db: &KV, state_root: &Hash256, + spec: &ChainSpec, ) -> Result>, Error> { let total_timer = metrics::start_timer(&metrics::BEACON_STATE_READ_TIMES); match db.get_bytes(DBColumn::BeaconState.into(), state_root.as_bytes())? { Some(bytes) => { let overhead_timer = metrics::start_timer(&metrics::BEACON_STATE_READ_OVERHEAD_TIMES); - let container = StorageContainer::from_ssz_bytes(&bytes)?; + let container = StorageContainer::from_ssz_bytes(&bytes, spec)?; metrics::stop_timer(overhead_timer); metrics::stop_timer(total_timer); @@ -44,7 +45,7 @@ pub fn get_full_state, E: EthSpec>( /// A container for storing `BeaconState` components. // TODO: would be more space efficient with the caches stored separately and referenced by hash -#[derive(Encode, Decode)] +#[derive(Encode)] pub struct StorageContainer { state: BeaconState, committee_caches: Vec, @@ -58,6 +59,27 @@ impl StorageContainer { committee_caches: state.committee_caches().to_vec(), } } + + pub fn from_ssz_bytes(bytes: &[u8], spec: &ChainSpec) -> Result { + // We need to use the slot-switching `from_ssz_bytes` of `BeaconState`, which doesn't + // compose with the other SSZ utils, so we duplicate some parts of `ssz_derive` here. + let mut builder = ssz::SszDecoderBuilder::new(bytes); + + // Register the message type as `BeaconStateBase`, even though that isn't accurate. + // Really we just need some variable-length type to provide here. + builder.register_type::>()?; + builder.register_type::>()?; + + let mut decoder = builder.build()?; + + let state = decoder.decode_next_with(|bytes| BeaconState::from_ssz_bytes(bytes, spec))?; + let committee_caches = decoder.decode_next()?; + + Ok(Self { + state, + committee_caches, + }) + } } impl TryInto> for StorageContainer { diff --git a/beacon_node/store/src/impls/partial_beacon_state.rs b/beacon_node/store/src/impls/partial_beacon_state.rs deleted file mode 100644 index c3c284314b4..00000000000 --- a/beacon_node/store/src/impls/partial_beacon_state.rs +++ /dev/null @@ -1,16 +0,0 @@ -use crate::*; -use ssz::{Decode, Encode}; - -impl StoreItem for PartialBeaconState { - fn db_column() -> DBColumn { - DBColumn::BeaconState - } - - fn as_store_bytes(&self) -> Vec { - self.as_ssz_bytes() - } - - fn from_store_bytes(bytes: &[u8]) -> Result { - Ok(Self::from_ssz_bytes(bytes)?) - } -} diff --git a/beacon_node/store/src/partial_beacon_state.rs b/beacon_node/store/src/partial_beacon_state.rs index fee317a29e5..bd7bf08f354 100644 --- a/beacon_node/store/src/partial_beacon_state.rs +++ b/beacon_node/store/src/partial_beacon_state.rs @@ -2,8 +2,8 @@ use crate::chunked_vector::{ load_variable_list_from_db, load_vector_from_db, BlockRoots, HistoricalRoots, RandaoMixes, StateRoots, }; -use crate::{Error, KeyValueStore}; -use ssz::{Decode, DecodeError}; +use crate::{get_key_for_col, DBColumn, Error, KeyValueStore, KeyValueStoreOp}; +use ssz::{Decode, DecodeError, Encode}; use ssz_derive::{Decode, Encode}; use std::convert::TryInto; use types::superstruct; @@ -90,41 +90,6 @@ where pub next_sync_committee: SyncCommittee, } -impl Decode for PartialBeaconState { - fn is_ssz_fixed_len() -> bool { - assert!( - ! as Decode>::is_ssz_fixed_len() - && ! as Decode>::is_ssz_fixed_len() - ); - false - } - - fn from_ssz_bytes(bytes: &[u8]) -> Result { - // Slot is after genesis_time (u64) and genesis_validators_root (Hash256). - let slot_offset = ::ssz_fixed_len() + ::ssz_fixed_len(); - let slot_len = ::ssz_fixed_len(); - if bytes.len() < slot_offset + slot_len { - return Err(DecodeError::InvalidByteLength { - len: bytes.len(), - expected: slot_offset + slot_len, - }); - } - - let slot = Slot::from_ssz_bytes(&bytes[slot_offset..slot_offset + slot_len])?; - - let fork_schedule = get_fork_schedule_ssz()?; - - if fork_schedule - .altair_fork_slot - .map_or(true, |altair_slot| slot < altair_slot) - { - PartialBeaconStateBase::from_ssz_bytes(bytes).map(Self::Base) - } else { - PartialBeaconStateAltair::from_ssz_bytes(bytes).map(Self::Altair) - } - } -} - /// Implement the conversion function from BeaconState -> PartialBeaconState. macro_rules! impl_from_state_forgetful { ($s:ident, $outer:ident, $variant_name:ident, $struct_name:ident, [$($extra_fields:ident),*]) => { @@ -200,6 +165,37 @@ impl PartialBeaconState { } } + /// SSZ decode. + pub fn from_ssz_bytes(bytes: &[u8], spec: &ChainSpec) -> Result { + // Slot is after genesis_time (u64) and genesis_validators_root (Hash256). + let slot_offset = ::ssz_fixed_len() + ::ssz_fixed_len(); + let slot_len = ::ssz_fixed_len(); + if bytes.len() < slot_offset + slot_len { + return Err(DecodeError::InvalidByteLength { + len: bytes.len(), + expected: slot_offset + slot_len, + }); + } + + let slot = Slot::from_ssz_bytes(&bytes[slot_offset..slot_offset + slot_len])?; + + if spec + .altair_fork_slot + .map_or(true, |altair_slot| slot < altair_slot) + { + PartialBeaconStateBase::from_ssz_bytes(bytes).map(Self::Base) + } else { + PartialBeaconStateAltair::from_ssz_bytes(bytes).map(Self::Altair) + } + } + + /// Prepare the partial state for storage in the KV database. + #[must_use] + pub fn as_kv_store_op(&self, state_root: Hash256) -> KeyValueStoreOp { + let db_key = get_key_for_col(DBColumn::BeaconState.into(), state_root.as_bytes()); + KeyValueStoreOp::PutKeyValue(db_key, self.as_ssz_bytes()) + } + pub fn load_block_roots>( &mut self, store: &S, diff --git a/common/eth2_network_config/src/lib.rs b/common/eth2_network_config/src/lib.rs index b2745b1e3d8..f3f2d1a33af 100644 --- a/common/eth2_network_config/src/lib.rs +++ b/common/eth2_network_config/src/lib.rs @@ -1,11 +1,10 @@ use eth2_config::{predefined_networks_dir, *}; use enr::{CombinedKey, Enr}; -use ssz::Decode; use std::fs::{create_dir_all, File}; use std::io::{Read, Write}; use std::path::PathBuf; -use types::{AltairConfig, BeaconState, EthSpec, EthSpecId, YamlConfig}; +use types::{AltairConfig, BeaconState, ChainSpec, EthSpec, EthSpecId, YamlConfig}; pub const ADDRESS_FILE: &str = "deposit_contract.txt"; pub const DEPLOY_BLOCK_FILE: &str = "deploy_block.txt"; @@ -105,14 +104,25 @@ impl Eth2NetworkConfig { self.genesis_state_bytes.is_some() } + /// Construct a consolidated `ChainSpec` from the YAML config. + pub fn chain_spec(&self) -> Result { + ChainSpec::from_yaml::(&self.yaml_config, &self.altair_config).ok_or_else(|| { + format!( + "YAML configuration incompatible with spec constants for {}", + self.yaml_config.config_name + ) + }) + } + /// Attempts to deserialize `self.beacon_state`, returning an error if it's missing or invalid. pub fn beacon_state(&self) -> Result, String> { + let spec = self.chain_spec::()?; let genesis_state_bytes = self .genesis_state_bytes .as_ref() .ok_or("Genesis state is unknown")?; - BeaconState::from_ssz_bytes(genesis_state_bytes) + BeaconState::from_ssz_bytes(genesis_state_bytes, &spec) .map_err(|e| format!("Genesis state SSZ bytes are invalid: {:?}", e)) } diff --git a/consensus/ssz/src/decode.rs b/consensus/ssz/src/decode.rs index 38cd7c5fdcd..e3c0b190200 100644 --- a/consensus/ssz/src/decode.rs +++ b/consensus/ssz/src/decode.rs @@ -277,6 +277,14 @@ impl<'a> SszDecoder<'a> { pub fn decode_next(&mut self) -> Result { T::from_ssz_bytes(self.items.remove(0)) } + + /// Decodes the next item using the provided function. + pub fn decode_next_with(&mut self, f: F) -> Result + where + F: FnOnce(&'a [u8]) -> Result, + { + f(self.items.remove(0)) + } } /// Reads a `BYTES_PER_LENGTH_OFFSET`-byte union index from `bytes`, where `bytes.len() >= diff --git a/consensus/state_processing/src/genesis.rs b/consensus/state_processing/src/genesis.rs index 5182533d840..0054d48c94d 100644 --- a/consensus/state_processing/src/genesis.rs +++ b/consensus/state_processing/src/genesis.rs @@ -41,9 +41,7 @@ pub fn initialize_beacon_state_from_eth1( // To support testnets with Altair enabled from genesis, perform a possible state upgrade here. // This must happen *after* deposits and activations are processed or the calculation of sync // committees during the upgrade will fail. - if get_fork_schedule().map_or(false, |schedule| { - schedule.altair_fork_slot == Some(spec.genesis_slot) - }) { + if spec.altair_fork_slot == Some(spec.genesis_slot) { state = state.upgrade_to_altair(spec)?; // Reset the sync committees (this seems to be what the tests want) diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index 4b882a29fb3..ea772baf6cb 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -52,49 +52,13 @@ pub struct BeaconBlock { pub body: BeaconBlockBodyAltair, } -/// Custom `Decode` implementation for blocks that differentiates between hard fork blocks by slot. -impl Decode for BeaconBlock { - fn is_ssz_fixed_len() -> bool { - assert!( - ! as Decode>::is_ssz_fixed_len() - && ! as Decode>::is_ssz_fixed_len() - ); - false - } - - fn from_ssz_bytes(bytes: &[u8]) -> Result { - let slot_len = ::ssz_fixed_len(); - if bytes.len() < slot_len { - return Err(DecodeError::InvalidByteLength { - len: bytes.len(), - expected: slot_len, - }); - } - - let slot = Slot::from_ssz_bytes(&bytes[0..slot_len])?; - - let fork_schedule = get_fork_schedule_ssz()?; - - if fork_schedule - .altair_fork_slot - .map_or(true, |altair_slot| slot < altair_slot) - { - BeaconBlockBase::from_ssz_bytes(bytes).map(Self::Base) - } else { - BeaconBlockAltair::from_ssz_bytes(bytes).map(Self::Altair) - } - } -} - impl SignedRoot for BeaconBlock {} impl<'a, T: EthSpec> SignedRoot for BeaconBlockRef<'a, T> {} impl BeaconBlock { /// Returns an empty block to be used during genesis. pub fn empty(spec: &ChainSpec) -> Self { - if get_fork_schedule().map_or(false, |schedule| { - schedule.altair_fork_slot == Some(spec.genesis_slot) - }) { + if spec.altair_fork_slot == Some(spec.genesis_slot) { Self::Altair(BeaconBlockAltair::empty(spec)) } else { Self::Base(BeaconBlockBase::empty(spec)) @@ -196,6 +160,28 @@ impl BeaconBlock { BeaconBlock::Base(block) } + /// Custom SSZ decoder that takes a `ChainSpec` as context. + pub fn from_ssz_bytes(bytes: &[u8], spec: &ChainSpec) -> Result { + let slot_len = ::ssz_fixed_len(); + if bytes.len() < slot_len { + return Err(DecodeError::InvalidByteLength { + len: bytes.len(), + expected: slot_len, + }); + } + + let slot = Slot::from_ssz_bytes(&bytes[0..slot_len])?; + + if spec + .altair_fork_slot + .map_or(true, |altair_slot| slot < altair_slot) + { + BeaconBlockBase::from_ssz_bytes(bytes).map(Self::Base) + } else { + BeaconBlockAltair::from_ssz_bytes(bytes).map(Self::Altair) + } + } + /// Convenience accessor for the `body` as a `BeaconBlockBodyRef`. pub fn body(&self) -> BeaconBlockBodyRef<'_, T> { self.to_ref().body() diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 9d5d41075d9..e073fad0150 100644 --- a/consensus/types/src/beacon_state.rs +++ b/consensus/types/src/beacon_state.rs @@ -290,43 +290,6 @@ impl Clone for BeaconState { } } -impl Decode for BeaconState { - fn is_ssz_fixed_len() -> bool { - assert!( - ! as Decode>::is_ssz_fixed_len() - && ! as Decode>::is_ssz_fixed_len() - ); - false - } - - // FIXME(altair): not sure if we should abstract this pattern - // it's repeated on PartialBeaconState & BeaconBlock - fn from_ssz_bytes(bytes: &[u8]) -> Result { - // Slot is after genesis_time (u64) and genesis_validators_root (Hash256). - let slot_offset = ::ssz_fixed_len() + ::ssz_fixed_len(); - let slot_len = ::ssz_fixed_len(); - if bytes.len() < slot_offset + slot_len { - return Err(DecodeError::InvalidByteLength { - len: bytes.len(), - expected: slot_offset + slot_len, - }); - } - - let slot = Slot::from_ssz_bytes(&bytes[slot_offset..slot_offset + slot_len])?; - - let fork_schedule = get_fork_schedule_ssz()?; - - if fork_schedule - .altair_fork_slot - .map_or(true, |altair_slot| slot < altair_slot) - { - BeaconStateBase::from_ssz_bytes(bytes).map(Self::Base) - } else { - BeaconStateAltair::from_ssz_bytes(bytes).map(Self::Altair) - } - } -} - impl BeaconState { /// Create a new BeaconState suitable for genesis. /// @@ -387,6 +350,30 @@ impl BeaconState { }) } + /// Specialised deserialisation method that uses the `ChainSpec` as context. + pub fn from_ssz_bytes(bytes: &[u8], spec: &ChainSpec) -> Result { + // Slot is after genesis_time (u64) and genesis_validators_root (Hash256). + let slot_offset = ::ssz_fixed_len() + ::ssz_fixed_len(); + let slot_len = ::ssz_fixed_len(); + if bytes.len() < slot_offset + slot_len { + return Err(DecodeError::InvalidByteLength { + len: bytes.len(), + expected: slot_offset + slot_len, + }); + } + + let slot = Slot::from_ssz_bytes(&bytes[slot_offset..slot_offset + slot_len])?; + + if spec + .altair_fork_slot + .map_or(true, |altair_slot| slot < altair_slot) + { + BeaconStateBase::from_ssz_bytes(bytes).map(Self::Base) + } else { + BeaconStateAltair::from_ssz_bytes(bytes).map(Self::Altair) + } + } + /// Returns the `tree_hash_root` of the state. /// /// Spec v0.12.1 diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index ba80d80384e..4ffec29d04a 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -118,7 +118,8 @@ pub struct ChainSpec { domain_sync_committee_selection_proof: u32, domain_contribution_and_proof: u32, pub altair_fork_version: [u8; 4], - pub altair_fork_slot: Slot, + /// The Altair fork slot is optional, with `None` representing "Altair never happens". + pub altair_fork_slot: Option, /* * Networking @@ -134,6 +135,17 @@ pub struct ChainSpec { } impl ChainSpec { + /// Construct a `ChainSpec` from several YAML config files. + pub fn from_yaml( + phase0_yaml: &YamlConfig, + altair_yaml: &AltairConfig, + ) -> Option { + let mut spec = T::default_spec(); + spec = phase0_yaml.apply_to_chain_spec::(&spec)?; + spec = altair_yaml.apply_to_chain_spec::(&spec)?; + Some(spec) + } + /// Returns an `EnrForkId` for the given `slot`. /// /// Presently, we don't have any forks so we just ignore the slot. In the future this function @@ -349,7 +361,7 @@ impl ChainSpec { domain_sync_committee_selection_proof: 8, domain_contribution_and_proof: 9, altair_fork_version: [0x01, 0x00, 0x00, 0x00], - altair_fork_slot: Slot::new(u64::MAX), + altair_fork_slot: Some(Slot::new(u64::MAX)), /* * Network specific @@ -388,7 +400,7 @@ impl ChainSpec { // Altair epochs_per_sync_committee_period: Epoch::new(8), altair_fork_version: [0x01, 0x00, 0x00, 0x01], - altair_fork_slot: Slot::new(u64::MAX), + altair_fork_slot: Some(Slot::new(u64::MAX)), // Other network_id: 2, // lighthouse testnet network id deposit_chain_id: 5, @@ -400,24 +412,6 @@ impl ChainSpec { ..ChainSpec::mainnet() } } - - /// Suits the `v0.12.3` version of the eth2 spec: - /// https://github.com/ethereum/eth2.0-specs/blob/v0.12.3/configs/mainnet/phase0.yaml - /// - /// This method only needs to exist whilst we provide support for "legacy" testnets prior to v1.0.0 - /// (e.g., Medalla, Pyrmont, Spadina, Altona, etc.). - pub fn v012_legacy() -> Self { - let boot_nodes = vec![]; - - Self { - genesis_delay: 172_800, // 2 days - inactivity_penalty_quotient: u64::pow(2, 24), - min_slashing_penalty_quotient: 32, - eth1_follow_distance: 1024, - boot_nodes, - ..ChainSpec::mainnet() - } - } } impl Default for ChainSpec { @@ -899,7 +893,7 @@ impl AltairConfig { domain_sync_committee_selection_proof: self.domain_sync_committee_selection_proof, domain_contribution_and_proof: self.domain_contribution_and_proof, altair_fork_version: self.altair_fork_version, - altair_fork_slot: self.altair_fork_slot, + altair_fork_slot: Some(self.altair_fork_slot), ..chain_spec.clone() }) } diff --git a/consensus/types/src/fork_name.rs b/consensus/types/src/fork_name.rs new file mode 100644 index 00000000000..76f68772092 --- /dev/null +++ b/consensus/types/src/fork_name.rs @@ -0,0 +1,28 @@ +use crate::ChainSpec; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ForkName { + Genesis, + Altair, +} + +impl ForkName { + pub fn list_all() -> Vec { + vec![ForkName::Genesis, ForkName::Altair] + } + + /// Set the activation slots in the given `ChainSpec` so that the fork named by `self` + /// is the only fork in effect from genesis. + pub fn make_genesis_spec(&self, mut spec: ChainSpec) -> ChainSpec { + match self { + ForkName::Genesis => { + spec.altair_fork_slot = None; + spec + } + ForkName::Altair => { + spec.altair_fork_slot = Some(spec.genesis_slot); + spec + } + } + } +} diff --git a/consensus/types/src/fork_schedule.rs b/consensus/types/src/fork_schedule.rs deleted file mode 100644 index 5b4e634e71c..00000000000 --- a/consensus/types/src/fork_schedule.rs +++ /dev/null @@ -1,40 +0,0 @@ -use crate::{ChainSpec, Slot}; -use lazy_static::lazy_static; -use parking_lot::RwLock; - -lazy_static! { - static ref FORK_SCHEDULE: RwLock> = RwLock::new(None); -} - -/// Initialise the global fork schedule. -/// -/// MUST be called before any of the types that rely on it are used. -pub fn init_fork_schedule(fork_schedule: ForkSchedule) { - *FORK_SCHEDULE.write() = Some(fork_schedule); -} - -/// Read a copy of the fork schedule from the global variable. -pub fn get_fork_schedule() -> Option { - FORK_SCHEDULE.read().clone() -} - -/// Convenience method for getting the fork schedule during an SSZ decode. -pub fn get_fork_schedule_ssz() -> Result { - get_fork_schedule() - .ok_or_else(|| ssz::DecodeError::BytesInvalid("fork schedule not initialised".into())) -} - -/// Constants related to hard-fork upgrades. -#[derive(Debug, Clone)] -pub struct ForkSchedule { - /// A `None` value indicates that Altair will not take place in this schedule. - pub altair_fork_slot: Option, -} - -impl From<&ChainSpec> for ForkSchedule { - fn from(spec: &ChainSpec) -> Self { - ForkSchedule { - altair_fork_slot: Some(spec.altair_fork_slot), - } - } -} diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 0364d1928fa..6a4ccfedae6 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -32,6 +32,7 @@ pub mod eth1_data; pub mod eth_spec; pub mod fork; pub mod fork_data; +pub mod fork_name; pub mod free_attestation; pub mod graffiti; pub mod historical_batch; @@ -51,7 +52,6 @@ pub mod validator_subscription; pub mod voluntary_exit; #[macro_use] pub mod slot_epoch_macros; -pub mod fork_schedule; pub mod participation_flags; pub mod slot_epoch; pub mod subnet_id; @@ -86,9 +86,7 @@ pub use crate::eth1_data::Eth1Data; pub use crate::eth_spec::EthSpecId; pub use crate::fork::Fork; pub use crate::fork_data::ForkData; -pub use crate::fork_schedule::{ - get_fork_schedule, get_fork_schedule_ssz, init_fork_schedule, ForkSchedule, -}; +pub use crate::fork_name::ForkName; pub use crate::free_attestation::FreeAttestation; pub use crate::graffiti::{Graffiti, GRAFFITI_BYTES_LEN}; pub use crate::historical_batch::HistoricalBatch; diff --git a/consensus/types/src/signed_beacon_block.rs b/consensus/types/src/signed_beacon_block.rs index 4fead0507ee..45ce2236c7f 100644 --- a/consensus/types/src/signed_beacon_block.rs +++ b/consensus/types/src/signed_beacon_block.rs @@ -4,7 +4,6 @@ use crate::{ }; use bls::Signature; use serde_derive::{Deserialize, Serialize}; -use ssz::Decode; use ssz_derive::{Decode, Encode}; use std::fmt; use superstruct::superstruct; @@ -69,25 +68,27 @@ pub struct SignedBeaconBlock { pub signature: Signature, } -/// Proxy the decode implementation to `BeaconBlock`'s slot-switching impl. -#[derive(Debug, Decode)] -struct SignedBeaconBlockForDecoding { - pub message: BeaconBlock, - pub signature: Signature, -} +impl SignedBeaconBlock { + /// SSZ decode. + pub fn from_ssz_bytes(bytes: &[u8], spec: &ChainSpec) -> Result { + // We need to use the slot-switching `from_ssz_bytes` of `BeaconBlock`, which doesn't + // compose with the other SSZ utils, so we duplicate some parts of `ssz_derive` here. + let mut builder = ssz::SszDecoderBuilder::new(bytes); -impl Decode for SignedBeaconBlock { - fn is_ssz_fixed_len() -> bool { - as Decode>::is_ssz_fixed_len() - } + // Register the message type as `BeaconBlockBase`, even though that isn't accurate. + // Really we just need some variable-length type to provide here. + builder.register_type::>()?; + builder.register_type::()?; - fn from_ssz_bytes(bytes: &[u8]) -> Result { - SignedBeaconBlockForDecoding::from_ssz_bytes(bytes) - .map(|block| SignedBeaconBlock::from_block(block.message, block.signature)) + let mut decoder = builder.build()?; + + // Read the first item as a `BeaconBlock`. + let message = decoder.decode_next_with(|bytes| BeaconBlock::from_ssz_bytes(bytes, spec))?; + let signature = decoder.decode_next()?; + + Ok(Self::from_block(message, signature)) } -} -impl SignedBeaconBlock { /// Create a new `SignedBeaconBlock` from a `BeaconBlock` and `Signature`. pub fn from_block(block: BeaconBlock, signature: Signature) -> Self { match block { diff --git a/testing/ef_tests/src/cases.rs b/testing/ef_tests/src/cases.rs index 818af1a6eca..b97cdcc9149 100644 --- a/testing/ef_tests/src/cases.rs +++ b/testing/ef_tests/src/cases.rs @@ -2,6 +2,7 @@ use super::*; use rayon::prelude::*; use std::fmt::Debug; use std::path::{Path, PathBuf}; +use types::ForkName; mod bls_aggregate_sigs; mod bls_aggregate_verify; @@ -37,7 +38,7 @@ pub use ssz_static::*; pub trait LoadCase: Sized { /// Load the test case from a test case directory. - fn load_from_dir(_path: &Path) -> Result; + fn load_from_dir(_path: &Path, _fork_name: ForkName) -> Result; } pub trait Case: Debug + Sync { @@ -48,11 +49,18 @@ pub trait Case: Debug + Sync { "no description".to_string() } + /// Whether or not this test exists for the given `fork_name`. + /// + /// Returns `true` by default. + fn is_enabled_for_fork(_fork_name: ForkName) -> bool { + true + } + /// Execute a test and return the result. /// /// `case_index` reports the index of the case in the set of test cases. It is not strictly /// necessary, but it's useful when troubleshooting specific failing tests. - fn result(&self, case_index: usize) -> Result<(), Error>; + fn result(&self, case_index: usize, fork_name: ForkName) -> Result<(), Error>; } #[derive(Debug)] @@ -61,11 +69,11 @@ pub struct Cases { } impl Cases { - pub fn test_results(&self) -> Vec { + pub fn test_results(&self, fork_name: ForkName) -> Vec { self.test_cases .into_par_iter() .enumerate() - .map(|(i, (ref path, ref tc))| CaseResult::new(i, path, tc, tc.result(i))) + .map(|(i, (ref path, ref tc))| CaseResult::new(i, path, tc, tc.result(i, fork_name))) .collect() } } diff --git a/testing/ef_tests/src/cases/bls_aggregate_sigs.rs b/testing/ef_tests/src/cases/bls_aggregate_sigs.rs index 776e4107183..dfe6fe528a3 100644 --- a/testing/ef_tests/src/cases/bls_aggregate_sigs.rs +++ b/testing/ef_tests/src/cases/bls_aggregate_sigs.rs @@ -13,7 +13,7 @@ pub struct BlsAggregateSigs { impl BlsCase for BlsAggregateSigs {} impl Case for BlsAggregateSigs { - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { let mut aggregate_signature = AggregateSignature::infinity(); for key_str in &self.input { diff --git a/testing/ef_tests/src/cases/bls_aggregate_verify.rs b/testing/ef_tests/src/cases/bls_aggregate_verify.rs index 4f6df9981bb..3650e8a0d7f 100644 --- a/testing/ef_tests/src/cases/bls_aggregate_verify.rs +++ b/testing/ef_tests/src/cases/bls_aggregate_verify.rs @@ -21,7 +21,7 @@ pub struct BlsAggregateVerify { impl BlsCase for BlsAggregateVerify {} impl Case for BlsAggregateVerify { - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { let messages = self .input .messages diff --git a/testing/ef_tests/src/cases/bls_fast_aggregate_verify.rs b/testing/ef_tests/src/cases/bls_fast_aggregate_verify.rs index 7a0d870e04c..71743ad99b6 100644 --- a/testing/ef_tests/src/cases/bls_fast_aggregate_verify.rs +++ b/testing/ef_tests/src/cases/bls_fast_aggregate_verify.rs @@ -23,7 +23,7 @@ pub struct BlsFastAggregateVerify { impl BlsCase for BlsFastAggregateVerify {} impl Case for BlsFastAggregateVerify { - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { let message = Hash256::from_slice( &hex::decode(&self.input.message[2..]) .map_err(|e| Error::FailedToParseTest(format!("{:?}", e)))?, diff --git a/testing/ef_tests/src/cases/bls_sign_msg.rs b/testing/ef_tests/src/cases/bls_sign_msg.rs index 6687dda2280..77d30281d11 100644 --- a/testing/ef_tests/src/cases/bls_sign_msg.rs +++ b/testing/ef_tests/src/cases/bls_sign_msg.rs @@ -20,7 +20,7 @@ pub struct BlsSign { impl BlsCase for BlsSign {} impl Case for BlsSign { - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { // Convert private_key and message to required types let sk = hex::decode(&self.input.privkey[2..]) .map_err(|e| Error::FailedToParseTest(format!("{:?}", e)))?; diff --git a/testing/ef_tests/src/cases/bls_verify_msg.rs b/testing/ef_tests/src/cases/bls_verify_msg.rs index 0684d76989a..83fd949684c 100644 --- a/testing/ef_tests/src/cases/bls_verify_msg.rs +++ b/testing/ef_tests/src/cases/bls_verify_msg.rs @@ -22,7 +22,7 @@ pub struct BlsVerify { impl BlsCase for BlsVerify {} impl Case for BlsVerify { - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { let message = hex::decode(&self.input.message[2..]) .map_err(|e| Error::FailedToParseTest(format!("{:?}", e)))?; diff --git a/testing/ef_tests/src/cases/common.rs b/testing/ef_tests/src/cases/common.rs index e648ef6ec51..4d4f6b8bb07 100644 --- a/testing/ef_tests/src/cases/common.rs +++ b/testing/ef_tests/src/cases/common.rs @@ -1,6 +1,7 @@ use crate::cases::LoadCase; use crate::decode::yaml_decode_file; use crate::error::Error; +use crate::testing_spec; use serde_derive::Deserialize; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; @@ -8,12 +9,13 @@ use std::convert::TryFrom; use std::fmt::Debug; use std::path::Path; use tree_hash::TreeHash; +use types::{BeaconState, EthSpec, ForkName, SignedBeaconBlock}; /// Trait for all BLS cases to eliminate some boilerplate. pub trait BlsCase: serde::de::DeserializeOwned {} impl LoadCase for T { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { yaml_decode_file(&path.join("data.yaml")) } } @@ -60,13 +62,33 @@ macro_rules! uint_wrapper { uint_wrapper!(TestU128, ethereum_types::U128); uint_wrapper!(TestU256, ethereum_types::U256); -/// Trait alias for all deez bounds +/// Trait for types that can be used in SSZ static tests. pub trait SszStaticType: - serde::de::DeserializeOwned + Decode + Encode + TreeHash + Clone + PartialEq + Debug + Sync + serde::de::DeserializeOwned + Encode + TreeHash + Clone + PartialEq + Debug + Sync { + // fn decode(bytes: &[u8], fork_name: ForkName) -> Result; } impl SszStaticType for T where - T: serde::de::DeserializeOwned + Decode + Encode + TreeHash + Clone + PartialEq + Debug + Sync + T: serde::de::DeserializeOwned + Encode + TreeHash + Clone + PartialEq + Debug + Sync { } +/* +{ + fn decode(bytes: &[u8], _: ForkName) -> Result { + Self::from_ssz_bytes(bytes) + } +} + +impl SszStaticType for BeaconState { + fn decode(bytes: &[u8], fork_name: ForkName) -> Result { + Self::from_ssz_bytes(bytes, &testing_spec::(fork_name)) + } +} + +impl SszStaticType for SignedBeaconBlock { + fn decode(bytes: &[u8], fork_name: ForkName) -> Result { + Self::from_ssz_bytes(bytes, &testing_spec::(fork_name)) + } +} +*/ diff --git a/testing/ef_tests/src/cases/epoch_processing.rs b/testing/ef_tests/src/cases/epoch_processing.rs index 035cea8ce94..3888ea7911f 100644 --- a/testing/ef_tests/src/cases/epoch_processing.rs +++ b/testing/ef_tests/src/cases/epoch_processing.rs @@ -1,11 +1,10 @@ use super::*; use crate::bls_setting::BlsSetting; use crate::case_result::compare_beacon_state_results_without_caches; -use crate::decode::{snappy_decode_file, yaml_decode_file}; +use crate::decode::{ssz_decode_state, yaml_decode_file}; use crate::type_name; use crate::type_name::TypeName; use serde_derive::Deserialize; -use ssz::Decode; use state_processing::per_epoch_processing::validator_statuses::ValidatorStatuses; use state_processing::per_epoch_processing::{ altair, base, @@ -17,7 +16,7 @@ use state_processing::per_epoch_processing::{ use state_processing::EpochProcessingError; use std::marker::PhantomData; use std::path::{Path, PathBuf}; -use types::{BeaconState, ChainSpec, EthSpec}; +use types::{BeaconState, ChainSpec, EthSpec, ForkName}; #[derive(Debug, Clone, Default, Deserialize)] pub struct Metadata { @@ -190,23 +189,25 @@ impl EpochTransition for SyncCommitteeUpdates { } impl> LoadCase for EpochProcessing { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { + let spec = &testing_spec::(fork_name); let metadata_path = path.join("meta.yaml"); let metadata: Metadata = if metadata_path.is_file() { yaml_decode_file(&metadata_path)? + } else if T::name() == "sync_committee_updates" { + // FIXME(altair): this is a hack because the epoch tests are missing metadata + // and the sync aggregate tests need real BLS + Metadata { + description: None, + bls_setting: Some(BlsSetting::Required), + } } else { Metadata::default() }; - let pre = BeaconState::from_ssz_bytes( - snappy_decode_file(&path.join("pre.ssz_snappy"))?.as_slice(), - ) - .expect("Could not ssz decode pre beacon state"); + let pre = ssz_decode_state(&path.join("pre.ssz_snappy"), spec)?; let post_file = path.join("post.ssz_snappy"); let post = if post_file.is_file() { - Some( - BeaconState::from_ssz_bytes(snappy_decode_file(&post_file)?.as_slice()) - .expect("Could not ssz decode post beacon state"), - ) + Some(ssz_decode_state(&post_file, spec)?) } else { None }; @@ -229,11 +230,21 @@ impl> Case for EpochProcessing { .unwrap_or_else(String::new) } - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + match fork_name { + // No sync committee tests for genesis fork. + ForkName::Genesis => T::name() != "sync_committee_updates", + ForkName::Altair => true, + } + } + + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { + self.metadata.bls_setting.unwrap_or_default().check()?; + let mut state = self.pre.clone(); let mut expected = self.post.clone(); - let spec = &E::default_spec(); + let spec = &testing_spec::(fork_name); let mut result = (|| { // Processing requires the committee caches. diff --git a/testing/ef_tests/src/cases/genesis_initialization.rs b/testing/ef_tests/src/cases/genesis_initialization.rs index 0facb33d4b8..dc1b2a68bab 100644 --- a/testing/ef_tests/src/cases/genesis_initialization.rs +++ b/testing/ef_tests/src/cases/genesis_initialization.rs @@ -1,10 +1,10 @@ use super::*; use crate::case_result::compare_beacon_state_results_without_caches; -use crate::decode::{ssz_decode_file, yaml_decode_file}; +use crate::decode::{ssz_decode_file, ssz_decode_state, yaml_decode_file}; use serde_derive::Deserialize; use state_processing::initialize_beacon_state_from_eth1; use std::path::PathBuf; -use types::{BeaconState, Deposit, EthSpec, Hash256}; +use types::{BeaconState, Deposit, EthSpec, ForkName, Hash256}; #[derive(Debug, Clone, Deserialize)] struct Metadata { @@ -28,7 +28,7 @@ pub struct GenesisInitialization { } impl LoadCase for GenesisInitialization { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { let Eth1 { eth1_block_hash, eth1_timestamp, @@ -40,7 +40,8 @@ impl LoadCase for GenesisInitialization { ssz_decode_file(&path.join(filename)) }) .collect::>()?; - let state = ssz_decode_file(&path.join("state.ssz_snappy"))?; + let spec = &testing_spec::(fork_name); + let state = ssz_decode_state(&path.join("state.ssz_snappy"), spec)?; Ok(Self { path: path.into(), @@ -53,8 +54,8 @@ impl LoadCase for GenesisInitialization { } impl Case for GenesisInitialization { - fn result(&self, _case_index: usize) -> Result<(), Error> { - let spec = &E::default_spec(); + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { + let spec = &testing_spec::(fork_name); let mut result = initialize_beacon_state_from_eth1( self.eth1_block_hash, diff --git a/testing/ef_tests/src/cases/genesis_validity.rs b/testing/ef_tests/src/cases/genesis_validity.rs index 80abe9a3109..4a722c96ddb 100644 --- a/testing/ef_tests/src/cases/genesis_validity.rs +++ b/testing/ef_tests/src/cases/genesis_validity.rs @@ -1,9 +1,9 @@ use super::*; -use crate::decode::{ssz_decode_file, yaml_decode_file}; +use crate::decode::{ssz_decode_state, yaml_decode_file}; use serde_derive::Deserialize; use state_processing::is_valid_genesis_state; use std::path::Path; -use types::{BeaconState, EthSpec}; +use types::{BeaconState, EthSpec, ForkName}; #[derive(Debug, Clone, Deserialize)] #[serde(bound = "E: EthSpec")] @@ -13,8 +13,9 @@ pub struct GenesisValidity { } impl LoadCase for GenesisValidity { - fn load_from_dir(path: &Path) -> Result { - let genesis = ssz_decode_file(&path.join("genesis.ssz_snappy"))?; + fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { + let spec = &testing_spec::(fork_name); + let genesis = ssz_decode_state(&path.join("genesis.ssz_snappy"), spec)?; let is_valid = yaml_decode_file(&path.join("is_valid.yaml"))?; Ok(Self { genesis, is_valid }) @@ -22,8 +23,8 @@ impl LoadCase for GenesisValidity { } impl Case for GenesisValidity { - fn result(&self, _case_index: usize) -> Result<(), Error> { - let spec = &E::default_spec(); + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { + let spec = &testing_spec::(fork_name); let is_valid = is_valid_genesis_state(&self.genesis, spec); diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index da49cf229dd..a3220e53991 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -1,10 +1,10 @@ use super::*; use crate::bls_setting::BlsSetting; use crate::case_result::compare_beacon_state_results_without_caches; -use crate::decode::{ssz_decode_file, yaml_decode_file}; +use crate::decode::{ssz_decode_file, ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; +use crate::testing_spec; use crate::type_name::TypeName; use serde_derive::Deserialize; -use ssz::Decode; use state_processing::per_block_processing::{ errors::BlockProcessingError, process_block_header, @@ -17,7 +17,7 @@ use state_processing::per_block_processing::{ use std::fmt::Debug; use std::path::Path; use types::{ - Attestation, AttesterSlashing, BeaconBlock, BeaconState, ChainSpec, Deposit, EthSpec, + Attestation, AttesterSlashing, BeaconBlock, BeaconState, ChainSpec, Deposit, EthSpec, ForkName, ProposerSlashing, SignedVoluntaryExit, SyncAggregate, }; @@ -35,7 +35,7 @@ pub struct Operations> { pub post: Option>, } -pub trait Operation: Decode + TypeName + Debug + Sync { +pub trait Operation: TypeName + Debug + Sync + Sized { fn handler_name() -> String { Self::name().to_lowercase() } @@ -44,6 +44,8 @@ pub trait Operation: Decode + TypeName + Debug + Sync { format!("{}.ssz_snappy", Self::handler_name()) } + fn decode(path: &Path, spec: &ChainSpec) -> Result; + fn apply_to( &self, state: &mut BeaconState, @@ -52,6 +54,10 @@ pub trait Operation: Decode + TypeName + Debug + Sync { } impl Operation for Attestation { + fn decode(path: &Path, _spec: &ChainSpec) -> Result { + ssz_decode_file(path) + } + fn apply_to( &self, state: &mut BeaconState, @@ -73,6 +79,10 @@ impl Operation for AttesterSlashing { "attester_slashing".into() } + fn decode(path: &Path, _spec: &ChainSpec) -> Result { + ssz_decode_file(path) + } + fn apply_to( &self, state: &mut BeaconState, @@ -83,6 +93,10 @@ impl Operation for AttesterSlashing { } impl Operation for Deposit { + fn decode(path: &Path, _spec: &ChainSpec) -> Result { + ssz_decode_file(path) + } + fn apply_to( &self, state: &mut BeaconState, @@ -97,6 +111,10 @@ impl Operation for ProposerSlashing { "proposer_slashing".into() } + fn decode(path: &Path, _spec: &ChainSpec) -> Result { + ssz_decode_file(path) + } + fn apply_to( &self, state: &mut BeaconState, @@ -111,6 +129,10 @@ impl Operation for SignedVoluntaryExit { "voluntary_exit".into() } + fn decode(path: &Path, _spec: &ChainSpec) -> Result { + ssz_decode_file(path) + } + fn apply_to( &self, state: &mut BeaconState, @@ -129,6 +151,10 @@ impl Operation for BeaconBlock { "block.ssz_snappy".into() } + fn decode(path: &Path, spec: &ChainSpec) -> Result { + ssz_decode_file_with(path, |bytes| BeaconBlock::from_ssz_bytes(bytes, spec)) + } + fn apply_to( &self, state: &mut BeaconState, @@ -148,6 +174,10 @@ impl Operation for SyncAggregate { "sync_aggregate.ssz_snappy".into() } + fn decode(path: &Path, _spec: &ChainSpec) -> Result { + ssz_decode_file(path) + } + fn apply_to( &self, state: &mut BeaconState, @@ -159,7 +189,8 @@ impl Operation for SyncAggregate { } impl> LoadCase for Operations { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { + let spec = &testing_spec::(fork_name); let metadata_path = path.join("meta.yaml"); let metadata: Metadata = if metadata_path.is_file() { yaml_decode_file(&metadata_path)? @@ -167,12 +198,12 @@ impl> LoadCase for Operations { Metadata::default() }; - let pre = ssz_decode_file(&path.join("pre.ssz_snappy"))?; + let pre = ssz_decode_state(&path.join("pre.ssz_snappy"), spec)?; // Check BLS setting here before SSZ deserialization, as most types require signatures // to be valid. let (operation, bls_error) = if metadata.bls_setting.unwrap_or_default().check().is_ok() { - match ssz_decode_file(&path.join(O::filename())) { + match O::decode(&path.join(O::filename()), spec) { Ok(op) => (Some(op), None), Err(Error::InvalidBLSInput(error)) => (None, Some(error)), Err(e) => return Err(e), @@ -185,7 +216,7 @@ impl> LoadCase for Operations { if let Some(bls_error) = bls_error { panic!("input is unexpectedly invalid: {}", bls_error); } - Some(ssz_decode_file(&post_filename)?) + Some(ssz_decode_state(&post_filename, spec)?) } else { None }; @@ -207,8 +238,16 @@ impl> Case for Operations { .unwrap_or_else(String::new) } - fn result(&self, _case_index: usize) -> Result<(), Error> { - let spec = &E::default_spec(); + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + match fork_name { + // Genesis doesn't have sync aggregate tests + ForkName::Genesis => O::handler_name() != "sync_committee", + _ => true, + } + } + + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { + let spec = &testing_spec::(fork_name); let mut state = self.pre.clone(); let mut expected = self.post.clone(); diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index 792935b2a58..cb5708b12e1 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -1,12 +1,12 @@ use super::*; use crate::bls_setting::BlsSetting; use crate::case_result::compare_beacon_state_results_without_caches; -use crate::decode::{ssz_decode_file, yaml_decode_file}; +use crate::decode::{ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use serde_derive::Deserialize; use state_processing::{ per_block_processing, per_slot_processing, BlockProcessingError, BlockSignatureStrategy, }; -use types::{BeaconState, EthSpec, RelativeEpoch, SignedBeaconBlock}; +use types::{BeaconState, EthSpec, ForkName, RelativeEpoch, SignedBeaconBlock}; #[derive(Debug, Clone, Deserialize)] pub struct Metadata { @@ -25,18 +25,21 @@ pub struct SanityBlocks { } impl LoadCase for SanityBlocks { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { + let spec = &testing_spec::(fork_name); let metadata: Metadata = yaml_decode_file(&path.join("meta.yaml"))?; - let pre = ssz_decode_file(&path.join("pre.ssz_snappy"))?; - let blocks: Vec> = (0..metadata.blocks_count) + let pre = ssz_decode_state(&path.join("pre.ssz_snappy"), spec)?; + let blocks = (0..metadata.blocks_count) .map(|i| { let filename = format!("blocks_{}.ssz_snappy", i); - ssz_decode_file(&path.join(filename)) + ssz_decode_file_with(&path.join(filename), |bytes| { + SignedBeaconBlock::from_ssz_bytes(bytes, spec) + }) }) - .collect::>()?; + .collect::, _>>()?; let post_file = path.join("post.ssz_snappy"); let post = if post_file.is_file() { - Some(ssz_decode_file(&post_file)?) + Some(ssz_decode_state(&post_file, spec)?) } else { None }; @@ -58,12 +61,12 @@ impl Case for SanityBlocks { .unwrap_or_else(String::new) } - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { self.metadata.bls_setting.unwrap_or_default().check()?; let mut bulk_state = self.pre.clone(); let mut expected = self.post.clone(); - let spec = &E::default_spec(); + let spec = &testing_spec::(fork_name); // Processing requires the epoch cache. bulk_state.build_all_caches(spec).unwrap(); diff --git a/testing/ef_tests/src/cases/sanity_slots.rs b/testing/ef_tests/src/cases/sanity_slots.rs index 4ca20879288..93a05b3641b 100644 --- a/testing/ef_tests/src/cases/sanity_slots.rs +++ b/testing/ef_tests/src/cases/sanity_slots.rs @@ -1,10 +1,10 @@ use super::*; use crate::bls_setting::BlsSetting; use crate::case_result::compare_beacon_state_results_without_caches; -use crate::decode::{ssz_decode_file, yaml_decode_file}; +use crate::decode::{ssz_decode_state, yaml_decode_file}; use serde_derive::Deserialize; use state_processing::per_slot_processing; -use types::{BeaconState, EthSpec}; +use types::{BeaconState, EthSpec, ForkName}; #[derive(Debug, Clone, Default, Deserialize)] pub struct Metadata { @@ -22,18 +22,19 @@ pub struct SanitySlots { } impl LoadCase for SanitySlots { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, fork_name: ForkName) -> Result { + let spec = &testing_spec::(fork_name); let metadata_path = path.join("meta.yaml"); let metadata: Metadata = if metadata_path.is_file() { yaml_decode_file(&metadata_path)? } else { Metadata::default() }; - let pre = ssz_decode_file(&path.join("pre.ssz_snappy"))?; + let pre = ssz_decode_state(&path.join("pre.ssz_snappy"), spec)?; let slots: u64 = yaml_decode_file(&path.join("slots.yaml"))?; let post_file = path.join("post.ssz_snappy"); let post = if post_file.is_file() { - Some(ssz_decode_file(&post_file)?) + Some(ssz_decode_state(&post_file, spec)?) } else { None }; @@ -55,12 +56,12 @@ impl Case for SanitySlots { .unwrap_or_else(String::new) } - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { self.metadata.bls_setting.unwrap_or_default().check()?; let mut state = self.pre.clone(); let mut expected = self.post.clone(); - let spec = &E::default_spec(); + let spec = &testing_spec::(fork_name); // Processing requires the epoch cache. state.build_all_caches(spec).unwrap(); diff --git a/testing/ef_tests/src/cases/shuffling.rs b/testing/ef_tests/src/cases/shuffling.rs index 2ed5c0bd464..b5ce019f5ca 100644 --- a/testing/ef_tests/src/cases/shuffling.rs +++ b/testing/ef_tests/src/cases/shuffling.rs @@ -4,6 +4,7 @@ use crate::decode::yaml_decode_file; use serde_derive::Deserialize; use std::marker::PhantomData; use swap_or_not_shuffle::{compute_shuffled_index, shuffle_list}; +use types::ForkName; #[derive(Debug, Clone, Deserialize)] pub struct Shuffling { @@ -15,13 +16,13 @@ pub struct Shuffling { } impl LoadCase for Shuffling { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { yaml_decode_file(&path.join("mapping.yaml")) } } impl Case for Shuffling { - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { if self.count == 0 { compare_result::<_, Error>(&Ok(vec![]), &Some(self.mapping.clone()))?; } else { diff --git a/testing/ef_tests/src/cases/ssz_generic.rs b/testing/ef_tests/src/cases/ssz_generic.rs index 6972984ddb5..9b46001f97b 100644 --- a/testing/ef_tests/src/cases/ssz_generic.rs +++ b/testing/ef_tests/src/cases/ssz_generic.rs @@ -10,7 +10,7 @@ use ssz_derive::{Decode, Encode}; use std::path::{Path, PathBuf}; use tree_hash_derive::TreeHash; use types::typenum::*; -use types::{BitList, BitVector, FixedVector, VariableList}; +use types::{BitList, BitVector, FixedVector, ForkName, VariableList}; #[derive(Debug, Clone, Deserialize)] struct Metadata { @@ -26,7 +26,7 @@ pub struct SszGeneric { } impl LoadCase for SszGeneric { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { let components = path .components() .map(|c| c.as_os_str().to_string_lossy().into_owned()) @@ -118,7 +118,7 @@ macro_rules! type_dispatch { } impl Case for SszGeneric { - fn result(&self, _case_index: usize) -> Result<(), Error> { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { let parts = self.case_name.split('_').collect::>(); match self.handler_name.as_str() { @@ -194,7 +194,7 @@ impl Case for SszGeneric { } } -fn ssz_generic_test(path: &Path) -> Result<(), Error> { +fn ssz_generic_test(path: &Path) -> Result<(), Error> { let meta_path = path.join("meta.yaml"); let meta: Option = if meta_path.is_file() { Some(yaml_decode_file(&meta_path)?) @@ -215,7 +215,7 @@ fn ssz_generic_test(path: &Path) -> Result<(), Error> { // Valid // TODO: signing root (annoying because of traits) if let Some(value) = value { - check_serialization(&value, &serialized)?; + check_serialization(&value, &serialized, T::from_ssz_bytes)?; if let Some(ref meta) = meta { check_tree_hash(&meta.root, value.tree_hash_root().as_bytes())?; diff --git a/testing/ef_tests/src/cases/ssz_static.rs b/testing/ef_tests/src/cases/ssz_static.rs index 8b3ae631a52..5d2c3e3231b 100644 --- a/testing/ef_tests/src/cases/ssz_static.rs +++ b/testing/ef_tests/src/cases/ssz_static.rs @@ -4,8 +4,10 @@ use crate::cases::common::SszStaticType; use crate::decode::{snappy_decode_file, yaml_decode_file}; use cached_tree_hash::{CacheArena, CachedTreeHash}; use serde_derive::Deserialize; +use ssz::Decode; use std::marker::PhantomData; -use types::Hash256; +use tree_hash::TreeHash; +use types::{BeaconState, ForkName, Hash256}; #[derive(Debug, Clone, Deserialize)] struct SszStaticRoots { @@ -21,11 +23,10 @@ pub struct SszStatic { } #[derive(Debug, Clone)] -pub struct SszStaticTHC { +pub struct SszStaticTHC { roots: SszStaticRoots, serialized: Vec, value: T, - _phantom: PhantomData, } fn load_from_dir(path: &Path) -> Result<(SszStaticRoots, Vec, T), Error> { @@ -38,7 +39,7 @@ fn load_from_dir(path: &Path) -> Result<(SszStaticRoots, Vec LoadCase for SszStatic { - fn load_from_dir(path: &Path) -> Result { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { load_from_dir(path).map(|(roots, serialized, value)| Self { roots, serialized, @@ -47,25 +48,28 @@ impl LoadCase for SszStatic { } } -impl, C: Debug + Sync> LoadCase for SszStaticTHC { - fn load_from_dir(path: &Path) -> Result { +impl LoadCase for SszStaticTHC { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { load_from_dir(path).map(|(roots, serialized, value)| Self { roots, serialized, value, - _phantom: PhantomData, }) } } -pub fn check_serialization(value: &T, serialized: &[u8]) -> Result<(), Error> { +pub fn check_serialization( + value: &T, + serialized: &[u8], + deserializer: impl FnOnce(&[u8]) -> Result, +) -> Result<(), Error> { // Check serialization let serialized_result = value.as_ssz_bytes(); compare_result::(&Ok(value.ssz_bytes_len()), &Some(serialized.len()))?; compare_result::, Error>(&Ok(serialized_result), &Some(serialized.to_vec()))?; // Check deserialization - let deserialized_result = T::from_ssz_bytes(serialized); + let deserialized_result = deserializer(serialized); compare_result(&deserialized_result, &Some(value.clone()))?; Ok(()) @@ -79,17 +83,20 @@ pub fn check_tree_hash(expected_str: &str, actual_root: &[u8]) -> Result<(), Err compare_result::(&Ok(tree_hash_root), &Some(expected_root)) } -impl Case for SszStatic { - fn result(&self, _case_index: usize) -> Result<(), Error> { - check_serialization(&self.value, &self.serialized)?; +impl Case for SszStatic { + fn result(&self, _case_index: usize, _fork_name: ForkName) -> Result<(), Error> { + check_serialization(&self.value, &self.serialized, T::from_ssz_bytes)?; check_tree_hash(&self.roots.root, self.value.tree_hash_root().as_bytes())?; Ok(()) } } -impl, C: Debug + Sync> Case for SszStaticTHC { - fn result(&self, _case_index: usize) -> Result<(), Error> { - check_serialization(&self.value, &self.serialized)?; +impl Case for SszStaticTHC> { + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { + let spec = &testing_spec::(fork_name); + check_serialization(&self.value, &self.serialized, |bytes| { + BeaconState::from_ssz_bytes(bytes, spec) + })?; check_tree_hash(&self.roots.root, self.value.tree_hash_root().as_bytes())?; let arena = &mut CacheArena::default(); diff --git a/testing/ef_tests/src/decode.rs b/testing/ef_tests/src/decode.rs index 6d195626c7b..a885a43e9e3 100644 --- a/testing/ef_tests/src/decode.rs +++ b/testing/ef_tests/src/decode.rs @@ -2,6 +2,7 @@ use super::*; use snap::raw::Decoder; use std::fs::{self}; use std::path::Path; +use types::{BeaconState, EthSpec}; pub fn yaml_decode(string: &str) -> Result { serde_yaml::from_str(string).map_err(|e| Error::FailedToParseTest(format!("{:?}", e))) @@ -32,9 +33,12 @@ pub fn snappy_decode_file(path: &Path) -> Result, Error> { }) } -pub fn ssz_decode_file(path: &Path) -> Result { +pub fn ssz_decode_file_with(path: &Path, f: F) -> Result +where + F: FnOnce(&[u8]) -> Result, +{ let bytes = snappy_decode_file(path)?; - T::from_ssz_bytes(&bytes).map_err(|e| { + f(&bytes).map_err(|e| { match e { // NOTE: this is a bit hacky, but seemingly better than the alternatives ssz::DecodeError::BytesInvalid(message) @@ -50,3 +54,14 @@ pub fn ssz_decode_file(path: &Path) -> Result { } }) } + +pub fn ssz_decode_file(path: &Path) -> Result { + ssz_decode_file_with(path, T::from_ssz_bytes) +} + +pub fn ssz_decode_state( + path: &Path, + spec: &ChainSpec, +) -> Result, Error> { + ssz_decode_file_with(path, |bytes| BeaconState::from_ssz_bytes(bytes, spec)) +} diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index d1a8f115916..f51de4966ca 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -1,15 +1,12 @@ use crate::cases::{self, Case, Cases, EpochTransition, LoadCase, Operation}; +use crate::type_name; use crate::type_name::TypeName; -use crate::{get_fork_name, init_testing_fork_schedule, type_name}; use cached_tree_hash::CachedTreeHash; -use parking_lot::Once; use std::fmt::Debug; use std::fs; use std::marker::PhantomData; use std::path::PathBuf; -use types::EthSpec; - -static INIT_FORK: Once = Once::new(); +use types::{BeaconState, EthSpec, ForkName}; pub trait Handler { type Case: Case + LoadCase; @@ -22,25 +19,29 @@ pub trait Handler { fn handler_name() -> String; + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + Self::Case::is_enabled_for_fork(fork_name) + } + fn run() { - // XXX: This is a a bit of a hack. - let fork_name = get_fork_name(); - INIT_FORK.call_once(|| { - init_testing_fork_schedule(&fork_name); - }); - - // If the test is for the "general" config, then all its files lived under phase0. - let effective_fork_name = if Self::config_name() == "general" { - "phase0" - } else { - &fork_name + for fork_name in ForkName::list_all() { + if Self::is_enabled_for_fork(fork_name) { + Self::run_for_fork(fork_name) + } + } + } + + fn run_for_fork(fork_name: ForkName) { + let fork_name_str = match fork_name { + ForkName::Genesis => "phase0", + ForkName::Altair => "altair", }; let handler_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) .join("eth2.0-spec-tests") .join("tests") .join(Self::config_name()) - .join(effective_fork_name) + .join(fork_name_str) .join(Self::runner_name()) .join(Self::handler_name()); @@ -56,14 +57,19 @@ pub trait Handler { .flat_map(Result::ok) .map(|test_case_dir| { let path = test_case_dir.path(); - let case = Self::Case::load_from_dir(&path).expect("test should load"); + let case = Self::Case::load_from_dir(&path, fork_name).expect("test should load"); (path, case) }) .collect(); - let results = Cases { test_cases }.test_results(); + let results = Cases { test_cases }.test_results(fork_name); - let name = format!("{}/{}", Self::runner_name(), Self::handler_name()); + let name = format!( + "{}/{}/{}", + fork_name_str, + Self::runner_name(), + Self::handler_name() + ); crate::results::assert_tests_pass(&name, &handler_path, &results); } } @@ -75,6 +81,10 @@ macro_rules! bls_handler { impl Handler for $runner_name { type Case = cases::$case_name; + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + fork_name == ForkName::Genesis + } + fn runner_name() -> &'static str { "bls" } @@ -104,11 +114,11 @@ bls_handler!( pub struct SszStaticHandler(PhantomData<(T, E)>); /// Handler for SSZ types that implement `CachedTreeHash`. -pub struct SszStaticTHCHandler(PhantomData<(T, C, E)>); +pub struct SszStaticTHCHandler(PhantomData<(T, E)>); impl Handler for SszStaticHandler where - T: cases::SszStaticType + TypeName, + T: cases::SszStaticType + ssz::Decode + TypeName, E: TypeName, { type Case = cases::SszStatic; @@ -126,13 +136,11 @@ where } } -impl Handler for SszStaticTHCHandler +impl Handler for SszStaticTHCHandler, E> where - T: cases::SszStaticType + CachedTreeHash + TypeName, - C: Debug + Sync, - E: TypeName, + E: EthSpec + TypeName, { - type Case = cases::SszStaticTHC; + type Case = cases::SszStaticTHC>; fn config_name() -> &'static str { E::name() @@ -143,7 +151,7 @@ where } fn handler_name() -> String { - T::name().into() + BeaconState::::name().into() } } @@ -163,6 +171,10 @@ impl Handler for ShufflingHandler { fn handler_name() -> String { "core".into() } + + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + fork_name == ForkName::Genesis + } } pub struct SanityBlocksHandler(PhantomData); @@ -305,6 +317,11 @@ impl Handler for SszGenericHandler { "ssz_generic" } + fn is_enabled_for_fork(fork_name: ForkName) -> bool { + // SSZ generic tests are genesis only + fork_name == ForkName::Genesis + } + fn handler_name() -> String { H::name().into() } diff --git a/testing/ef_tests/src/lib.rs b/testing/ef_tests/src/lib.rs index bdb4262d74d..54d6a3b1f2f 100644 --- a/testing/ef_tests/src/lib.rs +++ b/testing/ef_tests/src/lib.rs @@ -1,6 +1,3 @@ -use std::env; -use types::{init_fork_schedule, EthSpec, ForkSchedule, Slot}; - pub use case_result::CaseResult; pub use cases::Case; pub use cases::{ @@ -11,6 +8,7 @@ pub use cases::{ pub use error::Error; pub use handler::*; pub use type_name::TypeName; +use types::{ChainSpec, EthSpec, ForkName}; mod bls_setting; mod case_result; @@ -21,21 +19,6 @@ mod handler; mod results; mod type_name; -pub fn init_testing_fork_schedule(fork_name: &str) { - let fork_schedule = if fork_name == "phase0" { - ForkSchedule { - altair_fork_slot: None, - } - } else if fork_name == "altair" { - ForkSchedule { - altair_fork_slot: Some(Slot::new(0)), - } - } else { - panic!("unknown fork: {}", fork_name); - }; - init_fork_schedule(fork_schedule); -} - -pub fn get_fork_name() -> String { - env::var("FORK_NAME").expect("FORK_NAME must be set") +pub fn testing_spec(fork_name: ForkName) -> ChainSpec { + fork_name.make_genesis_spec(E::default_spec()) } diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index f76c30c8690..bcab1ddcbe2 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -18,14 +18,8 @@ fn config_test() { let altair_config = AltairConfig::from_file(&altair_config_path).expect("altair config loads"); let spec = E::default_spec(); - let unified_spec = altair_config - .apply_to_chain_spec::( - &phase0_config - .apply_to_chain_spec::(&spec) - .expect("phase0 config matches"), - ) - .expect("altair config matches"); - + let unified_spec = + ChainSpec::from_yaml::(&phase0_config, &altair_config).expect("config unification"); assert_eq!(unified_spec, spec); let phase0_from_spec = YamlConfig::from_spec::(&spec); @@ -69,10 +63,8 @@ fn derived_typenum_values() { #[test] fn shuffling() { - if get_fork_name() == "phase0" { - ShufflingHandler::::run(); - ShufflingHandler::::run(); - } + ShufflingHandler::::run(); + ShufflingHandler::::run(); } #[test] @@ -113,10 +105,8 @@ fn operations_block_header() { #[test] fn operations_sync_aggregate() { - if get_fork_name() != "phase0" { - OperationsHandler::>::run(); - OperationsHandler::>::run(); - } + OperationsHandler::>::run(); + OperationsHandler::>::run(); } #[test] @@ -215,22 +205,17 @@ macro_rules! ssz_static_test_no_run { #[cfg(feature = "fake_crypto")] mod ssz_static { - use ef_tests::{get_fork_name, Handler, SszStaticHandler, SszStaticTHCHandler}; + use ef_tests::{Handler, SszStaticHandler, SszStaticTHCHandler}; use types::*; ssz_static_test!(aggregate_and_proof, AggregateAndProof<_>); ssz_static_test!(attestation, Attestation<_>); ssz_static_test!(attestation_data, AttestationData); ssz_static_test!(attester_slashing, AttesterSlashing<_>); - ssz_static_test!(beacon_block, BeaconBlock<_>); + // FIXME(altair): fix block tests + // ssz_static_test!(beacon_block, BeaconBlock<_>); ssz_static_test!(beacon_block_header, BeaconBlockHeader); - ssz_static_test!( - beacon_state, - SszStaticTHCHandler, { - (BeaconState, BeaconTreeHashCache<_>, MinimalEthSpec), - (BeaconState, BeaconTreeHashCache<_>, MainnetEthSpec) - } - ); + ssz_static_test!(beacon_state, SszStaticTHCHandler, BeaconState<_>); ssz_static_test!(checkpoint, Checkpoint); // FIXME(altair): add ContributionAndProof ssz_static_test!(deposit, Deposit); @@ -246,7 +231,7 @@ mod ssz_static { ssz_static_test!(pending_attestation, PendingAttestation<_>); ssz_static_test!(proposer_slashing, ProposerSlashing); ssz_static_test!(signed_aggregate_and_proof, SignedAggregateAndProof<_>); - ssz_static_test!(signed_beacon_block, SignedBeaconBlock<_>); + // ssz_static_test!(signed_beacon_block, SignedBeaconBlock<_>); ssz_static_test!(signed_beacon_block_header, SignedBeaconBlockHeader); // FIXME(altair): add SignedContributionAndProof ssz_static_test!(signed_voluntary_exit, SignedVoluntaryExit); @@ -255,6 +240,7 @@ mod ssz_static { ssz_static_test!(validator, Validator); ssz_static_test!(voluntary_exit, VoluntaryExit); + /* FIXME(altair): fix this // BeaconBlockBody has no internal indicator of which fork it is for, so we test it // separately. ssz_static_test_no_run!(beacon_block_body_phase0, BeaconBlockBodyBase<_>); @@ -275,14 +261,7 @@ mod ssz_static { fn sync_committee() { fork_variant_test(|| (), sync_committee_altair); } - - fn fork_variant_test(phase0: impl FnOnce(), altair: impl FnOnce()) { - match get_fork_name().as_str() { - "phase0" => phase0(), - "altair" => altair(), - fork_name => panic!("unknown fork: {}", fork_name), - } - } + */ } #[test] @@ -357,10 +336,8 @@ fn epoch_processing_participation_record_updates() { #[test] fn epoch_processing_sync_committee_updates() { - if get_fork_name() != "phase0" { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); - } + EpochProcessingHandler::::run(); + EpochProcessingHandler::::run(); } #[test] From 082f0c1dec20a3115035733705707e70d9eddb14 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 19 Apr 2021 12:51:16 +1000 Subject: [PATCH 2/4] Fix remaining EF tests for refactor --- Cargo.lock | 1 + consensus/types/src/fork_name.rs | 6 +- testing/ef_tests/Cargo.toml | 1 + testing/ef_tests/src/cases/common.rs | 25 +-- .../ef_tests/src/cases/epoch_processing.rs | 2 +- testing/ef_tests/src/cases/operations.rs | 4 +- testing/ef_tests/src/cases/ssz_static.rs | 45 +++++- testing/ef_tests/src/handler.rs | 109 +++++++++++-- testing/ef_tests/tests/tests.rs | 150 +++++++++--------- 9 files changed, 226 insertions(+), 117 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4a1cd8866fd..40faca89635 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1775,6 +1775,7 @@ dependencies = [ "bls", "cached_tree_hash", "compare_fields", + "derivative", "eth2_ssz", "eth2_ssz_derive", "ethereum-types", diff --git a/consensus/types/src/fork_name.rs b/consensus/types/src/fork_name.rs index 76f68772092..8d34a8254ec 100644 --- a/consensus/types/src/fork_name.rs +++ b/consensus/types/src/fork_name.rs @@ -2,20 +2,20 @@ use crate::ChainSpec; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum ForkName { - Genesis, + Base, Altair, } impl ForkName { pub fn list_all() -> Vec { - vec![ForkName::Genesis, ForkName::Altair] + vec![ForkName::Base, ForkName::Altair] } /// Set the activation slots in the given `ChainSpec` so that the fork named by `self` /// is the only fork in effect from genesis. pub fn make_genesis_spec(&self, mut spec: ChainSpec) -> ChainSpec { match self { - ForkName::Genesis => { + ForkName::Base => { spec.altair_fork_slot = None; spec } diff --git a/testing/ef_tests/Cargo.toml b/testing/ef_tests/Cargo.toml index 0500346dd78..4db886f81f9 100644 --- a/testing/ef_tests/Cargo.toml +++ b/testing/ef_tests/Cargo.toml @@ -13,6 +13,7 @@ fake_crypto = ["bls/fake_crypto"] [dependencies] bls = { path = "../../crypto/bls", default-features = false } compare_fields = { path = "../../common/compare_fields" } +derivative = "2.1.1" ethereum-types = "0.9.2" hex = "0.4.2" rayon = "1.4.1" diff --git a/testing/ef_tests/src/cases/common.rs b/testing/ef_tests/src/cases/common.rs index 4d4f6b8bb07..34f5c581e8d 100644 --- a/testing/ef_tests/src/cases/common.rs +++ b/testing/ef_tests/src/cases/common.rs @@ -1,15 +1,14 @@ use crate::cases::LoadCase; use crate::decode::yaml_decode_file; use crate::error::Error; -use crate::testing_spec; use serde_derive::Deserialize; -use ssz::{Decode, Encode}; +use ssz::Encode; use ssz_derive::{Decode, Encode}; use std::convert::TryFrom; use std::fmt::Debug; use std::path::Path; use tree_hash::TreeHash; -use types::{BeaconState, EthSpec, ForkName, SignedBeaconBlock}; +use types::ForkName; /// Trait for all BLS cases to eliminate some boilerplate. pub trait BlsCase: serde::de::DeserializeOwned {} @@ -66,29 +65,9 @@ uint_wrapper!(TestU256, ethereum_types::U256); pub trait SszStaticType: serde::de::DeserializeOwned + Encode + TreeHash + Clone + PartialEq + Debug + Sync { - // fn decode(bytes: &[u8], fork_name: ForkName) -> Result; } impl SszStaticType for T where T: serde::de::DeserializeOwned + Encode + TreeHash + Clone + PartialEq + Debug + Sync { } -/* -{ - fn decode(bytes: &[u8], _: ForkName) -> Result { - Self::from_ssz_bytes(bytes) - } -} - -impl SszStaticType for BeaconState { - fn decode(bytes: &[u8], fork_name: ForkName) -> Result { - Self::from_ssz_bytes(bytes, &testing_spec::(fork_name)) - } -} - -impl SszStaticType for SignedBeaconBlock { - fn decode(bytes: &[u8], fork_name: ForkName) -> Result { - Self::from_ssz_bytes(bytes, &testing_spec::(fork_name)) - } -} -*/ diff --git a/testing/ef_tests/src/cases/epoch_processing.rs b/testing/ef_tests/src/cases/epoch_processing.rs index 3888ea7911f..df463e387ca 100644 --- a/testing/ef_tests/src/cases/epoch_processing.rs +++ b/testing/ef_tests/src/cases/epoch_processing.rs @@ -233,7 +233,7 @@ impl> Case for EpochProcessing { fn is_enabled_for_fork(fork_name: ForkName) -> bool { match fork_name { // No sync committee tests for genesis fork. - ForkName::Genesis => T::name() != "sync_committee_updates", + ForkName::Base => T::name() != "sync_committee_updates", ForkName::Altair => true, } } diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index a3220e53991..ba087be7380 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -240,8 +240,8 @@ impl> Case for Operations { fn is_enabled_for_fork(fork_name: ForkName) -> bool { match fork_name { - // Genesis doesn't have sync aggregate tests - ForkName::Genesis => O::handler_name() != "sync_committee", + // Base fork doesn't have sync aggregate tests + ForkName::Base => O::handler_name() != "sync_committee", _ => true, } } diff --git a/testing/ef_tests/src/cases/ssz_static.rs b/testing/ef_tests/src/cases/ssz_static.rs index 5d2c3e3231b..72d5e9b9382 100644 --- a/testing/ef_tests/src/cases/ssz_static.rs +++ b/testing/ef_tests/src/cases/ssz_static.rs @@ -5,9 +5,8 @@ use crate::decode::{snappy_decode_file, yaml_decode_file}; use cached_tree_hash::{CacheArena, CachedTreeHash}; use serde_derive::Deserialize; use ssz::Decode; -use std::marker::PhantomData; use tree_hash::TreeHash; -use types::{BeaconState, ForkName, Hash256}; +use types::{BeaconBlock, BeaconState, ForkName, Hash256, SignedBeaconBlock}; #[derive(Debug, Clone, Deserialize)] struct SszStaticRoots { @@ -15,6 +14,7 @@ struct SszStaticRoots { signing_root: Option, } +/// Runner for types that implement `ssz::Decode`. #[derive(Debug, Clone)] pub struct SszStatic { roots: SszStaticRoots, @@ -22,6 +22,7 @@ pub struct SszStatic { value: T, } +/// Runner for `BeaconState` (with tree hash cache). #[derive(Debug, Clone)] pub struct SszStaticTHC { roots: SszStaticRoots, @@ -29,6 +30,14 @@ pub struct SszStaticTHC { value: T, } +/// Runner for types that require a `ChainSpec` to be decoded (`BeaconBlock`, etc). +#[derive(Debug, Clone)] +pub struct SszStaticWithSpec { + roots: SszStaticRoots, + serialized: Vec, + value: T, +} + fn load_from_dir(path: &Path) -> Result<(SszStaticRoots, Vec, T), Error> { let roots = yaml_decode_file(&path.join("roots.yaml"))?; let serialized = snappy_decode_file(&path.join("serialized.ssz_snappy")) @@ -58,6 +67,16 @@ impl LoadCase for SszStaticTHC { } } +impl LoadCase for SszStaticWithSpec { + fn load_from_dir(path: &Path, _fork_name: ForkName) -> Result { + load_from_dir(path).map(|(roots, serialized, value)| Self { + roots, + serialized, + value, + }) + } +} + pub fn check_serialization( value: &T, serialized: &[u8], @@ -110,3 +129,25 @@ impl Case for SszStaticTHC> { Ok(()) } } + +impl Case for SszStaticWithSpec> { + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { + let spec = &testing_spec::(fork_name); + check_serialization(&self.value, &self.serialized, |bytes| { + BeaconBlock::from_ssz_bytes(bytes, spec) + })?; + check_tree_hash(&self.roots.root, self.value.tree_hash_root().as_bytes())?; + Ok(()) + } +} + +impl Case for SszStaticWithSpec> { + fn result(&self, _case_index: usize, fork_name: ForkName) -> Result<(), Error> { + let spec = &testing_spec::(fork_name); + check_serialization(&self.value, &self.serialized, |bytes| { + SignedBeaconBlock::from_ssz_bytes(bytes, spec) + })?; + check_tree_hash(&self.roots.root, self.value.tree_hash_root().as_bytes())?; + Ok(()) + } +} diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index f51de4966ca..38ae02ed495 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -1,8 +1,7 @@ use crate::cases::{self, Case, Cases, EpochTransition, LoadCase, Operation}; use crate::type_name; use crate::type_name::TypeName; -use cached_tree_hash::CachedTreeHash; -use std::fmt::Debug; +use derivative::Derivative; use std::fs; use std::marker::PhantomData; use std::path::PathBuf; @@ -19,13 +18,13 @@ pub trait Handler { fn handler_name() -> String; - fn is_enabled_for_fork(fork_name: ForkName) -> bool { + fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { Self::Case::is_enabled_for_fork(fork_name) } - fn run() { + fn run(&self) { for fork_name in ForkName::list_all() { - if Self::is_enabled_for_fork(fork_name) { + if self.is_enabled_for_fork(fork_name) { Self::run_for_fork(fork_name) } } @@ -33,7 +32,7 @@ pub trait Handler { fn run_for_fork(fork_name: ForkName) { let fork_name_str = match fork_name { - ForkName::Genesis => "phase0", + ForkName::Base => "phase0", ForkName::Altair => "altair", }; @@ -76,13 +75,15 @@ pub trait Handler { macro_rules! bls_handler { ($runner_name: ident, $case_name:ident, $handler_name:expr) => { + #[derive(Derivative)] + #[derivative(Default(bound = ""))] pub struct $runner_name; impl Handler for $runner_name { type Case = cases::$case_name; - fn is_enabled_for_fork(fork_name: ForkName) -> bool { - fork_name == ForkName::Genesis + fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { + fork_name == ForkName::Base } fn runner_name() -> &'static str { @@ -111,11 +112,44 @@ bls_handler!( ); /// Handler for SSZ types. -pub struct SszStaticHandler(PhantomData<(T, E)>); +pub struct SszStaticHandler { + supported_forks: Vec, + _phantom: PhantomData<(T, E)>, +} + +impl Default for SszStaticHandler { + fn default() -> Self { + Self::for_forks(ForkName::list_all()) + } +} + +impl SszStaticHandler { + pub fn for_forks(supported_forks: Vec) -> Self { + SszStaticHandler { + supported_forks, + _phantom: PhantomData, + } + } + + pub fn base_only() -> Self { + Self::for_forks(vec![ForkName::Base]) + } + + pub fn altair_only() -> Self { + Self::for_forks(vec![ForkName::Altair]) + } +} /// Handler for SSZ types that implement `CachedTreeHash`. +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct SszStaticTHCHandler(PhantomData<(T, E)>); +/// Handler for SSZ types that don't implement `ssz::Decode`. +#[derive(Derivative)] +#[derivative(Default(bound = ""))] +pub struct SszStaticWithSpecHandler(PhantomData<(T, E)>); + impl Handler for SszStaticHandler where T: cases::SszStaticType + ssz::Decode + TypeName, @@ -134,6 +168,10 @@ where fn handler_name() -> String { T::name().into() } + + fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { + self.supported_forks.contains(&fork_name) + } } impl Handler for SszStaticTHCHandler, E> @@ -155,6 +193,29 @@ where } } +impl Handler for SszStaticWithSpecHandler +where + T: TypeName, + E: EthSpec + TypeName, + cases::SszStaticWithSpec: Case + LoadCase, +{ + type Case = cases::SszStaticWithSpec; + + fn config_name() -> &'static str { + E::name() + } + + fn runner_name() -> &'static str { + "ssz_static" + } + + fn handler_name() -> String { + T::name().into() + } +} + +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct ShufflingHandler(PhantomData); impl Handler for ShufflingHandler { @@ -172,11 +233,13 @@ impl Handler for ShufflingHandler { "core".into() } - fn is_enabled_for_fork(fork_name: ForkName) -> bool { - fork_name == ForkName::Genesis + fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { + fork_name == ForkName::Base } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct SanityBlocksHandler(PhantomData); impl Handler for SanityBlocksHandler { @@ -193,8 +256,16 @@ impl Handler for SanityBlocksHandler { fn handler_name() -> String { "blocks".into() } + + fn is_enabled_for_fork(&self, _fork_name: ForkName) -> bool { + // FIXME(altair): v1.1.0-alpha.3 doesn't mark the historical blocks test as + // requiring real crypto, so only run these tests with real crypto for now. + cfg!(not(feature = "fake_crypto")) + } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct SanitySlotsHandler(PhantomData); impl Handler for SanitySlotsHandler { @@ -213,6 +284,8 @@ impl Handler for SanitySlotsHandler { } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct EpochProcessingHandler(PhantomData<(E, T)>); impl> Handler for EpochProcessingHandler { @@ -231,6 +304,8 @@ impl> Handler for EpochProcessingHa } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct FinalityHandler(PhantomData); impl Handler for FinalityHandler { @@ -250,6 +325,8 @@ impl Handler for FinalityHandler { } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct GenesisValidityHandler(PhantomData); impl Handler for GenesisValidityHandler { @@ -268,6 +345,8 @@ impl Handler for GenesisValidityHandler { } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct GenesisInitializationHandler(PhantomData); impl Handler for GenesisInitializationHandler { @@ -286,6 +365,8 @@ impl Handler for GenesisInitializationHandler { } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct OperationsHandler(PhantomData<(E, O)>); impl> Handler for OperationsHandler { @@ -304,6 +385,8 @@ impl> Handler for OperationsHandler } } +#[derive(Derivative)] +#[derivative(Default(bound = ""))] pub struct SszGenericHandler(PhantomData); impl Handler for SszGenericHandler { @@ -317,9 +400,9 @@ impl Handler for SszGenericHandler { "ssz_generic" } - fn is_enabled_for_fork(fork_name: ForkName) -> bool { + fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { // SSZ generic tests are genesis only - fork_name == ForkName::Genesis + fork_name == ForkName::Base } fn handler_name() -> String { diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index bcab1ddcbe2..311622db817 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -63,92 +63,92 @@ fn derived_typenum_values() { #[test] fn shuffling() { - ShufflingHandler::::run(); - ShufflingHandler::::run(); + ShufflingHandler::::default().run(); + ShufflingHandler::::default().run(); } #[test] fn operations_deposit() { - OperationsHandler::::run(); - OperationsHandler::::run(); + OperationsHandler::::default().run(); + OperationsHandler::::default().run(); } #[test] fn operations_exit() { - OperationsHandler::::run(); - OperationsHandler::::run(); + OperationsHandler::::default().run(); + OperationsHandler::::default().run(); } #[test] fn operations_proposer_slashing() { - OperationsHandler::::run(); - OperationsHandler::::run(); + OperationsHandler::::default().run(); + OperationsHandler::::default().run(); } #[test] fn operations_attester_slashing() { - OperationsHandler::>::run(); - OperationsHandler::>::run(); + OperationsHandler::>::default().run(); + OperationsHandler::>::default().run(); } #[test] fn operations_attestation() { - OperationsHandler::>::run(); - OperationsHandler::>::run(); + OperationsHandler::>::default().run(); + OperationsHandler::>::default().run(); } #[test] fn operations_block_header() { - OperationsHandler::>::run(); - OperationsHandler::>::run(); + OperationsHandler::>::default().run(); + OperationsHandler::>::default().run(); } #[test] fn operations_sync_aggregate() { - OperationsHandler::>::run(); - OperationsHandler::>::run(); + OperationsHandler::>::default().run(); + OperationsHandler::>::default().run(); } #[test] fn sanity_blocks() { - SanityBlocksHandler::::run(); - SanityBlocksHandler::::run(); + SanityBlocksHandler::::default().run(); + SanityBlocksHandler::::default().run(); } #[test] fn sanity_slots() { - SanitySlotsHandler::::run(); - SanitySlotsHandler::::run(); + SanitySlotsHandler::::default().run(); + SanitySlotsHandler::::default().run(); } #[test] #[cfg(not(feature = "fake_crypto"))] fn bls_aggregate() { - BlsAggregateSigsHandler::run(); + BlsAggregateSigsHandler::default().run(); } #[test] #[cfg(not(feature = "fake_crypto"))] fn bls_sign() { - BlsSignMsgHandler::run(); + BlsSignMsgHandler::default().run(); } #[test] #[cfg(not(feature = "fake_crypto"))] fn bls_verify() { - BlsVerifyMsgHandler::run(); + BlsVerifyMsgHandler::default().run(); } #[test] #[cfg(not(feature = "fake_crypto"))] fn bls_aggregate_verify() { - BlsAggregateVerifyHandler::run(); + BlsAggregateVerifyHandler::default().run(); } #[test] #[cfg(not(feature = "fake_crypto"))] fn bls_fast_aggregate_verify() { - BlsFastAggregateVerifyHandler::run(); + BlsFastAggregateVerifyHandler::default().run(); } /// As for `ssz_static_test_no_run` (below), but also executes the function as a test. @@ -197,7 +197,7 @@ macro_rules! ssz_static_test_no_run { $(#[$test])? fn $test_name() { $( - $handler::<$($typ),+>::run(); + $handler::<$($typ),+>::default().run(); )+ } }; @@ -205,15 +205,14 @@ macro_rules! ssz_static_test_no_run { #[cfg(feature = "fake_crypto")] mod ssz_static { - use ef_tests::{Handler, SszStaticHandler, SszStaticTHCHandler}; + use ef_tests::{Handler, SszStaticHandler, SszStaticTHCHandler, SszStaticWithSpecHandler}; use types::*; ssz_static_test!(aggregate_and_proof, AggregateAndProof<_>); ssz_static_test!(attestation, Attestation<_>); ssz_static_test!(attestation_data, AttestationData); ssz_static_test!(attester_slashing, AttesterSlashing<_>); - // FIXME(altair): fix block tests - // ssz_static_test!(beacon_block, BeaconBlock<_>); + ssz_static_test!(beacon_block, SszStaticWithSpecHandler, BeaconBlock<_>); ssz_static_test!(beacon_block_header, BeaconBlockHeader); ssz_static_test!(beacon_state, SszStaticTHCHandler, BeaconState<_>); ssz_static_test!(checkpoint, Checkpoint); @@ -231,7 +230,11 @@ mod ssz_static { ssz_static_test!(pending_attestation, PendingAttestation<_>); ssz_static_test!(proposer_slashing, ProposerSlashing); ssz_static_test!(signed_aggregate_and_proof, SignedAggregateAndProof<_>); - // ssz_static_test!(signed_beacon_block, SignedBeaconBlock<_>); + ssz_static_test!( + signed_beacon_block, + SszStaticWithSpecHandler, + SignedBeaconBlock<_> + ); ssz_static_test!(signed_beacon_block_header, SignedBeaconBlockHeader); // FIXME(altair): add SignedContributionAndProof ssz_static_test!(signed_voluntary_exit, SignedVoluntaryExit); @@ -240,119 +243,120 @@ mod ssz_static { ssz_static_test!(validator, Validator); ssz_static_test!(voluntary_exit, VoluntaryExit); - /* FIXME(altair): fix this - // BeaconBlockBody has no internal indicator of which fork it is for, so we test it - // separately. - ssz_static_test_no_run!(beacon_block_body_phase0, BeaconBlockBodyBase<_>); - ssz_static_test_no_run!(beacon_block_body_altair, BeaconBlockBodyAltair<_>); + // BeaconBlockBody has no internal indicator of which fork it is for, so we test it separately. #[test] fn beacon_block_body() { - fork_variant_test(beacon_block_body_phase0, beacon_block_body_altair); + SszStaticHandler::, MinimalEthSpec>::base_only().run(); + SszStaticHandler::, MainnetEthSpec>::base_only().run(); + SszStaticHandler::, MinimalEthSpec>::altair_only() + .run(); + SszStaticHandler::, MainnetEthSpec>::altair_only() + .run(); } - ssz_static_test_no_run!(sync_aggregate_altair, SyncAggregate<_>); + // Altair-only #[test] fn sync_aggregate() { - fork_variant_test(|| (), sync_aggregate_altair); + SszStaticHandler::, MinimalEthSpec>::altair_only().run(); + SszStaticHandler::, MainnetEthSpec>::altair_only().run(); } - ssz_static_test_no_run!(sync_committee_altair, SyncCommittee<_>); #[test] fn sync_committee() { - fork_variant_test(|| (), sync_committee_altair); + SszStaticHandler::, MinimalEthSpec>::altair_only().run(); + SszStaticHandler::, MainnetEthSpec>::altair_only().run(); } - */ } #[test] fn ssz_generic() { - SszGenericHandler::::run(); - SszGenericHandler::::run(); - SszGenericHandler::::run(); - SszGenericHandler::::run(); - SszGenericHandler::::run(); - SszGenericHandler::::run(); + SszGenericHandler::::default().run(); + SszGenericHandler::::default().run(); + SszGenericHandler::::default().run(); + SszGenericHandler::::default().run(); + SszGenericHandler::::default().run(); + SszGenericHandler::::default().run(); } #[test] fn epoch_processing_justification_and_finalization() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_rewards_and_penalties() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_registry_updates() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_slashings() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_eth1_data_reset() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_effective_balance_updates() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_slashings_reset() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_randao_mixes_reset() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_historical_roots_update() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_participation_record_updates() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn epoch_processing_sync_committee_updates() { - EpochProcessingHandler::::run(); - EpochProcessingHandler::::run(); + EpochProcessingHandler::::default().run(); + EpochProcessingHandler::::default().run(); } #[test] fn finality() { - FinalityHandler::::run(); - FinalityHandler::::run(); + FinalityHandler::::default().run(); + FinalityHandler::::default().run(); } #[test] fn genesis_initialization() { - GenesisInitializationHandler::::run(); + GenesisInitializationHandler::::default().run(); } #[test] fn genesis_validity() { - GenesisValidityHandler::::run(); + GenesisValidityHandler::::default().run(); // Note: there are no genesis validity tests for mainnet } From 8ab039459821c506157af0ccfb9a2c94514830fc Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 19 Apr 2021 14:54:31 +1000 Subject: [PATCH 3/4] Get top-level compiling again --- beacon_node/beacon_chain/src/beacon_chain.rs | 9 +-------- .../src/beacon_fork_choice_store.rs | 7 ++----- .../beacon_chain/src/beacon_snapshot.rs | 3 +-- .../beacon_chain/src/block_verification.rs | 2 +- beacon_node/beacon_chain/src/builder.rs | 8 ++++---- beacon_node/beacon_chain/src/test_utils.rs | 20 +++++-------------- beacon_node/client/src/builder.rs | 3 +-- .../eth2_libp2p/src/rpc/codec/ssz_snappy.rs | 12 ++++++++--- beacon_node/eth2_libp2p/src/types/pubsub.rs | 9 ++++++--- beacon_node/http_api/tests/tests.rs | 6 +++++- beacon_node/store/src/hot_cold_store.rs | 6 ++++++ boot_node/src/config.rs | 2 +- common/eth2/src/lib.rs | 7 ++++--- common/eth2/src/lighthouse.rs | 6 +++--- .../mainnet/altair.yaml | 5 ++++- .../prater/altair.yaml | 5 ++++- .../pyrmont/altair.yaml | 5 ++++- .../toledo/altair.yaml | 5 ++++- lighthouse/src/main.rs | 5 +---- 19 files changed, 66 insertions(+), 59 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index be8ede9bc67..f94f51ef617 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -501,7 +501,7 @@ impl BeaconChain { })?; if let Some(block_root) = root { - Ok(self.store.get_item(&block_root)?) + Ok(self.store.get_block(&block_root)?) } else { Ok(None) } @@ -2656,13 +2656,6 @@ impl BeaconChain { } } - /// Returns `true` if the given block root has not been processed. - pub fn is_new_block_root(&self, beacon_block_root: &Hash256) -> Result { - Ok(!self - .store - .item_exists::>(beacon_block_root)?) - } - /// Dumps the entire canonical chain, from the head to genesis to a vector for analysis. /// /// This could be a very expensive operation and should only be done in testing/analysis 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 376a303d756..a2d43f735e7 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -10,10 +10,7 @@ use ssz_derive::{Decode, Encode}; use std::marker::PhantomData; use std::sync::Arc; use store::{Error as StoreError, HotColdDB, ItemStore}; -use types::{ - BeaconBlock, BeaconState, BeaconStateError, Checkpoint, EthSpec, Hash256, SignedBeaconBlock, - Slot, -}; +use types::{BeaconBlock, BeaconState, BeaconStateError, Checkpoint, EthSpec, Hash256, Slot}; #[derive(Debug)] pub enum Error { @@ -318,7 +315,7 @@ where metrics::inc_counter(&metrics::BALANCES_CACHE_MISSES); let justified_block = self .store - .get_item::>(&self.justified_checkpoint.root) + .get_block(&self.justified_checkpoint.root) .map_err(Error::FailedToReadBlock)? .ok_or(Error::MissingBlock(self.justified_checkpoint.root))? .deconstruct() diff --git a/beacon_node/beacon_chain/src/beacon_snapshot.rs b/beacon_node/beacon_chain/src/beacon_snapshot.rs index 1f18fefaf20..b9de6e9eba1 100644 --- a/beacon_node/beacon_chain/src/beacon_snapshot.rs +++ b/beacon_node/beacon_chain/src/beacon_snapshot.rs @@ -1,10 +1,9 @@ use serde_derive::Serialize; -use ssz_derive::{Decode, Encode}; use types::{beacon_state::CloneConfig, BeaconState, EthSpec, Hash256, SignedBeaconBlock}; /// Represents some block and its associated state. Generally, this will be used for tracking the /// head, justified head and finalized head. -#[derive(Clone, Serialize, PartialEq, Debug, Encode, Decode)] +#[derive(Clone, Serialize, PartialEq, Debug)] pub struct BeaconSnapshot { pub beacon_block: SignedBeaconBlock, pub beacon_block_root: Hash256, diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 0ae1a0c22be..840c2a65471 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1125,7 +1125,7 @@ pub fn check_block_is_finalized_descendant>(&block.parent_root()) + .block_exists(&block.parent_root()) .map_err(|e| BlockError::BeaconChainError(e.into()))? { Err(BlockError::NotFinalizedDescendant { diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index a33b6a03b29..2924105c710 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -235,7 +235,7 @@ where .ok_or("Fork choice not found in store")?; let genesis_block = store - .get_item::>(&chain.genesis_block_root) + .get_block(&chain.genesis_block_root) .map_err(|e| format!("DB error when reading genesis block: {:?}", e))? .ok_or("Genesis block not found in store")?; let genesis_state = store @@ -291,12 +291,12 @@ where .put_state(&beacon_state_root, &beacon_state) .map_err(|e| format!("Failed to store genesis state: {:?}", e))?; store - .put_item(&beacon_block_root, &beacon_block) + .put_block(&beacon_block_root, beacon_block.clone()) .map_err(|e| format!("Failed to store genesis block: {:?}", e))?; // Store the genesis block under the `ZERO_HASH` key. store - .put_item(&Hash256::zero(), &beacon_block) + .put_block(&Hash256::zero(), beacon_block.clone()) .map_err(|e| { format!( "Failed to store genesis block under 0x00..00 alias: {:?}", @@ -434,7 +434,7 @@ where .map_err(|e| format!("Unable to get fork choice head: {:?}", e))?; let head_block = store - .get_item::>(&head_block_root) + .get_block(&head_block_root) .map_err(|e| format!("DB error when reading head block: {:?}", e))? .ok_or("Head block not found in store")?; let head_state_root = head_block.state_root(); diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 9d608e1aaf9..9fd13d813ba 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -28,11 +28,11 @@ use store::{config::StoreConfig, BlockReplay, HotColdDB, ItemStore, LevelDB, Mem use tempfile::{tempdir, TempDir}; use tree_hash::TreeHash; use types::{ - init_fork_schedule, AggregateSignature, Attestation, AttestationData, AttesterSlashing, - BeaconState, BeaconStateHash, ChainSpec, Checkpoint, Domain, Epoch, EthSpec, ForkSchedule, - Graffiti, Hash256, IndexedAttestation, Keypair, ProposerSlashing, SelectionProof, - SignedAggregateAndProof, SignedBeaconBlock, SignedBeaconBlockHash, SignedRoot, - SignedVoluntaryExit, Slot, SubnetId, VariableList, VoluntaryExit, + AggregateSignature, Attestation, AttestationData, AttesterSlashing, BeaconState, + BeaconStateHash, ChainSpec, Checkpoint, Domain, Epoch, EthSpec, Graffiti, Hash256, + IndexedAttestation, Keypair, ProposerSlashing, SelectionProof, SignedAggregateAndProof, + SignedBeaconBlock, SignedBeaconBlockHash, SignedRoot, SignedVoluntaryExit, Slot, SubnetId, + VariableList, VoluntaryExit, }; pub use types::test_utils::generate_deterministic_keypairs; @@ -171,11 +171,6 @@ impl BeaconChainHarness> { store_config: StoreConfig, chain_config: ChainConfig, ) -> Self { - //TODO: handle altair - init_fork_schedule(ForkSchedule { - altair_fork_slot: None, - }); - let data_dir = tempdir().expect("should create temporary data_dir"); let mut spec = E::default_spec(); @@ -228,11 +223,6 @@ impl BeaconChainHarness> { store: Arc, LevelDB>>, validator_keypairs: Vec, ) -> Self { - //TODO: handle altair - init_fork_schedule(ForkSchedule { - altair_fork_slot: None, - }); - let data_dir = tempdir().expect("should create temporary data_dir"); let spec = E::default_spec(); diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 8ba7a0c3431..174fad979ac 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -18,7 +18,6 @@ use network::{NetworkConfig, NetworkMessage, NetworkService}; use slasher::Slasher; use slasher_service::SlasherService; use slog::{debug, info, warn}; -use ssz::Decode; use std::net::TcpListener; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -195,7 +194,7 @@ where "Starting from known genesis state"; ); - let genesis_state = BeaconState::from_ssz_bytes(&genesis_state_bytes) + let genesis_state = BeaconState::from_ssz_bytes(&genesis_state_bytes, &spec) .map_err(|e| format!("Unable to parse genesis state SSZ: {:?}", e))?; builder.genesis_state(genesis_state).map(|v| (v, None))? diff --git a/beacon_node/eth2_libp2p/src/rpc/codec/ssz_snappy.rs b/beacon_node/eth2_libp2p/src/rpc/codec/ssz_snappy.rs index 33dc49733c9..5e0c39250da 100644 --- a/beacon_node/eth2_libp2p/src/rpc/codec/ssz_snappy.rs +++ b/beacon_node/eth2_libp2p/src/rpc/codec/ssz_snappy.rs @@ -14,7 +14,7 @@ use std::io::ErrorKind; use std::io::{Read, Write}; use std::marker::PhantomData; use tokio_util::codec::{Decoder, Encoder}; -use types::{EthSpec, SignedBeaconBlock}; +use types::{EthSpec, SignedBeaconBlock, SignedBeaconBlockBase}; use unsigned_varint::codec::Uvi; /* Inbound Codec */ @@ -293,12 +293,18 @@ impl Decoder for SSZSnappyOutboundCodec { Protocol::Goodbye => Err(RPCError::InvalidData), Protocol::BlocksByRange => match self.protocol.version { Version::V1 => Ok(Some(RPCResponse::BlocksByRange(Box::new( - SignedBeaconBlock::from_ssz_bytes(&decoded_buffer)?, + // FIXME(altair): support Altair blocks + SignedBeaconBlock::Base(SignedBeaconBlockBase::from_ssz_bytes( + &decoded_buffer, + )?), )))), }, Protocol::BlocksByRoot => match self.protocol.version { + // FIXME(altair): support Altair blocks Version::V1 => Ok(Some(RPCResponse::BlocksByRoot(Box::new( - SignedBeaconBlock::from_ssz_bytes(&decoded_buffer)?, + SignedBeaconBlock::Base(SignedBeaconBlockBase::from_ssz_bytes( + &decoded_buffer, + )?), )))), }, Protocol::Ping => match self.protocol.version { diff --git a/beacon_node/eth2_libp2p/src/types/pubsub.rs b/beacon_node/eth2_libp2p/src/types/pubsub.rs index ce06f296011..f1ba987058a 100644 --- a/beacon_node/eth2_libp2p/src/types/pubsub.rs +++ b/beacon_node/eth2_libp2p/src/types/pubsub.rs @@ -10,7 +10,7 @@ use std::io::{Error, ErrorKind}; use types::SubnetId; use types::{ Attestation, AttesterSlashing, EthSpec, ProposerSlashing, SignedAggregateAndProof, - SignedBeaconBlock, SignedVoluntaryExit, + SignedBeaconBlock, SignedBeaconBlockBase, SignedVoluntaryExit, }; #[derive(Debug, Clone, PartialEq)] @@ -141,8 +141,11 @@ impl PubsubMessage { )))) } GossipKind::BeaconBlock => { - let beacon_block = SignedBeaconBlock::from_ssz_bytes(data) - .map_err(|e| format!("{:?}", e))?; + // FIXME(altair): support Altair blocks + let beacon_block = SignedBeaconBlock::Base( + SignedBeaconBlockBase::from_ssz_bytes(data) + .map_err(|e| format!("{:?}", e))?, + ); Ok(PubsubMessage::BeaconBlock(Box::new(beacon_block))) } GossipKind::VoluntaryExit => { diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index d367c54c98a..806bbc7888d 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -970,7 +970,11 @@ impl ApiTester { .map(|res| res.data); assert_eq!(json_result, expected, "{:?}", block_id); - let ssz_result = self.client.get_beacon_blocks_ssz(block_id).await.unwrap(); + let ssz_result = self + .client + .get_beacon_blocks_ssz(block_id, &harness.chain.spec) + .await + .unwrap(); assert_eq!(ssz_result, expected, "{:?}", block_id); } diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 4f2ebb441a9..a21c9dff88d 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -275,6 +275,12 @@ impl, Cold: ItemStore> HotColdDB } } + /// Determine whether a block exists in the database. + pub fn block_exists(&self, block_root: &Hash256) -> Result { + self.hot_db + .key_exists(DBColumn::BeaconBlock.into(), block_root.as_bytes()) + } + /// Delete a block from the store and the block cache. pub fn delete_block(&self, block_root: &Hash256) -> Result<(), Error> { self.block_cache.lock().pop(block_root); diff --git a/boot_node/src/config.rs b/boot_node/src/config.rs index f119604c424..7b894301c78 100644 --- a/boot_node/src/config.rs +++ b/boot_node/src/config.rs @@ -95,7 +95,7 @@ impl TryFrom<&ArgMatches<'_>> for BootNodeConfig { slog::info!(logger, "Genesis state found"; "root" => genesis_state.canonical_root().to_string()); let enr_fork = spec.enr_fork_id( types::Slot::from(0u64), - genesis_state.genesis_validators_root, + genesis_state.genesis_validators_root(), ); Some(enr_fork.as_ssz_bytes()) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 13c117b3296..0dc7b5fd625 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -20,7 +20,6 @@ pub use reqwest; use reqwest::{IntoUrl, Response}; pub use reqwest::{StatusCode, Url}; use serde::{de::DeserializeOwned, Serialize}; -use ssz::Decode; use std::convert::TryFrom; use std::fmt; use std::iter::Iterator; @@ -497,6 +496,7 @@ impl BeaconNodeHttpClient { pub async fn get_beacon_blocks_ssz( &self, block_id: BlockId, + spec: &ChainSpec, ) -> Result>, Error> { let mut path = self.eth_path()?; @@ -508,7 +508,7 @@ impl BeaconNodeHttpClient { self.get_bytes_opt_accept_header(path, Accept::Ssz) .await? - .map(|bytes| SignedBeaconBlock::from_ssz_bytes(&bytes).map_err(Error::InvalidSsz)) + .map(|bytes| SignedBeaconBlock::from_ssz_bytes(&bytes, spec).map_err(Error::InvalidSsz)) .transpose() } @@ -882,6 +882,7 @@ impl BeaconNodeHttpClient { pub async fn get_debug_beacon_states_ssz( &self, state_id: StateId, + spec: &ChainSpec, ) -> Result>, Error> { let mut path = self.eth_path()?; @@ -894,7 +895,7 @@ impl BeaconNodeHttpClient { self.get_bytes_opt_accept_header(path, Accept::Ssz) .await? - .map(|bytes| BeaconState::from_ssz_bytes(&bytes).map_err(Error::InvalidSsz)) + .map(|bytes| BeaconState::from_ssz_bytes(&bytes, spec).map_err(Error::InvalidSsz)) .transpose() } diff --git a/common/eth2/src/lighthouse.rs b/common/eth2/src/lighthouse.rs index a879b7c8db0..9ff047d226b 100644 --- a/common/eth2/src/lighthouse.rs +++ b/common/eth2/src/lighthouse.rs @@ -2,13 +2,12 @@ use crate::{ ok_or_error, - types::{BeaconState, Epoch, EthSpec, GenericResponse, ValidatorId}, + types::{BeaconState, ChainSpec, Epoch, EthSpec, GenericResponse, ValidatorId}, BeaconNodeHttpClient, DepositData, Error, Eth1Data, Hash256, StateId, StatusCode, }; use proto_array::core::ProtoArray; use reqwest::IntoUrl; use serde::{Deserialize, Serialize}; -use ssz::Decode; use ssz_derive::{Decode, Encode}; pub use eth2_libp2p::{types::SyncState, PeerInfo}; @@ -340,6 +339,7 @@ impl BeaconNodeHttpClient { pub async fn get_lighthouse_beacon_states_ssz( &self, state_id: &StateId, + spec: &ChainSpec, ) -> Result>, Error> { let mut path = self.server.clone(); @@ -353,7 +353,7 @@ impl BeaconNodeHttpClient { self.get_bytes_opt(path) .await? - .map(|bytes| BeaconState::from_ssz_bytes(&bytes).map_err(Error::InvalidSsz)) + .map(|bytes| BeaconState::from_ssz_bytes(&bytes, spec).map_err(Error::InvalidSsz)) .transpose() } diff --git a/common/eth2_network_config/built_in_network_configs/mainnet/altair.yaml b/common/eth2_network_config/built_in_network_configs/mainnet/altair.yaml index 16c51c4283e..2a2552da401 100644 --- a/common/eth2_network_config/built_in_network_configs/mainnet/altair.yaml +++ b/common/eth2_network_config/built_in_network_configs/mainnet/altair.yaml @@ -31,12 +31,15 @@ EPOCHS_PER_SYNC_COMMITTEE_PERIOD: 256 # Signature domains # --------------------------------------------------------------- DOMAIN_SYNC_COMMITTEE: 0x07000000 +DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF: 0x08000000 +DOMAIN_CONTRIBUTION_AND_PROOF: 0x09000000 # Fork # --------------------------------------------------------------- +# 0x01000000 ALTAIR_FORK_VERSION: 0x01000000 -# FIXME(altair): TBD, set to int max for now +# TBD ALTAIR_FORK_SLOT: 18446744073709551615 diff --git a/common/eth2_network_config/built_in_network_configs/prater/altair.yaml b/common/eth2_network_config/built_in_network_configs/prater/altair.yaml index 16c51c4283e..2a2552da401 100644 --- a/common/eth2_network_config/built_in_network_configs/prater/altair.yaml +++ b/common/eth2_network_config/built_in_network_configs/prater/altair.yaml @@ -31,12 +31,15 @@ EPOCHS_PER_SYNC_COMMITTEE_PERIOD: 256 # Signature domains # --------------------------------------------------------------- DOMAIN_SYNC_COMMITTEE: 0x07000000 +DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF: 0x08000000 +DOMAIN_CONTRIBUTION_AND_PROOF: 0x09000000 # Fork # --------------------------------------------------------------- +# 0x01000000 ALTAIR_FORK_VERSION: 0x01000000 -# FIXME(altair): TBD, set to int max for now +# TBD ALTAIR_FORK_SLOT: 18446744073709551615 diff --git a/common/eth2_network_config/built_in_network_configs/pyrmont/altair.yaml b/common/eth2_network_config/built_in_network_configs/pyrmont/altair.yaml index 16c51c4283e..2a2552da401 100644 --- a/common/eth2_network_config/built_in_network_configs/pyrmont/altair.yaml +++ b/common/eth2_network_config/built_in_network_configs/pyrmont/altair.yaml @@ -31,12 +31,15 @@ EPOCHS_PER_SYNC_COMMITTEE_PERIOD: 256 # Signature domains # --------------------------------------------------------------- DOMAIN_SYNC_COMMITTEE: 0x07000000 +DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF: 0x08000000 +DOMAIN_CONTRIBUTION_AND_PROOF: 0x09000000 # Fork # --------------------------------------------------------------- +# 0x01000000 ALTAIR_FORK_VERSION: 0x01000000 -# FIXME(altair): TBD, set to int max for now +# TBD ALTAIR_FORK_SLOT: 18446744073709551615 diff --git a/common/eth2_network_config/built_in_network_configs/toledo/altair.yaml b/common/eth2_network_config/built_in_network_configs/toledo/altair.yaml index 16c51c4283e..2a2552da401 100644 --- a/common/eth2_network_config/built_in_network_configs/toledo/altair.yaml +++ b/common/eth2_network_config/built_in_network_configs/toledo/altair.yaml @@ -31,12 +31,15 @@ EPOCHS_PER_SYNC_COMMITTEE_PERIOD: 256 # Signature domains # --------------------------------------------------------------- DOMAIN_SYNC_COMMITTEE: 0x07000000 +DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF: 0x08000000 +DOMAIN_CONTRIBUTION_AND_PROOF: 0x09000000 # Fork # --------------------------------------------------------------- +# 0x01000000 ALTAIR_FORK_VERSION: 0x01000000 -# FIXME(altair): TBD, set to int max for now +# TBD ALTAIR_FORK_SLOT: 18446744073709551615 diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 06de89d4529..9a26a0189e9 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -9,7 +9,7 @@ use lighthouse_version::VERSION; use slog::{crit, info, warn}; use std::path::PathBuf; use std::process::exit; -use types::{init_fork_schedule, EthSpec, EthSpecId, ForkSchedule}; +use types::{EthSpec, EthSpecId}; use validator_client::ProductionValidatorClient; fn bls_library_name() -> &'static str { @@ -213,9 +213,6 @@ fn run( .optional_eth2_network_config(Some(testnet_config))? .build()?; - // Initialize fork schedule globals. - init_fork_schedule(ForkSchedule::from(&environment.eth2_config.spec)); - let log = environment.core_context().log().clone(); // Allow Prometheus to export the time at which the process was started. From 4e9e2b3f6f66f8f8c885ab52995154683551d25d Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 19 Apr 2021 16:28:25 +1000 Subject: [PATCH 4/4] Bubble YAML config changes --- beacon_node/http_api/src/lib.rs | 10 +- beacon_node/http_api/tests/tests.rs | 2 +- boot_node/src/config.rs | 6 +- common/eth2/src/lib.rs | 2 +- common/eth2/src/lighthouse_vc/http_client.rs | 2 +- common/eth2_network_config/src/lib.rs | 61 +++-- consensus/types/src/chain_spec.rs | 236 ++++++++++++------ consensus/types/src/lib.rs | 2 +- lcli/src/new_testnet.rs | 4 +- lighthouse/environment/src/lib.rs | 16 +- .../environment/tests/environment_builder.rs | 4 +- testing/ef_tests/tests/tests.rs | 16 +- validator_client/src/beacon_node_fallback.rs | 30 +-- validator_client/src/http_api/mod.rs | 4 +- validator_client/src/http_api/tests.rs | 2 +- 15 files changed, 226 insertions(+), 171 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index f44ce8d361c..82e38b39b0c 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -37,7 +37,7 @@ use tokio::sync::mpsc::UnboundedSender; use tokio_stream::{wrappers::BroadcastStream, StreamExt}; use types::{ Attestation, AttesterSlashing, CommitteeCache, Epoch, EthSpec, ProposerSlashing, RelativeEpoch, - SignedAggregateAndProof, SignedBeaconBlock, SignedVoluntaryExit, Slot, YamlConfig, + SignedAggregateAndProof, SignedBeaconBlock, SignedVoluntaryExit, Slot, StandardConfig, }; use warp::http::StatusCode; use warp::sse::Event; @@ -1274,11 +1274,9 @@ pub fn serve( .and(chain_filter.clone()) .and_then(|chain: Arc>| { blocking_json_task(move || { - Ok(api_types::GenericResponse::from(YamlConfig::from_spec::< - T::EthSpec, - >( - &chain.spec - ))) + Ok(api_types::GenericResponse::from( + StandardConfig::from_chain_spec::(&chain.spec), + )) }) }); diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 806bbc7888d..e720bec8e57 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1218,7 +1218,7 @@ impl ApiTester { pub async fn test_get_config_spec(self) -> Self { let result = self.client.get_config_spec().await.unwrap().data; - let expected = YamlConfig::from_spec::(&self.chain.spec); + let expected = StandardConfig::from_spec::(&self.chain.spec); assert_eq!(result, expected); diff --git a/boot_node/src/config.rs b/boot_node/src/config.rs index 7b894301c78..54f450a54ee 100644 --- a/boot_node/src/config.rs +++ b/boot_node/src/config.rs @@ -83,11 +83,7 @@ impl TryFrom<&ArgMatches<'_>> for BootNodeConfig { } else { // build the enr_fork_id and add it to the local_enr if it exists let enr_fork = { - // FIXME(altair): abstract `apply_to_chain_spec` pattern, use altair spec - let spec = eth2_network_config - .yaml_config - .apply_to_chain_spec::(&T::default_spec()) - .ok_or("The loaded config is not compatible with the current spec")?; + let spec = eth2_network_config.chain_spec::()?; if eth2_network_config.beacon_state_is_known() { let genesis_state = eth2_network_config.beacon_state::()?; diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 0dc7b5fd625..268902b745a 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -714,7 +714,7 @@ impl BeaconNodeHttpClient { } /// `GET config/spec` - pub async fn get_config_spec(&self) -> Result, Error> { + pub async fn get_config_spec(&self) -> Result, Error> { let mut path = self.eth_path()?; path.path_segments_mut() diff --git a/common/eth2/src/lighthouse_vc/http_client.rs b/common/eth2/src/lighthouse_vc/http_client.rs index 2258e93a2bc..3fd8db3e57b 100644 --- a/common/eth2/src/lighthouse_vc/http_client.rs +++ b/common/eth2/src/lighthouse_vc/http_client.rs @@ -210,7 +210,7 @@ impl ValidatorClientHttpClient { } /// `GET lighthouse/spec` - pub async fn get_lighthouse_spec(&self) -> Result, Error> { + pub async fn get_lighthouse_spec(&self) -> Result, Error> { let mut path = self.server.clone(); path.path_segments_mut() diff --git a/common/eth2_network_config/src/lib.rs b/common/eth2_network_config/src/lib.rs index f3f2d1a33af..d6f157828ae 100644 --- a/common/eth2_network_config/src/lib.rs +++ b/common/eth2_network_config/src/lib.rs @@ -4,20 +4,20 @@ use enr::{CombinedKey, Enr}; use std::fs::{create_dir_all, File}; use std::io::{Read, Write}; use std::path::PathBuf; -use types::{AltairConfig, BeaconState, ChainSpec, EthSpec, EthSpecId, YamlConfig}; +use types::{AltairConfig, BaseConfig, BeaconState, ChainSpec, EthSpec, EthSpecId}; pub const ADDRESS_FILE: &str = "deposit_contract.txt"; pub const DEPLOY_BLOCK_FILE: &str = "deploy_block.txt"; pub const BOOT_ENR_FILE: &str = "boot_enr.yaml"; pub const GENESIS_STATE_FILE: &str = "genesis.ssz"; -pub const YAML_CONFIG_FILE: &str = "config.yaml"; +pub const BASE_CONFIG_FILE: &str = "config.yaml"; pub const ALTAIR_CONFIG_FILE: &str = "altair.yaml"; #[derive(Copy, Clone, Debug, PartialEq)] pub struct HardcodedNet { pub name: &'static str, pub genesis_is_known: bool, - pub yaml_config: &'static [u8], + pub base_config: &'static [u8], pub altair_config: &'static [u8], pub deploy_block: &'static [u8], pub boot_enr: &'static [u8], @@ -31,7 +31,7 @@ macro_rules! define_net { HardcodedNet { name: ETH2_NET_DIR.name, genesis_is_known: ETH2_NET_DIR.genesis_is_known, - yaml_config: $include_file!("../", "config.yaml"), + base_config: $include_file!("../", "config.yaml"), altair_config: $include_file!("../", "altair.yaml"), deploy_block: $include_file!("../", "deploy_block.txt"), boot_enr: $include_file!("../", "boot_enr.yaml"), @@ -58,7 +58,7 @@ pub struct Eth2NetworkConfig { pub deposit_contract_deploy_block: u64, pub boot_enr: Option>>, pub genesis_state_bytes: Option>, - pub yaml_config: YamlConfig, + pub base_config: BaseConfig, pub altair_config: AltairConfig, } @@ -84,7 +84,7 @@ impl Eth2NetworkConfig { ), genesis_state_bytes: Some(net.genesis_state_bytes.to_vec()) .filter(|bytes| !bytes.is_empty()), - yaml_config: serde_yaml::from_reader(net.yaml_config) + base_config: serde_yaml::from_reader(net.base_config) .map_err(|e| format!("Unable to parse yaml config: {:?}", e))?, altair_config: serde_yaml::from_reader(net.altair_config) .map_err(|e| format!("Unable to parse Altair config: {:?}", e))?, @@ -94,9 +94,9 @@ impl Eth2NetworkConfig { /// Returns an identifier that should be used for selecting an `EthSpec` instance for this /// network configuration. pub fn eth_spec_id(&self) -> Result { - self.yaml_config + self.base_config .eth_spec_id() - .ok_or_else(|| format!("Unknown CONFIG_NAME: {}", self.yaml_config.config_name)) + .ok_or_else(|| format!("Unknown CONFIG_NAME: {}", self.base_config.config_name)) } /// Returns `true` if this configuration contains a `BeaconState`. @@ -106,12 +106,13 @@ impl Eth2NetworkConfig { /// Construct a consolidated `ChainSpec` from the YAML config. pub fn chain_spec(&self) -> Result { - ChainSpec::from_yaml::(&self.yaml_config, &self.altair_config).ok_or_else(|| { - format!( - "YAML configuration incompatible with spec constants for {}", - self.yaml_config.config_name - ) - }) + ChainSpec::from_standard_config::(&self.base_config, Some(&self.altair_config)) + .ok_or_else(|| { + format!( + "YAML configuration incompatible with spec constants for {}", + self.base_config.config_name + ) + }) } /// Attempts to deserialize `self.beacon_state`, returning an error if it's missing or invalid. @@ -172,7 +173,8 @@ impl Eth2NetworkConfig { write_to_yaml_file!(BOOT_ENR_FILE, boot_enr); } - write_to_yaml_file!(YAML_CONFIG_FILE, &self.yaml_config); + write_to_yaml_file!(BASE_CONFIG_FILE, &self.base_config); + write_to_yaml_file!(ALTAIR_CONFIG_FILE, &self.altair_config); // The genesis state is a special case because it uses SSZ, not YAML. if let Some(genesis_state_bytes) = &self.genesis_state_bytes { @@ -213,7 +215,7 @@ impl Eth2NetworkConfig { let deposit_contract_deploy_block = load_from_file!(DEPLOY_BLOCK_FILE); let boot_enr = optional_load_from_file!(BOOT_ENR_FILE); - let yaml_config = load_from_file!(YAML_CONFIG_FILE); + let base_config = load_from_file!(BASE_CONFIG_FILE); let altair_config = load_from_file!(ALTAIR_CONFIG_FILE); // The genesis state is a special case because it uses SSZ, not YAML. @@ -236,7 +238,7 @@ impl Eth2NetworkConfig { deposit_contract_deploy_block, boot_enr, genesis_state_bytes, - yaml_config, + base_config, altair_config, }) } @@ -247,7 +249,7 @@ mod tests { use super::*; use ssz::Encode; use tempfile::Builder as TempBuilder; - use types::{Eth1Data, Hash256, MainnetEthSpec, V012LegacyEthSpec, YamlConfig}; + use types::{BaseConfig, Eth1Data, Hash256, MainnetEthSpec}; type E = MainnetEthSpec; @@ -263,10 +265,7 @@ mod tests { || net.name == "prater" { // Ensure we can parse the YAML config to a chain spec. - config - .yaml_config - .apply_to_chain_spec::(&E::default_spec()) - .unwrap(); + config.chain_spec::().unwrap(); } assert_eq!( @@ -291,16 +290,23 @@ mod tests { // TODO: figure out how to generate ENR and add some here. let boot_enr = None; let genesis_state = Some(BeaconState::new(42, eth1_data, spec)); - let yaml_config = Some(YamlConfig::from_spec::(spec)); + let base_config = BaseConfig::from_chain_spec::(spec); + let altair_config = AltairConfig::from_chain_spec::(spec).unwrap(); - do_test::(boot_enr, genesis_state, yaml_config); - do_test::(None, None, None); + do_test::( + boot_enr, + genesis_state, + base_config.clone(), + altair_config.clone(), + ); + do_test::(None, None, base_config, altair_config); } fn do_test( boot_enr: Option>>, genesis_state: Option>, - yaml_config: Option, + base_config: BaseConfig, + altair_config: AltairConfig, ) { let temp_dir = TempBuilder::new() .prefix("eth2_testnet_test") @@ -313,7 +319,8 @@ mod tests { deposit_contract_deploy_block, boot_enr, genesis_state_bytes: genesis_state.as_ref().map(Encode::as_ssz_bytes), - yaml_config, + base_config, + altair_config, }; testnet diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 4ffec29d04a..08c23a91b94 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -1,3 +1,12 @@ +//! This file contains several different representations of the beacon chain configuration +//! parameters. +//! +//! Arguably the most import of these is `ChainSpec`, which is used throughout Lighthouse as the +//! source-of-truth regarding spec-level configuration. +//! +//! The other types exist for interoperability with other systems. The `StandardConfig` is an object +//! intended to match an EF spec configuration (usually YAML), and is broken into sub-parts for +//! each relevant fork. It is also serialised as JSON for the standardised HTTP API. use crate::*; use int_to_bytes::int_to_bytes4; use serde_derive::{Deserialize, Serialize}; @@ -135,14 +144,17 @@ pub struct ChainSpec { } impl ChainSpec { - /// Construct a `ChainSpec` from several YAML config files. - pub fn from_yaml( - phase0_yaml: &YamlConfig, - altair_yaml: &AltairConfig, + /// Construct a `ChainSpec` from several standard config files. + pub fn from_standard_config( + base: &BaseConfig, + altair: Option<&AltairConfig>, ) -> Option { let mut spec = T::default_spec(); - spec = phase0_yaml.apply_to_chain_spec::(&spec)?; - spec = altair_yaml.apply_to_chain_spec::(&spec)?; + spec = base.apply_to_chain_spec::(&spec)?; + + if let Some(altair_config) = altair { + spec = altair_config.apply_to_chain_spec::(&spec)?; + } Some(spec) } @@ -420,62 +432,37 @@ impl Default for ChainSpec { } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_mainnet_spec_can_be_constructed() { - let _ = ChainSpec::mainnet(); - } - - #[allow(clippy::useless_vec)] - fn test_domain(domain_type: Domain, raw_domain: u32, spec: &ChainSpec) { - let previous_version = [0, 0, 0, 1]; - let current_version = [0, 0, 0, 2]; - let genesis_validators_root = Hash256::from_low_u64_le(77); - let fork_epoch = Epoch::new(1024); - let fork = Fork { - previous_version, - current_version, - epoch: fork_epoch, - }; +/// Configuration struct for compatibility with the spec's .yaml configuration +#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] +pub struct StandardConfig { + #[serde(flatten)] + pub base: BaseConfig, + /// Configuration related to the Altair hard fork. + #[serde(flatten)] + pub altair: Option, - for (epoch, version) in vec![ - (fork_epoch - 1, previous_version), - (fork_epoch, current_version), - (fork_epoch + 1, current_version), - ] { - let domain1 = spec.get_domain(epoch, domain_type, &fork, genesis_validators_root); - let domain2 = spec.compute_domain(domain_type, version, genesis_validators_root); + // Extra fields (could be from a future hard-fork that we don't yet know). + #[serde(flatten)] + pub extra_fields: HashMap, +} - assert_eq!(domain1, domain2); - assert_eq!(&domain1.as_bytes()[0..4], &int_to_bytes4(raw_domain)[..]); +impl StandardConfig { + pub fn from_chain_spec(spec: &ChainSpec) -> Self { + let base = BaseConfig::from_chain_spec::(spec); + let altair = AltairConfig::from_chain_spec::(spec); + let extra_fields = HashMap::new(); + Self { + base, + altair, + extra_fields, } } - - #[test] - fn test_get_domain() { - let spec = ChainSpec::mainnet(); - - test_domain(Domain::BeaconProposer, spec.domain_beacon_proposer, &spec); - test_domain(Domain::BeaconAttester, spec.domain_beacon_attester, &spec); - test_domain(Domain::Randao, spec.domain_randao, &spec); - test_domain(Domain::Deposit, spec.domain_deposit, &spec); - test_domain(Domain::VoluntaryExit, spec.domain_voluntary_exit, &spec); - test_domain(Domain::SelectionProof, spec.domain_selection_proof, &spec); - test_domain( - Domain::AggregateAndProof, - spec.domain_aggregate_and_proof, - &spec, - ); - } } -/// YAML config file as defined by the spec. +/// Configuration related to the base/phase0/genesis fork (YAML/JSON version). #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] #[serde(rename_all = "UPPERCASE")] -pub struct YamlConfig { +pub struct BaseConfig { pub config_name: String, // ChainSpec #[serde(with = "serde_utils::quoted_u64")] @@ -599,20 +586,16 @@ pub struct YamlConfig { #[serde(with = "serde_utils::quoted_u64")] deposit_network_id: u64, deposit_contract_address: Address, - - // Extra fields (could be from a future hard-fork that we don't yet know). - #[serde(flatten)] - pub extra_fields: HashMap, } -impl Default for YamlConfig { +impl Default for BaseConfig { fn default() -> Self { let chain_spec = MainnetEthSpec::default_spec(); - YamlConfig::from_spec::(&chain_spec) + BaseConfig::from_chain_spec::(&chain_spec) } } -impl YamlConfig { +impl BaseConfig { /// Maps `self.config_name` to an identifier for an `EthSpec` instance. /// /// Returns `None` if there is no match. @@ -627,7 +610,7 @@ impl YamlConfig { }) } - pub fn from_spec(spec: &ChainSpec) -> Self { + pub fn from_chain_spec(spec: &ChainSpec) -> Self { Self { config_name: T::spec_name().to_string(), // ChainSpec @@ -694,8 +677,6 @@ impl YamlConfig { deposit_chain_id: spec.deposit_chain_id, deposit_network_id: spec.deposit_network_id, deposit_contract_address: spec.deposit_contract_address, - - extra_fields: HashMap::new(), } } @@ -877,26 +858,115 @@ impl AltairConfig { } pub fn apply_to_chain_spec(&self, chain_spec: &ChainSpec) -> Option { - if self.sync_committee_size != T::SyncCommitteeSize::to_u64() - || self.sync_pubkeys_per_aggregate != T::SyncPubkeysPerAggregate::to_u64() + // Pattern-match to avoid missing any fields. + let &AltairConfig { + // Ignore config name. + config_name: _, + inactivity_penalty_quotient_altair, + min_slashing_penalty_quotient_altair, + proportional_slashing_multiplier_altair, + sync_committee_size, + sync_pubkeys_per_aggregate, + inactivity_score_bias, + epochs_per_sync_committee_period, + domain_sync_committee, + domain_sync_committee_selection_proof, + domain_contribution_and_proof, + altair_fork_version, + altair_fork_slot, + } = self; + + if sync_committee_size != T::SyncCommitteeSize::to_u64() + || sync_pubkeys_per_aggregate != T::SyncPubkeysPerAggregate::to_u64() { return None; } Some(ChainSpec { - inactivity_penalty_quotient_altair: self.inactivity_penalty_quotient_altair, - min_slashing_penalty_quotient_altair: self.min_slashing_penalty_quotient_altair, - proportional_slashing_multiplier_altair: self.proportional_slashing_multiplier_altair, - inactivity_score_bias: self.inactivity_score_bias, - epochs_per_sync_committee_period: self.epochs_per_sync_committee_period, - domain_sync_committee: self.domain_sync_committee, - domain_sync_committee_selection_proof: self.domain_sync_committee_selection_proof, - domain_contribution_and_proof: self.domain_contribution_and_proof, - altair_fork_version: self.altair_fork_version, - altair_fork_slot: Some(self.altair_fork_slot), + inactivity_penalty_quotient_altair, + min_slashing_penalty_quotient_altair, + proportional_slashing_multiplier_altair, + inactivity_score_bias, + epochs_per_sync_committee_period, + domain_sync_committee, + domain_sync_committee_selection_proof, + domain_contribution_and_proof, + altair_fork_version, + altair_fork_slot: Some(altair_fork_slot), ..chain_spec.clone() }) } + + pub fn from_chain_spec(spec: &ChainSpec) -> Option { + Some(Self { + config_name: T::spec_name().to_string(), + inactivity_penalty_quotient_altair: spec.inactivity_penalty_quotient_altair, + min_slashing_penalty_quotient_altair: spec.min_slashing_penalty_quotient_altair, + proportional_slashing_multiplier_altair: spec.proportional_slashing_multiplier_altair, + sync_committee_size: T::SyncCommitteeSize::to_u64(), + sync_pubkeys_per_aggregate: T::SyncPubkeysPerAggregate::to_u64(), + inactivity_score_bias: spec.inactivity_score_bias, + epochs_per_sync_committee_period: spec.epochs_per_sync_committee_period, + domain_sync_committee: spec.domain_sync_committee, + domain_sync_committee_selection_proof: spec.domain_sync_committee_selection_proof, + domain_contribution_and_proof: spec.domain_contribution_and_proof, + altair_fork_version: spec.altair_fork_version, + altair_fork_slot: spec.altair_fork_slot?, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_mainnet_spec_can_be_constructed() { + let _ = ChainSpec::mainnet(); + } + + #[allow(clippy::useless_vec)] + fn test_domain(domain_type: Domain, raw_domain: u32, spec: &ChainSpec) { + let previous_version = [0, 0, 0, 1]; + let current_version = [0, 0, 0, 2]; + let genesis_validators_root = Hash256::from_low_u64_le(77); + let fork_epoch = Epoch::new(1024); + let fork = Fork { + previous_version, + current_version, + epoch: fork_epoch, + }; + + for (epoch, version) in vec![ + (fork_epoch - 1, previous_version), + (fork_epoch, current_version), + (fork_epoch + 1, current_version), + ] { + let domain1 = spec.get_domain(epoch, domain_type, &fork, genesis_validators_root); + let domain2 = spec.compute_domain(domain_type, version, genesis_validators_root); + + assert_eq!(domain1, domain2); + assert_eq!(&domain1.as_bytes()[0..4], &int_to_bytes4(raw_domain)[..]); + } + } + + #[test] + fn test_get_domain() { + let spec = ChainSpec::mainnet(); + + test_domain(Domain::BeaconProposer, spec.domain_beacon_proposer, &spec); + test_domain(Domain::BeaconAttester, spec.domain_beacon_attester, &spec); + test_domain(Domain::Randao, spec.domain_randao, &spec); + test_domain(Domain::Deposit, spec.domain_deposit, &spec); + test_domain(Domain::VoluntaryExit, spec.domain_voluntary_exit, &spec); + test_domain(Domain::SelectionProof, spec.domain_selection_proof, &spec); + test_domain( + Domain::AggregateAndProof, + spec.domain_aggregate_and_proof, + &spec, + ); + test_domain(Domain::SyncCommittee, spec.domain_sync_committee, &spec); + } } #[cfg(test)] @@ -916,7 +986,7 @@ mod yaml_tests { .expect("error opening file"); let minimal_spec = ChainSpec::minimal(); - let yamlconfig = YamlConfig::from_spec::(&minimal_spec); + let yamlconfig = BaseConfig::from_chain_spec::(&minimal_spec); // write fresh minimal config to file serde_yaml::to_writer(writer, &yamlconfig).expect("failed to write or serialize"); @@ -926,7 +996,7 @@ mod yaml_tests { .open(tmp_file.as_ref()) .expect("error while opening the file"); // deserialize minimal config from file - let from: YamlConfig = serde_yaml::from_reader(reader).expect("error while deserializing"); + let from: BaseConfig = serde_yaml::from_reader(reader).expect("error while deserializing"); assert_eq!(from, yamlconfig); } @@ -939,7 +1009,7 @@ mod yaml_tests { .open(tmp_file.as_ref()) .expect("error opening file"); let mainnet_spec = ChainSpec::mainnet(); - let yamlconfig = YamlConfig::from_spec::(&mainnet_spec); + let yamlconfig = BaseConfig::from_chain_spec::(&mainnet_spec); serde_yaml::to_writer(writer, &yamlconfig).expect("failed to write or serialize"); let reader = OpenOptions::new() @@ -947,7 +1017,7 @@ mod yaml_tests { .write(false) .open(tmp_file.as_ref()) .expect("error while opening the file"); - let from: YamlConfig = serde_yaml::from_reader(reader).expect("error while deserializing"); + let from: BaseConfig = serde_yaml::from_reader(reader).expect("error while deserializing"); assert_eq!(from, yamlconfig); } @@ -960,7 +1030,7 @@ mod yaml_tests { .open(tmp_file.as_ref()) .expect("error opening file"); let mainnet_spec = ChainSpec::mainnet(); - let mut yamlconfig = YamlConfig::from_spec::(&mainnet_spec); + let mut yamlconfig = BaseConfig::from_chain_spec::(&mainnet_spec); let (k1, v1) = ("SAMPLE_HARDFORK_KEY1", "123456789"); let (k2, v2) = ("SAMPLE_HARDFORK_KEY2", "987654321"); yamlconfig.extra_fields.insert(k1.into(), v1.into()); @@ -972,14 +1042,14 @@ mod yaml_tests { .write(false) .open(tmp_file.as_ref()) .expect("error while opening the file"); - let from: YamlConfig = serde_yaml::from_reader(reader).expect("error while deserializing"); + let from: BaseConfig = serde_yaml::from_reader(reader).expect("error while deserializing"); assert_eq!(from, yamlconfig); } #[test] fn apply_to_spec() { let mut spec = ChainSpec::minimal(); - let yamlconfig = YamlConfig::from_spec::(&spec); + let yamlconfig = BaseConfig::from_chain_spec::(&spec); // modifying the original spec spec.max_committees_per_slot += 1; diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 6a4ccfedae6..66825ddeb32 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -76,7 +76,7 @@ pub use crate::beacon_block_body::{ pub use crate::beacon_block_header::BeaconBlockHeader; pub use crate::beacon_committee::{BeaconCommittee, OwnedBeaconCommittee}; pub use crate::beacon_state::{BeaconTreeHashCache, Error as BeaconStateError, *}; -pub use crate::chain_spec::{AltairConfig, ChainSpec, Domain, YamlConfig}; +pub use crate::chain_spec::{AltairConfig, BaseConfig, ChainSpec, Domain, StandardConfig}; pub use crate::checkpoint::Checkpoint; pub use crate::deposit::{Deposit, DEPOSIT_TREE_DEPTH}; pub use crate::deposit_data::DepositData; diff --git a/lcli/src/new_testnet.rs b/lcli/src/new_testnet.rs index af086102d7a..28efabecad6 100644 --- a/lcli/src/new_testnet.rs +++ b/lcli/src/new_testnet.rs @@ -4,7 +4,7 @@ use clap_utils::{ }; use eth2_network_config::Eth2NetworkConfig; use std::path::PathBuf; -use types::{Address, EthSpec, YamlConfig}; +use types::{Address, EthSpec, StandardConfig}; pub fn run(matches: &ArgMatches) -> Result<(), String> { let testnet_dir_path = parse_path_with_default_in_home_dir( @@ -60,7 +60,7 @@ pub fn run(matches: &ArgMatches) -> Result<(), String> { deposit_contract_deploy_block, boot_enr: Some(vec![]), genesis_state_bytes: None, - yaml_config: Some(YamlConfig::from_spec::(&spec)), + yaml_config: Some(StandardConfig::from_spec::(&spec)), }; testnet.write_to_file(testnet_dir_path, overwrite_files) diff --git a/lighthouse/environment/src/lib.rs b/lighthouse/environment/src/lib.rs index 2d9bd42bc81..ae5c0cfa80b 100644 --- a/lighthouse/environment/src/lib.rs +++ b/lighthouse/environment/src/lib.rs @@ -212,21 +212,7 @@ impl EnvironmentBuilder { eth2_network_config: Eth2NetworkConfig, ) -> Result { // Create a new chain spec from the default configuration. - self.eth2_config.spec = eth2_network_config - .yaml_config - .apply_to_chain_spec::(&self.eth2_config.spec) - .and_then(|spec| { - eth2_network_config - .altair_config - .apply_to_chain_spec::(&spec) - }) - .ok_or_else(|| { - format!( - "The loaded config is not compatible with the {} spec", - &self.eth2_config.eth_spec_id - ) - })?; - + self.eth2_config.spec = eth2_network_config.chain_spec::()?; self.testnet = Some(eth2_network_config); Ok(self) diff --git a/lighthouse/environment/tests/environment_builder.rs b/lighthouse/environment/tests/environment_builder.rs index ce03e686cf3..5538eb0e540 100644 --- a/lighthouse/environment/tests/environment_builder.rs +++ b/lighthouse/environment/tests/environment_builder.rs @@ -3,7 +3,7 @@ use environment::EnvironmentBuilder; use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK}; use std::path::PathBuf; -use types::{V012LegacyEthSpec, YamlConfig}; +use types::{StandardConfig, V012LegacyEthSpec}; fn builder() -> EnvironmentBuilder { EnvironmentBuilder::v012_legacy() @@ -26,7 +26,7 @@ mod setup_eth2_config { let config_yaml = PathBuf::from("./tests/testnet_dir/config.yaml"); eth2_network_config.yaml_config = Some( - YamlConfig::from_file(config_yaml.as_path()).expect("should load yaml config"), + StandardConfig::from_file(config_yaml.as_path()).expect("should load yaml config"), ); let environment = builder() diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 311622db817..52fe6135cb3 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -1,7 +1,6 @@ #![cfg(feature = "ef_tests")] use ef_tests::*; -use std::collections::HashMap; use std::path::PathBuf; use types::*; @@ -14,22 +13,19 @@ fn config_test() { .join("config"); let phase0_config_path = config_dir.join("phase0.yaml"); let altair_config_path = config_dir.join("altair.yaml"); - let phase0_config = YamlConfig::from_file(&phase0_config_path).expect("config file loads OK"); + let phase0_config = BaseConfig::from_file(&phase0_config_path).expect("config file loads OK"); let altair_config = AltairConfig::from_file(&altair_config_path).expect("altair config loads"); let spec = E::default_spec(); - let unified_spec = - ChainSpec::from_yaml::(&phase0_config, &altair_config).expect("config unification"); + let unified_spec = ChainSpec::from_standard_config::(&phase0_config, Some(&altair_config)) + .expect("config unification"); assert_eq!(unified_spec, spec); - let phase0_from_spec = YamlConfig::from_spec::(&spec); + let phase0_from_spec = BaseConfig::from_chain_spec::(&spec); assert_eq!(phase0_from_spec, phase0_config); - assert_eq!( - phase0_config.extra_fields, - HashMap::new(), - "not all config fields read" - ); + let altair_from_spec = AltairConfig::from_chain_spec::(&spec); + assert_eq!(altair_from_spec, Some(altair_config)); } #[test] diff --git a/validator_client/src/beacon_node_fallback.rs b/validator_client/src/beacon_node_fallback.rs index 78def569e8b..fb058c6ede2 100644 --- a/validator_client/src/beacon_node_fallback.rs +++ b/validator_client/src/beacon_node_fallback.rs @@ -209,7 +209,7 @@ impl CandidateBeaconNode { /// Checks if the node has the correct specification. async fn is_compatible(&self, spec: &ChainSpec, log: &Logger) -> Result<(), CandidateError> { - let yaml_config = self + let std_config = self .beacon_node .get_config_spec() .await @@ -224,23 +224,25 @@ impl CandidateBeaconNode { })? .data; - let beacon_node_spec = yaml_config - .apply_to_chain_spec::(&E::default_spec()) - .ok_or_else(|| { - error!( - log, - "The minimal/mainnet spec type of the beacon node does not match the validator \ - client. See the --network command."; - "endpoint" => %self.beacon_node, - ); - CandidateError::Incompatible - })?; + let beacon_node_spec = ChainSpec::from_standard_config::( + &std_config.base, + std_config.altair.as_ref(), + ) + .ok_or_else(|| { + error!( + log, + "The minimal/mainnet spec type of the beacon node does not match the validator \ + client. See the --network command."; + "endpoint" => %self.beacon_node, + ); + CandidateError::Incompatible + })?; - if !yaml_config.extra_fields.is_empty() { + if !std_config.extra_fields.is_empty() { debug!( log, "Beacon spec includes unknown fields"; - "fields" => ?yaml_config.extra_fields + "fields" => ?std_config.extra_fields ); } diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index ccf465f53f5..b9779e20d25 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -16,7 +16,7 @@ use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use std::path::PathBuf; use std::sync::{Arc, Weak}; use tokio::runtime::Runtime; -use types::{ChainSpec, EthSpec, YamlConfig}; +use types::{ChainSpec, EthSpec, StandardConfig}; use validator_dir::Builder as ValidatorDirBuilder; use warp::{ http::{ @@ -192,7 +192,7 @@ pub fn serve( .and_then(|spec: Arc<_>, signer| { blocking_signed_json_task(signer, move || { Ok(api_types::GenericResponse::from( - YamlConfig::from_spec::(&spec), + StandardConfig::from_chain_spec::(&spec), )) }) }); diff --git a/validator_client/src/http_api/tests.rs b/validator_client/src/http_api/tests.rs index d7fcc38c1f7..7b057b450f1 100644 --- a/validator_client/src/http_api/tests.rs +++ b/validator_client/src/http_api/tests.rs @@ -152,7 +152,7 @@ impl ApiTester { pub async fn test_get_lighthouse_spec(self) -> Self { let result = self.client.get_lighthouse_spec().await.unwrap().data; - let expected = YamlConfig::from_spec::(&E::default_spec()); + let expected = StandardConfig::from_spec::(&E::default_spec()); assert_eq!(result, expected);