From e368397bf581da7f76b3d79a8bab855c54f466a9 Mon Sep 17 00:00:00 2001 From: Mac L Date: Mon, 5 Feb 2024 16:32:30 +1100 Subject: [PATCH] Use the more descriptive `user_index` instead of `id` --- validator_client/src/beacon_node_fallback.rs | 50 ++++++++++++-------- validator_client/src/beacon_node_health.rs | 20 ++++---- validator_client/src/http_api/mod.rs | 10 +++- validator_client/src/lib.rs | 9 ++-- validator_client/src/notifier.rs | 7 +-- 5 files changed, 57 insertions(+), 39 deletions(-) diff --git a/validator_client/src/beacon_node_fallback.rs b/validator_client/src/beacon_node_fallback.rs index 34230aafbc..f6ee1681bd 100644 --- a/validator_client/src/beacon_node_fallback.rs +++ b/validator_client/src/beacon_node_fallback.rs @@ -147,7 +147,7 @@ pub enum CandidateError { #[derive(Debug, Clone)] pub struct CandidateInfo { - pub id: usize, + pub index: usize, pub node: String, pub health: Option, } @@ -156,7 +156,7 @@ pub struct CandidateInfo { /// for a query. #[derive(Debug)] pub struct CandidateBeaconNode { - pub id: usize, + pub index: usize, pub beacon_node: BeaconNodeHttpClient, pub health: PLRwLock>, _phantom: PhantomData, @@ -164,7 +164,7 @@ pub struct CandidateBeaconNode { impl PartialEq for CandidateBeaconNode { fn eq(&self, other: &Self) -> bool { - self.id == other.id && self.beacon_node == other.beacon_node + self.index == other.index && self.beacon_node == other.beacon_node } } @@ -189,9 +189,9 @@ impl PartialOrd for CandidateBeaconNode { impl CandidateBeaconNode { /// Instantiate a new node. - pub fn new(beacon_node: BeaconNodeHttpClient, id: usize) -> Self { + pub fn new(beacon_node: BeaconNodeHttpClient, index: usize) -> Self { Self { - id, + index, beacon_node, health: PLRwLock::new(Err(CandidateError::Uninitialized)), _phantom: PhantomData, @@ -239,7 +239,7 @@ impl CandidateBeaconNode { }; let new_health = BeaconNodeHealth::from_status( - self.id, + self.index, sync_distance, head, optimistic_status, @@ -408,7 +408,11 @@ impl BeaconNodeFallback { match candidate.health() { Ok(health) => { - if self.distance_tiers.compute_distance_tier(health.health_tier.sync_distance) == SyncDistanceTier::Synced { + if self + .distance_tiers + .compute_distance_tier(health.health_tier.sync_distance) + == SyncDistanceTier::Synced + { num_synced += 1; } num_available += 1; @@ -417,7 +421,11 @@ impl BeaconNodeFallback { Err(_) => continue, } - candidate_info.push(CandidateInfo { id: candidate.id, node: candidate.beacon_node.to_string(), health: health.ok() }); + candidate_info.push(CandidateInfo { + index: candidate.index, + node: candidate.beacon_node.to_string(), + health: health.ok(), + }); } (candidate_info, num_available, num_synced) @@ -680,12 +688,12 @@ mod tests { let optimistic_status = IsOptimistic::No; let execution_status = ExecutionEngineHealth::Healthy; - fn new_candidate(id: usize) -> CandidateBeaconNode { + fn new_candidate(index: usize) -> CandidateBeaconNode { let beacon_node = BeaconNodeHttpClient::new( - SensitiveUrl::parse(&format!("http://example_{id}.com")).unwrap(), - Timeouts::set_all(Duration::from_secs(id as u64)), + SensitiveUrl::parse(&format!("http://example_{index}.com")).unwrap(), + Timeouts::set_all(Duration::from_secs(index as u64)), ); - CandidateBeaconNode::new(beacon_node, id) + CandidateBeaconNode::new(beacon_node, index) } let candidate_1 = new_candidate(1); @@ -705,16 +713,16 @@ mod tests { let small = SyncDistanceTier::Small; // Despite `health_1` having a larger sync distance, it is inside the `synced` range which - // does not tie-break on sync distance and so will tie-break on `id` instead. + // does not tie-break on sync distance and so will tie-break on `user_index` instead. let health_1 = BeaconNodeHealth { - id: 1, + user_index: 1, head, optimistic_status, execution_status, health_tier: BeaconNodeHealthTier::new(1, Slot::new(2), synced), }; let health_2 = BeaconNodeHealth { - id: 2, + user_index: 2, head, optimistic_status, execution_status, @@ -722,16 +730,16 @@ mod tests { }; // `health_3` and `health_4` have the same health tier and sync distance so should - // tie-break on `id`. + // tie-break on `user_index`. let health_3 = BeaconNodeHealth { - id: 3, + user_index: 3, head, optimistic_status, execution_status, health_tier: BeaconNodeHealthTier::new(3, Slot::new(9), small), }; let health_4 = BeaconNodeHealth { - id: 4, + user_index: 4, head, optimistic_status, execution_status, @@ -739,16 +747,16 @@ mod tests { }; // `health_5` has a smaller sync distance and is outside the `synced` range so should be - // sorted first. Note the values of `id`. + // sorted first. Note the values of `user_index`. let health_5 = BeaconNodeHealth { - id: 6, + user_index: 6, head, optimistic_status, execution_status, health_tier: BeaconNodeHealthTier::new(4, Slot::new(9), small), }; let health_6 = BeaconNodeHealth { - id: 5, + user_index: 5, head, optimistic_status, execution_status, diff --git a/validator_client/src/beacon_node_health.rs b/validator_client/src/beacon_node_health.rs index 4761ad2cec..0c060f9e11 100644 --- a/validator_client/src/beacon_node_health.rs +++ b/validator_client/src/beacon_node_health.rs @@ -154,10 +154,10 @@ impl BeaconNodeHealthTier { /// Beacon Node Health metrics. #[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] pub struct BeaconNodeHealth { - // The ID of the Beacon Node. This should correspond with its position in the `--beacon-nodes` - // list. Note that the ID field is used to tie-break nodes with the same health so that nodes - // with a lower ID are preferred. - pub id: usize, + // The index of the Beacon Node. This should correspond with its position in the + // `--beacon-nodes` list. Note that the `user_index` field is used to tie-break nodes with the + // same health so that nodes with a lower index are preferred. + pub user_index: usize, // The slot number of the head. pub head: Slot, // Whether the node is optimistically synced. @@ -173,8 +173,8 @@ impl Ord for BeaconNodeHealth { fn cmp(&self, other: &Self) -> Ordering { let ordering = self.health_tier.cmp(&other.health_tier); if ordering == Ordering::Equal { - // Tie-break node health by ID. - self.id.cmp(&other.id) + // Tie-break node health by `user_index`. + self.user_index.cmp(&other.user_index) } else { ordering } @@ -189,7 +189,7 @@ impl PartialOrd for BeaconNodeHealth { impl BeaconNodeHealth { pub fn from_status( - id: usize, + user_index: usize, sync_distance: Slot, head: Slot, optimistic_status: IsOptimistic, @@ -204,7 +204,7 @@ impl BeaconNodeHealth { ); Self { - id, + user_index, head, optimistic_status, execution_status, @@ -212,8 +212,8 @@ impl BeaconNodeHealth { } } - pub fn get_id(&self) -> usize { - self.id + pub fn get_index(&self) -> usize { + self.user_index } pub fn get_health_tier(&self) -> BeaconNodeHealthTier { diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index ddf7cce3eb..b7e7e790ed 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -441,11 +441,17 @@ pub fn serve( let mut result: HashMap<(usize, String), Result> = HashMap::new(); for node in &*block_filter.beacon_nodes.candidates.read().await { - result.insert((node.id, node.beacon_node.to_string()), *node.health.read()); + result.insert( + (node.index, node.beacon_node.to_string()), + *node.health.read(), + ); } if let Some(proposer_nodes) = &block_filter.proposer_nodes { for node in &*proposer_nodes.candidates.read().await { - result.insert((node.id, node.beacon_node.to_string()), *node.health.read()); + result.insert( + (node.index, node.beacon_node.to_string()), + *node.health.read(), + ); } } diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 1a9972b44d..fce470f22d 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -339,18 +339,21 @@ impl ProductionValidatorClient { .collect::, String>>()?; let num_nodes = beacon_nodes.len(); - + // User order of `beacon_nodes` is preserved, so `index` corresponds to the position of + // the node in `--beacon_nodes`. let candidates = beacon_nodes .into_iter() .enumerate() - .map(|(id, node)| CandidateBeaconNode::new(node, id)) + .map(|(index, node)| CandidateBeaconNode::new(node, index)) .collect(); let proposer_nodes_num = proposer_nodes.len(); + // User order of `proposer_nodes` is preserved, so `index` corresponds to the position of + // the node in `--proposer_nodes`. let proposer_candidates = proposer_nodes .into_iter() .enumerate() - .map(|(id, node)| CandidateBeaconNode::new(node, id)) + .map(|(index, node)| CandidateBeaconNode::new(node, index)) .collect(); // Set the count for beacon node fallbacks excluding the primary beacon node. diff --git a/validator_client/src/notifier.rs b/validator_client/src/notifier.rs index a2c353a5e8..e23989f68e 100644 --- a/validator_client/src/notifier.rs +++ b/validator_client/src/notifier.rs @@ -39,7 +39,8 @@ async fn notify( duties_service: &DutiesService, log: &Logger, ) { - let (candidate_info, num_available, num_synced) = duties_service.beacon_nodes.get_notifier_info().await; + let (candidate_info, num_available, num_synced) = + duties_service.beacon_nodes.get_notifier_info().await; let num_total = candidate_info.len(); let num_synced_fallback = num_synced.saturating_sub(1); @@ -89,7 +90,7 @@ async fn notify( log, "Beacon node info"; "status" => "Connected", - "id" => info.id, + "index" => info.index, "endpoint" => info.node, "head_slot" => %health.head, "is_optimistic" => ?health.optimistic_status, @@ -101,7 +102,7 @@ async fn notify( log, "Beacon node info"; "status" => "Disconnected", - "id" => info.id, + "index" => info.index, "endpoint" => info.node, ); }