merge queue: embarking unstable (f08e8f5) and [#5281 + #5270 + #5266 + #5275] together

beacon_node/beacon_chain/src/builder.rs

@@ -12,7 +12,7 @@ use crate::light_client_server_cache::LightClientServerCache;
 use crate::migrate::{BackgroundMigrator, MigratorConfig};
 use crate::persisted_beacon_chain::PersistedBeaconChain;
 use crate::shuffling_cache::{BlockShufflingIds, ShufflingCache};
-use crate::snapshot_cache::{SnapshotCache, DEFAULT_SNAPSHOT_CACHE_SIZE};
+use crate::snapshot_cache::SnapshotCache;
 use crate::timeout_rw_lock::TimeoutRwLock;
 use crate::validator_monitor::{ValidatorMonitor, ValidatorMonitorConfig};
 use crate::validator_pubkey_cache::ValidatorPubkeyCache;
@@ -870,6 +870,7 @@ where
         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 snapshot_cache_size = self.chain_config.snapshot_cache_size;
 
         // Calculate the weak subjectivity point in which to backfill blocks to.
         let genesis_backfill_slot = if self.chain_config.genesis_backfill {
@@ -946,7 +947,7 @@ where
             event_handler: self.event_handler,
             head_tracker,
             snapshot_cache: TimeoutRwLock::new(SnapshotCache::new(
-                DEFAULT_SNAPSHOT_CACHE_SIZE,
+                snapshot_cache_size,
                 head_for_snapshot_cache,
             )),
             shuffling_cache: TimeoutRwLock::new(ShufflingCache::new(

beacon_node/beacon_chain/src/chain_config.rs

@@ -72,6 +72,8 @@ pub struct ChainConfig {
     pub optimistic_finalized_sync: bool,
     /// The size of the shuffling cache,
     pub shuffling_cache_size: usize,
+    /// The size of the snapshot cache.
+    pub snapshot_cache_size: usize,
     /// If using a weak-subjectivity sync, whether we should download blocks all the way back to
     /// genesis.
     pub genesis_backfill: bool,
@@ -112,6 +114,7 @@ impl Default for ChainConfig {
             // This value isn't actually read except in tests.
             optimistic_finalized_sync: true,
             shuffling_cache_size: crate::shuffling_cache::DEFAULT_CACHE_SIZE,
+            snapshot_cache_size: crate::snapshot_cache::DEFAULT_SNAPSHOT_CACHE_SIZE,
             genesis_backfill: false,
             always_prepare_payload: false,
             progressive_balances_mode: ProgressiveBalancesMode::Fast,

beacon_node/beacon_chain/src/lib.rs

@@ -50,7 +50,7 @@ mod pre_finalization_cache;
 pub mod proposer_prep_service;
 pub mod schema_change;
 pub mod shuffling_cache;
-mod snapshot_cache;
+pub mod snapshot_cache;
 pub mod state_advance_timer;
 pub mod sync_committee_rewards;
 pub mod sync_committee_verification;

beacon_node/beacon_chain/src/snapshot_cache.rs

@@ -9,7 +9,7 @@ use types::{
 };
 
 /// The default size of the cache.
-pub const DEFAULT_SNAPSHOT_CACHE_SIZE: usize = 4;
+pub const DEFAULT_SNAPSHOT_CACHE_SIZE: usize = 3;
 
 /// The minimum block delay to clone the state in the cache instead of removing it.
 /// This helps keep block processing fast during re-orgs from late blocks.
@@ -174,6 +174,7 @@ impl<T: EthSpec> SnapshotCache<T> {
         self.snapshots.iter().map(|s| s.beacon_block_root).collect()
     }
 
+    #[allow(clippy::len_without_is_empty)]
     /// The number of snapshots contained in `self`.
     pub fn len(&self) -> usize {
         self.snapshots.len()

beacon_node/execution_layer/src/engine_api/new_payload_request.rs

@@ -86,7 +86,7 @@ impl<'block, E: EthSpec> NewPayloadRequest<'block, E> {
     ///
     /// https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.2/specs/deneb/beacon-chain.md#modified-verify_and_notify_new_payload
     pub fn perform_optimistic_sync_verifications(&self) -> Result<(), Error> {
-        self.verfiy_payload_block_hash()?;
+        self.verify_payload_block_hash()?;
         self.verify_versioned_hashes()?;
 
         Ok(())
@@ -98,7 +98,7 @@ impl<'block, E: EthSpec> NewPayloadRequest<'block, E> {
     ///
     /// Equivalent to `is_valid_block_hash` in the spec:
     /// https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.2/specs/deneb/beacon-chain.md#is_valid_block_hash
-    pub fn verfiy_payload_block_hash(&self) -> Result<(), Error> {
+    pub fn verify_payload_block_hash(&self) -> Result<(), Error> {
         let payload = self.execution_payload_ref();
         let parent_beacon_block_root = self.parent_beacon_block_root().ok().cloned();
 

beacon_node/execution_layer/src/lib.rs

@@ -7,6 +7,7 @@
 use crate::payload_cache::PayloadCache;
 use arc_swap::ArcSwapOption;
 use auth::{strip_prefix, Auth, JwtKey};
+pub use block_hash::calculate_execution_block_hash;
 use builder_client::BuilderHttpClient;
 pub use engine_api::EngineCapabilities;
 use engine_api::Error as ApiError;

beacon_node/src/cli.rs

@@ -622,6 +622,13 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                 .help("Specifies how many states from the freezer database should cache in memory [default: 1]")
                 .takes_value(true)
         )
+        .arg(
+            Arg::with_name("state-cache-size")
+                .long("state-cache-size")
+                .value_name("STATE_CACHE_SIZE")
+                .help("Specifies the size of the snapshot cache [default: 3]")
+                .takes_value(true)
+        )
         /*
          * Execution Layer Integration
          */

beacon_node/src/config.rs

@@ -170,6 +170,9 @@ pub fn get_config<E: EthSpec>(
     if let Some(cache_size) = clap_utils::parse_optional(cli_args, "shuffling-cache-size")? {
         client_config.chain.shuffling_cache_size = cache_size;
     }
+    if let Some(cache_size) = clap_utils::parse_optional(cli_args, "state-cache-size")? {
+        client_config.chain.snapshot_cache_size = cache_size;
+    }
 
     /*
      * Prometheus metrics HTTP server

book/src/help_bn.md

@@ -461,6 +461,9 @@ OPTIONS:
         --slots-per-restore-point <SLOT_COUNT>
             Specifies how often a freezer DB restore point should be stored. Cannot be changed after initialization.
             [default: 8192 (mainnet) or 64 (minimal)]
+        --state-cache-size <STATE_CACHE_SIZE>
+            Specifies the size of the snapshot cache [default: 3]
+
         --suggested-fee-recipient <SUGGESTED-FEE-RECIPIENT>
             Emergency fallback fee recipient for use in case the validator client does not have one configured. You
             should set this flag on the validator client instead of (or in addition to) setting it here.

lighthouse/tests/beacon_node.rs

@@ -23,8 +23,11 @@ use types::{
     Address, Checkpoint, Epoch, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec,
     ProgressiveBalancesMode,
 };
+use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port};
 
 const DEFAULT_ETH1_ENDPOINT: &str = "http://localhost:8545/";
+
+// These dummy ports should ONLY be used for `enr-xxx-port` flags that do not bind.
 const DUMMY_ENR_TCP_PORT: u16 = 7777;
 const DUMMY_ENR_UDP_PORT: u16 = 8888;
 const DUMMY_ENR_QUIC_PORT: u16 = 9999;
@@ -172,6 +175,26 @@ fn shuffling_cache_set() {
         .with_config(|config| assert_eq!(config.chain.shuffling_cache_size, 500));
 }
 
+#[test]
+fn snapshot_cache_default() {
+    CommandLineTest::new()
+        .run_with_zero_port()
+        .with_config(|config| {
+            assert_eq!(
+                config.chain.snapshot_cache_size,
+                beacon_node::beacon_chain::snapshot_cache::DEFAULT_SNAPSHOT_CACHE_SIZE
+            )
+        });
+}
+
+#[test]
+fn snapshot_cache_set() {
+    CommandLineTest::new()
+        .flag("state-cache-size", Some("500"))
+        .run_with_zero_port()
+        .with_config(|config| assert_eq!(config.chain.snapshot_cache_size, 500));
+}
+
 #[test]
 fn fork_choice_before_proposal_timeout_default() {
     CommandLineTest::new()
@@ -871,7 +894,7 @@ fn network_port_flag_over_ipv4() {
             );
         });
 
-    let port = 9000;
+    let port = unused_tcp4_port().expect("Unable to find unused port.");
     CommandLineTest::new()
         .flag("port", Some(port.to_string().as_str()))
         .flag("allow-insecure-genesis-sync", None)
@@ -908,7 +931,7 @@ fn network_port_flag_over_ipv6() {
             );
         });
 
-    let port = 9000;
+    let port = unused_tcp4_port().expect("Unable to find unused port.");
     CommandLineTest::new()
         .flag("listen-address", Some("::1"))
         .flag("port", Some(port.to_string().as_str()))
@@ -958,8 +981,8 @@ fn network_port_flag_over_ipv4_and_ipv6() {
             );
         });
 
-    let port = 19000;
-    let port6 = 29000;
+    let port = unused_tcp4_port().expect("Unable to find unused port.");
+    let port6 = unused_tcp6_port().expect("Unable to find unused port.");
     CommandLineTest::new()
         .flag("listen-address", Some("127.0.0.1"))
         .flag("listen-address", Some("::1"))
@@ -1300,9 +1323,8 @@ fn enr_tcp6_port_flag() {
 fn enr_match_flag_over_ipv4() {
     let addr = "127.0.0.2".parse::<Ipv4Addr>().unwrap();
 
-    // the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports).
-    let udp4_port = DUMMY_ENR_UDP_PORT;
-    let tcp4_port = DUMMY_ENR_TCP_PORT;
+    let udp4_port = unused_udp4_port().expect("Unable to find unused port.");
+    let tcp4_port = unused_tcp4_port().expect("Unable to find unused port.");
 
     CommandLineTest::new()
         .flag("enr-match", None)
@@ -1332,9 +1354,8 @@ fn enr_match_flag_over_ipv6() {
     const ADDR: &str = "::1";
     let addr = ADDR.parse::<Ipv6Addr>().unwrap();
 
-    // the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports).
-    let udp6_port = DUMMY_ENR_UDP_PORT;
-    let tcp6_port = DUMMY_ENR_TCP_PORT;
+    let udp6_port = unused_udp6_port().expect("Unable to find unused port.");
+    let tcp6_port = unused_tcp6_port().expect("Unable to find unused port.");
 
     CommandLineTest::new()
         .flag("enr-match", None)
@@ -1363,15 +1384,13 @@ fn enr_match_flag_over_ipv6() {
 fn enr_match_flag_over_ipv4_and_ipv6() {
     const IPV6_ADDR: &str = "::1";
 
-    // the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports).
-    let udp6_port = DUMMY_ENR_UDP_PORT;
-    let tcp6_port = DUMMY_ENR_TCP_PORT;
+    let udp6_port = unused_udp6_port().expect("Unable to find unused port.");
+    let tcp6_port = unused_tcp6_port().expect("Unable to find unused port.");
     let ipv6_addr = IPV6_ADDR.parse::<Ipv6Addr>().unwrap();
 
     const IPV4_ADDR: &str = "127.0.0.1";
-    // the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports).
-    let udp4_port = DUMMY_ENR_UDP_PORT;
-    let tcp4_port = DUMMY_ENR_TCP_PORT;
+    let udp4_port = unused_udp4_port().expect("Unable to find unused port.");
+    let tcp4_port = unused_tcp4_port().expect("Unable to find unused port.");
     let ipv4_addr = IPV4_ADDR.parse::<Ipv4Addr>().unwrap();
 
     CommandLineTest::new()