diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 400ca77711..369783b7f8 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -501,13 +501,38 @@ fn disable_run_on_all() { }); } +/// Tests for validator fallback parameter flags. #[test] -fn sync_tolerance_flag() { +fn beacon_node_sync_tolerance_flag() { CommandLineTest::new() - .flag("beacon-node-sync-tolerance", Some("8")) + .flag("beacon-node-sync-tolerance", Some("4")) .run() .with_config(|config| { - assert_eq!(config.beacon_node_fallback.sync_tolerance, Some(8)); + assert_eq!(config.beacon_node_fallback.sync_tolerance, Some(4)); + }); +} +#[test] +fn beacon_node_small_sync_distance_modifier_flag() { + CommandLineTest::new() + .flag("beacon-node-small-sync-distance-modifer", Some("16")) + .run() + .with_config(|config| { + assert_eq!( + config.beacon_node_fallback.small_sync_distance_modifier, + Some(16) + ); + }); +} +#[test] +fn beacon_node_medium_sync_distance_modifier_flag() { + CommandLineTest::new() + .flag("beacon-node-medium-sync-distance-modifer", Some("32")) + .run() + .with_config(|config| { + assert_eq!( + config.beacon_node_fallback.medium_sync_distance_modifier, + Some(32) + ); }); } diff --git a/validator_client/src/beacon_node_fallback.rs b/validator_client/src/beacon_node_fallback.rs index a380440e9e..5d841791ef 100644 --- a/validator_client/src/beacon_node_fallback.rs +++ b/validator_client/src/beacon_node_fallback.rs @@ -45,6 +45,12 @@ pub struct Config { /// Sets the number of slots behind the head a beacon node is allowed to be to still be /// considered `synced`. pub sync_tolerance: Option, + /// Sets the size of the range of the `small` sync distance tier. This range starts immediately + /// after `sync_tolerance`. + pub small_sync_distance_modifier: Option, + /// Sets the size of the range of the `medium` sync distance tier. This range starts immediately + /// after the `small` range. + pub medium_sync_distance_modifier: Option, } /// Indicates a measurement of latency between the VC and a BN. @@ -628,6 +634,11 @@ mod tests { #[test] fn check_candidate_order() { + // These fields is irrelvant for sorting. They are set to arbitrary values. + let head = Slot::new(99); + let optimistic_status = IsOptimistic::No; + let execution_status = ExecutionEngineHealth::Healthy; + fn new_candidate(id: usize) -> CandidateBeaconNode { let beacon_node = BeaconNodeHttpClient::new( SensitiveUrl::parse(&format!("http://example_{id}.com")).unwrap(), @@ -636,11 +647,6 @@ mod tests { CandidateBeaconNode::new(beacon_node, id) } - // These fields is irrelvant for sorting. They are set to arbitrary values. - let head = Slot::new(99); - let optimistic_status = IsOptimistic::No; - let execution_status = ExecutionEngineHealth::Healthy; - let candidate_1 = new_candidate(1); let expected_candidate_1 = new_candidate(1); let candidate_2 = new_candidate(2); @@ -681,32 +687,32 @@ mod tests { head, optimistic_status, execution_status, - health_tier: BeaconNodeHealthTier::new(3, Slot::new(8), small), + health_tier: BeaconNodeHealthTier::new(3, Slot::new(9), small), }; let health_4 = BeaconNodeHealth { id: 4, head, optimistic_status, execution_status, - health_tier: BeaconNodeHealthTier::new(3, Slot::new(8), small), + health_tier: BeaconNodeHealthTier::new(3, Slot::new(9), small), }; // `health_5` has a smaller sync distance and is outside the `synced` range so should be - // sorted first. + // sorted first. Note the values of `id`. let health_5 = BeaconNodeHealth { - id: 5, - head, - optimistic_status, - execution_status, - health_tier: BeaconNodeHealthTier::new(4, Slot::new(8), small), - }; - let health_6 = BeaconNodeHealth { id: 6, head, optimistic_status, execution_status, health_tier: BeaconNodeHealthTier::new(4, Slot::new(9), small), }; + let health_6 = BeaconNodeHealth { + id: 5, + head, + optimistic_status, + execution_status, + health_tier: BeaconNodeHealthTier::new(4, Slot::new(10), small), + }; *candidate_1.health.write() = Some(health_1); *candidate_2.health.write() = Some(health_2); diff --git a/validator_client/src/beacon_node_health.rs b/validator_client/src/beacon_node_health.rs index 1831c8fa0d..4f8402d2e5 100644 --- a/validator_client/src/beacon_node_health.rs +++ b/validator_client/src/beacon_node_health.rs @@ -6,9 +6,15 @@ use types::Slot; // Sync distances between 0 and DEFAULT_SYNC_TOLERANCE are considered `synced`. // Sync distance tiers are determined by the different modifiers. -const DEFAULT_SYNC_TOLERANCE: Slot = Slot::new(4); -const SYNC_DISTANCE_SMALL_MODIFIER: Slot = Slot::new(7); -const SYNC_DISTANCE_MEDIUM_MODIFIER: Slot = Slot::new(31); +// +// The default range is the following: +// Synced: 0..=8 +// Small: 9..=16 +// Medium: 17..=64 +// Large: 65.. +const DEFAULT_SYNC_TOLERANCE: Slot = Slot::new(8); +const DEFAULT_SMALL_SYNC_DISTANCE_MODIFIER: Slot = Slot::new(8); +const DEFAULT_MEDIUM_SYNC_DISTANCE_MODIFIER: Slot = Slot::new(48); type HealthTier = u8; type SyncDistance = Slot; @@ -23,7 +29,7 @@ pub enum SyncDistanceTier { } /// Contains the different sync distance tiers which are determined at runtime by the -/// `sync_tolerance` CLI flag. +/// `sync_tolerance` CLI flag and the `sync_distance_modifier` flags. #[derive(Clone, Debug)] pub struct BeaconNodeSyncDistanceTiers { synced: SyncDistance, @@ -33,14 +39,23 @@ pub struct BeaconNodeSyncDistanceTiers { impl BeaconNodeSyncDistanceTiers { pub fn from_config(config: &Config) -> Self { - if let Some(sync_tolerance) = config.sync_tolerance { - Self { - synced: Slot::new(sync_tolerance), - small: Slot::new(sync_tolerance) + SYNC_DISTANCE_SMALL_MODIFIER, - medium: Slot::new(sync_tolerance) + SYNC_DISTANCE_MEDIUM_MODIFIER, - } - } else { - Self::default() + let synced = config + .sync_tolerance + .map(Slot::new) + .unwrap_or(DEFAULT_SYNC_TOLERANCE); + let small_mod = config + .small_sync_distance_modifier + .map(Slot::new) + .unwrap_or(DEFAULT_SMALL_SYNC_DISTANCE_MODIFIER); + let medium_mod = config + .medium_sync_distance_modifier + .map(Slot::new) + .unwrap_or(DEFAULT_MEDIUM_SYNC_DISTANCE_MODIFIER); + + Self { + synced, + small: synced + small_mod, + medium: synced + small_mod + medium_mod, } } @@ -69,8 +84,10 @@ impl Default for BeaconNodeSyncDistanceTiers { fn default() -> Self { Self { synced: DEFAULT_SYNC_TOLERANCE, - small: DEFAULT_SYNC_TOLERANCE + SYNC_DISTANCE_SMALL_MODIFIER, - medium: DEFAULT_SYNC_TOLERANCE + SYNC_DISTANCE_MEDIUM_MODIFIER, + small: DEFAULT_SYNC_TOLERANCE + DEFAULT_SMALL_SYNC_DISTANCE_MODIFIER, + medium: DEFAULT_SYNC_TOLERANCE + + DEFAULT_SMALL_SYNC_DISTANCE_MODIFIER + + DEFAULT_MEDIUM_SYNC_DISTANCE_MODIFIER, } } } @@ -284,7 +301,10 @@ impl BeaconNodeHealth { mod tests { use super::ExecutionEngineHealth::{Healthy, Unhealthy}; - use super::{BeaconNodeHealth, BeaconNodeSyncDistanceTiers, IsOptimistic, SyncDistanceTier}; + use super::{ + BeaconNodeHealth, BeaconNodeHealthTier, BeaconNodeSyncDistanceTiers, IsOptimistic, + SyncDistanceTier, + }; use crate::beacon_node_fallback::Config; use slot_clock::{SlotClock, TestingSlotClock}; use std::time::Duration; @@ -302,7 +322,7 @@ mod tests { let mut health_vec = vec![]; - for head_slot in (0..=64).rev() { + for head_slot in 0..=64 { for optimistic_status in &[IsOptimistic::No, IsOptimistic::Yes] { for ee_health in &[Healthy, Unhealthy] { let health = BeaconNodeHealth::from_status( @@ -352,63 +372,73 @@ mod tests { } } - #[test] - fn sync_tolerance() { - let config = Config { - disable_run_on_all: false, - sync_tolerance: Some(8), - }; - let distance_tiers = BeaconNodeSyncDistanceTiers::from_config(&config); + fn new_distance_tier( + distance: u64, + distance_tiers: &BeaconNodeSyncDistanceTiers, + ) -> BeaconNodeHealthTier { + BeaconNodeHealth::compute_health_tier( + Slot::new(distance), + IsOptimistic::No, + Healthy, + distance_tiers, + ) + } - let synced_low = BeaconNodeHealth::compute_health_tier( - Slot::new(0), - IsOptimistic::No, - Healthy, - &distance_tiers, - ); - let synced_high = BeaconNodeHealth::compute_health_tier( - Slot::new(8), - IsOptimistic::No, - Healthy, - &distance_tiers, - ); - let small_low = BeaconNodeHealth::compute_health_tier( - Slot::new(9), - IsOptimistic::No, - Healthy, - &distance_tiers, - ); - let small_high = BeaconNodeHealth::compute_health_tier( - Slot::new(15), - IsOptimistic::No, - Healthy, - &distance_tiers, - ); - let medium_low = BeaconNodeHealth::compute_health_tier( - Slot::new(16), - IsOptimistic::No, - Healthy, - &distance_tiers, - ); - let medium_high = BeaconNodeHealth::compute_health_tier( - Slot::new(39), - IsOptimistic::No, - Healthy, - &distance_tiers, - ); - let large = BeaconNodeHealth::compute_health_tier( - Slot::new(40), - IsOptimistic::No, - Healthy, - &distance_tiers, - ); + #[test] + fn sync_tolerance_default() { + let distance_tiers = BeaconNodeSyncDistanceTiers::default(); + + let synced_low = new_distance_tier(0, &distance_tiers); + let synced_high = new_distance_tier(8, &distance_tiers); + + let small_low = new_distance_tier(9, &distance_tiers); + let small_high = new_distance_tier(16, &distance_tiers); + + let medium_low = new_distance_tier(17, &distance_tiers); + let medium_high = new_distance_tier(64, &distance_tiers); + let large = new_distance_tier(65, &distance_tiers); assert!(synced_low.tier == 1); assert!(synced_high.tier == 1); assert!(small_low.tier == 2); assert!(small_high.tier == 2); assert!(medium_low.tier == 4); - assert!(medium_high.tier == 4); + assert_eq!(medium_high.tier, 4); + assert!(large.tier == 10); + } + + #[test] + fn sync_tolerance_from_config() { + // Config should set the tiers as: + // synced: 0..=4 + // small: 5..=8 + // medium 9..=12 + // large: 13.. + let config = Config { + disable_run_on_all: false, + sync_tolerance: Some(4), + small_sync_distance_modifier: Some(4), + medium_sync_distance_modifier: Some(4), + }; + let distance_tiers = BeaconNodeSyncDistanceTiers::from_config(&config); + + let synced_low = new_distance_tier(0, &distance_tiers); + let synced_high = new_distance_tier(4, &distance_tiers); + + let small_low = new_distance_tier(5, &distance_tiers); + let small_high = new_distance_tier(8, &distance_tiers); + + let medium_low = new_distance_tier(9, &distance_tiers); + let medium_high = new_distance_tier(12, &distance_tiers); + + let large = new_distance_tier(13, &distance_tiers); + + assert!(synced_low.tier == 1); + assert!(synced_high.tier == 1); + assert!(small_low.tier == 2); + assert!(small_high.tier == 2); + assert!(medium_low.tier == 4); + assert_eq!(medium_high.tier, 4); assert!(large.tier == 10); } } diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 012c31ff73..fe7ca3b1d1 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -371,6 +371,24 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { connected Beacon Node.") .takes_value(true), ) + .arg( + Arg::with_name("beacon-node-small-sync-distance-modifier") + .long("beacon-node-small-sync-distance-modifier") + .help("Only use this if you know what you are doing. Incorrectly setting this value \ + can result in suboptimal fallback behaviour. Sets the size (in slots) of the \ + `small` sync distance range when calculating the health tiers of connected \ + Beacon Nodes. The range falls immediately after the end of the `synced` range.") + .takes_value(true) + ) + .arg( + Arg::with_name("beacon-node-medium-sync-distance-modifier") + .long("beacon-node-medium-sync-distance-modifier") + .help("Only use this if you know what you are doing. Incorrectly setting this value \ + can result in suboptimal fallback behaviour. Sets the size (in slots) of the \ + `medium` sync distance range when calculating the health tiers of connected \ + Beacon Nodes. The range falls immediately after the end of the `small` range.") + .takes_value(true) + ) /* * Experimental/development options. */ diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 81e1ac1411..3d53dfa562 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -271,6 +271,24 @@ impl Config { ); } + if let Some(small_modifier) = cli_args.value_of("beacon-node-small-sync-distance-modifier") + { + config.beacon_node_fallback.small_sync_distance_modifier = Some( + small_modifier + .parse::() + .map_err(|_| "beacon-node-small-sync-distance-modifier is not a valid u64.")?, + ); + } + + if let Some(medium_modifier) = + cli_args.value_of("beacon-node-medium-sync-distance-modifier") + { + config.beacon_node_fallback.medium_sync_distance_modifier = + Some(medium_modifier.parse::().map_err(|_| { + "beacon-node-medium-sync-distance-modifier is not a valid u64." + })?); + } + /* * Http API server */