feat(dre): Improve the summary of the subnet resize NNS proposals (#924)

rs/cli/src/commands/subnet/resize.rs

@@ -7,11 +7,11 @@ use crate::commands::{AuthRequirement, ExecutableCommand};
 #[derive(Args, Debug)]
 pub struct Resize {
     /// Number of nodes to be added
-    #[clap(long)]
+    #[clap(long, default_value = "0")]
     pub add: usize,
 
     /// Number of nodes to be removed
-    #[clap(long)]
+    #[clap(long, default_value = "0")]
     pub remove: usize,
 
     /// Features or Node IDs to exclude from the available nodes pool
@@ -32,7 +32,7 @@ pub struct Resize {
     pub motivation: String,
 
     /// The ID of the subnet.
-    #[clap(long, short)]
+    #[clap(long, short, alias = "subnet-id")]
     pub id: PrincipalId,
 }
 
@@ -43,7 +43,10 @@ impl ExecutableCommand for Resize {
 
     async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> {
         let runner = ctx.runner().await?;
-        runner
+
+        let subnet_manager = ctx.subnet_manager().await;
+
+        let subnet_change_response = subnet_manager
             .subnet_resize(
                 SubnetResizeRequest {
                     subnet: self.id,
@@ -54,10 +57,11 @@ impl ExecutableCommand for Resize {
                     include: self.include.clone().into(),
                 },
                 self.motivation.clone(),
-                ctx.forum_post_link(),
                 &runner.health_of_nodes().await?,
             )
-            .await
+            .await?;
+
+        runner.propose_subnet_change(subnet_change_response, ctx.forum_post_link()).await
     }
 
     fn validate(&self, _cmd: &mut clap::Command) {}

rs/cli/src/runner.rs

@@ -124,54 +124,6 @@ impl Runner {
         health_client.nodes().await
     }
 
-    pub async fn subnet_resize(
-        &self,
-        request: ic_management_types::requests::SubnetResizeRequest,
-        motivation: String,
-        forum_post_link: Option<String>,
-        health_of_nodes: &IndexMap<PrincipalId, HealthStatus>,
-    ) -> anyhow::Result<()> {
-        let change = self
-            .registry
-            .modify_subnet_nodes(SubnetQueryBy::SubnetId(request.subnet))
-            .await?
-            .excluding_from_available(request.exclude.clone().unwrap_or_default())
-            .including_from_available(request.only.clone().unwrap_or_default())
-            .including_from_available(request.include.clone().unwrap_or_default())
-            .resize(request.add, request.remove, 0, health_of_nodes)?;
-
-        let change = SubnetChangeResponse::from(&change).with_health_of_nodes(health_of_nodes.clone());
-
-        if self.verbose {
-            if let Some(run_log) = &change.run_log {
-                println!("{}\n", run_log.join("\n"));
-            }
-        }
-
-        if change.added_with_desc.is_empty() && change.removed_with_desc.is_empty() {
-            return Ok(());
-        }
-        if change.added_with_desc.len() == change.removed_with_desc.len() {
-            self.run_membership_change(change.clone(), replace_proposal_options(&change, forum_post_link)?)
-                .await
-        } else {
-            let action = if change.added_with_desc.len() < change.removed_with_desc.len() {
-                "Removing nodes from"
-            } else {
-                "Adding nodes to"
-            };
-            self.run_membership_change(
-                change,
-                ProposeOptions {
-                    title: format!("{action} subnet {}", request.subnet).into(),
-                    summary: format!("{action} subnet {}", request.subnet).into(),
-                    motivation: motivation.clone().into(),
-                    forum_post_link,
-                },
-            )
-            .await
-        }
-    }
     pub async fn subnet_create(
         &self,
         request: ic_management_types::requests::SubnetCreateRequest,

rs/cli/src/subnet_manager.rs

@@ -10,9 +10,11 @@ use decentralization::{
 };
 use ic_management_backend::health::{self, HealthStatusQuerier};
 use ic_management_backend::lazy_registry::LazyRegistry;
+use ic_management_types::HealthStatus;
 use ic_management_types::MinNakamotoCoefficients;
 use ic_management_types::Network;
 use ic_types::PrincipalId;
+use indexmap::IndexMap;
 use log::{info, warn};
 
 #[derive(Clone)]
@@ -181,4 +183,42 @@ impl SubnetManager {
 
         Ok(change)
     }
+
+    pub async fn subnet_resize(
+        &self,
+        request: ic_management_types::requests::SubnetResizeRequest,
+        proposal_motivation: String,
+        health_of_nodes: &IndexMap<PrincipalId, HealthStatus>,
+    ) -> anyhow::Result<SubnetChangeResponse> {
+        let registry = self.registry_instance.clone();
+        let mut motivations = vec![];
+
+        let change = registry
+            .modify_subnet_nodes(SubnetQueryBy::SubnetId(request.subnet))
+            .await?
+            .excluding_from_available(request.exclude.clone().unwrap_or_default())
+            .including_from_available(request.only.clone().unwrap_or_default())
+            .including_from_available(request.include.clone().unwrap_or_default())
+            .resize(request.add, request.remove, 0, health_of_nodes)?;
+
+        for (n, _) in change.removed().iter() {
+            motivations.push(format!("removing {} as per user request", n.id));
+        }
+
+        for (n, _) in change.added().iter() {
+            motivations.push(format!("adding {} as per user request", n.id));
+        }
+
+        let motivation = format!(
+                "{}\n{}\n\nNOTE: The information below is provided for your convenience. Please independently verify the decentralization changes rather than relying solely on this summary.\nCode for calculating replacements is at https://github.com/dfinity/dre/blob/79066127f58c852eaf4adda11610e815a426878c/rs/decentralization/src/network.rs#L912",
+                proposal_motivation,
+                motivations.iter().map(|s| format!(" - {}", s)).collect::<Vec<String>>().join("\n")
+            );
+
+        let change = SubnetChangeResponse::from(&change)
+            .with_health_of_nodes(health_of_nodes.clone())
+            .with_motivation(motivation);
+
+        Ok(change)
+    }
 }