diff --git a/Cargo.lock b/Cargo.lock index 87c20ee0c..fb79228d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -622,7 +622,7 @@ dependencies = [ [[package]] name = "beacon_node" -version = "3.5.1" +version = "4.0.1-rc.0" dependencies = [ "beacon_chain", "clap", @@ -791,7 +791,7 @@ dependencies = [ [[package]] name = "boot_node" -version = "3.5.1" +version = "4.0.1-rc.0" dependencies = [ "beacon_node", "clap", @@ -1619,16 +1619,6 @@ version = "0.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b72465f46d518f6015d9cf07f7f3013a95dd6b9c2747c3d65ae0cce43929d14f" -[[package]] -name = "delay_map" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c4d75d3abfe4830dcbf9bcb1b926954e121669f74dd1ca7aa0183b1755d83f6" -dependencies = [ - "futures", - "tokio-util 0.6.10", -] - [[package]] name = "delay_map" version = "0.3.0" @@ -1830,15 +1820,15 @@ dependencies = [ [[package]] name = "discv5" -version = "0.1.0" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d767c0e59b3e8d65222d95df723cc2ea1da92bb0f27c563607e6f0bde064f255" +checksum = "b009a99b85b58900df46435307fc5c4c845af7e182582b1fbf869572fa9fce69" dependencies = [ "aes 0.7.5", "aes-gcm 0.9.4", "arrayvec", - "delay_map 0.1.2", - "enr", + "delay_map", + "enr 0.7.0", "fnv", "futures", "hashlink 0.7.0", @@ -1987,6 +1977,25 @@ name = "enr" version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26fa0a0be8915790626d5759eb51fe47435a8eac92c2f212bd2da9aa7f30ea56" +dependencies = [ + "base64 0.13.1", + "bs58", + "bytes", + "hex", + "k256", + "log", + "rand 0.8.5", + "rlp", + "serde", + "sha3 0.10.6", + "zeroize", +] + +[[package]] +name = "enr" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "492a7e5fc2504d5fdce8e124d3e263b244a68b283cac67a69eda0cd43e0aebad" dependencies = [ "base64 0.13.1", "bs58", @@ -2235,7 +2244,7 @@ dependencies = [ name = "eth2_network_config" version = "0.2.0" dependencies = [ - "enr", + "discv5", "eth2_config", "eth2_ssz", "kzg", @@ -2388,7 +2397,7 @@ dependencies = [ "async-stream", "blst", "bs58", - "enr", + "enr 0.6.2", "hex", "integer-sqrt", "multiaddr 0.14.0", @@ -3794,7 +3803,7 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "lcli" -version = "3.5.1" +version = "4.0.1-rc.0" dependencies = [ "account_utils", "beacon_chain", @@ -4400,7 +4409,7 @@ dependencies = [ [[package]] name = "lighthouse" -version = "3.5.1" +version = "4.0.1-rc.0" dependencies = [ "account_manager", "account_utils", @@ -4450,7 +4459,7 @@ dependencies = [ name = "lighthouse_network" version = "0.2.0" dependencies = [ - "delay_map 0.3.0", + "delay_map", "directory", "dirs", "discv5", @@ -5067,7 +5076,7 @@ name = "network" version = "0.2.0" dependencies = [ "beacon_chain", - "delay_map 0.3.0", + "delay_map", "derivative", "environment", "error-chain", diff --git a/beacon_node/Cargo.toml b/beacon_node/Cargo.toml index c7eccf2e3..5e519e06b 100644 --- a/beacon_node/Cargo.toml +++ b/beacon_node/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "beacon_node" -version = "3.5.1" +version = "4.0.1-rc.0" authors = ["Paul Hauner ", "Age Manning BeaconChain { pub fn load_fork_choice( store: BeaconStore, reset_payload_statuses: ResetPayloadStatuses, - count_unrealized_full: CountUnrealizedFull, spec: &ChainSpec, log: &Logger, ) -> Result>, Error> { @@ -502,7 +501,6 @@ impl BeaconChain { persisted_fork_choice.fork_choice, reset_payload_statuses, fc_store, - count_unrealized_full, spec, log, )?)) @@ -1986,7 +1984,6 @@ impl BeaconChain { self.slot()?, verified.indexed_attestation(), AttestationFromBlock::False, - &self.spec, ) .map_err(Into::into) } @@ -2953,7 +2950,7 @@ impl BeaconChain { &state, payload_verification_status, &self.spec, - count_unrealized.and(self.config.count_unrealized.into()), + count_unrealized, ) .map_err(|e| BlockError::BeaconChainError(e.into()))?; } @@ -3096,7 +3093,6 @@ impl BeaconChain { ResetPayloadStatuses::always_reset_conditionally( self.config.always_reset_payload_statuses, ), - self.config.count_unrealized_full, &self.store, &self.spec, &self.log, 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 b17613da0..71160fcb6 100644 --- a/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs +++ b/beacon_node/beacon_chain/src/beacon_fork_choice_store.rs @@ -20,6 +20,14 @@ use types::{ Hash256, Slot, }; +/// Ensure this justified checkpoint has an epoch of 0 so that it is never +/// greater than the justified checkpoint and enshrined as the actual justified +/// checkpoint. +const JUNK_BEST_JUSTIFIED_CHECKPOINT: Checkpoint = Checkpoint { + epoch: Epoch::new(0), + root: Hash256::repeat_byte(0), +}; + #[derive(Debug)] pub enum Error { UnableToReadSlot, @@ -144,7 +152,6 @@ pub struct BeaconForkChoiceStore, Cold: ItemStore< finalized_checkpoint: Checkpoint, justified_checkpoint: Checkpoint, justified_balances: JustifiedBalances, - best_justified_checkpoint: Checkpoint, unrealized_justified_checkpoint: Checkpoint, unrealized_finalized_checkpoint: Checkpoint, proposer_boost_root: Hash256, @@ -194,7 +201,6 @@ where justified_checkpoint, justified_balances, finalized_checkpoint, - best_justified_checkpoint: justified_checkpoint, unrealized_justified_checkpoint: justified_checkpoint, unrealized_finalized_checkpoint: finalized_checkpoint, proposer_boost_root: Hash256::zero(), @@ -212,7 +218,7 @@ where finalized_checkpoint: self.finalized_checkpoint, justified_checkpoint: self.justified_checkpoint, justified_balances: self.justified_balances.effective_balances.clone(), - best_justified_checkpoint: self.best_justified_checkpoint, + best_justified_checkpoint: JUNK_BEST_JUSTIFIED_CHECKPOINT, unrealized_justified_checkpoint: self.unrealized_justified_checkpoint, unrealized_finalized_checkpoint: self.unrealized_finalized_checkpoint, proposer_boost_root: self.proposer_boost_root, @@ -234,7 +240,6 @@ where finalized_checkpoint: persisted.finalized_checkpoint, justified_checkpoint: persisted.justified_checkpoint, justified_balances, - best_justified_checkpoint: persisted.best_justified_checkpoint, unrealized_justified_checkpoint: persisted.unrealized_justified_checkpoint, unrealized_finalized_checkpoint: persisted.unrealized_finalized_checkpoint, proposer_boost_root: persisted.proposer_boost_root, @@ -277,10 +282,6 @@ where &self.justified_balances } - fn best_justified_checkpoint(&self) -> &Checkpoint { - &self.best_justified_checkpoint - } - fn finalized_checkpoint(&self) -> &Checkpoint { &self.finalized_checkpoint } @@ -333,10 +334,6 @@ where Ok(()) } - fn set_best_justified_checkpoint(&mut self, checkpoint: Checkpoint) { - self.best_justified_checkpoint = checkpoint - } - fn set_unrealized_justified_checkpoint(&mut self, checkpoint: Checkpoint) { self.unrealized_justified_checkpoint = checkpoint; } diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 2f80c1b3a..a1f7abf06 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1550,7 +1550,6 @@ impl ExecutionPendingBlock { current_slot, indexed_attestation, AttestationFromBlock::True, - &chain.spec, ) { Ok(()) => Ok(()), // Ignore invalid attestations whilst importing attestations from a block. The diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 2d92ad6cb..3698549ed 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -19,7 +19,7 @@ use crate::{ }; use eth1::Config as Eth1Config; use execution_layer::ExecutionLayer; -use fork_choice::{ForkChoice, ResetPayloadStatuses}; +use fork_choice::{CountUnrealized, ForkChoice, ResetPayloadStatuses}; use futures::channel::mpsc::Sender; use kzg::{Kzg, TrustedSetup}; use operation_pool::{OperationPool, PersistedOperationPool}; @@ -269,7 +269,6 @@ where ResetPayloadStatuses::always_reset_conditionally( self.chain_config.always_reset_payload_statuses, ), - self.chain_config.count_unrealized_full, &self.spec, log, ) @@ -388,7 +387,6 @@ where &genesis.beacon_block, &genesis.beacon_state, current_slot, - self.chain_config.count_unrealized_full, &self.spec, ) .map_err(|e| format!("Unable to initialize ForkChoice: {:?}", e))?; @@ -507,7 +505,6 @@ where &snapshot.beacon_block, &snapshot.beacon_state, current_slot, - self.chain_config.count_unrealized_full, &self.spec, ) .map_err(|e| format!("Unable to initialize ForkChoice: {:?}", e))?; @@ -698,8 +695,7 @@ where store.clone(), Some(current_slot), &self.spec, - self.chain_config.count_unrealized.into(), - self.chain_config.count_unrealized_full, + CountUnrealized::True, )?; } @@ -782,6 +778,7 @@ where let genesis_time = head_snapshot.beacon_state.genesis_time(); let head_for_snapshot_cache = head_snapshot.clone(); let canonical_head = CanonicalHead::new(fork_choice, Arc::new(head_snapshot)); + let shuffling_cache_size = self.chain_config.shuffling_cache_size; let beacon_chain = BeaconChain { spec: self.spec, @@ -835,7 +832,7 @@ where DEFAULT_SNAPSHOT_CACHE_SIZE, head_for_snapshot_cache, )), - shuffling_cache: TimeoutRwLock::new(ShufflingCache::new()), + shuffling_cache: TimeoutRwLock::new(ShufflingCache::new(shuffling_cache_size)), eth1_finalization_cache: TimeoutRwLock::new(Eth1FinalizationCache::new(log.clone())), beacon_proposer_cache: <_>::default(), block_times_cache: <_>::default(), diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 647089f9f..685b8f655 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -45,8 +45,7 @@ use crate::{ }; use eth2::types::{EventKind, SseChainReorg, SseFinalizedCheckpoint, SseHead, SseLateHead}; use fork_choice::{ - CountUnrealizedFull, ExecutionStatus, ForkChoiceView, ForkchoiceUpdateParameters, ProtoBlock, - ResetPayloadStatuses, + ExecutionStatus, ForkChoiceView, ForkchoiceUpdateParameters, ProtoBlock, ResetPayloadStatuses, }; use itertools::process_results; use parking_lot::{Mutex, RwLock, RwLockReadGuard, RwLockWriteGuard}; @@ -285,19 +284,13 @@ impl CanonicalHead { // defensive programming. mut fork_choice_write_lock: RwLockWriteGuard>, reset_payload_statuses: ResetPayloadStatuses, - count_unrealized_full: CountUnrealizedFull, store: &BeaconStore, spec: &ChainSpec, log: &Logger, ) -> Result<(), Error> { - let fork_choice = >::load_fork_choice( - store.clone(), - reset_payload_statuses, - count_unrealized_full, - spec, - log, - )? - .ok_or(Error::MissingPersistedForkChoice)?; + let fork_choice = + >::load_fork_choice(store.clone(), reset_payload_statuses, spec, log)? + .ok_or(Error::MissingPersistedForkChoice)?; let fork_choice_view = fork_choice.cached_fork_choice_view(); let beacon_block_root = fork_choice_view.head_block_root; let beacon_block = store diff --git a/beacon_node/beacon_chain/src/chain_config.rs b/beacon_node/beacon_chain/src/chain_config.rs index 6e3538aed..1a5394256 100644 --- a/beacon_node/beacon_chain/src/chain_config.rs +++ b/beacon_node/beacon_chain/src/chain_config.rs @@ -1,4 +1,4 @@ -pub use proto_array::{CountUnrealizedFull, ReOrgThreshold}; +pub use proto_array::ReOrgThreshold; use serde_derive::{Deserialize, Serialize}; use std::time::Duration; use types::{Checkpoint, Epoch}; @@ -48,16 +48,11 @@ pub struct ChainConfig { pub builder_fallback_epochs_since_finalization: usize, /// Whether any chain health checks should be considered when deciding whether to use the builder API. pub builder_fallback_disable_checks: bool, - /// When set to `true`, weigh the "unrealized" FFG progression when choosing a head in fork - /// choice. - pub count_unrealized: bool, /// When set to `true`, forget any valid/invalid/optimistic statuses in fork choice during start /// up. pub always_reset_payload_statuses: bool, /// Whether to apply paranoid checks to blocks proposed by this beacon node. pub paranoid_block_proposal: bool, - /// Whether to strictly count unrealized justified votes. - pub count_unrealized_full: CountUnrealizedFull, /// Optionally set timeout for calls to checkpoint sync endpoint. pub checkpoint_sync_url_timeout: u64, /// The offset before the start of a proposal slot at which payload attributes should be sent. @@ -67,6 +62,8 @@ pub struct ChainConfig { pub prepare_payload_lookahead: Duration, /// Use EL-free optimistic sync for the finalized part of the chain. pub optimistic_finalized_sync: bool, + /// The size of the shuffling cache, + pub shuffling_cache_size: usize, /// Whether to send payload attributes every slot, regardless of connected proposers. /// /// This is useful for block builders and testing. @@ -89,14 +86,13 @@ impl Default for ChainConfig { builder_fallback_skips_per_epoch: 8, builder_fallback_epochs_since_finalization: 3, builder_fallback_disable_checks: false, - count_unrealized: true, always_reset_payload_statuses: false, paranoid_block_proposal: false, - count_unrealized_full: CountUnrealizedFull::default(), checkpoint_sync_url_timeout: 60, prepare_payload_lookahead: Duration::from_secs(4), // This value isn't actually read except in tests. optimistic_finalized_sync: true, + shuffling_cache_size: crate::shuffling_cache::DEFAULT_CACHE_SIZE, always_prepare_payload: false, } } diff --git a/beacon_node/beacon_chain/src/fork_revert.rs b/beacon_node/beacon_chain/src/fork_revert.rs index 6d5b5ddc4..ef23248ab 100644 --- a/beacon_node/beacon_chain/src/fork_revert.rs +++ b/beacon_node/beacon_chain/src/fork_revert.rs @@ -1,7 +1,6 @@ use crate::{BeaconForkChoiceStore, BeaconSnapshot}; use fork_choice::{CountUnrealized, ForkChoice, PayloadVerificationStatus}; use itertools::process_results; -use proto_array::CountUnrealizedFull; use slog::{info, warn, Logger}; use state_processing::state_advance::complete_state_advance; use state_processing::{ @@ -102,7 +101,6 @@ pub fn reset_fork_choice_to_finalization, Cold: It current_slot: Option, spec: &ChainSpec, count_unrealized_config: CountUnrealized, - count_unrealized_full_config: CountUnrealizedFull, ) -> Result, E>, String> { // Fetch finalized block. let finalized_checkpoint = head_state.finalized_checkpoint(); @@ -156,7 +154,6 @@ pub fn reset_fork_choice_to_finalization, Cold: It &finalized_snapshot.beacon_block, &finalized_snapshot.beacon_state, current_slot, - count_unrealized_full_config, spec, ) .map_err(|e| format!("Unable to reset fork choice for revert: {:?}", e))?; diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs index 9519eebe0..feafa7fd8 100644 --- a/beacon_node/beacon_chain/src/lib.rs +++ b/beacon_node/beacon_chain/src/lib.rs @@ -43,7 +43,7 @@ mod persisted_fork_choice; mod pre_finalization_cache; pub mod proposer_prep_service; pub mod schema_change; -mod shuffling_cache; +pub mod shuffling_cache; mod snapshot_cache; pub mod state_advance_timer; pub mod sync_committee_rewards; @@ -60,7 +60,7 @@ pub use self::beacon_chain::{ INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON, MAXIMUM_GOSSIP_CLOCK_DISPARITY, }; pub use self::beacon_snapshot::BeaconSnapshot; -pub use self::chain_config::{ChainConfig, CountUnrealizedFull}; +pub use self::chain_config::ChainConfig; pub use self::errors::{BeaconChainError, BlockProductionError}; pub use self::historical_blocks::HistoricalBlockError; pub use attestation_verification::Error as AttestationError; diff --git a/beacon_node/beacon_chain/src/schema_change.rs b/beacon_node/beacon_chain/src/schema_change.rs index 35202a3c5..5808e648a 100644 --- a/beacon_node/beacon_chain/src/schema_change.rs +++ b/beacon_node/beacon_chain/src/schema_change.rs @@ -3,6 +3,7 @@ mod migration_schema_v12; mod migration_schema_v13; mod migration_schema_v14; mod migration_schema_v15; +mod migration_schema_v16; use crate::beacon_chain::{BeaconChainTypes, ETH1_CACHE_DB_KEY}; use crate::eth1_chain::SszEth1; @@ -132,6 +133,14 @@ pub fn migrate_schema( let ops = migration_schema_v15::downgrade_from_v15::(db.clone(), log)?; db.store_schema_version_atomically(to, ops) } + (SchemaVersion(15), SchemaVersion(16)) => { + let ops = migration_schema_v16::upgrade_to_v16::(db.clone(), log)?; + db.store_schema_version_atomically(to, ops) + } + (SchemaVersion(16), SchemaVersion(15)) => { + let ops = migration_schema_v16::downgrade_from_v16::(db.clone(), log)?; + db.store_schema_version_atomically(to, ops) + } // Anything else is an error. (_, _) => Err(HotColdDBError::UnsupportedSchemaVersion { target_version: to, diff --git a/beacon_node/beacon_chain/src/schema_change/migration_schema_v16.rs b/beacon_node/beacon_chain/src/schema_change/migration_schema_v16.rs new file mode 100644 index 000000000..230573b02 --- /dev/null +++ b/beacon_node/beacon_chain/src/schema_change/migration_schema_v16.rs @@ -0,0 +1,46 @@ +use crate::beacon_chain::{BeaconChainTypes, FORK_CHOICE_DB_KEY}; +use crate::persisted_fork_choice::PersistedForkChoiceV11; +use slog::{debug, Logger}; +use std::sync::Arc; +use store::{Error, HotColdDB, KeyValueStoreOp, StoreItem}; + +pub fn upgrade_to_v16( + db: Arc>, + log: Logger, +) -> Result, Error> { + drop_balances_cache::(db, log) +} + +pub fn downgrade_from_v16( + db: Arc>, + log: Logger, +) -> Result, Error> { + drop_balances_cache::(db, log) +} + +/// Drop the balances cache from the fork choice store. +/// +/// There aren't any type-level changes in this schema migration, however the +/// way that we compute the `JustifiedBalances` has changed due to: +/// https://github.com/sigp/lighthouse/pull/3962 +pub fn drop_balances_cache( + db: Arc>, + log: Logger, +) -> Result, Error> { + let mut persisted_fork_choice = db + .get_item::(&FORK_CHOICE_DB_KEY)? + .ok_or_else(|| Error::SchemaMigrationError("fork choice missing from database".into()))?; + + debug!( + log, + "Dropping fork choice balances cache"; + "item_count" => persisted_fork_choice.fork_choice_store.balances_cache.items.len() + ); + + // Drop all items in the balances cache. + persisted_fork_choice.fork_choice_store.balances_cache = <_>::default(); + + let kv_op = persisted_fork_choice.as_kv_store_op(FORK_CHOICE_DB_KEY); + + Ok(vec![kv_op]) +} diff --git a/beacon_node/beacon_chain/src/shuffling_cache.rs b/beacon_node/beacon_chain/src/shuffling_cache.rs index a01847a0e..91a1e24d8 100644 --- a/beacon_node/beacon_chain/src/shuffling_cache.rs +++ b/beacon_node/beacon_chain/src/shuffling_cache.rs @@ -9,7 +9,7 @@ use types::{beacon_state::CommitteeCache, AttestationShufflingId, Epoch, Hash256 /// Each entry should be `8 + 800,000 = 800,008` bytes in size with 100k validators. (8-byte hash + /// 100k indices). Therefore, this cache should be approx `16 * 800,008 = 12.8 MB`. (Note: this /// ignores a few extra bytes in the caches that should be insignificant compared to the indices). -const CACHE_SIZE: usize = 16; +pub const DEFAULT_CACHE_SIZE: usize = 16; /// The maximum number of concurrent committee cache "promises" that can be issued. In effect, this /// limits the number of concurrent states that can be loaded into memory for the committee cache. @@ -54,9 +54,9 @@ pub struct ShufflingCache { } impl ShufflingCache { - pub fn new() -> Self { + pub fn new(cache_size: usize) -> Self { Self { - cache: LruCache::new(CACHE_SIZE), + cache: LruCache::new(cache_size), } } @@ -172,7 +172,7 @@ impl ToArcCommitteeCache for Arc { impl Default for ShufflingCache { fn default() -> Self { - Self::new() + Self::new(DEFAULT_CACHE_SIZE) } } @@ -249,7 +249,7 @@ mod test { fn resolved_promise() { let (committee_a, _) = committee_caches(); let id_a = shuffling_id(1); - let mut cache = ShufflingCache::new(); + let mut cache = ShufflingCache::default(); // Create a promise. let sender = cache.create_promise(id_a.clone()).unwrap(); @@ -276,7 +276,7 @@ mod test { #[test] fn unresolved_promise() { let id_a = shuffling_id(1); - let mut cache = ShufflingCache::new(); + let mut cache = ShufflingCache::default(); // Create a promise. let sender = cache.create_promise(id_a.clone()).unwrap(); @@ -301,7 +301,7 @@ mod test { fn two_promises() { let (committee_a, committee_b) = committee_caches(); let (id_a, id_b) = (shuffling_id(1), shuffling_id(2)); - let mut cache = ShufflingCache::new(); + let mut cache = ShufflingCache::default(); // Create promise A. let sender_a = cache.create_promise(id_a.clone()).unwrap(); @@ -355,7 +355,7 @@ mod test { #[test] fn too_many_promises() { - let mut cache = ShufflingCache::new(); + let mut cache = ShufflingCache::default(); for i in 0..MAX_CONCURRENT_PROMISES { cache.create_promise(shuffling_id(i as u64)).unwrap(); diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index 384fcbe5d..b4eabc809 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -500,7 +500,7 @@ async fn unaggregated_attestations_added_to_fork_choice_some_none() { // Move forward a slot so all queued attestations can be processed. harness.advance_slot(); fork_choice - .update_time(harness.chain.slot().unwrap(), &harness.chain.spec) + .update_time(harness.chain.slot().unwrap()) .unwrap(); let validator_slots: Vec<(usize, Slot)> = (0..VALIDATOR_COUNT) @@ -614,7 +614,7 @@ async fn unaggregated_attestations_added_to_fork_choice_all_updated() { // Move forward a slot so all queued attestations can be processed. harness.advance_slot(); fork_choice - .update_time(harness.chain.slot().unwrap(), &harness.chain.spec) + .update_time(harness.chain.slot().unwrap()) .unwrap(); let validators: Vec = (0..VALIDATOR_COUNT).collect(); diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 5e26068f7..020e890c7 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -54,8 +54,8 @@ use system_health::observe_system_health_bn; use tokio::sync::mpsc::{Sender, UnboundedSender}; use tokio_stream::{wrappers::BroadcastStream, StreamExt}; use types::{ - Attestation, AttestationData, AttesterSlashing, BeaconStateError, BlindedPayload, - CommitteeCache, ConfigAndPreset, Epoch, EthSpec, ForkName, FullPayload, + Attestation, AttestationData, AttestationShufflingId, AttesterSlashing, BeaconStateError, + BlindedPayload, CommitteeCache, ConfigAndPreset, Epoch, EthSpec, ForkName, FullPayload, ProposerPreparationData, ProposerSlashing, RelativeEpoch, SignedAggregateAndProof, SignedBeaconBlock, SignedBlindedBeaconBlock, SignedBlsToExecutionChange, SignedContributionAndProof, SignedValidatorRegistrationData, SignedVoluntaryExit, Slot, @@ -784,39 +784,112 @@ pub fn serve( let current_epoch = state.current_epoch(); let epoch = query.epoch.unwrap_or(current_epoch); - let committee_cache = - match RelativeEpoch::from_epoch(current_epoch, epoch) { - Ok(relative_epoch) - if state - .committee_cache_is_initialized(relative_epoch) => - { - state.committee_cache(relative_epoch).map(Cow::Borrowed) - } - _ => CommitteeCache::initialized(state, epoch, &chain.spec) + // Attempt to obtain the committee_cache from the beacon chain + let decision_slot = (epoch.saturating_sub(2u64)) + .end_slot(T::EthSpec::slots_per_epoch()); + // Find the decision block and skip to another method on any kind + // of failure + let shuffling_id = if let Ok(Some(shuffling_decision_block)) = + chain.block_root_at_slot(decision_slot, WhenSlotSkipped::Prev) + { + Some(AttestationShufflingId { + shuffling_epoch: epoch, + shuffling_decision_block, + }) + } else { + None + }; + + // Attempt to read from the chain cache if there exists a + // shuffling_id + let maybe_cached_shuffling = if let Some(shuffling_id) = + shuffling_id.as_ref() + { + chain + .shuffling_cache + .try_write_for(std::time::Duration::from_secs(1)) + .and_then(|mut cache_write| cache_write.get(shuffling_id)) + .and_then(|cache_item| cache_item.wait().ok()) + } else { + None + }; + + let committee_cache = if let Some(ref shuffling) = + maybe_cached_shuffling + { + Cow::Borrowed(&**shuffling) + } else { + let possibly_built_cache = + match RelativeEpoch::from_epoch(current_epoch, epoch) { + Ok(relative_epoch) + if state.committee_cache_is_initialized( + relative_epoch, + ) => + { + state + .committee_cache(relative_epoch) + .map(Cow::Borrowed) + } + _ => CommitteeCache::initialized( + state, + epoch, + &chain.spec, + ) .map(Cow::Owned), - } - .map_err(|e| match e { - BeaconStateError::EpochOutOfBounds => { - let max_sprp = - T::EthSpec::slots_per_historical_root() as u64; - let first_subsequent_restore_point_slot = ((epoch - .start_slot(T::EthSpec::slots_per_epoch()) - / max_sprp) - + 1) - * max_sprp; - if epoch < current_epoch { - warp_utils::reject::custom_bad_request(format!( - "epoch out of bounds, try state at slot {}", - first_subsequent_restore_point_slot, - )) - } else { - warp_utils::reject::custom_bad_request( - "epoch out of bounds, too far in future".into(), - ) + } + .map_err(|e| { + match e { + BeaconStateError::EpochOutOfBounds => { + let max_sprp = + T::EthSpec::slots_per_historical_root() + as u64; + let first_subsequent_restore_point_slot = + ((epoch.start_slot( + T::EthSpec::slots_per_epoch(), + ) / max_sprp) + + 1) + * max_sprp; + if epoch < current_epoch { + warp_utils::reject::custom_bad_request( + format!( + "epoch out of bounds, \ + try state at slot {}", + first_subsequent_restore_point_slot, + ), + ) + } else { + warp_utils::reject::custom_bad_request( + "epoch out of bounds, \ + too far in future" + .into(), + ) + } + } + _ => { + warp_utils::reject::beacon_chain_error(e.into()) + } + } + })?; + + // Attempt to write to the beacon cache (only if the cache + // size is not the default value). + if chain.config.shuffling_cache_size + != beacon_chain::shuffling_cache::DEFAULT_CACHE_SIZE + { + if let Some(shuffling_id) = shuffling_id { + if let Some(mut cache_write) = chain + .shuffling_cache + .try_write_for(std::time::Duration::from_secs(1)) + { + cache_write.insert_committee_cache( + shuffling_id, + &*possibly_built_cache, + ); } } - _ => warp_utils::reject::beacon_chain_error(e.into()), - })?; + } + possibly_built_cache + }; // Use either the supplied slot or all slots in the epoch. let slots = diff --git a/beacon_node/lighthouse_network/Cargo.toml b/beacon_node/lighthouse_network/Cargo.toml index 2ec8baaf5..dda797187 100644 --- a/beacon_node/lighthouse_network/Cargo.toml +++ b/beacon_node/lighthouse_network/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Sigma Prime "] edition = "2021" [dependencies] -discv5 = { version = "0.1.0", features = ["libp2p"] } +discv5 = { version = "0.2.2", features = ["libp2p"] } unsigned-varint = { version = "0.6.0", features = ["codec"] } types = { path = "../../consensus/types" } eth2_ssz_types = "0.2.2" diff --git a/beacon_node/lighthouse_network/src/discovery/mod.rs b/beacon_node/lighthouse_network/src/discovery/mod.rs index b9c4e76fe..dda68aff9 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/beacon_node/lighthouse_network/src/discovery/mod.rs @@ -177,6 +177,13 @@ pub struct Discovery { /// always false. pub started: bool, + /// This keeps track of whether an external UDP port change should also indicate an internal + /// TCP port change. As we cannot detect our external TCP port, we assume that the external UDP + /// port is also our external TCP port. This assumption only holds if the user has not + /// explicitly set their ENR TCP port via the CLI config. The first indicates tcp4 and the + /// second indicates tcp6. + update_tcp_port: (bool, bool), + /// Logger for the discovery behaviour. log: slog::Logger, } @@ -197,6 +204,7 @@ impl Discovery { }; let local_enr = network_globals.local_enr.read().clone(); + let local_node_id = local_enr.node_id(); info!(log, "ENR Initialised"; "enr" => local_enr.to_base64(), "seq" => local_enr.seq(), "id"=> %local_enr.node_id(), "ip4" => ?local_enr.ip4(), "udp4"=> ?local_enr.udp4(), "tcp4" => ?local_enr.tcp6() @@ -217,6 +225,10 @@ impl Discovery { // Add bootnodes to routing table for bootnode_enr in config.boot_nodes_enr.clone() { + if bootnode_enr.node_id() == local_node_id { + // If we are a boot node, ignore adding it to the routing table + continue; + } debug!( log, "Adding node to routing table"; @@ -295,6 +307,11 @@ impl Discovery { } } + let update_tcp_port = ( + config.enr_tcp4_port.is_none(), + config.enr_tcp6_port.is_none(), + ); + Ok(Self { cached_enrs: LruCache::new(50), network_globals, @@ -304,6 +321,7 @@ impl Discovery { discv5, event_stream, started: !config.disable_discovery, + update_tcp_port, log, enr_dir, }) @@ -1014,6 +1032,13 @@ impl NetworkBehaviour for Discovery { metrics::check_nat(); // Discv5 will have updated our local ENR. We save the updated version // to disk. + + if (self.update_tcp_port.0 && socket_addr.is_ipv4()) + || (self.update_tcp_port.1 && socket_addr.is_ipv6()) + { + // Update the TCP port in the ENR + self.discv5.update_local_enr_socket(socket_addr, true); + } let enr = self.discv5.local_enr(); enr::save_enr_to_disk(Path::new(&self.enr_dir), &enr, &self.log); // update network globals diff --git a/beacon_node/lighthouse_network/src/metrics.rs b/beacon_node/lighthouse_network/src/metrics.rs index aed68e27f..58cc99201 100644 --- a/beacon_node/lighthouse_network/src/metrics.rs +++ b/beacon_node/lighthouse_network/src/metrics.rs @@ -167,7 +167,8 @@ pub fn check_nat() { } pub fn scrape_discovery_metrics() { - let metrics = discv5::metrics::Metrics::from(discv5::Discv5::raw_metrics()); + let metrics = + discv5::metrics::Metrics::from(discv5::Discv5::::raw_metrics()); set_float_gauge(&DISCOVERY_REQS, metrics.unsolicited_requests_per_second); set_gauge(&DISCOVERY_SESSIONS, metrics.active_sessions as i64); set_gauge(&DISCOVERY_SENT_BYTES, metrics.bytes_sent as i64); diff --git a/beacon_node/network/src/beacon_processor/work_reprocessing_queue.rs b/beacon_node/network/src/beacon_processor/work_reprocessing_queue.rs index 4d0bdc002..0e4a08f5d 100644 --- a/beacon_node/network/src/beacon_processor/work_reprocessing_queue.rs +++ b/beacon_node/network/src/beacon_processor/work_reprocessing_queue.rs @@ -572,6 +572,9 @@ impl ReprocessQueue { }) => { // Unqueue the attestations we have for this root, if any. if let Some(queued_ids) = self.awaiting_attestations_per_root.remove(&block_root) { + let mut sent_count = 0; + let mut failed_to_send_count = 0; + for id in queued_ids { metrics::inc_counter( &metrics::BEACON_PROCESSOR_REPROCESSING_QUEUE_MATCHED_ATTESTATIONS, @@ -596,10 +599,9 @@ impl ReprocessQueue { // Send the work. if self.ready_work_tx.try_send(work).is_err() { - error!( - log, - "Failed to send scheduled attestation"; - ); + failed_to_send_count += 1; + } else { + sent_count += 1; } } else { // There is a mismatch between the attestation ids registered for this @@ -612,6 +614,18 @@ impl ReprocessQueue { ); } } + + if failed_to_send_count > 0 { + error!( + log, + "Ignored scheduled attestation(s) for block"; + "hint" => "system may be overloaded", + "parent_root" => ?parent_root, + "block_root" => ?block_root, + "failed_count" => failed_to_send_count, + "sent_count" => sent_count, + ); + } } // Unqueue the light client optimistic updates we have for this root, if any. if let Some(queued_lc_id) = self @@ -726,7 +740,9 @@ impl ReprocessQueue { if self.ready_work_tx.try_send(work).is_err() { error!( log, - "Failed to send scheduled attestation"; + "Ignored scheduled attestation"; + "hint" => "system may be overloaded", + "beacon_block_root" => ?root ); } diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index d708d145f..083330753 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -377,6 +377,14 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { address of this server (e.g., http://localhost:5054).") .takes_value(true), ) + .arg( + Arg::with_name("shuffling-cache-size") + .long("shuffling-cache-size") + .help("Some HTTP API requests can be optimised by caching the shufflings at each epoch. \ + This flag allows the user to set the shuffling cache size in epochs. \ + Shufflings are dependent on validator count and setting this value to a large number can consume a large amount of memory.") + .takes_value(true) + ) /* * Monitoring metrics @@ -1000,8 +1008,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("count-unrealized") .long("count-unrealized") .hidden(true) - .help("Enables an alternative, potentially more performant FFG \ - vote tracking method.") + .help("This flag is deprecated and has no effect.") .takes_value(true) .default_value("true") ) @@ -1009,7 +1016,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("count-unrealized-full") .long("count-unrealized-full") .hidden(true) - .help("Stricter version of `count-unrealized`.") + .help("This flag is deprecated and has no effect.") .takes_value(true) .default_value("false") ) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index c0bf83a0f..b9bdf1e96 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -149,6 +149,10 @@ pub fn get_config( client_config.http_api.allow_sync_stalled = true; } + if let Some(cache_size) = clap_utils::parse_optional(cli_args, "shuffling-cache-size")? { + client_config.chain.shuffling_cache_size = cache_size; + } + /* * Prometheus metrics HTTP server */ @@ -742,10 +746,21 @@ pub fn get_config( client_config.chain.fork_choice_before_proposal_timeout_ms = timeout; } - client_config.chain.count_unrealized = - clap_utils::parse_required(cli_args, "count-unrealized")?; - client_config.chain.count_unrealized_full = - clap_utils::parse_required::(cli_args, "count-unrealized-full")?.into(); + if !clap_utils::parse_required::(cli_args, "count-unrealized")? { + warn!( + log, + "The flag --count-unrealized is deprecated and will be removed"; + "info" => "any use of the flag will have no effect" + ); + } + + if clap_utils::parse_required::(cli_args, "count-unrealized-full")? { + warn!( + log, + "The flag --count-unrealized-full is deprecated and will be removed"; + "info" => "setting it to `true` has no effect" + ); + } client_config.chain.always_reset_payload_statuses = cli_args.is_present("reset-payload-statuses"); diff --git a/beacon_node/store/src/metadata.rs b/beacon_node/store/src/metadata.rs index 92117254f..6d27f0cc8 100644 --- a/beacon_node/store/src/metadata.rs +++ b/beacon_node/store/src/metadata.rs @@ -4,7 +4,7 @@ use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use types::{Checkpoint, Hash256, Slot}; -pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(15); +pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(16); // All the keys that get stored under the `BeaconMeta` column. // diff --git a/book/src/advanced_networking.md b/book/src/advanced_networking.md index fb7f07a51..08d276ba3 100644 --- a/book/src/advanced_networking.md +++ b/book/src/advanced_networking.md @@ -41,7 +41,7 @@ drastically and use the (recommended) default. ### NAT Traversal (Port Forwarding) -Lighthouse, by default, used port 9000 for both TCP and UDP. Lighthouse will +Lighthouse, by default, uses port 9000 for both TCP and UDP. Lighthouse will still function if it is behind a NAT without any port mappings. Although Lighthouse still functions, we recommend that some mechanism is used to ensure that your Lighthouse node is publicly accessible. This will typically improve @@ -54,6 +54,16 @@ node will inform you of established routes in this case). If UPnP is not enabled, we recommend you manually set up port mappings to both of Lighthouse's TCP and UDP ports (9000 by default). +> Note: Lighthouse needs to advertise its publicly accessible ports in +> order to inform its peers that it is contactable and how to connect to it. +> Lighthouse has an automated way of doing this for the UDP port. This means +> Lighthouse can detect its external UDP port. There is no such mechanism for the +> TCP port. As such, we assume that the external UDP and external TCP port is the +> same (i.e external 5050 UDP/TCP mapping to internal 9000 is fine). If you are setting up differing external UDP and TCP ports, you should +> explicitly specify them using the `--enr-tcp-port` and `--enr-udp-port` as +> explained in the following section. + + ### ENR Configuration Lighthouse has a number of CLI parameters for constructing and modifying the diff --git a/book/src/faq.md b/book/src/faq.md index 5bfae3fa8..43de40eee 100644 --- a/book/src/faq.md +++ b/book/src/faq.md @@ -128,8 +128,9 @@ same `datadir` as a previous network. I.e if you have been running the `datadir` (the `datadir` is also printed out in the beacon node's logs on boot-up). -If you find yourself with a low peer count and is not reaching the target you -expect. Try setting up the correct port forwards as described [here](./advanced_networking.md#nat-traversal-port-forwarding). +If you find yourself with a low peer count and it's not reaching the target you +expect. Try setting up the correct port forwards as described +[here](./advanced_networking.md#nat-traversal-port-forwarding). ### What should I do if I lose my slashing protection database? diff --git a/boot_node/Cargo.toml b/boot_node/Cargo.toml index 266dcdfe6..b18d38ccd 100644 --- a/boot_node/Cargo.toml +++ b/boot_node/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "boot_node" -version = "3.5.1" +version = "4.0.1-rc.0" authors = ["Sigma Prime "] edition = "2021" diff --git a/boot_node/src/server.rs b/boot_node/src/server.rs index 8f38fb300..3f5419c2c 100644 --- a/boot_node/src/server.rs +++ b/boot_node/src/server.rs @@ -44,7 +44,7 @@ pub async fn run(config: BootNodeConfig, log: slog::Logger) { info!(log, "Contact information"; "multiaddrs" => ?local_enr.multiaddr_p2p()); // construct the discv5 server - let mut discv5 = Discv5::new(local_enr.clone(), local_key, discv5_config).unwrap(); + let mut discv5: Discv5 = Discv5::new(local_enr.clone(), local_key, discv5_config).unwrap(); // If there are any bootnodes add them to the routing table for enr in boot_nodes { diff --git a/common/eth2_network_config/Cargo.toml b/common/eth2_network_config/Cargo.toml index 5ad374b8c..042bf9b1c 100644 --- a/common/eth2_network_config/Cargo.toml +++ b/common/eth2_network_config/Cargo.toml @@ -20,4 +20,4 @@ types = { path = "../../consensus/types"} kzg = { path = "../../crypto/kzg" } eth2_ssz = "0.4.1" eth2_config = { path = "../eth2_config"} -enr = { version = "0.6.2", features = ["ed25519", "k256"] } +discv5 = "0.2.2" diff --git a/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml b/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml index 83e6de790..b5ce752b7 100644 --- a/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml +++ b/common/eth2_network_config/built_in_network_configs/mainnet/config.yaml @@ -38,7 +38,7 @@ BELLATRIX_FORK_VERSION: 0x02000000 BELLATRIX_FORK_EPOCH: 144896 # Sept 6, 2022, 11:34:47am UTC # Capella CAPELLA_FORK_VERSION: 0x03000000 -CAPELLA_FORK_EPOCH: 18446744073709551615 +CAPELLA_FORK_EPOCH: 194048 # April 12, 2023, 10:27:35pm UTC # Eip4844 EIP4844_FORK_VERSION: 0x04000000 EIP4844_FORK_EPOCH: 18446744073709551615 diff --git a/common/eth2_network_config/src/lib.rs b/common/eth2_network_config/src/lib.rs index 1a1d2a432..3221a5cd6 100644 --- a/common/eth2_network_config/src/lib.rs +++ b/common/eth2_network_config/src/lib.rs @@ -11,7 +11,7 @@ //! To add a new built-in testnet, add it to the `define_hardcoded_nets` invocation in the `eth2_config` //! crate. -use enr::{CombinedKey, Enr}; +use discv5::enr::{CombinedKey, Enr}; use eth2_config::{instantiate_hardcoded_nets, HardcodedNet}; use kzg::TrustedSetup; use std::fs::{create_dir_all, File}; diff --git a/common/lighthouse_version/src/lib.rs b/common/lighthouse_version/src/lib.rs index 10d1a8c32..f4e19e796 100644 --- a/common/lighthouse_version/src/lib.rs +++ b/common/lighthouse_version/src/lib.rs @@ -17,8 +17,8 @@ pub const VERSION: &str = git_version!( // NOTE: using --match instead of --exclude for compatibility with old Git "--match=thiswillnevermatchlol" ], - prefix = "Lighthouse/v3.5.1-", - fallback = "Lighthouse/v3.5.1" + prefix = "Lighthouse/v4.0.1-rc.0-", + fallback = "Lighthouse/v4.0.1-rc.0" ); /// Returns `VERSION`, but with platform information appended to the end. diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index dcb734a17..d717c31aa 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1,6 +1,6 @@ use crate::{ForkChoiceStore, InvalidationOperation}; use proto_array::{ - Block as ProtoBlock, CountUnrealizedFull, ExecutionStatus, ProposerHeadError, ProposerHeadInfo, + Block as ProtoBlock, ExecutionStatus, ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, }; use slog::{crit, debug, warn, Logger}; @@ -187,51 +187,6 @@ impl CountUnrealized { pub fn is_true(&self) -> bool { matches!(self, CountUnrealized::True) } - - pub fn and(&self, other: CountUnrealized) -> CountUnrealized { - if self.is_true() && other.is_true() { - CountUnrealized::True - } else { - CountUnrealized::False - } - } -} - -impl From for CountUnrealized { - fn from(count_unrealized: bool) -> Self { - if count_unrealized { - CountUnrealized::True - } else { - CountUnrealized::False - } - } -} - -#[derive(Copy, Clone)] -enum UpdateJustifiedCheckpointSlots { - OnTick { - current_slot: Slot, - }, - OnBlock { - state_slot: Slot, - current_slot: Slot, - }, -} - -impl UpdateJustifiedCheckpointSlots { - fn current_slot(&self) -> Slot { - match self { - UpdateJustifiedCheckpointSlots::OnTick { current_slot } => *current_slot, - UpdateJustifiedCheckpointSlots::OnBlock { current_slot, .. } => *current_slot, - } - } - - fn state_slot(&self) -> Option { - match self { - UpdateJustifiedCheckpointSlots::OnTick { .. } => None, - UpdateJustifiedCheckpointSlots::OnBlock { state_slot, .. } => Some(*state_slot), - } - } } /// Indicates if a block has been verified by an execution payload. @@ -393,7 +348,6 @@ where anchor_block: &SignedBeaconBlock, anchor_state: &BeaconState, current_slot: Option, - count_unrealized_full_config: CountUnrealizedFull, spec: &ChainSpec, ) -> Result> { // Sanity check: the anchor must lie on an epoch boundary. @@ -440,7 +394,6 @@ where current_epoch_shuffling_id, next_epoch_shuffling_id, execution_status, - count_unrealized_full_config, )?; let mut fork_choice = Self { @@ -533,7 +486,7 @@ where // Provide the slot (as per the system clock) to the `fc_store` and then return its view of // the current slot. The `fc_store` will ensure that the `current_slot` is never // decreasing, a property which we must maintain. - let current_slot = self.update_time(system_time_current_slot, spec)?; + let current_slot = self.update_time(system_time_current_slot)?; let store = &mut self.fc_store; @@ -654,58 +607,6 @@ where } } - /// Returns `true` if the given `store` should be updated to set - /// `state.current_justified_checkpoint` its `justified_checkpoint`. - /// - /// ## Specification - /// - /// Is equivalent to: - /// - /// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/fork-choice.md#should_update_justified_checkpoint - fn should_update_justified_checkpoint( - &mut self, - new_justified_checkpoint: Checkpoint, - slots: UpdateJustifiedCheckpointSlots, - spec: &ChainSpec, - ) -> Result> { - self.update_time(slots.current_slot(), spec)?; - - if compute_slots_since_epoch_start::(self.fc_store.get_current_slot()) - < spec.safe_slots_to_update_justified - { - return Ok(true); - } - - let justified_slot = - compute_start_slot_at_epoch::(self.fc_store.justified_checkpoint().epoch); - - // This sanity check is not in the spec, but the invariant is implied. - if let Some(state_slot) = slots.state_slot() { - if justified_slot >= state_slot { - return Err(Error::AttemptToRevertJustification { - store: justified_slot, - state: state_slot, - }); - } - } - - // We know that the slot for `new_justified_checkpoint.root` is not greater than - // `state.slot`, since a state cannot justify its own slot. - // - // We know that `new_justified_checkpoint.root` is an ancestor of `state`, since a `state` - // only ever justifies ancestors. - // - // A prior `if` statement protects against a justified_slot that is greater than - // `state.slot` - let justified_ancestor = - self.get_ancestor(new_justified_checkpoint.root, justified_slot)?; - if justified_ancestor != Some(self.fc_store.justified_checkpoint().root) { - return Ok(false); - } - - Ok(true) - } - /// See `ProtoArrayForkChoice::process_execution_payload_validation` for documentation. pub fn on_valid_execution_payload( &mut self, @@ -759,7 +660,7 @@ where // Provide the slot (as per the system clock) to the `fc_store` and then return its view of // the current slot. The `fc_store` will ensure that the `current_slot` is never // decreasing, a property which we must maintain. - let current_slot = self.update_time(system_time_current_slot, spec)?; + let current_slot = self.update_time(system_time_current_slot)?; // Parent block must be known. let parent_block = self @@ -814,17 +715,10 @@ where self.fc_store.set_proposer_boost_root(block_root); } - let update_justified_checkpoint_slots = UpdateJustifiedCheckpointSlots::OnBlock { - state_slot: state.slot(), - current_slot, - }; - // Update store with checkpoints if necessary self.update_checkpoints( state.current_justified_checkpoint(), state.finalized_checkpoint(), - update_justified_checkpoint_slots, - spec, )?; // Update unrealized justified/finalized checkpoints. @@ -908,11 +802,9 @@ where // If block is from past epochs, try to update store's justified & finalized checkpoints right away if block.slot().epoch(E::slots_per_epoch()) < current_slot.epoch(E::slots_per_epoch()) { - self.update_checkpoints( + self.pull_up_store_checkpoints( unrealized_justified_checkpoint, unrealized_finalized_checkpoint, - update_justified_checkpoint_slots, - spec, )?; } @@ -1007,29 +899,19 @@ where &mut self, justified_checkpoint: Checkpoint, finalized_checkpoint: Checkpoint, - slots: UpdateJustifiedCheckpointSlots, - spec: &ChainSpec, ) -> Result<(), Error> { // Update justified checkpoint. if justified_checkpoint.epoch > self.fc_store.justified_checkpoint().epoch { - if justified_checkpoint.epoch > self.fc_store.best_justified_checkpoint().epoch { - self.fc_store - .set_best_justified_checkpoint(justified_checkpoint); - } - if self.should_update_justified_checkpoint(justified_checkpoint, slots, spec)? { - self.fc_store - .set_justified_checkpoint(justified_checkpoint) - .map_err(Error::UnableToSetJustifiedCheckpoint)?; - } + self.fc_store + .set_justified_checkpoint(justified_checkpoint) + .map_err(Error::UnableToSetJustifiedCheckpoint)?; } // Update finalized checkpoint. if finalized_checkpoint.epoch > self.fc_store.finalized_checkpoint().epoch { self.fc_store.set_finalized_checkpoint(finalized_checkpoint); - self.fc_store - .set_justified_checkpoint(justified_checkpoint) - .map_err(Error::UnableToSetJustifiedCheckpoint)?; } + Ok(()) } @@ -1170,9 +1052,8 @@ where system_time_current_slot: Slot, attestation: &IndexedAttestation, is_from_block: AttestationFromBlock, - spec: &ChainSpec, ) -> Result<(), Error> { - self.update_time(system_time_current_slot, spec)?; + self.update_time(system_time_current_slot)?; // Ignore any attestations to the zero hash. // @@ -1233,16 +1114,12 @@ where /// Call `on_tick` for all slots between `fc_store.get_current_slot()` and the provided /// `current_slot`. Returns the value of `self.fc_store.get_current_slot`. - pub fn update_time( - &mut self, - current_slot: Slot, - spec: &ChainSpec, - ) -> Result> { + pub fn update_time(&mut self, current_slot: Slot) -> Result> { while self.fc_store.get_current_slot() < current_slot { let previous_slot = self.fc_store.get_current_slot(); // Note: we are relying upon `on_tick` to update `fc_store.time` to ensure we don't // get stuck in a loop. - self.on_tick(previous_slot + 1, spec)? + self.on_tick(previous_slot + 1)? } // Process any attestations that might now be eligible. @@ -1258,7 +1135,7 @@ where /// Equivalent to: /// /// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/fork-choice.md#on_tick - fn on_tick(&mut self, time: Slot, spec: &ChainSpec) -> Result<(), Error> { + fn on_tick(&mut self, time: Slot) -> Result<(), Error> { let store = &mut self.fc_store; let previous_slot = store.get_current_slot(); @@ -1286,26 +1163,27 @@ where return Ok(()); } - if store.best_justified_checkpoint().epoch > store.justified_checkpoint().epoch { - let store = &self.fc_store; - if self.is_finalized_checkpoint_or_descendant(store.best_justified_checkpoint().root) { - let store = &mut self.fc_store; - store - .set_justified_checkpoint(*store.best_justified_checkpoint()) - .map_err(Error::ForkChoiceStoreError)?; - } - } - - // Update store.justified_checkpoint if a better unrealized justified checkpoint is known + // Update the justified/finalized checkpoints based upon the + // best-observed unrealized justification/finality. let unrealized_justified_checkpoint = *self.fc_store.unrealized_justified_checkpoint(); let unrealized_finalized_checkpoint = *self.fc_store.unrealized_finalized_checkpoint(); + self.pull_up_store_checkpoints( + unrealized_justified_checkpoint, + unrealized_finalized_checkpoint, + )?; + + Ok(()) + } + + fn pull_up_store_checkpoints( + &mut self, + unrealized_justified_checkpoint: Checkpoint, + unrealized_finalized_checkpoint: Checkpoint, + ) -> Result<(), Error> { self.update_checkpoints( unrealized_justified_checkpoint, unrealized_finalized_checkpoint, - UpdateJustifiedCheckpointSlots::OnTick { current_slot }, - spec, - )?; - Ok(()) + ) } /// Processes and removes from the queue any queued attestations which may now be eligible for @@ -1471,16 +1349,6 @@ where *self.fc_store.justified_checkpoint() } - /// Return the best justified checkpoint. - /// - /// ## Warning - /// - /// This is distinct to the "justified checkpoint" or the "current justified checkpoint". This - /// "best justified checkpoint" value should only be used internally or for testing. - pub fn best_justified_checkpoint(&self) -> Checkpoint { - *self.fc_store.best_justified_checkpoint() - } - pub fn unrealized_justified_checkpoint(&self) -> Checkpoint { *self.fc_store.unrealized_justified_checkpoint() } @@ -1541,13 +1409,11 @@ where pub fn proto_array_from_persisted( persisted: &PersistedForkChoice, reset_payload_statuses: ResetPayloadStatuses, - count_unrealized_full: CountUnrealizedFull, spec: &ChainSpec, log: &Logger, ) -> Result> { - let mut proto_array = - ProtoArrayForkChoice::from_bytes(&persisted.proto_array_bytes, count_unrealized_full) - .map_err(Error::InvalidProtoArrayBytes)?; + let mut proto_array = ProtoArrayForkChoice::from_bytes(&persisted.proto_array_bytes) + .map_err(Error::InvalidProtoArrayBytes)?; let contains_invalid_payloads = proto_array.contains_invalid_payloads(); debug!( @@ -1578,7 +1444,7 @@ where "error" => e, "info" => "please report this error", ); - ProtoArrayForkChoice::from_bytes(&persisted.proto_array_bytes, count_unrealized_full) + ProtoArrayForkChoice::from_bytes(&persisted.proto_array_bytes) .map_err(Error::InvalidProtoArrayBytes) } else { debug!( @@ -1595,17 +1461,11 @@ where persisted: PersistedForkChoice, reset_payload_statuses: ResetPayloadStatuses, fc_store: T, - count_unrealized_full: CountUnrealizedFull, spec: &ChainSpec, log: &Logger, ) -> Result> { - let proto_array = Self::proto_array_from_persisted( - &persisted, - reset_payload_statuses, - count_unrealized_full, - spec, - log, - )?; + let proto_array = + Self::proto_array_from_persisted(&persisted, reset_payload_statuses, spec, log)?; let current_slot = fc_store.get_current_slot(); diff --git a/consensus/fork_choice/src/fork_choice_store.rs b/consensus/fork_choice/src/fork_choice_store.rs index 9500b1c7d..320f10141 100644 --- a/consensus/fork_choice/src/fork_choice_store.rs +++ b/consensus/fork_choice/src/fork_choice_store.rs @@ -47,9 +47,6 @@ pub trait ForkChoiceStore: Sized { /// Returns balances from the `state` identified by `justified_checkpoint.root`. fn justified_balances(&self) -> &JustifiedBalances; - /// Returns the `best_justified_checkpoint`. - fn best_justified_checkpoint(&self) -> &Checkpoint; - /// Returns the `finalized_checkpoint`. fn finalized_checkpoint(&self) -> &Checkpoint; @@ -68,9 +65,6 @@ pub trait ForkChoiceStore: Sized { /// Sets the `justified_checkpoint`. fn set_justified_checkpoint(&mut self, checkpoint: Checkpoint) -> Result<(), Self::Error>; - /// Sets the `best_justified_checkpoint`. - fn set_best_justified_checkpoint(&mut self, checkpoint: Checkpoint); - /// Sets the `unrealized_justified_checkpoint`. fn set_unrealized_justified_checkpoint(&mut self, checkpoint: Checkpoint); diff --git a/consensus/fork_choice/src/lib.rs b/consensus/fork_choice/src/lib.rs index b307c66d8..397a2ff89 100644 --- a/consensus/fork_choice/src/lib.rs +++ b/consensus/fork_choice/src/lib.rs @@ -7,6 +7,4 @@ pub use crate::fork_choice::{ PersistedForkChoice, QueuedAttestation, ResetPayloadStatuses, }; pub use fork_choice_store::ForkChoiceStore; -pub use proto_array::{ - Block as ProtoBlock, CountUnrealizedFull, ExecutionStatus, InvalidationOperation, -}; +pub use proto_array::{Block as ProtoBlock, ExecutionStatus, InvalidationOperation}; diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 00bd1f763..82bf642f1 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -104,16 +104,6 @@ impl ForkChoiceTest { self } - /// Assert the epochs match. - pub fn assert_best_justified_epoch(self, epoch: u64) -> Self { - assert_eq!( - self.get(|fc_store| fc_store.best_justified_checkpoint().epoch), - Epoch::new(epoch), - "best_justified_epoch" - ); - self - } - /// Assert the given slot is greater than the head slot. pub fn assert_finalized_epoch_is_less_than(self, epoch: Epoch) -> Self { assert!(self.harness.finalized_checkpoint().epoch < epoch); @@ -151,7 +141,7 @@ impl ForkChoiceTest { .chain .canonical_head .fork_choice_write_lock() - .update_time(self.harness.chain.slot().unwrap(), &self.harness.spec) + .update_time(self.harness.chain.slot().unwrap()) .unwrap(); func( self.harness @@ -241,6 +231,11 @@ impl ForkChoiceTest { /// /// If the chain is presently in an unsafe period, transition through it and the following safe /// period. + /// + /// Note: the `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` variable has been removed + /// from the fork choice spec in Q1 2023. We're still leaving references to + /// it in our tests because (a) it's easier and (b) it allows us to easily + /// test for the absence of that parameter. pub fn move_to_next_unsafe_period(self) -> Self { self.move_inside_safe_to_update() .move_outside_safe_to_update() @@ -534,7 +529,6 @@ async fn justified_checkpoint_updates_with_descendent_outside_safe_slots() { .unwrap() .move_outside_safe_to_update() .assert_justified_epoch(2) - .assert_best_justified_epoch(2) .apply_blocks(1) .await .assert_justified_epoch(3); @@ -551,11 +545,9 @@ async fn justified_checkpoint_updates_first_justification_outside_safe_to_update .unwrap() .move_to_next_unsafe_period() .assert_justified_epoch(0) - .assert_best_justified_epoch(0) .apply_blocks(1) .await - .assert_justified_epoch(2) - .assert_best_justified_epoch(2); + .assert_justified_epoch(2); } /// - The new justified checkpoint **does not** descend from the current. @@ -583,8 +575,7 @@ async fn justified_checkpoint_updates_with_non_descendent_inside_safe_slots_with .unwrap(); }) .await - .assert_justified_epoch(3) - .assert_best_justified_epoch(3); + .assert_justified_epoch(3); } /// - The new justified checkpoint **does not** descend from the current. @@ -612,8 +603,9 @@ async fn justified_checkpoint_updates_with_non_descendent_outside_safe_slots_wit .unwrap(); }) .await - .assert_justified_epoch(2) - .assert_best_justified_epoch(3); + // Now that `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` has been removed, the new + // block should have updated the justified checkpoint. + .assert_justified_epoch(3); } /// - The new justified checkpoint **does not** descend from the current. @@ -641,8 +633,7 @@ async fn justified_checkpoint_updates_with_non_descendent_outside_safe_slots_wit .unwrap(); }) .await - .assert_justified_epoch(3) - .assert_best_justified_epoch(3); + .assert_justified_epoch(3); } /// Check that the balances are obtained correctly. diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index 68b3fb719..157f072ad 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -3,7 +3,6 @@ mod ffg_updates; mod no_votes; mod votes; -use crate::proto_array::CountUnrealizedFull; use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice}; use crate::{InvalidationOperation, JustifiedBalances}; use serde_derive::{Deserialize, Serialize}; @@ -88,7 +87,6 @@ impl ForkChoiceTestDefinition { junk_shuffling_id.clone(), junk_shuffling_id, ExecutionStatus::Optimistic(ExecutionBlockHash::zero()), - CountUnrealizedFull::default(), ) .expect("should create fork choice struct"); let equivocating_indices = BTreeSet::new(); @@ -307,8 +305,8 @@ fn get_checkpoint(i: u64) -> Checkpoint { fn check_bytes_round_trip(original: &ProtoArrayForkChoice) { let bytes = original.as_bytes(); - let decoded = ProtoArrayForkChoice::from_bytes(&bytes, CountUnrealizedFull::default()) - .expect("fork choice should decode from bytes"); + let decoded = + ProtoArrayForkChoice::from_bytes(&bytes).expect("fork choice should decode from bytes"); assert!( *original == decoded, "fork choice should encode and decode without change" diff --git a/consensus/proto_array/src/justified_balances.rs b/consensus/proto_array/src/justified_balances.rs index 75f6c2f7c..c8787817f 100644 --- a/consensus/proto_array/src/justified_balances.rs +++ b/consensus/proto_array/src/justified_balances.rs @@ -24,7 +24,7 @@ impl JustifiedBalances { .validators() .iter() .map(|validator| { - if validator.is_active_at(current_epoch) { + if !validator.slashed && validator.is_active_at(current_epoch) { total_effective_balance.safe_add_assign(validator.effective_balance)?; num_active_validators.safe_add_assign(1)?; diff --git a/consensus/proto_array/src/lib.rs b/consensus/proto_array/src/lib.rs index f2b29e1c7..e84139345 100644 --- a/consensus/proto_array/src/lib.rs +++ b/consensus/proto_array/src/lib.rs @@ -6,9 +6,7 @@ mod proto_array_fork_choice; mod ssz_container; pub use crate::justified_balances::JustifiedBalances; -pub use crate::proto_array::{ - calculate_committee_fraction, CountUnrealizedFull, InvalidationOperation, -}; +pub use crate::proto_array::{calculate_committee_fraction, InvalidationOperation}; pub use crate::proto_array_fork_choice::{ Block, DoNotReOrg, ExecutionStatus, ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index bf50c0802..2c19206cb 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -118,24 +118,6 @@ impl Default for ProposerBoost { } } -/// Indicate whether we should strictly count unrealized justification/finalization votes. -#[derive(Default, PartialEq, Eq, Debug, Serialize, Deserialize, Copy, Clone)] -pub enum CountUnrealizedFull { - True, - #[default] - False, -} - -impl From for CountUnrealizedFull { - fn from(b: bool) -> Self { - if b { - CountUnrealizedFull::True - } else { - CountUnrealizedFull::False - } - } -} - #[derive(PartialEq, Debug, Serialize, Deserialize, Clone)] pub struct ProtoArray { /// Do not attempt to prune the tree unless it has at least this many nodes. Small prunes @@ -146,7 +128,6 @@ pub struct ProtoArray { pub nodes: Vec, pub indices: HashMap, pub previous_proposer_boost: ProposerBoost, - pub count_unrealized_full: CountUnrealizedFull, } impl ProtoArray { @@ -684,9 +665,9 @@ impl ProtoArray { start_root: *justified_root, justified_checkpoint: self.justified_checkpoint, finalized_checkpoint: self.finalized_checkpoint, - head_root: justified_node.root, - head_justified_checkpoint: justified_node.justified_checkpoint, - head_finalized_checkpoint: justified_node.finalized_checkpoint, + head_root: best_node.root, + head_justified_checkpoint: best_node.justified_checkpoint, + head_finalized_checkpoint: best_node.finalized_checkpoint, }))); } @@ -900,55 +881,44 @@ impl ProtoArray { } let genesis_epoch = Epoch::new(0); - - let checkpoint_match_predicate = - |node_justified_checkpoint: Checkpoint, node_finalized_checkpoint: Checkpoint| { - let correct_justified = node_justified_checkpoint == self.justified_checkpoint - || self.justified_checkpoint.epoch == genesis_epoch; - let correct_finalized = node_finalized_checkpoint == self.finalized_checkpoint - || self.finalized_checkpoint.epoch == genesis_epoch; - correct_justified && correct_finalized + let current_epoch = current_slot.epoch(E::slots_per_epoch()); + let node_epoch = node.slot.epoch(E::slots_per_epoch()); + let node_justified_checkpoint = + if let Some(justified_checkpoint) = node.justified_checkpoint { + justified_checkpoint + } else { + // The node does not have any information about the justified + // checkpoint. This indicates an inconsistent proto-array. + return false; }; - if let ( - Some(unrealized_justified_checkpoint), - Some(unrealized_finalized_checkpoint), - Some(justified_checkpoint), - Some(finalized_checkpoint), - ) = ( - node.unrealized_justified_checkpoint, - node.unrealized_finalized_checkpoint, - node.justified_checkpoint, - node.finalized_checkpoint, - ) { - let current_epoch = current_slot.epoch(E::slots_per_epoch()); - - // If previous epoch is justified, pull up all tips to at least the previous epoch - if CountUnrealizedFull::True == self.count_unrealized_full - && (current_epoch > genesis_epoch - && self.justified_checkpoint.epoch + 1 == current_epoch) - { - unrealized_justified_checkpoint.epoch + 1 >= current_epoch - // If previous epoch is not justified, pull up only tips from past epochs up to the current epoch - } else { - // If block is from a previous epoch, filter using unrealized justification & finalization information - if node.slot.epoch(E::slots_per_epoch()) < current_epoch { - checkpoint_match_predicate( - unrealized_justified_checkpoint, - unrealized_finalized_checkpoint, - ) - // If block is from the current epoch, filter using the head state's justification & finalization information - } else { - checkpoint_match_predicate(justified_checkpoint, finalized_checkpoint) - } - } - } else if let (Some(justified_checkpoint), Some(finalized_checkpoint)) = - (node.justified_checkpoint, node.finalized_checkpoint) - { - checkpoint_match_predicate(justified_checkpoint, finalized_checkpoint) + let voting_source = if current_epoch > node_epoch { + // The block is from a prior epoch, the voting source will be pulled-up. + node.unrealized_justified_checkpoint + // Sometimes we don't track the unrealized justification. In + // that case, just use the fully-realized justified checkpoint. + .unwrap_or(node_justified_checkpoint) } else { - false + // The block is not from a prior epoch, therefore the voting source + // is not pulled up. + node_justified_checkpoint + }; + + let mut correct_justified = self.justified_checkpoint.epoch == genesis_epoch + || voting_source.epoch == self.justified_checkpoint.epoch; + + if let Some(node_unrealized_justified_checkpoint) = node.unrealized_justified_checkpoint { + if !correct_justified && self.justified_checkpoint.epoch + 1 == current_epoch { + correct_justified = node_unrealized_justified_checkpoint.epoch + >= self.justified_checkpoint.epoch + && voting_source.epoch + 2 >= current_epoch; + } } + + let correct_finalized = self.finalized_checkpoint.epoch == genesis_epoch + || self.is_finalized_checkpoint_or_descendant::(node.root); + + correct_justified && correct_finalized } /// Return a reverse iterator over the nodes which comprise the chain ending at `block_root`. diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 0e0d806e7..eae54e734 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1,8 +1,8 @@ use crate::{ error::Error, proto_array::{ - calculate_committee_fraction, CountUnrealizedFull, InvalidationOperation, Iter, - ProposerBoost, ProtoArray, ProtoNode, + calculate_committee_fraction, InvalidationOperation, Iter, ProposerBoost, ProtoArray, + ProtoNode, }, ssz_container::SszContainer, JustifiedBalances, @@ -307,7 +307,6 @@ impl ProtoArrayForkChoice { current_epoch_shuffling_id: AttestationShufflingId, next_epoch_shuffling_id: AttestationShufflingId, execution_status: ExecutionStatus, - count_unrealized_full: CountUnrealizedFull, ) -> Result { let mut proto_array = ProtoArray { prune_threshold: DEFAULT_PRUNE_THRESHOLD, @@ -316,7 +315,6 @@ impl ProtoArrayForkChoice { nodes: Vec::with_capacity(1), indices: HashMap::with_capacity(1), previous_proposer_boost: ProposerBoost::default(), - count_unrealized_full, }; let block = Block { @@ -780,13 +778,10 @@ impl ProtoArrayForkChoice { SszContainer::from(self).as_ssz_bytes() } - pub fn from_bytes( - bytes: &[u8], - count_unrealized_full: CountUnrealizedFull, - ) -> Result { + pub fn from_bytes(bytes: &[u8]) -> Result { let container = SszContainer::from_ssz_bytes(bytes) .map_err(|e| format!("Failed to decode ProtoArrayForkChoice: {:?}", e))?; - (container, count_unrealized_full) + container .try_into() .map_err(|e| format!("Failed to initialize ProtoArrayForkChoice: {e:?}")) } @@ -950,7 +945,6 @@ mod test_compute_deltas { junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, - CountUnrealizedFull::default(), ) .unwrap(); @@ -1076,7 +1070,6 @@ mod test_compute_deltas { junk_shuffling_id.clone(), junk_shuffling_id.clone(), execution_status, - CountUnrealizedFull::default(), ) .unwrap(); diff --git a/consensus/proto_array/src/ssz_container.rs b/consensus/proto_array/src/ssz_container.rs index 1a20ef967..ed1efaae1 100644 --- a/consensus/proto_array/src/ssz_container.rs +++ b/consensus/proto_array/src/ssz_container.rs @@ -1,6 +1,6 @@ use crate::proto_array::ProposerBoost; use crate::{ - proto_array::{CountUnrealizedFull, ProtoArray, ProtoNode}, + proto_array::{ProtoArray, ProtoNode}, proto_array_fork_choice::{ElasticList, ProtoArrayForkChoice, VoteTracker}, Error, JustifiedBalances, }; @@ -43,12 +43,10 @@ impl From<&ProtoArrayForkChoice> for SszContainer { } } -impl TryFrom<(SszContainer, CountUnrealizedFull)> for ProtoArrayForkChoice { +impl TryFrom for ProtoArrayForkChoice { type Error = Error; - fn try_from( - (from, count_unrealized_full): (SszContainer, CountUnrealizedFull), - ) -> Result { + fn try_from(from: SszContainer) -> Result { let proto_array = ProtoArray { prune_threshold: from.prune_threshold, justified_checkpoint: from.justified_checkpoint, @@ -56,7 +54,6 @@ impl TryFrom<(SszContainer, CountUnrealizedFull)> for ProtoArrayForkChoice { nodes: from.nodes, indices: from.indices.into_iter().collect::>(), previous_proposer_boost: from.previous_proposer_boost, - count_unrealized_full, }; Ok(Self { diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 1f947c9e7..61d46b6cc 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -633,7 +633,7 @@ impl ChainSpec { * Capella hard fork params */ capella_fork_version: [0x03, 00, 00, 00], - capella_fork_epoch: None, + capella_fork_epoch: Some(Epoch::new(194048)), max_validators_per_withdrawals_sweep: 16384, /* diff --git a/lcli/Cargo.toml b/lcli/Cargo.toml index cd530109e..9d48e85b1 100644 --- a/lcli/Cargo.toml +++ b/lcli/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "lcli" description = "Lighthouse CLI (modeled after zcli)" -version = "3.5.1" +version = "4.0.1-rc.0" authors = ["Paul Hauner "] edition = "2021" diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index 23f38d3c1..849e2fc99 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lighthouse" -version = "3.5.1" +version = "4.0.1-rc.0" authors = ["Sigma Prime "] edition = "2021" autotests = false diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 7f7aca120..8ea89f49d 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -1,4 +1,4 @@ -use beacon_node::{beacon_chain::CountUnrealizedFull, ClientConfig as Config}; +use beacon_node::ClientConfig as Config; use crate::exec::{CommandLineTestExec, CompletedTest}; use beacon_node::beacon_chain::chain_config::{ @@ -118,6 +118,26 @@ fn disable_lock_timeouts_flag() { .with_config(|config| assert!(!config.chain.enable_lock_timeouts)); } +#[test] +fn shuffling_cache_default() { + CommandLineTest::new() + .run_with_zero_port() + .with_config(|config| { + assert_eq!( + config.chain.shuffling_cache_size, + beacon_node::beacon_chain::shuffling_cache::DEFAULT_CACHE_SIZE + ) + }); +} + +#[test] +fn shuffling_cache_set() { + CommandLineTest::new() + .flag("shuffling-cache-size", Some("500")) + .run_with_zero_port() + .with_config(|config| assert_eq!(config.chain.shuffling_cache_size, 500)); +} + #[test] fn fork_choice_before_proposal_timeout_default() { CommandLineTest::new() @@ -212,74 +232,58 @@ fn paranoid_block_proposal_on() { .with_config(|config| assert!(config.chain.paranoid_block_proposal)); } -#[test] -fn count_unrealized_default() { - CommandLineTest::new() - .run_with_zero_port() - .with_config(|config| assert!(config.chain.count_unrealized)); -} - #[test] fn count_unrealized_no_arg() { CommandLineTest::new() .flag("count-unrealized", None) - .run_with_zero_port() - .with_config(|config| assert!(config.chain.count_unrealized)); + // This flag should be ignored, so there's nothing to test but that the + // client starts with the flag present. + .run_with_zero_port(); } #[test] fn count_unrealized_false() { CommandLineTest::new() .flag("count-unrealized", Some("false")) - .run_with_zero_port() - .with_config(|config| assert!(!config.chain.count_unrealized)); + // This flag should be ignored, so there's nothing to test but that the + // client starts with the flag present. + .run_with_zero_port(); } #[test] fn count_unrealized_true() { CommandLineTest::new() .flag("count-unrealized", Some("true")) - .run_with_zero_port() - .with_config(|config| assert!(config.chain.count_unrealized)); + // This flag should be ignored, so there's nothing to test but that the + // client starts with the flag present. + .run_with_zero_port(); } #[test] fn count_unrealized_full_no_arg() { CommandLineTest::new() .flag("count-unrealized-full", None) - .run_with_zero_port() - .with_config(|config| { - assert_eq!( - config.chain.count_unrealized_full, - CountUnrealizedFull::False - ) - }); + // This flag should be ignored, so there's nothing to test but that the + // client starts with the flag present. + .run_with_zero_port(); } #[test] fn count_unrealized_full_false() { CommandLineTest::new() .flag("count-unrealized-full", Some("false")) - .run_with_zero_port() - .with_config(|config| { - assert_eq!( - config.chain.count_unrealized_full, - CountUnrealizedFull::False - ) - }); + // This flag should be ignored, so there's nothing to test but that the + // client starts with the flag present. + .run_with_zero_port(); } #[test] fn count_unrealized_full_true() { CommandLineTest::new() .flag("count-unrealized-full", Some("true")) - .run_with_zero_port() - .with_config(|config| { - assert_eq!( - config.chain.count_unrealized_full, - CountUnrealizedFull::True - ) - }); + // This flag should be ignored, so there's nothing to test but that the + // client starts with the flag present. + .run_with_zero_port(); } #[test] diff --git a/testing/ef_tests/Makefile b/testing/ef_tests/Makefile index 1feba41c8..f7562f477 100644 --- a/testing/ef_tests/Makefile +++ b/testing/ef_tests/Makefile @@ -1,4 +1,4 @@ -TESTS_TAG := v1.3.0-rc.1 +TESTS_TAG := v1.3.0-rc.4 TESTS = general minimal mainnet TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS)) diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 8b4d0caf5..0e8c9a9ae 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -45,7 +45,6 @@ pub struct Checks { justified_checkpoint: Option, justified_checkpoint_root: Option, finalized_checkpoint: Option, - best_justified_checkpoint: Option, u_justified_checkpoint: Option, u_finalized_checkpoint: Option, proposer_boost_root: Option, @@ -229,7 +228,6 @@ impl Case for ForkChoiceTest { justified_checkpoint, justified_checkpoint_root, finalized_checkpoint, - best_justified_checkpoint, u_justified_checkpoint, u_finalized_checkpoint, proposer_boost_root, @@ -260,11 +258,6 @@ impl Case for ForkChoiceTest { tester.check_finalized_checkpoint(*expected_finalized_checkpoint)?; } - if let Some(expected_best_justified_checkpoint) = best_justified_checkpoint { - tester - .check_best_justified_checkpoint(*expected_best_justified_checkpoint)?; - } - if let Some(expected_u_justified_checkpoint) = u_justified_checkpoint { tester.check_u_justified_checkpoint(*expected_u_justified_checkpoint)?; } @@ -378,7 +371,7 @@ impl Tester { .chain .canonical_head .fork_choice_write_lock() - .update_time(slot, &self.spec) + .update_time(slot) .unwrap(); } @@ -388,7 +381,7 @@ impl Tester { let result = self.block_on_dangerous(self.harness.chain.process_block( block_root, block.clone(), - CountUnrealized::False, + CountUnrealized::True, NotifyExecutionLayer::Yes, ))?; if result.is_ok() != valid { @@ -448,7 +441,7 @@ impl Tester { &state, PayloadVerificationStatus::Irrelevant, &self.harness.chain.spec, - self.harness.chain.config.count_unrealized.into(), + CountUnrealized::True, ); if result.is_ok() { @@ -576,23 +569,6 @@ impl Tester { check_equal("finalized_checkpoint", fc_checkpoint, expected_checkpoint) } - pub fn check_best_justified_checkpoint( - &self, - expected_checkpoint: Checkpoint, - ) -> Result<(), Error> { - let best_justified_checkpoint = self - .harness - .chain - .canonical_head - .fork_choice_read_lock() - .best_justified_checkpoint(); - check_equal( - "best_justified_checkpoint", - best_justified_checkpoint, - expected_checkpoint, - ) - } - pub fn check_u_justified_checkpoint( &self, expected_checkpoint: Checkpoint, diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index 3ed81977f..9a20c3a3e 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -555,6 +555,11 @@ impl Handler for ForkChoiceHandler { return false; } + // Tests are no longer generated for the base/phase0 specification. + if fork_name == ForkName::Base { + return false; + } + // These tests check block validity (which may include signatures) and there is no need to // run them with fake crypto. cfg!(not(feature = "fake_crypto")) diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 7590fc84b..fe2c5f796 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -528,6 +528,18 @@ fn fork_choice_ex_ante() { ForkChoiceHandler::::new("ex_ante").run(); } +#[test] +fn fork_choice_reorg() { + ForkChoiceHandler::::new("reorg").run(); + // There is no mainnet variant for this test. +} + +#[test] +fn fork_choice_withholding() { + ForkChoiceHandler::::new("withholding").run(); + // There is no mainnet variant for this test. +} + #[test] fn optimistic_sync() { OptimisticSyncHandler::::default().run();