From c4f7745cb25c7261fbfc25d5f185e956f473054f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20Tomi=C4=87?= Date: Wed, 14 Jun 2023 10:48:46 +0000 Subject: [PATCH] fix(backend): Make more operations deterministic --- rs/decentralization/src/lib.rs | 20 ++++++++--------- rs/decentralization/src/nakamoto/mod.rs | 12 +++++----- rs/ic-management-backend/src/health.rs | 6 ++--- rs/ic-management-backend/src/registry.rs | 28 ++++++++++++------------ 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/rs/decentralization/src/lib.rs b/rs/decentralization/src/lib.rs index 30f66c13..a66f6a37 100644 --- a/rs/decentralization/src/lib.rs +++ b/rs/decentralization/src/lib.rs @@ -2,7 +2,7 @@ pub mod nakamoto; pub mod network; use colored::Colorize; use itertools::{EitherOrBoth::*, Itertools}; -use std::collections::{BTreeMap, HashMap}; +use std::collections::BTreeMap; use std::fmt::{Display, Formatter}; use ic_base_types::PrincipalId; @@ -20,11 +20,11 @@ pub struct SubnetChangeResponse { pub motivation: Option, pub comment: Option, pub run_log: Option>, - pub feature_diff: HashMap, + pub feature_diff: BTreeMap, pub proposal_id: Option, } -pub type FeatureDiff = HashMap; +pub type FeatureDiff = BTreeMap; impl SubnetChangeResponse { pub fn with_motivation(self, motivation: String) -> Self { @@ -55,7 +55,7 @@ impl From<&network::SubnetChange> for SubnetChangeResponse { NodeFeature::variants() .into_iter() .map(|f| (f, FeatureDiff::new())) - .collect::>(), + .collect::>(), |mut acc, n| { for f in NodeFeature::variants() { acc.get_mut(&f).unwrap().entry(n.get_feature(&f)).or_insert((0, 0)).0 += 1; @@ -126,25 +126,25 @@ impl Display for SubnetChangeResponse { } )?; - let feature_diff = BTreeMap::from_iter(self.feature_diff.iter()); - let rows = feature_diff.values().map(|diff| diff.len()).max().unwrap_or(0); + let rows = self.feature_diff.values().map(|diff| diff.len()).max().unwrap_or(0); let mut table = tabular::Table::new( - &feature_diff + &self + .feature_diff .keys() .map(|_| " {:<} {:>}") .collect::>() .join(""), ); table.add_row( - feature_diff + self.feature_diff .keys() .fold(tabular::Row::new(), |acc, k| acc.with_cell(k.to_string()).with_cell("")), ); - table.add_row(feature_diff.keys().fold(tabular::Row::new(), |acc, k| { + table.add_row(self.feature_diff.keys().fold(tabular::Row::new(), |acc, k| { acc.with_cell("-".repeat(k.to_string().len())).with_cell("") })); for i in 0..rows { - table.add_row(feature_diff.values().fold(tabular::Row::new(), |acc, v| { + table.add_row(self.feature_diff.values().fold(tabular::Row::new(), |acc, v| { let (value, change) = v .iter() .sorted() diff --git a/rs/decentralization/src/nakamoto/mod.rs b/rs/decentralization/src/nakamoto/mod.rs index 28c59ea6..98c5df27 100644 --- a/rs/decentralization/src/nakamoto/mod.rs +++ b/rs/decentralization/src/nakamoto/mod.rs @@ -3,7 +3,7 @@ use counter::Counter; use itertools::Itertools; use serde::{Deserialize, Serialize}; use std::cmp::Ordering; -use std::collections::{BTreeMap, HashMap}; +use std::collections::BTreeMap; use std::fmt::{Debug, Display, Formatter}; use std::iter::{FromIterator, IntoIterator}; @@ -11,7 +11,7 @@ use ic_management_types::NodeFeature; #[derive(Eq, PartialEq, Clone, Serialize, Deserialize, Debug)] pub struct NodeFeatures { - pub feature_map: HashMap, + pub feature_map: BTreeMap, } impl NodeFeatures { @@ -21,7 +21,7 @@ impl NodeFeatures { #[cfg(test)] fn new_test_feature_set(value: &str) -> Self { - let mut result = HashMap::new(); + let mut result = BTreeMap::new(); for feature in NodeFeature::variants() { result.insert(feature, value.to_string()); } @@ -39,7 +39,7 @@ impl NodeFeatures { impl FromIterator<(NodeFeature, &'static str)> for NodeFeatures { fn from_iter>(iter: I) -> Self { Self { - feature_map: HashMap::from_iter(iter.into_iter().map(|x| (x.0, String::from(x.1)))), + feature_map: BTreeMap::from_iter(iter.into_iter().map(|x| (x.0, String::from(x.1)))), } } } @@ -47,7 +47,7 @@ impl FromIterator<(NodeFeature, &'static str)> for NodeFeatures { impl FromIterator<(NodeFeature, std::string::String)> for NodeFeatures { fn from_iter>(iter: I) -> Self { Self { - feature_map: HashMap::from_iter(iter), + feature_map: BTreeMap::from_iter(iter), } } } @@ -75,7 +75,7 @@ impl NakamotoScore { features_to_nodes_map.insert(feature, Vec::new()); } - // Convert a Vec> into a Vec> into a Vec> for node_features in slice_node_features.iter() { for feature in NodeFeature::variants() { diff --git a/rs/ic-management-backend/src/health.rs b/rs/ic-management-backend/src/health.rs index 58e4357b..6fbdbfdd 100644 --- a/rs/ic-management-backend/src/health.rs +++ b/rs/ic-management-backend/src/health.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, convert::TryInto, str::FromStr}; +use std::{collections::BTreeMap, convert::TryInto, str::FromStr}; use ic_base_types::PrincipalId; use ic_management_types::{Network, Status}; @@ -19,7 +19,7 @@ impl HealthClient { } } - pub async fn subnet(&self, subnet: PrincipalId) -> anyhow::Result> { + pub async fn subnet(&self, subnet: PrincipalId) -> anyhow::Result> { let query: InstantVector = Selector::new() .metric("up") .with("ic", &self.network.legacy_name()) @@ -45,7 +45,7 @@ impl HealthClient { .collect()) } - pub async fn nodes(&self) -> anyhow::Result> { + pub async fn nodes(&self) -> anyhow::Result> { let query: InstantVector = InstantVector(format!( r#" label_replace( diff --git a/rs/ic-management-backend/src/registry.rs b/rs/ic-management-backend/src/registry.rs index 7505c549..6c33294a 100644 --- a/rs/ic-management-backend/src/registry.rs +++ b/rs/ic-management-backend/src/registry.rs @@ -19,7 +19,7 @@ use itertools::Itertools; use std::convert::TryFrom; use std::sync::Arc; use std::{ - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, BTreeSet}, net::Ipv6Addr, }; @@ -80,12 +80,12 @@ impl RegistryEntry for SubnetRecord { } trait RegistryFamilyEntries { - fn get_family_entries(&self) -> Result>; - fn get_family_entries_versioned(&self) -> Result>; + fn get_family_entries(&self) -> Result>; + fn get_family_entries_versioned(&self) -> Result>; } impl RegistryFamilyEntries for LocalRegistry { - fn get_family_entries(&self) -> Result> { + fn get_family_entries(&self) -> Result> { let prefix_length = T::KEY_PREFIX.len(); Ok(self .get_key_family(T::KEY_PREFIX, self.get_latest_version())? @@ -100,10 +100,10 @@ impl RegistryFamilyEntries for LocalRegistry { ) }) }) - .collect::>()) + .collect::>()) } - fn get_family_entries_versioned(&self) -> Result> { + fn get_family_entries_versioned(&self) -> Result> { let prefix_length = T::KEY_PREFIX.len(); Ok(self .get_key_family(T::KEY_PREFIX, self.get_latest_version())? @@ -123,7 +123,7 @@ impl RegistryFamilyEntries for LocalRegistry { }) .unwrap_or_else(|_| panic!("failed to get entry {} for type {}", key, std::any::type_name::())) }) - .collect::>()) + .collect::>()) } } @@ -301,9 +301,9 @@ impl RegistryState { let providers = providers .into_iter() .map(|p| (p.principal_id, p)) - .collect::>(); - let data_center_records: HashMap = self.local_registry.get_family_entries()?; - let operator_records: HashMap = self.local_registry.get_family_entries()?; + .collect::>(); + let data_center_records: BTreeMap = self.local_registry.get_family_entries()?; + let operator_records: BTreeMap = self.local_registry.get_family_entries()?; self.operators = operator_records .iter() @@ -503,7 +503,7 @@ impl RegistryState { self.nodes.clone() } - pub async fn nodes_with_proposals(&self) -> Result> { + pub async fn nodes_with_proposals(&self) -> Result> { let nodes = self.nodes.clone(); let proposal_agent = proposal::ProposalAgent::new(self.nns_url.clone()); @@ -522,7 +522,7 @@ impl RegistryState { .collect()) } - pub async fn subnets_with_proposals(&self) -> Result> { + pub async fn subnets_with_proposals(&self) -> Result> { let subnets = self.subnets.clone(); let proposal_agent = proposal::ProposalAgent::new(self.nns_url.clone()); @@ -555,7 +555,7 @@ impl RegistryState { .unique() .take(NUM_RELEASE_BRANCHES_TO_KEEP) .collect::>(); - let subnet_versions: HashSet = self.subnets.values().map(|s| s.replica_version.clone()).collect(); + let subnet_versions: BTreeSet = self.subnets.values().map(|s| s.replica_version.clone()).collect(); let version_on_unassigned_nodes = self.get_unassigned_nodes_version().await?; Ok(self .replica_releases @@ -645,7 +645,7 @@ impl SubnetQuerier for RegistryState { .to_vec() .iter() .map(|n| self.nodes.get(n).and_then(|n| n.subnet)) - .collect::>(); + .collect::>(); if subnets.len() > 1 { return Err(NetworkError::IllegalRequest( "nodes don't belong to the same subnet".to_string(),