From 3a2b8c5e5b5cccb058bc32abbb1e51d01d20c3e8 Mon Sep 17 00:00:00 2001
From: Michael Vines <mvines@gmail.com>
Date: Thu, 24 Sep 2020 13:30:38 -0700
Subject: [PATCH] cli review feedback

---
 cli/src/cli.rs     | 10 ++++------
 cli/src/feature.rs | 20 ++++++++++++--------
 version/src/lib.rs |  2 +-
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/cli/src/cli.rs b/cli/src/cli.rs
index 8cbce97cc..5565f9142 100644
--- a/cli/src/cli.rs
+++ b/cli/src/cli.rs
@@ -523,11 +523,6 @@ pub fn parse_command(
     wallet_manager: &mut Option<Arc<RemoteWalletManager>>,
 ) -> Result<CliCommandInfo, Box<dyn error::Error>> {
     let response = match matches.subcommand() {
-        // Feature subcommand
-        ("feature", Some(matches)) => {
-            feature_parse_subcommand(matches, default_signer, wallet_manager)
-        }
-
         // Cluster Query Commands
         ("catchup", Some(matches)) => parse_catchup(matches, wallet_manager),
         ("cluster-date", Some(_matches)) => Ok(CliCommandInfo {
@@ -541,6 +536,9 @@ pub fn parse_command(
         ("create-address-with-seed", Some(matches)) => {
             parse_create_address_with_seed(matches, default_signer, wallet_manager)
         }
+        ("feature", Some(matches)) => {
+            parse_feature_subcommand(matches, default_signer, wallet_manager)
+        }
         ("fees", Some(_matches)) => Ok(CliCommandInfo {
             command: CliCommand::Fees,
             signers: vec![],
@@ -1962,9 +1960,9 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, '
                 ),
         )
         .cluster_query_subcommands()
+        .feature_subcommands()
         .nonce_subcommands()
         .stake_subcommands()
-        .feature_subcommands()
         .subcommand(
             SubCommand::with_name("airdrop")
                 .about("Request lamports")
diff --git a/cli/src/feature.rs b/cli/src/feature.rs
index 8e3ed826a..4626c1495 100644
--- a/cli/src/feature.rs
+++ b/cli/src/feature.rs
@@ -5,7 +5,7 @@ use crate::{
 use clap::{App, AppSettings, Arg, ArgMatches, SubCommand};
 use console::style;
 use solana_clap_utils::{input_parsers::*, input_validators::*, keypair::*};
-use solana_client::rpc_client::RpcClient;
+use solana_client::{client_error::ClientError, rpc_client::RpcClient};
 use solana_remote_wallet::remote_wallet::RemoteWalletManager;
 use solana_runtime::{
     feature::{self, Feature},
@@ -35,11 +35,12 @@ impl FeatureSubCommands for App<'_, '_> {
                     SubCommand::with_name("status")
                         .about("Query runtime feature status")
                         .arg(
-                            Arg::with_name("feature")
+                            Arg::with_name("features")
                                 .value_name("ADDRESS")
                                 .validator(is_valid_pubkey)
                                 .index(1)
-                                .help("Feature status to query [default: "),
+                                .multiple(true)
+                                .help("Feature status to query [default: all known features]"),
                         ),
                 )
                 .subcommand(
@@ -50,6 +51,7 @@ impl FeatureSubCommands for App<'_, '_> {
                                 .value_name("FEATURE_KEYPAIR")
                                 .validator(is_valid_signer)
                                 .index(1)
+                                .required(true)
                                 .help("The signer for the feature to activate"),
                         ),
                 ),
@@ -68,7 +70,7 @@ fn known_feature(feature: &Pubkey) -> Result<(), CliError> {
     }
 }
 
-pub fn feature_parse_subcommand(
+pub fn parse_feature_subcommand(
     matches: &ArgMatches<'_>,
     default_signer: &DefaultSigner,
     wallet_manager: &mut Option<Arc<RemoteWalletManager>>,
@@ -88,9 +90,11 @@ pub fn feature_parse_subcommand(
             }
         }
         ("status", Some(matches)) => {
-            let mut features = if let Some(feature) = pubkey_of(matches, "feature") {
-                known_feature(&feature)?;
-                vec![feature]
+            let mut features = if let Some(features) = pubkeys_of(matches, "features") {
+                for feature in &features {
+                    known_feature(feature)?;
+                }
+                features
             } else {
                 FEATURE_NAMES.keys().cloned().collect()
             };
@@ -117,7 +121,7 @@ pub fn process_feature_subcommand(
 }
 
 // Feature activation is only allowed when 95% of the active stake is on the current feature set
-fn feature_activation_allowed(rpc_client: &RpcClient) -> Result<bool, Box<dyn std::error::Error>> {
+fn feature_activation_allowed(rpc_client: &RpcClient) -> Result<bool, ClientError> {
     let my_feature_set = solana_version::Version::default().feature_set;
 
     let feature_set_map = rpc_client
diff --git a/version/src/lib.rs b/version/src/lib.rs
index 42cb11fc1..6b21555de 100644
--- a/version/src/lib.rs
+++ b/version/src/lib.rs
@@ -7,7 +7,7 @@ use std::{convert::TryInto, fmt};
 #[macro_use]
 extern crate solana_sdk_macro_frozen_abi;
 
-// Older version structure used by 1.3.12 and earlier releases
+// Older version structure used earlier 1.3.x releases
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, AbiExample)]
 pub struct LegacyVersion {
     major: u16,