From 4e352bffaff599cd590e6189490fbeab4c31d5bb Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Tue, 17 May 2022 14:32:52 +0800 Subject: [PATCH] cli: Fix display of staked validators (#25216) --- cli-output/src/cli_output.rs | 39 +++++++++++------------ cli-output/src/cli_version.rs | 60 +++++++++++++++++++++++++++++++++++ cli-output/src/lib.rs | 1 + cli/src/cluster_query.rs | 15 +++++---- cli/src/feature.rs | 57 ++++----------------------------- 5 files changed, 94 insertions(+), 78 deletions(-) create mode 100644 cli-output/src/cli_version.rs diff --git a/cli-output/src/cli_output.rs b/cli-output/src/cli_output.rs index 0e3fd878b..eb2ad8ab0 100644 --- a/cli-output/src/cli_output.rs +++ b/cli-output/src/cli_output.rs @@ -1,5 +1,7 @@ +#![allow(clippy::to_string_in_format_args)] use { crate::{ + cli_version::CliVersion, display::{ build_balance_message, build_balance_message_with_config, format_labeled_address, unix_timestamp_to_string, writeln_name_value, writeln_transaction, @@ -374,7 +376,7 @@ pub struct CliValidators { pub validators_reverse_sort: bool, #[serde(skip_serializing)] pub number_validators: bool, - pub stake_by_version: BTreeMap, + pub stake_by_version: BTreeMap, #[serde(skip_serializing)] pub use_lamports_unit: bool, } @@ -423,7 +425,8 @@ impl fmt::Display for CliValidators { "- ".to_string() }, validator.epoch_credits, - validator.version, + // convert to a string so that fill/alignment works correctly + validator.version.to_string(), build_balance_message_with_config( validator.activated_stake, &BuildBalanceMessageConfig { @@ -442,7 +445,7 @@ impl fmt::Display for CliValidators { 0 }; let header = style(format!( - "{:padding$} {:<44} {:<38} {} {} {} {} {} {} {}", + "{:padding$} {:<44} {:<38} {} {} {} {} {} {} {:>22}", " ", "Identity", "Vote Account", @@ -498,16 +501,9 @@ impl fmt::Display for CliValidators { CliValidatorsSortOrder::Version => { sorted_validators.sort_by(|a, b| { use std::cmp::Ordering; - let a_version = semver::Version::parse(a.version.as_str()).ok(); - let b_version = semver::Version::parse(b.version.as_str()).ok(); - match (a_version, b_version) { - (None, None) => a.version.cmp(&b.version), - (None, Some(_)) => Ordering::Less, - (Some(_), None) => Ordering::Greater, - (Some(va), Some(vb)) => match va.cmp(&vb) { - Ordering::Equal => a.activated_stake.cmp(&b.activated_stake), - ordering => ordering, - }, + match a.version.cmp(&b.version) { + Ordering::Equal => a.activated_stake.cmp(&b.activated_stake), + ordering => ordering, } }); } @@ -592,16 +588,17 @@ impl fmt::Display for CliValidators { writeln!(f)?; writeln!(f, "{}", style("Stake By Version:").bold())?; - for (version, info) in self.stake_by_version.iter() { + for (version, info) in self.stake_by_version.iter().rev() { writeln!( f, - "{:<8} - {:4} current validators ({:>5.2}%){}", - version, + "{:<7} - {:4} current validators ({:>5.2}%){}", + // convert to a string so that fill/alignment works correctly + version.to_string(), info.current_validators, 100. * info.current_active_stake as f64 / self.total_active_stake as f64, if info.delinquent_validators > 0 { format!( - ", {:3} delinquent validators ({:>5.2}%)", + " {:3} delinquent validators ({:>5.2}%)", info.delinquent_validators, 100. * info.delinquent_active_stake as f64 / self.total_active_stake as f64 ) @@ -626,7 +623,7 @@ pub struct CliValidator { pub credits: u64, // lifetime credits pub epoch_credits: u64, // credits earned in the current epoch pub activated_stake: u64, - pub version: String, + pub version: CliVersion, pub delinquent: bool, pub skip_rate: Option, } @@ -635,7 +632,7 @@ impl CliValidator { pub fn new( vote_account: &RpcVoteAccountInfo, current_epoch: Epoch, - version: String, + version: CliVersion, skip_rate: Option, address_labels: &HashMap, ) -> Self { @@ -652,7 +649,7 @@ impl CliValidator { pub fn new_delinquent( vote_account: &RpcVoteAccountInfo, current_epoch: Epoch, - version: String, + version: CliVersion, skip_rate: Option, address_labels: &HashMap, ) -> Self { @@ -669,7 +666,7 @@ impl CliValidator { fn _new( vote_account: &RpcVoteAccountInfo, current_epoch: Epoch, - version: String, + version: CliVersion, skip_rate: Option, address_labels: &HashMap, delinquent: bool, diff --git a/cli-output/src/cli_version.rs b/cli-output/src/cli_version.rs new file mode 100644 index 000000000..b163f99bc --- /dev/null +++ b/cli-output/src/cli_version.rs @@ -0,0 +1,60 @@ +use { + serde::{Deserialize, Deserializer, Serialize, Serializer}, + std::{fmt, str::FromStr}, +}; + +#[derive(Default, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Clone)] +pub struct CliVersion(Option); + +impl CliVersion { + pub fn unknown_version() -> Self { + Self(None) + } +} + +impl fmt::Display for CliVersion { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let s = match &self.0 { + None => "unknown".to_string(), + Some(version) => version.to_string(), + }; + write!(f, "{}", s) + } +} + +impl From for CliVersion { + fn from(version: semver::Version) -> Self { + Self(Some(version)) + } +} + +impl FromStr for CliVersion { + type Err = semver::Error; + fn from_str(s: &str) -> Result { + let version_option = if s == "unknown" { + None + } else { + Some(semver::Version::from_str(s)?) + }; + Ok(CliVersion(version_option)) + } +} + +impl Serialize for CliVersion { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&self.to_string()) + } +} + +impl<'de> Deserialize<'de> for CliVersion { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s: &str = Deserialize::deserialize(deserializer)?; + CliVersion::from_str(s).map_err(serde::de::Error::custom) + } +} diff --git a/cli-output/src/lib.rs b/cli-output/src/lib.rs index 9339ba810..2b413716d 100644 --- a/cli-output/src/lib.rs +++ b/cli-output/src/lib.rs @@ -1,5 +1,6 @@ #![allow(clippy::integer_arithmetic)] mod cli_output; +pub mod cli_version; pub mod display; pub use cli_output::*; diff --git a/cli/src/cluster_query.rs b/cli/src/cluster_query.rs index bf6a131a8..f39b64cde 100644 --- a/cli/src/cluster_query.rs +++ b/cli/src/cluster_query.rs @@ -14,6 +14,7 @@ use { offline::{blockhash_arg, BLOCKHASH_ARG}, }, solana_cli_output::{ + cli_version::CliVersion, display::{ build_balance_message, format_labeled_address, new_spinner_progress_bar, println_transaction, unix_timestamp_to_string, writeln_name_value, @@ -1872,13 +1873,13 @@ pub fn process_show_validators( progress_bar.set_message("Fetching version information..."); let mut node_version = HashMap::new(); - let unknown_version = "unknown".to_string(); for contact_info in rpc_client.get_cluster_nodes()? { node_version.insert( contact_info.pubkey, contact_info .version - .unwrap_or_else(|| unknown_version.clone()), + .and_then(|version| CliVersion::from_str(&version).ok()) + .unwrap_or_else(CliVersion::unknown_version), ); } @@ -1907,8 +1908,8 @@ pub fn process_show_validators( epoch_info.epoch, node_version .get(&vote_account.node_pubkey) - .unwrap_or(&unknown_version) - .clone(), + .cloned() + .unwrap_or_else(CliVersion::unknown_version), skip_rate.get(&vote_account.node_pubkey).cloned(), &config.address_labels, ) @@ -1923,15 +1924,15 @@ pub fn process_show_validators( epoch_info.epoch, node_version .get(&vote_account.node_pubkey) - .unwrap_or(&unknown_version) - .clone(), + .cloned() + .unwrap_or_else(CliVersion::unknown_version), skip_rate.get(&vote_account.node_pubkey).cloned(), &config.address_labels, ) }) .collect(); - let mut stake_by_version: BTreeMap<_, CliValidatorsStakeByVersion> = BTreeMap::new(); + let mut stake_by_version: BTreeMap = BTreeMap::new(); for validator in current_validators.iter() { let mut entry = stake_by_version .entry(validator.version.clone()) diff --git a/cli/src/feature.rs b/cli/src/feature.rs index eb13284a1..cef10916e 100644 --- a/cli/src/feature.rs +++ b/cli/src/feature.rs @@ -5,9 +5,9 @@ use { }, clap::{App, AppSettings, Arg, ArgMatches, SubCommand}, console::style, - serde::{Deserialize, Deserializer, Serialize, Serializer}, + serde::{Deserialize, Serialize}, solana_clap_utils::{input_parsers::*, input_validators::*, keypair::*}, - solana_cli_output::{QuietDisplay, VerboseDisplay}, + solana_cli_output::{cli_version::CliVersion, QuietDisplay, VerboseDisplay}, solana_client::{client_error::ClientError, rpc_client::RpcClient}, solana_remote_wallet::remote_wallet::RemoteWalletManager, solana_sdk::{ @@ -398,50 +398,6 @@ pub struct CliSoftwareVersionStats { rpc_percent: f32, } -#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Clone)] -struct CliVersion(Option); - -impl fmt::Display for CliVersion { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let s = match &self.0 { - None => "unknown".to_string(), - Some(version) => version.to_string(), - }; - write!(f, "{}", s) - } -} - -impl FromStr for CliVersion { - type Err = semver::Error; - fn from_str(s: &str) -> Result { - let version_option = if s == "unknown" { - None - } else { - Some(semver::Version::from_str(s)?) - }; - Ok(CliVersion(version_option)) - } -} - -impl Serialize for CliVersion { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_str(&self.to_string()) - } -} - -impl<'de> Deserialize<'de> for CliVersion { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let s: &str = Deserialize::deserialize(deserializer)?; - CliVersion::from_str(s).map_err(serde::de::Error::custom) - } -} - pub trait FeatureSubCommands { fn feature_subcommands(self) -> Self; } @@ -634,7 +590,8 @@ fn cluster_info_stats(rpc_client: &RpcClient) -> Result>(); @@ -661,7 +618,7 @@ fn cluster_info_stats(rpc_client: &RpcClient) -> Result