Revert EU country handling in decentralization code (#983)

rs/decentralization/src/nakamoto/mod.rs

@@ -960,14 +960,14 @@ mod tests {
 
     #[test]
     fn test_european_subnet_european_nodes_good() {
-        let subnet_initial = new_test_subnet_with_overrides(0, 0, 7, 1, (&NodeFeature::Country, &["EU", "EU", "EU", "EU", "EU", "EU", "CH"]))
+        let subnet_initial = new_test_subnet_with_overrides(0, 0, 7, 1, (&NodeFeature::Country, &["AT", "BE", "DE", "ES", "FR", "IT", "CH"]))
             .with_subnet_id(PrincipalId::from_str("bkfrj-6k62g-dycql-7h53p-atvkj-zg4to-gaogh-netha-ptybj-ntsgw-rqe").unwrap());
         assert_eq!(subnet_initial.check_business_rules().unwrap(), (0, vec![]));
     }
 
     #[test]
     fn test_european_subnet_european_nodes_bad_1() {
-        let subnet_mix = new_test_subnet_with_overrides(1, 0, 7, 1, (&NodeFeature::Country, &["EU", "China", "CH", "EU", "EU", "EU", "EU"]))
+        let subnet_mix = new_test_subnet_with_overrides(1, 0, 7, 1, (&NodeFeature::Country, &["AT", "BE", "DE", "ES", "FR", "IT", "IN"]))
             .with_subnet_id(PrincipalId::from_str("bkfrj-6k62g-dycql-7h53p-atvkj-zg4to-gaogh-netha-ptybj-ntsgw-rqe").unwrap());
         assert_eq!(
             subnet_mix.check_business_rules().unwrap(),
@@ -976,11 +976,11 @@ mod tests {
     }
     #[test]
     fn test_european_subnet_european_nodes_bad_2() {
-        let subnet_mix = new_test_subnet_with_overrides(1, 0, 7, 1, (&NodeFeature::Country, &["EU", "China", "US", "AU", "EU", "SA", "AR"]))
+        let subnet_mix = new_test_subnet_with_overrides(1, 0, 7, 1, (&NodeFeature::Country, &["AT", "BE", "DE", "ES", "US", "IN", "AR"]))
             .with_subnet_id(PrincipalId::from_str("bkfrj-6k62g-dycql-7h53p-atvkj-zg4to-gaogh-netha-ptybj-ntsgw-rqe").unwrap());
         assert_eq!(
             subnet_mix.check_business_rules().unwrap(),
-            (5000, vec!["European subnet has 5 non-European node(s)".to_string()])
+            (3000, vec!["European subnet has 3 non-European node(s)".to_string()])
         );
     }
 

rs/decentralization/src/network.rs

@@ -3,7 +3,7 @@ use crate::subnets::unhealthy_with_nodes;
 use crate::SubnetChangeResponse;
 use actix_web::http::StatusCode;
 use actix_web::{HttpResponse, ResponseError};
-use ahash::{AHashMap, AHashSet, HashSet};
+use ahash::{AHashMap, AHashSet, HashMap, HashSet};
 use anyhow::anyhow;
 use futures::future::BoxFuture;
 use ic_base_types::PrincipalId;
@@ -14,7 +14,6 @@ use log::{debug, info, warn};
 use rand::{seq::SliceRandom, SeedableRng};
 use serde::{Deserialize, Serialize};
 use std::cmp::Ordering;
-use std::collections::HashMap;
 use std::fmt::{Debug, Display, Formatter};
 use std::hash::Hash;
 
@@ -67,23 +66,9 @@ impl Node {
                 .values()
                 .any(|v| *v.to_lowercase() == *value.to_lowercase())
     }
-}
 
-impl Hash for Node {
-    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
-        self.id.hash(state);
-    }
-}
-
-impl PartialEq for Node {
-    fn eq(&self, other: &Self) -> bool {
-        self.id == other.id
-    }
-}
-
-impl From<&ic_management_types::Node> for Node {
-    fn from(n: &ic_management_types::Node) -> Self {
-        // Work around the current (as of 2024) registry configuration in which the EU countries are not properly marked.
+    pub fn is_country_from_eu(country: &str) -> bool {
+        // (As of 2024) the EU countries are not properly marked in the registry, so we check membership separately.
         let eu_countries: HashMap<&str, &str> = HashMap::from_iter([
             ("AT", "Austria"),
             ("BE", "Belgium"),
@@ -113,6 +98,24 @@ impl From<&ic_management_types::Node> for Node {
             ("SI", "Slovenia"),
             ("SK", "Slovakia"),
         ]);
+        eu_countries.contains_key(country)
+    }
+}
+
+impl Hash for Node {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.id.hash(state);
+    }
+}
+
+impl PartialEq for Node {
+    fn eq(&self, other: &Self) -> bool {
+        self.id == other.id
+    }
+}
+
+impl From<&ic_management_types::Node> for Node {
+    fn from(n: &ic_management_types::Node) -> Self {
         let country = n
             .operator
             .datacenter
@@ -125,10 +128,6 @@ impl From<&ic_management_types::Node> for Node {
             .as_ref()
             .map(|d| d.area.clone())
             .unwrap_or_else(|| "unknown".to_string());
-        let (country, area) = match eu_countries.get(&country.as_str()) {
-            Some(country) => ("EU".to_string(), country.to_string()),
-            None => (country, area),
-        };
 
         Self {
             id: n.principal,
@@ -382,7 +381,7 @@ impl DecentralizedSubnet {
         };
         match nakamoto_scores.feature_value_counts_max(&NodeFeature::Country) {
             Some((name, value)) => {
-                if is_european_subnet && name == "EU" {
+                if is_european_subnet && !Node::is_country_from_eu(name.as_str()) {
                     // European subnet is expected to be controlled by European countries
                 } else if value > max_nodes_per_country {
                     let penalty = (value - max_nodes_per_country) * 10;
@@ -407,8 +406,7 @@ impl DecentralizedSubnet {
             let non_european_nodes_count = country_counts
                 .iter()
                 .filter_map(|(country, count)| {
-                    println!("Country: {} Count: {}", country, count);
-                    if country.as_str() == "EU" || country.as_str() == "CH" {
+                    if Node::is_country_from_eu(country.as_str()) || country.as_str() == "CH" {
                         None
                     } else {
                         Some(*count)