feat(dre): Add business rules logging to subnet change responses (#1076)

dfinity · Nov 13, 2024 · dec9967 · dec9967
1 parent e018947
commit dec9967
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 14 deletions.
diff --git a/rs/decentralization/src/lib.rs b/rs/decentralization/src/lib.rs
@@ -22,6 +22,7 @@ pub struct SubnetChangeResponse {
     pub score_after: nakamoto::NakamotoScore,
     pub penalties_before_change: usize,
     pub penalties_after_change: usize,
+    pub business_rules_log: Vec<String>,
     pub motivation: Option<String>,
     pub comment: Option<String>,
     pub run_log: Option<Vec<String>>,
@@ -47,6 +48,7 @@ impl SubnetChangeResponse {
             score_after: nakamoto::NakamotoScore::new_from_nodes(&change.new_nodes),
             penalties_before_change: change.penalties_before_change,
             penalties_after_change: change.penalties_after_change,
+            business_rules_log: change.business_rules_log.clone(),
             motivation,
             comment: change.comment.clone(),
             run_log: Some(change.run_log.clone()),
@@ -180,8 +182,12 @@ impl Display for SubnetChangeResponse {
 
         writeln!(f, "\n\n```\n{}```\n", table)?;
 
-        if let Some(comment) = &self.comment {
-            writeln!(f, "### Business rules analysis\n{}", comment)?;
+        if !self.business_rules_log.is_empty() {
+            writeln!(
+                f,
+                "### Business rules check results after the membership change\n\n{}",
+                self.business_rules_log.join("\n")
+            )?;
         }
 
         Ok(())

diff --git a/rs/decentralization/src/network.rs b/rs/decentralization/src/network.rs
@@ -1187,9 +1187,9 @@ impl SubnetChangeRequest {
             .map_err(|e| NetworkError::ResizeFailed(e.to_string()))?
             .0;
 
-        let penalties_after_change = DecentralizedSubnet::check_business_rules_for_subnet_with_nodes(&self.subnet.id, &resized_subnet.nodes)
-            .map_err(|e| NetworkError::ResizeFailed(e.to_string()))?
-            .0;
+        let business_rules_check_after_change =
+            DecentralizedSubnet::check_business_rules_for_subnet_with_nodes(&self.subnet.id, &resized_subnet.nodes)
+                .map_err(|e| NetworkError::ResizeFailed(e.to_string()))?;
 
         let subnet_change = SubnetChange {
             subnet_id: self.subnet.id,
@@ -1198,7 +1198,8 @@ impl SubnetChangeRequest {
             removed_nodes: resized_subnet.removed_nodes,
             added_nodes: resized_subnet.added_nodes,
             penalties_before_change,
-            penalties_after_change,
+            penalties_after_change: business_rules_check_after_change.0,
+            business_rules_log: business_rules_check_after_change.1,
             comment: resized_subnet.comment,
             run_log: resized_subnet.run_log,
         };
@@ -1226,6 +1227,7 @@ pub struct SubnetChange {
     pub added_nodes: Vec<Node>,
     pub penalties_before_change: usize,
     pub penalties_after_change: usize,
+    pub business_rules_log: Vec<String>,
     pub comment: Option<String>,
     pub run_log: Vec<String>,
 }

diff --git a/rs/ic-management-backend/src/endpoints/query_decentralization.rs b/rs/ic-management-backend/src/endpoints/query_decentralization.rs
@@ -104,9 +104,9 @@ async fn get_decentralization_analysis(
     let penalties_before_change = DecentralizedSubnet::check_business_rules_for_subnet_with_nodes(&original_subnet.id, &original_subnet.nodes)
         .expect("Business rules check before should succeed")
         .0;
-    let penalties_after_change = DecentralizedSubnet::check_business_rules_for_subnet_with_nodes(&original_subnet.id, &updated_subnet.nodes)
-        .expect("Business rules check after should succeed")
-        .0;
+    let business_rules_check_after_change =
+        DecentralizedSubnet::check_business_rules_for_subnet_with_nodes(&original_subnet.id, &updated_subnet.nodes)
+            .expect("Business rules check after should succeed");
 
     let subnet_change = SubnetChange {
         subnet_id: original_subnet.id,
@@ -115,7 +115,8 @@ async fn get_decentralization_analysis(
         removed_nodes: updated_subnet.removed_nodes.clone(),
         added_nodes: updated_subnet.added_nodes.clone(),
         penalties_before_change,
-        penalties_after_change,
+        penalties_after_change: business_rules_check_after_change.0,
+        business_rules_log: business_rules_check_after_change.1,
         comment: updated_subnet.comment.clone(),
         run_log: updated_subnet.run_log.clone(),
     };

diff --git a/rs/ic-management-backend/src/subnets.rs b/rs/ic-management-backend/src/subnets.rs
@@ -18,17 +18,18 @@ pub fn get_proposed_subnet_changes(
         let penalties_before_change = DecentralizedSubnet::check_business_rules_for_subnet_with_nodes(&subnet.principal, &subnet_nodes)
             .expect("Business rules check should succeed")
             .0;
-        let penalties_after_change = DecentralizedSubnet::check_business_rules_for_subnet_with_nodes(&subnet.principal, &subnet_nodes)
-            .expect("Business rules check should succeed")
-            .0;
+        let business_rules_check_after_change = DecentralizedSubnet::check_business_rules_for_subnet_with_nodes(&subnet.principal, &subnet_nodes)
+            .expect("Business rules check should succeed");
+
         let change = SubnetChange {
             subnet_id: subnet.principal,
             old_nodes: subnet_nodes.clone(),
             new_nodes: subnet_nodes,
             removed_nodes: vec![],
             added_nodes: vec![],
             penalties_before_change,
-            penalties_after_change,
+            penalties_after_change: business_rules_check_after_change.0,
+            business_rules_log: business_rules_check_after_change.1,
             comment: None,
             run_log: vec![],
         }